Skip to content

Commit

Permalink
dwarf: Push correct CFA onto stack for dwarf expression evaluation. (#93
Browse files Browse the repository at this point in the history
)

dwarf: Push correct CFA onto stack for dwarf expression evaluation.

This change fixes a bug where stale CFAs were pushed onto the dwarf
expression stack before expression evaluation. Some optimising compilers
emit CFI which relies on this being correct.
  • Loading branch information
stephen-roberts-work authored and Dave Watson committed Nov 13, 2018
1 parent f551e16 commit 748a2df
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 12 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,6 @@ tests/[GL]ia64-test-readonly
tests/[GL]ia64-test-stack
tests/ia64-test-dyn1
tests/ia64-test-sig
tests/[GL]x64-test-dwarf-expressions
tests/*.log
tests/*.trs
2 changes: 1 addition & 1 deletion include/dwarf.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ extern int dwarf_find_unwind_table (struct elf_dyn_info *edi, unw_addr_space_t a
unw_word_t ip);
extern void dwarf_put_unwind_info (unw_addr_space_t as,
unw_proc_info_t *pi, void *arg);
extern int dwarf_eval_expr (struct dwarf_cursor *c, unw_word_t *addr,
extern int dwarf_eval_expr (struct dwarf_cursor *c, unw_word_t stack_val, unw_word_t *addr,
unw_word_t len, unw_word_t *valp,
int *is_register);
extern int
Expand Down
14 changes: 9 additions & 5 deletions src/dwarf/Gexpr.c
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ dwarf_stack_aligned(struct dwarf_cursor *c, unw_word_t cfa_addr,
}

HIDDEN int
dwarf_eval_expr (struct dwarf_cursor *c, unw_word_t *addr, unw_word_t len,
unw_word_t *valp, int *is_register)
dwarf_eval_expr (struct dwarf_cursor *c, unw_word_t stack_val, unw_word_t *addr,
unw_word_t len, unw_word_t *valp, int *is_register)
{
unw_word_t operand1 = 0, operand2 = 0, tmp1, tmp2 = 0, tmp3, end_addr;
uint8_t opcode, operands_signature, u8;
Expand Down Expand Up @@ -287,10 +287,14 @@ do { \
end_addr = *addr + len;
*is_register = 0;

Debug (14, "len=%lu, pushing cfa=0x%lx\n",
(unsigned long) len, (unsigned long) c->cfa);
Debug (14, "len=%lu, pushing initial value=0x%lx\n",
(unsigned long) len, (unsigned long) stack_val);

push (c->cfa); /* push current CFA as required by DWARF spec */
/* The DWARF standard requires the current CFA to be pushed onto the stack */
/* before evaluating DW_CFA_expression and DW_CFA_val_expression programs. */
/* DW_CFA_def_cfa_expressions do not take an initial value, but we push on */
/* a dummy value to keep this logic consistent. */
push (stack_val);

while (*addr < end_addr)
{
Expand Down
17 changes: 12 additions & 5 deletions src/dwarf/Gparser.c
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ create_state_record_for (struct dwarf_cursor *c, dwarf_state_record_t *sr,
}

static inline int
eval_location_expr (struct dwarf_cursor *c, unw_addr_space_t as,
eval_location_expr (struct dwarf_cursor *c, unw_word_t stack_val, unw_addr_space_t as,
unw_accessors_t *a, unw_word_t addr,
dwarf_loc_t *locp, void *arg)
{
Expand All @@ -746,7 +746,7 @@ eval_location_expr (struct dwarf_cursor *c, unw_addr_space_t as,
return ret;

/* evaluate the expression: */
if ((ret = dwarf_eval_expr (c, &addr, len, &val, &is_register)) < 0)
if ((ret = dwarf_eval_expr (c, stack_val, &addr, len, &val, &is_register)) < 0)
return ret;

if (is_register)
Expand Down Expand Up @@ -804,7 +804,10 @@ apply_reg_state (struct dwarf_cursor *c, struct dwarf_reg_state *rs)
assert (rs->reg.where[DWARF_CFA_REG_COLUMN] == DWARF_WHERE_EXPR);

addr = rs->reg.val[DWARF_CFA_REG_COLUMN];
if ((ret = eval_location_expr (c, as, a, addr, &cfa_loc, arg)) < 0)
/* The dwarf standard doesn't specify an initial value to be pushed on */
/* the stack before DW_CFA_def_cfa_expression evaluation. We push on a */
/* dummy value (0) to keep the eval_location_expr function consistent. */
if ((ret = eval_location_expr (c, 0, as, a, addr, &cfa_loc, arg)) < 0)
return ret;
/* the returned location better be a memory location... */
if (DWARF_IS_REG_LOC (cfa_loc))
Expand Down Expand Up @@ -844,13 +847,17 @@ apply_reg_state (struct dwarf_cursor *c, struct dwarf_reg_state *rs)

case DWARF_WHERE_EXPR:
addr = rs->reg.val[i];
if ((ret = eval_location_expr (c, as, a, addr, new_loc + i, arg)) < 0)
/* The dwarf standard requires the current CFA to be pushed on the */
/* stack before DW_CFA_expression evaluation. */
if ((ret = eval_location_expr (c, cfa, as, a, addr, new_loc + i, arg)) < 0)
return ret;
break;

case DWARF_WHERE_VAL_EXPR:
addr = rs->reg.val[i];
if ((ret = eval_location_expr (c, as, a, addr, new_loc + i, arg)) < 0)
/* The dwarf standard requires the current CFA to be pushed on the */
/* stack before DW_CFA_val_expression evaluation. */
if ((ret = eval_location_expr (c, cfa, as, a, addr, new_loc + i, arg)) < 0)
return ret;
new_loc[i] = DWARF_VAL_LOC (c, DWARF_GET_LOC (new_loc[i]));
break;
Expand Down
68 changes: 68 additions & 0 deletions tests/Gx64-test-dwarf-expressions.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>

#include <libunwind.h>

static int verbose;
static int nerrors;

#define panic(args...) \
do { printf (args); ++nerrors; } while (0)

// Assembly routine which sets up the stack for the test then calls another one
// which clobbers the stack, and which in turn calls recover_register below
extern int64_t DW_CFA_expression_testcase(int64_t regnum, int64_t height);

// recover_register is called by the assembly routines. It returns the value of
// a register at a specified height from the inner-most frame. The return value
// is propagated back through the assembly routines to the testcase.
extern int64_t recover_register(int64_t regnum, int64_t height)
{
// Initialize cursor to current frame
int rc, i;
unw_cursor_t cursor;
unw_context_t context;
unw_getcontext(&context);
unw_init_local(&cursor, &context);
// Unwind frames until required height from inner-most frame (i.e. this one)
for (i = 0; i < height; ++i)
{
rc = unw_step(&cursor);
if (rc < 0)
panic("%s: unw_step failed on step %d with return code %d", __FUNCTION__, i, rc);
else if (rc == 0)
panic("%s: unw_step failed to reach the end of the stack", __FUNCTION__);
unw_word_t pc;
rc = unw_get_reg(&cursor, UNW_REG_IP, &pc);
if (rc < 0 || pc == 0)
panic("%s: unw_get_reg failed to locate the program counter", __FUNCTION__);
}
// We're now at the required height, extract register
uint64_t value;
if ((rc = unw_get_reg(&cursor, (unw_regnum_t) regnum, &value)) != 0)
panic("%s: unw_get_reg failed to retrieve register %lu", __FUNCTION__, regnum);
return value;
}

int
main (int argc, char **argv)
{
if (argc > 1)
verbose = 1;

if (DW_CFA_expression_testcase(12, 1) != 0)
panic("r12 should be clobbered at height 1 (DW_CFA_expression_inner)");
if (DW_CFA_expression_testcase(12, 2) != 111222333)
panic("r12 should be restored at height 2 (DW_CFA_expression_testcase)");

if (nerrors > 0)
{
fprintf (stderr, "FAILURE: detected %d errors\n", nerrors);
exit (-1);
}

if (verbose)
printf ("SUCCESS.\n");
return 0;
}
5 changes: 5 additions & 0 deletions tests/Lx64-test-dwarf-expressions.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#define UNW_LOCAL_ONLY
#include <libunwind.h>
#if !defined(UNW_REMOTE_ONLY)
#include "Gx64-test-dwarf-expressions.c"
#endif
17 changes: 16 additions & 1 deletion tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ if ARCH_PPC64
if USE_ALTIVEC
noinst_PROGRAMS_arch += ppc64-test-altivec
endif #USE_ALTIVEC
endif #ARCH_PPC64
else #!ARCH_PPC64
if ARCH_X86_64
check_PROGRAMS_arch += Gx64-test-dwarf-expressions Lx64-test-dwarf-expressions
endif #ARCH X86_64
endif #!ARCH_PPC64
endif #!ARCH_IA64
check_PROGRAMS_cdep += Gtest-bt Ltest-bt Gtest-exc Ltest-exc \
Gtest-init Ltest-init \
Expand Down Expand Up @@ -139,6 +143,14 @@ Gia64_test_nat_SOURCES = Gia64-test-nat.c ia64-test-nat-asm.S
ia64_test_dyn1_SOURCES = ia64-test-dyn1.c ia64-dyn-asm.S flush-cache.S \
flush-cache.h
ppc64_test_altivec_SOURCES = ppc64-test-altivec.c ppc64-test-altivec-utils.c


Gx64_test_dwarf_expressions_SOURCES = Gx64-test-dwarf-expressions.c \
x64-test-dwarf-expressions.S
Lx64_test_dwarf_expressions_SOURCES = Lx64-test-dwarf-expressions.c \
x64-test-dwarf-expressions.S


Gtest_init_SOURCES = Gtest-init.cxx
Ltest_init_SOURCES = Ltest-init.cxx
Ltest_cxx_exceptions_SOURCES = Ltest-cxx-exceptions.cxx
Expand Down Expand Up @@ -232,3 +244,6 @@ Lia64_test_readonly_LDADD = $(LIBUNWIND_local)
ia64_test_dyn1_LDADD = $(LIBUNWIND)
ia64_test_sig_LDADD = $(LIBUNWIND)
ppc64_test_altivec_LDADD = $(LIBUNWIND)

Gx64_test_dwarf_expressions_LDADD = $(LIBUNWIND) $(LIBUNWIND_local)
Lx64_test_dwarf_expressions_LDADD = $(LIBUNWIND_local)
78 changes: 78 additions & 0 deletions tests/x64-test-dwarf-expressions.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
.global DW_CFA_expression_testcase

.extern recover_register

.text

# CFI expressions were added in DWARF v3 to allow compilers to specify memory
# locations or register values using DWARF programs. These programs are simple
# stack-based operations which allow the compiler to encode integer mathematics
# and other complex logic. CFI expressions are therefore more powerful than the
# conventional register + offset schemes.
#
# These tests capture a bug we have fixed in libunwind. CFI expression programs
# always start with the current CFA pushed onto the stack. This file contains a
# pair of routines which test CFI expression parsing. Specifically they test
# DW_CFA_expression logic, which uses DWARF expressions to compute the address
# where a non-volatile register was stored.
#
# Main calls DW_CFA_expression_testcase, which sets up known state in a
# non-volatile (caller-saved) register. We use r12 for this purpose. After this
# DW_CFA_expression_testcase then calls DW_CFA_expression_inner, which clobbers
# r12 after stashing its value on the stack. This routine contains a DWARF3 CFI
# expression to restore the value of r12 on unwind which should allow libunwind
# to recover clobbered state. DW_CFA_expression_inner calls recover_register to
# retrieve the cached register value. This function recovers the register value
# by using libunwind to unwind the stack through DW_CFA_expression_inner and up
# to the call site in DW_CFA_expression_testcase. If our expression is correct,
# libunwind will be able to restore r12 from the stack.
#
# BE CAREFUL WITH rdi, rsi, rax HERE! The arguments to recover_register are
# passed in via rdi, rsi and I just let them flow through unchanged. Similarly
# RAX flows back unchanged. Adding any function calls to the below may clobber
# these registers and cause this test to fail mysteriously.


########################################################
# Test: Restoring a register using a DW_CFA_expression #
# which uses implicit CFA pushed onto stack. #
########################################################

.type DW_CFA_expression_testcase STT_FUNC
DW_CFA_expression_testcase:
.cfi_startproc
push %r12
.cfi_adjust_cfa_offset 8
# Move our sentinel (known) value into non-volatile (Callee-saved) r12
mov $111222333, %r12
.cfi_rel_offset %r12, 0
call DW_CFA_expression_inner
pop %r12
.cfi_restore %r12
.cfi_adjust_cfa_offset -8
ret
.cfi_endproc
.size DW_CFA_expression_testcase,.-DW_CFA_expression_testcase

.type DW_CFA_expression_inner STT_FUNC
DW_CFA_expression_inner:
.cfi_startproc
push %r12
.cfi_adjust_cfa_offset 8
# !! IMPORTANT BIT !! The test is all about how we parse the following bytes.
# Now we use an expression to describe where our sentinel value is stored:
# DW_CFA_expression(0x10), r12(0x0c), Length(0x02), (preamble)
# DW_OP_lit16(0x40), DW_OP_minus(0x1c) (instructions)
# Parsing starts with the CFA on the stack, then pushes 16, then does a minus
# which is eqivalent to a=pop(), b=pop(), push(b-a), leaving us with a value
# of cfa-16 (cfa points at old rsp, cfa-8 is our rip, so we stored r12 at
# cfa-16).
xor %r12, %r12 # Trash r12
.cfi_escape 0x10, 0x0c, 0x2, 0x40, 0x1c # DW_CFA_expression for recovery
call recover_register
pop %r12
.cfi_restore %r12
.cfi_adjust_cfa_offset -8
ret
.cfi_endproc
.size DW_CFA_expression_inner,.-DW_CFA_expression_inner

0 comments on commit 748a2df

Please sign in to comment.