Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged
jkotas merged 21 commits intodotnet:mainfromfilipnavara:naot32
Jan 17, 2024

Conversation

@filipnavara
Copy link
Member

@filipnavarafilipnavara commentedJan 12, 2024
edited
Loading

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 toRhThrowEx which 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.

PaulusParssinen, AustinWise, rogihee, maxkatz6, and MrJul reacted with thumbs up emojiSingleAccretion, am11, PaulusParssinen, and maxkatz6 reacted with hooray emoji
@ghostghost added community-contributionIndicates that the PR has been added by a community member area-NativeAOT-coreclr labelsJan 12, 2024
@ghost
Copy link

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

Issue Details

null

Author:filipnavara
Assignees:-
Labels:

community-contribution,area-NativeAOT-coreclr

Milestone:-

@filipnavarafilipnavara added the NO-REVIEWExperimental/testing PR, do NOT review it labelJan 12, 2024
@filipnavarafilipnavara marked this pull request as ready for reviewJanuary 13, 2024 15:08
Comment on lines +152 to +159
if(destination<=Register.R7)
{
Builder.EmitShort(unchecked((short)(0x4478u+(byte)destination)));
}
else
{
Builder.EmitShort(unchecked((short)(0x44f0u+(byte)destination)));
}
Copy link
MemberAuthor

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.

Copy link
Member

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.

@filipnavarafilipnavara marked this pull request as draftJanuary 13, 2024 17:05
@filipnavarafilipnavara marked this pull request as ready for reviewJanuary 13, 2024 19:51
Comment on lines +42 to +48
<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>
Copy link
MemberAuthor

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?

am11 reacted with thumbs up emoji
Copy link
Member

@am11am11Jan 14, 2024
edited
Loading

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-musleabihf

of 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. :)

Copy link
MemberAuthor

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.

Copy link
Member

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.

filipnavara reacted with thumbs up emoji
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
#endif// FEATURE_EH_FUNCLETS
}

voidCompiler::unwindPushPopMaskCFI(regMaskTP regMask,bool isFloat)
Copy link
MemberAuthor

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
Copy link
MemberAuthor

filipnavara commentedJan 15, 2024
edited
Loading

Rough status of the linux-arm bring up:

  • Some of the assembler code is not position independent. It's relatively easy fix, keeping it for next round. Current workaround: Compile withPositionIndependentExecutable=false
  • FEATURE_64BIT_ALIGNMENT seems not to be defined for the GC and it sometimes triggers asserts. Current workaround: Define it in the CMakeLists.txt
  • Exception unwinding doesn't work. Current workaround: Enable DWARF unwinderhere. Ideally, we would emit EHABI exception data but it's not necessary for now. Bothlld andlldb know how to work with DWARF so linking and debugging works.
  • Exception handler address is misinterpreted somewhere (off by few bytes), so instructions at wrong address are executed. Exception handler return address is misinterpreted somewhere, so the app crashes after executing catch handler.Turns out the issue is that an address of a method is loaded with MOVW/MOVT instruction pair with a relocation and it's missing the thumb bit; later RhpCallCatchFunclet calls it withbx r4 and switches the processor to wrong mode.
  • Some assembly helpers are missing.
  • GC asserts onexecutionAborted

Copy link
Member

@jkotasjkotas left a 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
Copy link
MemberAuthor

  • Turns out the issue is that an address of a method is loaded with MOVW/MOVT instruction pair with a relocation and it's missing the thumb bit; later RhpCallCatchFunclet calls it withbx r4 and switches the processor to wrong mode.

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 resolvesR_ARM_THM_MOVW_PREL_NC relocations. Instead of((S + A) | T) – P it's resolved as(((S | T) + A)) – P (S | T is already in the symbol table, so that's not an explicit expression in the LLD code). RyuJIT generates the thumb bit in the embedded addend which is quite unusual but not prohibited.

@jkotas
Copy link
Member

license/cla bot is stuck...

@jkotasjkotas merged commit1a985d9 intodotnet:mainJan 17, 2024
@filipnavarafilipnavara deleted the naot32 branchJanuary 17, 2024 14:43
tmds pushed a commit to tmds/runtime that referenced this pull requestJan 23, 2024
* 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>
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsFeb 17, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@jkotasjkotasjkotas approved these changes

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments

+1 more reviewer

@am11am11am11 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

arch-arm32area-NativeAOT-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@filipnavara@jkotas@am11@MichalStrehovsky

[8]ページ先頭

©2009-2025 Movatter.jp