Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and enable -fvisibility=hidden by default when building with gcc #68

Open
wants to merge 2 commits into
base: git_master
Choose a base branch
from

Conversation

ldv-alt
Copy link

@ldv-alt ldv-alt commented Oct 19, 2018

This short series fixes build with gcc -fvisibility=hidden, and enables -fvisibility=hidden by default when building with gcc. As result, 168 internal symbols are no longer exported by the shared library.

FGAPI used to be defined to an empty body on non-windows platforms.
Provide a proper definition for gcc to fix build with gcc -fvisibility=hidden.

Add FGAPI attribute to definitions of glutStroke* and glutBitmap* variables,
to make them exported when gcc -fvisibility=hidden is in effect.
This effectively unexports the following 168 internal symbols:
B fgDisplay
B fgJoystick
B fgStructure
B fghBindBuffer
B fghBufferData
B fghDeleteBuffers
B fghDisableVertexAttribArray
B fghEnableVertexAttribArray
B fghGenBuffers
B fghVertexAttribPointer
B sball_initialized
D fgFontFixed8x13
D fgFontFixed9x15
D fgFontHelvetica10
D fgFontHelvetica12
D fgFontHelvetica18
D fgFontTimesRoman10
D fgFontTimesRoman24
D fgState
D fgStrokeMonoRoman
D fgStrokeRoman
D xi_opcode
T XParseGeometry
T fgAddToWindowDestroyList
T fgCheckActiveMenu
T fgCloseWindow
T fgCloseWindows
T fgCreateMenu
T fgCreateStructure
T fgCreateWindow
T fgDeactivateMenu
T fgDeinitialize
T fgDestroyMenu
T fgDestroyStructure
T fgDestroyWindow
T fgDisplayMenu
T fgElapsedTime
T fgEnumMenus
T fgEnumSubWindows
T fgEnumWindows
T fgError
T fgGetActiveMenu
T fgHandleExtensionEvents
T fgHasSpaceball
T fgHintPresent
T fgInitGL2
T fgInitialiseInputDevices
T fgInitialiseJoysticks
T fgInitialiseSpaceball
T fgInputDeviceClose
T fgInputDeviceDetect
T fgIsSpaceballXEvent
T fgJoystickClose
T fgJoystickDetect
T fgJoystickPollWindow
T fgListAppend
T fgListInit
T fgListInsert
T fgListLength
T fgListRemove
T fgMenuByID
T fgOpenWindow
T fgPlatformChangeDisplayMode
T fgPlatformCloseDisplay
T fgPlatformCloseWindow
T fgPlatformCreateWindow
T fgPlatformDeinitialiseInputDevices
T fgPlatformDestroyContext
T fgPlatformEnterGameMode
T fgPlatformFullScreenToggle
T fgPlatformGetConfig
T fgPlatformGetGLUTProcAddress
T fgPlatformGetGameModeVMaxExtent
T fgPlatformGetModifiers
T fgPlatformGetProcAddress
T fgPlatformGlutDeviceGet
T fgPlatformGlutGet
T fgPlatformGlutGetModeValues
T fgPlatformGlutSetIconTitle
T fgPlatformGlutSetWindowTitle
T fgPlatformGlutSwapBuffers
T fgPlatformHasSpaceball
T fgPlatformHideWindow
T fgPlatformIconifyWindow
T fgPlatformInitWork
T fgPlatformInitialize
T fgPlatformInitializeSpaceball
T fgPlatformJoystickClose
T fgPlatformJoystickInit
T fgPlatformJoystickOpen
T fgPlatformJoystickRawRead
T fgPlatformLeaveGameMode
T fgPlatformMainLoopPreliminaryWork
T fgPlatformOpenWindow
T fgPlatformPopWindow
T fgPlatformPosResZordWork
T fgPlatformPositionWindow
T fgPlatformProcessSingleEvent
T fgPlatformPushWindow
T fgPlatformRegisterDialDevice
T fgPlatformRememberState
T fgPlatformReshapeWindow
T fgPlatformRestoreState
T fgPlatformSetCursor
T fgPlatformSetWindow
T fgPlatformShowWindow
T fgPlatformSleepForEvents
T fgPlatformSpaceballClose
T fgPlatformSpaceballNumButtons
T fgPlatformSpaceballSetWindow
T fgPlatformSystemTime
T fgPlatformVisibilityWork
T fgPlatformWarpPointer
T fgPrintXIDeviceEvent
T fgPrintXILeaveEvent
T fgProcessWork
T fgRegisterDevices
T fgSetCursor
T fgSetWindow
T fgSpaceballClose
T fgSpaceballHandleXEvent
T fgSpaceballNumButtons
T fgSpaceballSetWindow
T fgSystemTime
T fgUpdateMenuHighlight
T fgWarning
T fgWindowByHandle
T fgWindowByID
T fghCalculateMenuBoxSize
T fghChooseConfig
T fghCloseInputDevices
T fghContextCreationError
T fghCreateNewContext
T fghDefaultReshape
T fghDrawGeometrySolid
T fghDrawGeometryWire
T fghFontByID
T fghGenerateCone
T fghGenerateCylinder
T fghGenerateTorus
T fghIsLegacyContextRequested
T fghMapBit
T fghNumberOfAuxBuffersRequested
T fghOnPositionNotify
T fghOnReshapeNotify
T fghParseCommandLineArguments
T fghPlatformGetCursorPos
T fghPlatformGlutGetGLX
T fghRedrawWindow
T fghRedrawWindowAndChildren
T glutJoystickGetCenter
T glutJoystickGetDeadBand
T glutJoystickGetMaxRange
T glutJoystickGetMinRange
T glutJoystickGetNumAxes
T glutJoystickGetNumButtons
T glutJoystickGetSaturation
T glutJoystickNotWorking
T glutJoystickSetCenter
T glutJoystickSetDeadBand
T glutJoystickSetMaxRange
T glutJoystickSetMinRange
T glutJoystickSetSaturation
T serial_close
T serial_flush
T serial_getchar
T serial_open
T serial_putchar
@rcmaniac25
Copy link
Contributor

So the one question I see is that you mark the font definitions in X11 l with FGAPI which, if I understand, will not be exported in the shared library. So how would a program link against them with GCC using X11?

@rcmaniac25
Copy link
Contributor

I also just realized, you only do this for GCC. Is it not possible for Clang or whatever non-Windows compiler is used?

@ldv-alt
Copy link
Author

ldv-alt commented Oct 21, 2018

Just the otherwise, I mark these font variables with FGAPI to make them exported.

@ldv-alt
Copy link
Author

ldv-alt commented Oct 21, 2018

This should be possible for clang and maybe other compilers.
My patch enables this feature just for gcc because I use gcc.
Enabling it for other compilers in a portable way would require some cmake magic, but I cannot volunteer to do it.

@dcnieho
Copy link
Collaborator

dcnieho commented Aug 13, 2019

@jtsiomb, should this be merged?

@jtsiomb
Copy link
Member

jtsiomb commented Aug 14, 2019

I don't see any harm in hiding any symbols which aren't part of the API. The only potential issue, is breaking programs which use some internal freeglut symbol to augment the functionality. For example using, fgDisplay to gain access to our connection to the X server. I looked at the list of symbols that will stop being visible, and I can't think of anything else someone might want to use, so I'd say just adding an FGAPI to fgDisplay just to avoid any such breakage.

Also in the list of symbols that will become unaccessible with this patch, I see a bunch of glut-prefixed joystick functions. I assume these should have the FGAPI prefix too.

@dcnieho
Copy link
Collaborator

dcnieho commented Sep 3, 2019

Ok, shall i merge and add FGAPI to fgDisplay?

@ldv-alt
Copy link
Author

ldv-alt commented Sep 3, 2019 via email

@jtsiomb
Copy link
Member

jtsiomb commented Sep 3, 2019

I don't mind if FGAPI is added to fgDisplay if you suppose it might be needed elsewhere.

Yes, I'd say let's play it safe and add FGAPI to fgDisplay, and also to the joystick functions which for some reason don't already have it. Otherwise I'm fine with merging this.

@jtsiomb
Copy link
Member

jtsiomb commented Apr 8, 2022

I see this was never merged. But I'm also having second thoughts about whether this should be merged.

I'm slightly nervous about having the public FGAPI definition in freeglut_std.h changed to add the visibility attribute. This should probably be something that affects only freeglut's build, not user programs. Maybe it should be hidden behind something like the windows FREEGLUT_EXPORTS macro used to select between dllexport and dllimport.

I'm also not sure if the #ifdef __GNUC__ block is guaranteed to be sufficient for the default visibility attribute to be added for all compilers where cmake decided to hide the symbols based on the CMAKE_C_VISIBILITY_PRESET setting.

@kusma
Copy link

kusma commented Sep 8, 2023

For the visibility thing, using __has_attribute is probably the right approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants