[libunwind] d3c7443 - Correctly update isSignalFrame when unwinding the stack via dwarf.

[Sterling Augustine via cfe-commits]

Author: Sterling Augustine
Date: 2019-11-07T14:48:35-08:00
New Revision: d3c744313c3cca0c076f031ec71e66ca74b12f2a

URL: https://github.com/llvm/llvm-project/commit/d3c744313c3cca0c076f031ec71e66ca74b12f2a
DIFF: https://github.com/llvm/llvm-project/commit/d3c744313c3cca0c076f031ec71e66ca74b12f2a.diff

LOG: Correctly update isSignalFrame when unwinding the stack via dwarf.

A "signal frame" is a function or block of code where execution arrives via a signal or interrupt, rather than via a normal call instruction. In fact, a particular instruction is interrupted by the signal and needs to be restarted. Therefore, when the signal handler is complete, execution needs to return to the interrupted instruction, rather than the instruction immediately following the call instruction, as in a normal call.

Stack unwinders need to know this to correctly unwind signal frames. Dwarf handily provides an "S" in the CIE augmentation string to describe this case, and the libunwind API provides various functions to for unwinders to determine it,.

The llvm libunwind implementation correctly sets it's internal variable "isSignalFrame" when initializing an unwind context. However, upon stepping up the stack, the current implementation correctly reads the augmentation string and sets it in the CIE info (which it then discards), libunwind doesn't update it's internal unwind context data structure.

This change fixes that, and provides compatibility with both the canonical libunwind and the libgcc implementation.

Reviewers: jfb

Subscribers: christof, libcxx-commits

Tags: #libc

Differential Revision: https://reviews.llvm.org/D69677

Added: 
    libunwind/test/signal_frame.pass.cpp

Modified: 
    libunwind/src/DwarfInstructions.hpp
    libunwind/src/UnwindCursor.hpp
    libunwind/src/UnwindLevel1-gcc-ext.c

Removed: 
    


################################################################################
diff  --git a/libunwind/src/DwarfInstructions.hpp b/libunwind/src/DwarfInstructions.hpp
index 29a070fa3e04..48ef1866d6e1 100644
--- a/libunwind/src/DwarfInstructions.hpp
+++ b/libunwind/src/DwarfInstructions.hpp
@@ -34,7 +34,7 @@ class DwarfInstructions {
   typedef typename A::sint_t sint_t;
 
   static int stepWithDwarf(A &addressSpace, pint_t pc, pint_t fdeStart,
-                           R &registers);
+                           R &registers, bool &isSignalFrame);
 
 private:
 
@@ -150,7 +150,8 @@ v128 DwarfInstructions<A, R>::getSavedVectorRegister(
 
 template <typename A, typename R>
 int DwarfInstructions<A, R>::stepWithDwarf(A &addressSpace, pint_t pc,
-                                           pint_t fdeStart, R &registers) {
+                                           pint_t fdeStart, R &registers,
+                                           bool &isSignalFrame) {
   FDE_Info fdeInfo;
   CIE_Info cieInfo;
   if (CFI_Parser<A>::decodeFDE(addressSpace, fdeStart, &fdeInfo,
@@ -196,6 +197,8 @@ int DwarfInstructions<A, R>::stepWithDwarf(A &addressSpace, pint_t pc,
       // restoring SP means setting it to CFA.
       newRegisters.setSP(cfa);
 
+      isSignalFrame = cieInfo.isSignalFrame;
+
 #if defined(_LIBUNWIND_TARGET_AARCH64)
       // If the target is aarch64 then the return address may have been signed
       // using the v8.3 pointer authentication extensions. The original

diff  --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index b4d44e111a65..4c18614b33d2 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -929,7 +929,7 @@ class UnwindCursor : public AbstractUnwindCursor{
     return DwarfInstructions<A, R>::stepWithDwarf(_addressSpace,
                                               (pint_t)this->getReg(UNW_REG_IP),
                                               (pint_t)_info.unwind_info,
-                                              _registers);
+                                              _registers, _isSignalFrame);
   }
 #endif
 

diff  --git a/libunwind/src/UnwindLevel1-gcc-ext.c b/libunwind/src/UnwindLevel1-gcc-ext.c
index 63e4083a4579..008df815665e 100644
--- a/libunwind/src/UnwindLevel1-gcc-ext.c
+++ b/libunwind/src/UnwindLevel1-gcc-ext.c
@@ -221,7 +221,14 @@ _LIBUNWIND_EXPORT uintptr_t _Unwind_GetCFA(struct _Unwind_Context *context) {
 _LIBUNWIND_EXPORT uintptr_t _Unwind_GetIPInfo(struct _Unwind_Context *context,
                                               int *ipBefore) {
   _LIBUNWIND_TRACE_API("_Unwind_GetIPInfo(context=%p)", (void *)context);
-  *ipBefore = 0;
+  int isSignalFrame = __unw_is_signal_frame((unw_cursor_t *)context);
+  // Negative means some kind of error (probably UNW_ENOINFO), but we have no
+  // good way to report that, and this maintains backward compatibility with the
+  // implementation that hard-coded zero in every case, even signal frames.
+  if (isSignalFrame <= 0)
+    *ipBefore = 0;
+  else
+    *ipBefore = 1;
   return _Unwind_GetIP(context);
 }
 

diff  --git a/libunwind/test/signal_frame.pass.cpp b/libunwind/test/signal_frame.pass.cpp
new file mode 100644
index 000000000000..b14e95a51528
--- /dev/null
+++ b/libunwind/test/signal_frame.pass.cpp
@@ -0,0 +1,25 @@
+// -*- 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
+//
+//===----------------------------------------------------------------------===//
+
+// Ensure that functions marked as signal frames are reported as such.
+
+#include <assert.h>
+#include <stdlib.h>
+#include <libunwind.h>
+
+int main(void) {
+  asm(".cfi_signal_frame");
+  unw_cursor_t cursor;
+  unw_context_t uc;
+  unw_getcontext(&uc);
+  unw_init_local(&cursor, &uc);
+  assert(unw_step(&cursor) > 0);
+  assert(unw_is_signal_frame(&cursor));
+  return 0;
+}