Skip to content

Commit

Permalink
Carry Libuwnind CFA RSP patch (#2503)
Browse files Browse the repository at this point in the history 
See JuliaLang/julia#39553.
  • Loading branch information
@Keno
Keno committed Feb 8, 2021
1 parent ffbd3c7 commit 801e854
Show file tree
Hide file tree
Showing 2 changed files with 309 additions and 0 deletions.
1 change: 1 addition & 0 deletions L/LibUnwind/build_tarballs.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ atomic_patch -p1 ${WORKSPACE}/srcdir/patches/libunwind-prefer-extbl.patch
atomic_patch -p1 ${WORKSPACE}/srcdir/patches/libunwind-static-arm.patch
atomic_patch -p0 ${WORKSPACE}/srcdir/patches/libunwind-configure-ppc64le.patch
atomic_patch -p0 ${WORKSPACE}/srcdir/patches/libunwind-configure-static-lzma.patch
atomic_patch -p1 ${WORKSPACE}/srcdir/patches/libunwind-cfa-rsp.patch
CFLAGS="${CFLAGS} -DPI -fPIC -I${prefix}/include"
./configure --prefix=$prefix --host=$target CFLAGS="${CFLAGS}" --libdir=${libdir} --enable-minidebuginfo --disable-tests
Expand Down
308 changes: 308 additions & 0 deletions L/LibUnwind/bundled/patches/libunwind-cfa-rsp.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,308 @@
commit e17f29435c902f80ed119ae33506fca1b57584f7
Author: Keno Fischer <keno@juliacomputing.com>
Date: Sat Feb 6 18:13:16 2021 -0500

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

diff --git a/include/dwarf.h b/include/dwarf.h
index fab93c61..b845e2eb 100644
--- a/include/dwarf.h
+++ b/include/dwarf.h
@@ -227,6 +227,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;

@@ -309,7 +310,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-aarch64.h b/include/libunwind-aarch64.h
index 85812e15..e1f69a5c 100644
--- a/include/libunwind-aarch64.h
+++ b/include/libunwind-aarch64.h
@@ -146,12 +146,10 @@ typedef enum
UNW_AARCH64_FPSR,
UNW_AARCH64_FPCR,

- /* For AArch64, the CFA is the value of SP (x31) at the call site of the
- previous frame. */
- UNW_AARCH64_CFA = UNW_AARCH64_SP,
-
UNW_TDEP_LAST_REG = UNW_AARCH64_FPCR,

+ UNW_AARCH64_CFA,
+
UNW_TDEP_IP = UNW_AARCH64_X30,
UNW_TDEP_SP = UNW_AARCH64_SP,
UNW_TDEP_EH = UNW_AARCH64_X0,
diff --git a/include/libunwind-arm.h b/include/libunwind-arm.h
index 6709b7ab..58d93926 100644
--- a/include/libunwind-arm.h
+++ b/include/libunwind-arm.h
@@ -42,7 +42,7 @@ extern "C" {
require recompiling all users of this library. Stack allocation is
relatively cheap and unwind-state copying is relatively rare, so we
want to err on making it rather too big than too small. */
-
+
/* FIXME for ARM. Too big? What do other things use for similar tasks? */
#define UNW_TDEP_CURSOR_LEN 4096

@@ -69,7 +69,7 @@ typedef enum
UNW_ARM_R13,
UNW_ARM_R14,
UNW_ARM_R15,
-
+
/* VFPv2 s0-s31 (obsolescent numberings). */
UNW_ARM_S0 = 64,
UNW_ARM_S1,
@@ -103,7 +103,7 @@ typedef enum
UNW_ARM_S29,
UNW_ARM_S30,
UNW_ARM_S31,
-
+
/* FPA register numberings. */
UNW_ARM_F0 = 96,
UNW_ARM_F1,
@@ -113,7 +113,7 @@ typedef enum
UNW_ARM_F5,
UNW_ARM_F6,
UNW_ARM_F7,
-
+
/* iWMMXt GR register numberings. */
UNW_ARM_wCGR0 = 104,
UNW_ARM_wCGR1,
@@ -123,7 +123,7 @@ typedef enum
UNW_ARM_wCGR5,
UNW_ARM_wCGR6,
UNW_ARM_wCGR7,
-
+
/* iWMMXt register numberings. */
UNW_ARM_wR0 = 112,
UNW_ARM_wR1,
@@ -141,9 +141,9 @@ typedef enum
UNW_ARM_wR13,
UNW_ARM_wR14,
UNW_ARM_wR15,
-
+
/* Two-byte encodings from here on. */
-
+
/* SPSR. */
UNW_ARM_SPSR = 128,
UNW_ARM_SPSR_FIQ,
@@ -151,7 +151,7 @@ typedef enum
UNW_ARM_SPSR_ABT,
UNW_ARM_SPSR_UND,
UNW_ARM_SPSR_SVC,
-
+
/* User mode registers. */
UNW_ARM_R8_USR = 144,
UNW_ARM_R9_USR,
@@ -160,7 +160,7 @@ typedef enum
UNW_ARM_R12_USR,
UNW_ARM_R13_USR,
UNW_ARM_R14_USR,
-
+
/* FIQ registers. */
UNW_ARM_R8_FIQ = 151,
UNW_ARM_R9_FIQ,
@@ -169,23 +169,23 @@ typedef enum
UNW_ARM_R12_FIQ,
UNW_ARM_R13_FIQ,
UNW_ARM_R14_FIQ,
-
+
/* IRQ registers. */
UNW_ARM_R13_IRQ = 158,
UNW_ARM_R14_IRQ,
-
+
/* ABT registers. */
UNW_ARM_R13_ABT = 160,
UNW_ARM_R14_ABT,
-
+
/* UND registers. */
UNW_ARM_R13_UND = 162,
UNW_ARM_R14_UND,
-
+
/* SVC registers. */
UNW_ARM_R13_SVC = 164,
UNW_ARM_R14_SVC,
-
+
/* iWMMXt control registers. */
UNW_ARM_wC0 = 192,
UNW_ARM_wC1,
diff --git a/src/aarch64/Gregs.c b/src/aarch64/Gregs.c
index a8843734..4b8a684d 100644
--- a/src/aarch64/Gregs.c
+++ b/src/aarch64/Gregs.c
@@ -84,12 +84,13 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
case UNW_AARCH64_X27:
case UNW_AARCH64_X28:
case UNW_AARCH64_X29:
+ case UNW_AARCH64_SP:
case UNW_AARCH64_PC:
case UNW_AARCH64_PSTATE:
loc = c->dwarf.loc[reg];
break;

- case UNW_AARCH64_SP:
+ case UNW_AARCH64_CFA:
if (write)
return -UNW_EREADONLYREG;
*valp = c->dwarf.cfa;
diff --git a/src/arm/Gregs.c b/src/arm/Gregs.c
index 0d52f0b2..021a79ba 100644
--- a/src/arm/Gregs.c
+++ b/src/arm/Gregs.c
@@ -29,7 +29,7 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
int write)
{
dwarf_loc_t loc = DWARF_NULL_LOC;
-
+
switch (reg)
{
case UNW_ARM_R15:
@@ -48,11 +48,11 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
case UNW_ARM_R10:
case UNW_ARM_R11:
case UNW_ARM_R12:
+ case UNW_ARM_R13:
case UNW_ARM_R14:
loc = c->dwarf.loc[reg - UNW_ARM_R0];
break;

- case UNW_ARM_R13:
case UNW_ARM_CFA:
if (write)
return -UNW_EREADONLYREG;
diff --git a/src/dwarf/Gparser.c b/src/dwarf/Gparser.c
index 7d255aee..986c4a89 100644
--- a/src/dwarf/Gparser.c
+++ b/src/dwarf/Gparser.c
@@ -500,6 +500,8 @@ setup_fde (struct dwarf_cursor *c, dwarf_state_record_t *sr)
memset (sr, 0, sizeof (*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, UNW_TDEP_SP, DWARF_WHERE_CFA, 0);

struct dwarf_cie_info *dci = c->pi.unwind_info;
sr->rs_current.ret_addr_column = dci->ret_addr_column;
@@ -826,6 +828,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/hppa/Gregs.c b/src/hppa/Gregs.c
index da0542c8..7b18968a 100644
--- a/src/hppa/Gregs.c
+++ b/src/hppa/Gregs.c
@@ -42,7 +42,6 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
break;

case UNW_HPPA_CFA:
- case UNW_HPPA_SP:
if (write)
return -UNW_EREADONLYREG;
*valp = c->dwarf.cfa;
diff --git a/src/mips/Gregs.c b/src/mips/Gregs.c
index 95194022..be581d31 100644
--- a/src/mips/Gregs.c
+++ b/src/mips/Gregs.c
@@ -31,7 +31,7 @@ tdep_access_reg (struct cursor *c, unw_regnum_t reg, unw_word_t *valp,
int write)
{
dwarf_loc_t loc = DWARF_NULL_LOC;
-
+
switch (reg)
{
case UNW_MIPS_R0:
diff --git a/src/x86/Gregs.c b/src/x86/Gregs.c
index 4a959261..9446d6c6 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_64/Gregs.c b/src/x86_64/Gregs.c
index baf8a24f..dff5bcbe 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;

0 comments on commit 801e854

Please sign in to comment.