Skip to content

Conversation

@Stylie777
Copy link
Contributor

@Stylie777 Stylie777 commented Oct 20, 2025

When originally introduced to libunwind as part of #112171, FEAT_PAuthLR had its Call Frame Instruction's (CFI's) in a different location to other Signing Authentication methods. To incorporate this in libunwind, a 4 byte offset was introduced to work with this. However, this design was reversed in #121551 so the CFI's are emitted in the same location as other methods. When making this change, the offset in libunwind was not removed, so libunwind's PC value would be incorrect.

As the 4 byte offset is no longer needed, that adjustment can be removed. results->ptrAuthDiversifier will still be set.

When originally introduced to libunwind as part of llvm#112171,
FEAT_PAuthLR had its Call Frame Instruction's (CFI's) in a different
location to other Signing Authentication methods. To incorporate this
in libunwind, an offset was introduced to work with this. However,
this design was reversed in llvm#121551 so the CFI's are emitted in the
same location as other methods. When making this change, the offset
in libunwind was not removed, so libunwinds PC value would be incorrect.

This can be removed from the code, as it is no longer needed.
@Stylie777 Stylie777 requested a review from a team as a code owner October 20, 2025 09:08
@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2025

@llvm/pr-subscribers-libunwind

Author: Jack Styles (Stylie777)

Changes

When originally introduced to libunwind as part of #112171, FEAT_PAuthLR had its Call Frame Instruction's (CFI's) in a different location to other Signing Authentication methods. To incorporate this in libunwind, an offset was introduced to work with this. However, this design was reversed in #121551 so the CFI's are emitted in the same location as other methods. When making this change, the offset in libunwind was not removed, so libunwinds PC value would be incorrect.

This can be removed from the code, as it is no longer needed.


Full diff: https://github.com/llvm/llvm-project/pull/164224.diff

1 Files Affected:

  • (modified) libunwind/src/DwarfParser.hpp (-6)
diff --git a/libunwind/src/DwarfParser.hpp b/libunwind/src/DwarfParser.hpp
index 25250e0810987..625780f1f4558 100644
--- a/libunwind/src/DwarfParser.hpp
+++ b/libunwind/src/DwarfParser.hpp
@@ -808,12 +808,6 @@ bool CFI_Parser<A>::parseFDEInstructions(A &addressSpace,
             results->savedRegisters[UNW_AARCH64_RA_SIGN_STATE].value ^ 0x3;
         results->setRegisterValue(UNW_AARCH64_RA_SIGN_STATE, value,
                                   initialState);
-        // When calculating the value of the PC, it is assumed that the CFI
-        // instruction is placed before the signing instruction, however it is
-        // placed after. Because of this, we need to take into account the CFI
-        // instruction is one instruction call later than expected, and reduce
-        // the PC value by 4 bytes to compensate.
-        results->ptrAuthDiversifier = fdeInfo.pcStart + codeOffset - 0x4;
         _LIBUNWIND_TRACE_DWARF(
             "DW_CFA_AARCH64_negate_ra_state_with_pc(pc=0x%" PRIx64 ")\n",
             static_cast<uint64_t>(results->ptrAuthDiversifier));

We still need to set this value when we look at PAuthLR, it was just
the 4 byte offset that needed removing
// instruction is one instruction call later than expected, and reduce
// the PC value by 4 bytes to compensate.
results->ptrAuthDiversifier = fdeInfo.pcStart + codeOffset - 0x4;
results->ptrAuthDiversifier = fdeInfo.pcStart + codeOffset;
Copy link
Member

Choose a reason for hiding this comment

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

Can you edit this comment instead of entirely discarding it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@Stylie777 Stylie777 merged commit a9e6f90 into llvm:main Oct 29, 2025
76 of 77 checks passed
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 30, 2025
…64224)

When originally introduced to libunwind as part of llvm#112171, FEAT_PAuthLR
had its Call Frame Instruction's (CFI's) in a different location to
other Signing Authentication methods. To incorporate this in libunwind,
a 4 byte offset was introduced to work with this. However, this design
was reversed in llvm#121551 so the CFI's are emitted in the same location as
other methods. When making this change, the offset in libunwind was not
removed, so libunwind's PC value would be incorrect.

As the 4 byte offset is no longer needed, that adjustment can be
removed. results->ptrAuthDiversifier will still be set.
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
…64224)

When originally introduced to libunwind as part of llvm#112171, FEAT_PAuthLR
had its Call Frame Instruction's (CFI's) in a different location to
other Signing Authentication methods. To incorporate this in libunwind,
a 4 byte offset was introduced to work with this. However, this design
was reversed in llvm#121551 so the CFI's are emitted in the same location as
other methods. When making this change, the offset in libunwind was not
removed, so libunwind's PC value would be incorrect.

As the 4 byte offset is no longer needed, that adjustment can be
removed. results->ptrAuthDiversifier will still be set.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants