Skip to content

[NativeAOT] Fix floating pointer register unwinding on Arm32#118944

Merged
jkotas merged 3 commits into
dotnet:mainfrom
jkotas:arm32-revert
Aug 27, 2025
Merged

[NativeAOT] Fix floating pointer register unwinding on Arm32#118944
jkotas merged 3 commits into
dotnet:mainfrom
jkotas:arm32-revert

Conversation

@jkotas

@jkotas jkotas commented Aug 20, 2025

Copy link
Copy Markdown
Member

NativeAOT specific register window implementation has to match the one in libunwind.

Fixes #118942.

@jkotas

jkotas commented Aug 20, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@jkotas

jkotas commented Aug 21, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas

jkotas commented Aug 21, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas

jkotas commented Aug 21, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas

jkotas commented Aug 22, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas

jkotas commented Aug 23, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

NativeAOT specific register window implementation has to match the one in libunwind.
@jkotas jkotas changed the title Partial revert "[NativeAOT] Fix floating pointer register unwinding (#117588)" [NativeAOT] Fix floating pointer register unwinding on Arm32 Aug 23, 2025
Comment thread src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas

jkotas commented Aug 24, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas
jkotas marked this pull request as ready for review August 24, 2025 07:07
Copilot AI review requested due to automatic review settings August 24, 2025 07:07
@am11

am11 commented Aug 24, 2025

Copy link
Copy Markdown
Member

Fixes #118942.

@jkotas
jkotas requested review from VSadov and janvorli August 25, 2025 13:11
@MichalStrehovsky

Copy link
Copy Markdown
Member

Do you know what was the exact condition that was triggering this? If it's something sane, I wonder if it would be worth a regression test.

If we weren't uploading XML files with test results into AzDo and AzDo wasn't rejecting the durations, we'd basically never hit this in any of our testing.

@jkotas

jkotas commented Aug 26, 2025

Copy link
Copy Markdown
Member Author

Do you know what was the exact condition that was triggering this?

The distilled repro would be a chain of multiple methods that interleave floating-point computation and exception handling in a specific shape, something like this:

void M1()
{
    double mustBeUsedMultipleTimesToGetEnregisteredIntoCalledSavedFPRegister;
    mustBeUsedMultipleTimesToGetEnregisteredIntoCalledSavedFPRegister = some computation;
    M2();
    do more stuff with mustBeUsedMultipleTimesToGetEnregisteredIntoCalledSavedFPRegister;
}

[NoInline]
void M2()
{
    // This method should not use any floating point
    try
    {
        M3();
    }
    catch
    {
    }
}

[NoInline]
void M3()
{
    // The JIT must enregister this into same register as the local variable in M1()
    double mustBeUsedMultipleTimesToGetEnregisteredIntoCalledSavedFPRegister;
    mustBeUsedMultipleTimesToGetEnregisteredIntoCalledSavedFPRegister = some other computation;
    M4();`
    do more stuff with mustBeUsedMultipleTimesToGetEnregisteredIntoCalledSavedFPRegister;
}

[NoInline]
void M4()
{
    throw new Exception();
}

I can try to add a test like this.

@janvorli janvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

@jkotas

jkotas commented Aug 26, 2025

Copy link
Copy Markdown
Member Author

I can try to add a test like this.

It turns out we have multiple tests like that, but they are non-effective for various reasons. I am just going to fix one of the existing tests do what it meant to test.

@jkotas

jkotas commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

Pushed test updated. #119108 validated that the updated test fails on both linux-arm and linux-arm64 without the fixes.

@jkotas

jkotas commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas jkotas closed this Aug 27, 2025
@jkotas jkotas reopened this Aug 27, 2025
@jkotas

jkotas commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas

jkotas commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas

jkotas commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

/ba-g infrastructure timeout

@jkotas
jkotas merged commit ed57bae into dotnet:main Aug 27, 2025
121 of 126 checks passed
@jkotas
jkotas deleted the arm32-revert branch August 27, 2025 19:32
@jkotas

jkotas commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

/backport to release/10.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/17276805531

@github-actions github-actions Bot locked and limited conversation to collaborators Sep 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ARM32 unwinding issue

5 participants