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

Add LinkerFlavor to NativeAOT #83558

Merged
merged 7 commits into from
Mar 22, 2023

Conversation

am11
Copy link
Member

@am11 am11 commented Mar 16, 2023

On linux-musl-arm64, for example, the hello world app goes from 2 MB (2049832 B) to 1.8 MB (1918952 B) when binutils-gold package is installed.

@am11 am11 requested a review from MichalStrehovsky as a code owner March 16, 2023 22:53
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Mar 16, 2023
@ghost
Copy link

ghost commented Mar 16, 2023

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

Issue Details

On linux-musl-arm64, for example, the hello world app goes from 2 MB (2049832 B) to 1.8 MB (1918952 B) when binutils-gold package is installed.

Author: am11
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@MichalStrehovsky
Copy link
Member

I pulled down the coredump from one of the failing ARM64 tests:

The stack is:

 # Child-SP          RetAddr               Call Site
00 0000ffff`e02dd4d0 0000aae3`2507baf0     System_Runtime_Intrinsics!UnixNativeCodeManager::EHEnumInit+0x8 [/__w/1/s/src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp @ 817] 
01 0000ffff`e02dd4d0 0000aae3`2507b960     System_Runtime_Intrinsics!S_P_CoreLib_System_Runtime_EH::FindFirstPassHandler+0x50 [/_/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs @ 795] 
02 0000ffff`e02dd580 0000aae3`2507b828     System_Runtime_Intrinsics!S_P_CoreLib_System_Runtime_EH::DispatchEx+0xe0 [/_/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs @ 637] 
03 0000ffff`e02dd610 0000aae3`24fbfe00     System_Runtime_Intrinsics!S_P_CoreLib_System_Runtime_EH::RhThrowEx+0x48 [/_/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs @ 573] 
04 0000ffff`e02dd630 0000aae3`25068514     System_Runtime_Intrinsics!RhpThrowEx+0xbc [/__w/1/s/src/coreclr/nativeaot/Runtime/arm64/ExceptionHandling.S @ 338] 
05 0000ffff`e02dd990 0000aae3`250682bc     System_Runtime_Intrinsics!S_P_CoreLib_System_Text_EncodingTable::InternalGetCodePageFromName+0x1f4 [/_/src/libraries/System.Private.CoreLib/src/System/Text/EncodingTable.cs @ 99] 

We segfault at this line:

I hope the size savings we're seeing are not because gold stripped the LSDA part of the unwinding information :(

@am11
Copy link
Member Author

am11 commented Mar 17, 2023

Good and bad news.

Good news is that dwarf-dump errors are lowered with gold:

    Found 9859 warnings and errors, expected between 12000 and 13000

Bad news: --gc-section issue strike again. Just like llvm linker (lld), ld.gold is also eagerly optimizing some sections with --gc-section which we depend on. The only difference is lld reports the issues at link time, while gold fails at run time. We probably need some annotations (or full initialization?) somewhere to help these linkers identify the required sections in the final object and carefully optimize it (currently they drop the sections wholesale).

Stats for hwapp app: release config with stripped symbols

Linker flavor Size in bytes With --gc-sections
bfd 2181200 no
gold 2181472 no
lld 2153832 no
mold 2150816 no
bfd 2049832 yes
gold 1918952 yes
lld -- fails to link -- yes
mold 1995168 yes

I think for now, we can just provide it as an option -p:LinkerFlavor= linker with default set to bfd on linux and --gc-sections disabled for the other flavors; until we figures out a correct way of preserving the section for safe linking.

@am11 am11 changed the title Prefer gold linker on linux Add LinkerFlavor to NativeAOT Mar 17, 2023
@jkotas
Copy link
Member

jkotas commented Mar 22, 2023

Just to make sure that I understand what this change does after the latest revision:

  • There are no changes in any defaults. We still use the same linker by default as before.
  • UseLLVMLinker property is replaced by a more flexible LinkerFlavor property
    Is my understanding correct?

@am11
Copy link
Member Author

am11 commented Mar 22, 2023

Yes, this is correct.

If some distro uses gold, mold or lld as default (build time option of gcc and llvm), it will now explicitly select/require bfd linker. This is what we want because we use --gc-sections with which other linkers strip away some section more eagerly than we expect and cause random runtime failures; hello world works but exceptional paths terminate with wrong signal.

@jkotas jkotas merged commit 7500625 into dotnet:main Mar 22, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants