[llvm-readobj] [ARMWinEH] Fix the interpretation of packed unwind CR=01 RegI=1

[mstorsjo]

Even though the table for how to expand packed unwind info at [1] doesn't explicitly say this, this case is mentioned at [2] under the case "Only x19 saved":

sub    sp,sp,#16                // reg save area allocation*
stp    x19,lr,[sp]              // save x19, lr
sub    sp,sp,#(framesz-16)      // allocate the remaining local area

This was discussed and clarified at [3].

[1] https://learn.microsoft.com/en-us/cpp/build/arm64-exception-handling?view=msvc-170#packed-unwind-data
[2] https://learn.microsoft.com/en-us/cpp/build/arm64-exception-handling?view=msvc-170#arm64-stack-frame-layout
[3] #169588 (comment)

[mstorsjo]

CC @pmsjt @LouisLaf

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: Martin Storsjö (mstorsjo)

Changes

Even though the table for how to expand packed unwind info at [1] doesn't explicitly say this, this case is mentioned at [2] under the case "Only x19 saved":

sub    sp,sp,#<!-- -->16                // reg save area allocation*
stp    x19,lr,[sp]              // save x19, lr
sub    sp,sp,#(framesz-16)      // allocate the remaining local area

This was discussed and clarified at [3].

[1] https://learn.microsoft.com/en-us/cpp/build/arm64-exception-handling?view=msvc-170#packed-unwind-data
[2] https://learn.microsoft.com/en-us/cpp/build/arm64-exception-handling?view=msvc-170#arm64-stack-frame-layout
[3] #169588 (comment)

---

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

2 Files Affected:

• (modified) llvm/test/tools/llvm-readobj/COFF/arm64-packed-unwind.s (+2-1)
• (modified) llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp (+8-4)

diff --git a/llvm/test/tools/llvm-readobj/COFF/arm64-packed-unwind.s b/llvm/test/tools/llvm-readobj/COFF/arm64-packed-unwind.s
index d9953ccc3f3d8..72e79b77a01ad 100644
--- a/llvm/test/tools/llvm-readobj/COFF/arm64-packed-unwind.s
+++ b/llvm/test/tools/llvm-readobj/COFF/arm64-packed-unwind.s
@@ -139,7 +139,8 @@
 // CHECK-NEXT:     FrameSize: 32
 // CHECK-NEXT:     Prologue [
 // CHECK-NEXT:       sub sp, sp, #16
-// CHECK-NEXT:       INVALID!
+// CHECK-NEXT:       stp x19, lr, [sp]
+// CHECK-NEXT:       sub sp, sp, #16
 // CHECK-NEXT:       end
 // CHECK-NEXT:     ]
 // CHECK-NEXT:   }
diff --git a/llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp b/llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp
index c6e409c63ef3a..8efd9f0182d53 100644
--- a/llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp
+++ b/llvm/tools/llvm-readobj/ARMWinEHPrinter.cpp
@@ -1457,10 +1457,14 @@ bool Decoder::dumpPackedARM64Entry(const object::COFFObjectFile &COFF,
       // The last register, an odd register without a pair
       if (RF.CR() == 1) {
         if (I == 0) { // If this is the only register pair
-          // CR=1 combined with RegI=1 doesn't map to a documented case;
-          // it doesn't map to any regular unwind info opcode, and the
-          // actual unwinder doesn't support it.
-          SW.startLine() << "INVALID!\n";
+          // CR=1 combined with RegI=1 maps to a special case; there's
+          // no unwind info opcode that saves a GPR together with LR
+          // with writeback to sp (no save_lrpair_x).
+          // Instead, this case expands to two instructions; a preceding
+          // (in prologue execution order) "sub sp, sp, #16", followed
+          // by a regular "stp x19, lr, [sp]" (save_lrpair).
+          SW.startLine() << format("stp x%d, lr, [sp]\n", 19);
+          SW.startLine() << format("sub sp, sp, #16\n");
         } else
           SW.startLine() << format("stp x%d, lr, [sp, #%d]\n", 19 + 2 * I,
                                    16 * I);

[efriedma-quic]

LGTM

[efriedma-quic]

Should we print a warning if RegF() > 0?

[mstorsjo]

Nah, I don't think that's necessary. It is well defined and works correctly on the latest versions, so it makes sense to just dump it the right way, and we just avoid creating it.

[efriedma-quic]

Okay

[efriedma-quic]

Okay