From 8c8c78e2db09c5dc66ad0188a088b1664483a13f Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@juliacomputing.com>
Date: Sun, 29 Aug 2021 11:07:54 -0700
Subject: [PATCH] x86_64: Stop aliasing RSP and CFA

RSP and CFA are different concepts. RSP refers to the physical
register, CFA is a virtual register that serves as the base
address for various other saved registers. It is true that
in many frames these are set to alias, however this is not
a requirement. For example, a function that performs a stack
switch would likely change the rsp in the middle of the function,
but would keep the CFA at the original RSP such that saved registers
may be appropriately recovered.

We are seeing incorrect unwinds in the Julia runtime when running
julia under rr. This is because injects code (with correct CFI)
that performs just such a stack switch [1]. GDB manages to unwind
this correctly, but libunwind incorrectly sets the rsp to the CFA
address, causing a misunwind.

Tested on x86_64, patches for other architectures are ported, but
not tested.

[1] https://github.com/rr-debugger/rr/blob/469c22059a4a1798d33a8a224457faf22b2c178c/src/preload/syscall_hook.S#L454
---
 include/dwarf.h                 |  3 +-
 include/libunwind_i.h           |  4 ++
 include/tdep-x86/dwarf-config.h |  2 -
 include/tdep-x86/libunwind_i.h  | 73 ++++++++++++---------------------
 src/dwarf/Gparser.c             | 15 +++++--
 src/x86/Gos-freebsd.c           |  1 +
 src/x86/Gregs.c                 |  2 +-
 src/x86/Gstep.c                 |  4 +-
 src/x86_64/Gos-freebsd.c        |  1 +
 src/x86_64/Gregs.c              |  2 +-
 src/x86_64/Gstep.c              |  2 +-
 11 files changed, 52 insertions(+), 57 deletions(-)

diff --git a/include/dwarf.h b/include/dwarf.h
index 175c419bb..23ff4c4f6 100644
--- a/include/dwarf.h
+++ b/include/dwarf.h
@@ -231,6 +231,7 @@ typedef enum
     DWARF_WHERE_REG,            /* register saved in another register */
     DWARF_WHERE_EXPR,           /* register saved */
     DWARF_WHERE_VAL_EXPR,       /* register has computed value */
+    DWARF_WHERE_CFA,            /* register is set to the computed cfa value */
   }
 dwarf_where_t;
 
@@ -313,7 +314,7 @@ typedef struct dwarf_cursor
     void *as_arg;               /* argument to address-space callbacks */
     unw_addr_space_t as;        /* reference to per-address-space info */
 
-    unw_word_t cfa;     /* canonical frame address; aka frame-/stack-pointer */
+    unw_word_t cfa;     /* canonical frame address; aka frame-pointer */
     unw_word_t ip;              /* instruction pointer */
     unw_word_t args_size;       /* size of arguments */
     unw_word_t eh_args[UNW_TDEP_NUM_EH_REGS];
diff --git a/include/libunwind_i.h b/include/libunwind_i.h
index fea5c2607..6c7dda9a8 100644
--- a/include/libunwind_i.h
+++ b/include/libunwind_i.h
@@ -346,6 +346,10 @@ static inline void invalidate_edi (struct elf_dyn_info *edi)
 
 #include "tdep/libunwind_i.h"
 
+#ifndef TDEP_DWARF_SP
+#define TDEP_DWARF_SP UNW_TDEP_SP
+#endif
+
 #ifndef tdep_get_func_addr
 # define tdep_get_func_addr(as,addr,v)          (*(v) = addr, 0)
 #endif
diff --git a/include/tdep-x86/dwarf-config.h b/include/tdep-x86/dwarf-config.h
index f76f9c1c4..11398e4e6 100644
--- a/include/tdep-x86/dwarf-config.h
+++ b/include/tdep-x86/dwarf-config.h
@@ -43,9 +43,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.  */
 typedef struct dwarf_loc
   {
     unw_word_t val;
-#ifndef UNW_LOCAL_ONLY
     unw_word_t type;            /* see X86_LOC_TYPE_* macros.  */
-#endif
   }
 dwarf_loc_t;
 
diff --git a/include/tdep-x86/libunwind_i.h b/include/tdep-x86/libunwind_i.h
index d4c5ccdb1..ad4edc2f5 100644
--- a/include/tdep-x86/libunwind_i.h
+++ b/include/tdep-x86/libunwind_i.h
@@ -84,15 +84,26 @@ dwarf_get_uc(const struct dwarf_cursor *cursor)
 }
 
 #define DWARF_GET_LOC(l)        ((l).val)
+# define DWARF_LOC_TYPE_MEM     (0 << 0)
+# define DWARF_LOC_TYPE_FP      (1 << 0)
+# define DWARF_LOC_TYPE_REG     (1 << 1)
+# define DWARF_LOC_TYPE_VAL     (1 << 2)
 
-#ifdef UNW_LOCAL_ONLY
+# define DWARF_IS_REG_LOC(l)    (((l).type & DWARF_LOC_TYPE_REG) != 0)
+# define DWARF_IS_FP_LOC(l)     (((l).type & DWARF_LOC_TYPE_FP) != 0)
+# define DWARF_IS_MEM_LOC(l)    ((l).type == DWARF_LOC_TYPE_MEM)
+# define DWARF_IS_VAL_LOC(l)    (((l).type & DWARF_LOC_TYPE_VAL) != 0)
+
+# define DWARF_LOC(r, t)        ((dwarf_loc_t) { .val = (r), .type = (t) })
 # define DWARF_NULL_LOC         DWARF_LOC (0, 0)
-# define DWARF_IS_NULL_LOC(l)   (DWARF_GET_LOC (l) == 0)
-# define DWARF_LOC(r, t)        ((dwarf_loc_t) { .val = (r) })
-# define DWARF_IS_REG_LOC(l)    0
+# define DWARF_IS_NULL_LOC(l)                                           \
+                ({ dwarf_loc_t _l = (l); _l.val == 0 && _l.type == 0; })
+# define DWARF_VAL_LOC(c,v)     DWARF_LOC ((v), DWARF_LOC_TYPE_VAL)
+# define DWARF_MEM_LOC(c,m)     DWARF_LOC ((m), DWARF_LOC_TYPE_MEM)
+
+#ifdef UNW_LOCAL_ONLY
 # define DWARF_REG_LOC(c,r)     (DWARF_LOC((unw_word_t)                      \
                                  tdep_uc_addr(dwarf_get_uc(c), (r)), 0))
-# define DWARF_MEM_LOC(c,m)     DWARF_LOC ((m), 0)
 # define DWARF_FPREG_LOC(c,r)   (DWARF_LOC((unw_word_t)                      \
                                  tdep_uc_addr(dwarf_get_uc(c), (r)), 0))
 
@@ -114,35 +125,8 @@ dwarf_putfp (struct dwarf_cursor *c, dwarf_loc_t loc, unw_fpreg_t val)
   return 0;
 }
 
-static inline int
-dwarf_get (struct dwarf_cursor *c, dwarf_loc_t loc, unw_word_t *val)
-{
-  if (!DWARF_GET_LOC (loc))
-    return -1;
-  return (*c->as->acc.access_mem) (c->as, DWARF_GET_LOC (loc), val,
-                                   0, c->as_arg);
-}
-
-static inline int
-dwarf_put (struct dwarf_cursor *c, dwarf_loc_t loc, unw_word_t val)
-{
-  if (!DWARF_GET_LOC (loc))
-    return -1;
-  return (*c->as->acc.access_mem) (c->as, DWARF_GET_LOC (loc), &val,
-                                   1, c->as_arg);
-}
-
 #else /* !UNW_LOCAL_ONLY */
-# define DWARF_LOC_TYPE_FP      (1 << 0)
-# define DWARF_LOC_TYPE_REG     (1 << 1)
-# define DWARF_NULL_LOC         DWARF_LOC (0, 0)
-# define DWARF_IS_NULL_LOC(l)                                           \
-                ({ dwarf_loc_t _l = (l); _l.val == 0 && _l.type == 0; })
-# define DWARF_LOC(r, t)        ((dwarf_loc_t) { .val = (r), .type = (t) })
-# define DWARF_IS_REG_LOC(l)    (((l).type & DWARF_LOC_TYPE_REG) != 0)
-# define DWARF_IS_FP_LOC(l)     (((l).type & DWARF_LOC_TYPE_FP) != 0)
 # define DWARF_REG_LOC(c,r)     DWARF_LOC((r), DWARF_LOC_TYPE_REG)
-# define DWARF_MEM_LOC(c,m)     DWARF_LOC ((m), 0)
 # define DWARF_FPREG_LOC(c,r)   DWARF_LOC((r), (DWARF_LOC_TYPE_REG      \
                                                 | DWARF_LOC_TYPE_FP))
 
@@ -192,38 +176,33 @@ dwarf_putfp (struct dwarf_cursor *c, dwarf_loc_t loc, unw_fpreg_t val)
                                    1, c->as_arg);
 }
 
+#endif /* !UNW_LOCAL_ONLY */
+
 static inline int
 dwarf_get (struct dwarf_cursor *c, dwarf_loc_t loc, unw_word_t *val)
 {
   if (DWARF_IS_NULL_LOC (loc))
     return -UNW_EBADREG;
 
-  /* If a code-generator were to save a value of type unw_word_t in a
-     floating-point register, we would have to support this case.  I
-     suppose it could happen with MMX registers, but does it really
-     happen?  */
-  assert (!DWARF_IS_FP_LOC (loc));
-
   if (DWARF_IS_REG_LOC (loc))
     return (*c->as->acc.access_reg) (c->as, DWARF_GET_LOC (loc), val,
                                      0, c->as_arg);
-  else
+  if (DWARF_IS_MEM_LOC (loc))
     return (*c->as->acc.access_mem) (c->as, DWARF_GET_LOC (loc), val,
                                      0, c->as_arg);
+  assert(DWARF_IS_VAL_LOC (loc));
+  *val = DWARF_GET_LOC (loc);
+  return 0;
 }
 
 static inline int
 dwarf_put (struct dwarf_cursor *c, dwarf_loc_t loc, unw_word_t val)
 {
+  assert(!DWARF_IS_VAL_LOC (loc));
+
   if (DWARF_IS_NULL_LOC (loc))
     return -UNW_EBADREG;
 
-  /* If a code-generator were to save a value of type unw_word_t in a
-     floating-point register, we would have to support this case.  I
-     suppose it could happen with MMX registers, but does it really
-     happen?  */
-  assert (!DWARF_IS_FP_LOC (loc));
-
   if (DWARF_IS_REG_LOC (loc))
     return (*c->as->acc.access_reg) (c->as, DWARF_GET_LOC (loc), &val,
                                      1, c->as_arg);
@@ -232,7 +211,9 @@ dwarf_put (struct dwarf_cursor *c, dwarf_loc_t loc, unw_word_t val)
                                      1, c->as_arg);
 }
 
-#endif /* !UNW_LOCAL_ONLY */
+// For historical reasons, the DWARF numbering does not match the libunwind
+// numbering, necessitating this override
+#define TDEP_DWARF_SP 4
 
 #define tdep_getcontext_trace           unw_getcontext
 #define tdep_init_done                  UNW_OBJ(init_done)
diff --git a/src/dwarf/Gparser.c b/src/dwarf/Gparser.c
index da170d4b3..70a62c505 100644
--- a/src/dwarf/Gparser.c
+++ b/src/dwarf/Gparser.c
@@ -508,6 +508,9 @@ setup_fde (struct dwarf_cursor *c, dwarf_state_record_t *sr)
   for (i = 0; i < DWARF_NUM_PRESERVED_REGS + 2; ++i)
     set_reg (sr, i, DWARF_WHERE_SAME, 0);
 
+  // SP defaults to CFA (but is overridable)
+  set_reg (sr, TDEP_DWARF_SP, DWARF_WHERE_CFA, 0);
+
   struct dwarf_cie_info *dci = c->pi.unwind_info;
   sr->rs_current.ret_addr_column  = dci->ret_addr_column;
   unw_word_t addr = dci->cie_instr_start;
@@ -792,14 +795,14 @@ apply_reg_state (struct dwarf_cursor *c, struct dwarf_reg_state *rs)
       /* As a special-case, if the stack-pointer is the CFA and the
          stack-pointer wasn't saved, popping the CFA implicitly pops
          the stack-pointer as well.  */
-      if ((rs->reg.val[DWARF_CFA_REG_COLUMN] == UNW_TDEP_SP)
-          && (UNW_TDEP_SP < ARRAY_SIZE(rs->reg.val))
-          && (rs->reg.where[UNW_TDEP_SP] == DWARF_WHERE_SAME))
+      if ((rs->reg.val[DWARF_CFA_REG_COLUMN] == TDEP_DWARF_SP)
+          && (TDEP_DWARF_SP < ARRAY_SIZE(rs->reg.val))
+          && (DWARF_IS_NULL_LOC(c->loc[TDEP_DWARF_SP])))
           cfa = c->cfa;
       else
         {
           regnum = dwarf_to_unw_regnum (rs->reg.val[DWARF_CFA_REG_COLUMN]);
-          if ((ret = unw_get_reg ((unw_cursor_t *) c, regnum, &cfa)) < 0)
+          if ((ret = unw_get_reg (dwarf_to_cursor(c), regnum, &cfa)) < 0)
             return ret;
         }
       cfa += rs->reg.val[DWARF_CFA_OFF_COLUMN];
@@ -836,6 +839,10 @@ apply_reg_state (struct dwarf_cursor *c, struct dwarf_reg_state *rs)
         case DWARF_WHERE_SAME:
           break;
 
+        case DWARF_WHERE_CFA:
+          new_loc[i] = DWARF_VAL_LOC (c, cfa);
+          break;
+
         case DWARF_WHERE_CFAREL:
           new_loc[i] = DWARF_MEM_LOC (c, cfa + rs->reg.val[i]);
           break;
diff --git a/src/x86/Gos-freebsd.c b/src/x86/Gos-freebsd.c
index 7dd014046..1b251d027 100644
--- a/src/x86/Gos-freebsd.c
+++ b/src/x86/Gos-freebsd.c
@@ -138,6 +138,7 @@ x86_handle_signal_frame (unw_cursor_t *cursor)
     c->dwarf.loc[ST0] = DWARF_NULL_LOC;
   } else if (c->sigcontext_format == X86_SCF_FREEBSD_SYSCALL) {
     c->dwarf.loc[EIP] = DWARF_LOC (c->dwarf.cfa, 0);
+    c->dwarf.loc[ESP] = DWARF_VAL_LOC (c, c->dwarf.cfa + 4);
     c->dwarf.loc[EAX] = DWARF_NULL_LOC;
     c->dwarf.cfa += 4;
     c->dwarf.use_prev_instr = 1;
diff --git a/src/x86/Gregs.c b/src/x86/Gregs.c
index 4a9592617..9446d6c62 100644
--- a/src/x86/Gregs.c
+++ b/src/x86/Gregs.c
@@ -53,7 +53,6 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
       break;
 
     case UNW_X86_CFA:
-    case UNW_X86_ESP:
       if (write)
         return -UNW_EREADONLYREG;
       *valp = c->dwarf.cfa;
@@ -81,6 +80,7 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
     case UNW_X86_ECX: loc = c->dwarf.loc[ECX]; break;
     case UNW_X86_EBX: loc = c->dwarf.loc[EBX]; break;
 
+    case UNW_X86_ESP: loc = c->dwarf.loc[ESP]; break;
     case UNW_X86_EBP: loc = c->dwarf.loc[EBP]; break;
     case UNW_X86_ESI: loc = c->dwarf.loc[ESI]; break;
     case UNW_X86_EDI: loc = c->dwarf.loc[EDI]; break;
diff --git a/src/x86/Gstep.c b/src/x86/Gstep.c
index 129b739a3..061dcbaaa 100644
--- a/src/x86/Gstep.c
+++ b/src/x86/Gstep.c
@@ -47,7 +47,7 @@ unw_step (unw_cursor_t *cursor)
     {
       /* DWARF failed, let's see if we can follow the frame-chain
          or skip over the signal trampoline.  */
-      struct dwarf_loc ebp_loc, eip_loc;
+      struct dwarf_loc ebp_loc, eip_loc, esp_loc;
 
       /* We could get here because of missing/bad unwind information.
          Validate all addresses before dereferencing. */
@@ -77,6 +77,7 @@ unw_step (unw_cursor_t *cursor)
                  c->dwarf.cfa);
 
           ebp_loc = DWARF_LOC (c->dwarf.cfa, 0);
+          esp_loc = DWARF_VAL_LOC (c, c->dwarf.cfa + 8);
           eip_loc = DWARF_LOC (c->dwarf.cfa + 4, 0);
           c->dwarf.cfa += 8;
 
@@ -87,6 +88,7 @@ unw_step (unw_cursor_t *cursor)
             c->dwarf.loc[i] = DWARF_NULL_LOC;
 
           c->dwarf.loc[EBP] = ebp_loc;
+          c->dwarf.loc[ESP] = esp_loc;
           c->dwarf.loc[EIP] = eip_loc;
           c->dwarf.use_prev_instr = 1;
         }
diff --git a/src/x86_64/Gos-freebsd.c b/src/x86_64/Gos-freebsd.c
index 8f28d1d8c..0c5a17940 100644
--- a/src/x86_64/Gos-freebsd.c
+++ b/src/x86_64/Gos-freebsd.c
@@ -133,6 +133,7 @@ x86_64_handle_signal_frame (unw_cursor_t *cursor)
     c->dwarf.loc[RCX] = c->dwarf.loc[R10];
     /*  rsp_loc = DWARF_LOC(c->dwarf.cfa - 8, 0);       */
     /*  rbp_loc = c->dwarf.loc[RBP];                    */
+    c->dwarf.loc[RSP] = DWARF_VAL_LOC (c, c->dwarf.cfa + 8);
     c->dwarf.loc[RIP] = DWARF_LOC (c->dwarf.cfa, 0);
     ret = dwarf_get (&c->dwarf, c->dwarf.loc[RIP], &c->dwarf.ip);
     Debug (1, "Frame Chain [RIP=0x%Lx] = 0x%Lx\n",
diff --git a/src/x86_64/Gregs.c b/src/x86_64/Gregs.c
index baf8a24f0..dff5bcbe7 100644
--- a/src/x86_64/Gregs.c
+++ b/src/x86_64/Gregs.c
@@ -79,7 +79,6 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
       break;
 
     case UNW_X86_64_CFA:
-    case UNW_X86_64_RSP:
       if (write)
         return -UNW_EREADONLYREG;
       *valp = c->dwarf.cfa;
@@ -107,6 +106,7 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
     case UNW_X86_64_RCX: loc = c->dwarf.loc[RCX]; break;
     case UNW_X86_64_RBX: loc = c->dwarf.loc[RBX]; break;
 
+    case UNW_X86_64_RSP: loc = c->dwarf.loc[RSP]; break;
     case UNW_X86_64_RBP: loc = c->dwarf.loc[RBP]; break;
     case UNW_X86_64_RSI: loc = c->dwarf.loc[RSI]; break;
     case UNW_X86_64_RDI: loc = c->dwarf.loc[RDI]; break;
diff --git a/src/x86_64/Gstep.c b/src/x86_64/Gstep.c
index 3c5c3830f..fdad298c7 100644
--- a/src/x86_64/Gstep.c
+++ b/src/x86_64/Gstep.c
@@ -223,7 +223,7 @@ unw_step (unw_cursor_t *cursor)
                   Debug (2, "RIP fixup didn't work, falling back\n");
                   unw_word_t rbp1 = 0;
                   rbp_loc = DWARF_LOC(rbp, 0);
-                  rsp_loc = DWARF_NULL_LOC;
+                  rsp_loc = DWARF_VAL_LOC(c, rbp + 16);
                   rip_loc = DWARF_LOC (rbp + 8, 0);
                   ret = dwarf_get (&c->dwarf, rbp_loc, &rbp1);
                   Debug (1, "[RBP=0x%lx] = 0x%lx (cfa = 0x%lx) -> 0x%lx\n",