[NativeAOT] Fix floating pointer register unwinding - #117588

Merged
jkotas merged 2 commits into
dotnet:mainfrom
jkotas:fix-116276
Jul 15, 2025
Merged

[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom
jkotas:fix-116276

Conversation

@jkotas

@jkotasjkotas commented Jul 13, 2025

Copy link
Copy Markdown
Member

Fixes#116276

CopilotAI review requested due to automatic review settings July 13, 2025 19:35
CopilotAI reviewed Jul 13, 2025
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.

  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)

src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522

  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service

dotnet-policy-serviceBot commented Jul 13, 2025

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.

@risc-vv

risc-vv commented Jul 13, 2025

Copy link
Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.

@jkotas

jkotas commented Jul 13, 2025•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).

This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.

The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.

@risc-vv

risc-vv commented Jul 13, 2025•
edited
Loading

Copy link
Copy Markdown
RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: a76fcf3a5593a61df9555e5e1202244b46d29877
CI: d6c9c1ab3a7411819463edc05ded301e89ba586a
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

jkotas
jkotas commented Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas

jkotas commented Jul 13, 2025

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-nativeaot-outerloop

@azure-pipelines

azure-pipelinesBot commented Jul 13, 2025

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

@filipnavara

filipnavara commented Jul 13, 2025•
edited
Loading

Copy link
Copy Markdown
Member

Wow, amazing work on getting to the bottom of it!

(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)

@jkotas

jkotas commented Jul 13, 2025

Copy link
Copy Markdown
MemberAuthor

I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.

Right, it is the case for default calling conventions of all platforms that we support currently.

@filipnavara

filipnavara commented Jul 13, 2025

Copy link
Copy Markdown
Member

It rechecked the specs and seems to be fine for ARM64 and LA64.

RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025
Open
Open
@am11

am11 commented Jul 14, 2025•
edited
Loading

Copy link
Copy Markdown
Member

However, that was already broken prior to this PR

I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).

I don't have any hardware or toolchain that supports this.

Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.

am11
am11 reviewed Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp Outdated
jkotas
jkotas commented Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp Outdated
@risc-vv

risc-vv commented Jul 14, 2025•
edited
Loading

Copy link
Copy Markdown
RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: 5543649c55213eef586b31081a059b12ee0af99f
CI: d6c9c1ab3a7411819463edc05ded301e89ba586a
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

@jkotas

jkotas commented Jul 14, 2025•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.

It is in the same category as #8300 or #5040 .

Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.

@jkotas
jkotas requested a review from janvorliJuly 14, 2025 05:56
@jkotas

jkotas commented Jul 14, 2025

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-nativeaot-outerloop

@azure-pipelines

azure-pipelinesBot commented Jul 14, 2025

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

@filipnavara

filipnavara commented Jul 14, 2025

Copy link
Copy Markdown
Member

I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).

Right. I don't think there's currently a code path that could hit it.

It is in the same category as #8300 or #5040 .

I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).

janvorli
janvorli approved these changes Jul 14, 2025
View reviewed changes

@janvorlijanvorli 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 Jul 15, 2025

Copy link
Copy Markdown
MemberAuthor

/ba-g known Android timeout

@jkotas
jkotas merged commit 6123c24 into dotnet:mainJul 15, 2025
@jkotas
jkotas deleted the fix-116276 branch July 15, 2025 12:09
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner
+2 more reviewers
@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.

6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
[NativeAOT] Fix floating pointer register unwinding - #117588
Merged
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Jul 15, 2025
Merged
[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Conversation
@jkotas
@jkotasjkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
Fixes#116276
CopilotAI
review requested due to automatic review settings
July 13, 2025 19:35
CopilotAI
reviewed
Jul 13, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.
  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522
  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 13, 2025

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.
@risc-vv
risc-vv
commented
Jul 13, 2025

Copy link


Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.
@jkotas
jkotas
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).
This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.
The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.
@risc-vv
risc-vv
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: a76fcf3a5593a61df9555e5e1202244b46d29877

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
jkotas
jkotas
commented
Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Wow, amazing work on getting to the bottom of it!
(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.
Right, it is the case for default calling conventions of all platforms that we support currently.
@filipnavara
filipnavara
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
It rechecked the specs and seems to be fine for ARM64 and LA64.
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025

Open


Open

@am11
am11
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

Member
However, that was already broken prior to this PR
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
I don't have any hardware or toolchain that supports this.
Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.
am11
am11
reviewed
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

jkotas
jkotas
commented
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

@risc-vv
risc-vv
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: 5543649c55213eef586b31081a059b12ee0af99f

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
@jkotas
jkotas
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.
It is in the same category as #8300 or #5040 .
Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.
@jkotas
jkotas
requested a review
from janvorliJuly 14, 2025 05:56
@jkotas
jkotas
commented
Jul 14, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 14, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 14, 2025

Copy link


Copy Markdown

Member
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
Right. I don't think there's currently a code path that could hit it.
It is in the same category as #8300 or #5040 .
I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).
janvorli
janvorli
approved these changes
Jul 14, 2025
View reviewed changes
@janvorlijanvorli
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
Jul 15, 2025

Copy link


Copy Markdown

MemberAuthor
/ba-g known Android timeout
@jkotas
jkotas
merged commit 6123c24
into
dotnet:mainJul 15, 2025
@jkotas
jkotas
deleted the

fix-116276

branch
July 15, 2025 12:09
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+2 more reviewers

@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.


6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NativeAOT] Fix floating pointer register unwinding - #117588
Merged
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Jul 15, 2025
Merged
[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Conversation
@jkotas
@jkotasjkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
Fixes#116276
CopilotAI
review requested due to automatic review settings
July 13, 2025 19:35
CopilotAI
reviewed
Jul 13, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.
  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522
  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 13, 2025

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.
@risc-vv
risc-vv
commented
Jul 13, 2025

Copy link


Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.
@jkotas
jkotas
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).
This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.
The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.
@risc-vv
risc-vv
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: a76fcf3a5593a61df9555e5e1202244b46d29877

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
jkotas
jkotas
commented
Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Wow, amazing work on getting to the bottom of it!
(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.
Right, it is the case for default calling conventions of all platforms that we support currently.
@filipnavara
filipnavara
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
It rechecked the specs and seems to be fine for ARM64 and LA64.
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025

Open


Open

@am11
am11
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

Member
However, that was already broken prior to this PR
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
I don't have any hardware or toolchain that supports this.
Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.
am11
am11
reviewed
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

jkotas
jkotas
commented
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

@risc-vv
risc-vv
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: 5543649c55213eef586b31081a059b12ee0af99f

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
@jkotas
jkotas
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.
It is in the same category as #8300 or #5040 .
Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.
@jkotas
jkotas
requested a review
from janvorliJuly 14, 2025 05:56
@jkotas
jkotas
commented
Jul 14, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 14, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 14, 2025

Copy link


Copy Markdown

Member
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
Right. I don't think there's currently a code path that could hit it.
It is in the same category as #8300 or #5040 .
I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).
janvorli
janvorli
approved these changes
Jul 14, 2025
View reviewed changes
@janvorlijanvorli
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
Jul 15, 2025

Copy link


Copy Markdown

MemberAuthor
/ba-g known Android timeout
@jkotas
jkotas
merged commit 6123c24
into
dotnet:mainJul 15, 2025
@jkotas
jkotas
deleted the

fix-116276

branch
July 15, 2025 12:09
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+2 more reviewers

@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.


6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NativeAOT] Fix floating pointer register unwinding - #117588
Merged
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Jul 15, 2025
Merged
[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Conversation
@jkotas
@jkotasjkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
Fixes#116276
CopilotAI
review requested due to automatic review settings
July 13, 2025 19:35
CopilotAI
reviewed
Jul 13, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.
  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522
  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 13, 2025

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.
@risc-vv
risc-vv
commented
Jul 13, 2025

Copy link


Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.
@jkotas
jkotas
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).
This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.
The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.
@risc-vv
risc-vv
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: a76fcf3a5593a61df9555e5e1202244b46d29877

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
jkotas
jkotas
commented
Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Wow, amazing work on getting to the bottom of it!
(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.
Right, it is the case for default calling conventions of all platforms that we support currently.
@filipnavara
filipnavara
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
It rechecked the specs and seems to be fine for ARM64 and LA64.
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025

Open


Open

@am11
am11
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

Member
However, that was already broken prior to this PR
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
I don't have any hardware or toolchain that supports this.
Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.
am11
am11
reviewed
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

jkotas
jkotas
commented
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

@risc-vv
risc-vv
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: 5543649c55213eef586b31081a059b12ee0af99f

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
@jkotas
jkotas
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.
It is in the same category as #8300 or #5040 .
Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.
@jkotas
jkotas
requested a review
from janvorliJuly 14, 2025 05:56
@jkotas
jkotas
commented
Jul 14, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 14, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 14, 2025

Copy link


Copy Markdown

Member
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
Right. I don't think there's currently a code path that could hit it.
It is in the same category as #8300 or #5040 .
I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).
janvorli
janvorli
approved these changes
Jul 14, 2025
View reviewed changes
@janvorlijanvorli
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
Jul 15, 2025

Copy link


Copy Markdown

MemberAuthor
/ba-g known Android timeout
@jkotas
jkotas
merged commit 6123c24
into
dotnet:mainJul 15, 2025
@jkotas
jkotas
deleted the

fix-116276

branch
July 15, 2025 12:09
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+2 more reviewers

@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.


6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
[NativeAOT] Fix floating pointer register unwinding - #117588
Merged
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Jul 15, 2025
Merged
[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Conversation
@jkotas
@jkotasjkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
Fixes#116276
CopilotAI
review requested due to automatic review settings
July 13, 2025 19:35
CopilotAI
reviewed
Jul 13, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.
  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522
  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 13, 2025

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.
@risc-vv
risc-vv
commented
Jul 13, 2025

Copy link


Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.
@jkotas
jkotas
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).
This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.
The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.
@risc-vv
risc-vv
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: a76fcf3a5593a61df9555e5e1202244b46d29877

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
jkotas
jkotas
commented
Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Wow, amazing work on getting to the bottom of it!
(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.
Right, it is the case for default calling conventions of all platforms that we support currently.
@filipnavara
filipnavara
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
It rechecked the specs and seems to be fine for ARM64 and LA64.
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025

Open


Open

@am11
am11
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

Member
However, that was already broken prior to this PR
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
I don't have any hardware or toolchain that supports this.
Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.
am11
am11
reviewed
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

jkotas
jkotas
commented
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

@risc-vv
risc-vv
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: 5543649c55213eef586b31081a059b12ee0af99f

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
@jkotas
jkotas
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.
It is in the same category as #8300 or #5040 .
Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.
@jkotas
jkotas
requested a review
from janvorliJuly 14, 2025 05:56
@jkotas
jkotas
commented
Jul 14, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 14, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 14, 2025

Copy link


Copy Markdown

Member
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
Right. I don't think there's currently a code path that could hit it.
It is in the same category as #8300 or #5040 .
I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).
janvorli
janvorli
approved these changes
Jul 14, 2025
View reviewed changes
@janvorlijanvorli
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
Jul 15, 2025

Copy link


Copy Markdown

MemberAuthor
/ba-g known Android timeout
@jkotas
jkotas
merged commit 6123c24
into
dotnet:mainJul 15, 2025
@jkotas
jkotas
deleted the

fix-116276

branch
July 15, 2025 12:09
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+2 more reviewers

@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.


6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NativeAOT] Fix floating pointer register unwinding - #117588
Merged
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Jul 15, 2025
Merged
[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Conversation
@jkotas
@jkotasjkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
Fixes#116276
CopilotAI
review requested due to automatic review settings
July 13, 2025 19:35
CopilotAI
reviewed
Jul 13, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.
  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522
  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 13, 2025

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.
@risc-vv
risc-vv
commented
Jul 13, 2025

Copy link


Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.
@jkotas
jkotas
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).
This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.
The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.
@risc-vv
risc-vv
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: a76fcf3a5593a61df9555e5e1202244b46d29877

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
jkotas
jkotas
commented
Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Wow, amazing work on getting to the bottom of it!
(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.
Right, it is the case for default calling conventions of all platforms that we support currently.
@filipnavara
filipnavara
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
It rechecked the specs and seems to be fine for ARM64 and LA64.
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025

Open


Open

@am11
am11
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

Member
However, that was already broken prior to this PR
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
I don't have any hardware or toolchain that supports this.
Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.
am11
am11
reviewed
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

jkotas
jkotas
commented
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

@risc-vv
risc-vv
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: 5543649c55213eef586b31081a059b12ee0af99f

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
@jkotas
jkotas
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.
It is in the same category as #8300 or #5040 .
Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.
@jkotas
jkotas
requested a review
from janvorliJuly 14, 2025 05:56
@jkotas
jkotas
commented
Jul 14, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 14, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 14, 2025

Copy link


Copy Markdown

Member
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
Right. I don't think there's currently a code path that could hit it.
It is in the same category as #8300 or #5040 .
I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).
janvorli
janvorli
approved these changes
Jul 14, 2025
View reviewed changes
@janvorlijanvorli
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
Jul 15, 2025

Copy link


Copy Markdown

MemberAuthor
/ba-g known Android timeout
@jkotas
jkotas
merged commit 6123c24
into
dotnet:mainJul 15, 2025
@jkotas
jkotas
deleted the

fix-116276

branch
July 15, 2025 12:09
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+2 more reviewers

@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.


6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[NativeAOT] Fix floating pointer register unwinding - #117588
Merged
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Jul 15, 2025
Merged
[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Conversation
@jkotas
@jkotasjkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
Fixes#116276
CopilotAI
review requested due to automatic review settings
July 13, 2025 19:35
CopilotAI
reviewed
Jul 13, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.
  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522
  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 13, 2025

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.
@risc-vv
risc-vv
commented
Jul 13, 2025

Copy link


Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.
@jkotas
jkotas
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).
This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.
The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.
@risc-vv
risc-vv
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: a76fcf3a5593a61df9555e5e1202244b46d29877

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
jkotas
jkotas
commented
Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Wow, amazing work on getting to the bottom of it!
(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.
Right, it is the case for default calling conventions of all platforms that we support currently.
@filipnavara
filipnavara
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
It rechecked the specs and seems to be fine for ARM64 and LA64.
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025

Open


Open

@am11
am11
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

Member
However, that was already broken prior to this PR
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
I don't have any hardware or toolchain that supports this.
Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.
am11
am11
reviewed
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

jkotas
jkotas
commented
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

@risc-vv
risc-vv
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: 5543649c55213eef586b31081a059b12ee0af99f

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
@jkotas
jkotas
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.
It is in the same category as #8300 or #5040 .
Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.
@jkotas
jkotas
requested a review
from janvorliJuly 14, 2025 05:56
@jkotas
jkotas
commented
Jul 14, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 14, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 14, 2025

Copy link


Copy Markdown

Member
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
Right. I don't think there's currently a code path that could hit it.
It is in the same category as #8300 or #5040 .
I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).
janvorli
janvorli
approved these changes
Jul 14, 2025
View reviewed changes
@janvorlijanvorli
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
Jul 15, 2025

Copy link


Copy Markdown

MemberAuthor
/ba-g known Android timeout
@jkotas
jkotas
merged commit 6123c24
into
dotnet:mainJul 15, 2025
@jkotas
jkotas
deleted the

fix-116276

branch
July 15, 2025 12:09
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+2 more reviewers

@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.


6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
[NativeAOT] Fix floating pointer register unwinding - #117588
Merged
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Jul 15, 2025
Merged
[NativeAOT] Fix floating pointer register unwinding#117588
jkotas merged 2 commits into
dotnet:mainfrom 
jkotas:fix-116276
Conversation
@jkotas
@jkotasjkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
Fixes#116276
CopilotAI
review requested due to automatic review settings
July 13, 2025 19:35
CopilotAI
reviewed
Jul 13, 2025
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull Request Overview
This PR fixes the floating-point register unwinding logic across several architectures by restricting valid ranges to the correct registers and replacing manual assertions/assignments with a bitcast helper.
  • Introduces unwindhelpers_bitcast for safe type-punning via memcpy.
  • Updates validFloatRegister, getFloatRegister, and setFloatRegister for ARM, ARM64, Loongarch64, and RISC-V to use the new bitcast and correct register ranges.
  • Removes legacy vector-register handling and replaces PORTABILITY_ASSERT with assert for invalid-register checks.
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp:522
  • Consider adding targeted unit tests for each architecture to verify that all valid floating-point registers are correctly unwound using the new unwindhelpers_bitcast implementation.
 return unwindhelpers_bitcast<double>(D[num - UNW_ARM_D8]);

Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 13, 2025

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.
@risc-vv
risc-vv
commented
Jul 13, 2025

Copy link


Copy Markdown

@dotnet/samsung Could you please take a look? These changes may be related to riscv64.
@jkotas
jkotas
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
The unwinder tried to treat floating point registers as vector registers. It led to all sorts of problems with storage size (8 byte storage for double vs. 16 byte storage for vector registers).
This bug was originally introduced by dotnet/corert#8290 . It is surprising that it took years to uncover it.
The bug got copied from arm64 unwinder to arm, riscv and loongarch unwinders in various forms. I have fixed those as well.
@risc-vv
risc-vv
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-QEMU: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 37h 33min 25s 850ms
REAL time: 38min 15s 882ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-CLR-VF2: 9084 / 9114 (99.67%)
=======================
passed: 9084
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9711
VIRTUAL time: 11h 58min 52s 922ms
REAL time: 48min 22s 894ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283771 / 284850 (99.62%)
=======================
passed: 283771
failed: 1070
skipped: 39
killed: 9
------------------------
TOTAL tests: 284889
VIRTUAL time: 32h 18min 24s 649ms
REAL time: 1h 10min 32s 31ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-VF2: 309260 / 311009 (99.44%)
=======================
passed: 309260
failed: 1741
skipped: 39
killed: 8
------------------------
TOTAL tests: 311048
VIRTUAL time: 21h 18min 26s 682ms
REAL time: 2h 10min 49s 503ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: a76fcf3a5593a61df9555e5e1202244b46d29877

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
jkotas
jkotas
commented
Jul 13, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 13, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 13, 2025•

edited

Loading

Copy link


Copy Markdown

Member
Wow, amazing work on getting to the bottom of it!
(I guess I'll need to re-read the DWARF specs to figure out how platforms with overlapping vector and FP registers should behave. I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.)
@jkotas
jkotas
commented
Jul 13, 2025

Copy link


Copy Markdown

MemberAuthor
I assume that most platforms don't save the high bits of vector registers but I am not sure if that's universally true.
Right, it is the case for default calling conventions of all platforms that we support currently.
@filipnavara
filipnavara
commented
Jul 13, 2025

Copy link


Copy Markdown

Member
It rechecked the specs and seems to be fine for ARM64 and LA64.
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that. However, that was already broken prior to this PR and I don't have any hardware or toolchain that supports this. (cc @am11 FYI)

This was referenced Jul 14, 2025

Open


Open

@am11
am11
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

Member
However, that was already broken prior to this PR
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
I don't have any hardware or toolchain that supports this.
Going by https://godbolt.org/z/d43s88nfE, at least it seems to know how to pass RVV types in v‑register.
am11
am11
reviewed
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

jkotas
jkotas
commented
Jul 14, 2025
View reviewed changes
Comment threadsrc/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp
Outdated

@risc-vv
risc-vv
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

RISC-V Release-CLR-VF2: 9083 / 9113 (99.67%)
=======================
passed: 9083
failed: 2
skipped: 597
killed: 28
------------------------
TOTAL tests: 9710
VIRTUAL time: 11h 5min 47s 145ms
REAL time: 45min 18s 492ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
RISC-V Release-FX-QEMU: 283858 / 284941 (99.62%)
=======================
passed: 283858
failed: 1074
skipped: 39
killed: 9
------------------------
TOTAL tests: 284980
VIRTUAL time: 32h 34min 23s 945ms
REAL time: 1h 11min 0s 439ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst
Build information and commands
GIT: 5543649c55213eef586b31081a059b12ee0af99f

CI: d6c9c1ab3a7411819463edc05ded301e89ba586a

REPO: dotnet/runtime

BRANCH: main

CONFIG: Release

LIB_CONFIG: Release
@jkotas
jkotas
commented
Jul 14, 2025•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
RV64 ratified an optional vector calling convention last year (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#calling-convention-variant) so we may eventually need to support that.
It is in the same category as #8300 or #5040 .
Also, RISCV vector extension is variable length like ARM SVE, so I expect we would want to finish implementing ARM SVE first and then base RISCV vector extension on that.
@jkotas
jkotas
requested a review
from janvorliJuly 14, 2025 05:56
@jkotas
jkotas
commented
Jul 14, 2025

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-nativeaot-outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jul 14, 2025

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@filipnavara
filipnavara
commented
Jul 14, 2025

Copy link


Copy Markdown

Member
I think we are not emitting RVV types from JIT and neither are we compiling with rv64gcv (yet).
Right. I don't think there's currently a code path that could hit it.
It is in the same category as #8300 or #5040 .
I was thinking more in the terms of unwinding native code like the GC poll code path. We likely cannot hit any vectorized code there (yet).
janvorli
janvorli
approved these changes
Jul 14, 2025
View reviewed changes
@janvorlijanvorli
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
Jul 15, 2025

Copy link


Copy Markdown

MemberAuthor
/ba-g known Android timeout
@jkotas
jkotas
merged commit 6123c24
into
dotnet:mainJul 15, 2025
@jkotas
jkotas
deleted the

fix-116276

branch
July 15, 2025 12:09
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Aug 15, 2025
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

Copilot code reviewCopilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@janvorlijanvorlijanvorli approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

+2 more reviewers

@am11am11am11 left review comments
@filipnavarafilipnavarafilipnavara approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jkotasjkotas

Labels

area-NativeAOT-coreclr

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

TimeSpan overflowed because the duration is too long.


6 participants

@jkotas@risc-vv@filipnavara@am11@janvorli