Skip to content

Track reg save locations via setRegisterLocation - #135488

Open
am11 wants to merge 4 commits into
dotnet:mainfrom
am11:feat/external/llvm-libunwind/reduce-patches
Open

am11 wants to merge 4 commits into
dotnet:mainfrom
am11:feat/external/llvm-libunwind/reduce-patches

Conversation

@am11

@am11 am11 commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Adapt the REGDISPLAY shims to the reduced llvm-libunwind patch:
setRegister only carries values (IP/SP), and save locations arrive
through setRegisterLocation. Pass pc + 1 to stepWithDwarf instead of
patching the CFI row comparison, use the upstream UnwindCursor
constructor, and call the templated findUnwindSections.

@am11
am11 requested a review from MichalStrehovsky as a code owner October 9, 2026 11:37
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 9, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib
See info in area-owners.md if you want to be subscribed.


uint64_t getRBP() const { return *pRbp; }
void setRBP(uint64_t value, uint64_t location) { pRbp = (PTR_uintptr_t)location; }
void setRBP(uint64_t) { }

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.

Is the expectation that these set... methods are never called and they exist just to make things compile? Should they call abort like other similar N/A methods in this file?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have cleaned up some of them, they are called on OSX which calls setRegisterLocation.

@am11

am11 commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

cc @janvorli, @filipnavara, @jkotas, this refactoring reduces the patch size we apply during llvm-libunwind updates.

When ready, please merge it without squashing.

@am11
am11 force-pushed the feat/external/llvm-libunwind/reduce-patches branch from a80b073 to 9df3e5a Compare October 9, 2026 13:27
am11 added 3 commits October 9, 2026 13:31
Adapt the `REGDISPLAY` shims to the reduced llvm-libunwind patch:
`setRegister` only carries values (IP/SP), and save locations arrive
through `setRegisterLocation`. Pass pc + 1 to `stepWithDwarf` instead of
patching the CFI row comparison, use the upstream `UnwindCursor`
constructor, and call the templated `findUnwindSections`.
@am11
am11 force-pushed the feat/external/llvm-libunwind/reduce-patches branch from 9df3e5a to 77eb937 Compare October 9, 2026 13:32
@am11
am11 force-pushed the feat/external/llvm-libunwind/reduce-patches branch from 77eb937 to 080ca24 Compare October 9, 2026 13:33
@am11

am11 commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Tiny improvement in size MichalStrehovsky/rt-sz#268. Flow-wise:

before:

DwarfInstructions / CompactUnwinder / Unwind-EHABI          (patched)
  setRegister(reg, value, location)  <-- 3rd arg everywhere
       |                          \
       v                           v
Registers_REGDISPLAY            Registers_x86_64/arm64/...   (patched)
  pRbx = location                 + location arrays  (unused)
                                  + getRegisterLocation (unused)
                                unw_cursor_t/unw_context_t enlarged
                                unw_set_reg / _Unwind_SetGR /
                                _Unwind_VRS_Set (+pos)  (public API changed)
                                unw_get_save_loc        (unused)
DwarfParser: codeOffset <= pcoffset                          (patched)

patch: 14 files, +668 / -278

after:

DwarfInstructions / CompactUnwinder
  setRegister(reg, value)               <-- upstream, unchanged
  setSavedRegisterLocation(reg, loc)    <-- new helper
       |
       |  R has setRegisterLocation()?
       +-- yes --> Registers_REGDISPLAY::setRegisterLocation
       |             pRbx = location
       +-- no  --> no-op   (libunwind's own register classes
                            stay upstream-sized)

Unwind-EHABI (ARM32)
  __unw_set_reg_location  -->  AbstractUnwindCursor::setRegLocation
    (hidden, internal)           default no-op; ArmUnwindCursor overrides

DwarfParser: upstream "<"      StepFrame passes pc + 1 instead

patch: 10 files, +160 / -48
public headers and DwarfParser.hpp: identical to upstream

@jkotas

jkotas commented Oct 11, 2026

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants