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 build issues for LLVM based toolchains. #1513

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

parona-source
Copy link

@parona-source parona-source commented Dec 27, 2024

Keeping it a draft as this would in its current state break builds for Windows. I'm unsure what is the intent with mangohud_opengl and what symbols should be visible in it for Windows.

With LLVM toolchain I mean clang as the compiler, lld is the linker and libcxx as the c++ stdlib.

I didn't touch mangohud_shim, but its visible symbols are shown as well. It could also be explicitly specified like mangohud_opengl.

$ nm -UD build/src/libMangoHud*.so
build/src/libMangoHud_dlsym.so:
0000000000002b20 T dlsym

build/src/libMangoHud_opengl.so:
0000000000054c10 T eglGetDisplay
0000000000054b60 T eglGetPlatformDisplay
0000000000054ce0 T eglGetProcAddress
00000000000549b0 T eglSwapBuffers
0000000000055e40 T glXCreateContext
0000000000055f20 T glXCreateContextAttribs
0000000000056010 T glXCreateContextAttribsARB
0000000000056100 T glXDestroyContext
0000000000055c20 T glXGetProcAddress
0000000000055d30 T glXGetProcAddressARB
00000000000565b0 T glXGetSwapIntervalMESA
00000000000561b0 T glXMakeCurrent
0000000000055830 T glXSwapBuffers
0000000000055720 T glXSwapBuffersMscOML
0000000000056360 T glXSwapIntervalEXT
00000000000564f0 T glXSwapIntervalMESA
0000000000056430 T glXSwapIntervalSGI
0000000000054520 T mangohud_find_egl_ptr
00000000000558f0 T mangohud_find_glx_ptr

build/src/libMangoHud_shim.so:
00000000000012e0 T eglGetDisplay
0000000000001340 T eglGetPlatformDisplay
00000000000013c0 T eglSwapBuffers
0000000000001280 T glXSwapBuffers
0000000000001420 T glXSwapBuffersMscOML

build/src/libMangoHud.so:
000000000004eb80 T overlay_GetDeviceProcAddr
000000000004f960 T overlay_GetInstanceProcAddr

Current HEAD built with GCC/ld.bfd
$ nm -UD build/src/libMangoHud*.so

build/src/libMangoHud_dlsym.so:
0000000000002b20 T dlsym

build/src/libMangoHud_opengl.so:
0000000000054c10 T eglGetDisplay
0000000000054b60 T eglGetPlatformDisplay
0000000000054ce0 T eglGetProcAddress
00000000000549b0 T eglSwapBuffers
0000000000055e40 T glXCreateContext
0000000000055f20 T glXCreateContextAttribs
0000000000056010 T glXCreateContextAttribsARB
0000000000056100 T glXDestroyContext
0000000000055c20 T glXGetProcAddress
0000000000055d30 T glXGetProcAddressARB
00000000000565b0 T glXGetSwapIntervalMESA
00000000000561b0 T glXMakeCurrent
0000000000055830 T glXSwapBuffers
0000000000055720 T glXSwapBuffersMscOML
0000000000056360 T glXSwapIntervalEXT
00000000000564f0 T glXSwapIntervalMESA
0000000000056430 T glXSwapIntervalSGI
0000000000054520 T mangohud_find_egl_ptr
00000000000558f0 T mangohud_find_glx_ptr

build/src/libMangoHud_shim.so:
00000000000012e0 T eglGetDisplay
0000000000001340 T eglGetPlatformDisplay
00000000000013c0 T eglSwapBuffers
0000000000001280 T glXSwapBuffers
0000000000001420 T glXSwapBuffersMscOML

build/src/libMangoHud.so:
000000000004eb80 T overlay_GetDeviceProcAddr
000000000004f960 T overlay_GetInstanceProcAddr

@flightlessmango
Copy link
Owner

mangohud_opengl has no purpose on windows, you can freely change so it's not compiled anymore in windows builds

@parona-source parona-source marked this pull request as ready for review December 27, 2024 12:24
if is_libcxx
message('Detected libcxx, not appending -static-libstc++ to linker arguments')
else
link_args+=['-static-libstdc++']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flightlessmango can't this be left for packagers / users to add with meson with -D cpp_link_args when/if needed ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a case where its enabled by default and you have to disable it yourself for the cases where its not desirable (libcxx / no static libstdc++ available).

@@ -211,11 +209,25 @@ mangohud_shared_lib = shared_library(
'MangoHud',
objects: mangohud_static_lib.extract_all_objects(),
link_with: mangohud_static_lib,
link_args : link_args,
link_args : [link_args, '-Wl,--version-script,@0@'.format(join_paths(meson.current_source_dir(), 'mangohud.version'))],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot of duplication, should probably be a variable

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about how to do that nicer, I have unique files for all libs and mangohub_opengl uses a different directory as its generated at build time.

Could also break it into several lines like in the example given in this meson issue about improving mesons handling for these.

mesonbuild/meson#3047

LLD is stricter with version scripts and will complain about nonexisting
symbols in version files.

Signed-off-by: Alfred Wingate <[email protected]>
Clang has a long standing bug where it will try to mix c++ libraries
instead of erroring out early or transparently linking against libcxx
statically instead.

Signed-off-by: Alfred Wingate <[email protected]>
LLVM alternative C++ standard library is happier if it can hide its own
symbols first without preprocessor macros complicating things.

FAILED: amdgpu.p/tests_test_amdgpu.cpp.o
clang++ -Iamdgpu.p -I. -I.. -I../include -I../subprojects/spdlog-1.14.1/include -Isubprojects/implot-0.16 -I../subprojects/implot-0.16 -Isubprojects/imgui-1.89.9 -I../subprojects/imgui-1.89.9 -I../subprojects/imgui-1.89.9/backends -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=c++14 -O3 -Werror=missing-declarations -Werror=return-type -Wno-unused-parameter -Qunused-arguments -fno-math-errno -fno-trapping-math -Wno-non-virtual-dtor -Wno-missing-field-initializers -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS '-DPACKAGE_VERSION="v0.7.2"' -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG -D_GNU_SOURCE -DHAVE_PTHREAD -DUSE_GCC_ATOMIC_BUILTINS -DHAVE_TIMESPEC_GET -DHAVE___BUILTIN_BSWAP32 -DHAVE___BUILTIN_BSWAP64 -DHAVE___BUILTIN_CLZ -DHAVE___BUILTIN_CLZLL -DHAVE___BUILTIN_CTZ -DHAVE___BUILTIN_EXPECT -DHAVE___BUILTIN_FFS -DHAVE___BUILTIN_FFSLL -DHAVE___BUILTIN_POPCOUNT -DHAVE___BUILTIN_POPCOUNTLL -DHAVE___BUILTIN_UNREACHABLE -O2 -pipe '-DIMGUI_API=__attribute__((visibility("default")))' -pthread -DSPDLOG_COMPILED_LIB -DTEST_ONLY -MD -MQ amdgpu.p/tests_test_amdgpu.cpp.o -MF amdgpu.p/tests_test_amdgpu.cpp.o.d -o amdgpu.p/tests_test_amdgpu.cpp.o -c ../tests/test_amdgpu.cpp
In file included from ../tests/test_amdgpu.cpp:9:
In file included from ../tests/../src/amdgpu.h:6:
In file included from ../tests/../src/overlay_params.h:6:
In file included from /usr/include/c++/v1/vector:3023:
In file included from /usr/include/c++/v1/locale:212:
/usr/include/c++/v1/ios:343:30: error: expected parameter declarator
  343 |   _LIBCPP_HIDE_FROM_ABI bool fail() const;
      |                              ^
/usr/include/cmocka.h:1616:22: note: expanded from macro 'fail'
 1616 | #define fail() _fail(__FILE__, __LINE__)
      |                      ^
<scratch space>:2:1: note: expanded from here
    2 | "/usr/include/c++/v1/ios"

Signed-off-by: Alfred Wingate <[email protected]>
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.

3 participants