[libunwind][AIX] Fix problem with stepping up from a leaf function when unwinding started in a signal handler

[xingxue]

Summary

The implementation of AIX unwinder gets the return address from the link area of the stack frame of a function and uses the return address to walk up functions. However, when unwinding starts from a signal handler and the function that raised the signal happens to be a leaf function and it does not have its own stack frame, the return address of the stack frame of the leaf function points to the caller of the function that calls the leaf function because the leaf function and its caller share the same stack frame. As a result, the caller of the leaf function is skipped. This patch fixes the problem by saving the LR value in sigcontext when the unwinder hits the signal handler trampoline frame and using it as the return address of the leaf function. The LR value from sigcontext is saved in the unwinding context slot for LR currently unused.

[hubert.reinterpretcast]

libunwind/src/UnwindCursor.hpp

2395

Not all branches modify the link register.

[xingxue]

Addressed a comment:

• fix the comment - not all branches change LR.

[xingxue]

libunwind/src/UnwindCursor.hpp

2395

Fixed the comment, thanks!

[hubert.reinterpretcast]

libunwind/src/UnwindCursor.hpp

2324

This only covers the case when stores_bc is 1 in the traceback table.

2398

The comment should say nothing about buying a stack frame. The link area is usable by a function for saving the return address even when said function does not increment the SP. As @stephenpeckham pointed out "offline", there can be even be non-leaf functions that do not buy a stack frame (e.g., because the functions they call are known to not write to the stack).

2404

Might be good to explicitly set returnAddress to 0 if lastStack is 0. That is, make this else if (lastStack).

2404–2406

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements now states that braces should be used for each part of an if/else chain when one part needs braces.

2502–2506

If lastStack truly "points to the stack frame of the next routine up" (as claimed where it is declared), then it is fine to restore the SP even given a leaf function.

[xingxue]

Addressed comments:

• removed mentioning of buying stack frame in a code comment
• added if (!lastStack) and under the condition, set returnAddress to NULL
• removed condition TBTable->tb.saves_lr || !lastStack when restoring SP
• added code to initialize the LR value in context to 0

libunwind/src/UnwindCursor.hpp

2398

Removed the mentioning of buying a stack frame, thanks!

2404

Changed as suggested, thanks!

2404–2406

Changed as suggested, thanks!

2502–2506

I agree. Removed those conditions, thanks!

[stephenpeckham]

libunwind/src/UnwindCursor.hpp

2504

I think the check should be for TBTable->tb.stores_bc. It is possible for stores_bc to be 0 but saves_lr to be 1. This case can occur if a "leaf" function calls an internal routine that the compiler knows is stackless. In this case, the LR is saved in the stack before calling the internal function, but the stack pointer is not updated.

2539

This code wasn't changed, but the check could be done at the beginning of the function instead of the end. With this change, isSignalFrame does not need to be updated.

[hubert.reinterpretcast]

libunwind/src/UnwindCursor.hpp

2324

@xingxue, is a change coming for this? I think this also falls into "cases that arise when unwinding past a signal handler".

2504

I don't think there should a condition here at all. A fix needs to be made at line 2324 (otherwise the wrong link area would be used for retrieving a saved LR when stores_bc is 0 and saves_lr is 1).

[xingxue]

libunwind/src/UnwindCursor.hpp

2324

Yes, @stephenpeckham has something that deals with it. I will refresh once that is finalized.

[xingxue]

libunwind/src/UnwindCursor.hpp

2539

isSignalFrame needs to be undated for libunwind call unw_is_signal_frame().

[xingxue]

Addressed comments.

• add code to handle the case where a function does not store the back chain (stores_bc is 0)
• swap the order of getting sigcontext from a signal trampoline frame, now trying stack + STKMINALIGN first and then stack + STKMIN
• remove the condition check when restoring the SP
• add the test case for the case where a function does not store the back chain

[xingxue]

libunwind/src/UnwindCursor.hpp

2324

Incorporated @stephenpeckham's fix and use the current stack as the lastStack if stores_bc is 0. Thanks, Steve!

2504

I don't think there should a condition here at all. A fix needs to be made at line 2324 (otherwise the wrong link area would be used for retrieving a saved LR when stores_bc is 0 and saves_lr is 1).

Code has been added for the case where stores_bc is 0. Removed the condition check here. Thanks!

2504

I think the check should be for TBTable->tb.stores_bc. It is possible for stores_bc to be 0 but saves_lr to be 1. This case can occur if a "leaf" function calls an internal routine that the compiler knows is stackless. In this case, the LR is saved in the stack before calling the internal function, but the stack pointer is not updated.

Code has been added for the case where stores_bc is 0, thanks!

[stephenpeckham]

libunwind/src/UnwindCursor.hpp

2539

Retracting this comment. The value of _isSignalFrame can be queried, so the value has to be computed here.

libunwind/test/aix_signal_unwind.pass.sh.S

159 ↗

(On Diff #554785)

I suggest you add "bl $+4" to modify LR as if an internal function had been called.

208 ↗

(On Diff #554785)

add "bl $+4"

[xingxue]

Addressed comments:

• add instruction bl $+4 in the assembly code of the test case

[xingxue]

libunwind/test/aix_signal_unwind.pass.sh.S

159 ↗

(On Diff #554785)

Added the instruction as suggested, thanks!

208 ↗

(On Diff #554785)

Added the instruction as suggested, thanks!

[stephenpeckham]

Both the libunwind and testcase changes look good.

libunwind/src/Registers.hpp

public:
   static int  getArch() { return REGISTERS_PPC; }
 
   uint64_t  getSP() const         { return _registers.__r1; }
   void      setSP(uint32_t value) { _registers.__r1 = value; }
   uint64_t  getIP() const         { return _registers.__srr0; }
   void      setIP(uint32_t value) { _registers.__srr0 = value; }
   uint64_t  getCR() const         { return _registers.__cr; }
   void      setCR(uint32_t value) { _registers.__cr = value; }
+  uint64_t  getLR() const         { return _registers.__lr; }
+  void      setLR(uint32_t value) { _registers.__lr = value; }
 
 private:
   struct ppc_thread_state_t {
     unsigned int __srr0; /* Instruction address register (PC) */
     unsigned int __srr1; /* Machine state register (supervisor) */
     unsigned int __r0;
     unsigned int __r1;
     unsigned int __r2;
public:
   static int  getArch() { return REGISTERS_PPC64; }
 
   uint64_t  getSP() const         { return _registers.__r1; }
   void      setSP(uint64_t value) { _registers.__r1 = value; }
   uint64_t  getIP() const         { return _registers.__srr0; }
   void      setIP(uint64_t value) { _registers.__srr0 = value; }
   uint64_t  getCR() const         { return _registers.__cr; }
   void      setCR(uint64_t value) { _registers.__cr = value; }
+  uint64_t  getLR() const         { return _registers.__lr; }
+  void      setLR(uint64_t value) { _registers.__lr = value; }
 
 private:
   struct ppc64_thread_state_t {
     uint64_t __srr0;    // Instruction address register (PC)
     uint64_t __srr1;    // Machine state register (supervisor)
     uint64_t __r0;
     uint64_t __r1;
     uint64_t __r2;

libunwind/src/UnwindCursor.hpp

#else
   const uint32_t loadTOCRegInst = 0x80410014;
   const int32_t unwPPCF0Index = UNW_PPC_F0;
   const int32_t unwPPCV0Index = UNW_PPC_V0;
 #endif
 
   R newRegisters = registers;
 
   // lastStack points to the stack frame of the next routine up.
   pint_t lastStack = *(reinterpret_cast<pint_t *>(registers.getSP()));
 
   // Return address is the address after call site instruction.
   pint_t returnAddress;
 
   if (isSignalFrame) {
     _LIBUNWIND_TRACE_UNWINDING("Possible signal handler frame: lastStack=%p",
                                reinterpret_cast<void *>(lastStack));
 
if (returnAddress < 0x10000000) {
                                    "Seems to be a valid address\n",
                                    reinterpret_cast<void *>(sigContext),
                                    reinterpret_cast<void *>(returnAddress));
       }
     }
     // Restore the condition register from sigcontext.
     newRegisters.setCR(sigContext->sc_jmpbuf.jmp_context.cr);
 
+    // Save the LR in sigcontext for stepping up when the function that
+    // raised the signal is a leaf function. This LR has the return address
+    // to the caller of the leaf function.
+    newRegisters.setLR(sigContext->sc_jmpbuf.jmp_context.lr);
+    _LIBUNWIND_TRACE_UNWINDING(
+        "Save LR=%p from sigcontext\n",
+        reinterpret_cast<void *>(sigContext->sc_jmpbuf.jmp_context.lr));
+
     // Restore GPRs from sigcontext.
     for (int i = 0; i < 32; ++i)
       newRegisters.setRegister(i, sigContext->sc_jmpbuf.jmp_context.gpr[i]);
 
     // Restore FPRs from sigcontext.
     for (int i = 0; i < 32; ++i)
       newRegisters.setFloatRegister(i + unwPPCF0Index,
                                     sigContext->sc_jmpbuf.jmp_context.fpr[i]);
 
     // Restore vector registers if there is an associated extended context
     // structure.
     if (sigContext->sc_jmpbuf.jmp_context.msr & __EXTCTX) {
       ucontext_t *uContext = reinterpret_cast<ucontext_t *>(sigContext);
       if (uContext->__extctx->__extctx_magic == __EXTCTX_MAGIC) {
         for (int i = 0; i < 32; ++i)
           newRegisters.setVectorRegister(
               i + unwPPCV0Index, *(reinterpret_cast<v128 *>(
                                      &(uContext->__extctx->__vmx.__vr[i]))));
       }
     }
   } else {
     // Step up a normal frame.
+
+    bool usedLRFromSigContext;
+    if (!lastStack) {
+      returnAddress = NULL;
+      usedLRFromSigContext = false;
+    } else if (!TBTable->tb.saves_lr && registers.getLR()) {
+      // Use the LR value saved from sigcontext if LR is not saved in the
+      // stack frame.
+      returnAddress = registers.getLR();
+      usedLRFromSigContext = true;
+      _LIBUNWIND_TRACE_UNWINDING("Use saved LR=%p\n",
+                                 reinterpret_cast<void *>(returnAddress));
+    } else {
+      // Otherwise, use the LR value in the stack link area.
       returnAddress = reinterpret_cast<pint_t *>(lastStack)[2];
+      usedLRFromSigContext = false;
+    }
+
+    // Reset LR in the current context.
+    newRegisters.setLR(NULL);
 
     _LIBUNWIND_TRACE_UNWINDING("Extract info from lastStack=%p, "
                                "returnAddress=%p\n",
                                reinterpret_cast<void *>(lastStack),
                                reinterpret_cast<void *>(returnAddress));
     _LIBUNWIND_TRACE_UNWINDING("fpr_regs=%d, gpr_regs=%d, saves_cr=%d\n",
                                TBTable->tb.fpr_saved, TBTable->tb.gpr_saved,
                                TBTable->tb.saves_cr);
if (isSignalFrame) {
     }
     if (TBTable->tb.saves_cr) {
       // Get the saved condition register. The condition register is only
       // a single word.
       newRegisters.setCR(
           *(reinterpret_cast<uint32_t *>(lastStack + sizeof(uintptr_t))));
     }
 
-    // Restore the SP.
+    // Restore the SP and move one frame up except when the LR saved from
+    // sigcontext is used as the return address.
+    if (!usedLRFromSigContext)
       newRegisters.setSP(lastStack);
 
     // The first instruction after return.
     uint32_t firstInstruction = *(reinterpret_cast<uint32_t *>(returnAddress));
 
     // Do we need to set the TOC register?
     _LIBUNWIND_TRACE_UNWINDING(
         "Current gpr2=%p\n",
         reinterpret_cast<void *>(newRegisters.getRegister(2)));
     if (firstInstruction == loadTOCRegInst) {
#endif
   registers = newRegisters;
 
   // Check if the next frame is a signal frame.
   pint_t nextStack = *(reinterpret_cast<pint_t *>(registers.getSP()));
 
   // Return address is the address after call site instruction.
   pint_t nextReturnAddress = reinterpret_cast<pint_t *>(nextStack)[2];
 
   if (nextReturnAddress > 0x01 && nextReturnAddress < 0x10000) {
     _LIBUNWIND_TRACE_UNWINDING("The next is a signal handler frame: "
                                "nextStack=%p, next return address=%p\n",
                                reinterpret_cast<void *>(nextStack),
                                reinterpret_cast<void *>(nextReturnAddress));
     isSignalFrame = true;
   } else {
     isSignalFrame = false;
   }
-
   return UNW_STEP_SUCCESS;
 }
 #endif // defined(_LIBUNWIND_SUPPORT_TBTAB_UNWIND)
 
 template <typename A, typename R>
 void UnwindCursor<A, R>::setInfoBasedOnIPRegister(bool isReturnAddress) {
 #if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN)
   _isSigReturn = false;

libunwind/src/UnwindRegistersSave.S

#define PPC64_STR(n) \
   PPC64_STR(29)
   PPC64_STR(30)
   PPC64_STR(31)
 
   mfcr  0
   std   0,  PPC64_OFFS_CR(3)
   mfxer 0
   std   0,  PPC64_OFFS_XER(3)
+#if defined(_AIX)
+  // LR value saved from the register is not used, initialize it to 0.
+  li 0, 0
+#else
   mflr  0
+#endif
   std   0,  PPC64_OFFS_LR(3)
   mfctr 0
   std   0,  PPC64_OFFS_CTR(3)
   mfvrsave    0
   std   0,  PPC64_OFFS_VRSAVE(3)
 
 #if defined(__VSX__)
   // save VS registers

 #if defined(__ALTIVEC__)
   // save VRSave register
   mfspr   0, 256
   stw     0, 156(3)
 #endif
   // save CR registers
   mfcr    0
   stw     0, 136(3)
+#if defined(_AIX)
+  // LR value from the register is not used, initialize it to 0.
+  li      0, 0
+  stw     0, 144(3)
+#endif
   // save CTR register
   mfctr   0
   stw     0, 148(3)
 
 #if !defined(__NO_FPRS__)
   // save float registers
   stfd    0, 160(3)
   stfd    1, 168(3)

libunwind/test/aix_signal_unwind.pass.cpp

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Test that _Unwind_Backtrace() walks from a signal handler and produces
+// a correct traceback when the function raising the signal is a leaf function
+// in the middle of the call chain.
+
+// REQUIRES: target=powerpc{{(64)?}}-ibm-aix
+
+#undef NDEBUG
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/debug.h>
+#include <unwind.h>
+
+#define FUNC_ARRAY_SIZE 10
+
+struct func_t {
+  const char *name1; // Function name obtained in the function.
+  char *name2;       // Function name obtained during unwind.
+} func[FUNC_ARRAY_SIZE];
+
+int funcIndex = 0;
+int lastFuncIndex = 0;
+
+// Get the function name from traceback table.
+char *getFuncName(uintptr_t pc, uint16_t *nameLen) {
+  uint32_t *p = reinterpret_cast<uint32_t *>(pc);
+
+  // Keep looking forward until a word of 0 is found. The traceback
+  // table starts at the following word.
+  while (*p)
+    ++p;
+  tbtable *TBTable = reinterpret_cast<tbtable *>(p + 1);
+
+  if (!TBTable->tb.name_present)
+    return NULL;
+
+  // Get to the optional portion of the traceback table.
+  p = reinterpret_cast<uint32_t *>(&TBTable->tb_ext);
+
+  // Skip field parminfo if it exists.
+  if (TBTable->tb.fixedparms || TBTable->tb.floatparms)
+    ++p;
+
+  // Skip field tb_offset if it exists.
+  if (TBTable->tb.has_tboff)
+    ++p;
+
+  // Skip field hand_mask if it exists.
+  if (TBTable->tb.int_hndl)
+    ++p;
+
+  // Skip fields ctl_info and ctl_info_disp if they exist.
+  if (TBTable->tb.has_ctl)
+    p += 1 + *p;
+
+  *nameLen = *reinterpret_cast<uint16_t *>(p);
+  return reinterpret_cast<char *>(p) + sizeof(uint16_t);
+}
+
+_Unwind_Reason_Code callBack(struct _Unwind_Context *uc, void *arg) {
+  (void)arg;
+  if (funcIndex >= 0) {
+    uint16_t nameLen;
+    uintptr_t ip = _Unwind_GetIP(uc);
+    func[funcIndex--].name2 = strndup(getFuncName(ip, &nameLen), nameLen);
+  }
+  return _URC_NO_REASON;
+}
+
+extern "C" void handler(int signum) {
+  (void)signum;
+  func[funcIndex].name1 = __func__;
+  lastFuncIndex = funcIndex;
+
+  // Walk stack frames for traceback.
+  _Unwind_Backtrace(callBack, NULL);
+
+  // Verify the traceback.
+  for (int i = 0; i <= lastFuncIndex; ++i) {
+    assert(!strcmp(func[i].name1, func[i].name2) &&
+           "Function names do not match");
+    free(func[i].name2);
+  }
+  exit(0);
+}
+
+volatile int *null = 0;
+
+// abc() is a leaf function that raises signal SIGSEGV.
+extern "C" __attribute__((noinline)) void abc() {
+  func[funcIndex++].name1 = __func__;
+  // Produce a SIGSEGV.
+  *null = 0;
+}
+
+extern "C" __attribute__((noinline)) void bar() {
+  func[funcIndex++].name1 = __func__;
+  abc();
+}
+
+extern "C" __attribute__((noinline)) void foo() {
+  func[funcIndex++].name1 = __func__;
+  bar();
+}
+
+int main() {
+  // Set signal handler for SIGSEGV.
+  signal(SIGSEGV, handler);
+
+  func[funcIndex++].name1 = __func__;
+  foo();
+}