- Notifications
You must be signed in to change notification settings - Fork5.2k
[NativeAOT] 32-bit platform ObjWriter and bit rot fixes#96890
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…dends to account for R2R ABI differences; use inline addend for 32-bit platforms; mark function symbols as Thumb code on ARM
ghost commentedJan 12, 2024
Tagging subscribers to this area:@agocke,@MichalStrehovsky,@jkotas Issue Detailsnull
|
…rned; the should not get the symbol size or the Thumb bit
| if(destination<=Register.R7) | ||
| { | ||
| Builder.EmitShort(unchecked((short)(0x4478u+(byte)destination))); | ||
| } | ||
| else | ||
| { | ||
| Builder.EmitShort(unchecked((short)(0x44f0u+(byte)destination))); | ||
| } |
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.
Review note: This affects R2R. It generates the same pattern as the JIT but it's one extra instruction. If you feel like we need to special-case it, I'll try to do it.
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 think it is fine. This pattern is better for private working set.
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/Dwarf/DwarfCie.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| <CrossCompileArchCondition="$(CrossCompileRid.EndsWith('-arm'))">arm</CrossCompileArch> | ||
| <CrossCompileAbi>gnu</CrossCompileAbi> | ||
| <CrossCompileAbiCondition="$(CrossCompileRid.EndsWith('-arm'))">gnueabihf</CrossCompileAbi> | ||
| <TargetTriple /> | ||
| <TargetTripleCondition="'$(CrossCompileArch)' != ''">$(CrossCompileArch)-linux-gnu</TargetTriple> | ||
| <TargetTripleCondition="'$(CrossCompileArch)' != ''">$(CrossCompileArch)-linux-$(CrossCompileAbi)</TargetTriple> |
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.
@am11 care to review this part?
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.
This looks good and neat. Other related/interesting triplets are:
arm-linux-androideabiarm-unknown-linux-gnueabiarm-unknown-linux-gnueabihfarm-unknown-linux-musleabiarm-unknown-linux-musleabihfarmv7-unknown-linux-gnueabiarmv7-unknown-linux-gnueabihfarmv7-unknown-linux-musleabiarmv7-unknown-linux-musleabihfof which, maybe<CrossCompileAbi Condition="'$(CrossCompileRid)' == 'linux-musl-arm'">musleabihf</CrossCompileAbi> could be added? We have prereq docker image available for it as well.
Side note:
AFAIK, triplet is considered an archaic concept by some toolchain enthusiasts. They recommend using explicit-march,-mtune,-mabi,-mcpu and-mfpu options. At some point we can delve into that and bring nativeaot and mono targets to new age. :)
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 guess I'll have some follow up PRs later on, so I can add the "musl" then.
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.
No problem, take your time. Basically the next line (49) can be deleted after that, sincealpine is unnecessary and more restrictive thanunknown or not setting the vendor at all; which we have for gnu and bionic.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
| #endif// FEATURE_EH_FUNCLETS | ||
| } | ||
| voidCompiler::unwindPushPopMaskCFI(regMaskTP regMask,bool isFloat) |
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.
Review note: This method is only ever called forTARGET_ARM. The order of registers was opposite to the one expected in DWARF. Along with the change inunwindPushPopCFI it now fixes the ARM target to generate correct DWARF descriptors.
Both of these issues were previously masked by the counter part code inObjWriter that converted DWARF codes to EHABI. Notably it reversed the register order at each code offset, and the.regsave opcode in the EHABI assembly automatically moved the call frame pointer by the register size.
filipnavara commentedJan 15, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Rough status of the linux-arm bring up:
|
jkotas left a comment
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.
LGTM. Thank you!
filipnavara commentedJan 16, 2024
I have a fix/workaround that I will submit in subsequent PR. There's a bug in the LLD linker in the way how it resolves |
jkotas commentedJan 16, 2024
license/cla bot is stuck... |
* Use PC-relative relocations for ARMEmitter.EmitMOV(dest, symbol)* Fix handling of signed offsets in IMAGE_REL_BASED_THUMB_BRANCH24 calculations* Generate position independent code in NativeAOT on linux-arm* Handle ARM32 in DwarfCie* Fix relocations emitted in 32-bit ELF for x86/arm32; apply correct addends to account for R2R ABI differences; use inline addend for 32-bit platforms; mark function symbols as Thumb code on ARM* ELF/ARM32: Emit thumb mapping symbols for executable sections* Try to revert ARMEmitter.EmitMOV* Convert IMAGE_REL_BASED_THUMB_MOV32_PCREL to ELF with the same offset as R2R* Unoptimal, but working, version of INLINE_GET_TLS_VAR for ARM32* Use PC-relative relocations for ARMEmitter.EmitMOV(dest, symbol)* Fat function pointers are not function symbols as far as ELF is concerned; the should not get the symbol size or the Thumb bit* Fix some bits and pieces of the ARM unwinding code* Don't try to use ObjWriter package on unsupported platforms* Generate valid ARM32 DWARF unwind info in JIT* Handle negative offsets in CFI_REL_OFFSET conversion (unused at the moment)* Add linux-arm support to cross-compile targets* Update src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm.incCo-authored-by: Jan Kotas <jkotas@microsoft.com>* Update ObjectWriter.csCo-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>* Fix the order of register push in CFI unwind codes on ARM.* Make jit-format happy without making the code look ugly---------Co-authored-by: Jan Kotas <jkotas@microsoft.com>Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
Uh oh!
There was an error while loading.Please reload this page.
Fixes enough of the code to get "Hello World" up and running on NativeAOT / linux-arm. I was testing on the "DwarfDump" smoke test and even the LLDB debugging works quite well. Got all the way to
RhThrowExwhich reaches the unimplemented parts of unwinding and exception handling.Generally speaking, it seems to be possible to use the DWARF unwinding by enabling the supporthere.
It still fails on the first attempt to unwind a frame with some bogus values in the registers, but it does seem to look up the LSDA pointers at least.