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

Fix compilation of NativeAOT with Xcode 16 #106039

Closed
wants to merge 1 commit into from

Conversation

filipnavara
Copy link
Member

Contributes to #104105

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 6, 2024
Copy link
Contributor

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

@jkotas
Copy link
Member

jkotas commented Aug 6, 2024

 ld: in /Users/runner/work/1/s/artifacts/bin/coreclr/osx.arm64.Release/aotsdk/libRuntime.ServerGC.a(WriteBarriers.S.o), N_ALT_ENTRY bit set on first atom in section __TEXT/__text file '/Users/runner/work/1/s/artifacts/bin/coreclr/osx.arm64.Release/aotsdk/libRuntime.ServerGC.a'
  clang: error: linker command failed with exit code 1 (use -v to see invocation)

@filipnavara filipnavara marked this pull request as draft August 6, 2024 20:48
@filipnavara
Copy link
Member Author

I'll retest with older version Xcode versions tomorrow. Need to switch the machine since my primary one is on macOS beta which doesn't run Xcode < 16.

.hidden C_FUNC(\Name)
#else
.alt_entry C_FUNC(\Name)
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be also done for NO_CFI_EPILOG?

#ifndef NO_CFI_EPILOG
        .global C_FUNC(\Name)
#endif
#if !defined(__APPLE__)
        .hidden C_FUNC(\Name)
#elif defined(NO_CFI_EPILOG)
        .alt_entry C_FUNC(\Name)
#endif

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done unconditionally. There are certain broken Clang versions where .alt_entry triggers errors but AFAIK it should not affect last couple of betas or any released version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. In theory this should fix the CI build but I haven't tested.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike ELF which can support pretty much arbitrary number of sections, the Mach-O object files are limited to ~255 sections. Apple uses the subsections-via-symbols linking approach to circumvent this limitation. In the subsections-via-symbols model the global symbols are used to slice the section into subsections (called atoms) which are then used as the linking unit (for purpose of dead stripping, possible reordering, etc.). Historically you had to opt-in into it but any Clang compiler in the past decade did so. The new linker included in Xcode 15 basically broke the support for object files without subsections-via-symbols, and the old linker is now scheduled to be completely removed.

.alt_entry is an escape hatch to introduce symbols that are globally visible, but at the same time don't cause the section/method to be split into two separate atoms. It's the only supported way to place globally visible labels into middle of the method, which is used by NativeAOT to mark instructions that (legally) cause SIGSEGVs.

In Apple Clang 16 (LLVM 17? 18?) there were additional checks added to prevent the misuse of global labels. Likewise, checks were added for misuse of the CFI opcodes. We need to correct the prior mistake of relying on undefined behavior in the linker that kept the assembly code order intact. That's why .alt_entry is unconditional. Unfortunately, there was a code in Clang that didn't correctly interpret .alt_entry for the purpose of consistency checks of the atoms and the CFI unwind information. It resulted in .alt_entry being incorrectly interpreted as a global label that splits atoms and any negative (relative) offsets in CFI epilogs trigger an error because it thinks the whole method goes into negative (absolute) offsets in CFI.

// This was reported and fixed upstream: https://github.com/llvm/llvm-project/issues/97116
// Until a fixed version of Xcode Clang is available we avoid emiting epilog CFI codes.
#if __clang_major__ >= 16 && __clang_major__ < 17 && defined(__apple_build_version__)
#define NO_CFI_EPILOG 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: #define NO_CFI_EPILOG (drop 1, since we are only checking defined/!defined)

@filipnavara
Copy link
Member Author

I'm back to the drawing board...

I managed to fix the whole compilation with .alt_symbol on Xcode 14/15. I'll write up on that later and submit individual fixes as separate PRs, it requires some CoreCLR fixes as well. Turns out, .alt_symbol alone doesn't solve the issue and it need to be combined with .private_extern or .global to make the full linking process working end-to-end in all cases.

That gets us back to this code test pattern:

.section __TEXT,__text
.globl _foo
_foo:
  .cfi_startproc
  sub sp, sp, 8
  .cfi_adjust_cfa_offset 8
  .alt_entry _bar
  //.global _bar
  //.private_extern _bar
_bar:
  add sp, sp, 8
  //.cfi_adjust_cfa_offset -8
  ret
  .cfi_endproc

Once you uncomment .global/.private_extern it hits llvm/llvm-project#82261. Short of object file post-processing (slapping N_EXT flag on all N_ALT_ENTRY symbols) I don't see a way to fix this without Apple fixing the compiler. Do you have any channel to get in contact with Apple engineers? I didn't get any answer on the feedback I filed (FB14191360) and given my previous experience with "Code-level support" I don't have high hopes for that avenue either. (cc @jroelofs since you fixed this bug in LLVM)

@jkotas
Copy link
Member

jkotas commented Aug 7, 2024

Do you have any channel to get in contact with Apple engineers?

I have raised this via Microsoft/Apple support channel.

@filipnavara
Copy link
Member Author

Closing in favor of #106442 and #106744.

@filipnavara filipnavara deleted the xcode16-naot branch August 21, 2024 03:51
@github-actions github-actions bot locked and limited conversation to collaborators Sep 20, 2024
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