-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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 lld support #58959
Fix lld support #58959
Conversation
I have found that when I've made a change to use the lld linker in July, I've missed to push part of the change that was supposed to go to Arcade. I am checking it to the runtime repo to ensure that it is fixed for the RC2 release. I will make the same change in the Arcade repo.
Tagging subscribers to this area: @hoyosjs Issue DetailsI have found that when I've made a change to use the lld linker in July, Without this change, the lld enabling flag is not used and we end up using I am checking it to the runtime repo to ensure that it is fixed for the
|
@@ -138,8 +138,8 @@ function(add_toolchain_linker_flag Flag) | |||
if (NOT Config STREQUAL "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this file will be overwrritten by the next arcade update, this would need to go into arcade.
(proposal dotnet/arcade#6768 was based on this very practical issue, as I myself including 10s of other people have forgotten about it in past, in various repos on several occasions .. a simple GitHub action can probably help 🙂)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, I have written that in the description above. We need to get this in very quickly to get it into the RC2, so I am making the same change in arcade and here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok, missed that. 👍
just noticed that most recently bot silently overwrote updates made in eng/common ~10 days ago: a539321. Overall this automation seems semi-complete.
The arcade clone of this change is dotnet/arcade#7883 |
I checked out f29484a and built this on a CentOS 8 ARM64 Raspberry Pi with
i.e. |
@crummel have you made a clean build? This is what I get when building it on my arm64 Ubuntu 18.04:
And I got the same from a cross build on my x64 Ubuntu. Can you please check the output of the (clean) build, close to the beginning, it should say something like:
We only look for lld with the same version syntax as the clang. So in my case, it was lld-9. Can you check if the lld is available in such a form? |
Do you know if there's a Microsoft-built arm64 SDK build with this fix included? I am using
|
It doesn't look like there is. There's a
|
I copied a link from https://github.com/dotnet/installer#installers-and-binaries Release/6.0.1XX (6.0.x Runtime): $ mkdir foo
$ curl -sSL https://aka.ms/dotnet/6.0.1xx/daily/dotnet-sdk-linux-arm64.tar.gz | tar xzf - -C foo
$ readelf -Wl foo/shared/Microsoft.NETCore.App/6.0.0-rtm.21504.6/libcoreclr.so
Elf file type is DYN (Shared object file)
Entry point 0x1b0000
There are 11 program headers, starting at offset 64
Program Headers:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
PHDR 0x000040 0x0000000000000040 0x0000000000000040 0x000268 0x000268 R 0x8
LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x1a58bc 0x1a58bc R 0x10000
LOAD 0x1b0000 0x00000000001b0000 0x00000000001b0000 0x4ab780 0x4ab780 R E 0x10000
LOAD 0x660000 0x0000000000660000 0x0000000000660000 0x024848 0x024848 RW 0x10000
LOAD 0x690000 0x0000000000690000 0x0000000000690000 0x01b140 0x080280 RW 0x10000
TLS 0x65b780 0x000000000065b780 0x000000000065b780 0x000000 0x0000e9 R 0x10
DYNAMIC 0x681eb8 0x0000000000681eb8 0x0000000000681eb8 0x000240 0x000240 RW 0x8
GNU_RELRO 0x660000 0x0000000000660000 0x0000000000660000 0x025000 0x025000 R 0x1
GNU_EH_FRAME 0x0fd9e8 0x00000000000fd9e8 0x00000000000fd9e8 0x01fe24 0x01fe24 R 0x4
GNU_STACK 0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW 0
NOTE 0x0002a8 0x00000000000002a8 0x00000000000002a8 0x000024 0x000024 R 0x4
Section to Segment mapping:
Segment Sections...
00
01 .note.gnu.build-id .dynsym .gnu.version .gnu.version_d .gnu.version_r .gnu.hash .dynstr .rela.dyn .rela.plt .gcc_except_table .rodata __tracepoints_strings .eh_frame_hdr .eh_frame
02 .text .init .fini .plt
03 .jcr .fini_array .init_array .data.rel.ro .dynamic .got .got.plt
04 .data .tm_clone_table __tracepoints __tracepoints_ptrs .bss
05 .tbss
06 .dynamic
07 .jcr .fini_array .init_array .data.rel.ro .dynamic .got .got.plt
08 .eh_frame_hdr
09
10 .note.gnu.build-id |
I had to do this to make it work for me: diff --git a/eng/common/native/init-compiler.sh b/eng/common/native/init-compiler.sh
index 28f5145a6f7..3247258d789 100644
--- a/eng/common/native/init-compiler.sh
+++ b/eng/common/native/init-compiler.sh
@@ -112,7 +112,7 @@ if [[ -z "$CC" ]]; then
fi
if [[ "$compiler" == "clang" ]]; then
- if command -v "lld$desired_version" > /dev/null; then
+ if command -v "lld" || command -v "lld$desired_version" > /dev/null; then
# Only lld version >= 9 can be considered stable
if [[ "$majorVersion" -ge 9 ]]; then
LDFLAGS="-fuse-ld=lld" The result:
|
That means your lld and clang are not the same versions. We generally expect the whole toolchain at the same version. The exception is objcopy, which has a bug in llvm9 (fixed in llvm10 onwards). See the ubuntu 16 cross build containers targetting arm64; how it is set up. |
But they are:
Sure, but the binaries named with a suffix is a Debian/Ubuntu convention, right? It's not true for RHEL. |
Yes we use naming convention for compiler in init-compiler which is how the packaging works across many operating systems and distros. For tools lld is the exception that was made by this PR; we typically add the introspection for tools in https://github.com/dotnet/runtime/blob/7afb66262c287b7832b872473d1e668038aa2496/eng/native/configuretools.cmake, |
@am11 the code in the init-tools.sh is used just to verify that the lld available is of high enough version. LLD before version 9 was not working properly. And IIRC, the version of LLD to use is picked by clang to match it when the |
@janvorli, previously cmake's |
But the lld is different from all the other tools. It is never invoked directly in any way, the compiler invokes it (and locates it) by itself based on the |
Thanks. I forgot about that difference. We could, however, still use find_program and set executable: # same for CXX
SET(CMAKE_C_LINK_EXECUTABLE "${CMAKE_LINKER_found-by-locate_toolchain_exec}
<OBJECTS> -o <TARGET> <CMAKE_C_LINK_FLAGS> <LINK_FLAGS> <LINK_LIBRARIES>") (supported in cmake 3.6, our lowest supported version) |
I would prefer to leave the decision on which lld to use on the compiler. I believe it would always pick one that matches the compiler version (based on my experiments) and I don't see a benefit of forcing a mismatching version. |
Right, we don't have to force set it explicitly, we could use existing |
Hey, @janvorli , even aside from fixing the build on RHEL-like environments, the actual fixes haven't landed in a 6.0 SDK yet, have they? I tried a recent RC2 and didn't get anywhere: #58959 (comment) |
@omajid, this fix has landed in RTM branch: #58959 (comment). |
Hmm, it is strange, the clone of this fix that I've made in the arcade repo is in the release/6.0-rc2 branch. I wonder how come the rc2 binaries don't have it. |
The lld linker usage was supposed to be optional (=> use it if installed, use standard ld if not). The build that doesn't use lld works fine on ARM64 devices with 4kB large memory pages, it only fails when the page size is larger and so the page where the gs cookie is located happens to be with code. So maybe we should enforce lld usage on arm64 linux. |
Yup, the fix is also in the latest RC2, downloaded from https://aka.ms/dotnet/6.0.1XX-rc2/daily/dotnet-sdk-linux-arm64.tar.gz (6.0.0-rc.2.21470.23)). |
Thanks. I can confirm that that a recent rc2 build indeed fixes things for me. |
I have found that when I've made a change to use the lld linker in July,
I've missed to push part of the change that was supposed to go to Arcade.
Without this change, the lld enabling flag is not used and we end up using
the default linker.
I am checking it to the runtime repo to ensure that it is fixed for the
RC2 release. I will make the same change in the Arcade repo.