Prefer lld in NativeAot when using clang

[sbomer]

This detects availability of lld and prefers it, when using clang. If not found, falls back on bfd as before, as discussed in #84794. Fixes #84934.

[ghost]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This detects availability of lld and prefers it, when using clang. If not found, falls back on bfd as before, as discussed in #84794. Fixes #84934.

Author:	sbomer
Assignees:	-
Labels:	area-NativeAOT-coreclr
Milestone:	-

[jkotas]

Build is failing with invalid linker name in argument '-fuse-ld=bfd'

[am11]

Otherwise LGTM, assuming we will add lld to the prerequisite docs: https://learn.microsoft.com/en-us/dotnet/core/deploying/native-aot/#prerequisites

[am11]

Suggested change

	<Exec Command="&quot;$(CppLinker)&quot; -fuse-ld=lld -Wl,--version" Condition="'$(LinkerFlavor)' == 'lld' or ('$(LinkerFlavor)' == '' and $(CppCompilerAndLinker.Contains('clang')))"
	<Exec Command="&quot;$(CppLinker)&quot; -fuse-ld=lld -Wl,--version" Condition="'$(LinkerFlavor)' == 'lld'" IgnoreExitCode="true" StandardOutputImportance="Low" ConsoleToMSBuild="true">

Revert this line: this is not correct. Before we were checking the --version regardless of LinkerFlavor emptiness. Which means FreeBSD and user-supplied -p:LinkkerFlavor cases were also considered.

[sbomer]

We're still checking the version when LinkerFlavor is lld, like before, whether user-supplied or due to the FreeBSD default above. Maybe I'm missing something - could you give an example?

[am11]

When LinkerFlavor is lld, we will always check the version. Which means _LinkerVersion can't be empty if lld exists and changes made in src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets are not needed, right?

[sbomer]

It's possible that LinkerFlavor is lld, but lld did not exist - then _LinkerVersion is empty.

[am11]

Then it will result in error one way or the other with fuse-ld=<non-existing-flavor>. We shouldn't be checking for emptiness.

fuse-ld is even sensitive to version. Try apt install clang-13 lld-14 and fuse-ld=lld will fail.

[sbomer]

The problem is that if we try to compare versions against an empty string, the build fails at that comparison:

A numeric comparison was attempted on "$(_LinkerVersion)" that evaluates to "" instead of a number, in condition "'$(LinkerFlavor)' == 'lld' and '$(_LinkerVersion)' > '12'"

This looks like an error in the build logic, whereas failing on the invocation with:

clang : error : invalid linker name in argument '-fuse-ld=lld

gives a clearer indication of what's wrong.

[am11]

Same as above, revert changes in this file.

[jkotas]

Otherwise LGTM, assuming we will add lld to the prerequisite docs: https://learn.microsoft.com/en-us/dotnet/core/deploying/native-aot/#prerequisites

My assumption was that it comes by default with clang, no?

[am11]

My assumption was that it comes by default with clang, no?

Nope, lld is a separate package on distros such as Ubuntu, Fedora and Alpine Linux.

[jkotas]

Ok, I have not realized that lld does not come with clang by default.

It means that lld is an optional light-up. If it is available, we will use it. If it is not available, we are happy to fallback to bfd silently (bfd is pretty much guaranteed to be available because of it comes as a dependency of clang).

llvm-objdump is on the same plan. If it is available, we will use it. If it is not available, we will be happy to fallback to objdump silently (again, objdump is pretty much guaranteed to be available because of it comes as a dependency of clang).

Including both lld and llvm-objdump in the instructions would make sense then.

I am wondering whether we should print something to the console when we use the fallback silently to make it clear that we prefer llvm-based tools. I can be useful for troubleshooting. Or even change the fallback to be explicit to make sure that people are not building with the less preferred tools by accident.

[sbomer]

This is similar to the optional light-up we have for clang, where if clang isn't installed, we will silently fall back on gcc if it's available.

Personally I would prefer the approach of defining our preferred tools, and requiring some explicit user action to take a different path - it makes things more predictable.

[jkotas]

I would prefer the approach of defining our preferred tools, and requiring some explicit user action to take a different path - it makes things more predictable.

Yes, especially given that there are 3 different packages that one has to install to get the preferred tools for out-of-the-box experience. It was just one package (clang) in .NET 7.

[am11]

Including both lld and llvm-objdump in the instructions would make sense then.

Yes, they have similarities. The difference is that fuse-ld=lld will select the exact matching version (clang-13 -> lld-13, clang-14 -> lld-14..), it does not depend on PATH. Whereas, llvm-objcopy is picked off of the PATH by default. In runtime builds, we don't have this problem because we use a cmake-based function to find the components (objcopy, objdump, ar, nm, readelf) matching the current toolchain flavor & version:

runtime/eng/native/configuretools.cmake

Lines 24 to 71 in 4719575

	function(locate_toolchain_exec exec var required)
	string(TOUPPER ${exec} EXEC_UPPERCASE)
	if(NOT "$ENV{CLR_${EXEC_UPPERCASE}}" STREQUAL "")
	set(${var} "$ENV{CLR_${EXEC_UPPERCASE}}" PARENT_SCOPE)
	return()
	endif()
	unset(EXEC_LOCATION_${exec} CACHE)
	find_program(EXEC_LOCATION_${exec}
	NAMES
	"${TOOLSET_PREFIX}${exec}${CLR_CMAKE_COMPILER_FILE_NAME_VERSION}"
	"${TOOLSET_PREFIX}${exec}")
	if (required AND EXEC_LOCATION_${exec} STREQUAL "EXEC_LOCATION_${exec}-NOTFOUND")
	message(FATAL_ERROR "Unable to find toolchain executable. Name: '${exec}', Prefix: '${TOOLSET_PREFIX}'")
	endif()
	if (NOT EXEC_LOCATION_${exec} STREQUAL "EXEC_LOCATION_${exec}-NOTFOUND")
	set(${var} ${EXEC_LOCATION_${exec}} PARENT_SCOPE)
	endif()
	endfunction()
	locate_toolchain_exec(ar CMAKE_AR YES)
	locate_toolchain_exec(nm CMAKE_NM YES)
	locate_toolchain_exec(ranlib CMAKE_RANLIB YES)
	if(CMAKE_C_COMPILER_ID MATCHES "Clang")
	locate_toolchain_exec(link CMAKE_LINKER YES)
	endif()
	if(NOT CLR_CMAKE_TARGET_APPLE AND (NOT CLR_CMAKE_TARGET_ANDROID OR CROSS_ROOTFS))
	locate_toolchain_exec(objdump CMAKE_OBJDUMP YES)
	locate_toolchain_exec(readelf CMAKE_READELF YES)
	unset(CMAKE_OBJCOPY CACHE)
	locate_toolchain_exec(objcopy CMAKE_OBJCOPY NO)
	if (CMAKE_OBJCOPY)
	execute_process(
	COMMAND ${CMAKE_OBJCOPY} --help
	OUTPUT_VARIABLE OBJCOPY_HELP_OUTPUT
	)
	endif()
	# if llvm-objcopy does not support --only-keep-debug argument, try to locate binutils' objcopy
	if (NOT CMAKE_OBJCOPY OR (CMAKE_C_COMPILER_ID MATCHES "Clang" AND NOT "${OBJCOPY_HELP_OUTPUT}" MATCHES "--only-keep-debug"))
	set(TOOLSET_PREFIX "")
	locate_toolchain_exec(objcopy CMAKE_OBJCOPY YES)

That is quite robust but implementing that in MSBuild without cmake dependency is non-trivial work.

[sbomer]

It looks like installing clang also installs matching llvm-objcopy-XX (with version suffix), at least on ubuntu. So there's no third package install needed, but the detection will miss it since we only look for unversioned llvm-objcopy. I think we will need to implement some kind of version-based detection if we want llvm-objcopy to be the default.

[jkotas]

If you can "apt-get install llvm" that will add version-less alias for llvm-objdump.

[sbomer]

Good to know - that seems to work on ubuntu 20.04 - but not 18.04. On 18.04 I get llvm-objdump but not llvm-objcopy.

[am11]

On Alpine, clang doesn't install llvm-objcopy, it requires llvm-dev package. Then llvm-dev doesn't install lldb, it requires explicit lldb. The toolchain prefix and version suffix is something we are not supporting. We discussed previously that if someone needs that, we should have properties to customize the desired toolchain; so we added <ObjCopyName>llvm-objcopy-# to match <CppCompilerAndLinker>clang-#.

[sbomer]

Also, lld is only available since version 7, while on 18.04 the default clang is 6, so I think we should recommend installing clang-7 or higher. Or maybe we should consider upping the ubuntu version in our instructions.

[jkotas]

This sounds like a very complicated matrix. Do we really want to change these defaults?

[jkotas]

This sounds like a very complicated matrix. Do we really want to change these defaults?

Hmm, we are exposed to it already. So trying to auto-detected what is on the box is about the best that we can do.

[sbomer]

Hmm, we are exposed to it already. So trying to auto-detected what is on the box is about the best that we can do.

Agreed. I don't have a strong opinion on what should be the default, but I think there should at least be an easy way to predictably use llvm tools. If we support Ubuntu 18.04 then I think that means we need to at least detect version of llvm-objcopy matching clang, and possibly detect higher versions of installed clang.

[sbomer]

Actually, to avoid version-based detection in MSBuild, could we consider Ubuntu 18.04 to have degraded support, and require more steps from the user (ask them either to symlink llvm-objcopy/clang, or set CppCompilerAndLinker/ObjCopyName)?

Then on Ubuntu 20.04 and Alpine 3.15 we can just update steps to recommend installing clang, lld, and llvm, and we would use the default versions.

[jkotas]

init-compiler.sh

init-compiler.sh is tied to dotnet/runtime engineering infrastructure. It lives in arcade repo and it is not meant to be a shipping artifact.

I would hope that we can land on something simpler for the linker detection than hundred lines shell script. I like the default experience today: Install clang, the rest just works, the implementation is not that complicated. It feels we are trying to complicate it quite a bit.

[jkotas]

To address the problem with bfd linker not being available in our cross-build containers, can we set LinkerFlavor=lld in the build container to make it just work?

[sbomer]

Yes, solving this in the runtime build only should be easy enough, and doesn't need to complicate the shipping logic.

The reason I was changing the product layer is that there was a desire to make the shipping defaults more similar to what we use in the runtime build. But we haven't landed on a tradeoff we like (since it requires extra install steps and/or more complicated detection logic).

@jkotas I think you were on board with making lld/llvm-objcopy the default, even if it means users need to install two extra packages, until we realized that it's problematic on Ubuntu 18.04:

1. The default clang version doesn't support lld
2. It seems to require overriding the objcopy name since there's no unversioned llvm-objcopy available in packages

For 2. I think we could solve it with a little extra detection logic if necessary. But detecting higher clang versions is going too far in the direction of init-compiler.sh. Does that match your view @jkotas?

Ubuntu 18.04 goes out of standard support the end of this month. I would like it if we could choose defaults that match what we do in our builds, and what makes most sense on more recent platforms.

I think it would be reasonable to document some extra steps needed to work around those issues on 18.04. The workaround would look something like:

• On Ubuntu 18.04, extra steps are required because the default tools are older than our recommended tools. You can either:
  • Install a more recent clang (at least clang-7), and set matching properties CppCompilerAndLinker=clang-7, ObjCopyName=llvm-objcopy-7 to use an LLVM toolchain, or
  • Set LinkerFlavor=bfd and ObjCopyName=objcopy to use GNU binutils.

If that is too much to ask of users, then maybe we should live with the fall-back that doesn't require any user action. It has worked well enough for clang/gcc so far.

[jkotas]

you were on board with making lld/llvm-objcopy the default, even if it means users need to install two extra packages, until we realized that it's problematic on Ubuntu 18.04

Yes, I have not realized that it will require installing additional packages and expose us to potential version mismatches. I think it is likely problematic on more than just Ubuntu 18. Every other distro seems to have their own tweaks on how the explicitly versioned binaries are setup, and it varies over time. With these insights, I think it is better to lean on the standard config by default that should be simple, and allow people to manually override when it does not work.

[agocke]

Hmm, I thought I commented on this issue, but I don't see it. I must have forgot to submit.

I've been thinking about the matrix here and it seems like, no matter what, we're going to have to be compatible with a variety of different linkers. Even if we ignore lld/bfd, we have to support ld64, the Mac linker. That means we're always going to be in a bit of a passive position of adapting to the system requirements.

Given that, it seems like we should roll with whatever the user has on the machine. Preference for one linker over another seems mostly academic, since it's unlikely that we'll get the exact same version of a linker we test with, regardless of which linker we choose. This also seems to be necessary since we want to be on the latest linker version in our official builds for fixes and patches, but most Linux distros ship with very old versions of the linkers.

So, no matter what we choose we probably won't be running on something we explicitly tested. In that case, I wouldn't provide a warning if we run with something other than our preferred linker, I would at most produce some sort of diagnostic message, like the "Generating native code" message that's output during Native AOT publish.

[sbomer]

Closing this since we are going with the existing defaults that are more likely to work on the most common linux distros - so bfd linker, and fallback from clang to gcc.