Skip to content

Build with clang-cl/lld-link fails due to missing symbol #1405

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

Open
Neumann-A opened this issue Feb 23, 2025 · 10 comments
Open

Build with clang-cl/lld-link fails due to missing symbol #1405

Neumann-A opened this issue Feb 23, 2025 · 10 comments

Comments

@Neumann-A
Copy link

Error:

[237/238] C:\WINDOWS\system32\cmd.exe /C "cd . && E:\vcpkg_cache\downloads\tools\cmake-3.30.1-windows\cmake-3.30.1-windows-i386\bin\cmake.exe -E vs_link_dll --intdir=src\vsg\CMakeFiles\vsg.dir --rc=rc.exe --mt="D:\vcpkg_folders\no_msvc\installed\x64-windows-static\compiler\msvc\WinSDK\Windows Kits\10\bin\10.0.26100.0\x64\mt.exe" --manifests  -- D:\vcpkg_folders\no_msvc\installed\x64-windows-static\compiler\llvm\bin\lld-link.exe  @CMakeFiles\vsg.rsp  /out:lib\vsg-14.dll /implib:lib\vsg.lib /pdb:lib\vsg-14.pdb /dll /version:1.1 /machine:x64 /nologo /DEBUG /INCREMENTAL:NO /OPT:REF /OPT:ICF && cd ."
FAILED: lib/vsg-14.dll lib/vsg.lib 
C:\WINDOWS\system32\cmd.exe /C "cd . && E:\vcpkg_cache\downloads\tools\cmake-3.30.1-windows\cmake-3.30.1-windows-i386\bin\cmake.exe -E vs_link_dll --intdir=src\vsg\CMakeFiles\vsg.dir --rc=rc.exe --mt="D:\vcpkg_folders\no_msvc\installed\x64-windows-static\compiler\msvc\WinSDK\Windows Kits\10\bin\10.0.26100.0\x64\mt.exe" --manifests  -- D:\vcpkg_folders\no_msvc\installed\x64-windows-static\compiler\llvm\bin\lld-link.exe  @CMakeFiles\vsg.rsp  /out:lib\vsg-14.dll /implib:lib\vsg.lib /pdb:lib\vsg-14.pdb /dll /version:1.1 /machine:x64 /nologo /DEBUG /INCREMENTAL:NO /OPT:REF /OPT:ICF && cd ."
LINK: command "D:\vcpkg_folders\no_msvc\installed\x64-windows-static\compiler\llvm\bin\lld-link.exe @CMakeFiles\vsg.rsp /out:lib\vsg-14.dll /implib:lib\vsg.lib /pdb:lib\vsg-14.pdb /dll /version:1.1 /machine:x64 /nologo /DEBUG /INCREMENTAL:NO /OPT:REF /OPT:ICF /MANIFEST:EMBED,ID=2" failed (exit code 1) with the following output:
lld-link: error: undefined symbol: public: void __cdecl vsg::Object::setValue<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>(class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &)

>>> referenced by src\vsg\CMakeFiles\vsg.dir\core\Data.cpp.obj

>>> referenced by src\vsg\CMakeFiles\vsg.dir\io\Path.cpp.obj

ninja: build stopped: subcommand failed.

Probably a include is missing since the definition of the functions is in Value.h and not Object.h. I fixed it by explicitly instantiating the function in Object.cpp (and including Value.h of course)

@robertosfield
Copy link
Collaborator

What build options did you use when building the VSG?

Could you send a PR with your changes so I can review and merge the fix.

@Neumann-A
Copy link
Author

What build options did you use when building the VSG?

Probably just the default. I don't see anything of relevance in the below cmd line:

[1/1] "E:/vcpkg_cache/downloads/tools/cmake-3.30.1-windows/cmake-3.30.1-windows-i386/bin/cmake.exe" -E chdir ".." "E:/vcpkg_cache/downloads/tools/cmake-3.30.1-windows/cmake-3.30.1-windows-i386/bin/cmake.exe" "D:/vcpkg_folders/no_msvc/buildtrees/vsg/src/v1.1.10-753f9ea293" "-G" "Ninja" "-DCMAKE_BUILD_TYPE=Release" "-DCMAKE_INSTALL_PREFIX=D:/vcpkg_folders/no_msvc/packages/vsg_x64-windows-static" "-DFETCHCONTENT_FULLY_DISCONNECTED=ON" "-DGLSLANG_MIN_VERSION=" "-DCMAKE_MAKE_PROGRAM=E:/vcpkg_cache/downloads/tools/ninja/1.12.1-windows/ninja.exe" "-DBUILD_SHARED_LIBS=ON" "-DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=D:/vcpkg_folders/no_msvc/triplets/x64-win-llvm/x64-win-llvm-toolchain.cmake" "-DVCPKG_TARGET_TRIPLET=x64-windows-static" "-DVCPKG_SET_CHARSET_FLAG=ON" "-DVCPKG_PLATFORM_TOOLSET=ClangCL" "-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON" "-DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=ON" "-DCMAKE_FIND_PACKAGE_NO_SYSTEM_PACKAGE_REGISTRY=ON" "-DCMAKE_INSTALL_SYSTEM_RUNTIME_LIBS_SKIP=TRUE" "-DCMAKE_VERBOSE_MAKEFILE=ON" "-DVCPKG_APPLOCAL_DEPS=OFF" "-DCMAKE_TOOLCHAIN_FILE=D:/vcpkg_folders/no_msvc/scripts/buildsystems/vcpkg.cmake" "-DCMAKE_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION=ON" "-DVCPKG_CXX_FLAGS=" "-DVCPKG_CXX_FLAGS_RELEASE=" "-DVCPKG_CXX_FLAGS_DEBUG=" "-DVCPKG_C_FLAGS=" "-DVCPKG_C_FLAGS_RELEASE=" "-DVCPKG_C_FLAGS_DEBUG=" "-DVCPKG_CRT_LINKAGE=dynamic" "-DVCPKG_LINKER_FLAGS=" "-DVCPKG_LINKER_FLAGS_RELEASE=" "-DVCPKG_LINKER_FLAGS_DEBUG=" "-DVCPKG_TARGET_ARCHITECTURE=x64" "-DCMAKE_INSTALL_LIBDIR:STRING=lib" "-DCMAKE_INSTALL_BINDIR:STRING=bin" "-D_VCPKG_ROOT_DIR=D:/vcpkg_folders/no_msvc" "-D_VCPKG_INSTALLED_DIR=D:/vcpkg_folders/no_msvc/installed" "-DVCPKG_MANIFEST_INSTALL=OFF" "-DVCPKG_HOST_TRIPLET=x64-windows-static" "-DCMAKE_POLICY_DEFAULT_CMP0149=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0137=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0128=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0126=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0117=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0092=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0091=OLD" "-DCMAKE_POLICY_DEFAULT_CMP0067=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0066=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0056=NEW" "-DCMAKE_POLICY_DEFAULT_CMP0012=NEW"

Could you send a PR with your changes so I can review and merge the fix.

No I don't think the fix is what you want to use. Here is the diff either way:

diff --git a/src/vsg/core/Object.cpp b/src/vsg/core/Object.cpp
index 8b39dedfb0..7f6e9b17de 100644
--- a/src/vsg/core/Object.cpp	
+++ b/src/vsg/core/Object.cpp
@@ -14,6 +14,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 #include <vsg/core/Auxiliary.h>
 #include <vsg/core/ConstVisitor.h>
 #include <vsg/core/Object.h>
+#include <vsg/core/Value.h>
 #include <vsg/core/Visitor.h>
 
 #include <vsg/io/Input.h>
@@ -242,3 +243,6 @@ void Object::operator delete(void* ptr)
 {
     vsg::deallocate(ptr);
 }
+
+// Explicitly instantiate the function template for std::string
+template void vsg::Object::setValue<std::string>(const std::string& key, const std::string& value);

Although I only observed it for std::string it probably also occurs for all other types if the Value.h include is missing. So basically, you want to Scan where setValue is used and make sure Value.h is directly included (and not indirectly how Object.h is currently included).

@robertosfield
Copy link
Collaborator

The diff looks impractical long term.

Other compilers have been working fine for years with the code as is, so could there be an issue with the version of clang and CMake?

@Neumann-A
Copy link
Author

Other compilers have been working fine for years with the code as is, so could there be an issue with the version of clang and CMake?

No, how often is the Compiler actually wrong? What does that even have to do with CMake?

The Code is wrong, the compiler is free to optimize the symbol away since it is a template and thus implicitly inline. So there are two cases here: 1) The case where a TU sees Value.h and Object.h and can create/inline setValue and 2) the case where a TU only sees the declaration in Object.h. In the latter case a call to setValue is emitted and the linker has to figure it out. If all the TUs seeing case 1) inline setValue no symbol for it is emitted which leaves the TUs seeing case 2) complaining about the missing symbol since no TU has it.

So how did it work fine for all those years? The code wasn't fully optimized.....probably because it is only tested with MSVC on windows?

Solutions? (one of the following)
a) avoid separating of Value.h from Object.h, either make it one header or move the required definition of Object::setValue actually into Object.h (instead of having it in Value.h which creates the problem)
b) create a single TU which explicitly instantiate all the possible symbols
c) make sure every user of setValue includes Object.h AND Value.h

but i am no language, compiler or linker lawyer, that is just my interpretation of what is happening.

@robertosfield
Copy link
Collaborator

robertosfield commented Feb 25, 2025 via email

@reedev
Copy link

reedev commented Feb 25, 2025

Hi, @Neumann-A

I think it is very productive when you make concrete suggestions without too many assumptions. Please be assured that this project (and related projects) has a long history on Unix platforms, and so not as you suggested.

If you describe what and how you tried out things, we can support you.

@Neumann-A
Copy link
Author

I am only speaking about Windows.

Feel free to give #1407 a spin in CI. I expect it to reproduce the error if it does not the next line might need a --config Release to actually build an optimized version since I currently don't know if the defaulting of CMAKE_BUILD_TYPE actually works for multi config generators. The docs say no so the default is probably the first element in CMAKE_CONFIGURATION_TYPES which is Debug. (Last CI logs show: vsg.vcxproj -> D:\a\VulkanSceneGraph\VulkanSceneGraph\lib\vsgd.lib so it has a debug suffix and thus you are not testing Release builds in CI)

@Neumann-A
Copy link
Author

Yep same error in your CI:

2025-02-25T17:17:42.8384980Z D:\a\VulkanSceneGraph\VulkanSceneGraph\include\vsg/platform/win32/Win32_Window.h(108,17): warning : unsafe buffer access [-Wunsafe-buffer-usage] [D:\a\VulkanSceneGraph\VulkanSceneGraph\src\vsg\vsg.vcxproj]
2025-02-25T17:17:46.3420071Z lld-link : error : undefined symbol: public: void __cdecl vsg::Object::setValue<class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>>>(class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> const &) [D:\a\VulkanSceneGraph\VulkanSceneGraph\src\vsg\vsg.vcxproj]
2025-02-25T17:17:46.3421743Z   >>> referenced by vsg.dir\Release\Data.obj
2025-02-25T17:17:46.3422061Z   >>> referenced by vsg.dir\Release\Path.obj
2025-02-25T17:17:46.4111703Z ##[error]Process completed with exit code 1.
2025-02-25T17:17:46.4274776Z Post job cleanup.

(might need to deactivate some warnings though to get a smaller log.)

@AnyOldName3
Copy link
Contributor

As a bit more context to try and make sure everyone's on the same page:

  • The VSG worked fine when built with GCC in an MSYS2 environment for Windows the last time I tried it, so it's known to be working in a Windows-but-not-MSVC situation.

  • clang-cl is a driver mode for Clang (which you can also get with a regular Clang binary with clang --driver-mode=cl where it attempts to do what cl.exe (the C/C++ compiler part of MSVC) would do, including:

    • supporting the command-line arguments that cl does instead of the ones GCC does.
    • using the MSVC ABI and name mangling rather than the MinGW ABI and name mangling.
    • exposing MSVC language extensions.

    Making it work can be a little tricky sometimes as it generally wants treating exactly like MSVC, but:

    • it's not that hard to write CMake or preprocessor stuff that will treat it like Clang and break.
    • it's not completely devoid of bugs, particularly when it comes to undocumented MSVC language extensions (also known as things that aren't valid C++ but compiled with MSVC anyway by accident).

Some cusory googling suggests that CMake sets the compiler ID for clang-cl to Clang and we're expected to use CMAKE_CXX_COMPILER_FRONTEND_VARIANT to determine whether we should feed it GCC-style or MSVC-style arguments. We're not doing that, and are passing it our normal Clang warning flags, which won't make it happy. It's attempting to treat the -Wall it's passed how MSVC would treat /Wall, which is definitely not what we want as MSVC has loads of warnings that only exist because some customer paid Microsoft to add warnings to enforce their personal code style instead of using a separate linter, and /Wall really does enable all of them. To emulate that, clang-cl passes -Weverything to the underlying process, and that enables all of Clang's opinionated-and-only-required-by-one-company warnings. That should be an easy fix as checking the correct variable would let us use our MSVC warning flags (which are currently no warning flags).

As for the error, it looks like it's just another self-sufficient header problem like I've been complaining about for a while, just a more complicated variant. Neumann-A's explanation of the mechanics of the problem seems to be basically correct - it's legitimate for the compiler to not expose the implementation of setValue from the few translation units that have one (ViewDependentState.cpp.obj and TileDatabase.cpp.obj) as it's inline thanks to being a template, so TUs where it's called can be expected to see their own implementation. Everything works with MSVC because MSVC won't instantiate the explicit char* specialisation in TUs where it's not explicitly called, so it doesn't need to link with the std::string specialisation. Everything works with regular GCC and Clang because they assume all symbols should be exported publicly by default, which makes sure the linker can see the implementation in the two TUs that have it. clang-cl mixes the behaviours as it has GCC/Clang's instantiate lots of templates that aren't even used behaviour combined with MSVC's prune everything that only needs private linkage as early as possible behaviour, so instantiates an unused template that depends on something that's not directly available and is hidden in the places where it would have been indirectly available.

@AnyOldName3
Copy link
Contributor

You can avoid the problem this issue is tracking by using MSVC or GCC or regular Clang for Windows builds. This issue's tracking a problem that's specific to clang-cl, and most of the time (excluding problems like this one) it's a drop-in replacement for MSVC and vice versa, so provided it's not for commercial use or you're willing to pay for it, the easiest way to avoid being affected is to just use MSVC instead of clang-cl. Every other library in your project can still use clang-cl.

If you're having trouble with regular Clang instead of specifically clang-cl, this issue report isn't the place to discuss it.

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

4 participants