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

Linking fails if SPIRV-Tools is installed separately and found as a cmake package #2358

Closed
manuelmohr opened this issue Feb 13, 2024 · 6 comments

Comments

@manuelmohr
Copy link

Linking fails if SPIRV-Tools is compiled first, installed, and found as a cmake package by SPIRV-LLVM-Translator.

I build both SPIRV-LLVM-Translator and SPIRV-Tools within a standard Ubuntu 22.04 environment.

The used versions are:

  • LLVM 16.0.6 pre-installed
  • SPIRV-LLVM-Translator v16.0.0
  • SPIRV-Tools v2023.2

If I build and install SPIRV-Tools first, and try to build SPIRV-LLVM-Translator next, cmake will successfully find the SPIRV-Tools cmake package and set up include paths and linker flags appropriately. But the final link step will fail:

/usr/bin/ld: tools/llvm-spirv/CMakeFiles/llvm-spirv.dir/llvm-spirv.cpp.o: in function `main':
llvm-spirv.cpp:(.text.startup.main+0x2732): undefined reference to `spvtools::SpirvTools::SpirvTools(spv_target_env)'
/usr/bin/ld: llvm-spirv.cpp:(.text.startup.main+0x276d): undefined reference to `spvtools::SpirvTools::SetMessageConsumer(std::function<void (spv_message_level_t, char const*, spv_position_t const&, char const*)>)'
/usr/bin/ld: llvm-spirv.cpp:(.text.startup.main+0x28d6): undefined reference to `spvtools::SpirvTools::Disassemble(unsigned int const*, unsigned long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, unsigned int) const'
/usr/bin/ld: llvm-spirv.cpp:(.text.startup.main+0x2961): undefined reference to `spvtools::SpirvTools::~SpirvTools()'
collect2: error: ld returned 1 exit status

After some investigation, I think this is because SPIRV-LLVM-Translator tries linking against SPIRV-Tools-shared, which has default-hidden symbol visibility, but SPIRV-LLVM-Translator references symbols that are regarded as private. Hence, linking fails.

From my current understanding, only the symbols marked as SPIRV_TOOLS_EXPORT in https://github.com/KhronosGroup/SPIRV-Tools/blob/main/include/spirv-tools/libspirv.h are accessible when linking against SPIRV-Tools-shared, hence the current usage here seems wrong (and it should be SPIRV-Tools-static instead).

If I change the cmake config to link against SPIRV-Tools-static instead (which has full public symbol visibility), linking succeeds.

See also KhronosGroup/SPIRV-Tools#3909 and https://github.com/KhronosGroup/SPIRV-Tools/blob/b7413609cf2d5adccc877d71a1d67ec43bea07c0/CMakeLists.txt#L162.

@svenvh
Copy link
Member

svenvh commented Feb 16, 2024

Thanks for the report. Until last year, this project's cmake configuration wasn't even considering SPIRV-Tools-static. I've created backport #2370 on top of the llvm_release_160 branch; any chance you could give this a try to see if it fixes your issue?

@manuelmohr
Copy link
Author

Hi @svenvh, thanks for the quick response.

While the fix proposed #2370 seems reasonable for the Ubuntu system package, there might be a misunderstanding in what my configuration was when I encountered the problem.

I wasn't actually using the Ubuntu 22.04 system package for SPIRV-Tools. I was compiling everything from source, including LLVM.
The used versions are:

  • LLVM on tag llvmorg-16.0.6 (hash 7cbf1a2591520c2491aa35339f227775f4d3adf6)
  • SPIRV-LLVM-Translator on tag v16.0.0 (hash 81c9ac632ead5c233728725179ed2530c2c820ec)
  • SPIRV-Tools on tag v2023.2 (hash 44d72a9b36702f093dd20815561a56778b2d181e).

Now I'm using a standard Ubuntu 22.04 system config (inside a Docker container) to build these packages. First, I build and install LLVM. Then, I build and install SPIRV-Tools via cmake without any special options. Then, I try to build SPIRV-LLVM-Translator also via cmake with the options -DLLVM_DIR=${MY_LLVM_BUILD_DIR}/lib/cmake/llvm -DLLVM_SPIRV_BUILD_EXTERNAL=YES. But this fails during linking because of the error described above.

And in this case, the fix in #2370 doesn't help, because I do get the SPIRV-Tools-shared library. However, it just doesn't contain the symbols (at least, as publicly accessible symbols) that SPIRV-LLVM-Translator expects. This seems like a conceptual issue as the parts of the API that SPIRV-LLVM-Translator uses are apparently regarded as private by SPIRV-Tools. To my understanding, linking against the shared library should always fail for this reason. But I'm not totally sure I understand the situation correctly, see the discussion in KhronosGroup/SPIRV-Tools#3909.

@svenvh
Copy link
Member

svenvh commented Feb 21, 2024

The unresolved symbols are part of the SPIRV-Tools C++ API. What seems odd to me is that those don't have any symbol visibility markings (unlike the C API in libspirv.h as you pointed out), so they would indeed be hidden under -fvisibility=hidden. Perhaps you can try sprinkling some SPIRV_TOOLS_EXPORT or __attribute__((visibility("default"))) into libspirv.hpp and see if that resolves the issue?

@manuelmohr
Copy link
Author

I can confirm that this single-line change to libspirv.hpp fixes the problem and makes SPIRV-LLVM-Translator link against the shared library correctly:

-class SpirvTools {
+class SPIRV_TOOLS_EXPORT SpirvTools {

So either SPIRV-Tools doesn't consider the C++ stuff part of the public API (which doesn't really make sense) or someone forgot to configure the correct symbol visibility for the C++ header (which sounds more likely). So we consider this to be a bug in SPIRV-Tools? Do you want to create an issue with them?

@svenvh
Copy link
Member

svenvh commented Feb 22, 2024

Thanks for confirming the fix! I have created KhronosGroup/SPIRV-Tools#5591 to fix this in SPIRV-Tools.

@svenvh
Copy link
Member

svenvh commented Apr 19, 2024

The SPIRV-Tools fix has just been merged, so closing this; but please reopen if you are still seeing issues.

@svenvh svenvh closed this as completed Apr 19, 2024
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

No branches or pull requests

2 participants