[llvm] prefix linker flag on non-MSVC compilers with `-Wl,`

[ashay-github]

Summary

Prior to this patch, a Windows build of llvm-lto using clang failed with
the error: LTO.def: unknown file type. The reason for this failure is
that .DEF files are used by the linker not by the clang compiler. The
MSVC compiler+linker handles this transparently, but if we're using
clang (or gcc), then we need to tell the compiler to forward this flag
to the linker. This patch adds the necessary -Wl flag to fix the
problem.

[ashay-github]

Here (https://github.com/llvm/llvm-project/issues/56271) is another instance of the same problem.

[beanz]

@ashay-github, just to make sure I'm understanding the issue, this occurs when building with clang, not clang-cl correct?

If the issue occurs with clang-cl, that seems like a bug we should fix in the clang-cl driver.

llvm/cmake/modules/AddLLVM.cmake

132

Does the same issue occur if you use compilers other than clang? I'm wondering if this change should be clang-specific.

[ashay-github]

just to make sure I'm understanding the issue, this occurs when building with clang, not clang-cl correct?

@beanz It looks like the problem exists in both clang and clang-cl.

>clang-cl build\tools\lto\LTO.def
build\tools\lto\LTO.def : fatal error LNK1107: invalid or corrupt file: cannot read at 0x9DC
clang-cl: error: linker command failed with exit code 1107 (use -v to see invocation)

>clang build\tools\lto\LTO.def
build\tools\lto\LTO.def : fatal error LNK1107: invalid or corrupt file: cannot read at 0x9DC
clang: error: linker command failed with exit code 1107 (use -v to see invocation)

>cl build\tools\lto\LTO.def
Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31630 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

Microsoft (R) Incremental Linker Version 14.33.31630.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:LTO.exe
/def:build\tools\lto\LTO.def

Thanks for the tip about the clang-cl driver, I'll look there.

llvm/cmake/modules/AddLLVM.cmake

132

Since both gcc and clang use -Wl to pass flags to the linker, I imagine that this problem is not clang specific. Unfortunately, I don't have gcc/MinGW setup to try this out.

[rnk]

lgtm

llvm/cmake/modules/AddLLVM.cmake

132

I think if one configured a cmake project with clang-cl, CMake would invoke the linker directly, and it would go down the if (MSVC) codepath above. The linker would directly understand the /def flag.

[mstorsjo]

llvm/cmake/modules/AddLLVM.cmake

132

I’m not so sure about this change. If building in mingw mode, the linker doesn’t support any ‘/DEF:` flag at all, in pretty sure that this will break such builds.

So can you clarify, in exactly what setup does if(MSVC) not trigger, but you still have got an msvc style linker that supports the /DEF: option?

[ashay-github]

llvm/cmake/modules/AddLLVM.cmake

132

So can you clarify, in exactly what setup does if(MSVC) not trigger, but you still have got an msvc style linker that supports the /DEF: option?

I see the error when I compile LLVM using clang.exe that comes bundled with Microsoft Visual Studio 2022. If I were to invoke cl.exe, then .def files are sent to the linker instead of being interpreted by the compiler.

It looks like ld can handle .def files without any special flags [https://sourceware.org/binutils/docs/ld/WIN32.html#index-using-a-DEF-file], so I think I'd need to add an exception for gcc so as to not break MinGW builds. Does that seem right to you? Oh, and thanks for catching this, @mstorsjo!

[mstorsjo]

llvm/cmake/modules/AddLLVM.cmake

132

So overall, I'm a bit sceptical to this build configuration - is there any reason why you can't build using clang-cl instead?

I think overall that building LLVM itself with clang in MSVC mode, without using clang-cl, isn't really supported. (CMake itself does support that configuration since CMake 3.15, but building LLVM does a lot of setting/testing/tweaking of compiler flags.)

I did test building this way, and while this issue seems to be the only hard error right now, the build spews endless amounts of warnings, so it's clearly not a configuration that is supported/maintained yet. On the other hand, if there's a good reason for doing it though and someone wants to work on it to improve it, I guess it can be accepted. But I'd like to hear a reason why the clang-cl interface doesn't work here.

Anyway, when it comes to linker flags for linkers on Windows, if we're going to support this configuration, there's three different cases:

• link.exe/lld-link options (the default when using cl/clang-cl)
• ld.bfd/ld.lld style options, passed via the compiler driver (where they need -Wl)
• link.exe/lld-link options passed via the compiler driver with -Wl

Currently, we can check if (MSVC) to detect the first case, and the second one either as an else() to if (MSVC), or if (MINGW). But IMO this is the big issue, we don't have a very neat way to detect the third case.

From digging around a bit, I found that you can check CMAKE_CXX_SIMULATE_ID, which is set to MSVC when building with clang-cl - so you could have something like this:

if (MSVC)
    use(-def:${DEFFILE})
elseif(CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC")
    use(-Wl,-def:${DEFFILE})
elseif(MINGW)
    use(${DEFFILE})
endif()

[rnk]

Regarding support for the clang.exe driver in an MSVC environment, I agree, it's not currently well supported, but it's a frequent user request. Lots of folks have GCC-ish build systems that they want to retarget to an MSVC environment. @akhuang is planning to do some clang driver work in this space for some of our users.

[ashay-github]

Updated flag to permit building under MingW

llvm/cmake/modules/AddLLVM.cmake

132

is there any reason why you can't build using cl instead?

Coincidentally, someone else asked me the same question. If I run into more build issues, I'll probably revert to cl, but I'm hesitant to leave this particular issue as a known, but undocumented, problem.

For this error, though, I have something very similar to what you suggested. It took a while for me to run this through clang under MSVC, MingW gcc with ld, and MingW gcc with lld, but the following seems to satisfy those configurations.

if(WIN32)
  if(MINGW)
    set(export_file_linker_flag "-Wl,\"${export_file_linker_flag}\"")
  elseif(MSVC)
    set(export_file_linker_flag "/DEF:\"${export_file_linker_flag}\"")
  else()
    set(export_file_linker_flag "-Wl,/DEF:\"${export_file_linker_flag}\"")
  endif()
endif()

[mstorsjo]

In D134165#3804230, @rnk wrote:

Regarding support for the clang.exe driver in an MSVC environment, I agree, it's not currently well supported, but it's a frequent user request. Lots of folks have GCC-ish build systems that they want to retarget to an MSVC environment. @akhuang is planning to do some clang driver work in this space for some of our users.

Yep, I've been there myself too. (Although I hadn't thought about the correct way of dealing with this in cmake until now.) I've seen @akhuang's patches - they're much appreciated!

llvm/cmake/modules/AddLLVM.cmake

121

Actually, I think we should make this else() into elseif(WIN32) here just for clarity., since the whole block is about making a def file and passing it to the linker, which is Windows specific.

132

Thanks for taking the time to actually test building this - that does indeed take some time.

I don't think we should add an extra if(WIN32) here - the whole section about making a def file is specific for windows already in itself. Instead we can make the surrounding else() into elseif(WIN32).

Secondly, while your suggestion works, I would prefer to explicitly check for this configuration (CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC"), than to assume with a generic else() that it refers to this rather specific/uncommon environment. When reading cmake code, it's otherwise not very evident to the reader that a generic else() means clang-msvc-mode-with-gnu-driver.

Also thirdly, I realized that we don't need a specific action for mingw here - the current behaviour (where there's just an if (MSVC) and doesn't do anything for mingw) works fine for mingw, since it just passes the file name as linker flag, which gets forwarded through the compiler driver to the linker as an input file.

That is, I think my preferred form would be something like this:

@@ -118,7 +118,7 @@ function(add_llvm_symbol_exports target_name export_file)
       set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                    LINK_FLAGS "  -Wl,--version-script,\"${CMAKE_CURRENT_BINARY_DIR}/${native_export_file}\"")
     endif()
-  else()
+  elseif(WIN32)
     set(native_export_file "${target_name}.def")

     add_custom_command(OUTPUT ${native_export_file}
@@ -130,6 +130,12 @@ function(add_llvm_symbol_exports target_name export_file)
     set(export_file_linker_flag "${CMAKE_CURRENT_BINARY_DIR}/${native_export_file}")
     if(MSVC)
       set(export_file_linker_flag "/DEF:\"${export_file_linker_flag}\"")
+    elseif(CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC")
+      set(export_file_linker_flag "-Wl,/DEF:\"${export_file_linker_flag}\"")
+    elseif(MINGW)
+      # ${export_file_linker_flag}, which is the plain file name, works as is
+      # when passed to the compiler driver, which then passes it on to the
+      # linker as an input file.
     endif()
     set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                  LINK_FLAGS " ${export_file_linker_flag}")

That makes the mingw case clearer so that it isn't forgotten/missed by the reader. Alternatively we could add at least quoting around it, like for the other /DEF cases too. Passing it with -Wl, doesn't seem to be necessary, so let's not change that aspect here since it's not needed.

Maybe we should even add comments explaining the two if (MSVC) and (CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC") cases, e.g. like this:

if(MSVC) # cl.exe or clang-cl, i.e. MSVC style command line interface
..
elseif (CMAKE_CXX_SIMULATE_ID STREQUAL "MSVC") # clang in msvc mode, calling a link.exe/lld-link style linker
...

[ashay-github]

• Explicitly added a case for MingW
• Quoted flag in the MingW case
• Added a catch-all block that throws an error

[ashay-github]

llvm/cmake/modules/AddLLVM.cmake

132

Thanks for practically rewriting the change! :)

I did take the liberty of adding a catch-all block so that any unsupported case doesn't go unhandled.

[mstorsjo]

Thanks, this looks good to me!

BTW, the canonical spelling for mingw is "MinGW", but in most cases I'd just type it with all lowercase.

llvm/cmake/modules/AddLLVM.cmake

add_custom_command(OUTPUT ${native_export_file}
       COMMENT "Creating export file for ${target_name}")
     if (${LLVM_LINKER_IS_SOLARISLD})
       set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                    LINK_FLAGS "  -Wl,-M,\"${CMAKE_CURRENT_BINARY_DIR}/${native_export_file}\"")
     else()
       set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                    LINK_FLAGS "  -Wl,--version-script,\"${CMAKE_CURRENT_BINARY_DIR}/${native_export_file}\"")
     endif()
   else()
     set(native_export_file "${target_name}.def")
 
     add_custom_command(OUTPUT ${native_export_file}
       COMMAND "${Python3_EXECUTABLE}" -c "import sys;print(''.join(['EXPORTS\\n']+sys.stdin.readlines(),))"
         < ${export_file} > ${native_export_file}
       DEPENDS ${export_file}
       VERBATIM
       COMMENT "Creating export file for ${target_name}")
     set(export_file_linker_flag "${CMAKE_CURRENT_BINARY_DIR}/${native_export_file}")
-    if(MSVC)
+    if(WIN32)
+      if(MINGW)
+        set(export_file_linker_flag "-Wl,\"${export_file_linker_flag}\"")
+      elseif(MSVC)
         set(export_file_linker_flag "/DEF:\"${export_file_linker_flag}\"")
+      else()
+        set(export_file_linker_flag "-Wl,/DEF:\"${export_file_linker_flag}\"")
+      endif()
     endif()
     set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                  LINK_FLAGS " ${export_file_linker_flag}")
   endif()
 
   add_custom_target(${target_name}_exports DEPENDS ${native_export_file})
   set_target_properties(${target_name}_exports PROPERTIES FOLDER "Misc")