m2-internship/libunwind-eh_elf
1
0
Fork 0
mirror of https://github.com/tobast/libunwind-eh_elf.git synced 2024-11-16 21:27:38 +01:00
Code Issues Releases Activity

dwarf: Fix incorrect cfi execution (#54)

Browse source 
During unwinding/resuming execution of a normal call frame,
it is not only necessary to use the previous instruction to lookup the unwind info
but also when executing the cfi program. Although the call usually don't modify
any unwinding state, it can happen for noreturn call or when the callee cleanup the stack.
In these cases, the next instruction after the call may have a cfi adjusting the state
(e.g. stack pointer) and such instruction should be executed.

3d9a694de8 worked around this issue by treating `cfi_restore_state`
specially. It works when the compiler use that instruction to restore the state, i.e.

```
    .cfi_remember_state
    je .L0
    push ...
    .cfi_def_cfi_offset <new_value>
    call noreturn
.L0
    .cfi_restore_state
```

which is what GCC ususally does. However, it is not necessarily the case and clang/LLVM doesn't
do that. Instead LLVM emits the following unwind info which is also perfectly valid but is not
handled by the special case.

```
    je .L0
    push ...
    .cfi_def_cfi_offset <new_value>
    call noreturn
.L0
    .cfi_def_cfi_offset <old_value>
```

e9e8ed73e3 also worked around this issue for another special case.

This patch fix this issue for all cfi types by adjusting the `end_ip` based on the type of the
current frame instead, similar to what's done in `fetch_proc_info`.
Since this requires using the same `use_prev_instr` value after `fetch_proc_info` returns,
the patch also remove the `need_unwind_info` parameter to the function and move the code updating
`use_prev_instr` after all use of the old value are done.
This commit is contained in:
Yichao Yu 2017-11-01 12:14:37 -04:00 committed by Dave Watson
parent 1870b26a00
commit f248ac0c6e
1 changed files with 42 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
src/dwarf/Gparser.c
View file

@ -112,9 +112,6 @@ run_cfi_program (struct dwarf_cursor *c, dwarf_state_record_t *sr,
unw_accessors_t *a = unw_get_accessors (as); unw_accessors_t *a = unw_get_accessors (as);
int ret = 0; int ret = 0;
/* Process everything up to and including the current 'end_ip',
including all the DW_CFA_advance_loc instructions. See
'c->use_prev_instr' use in 'fetch_proc_info' for details. */
while (*ip <= end_ip && *addr < end_addr && ret >= 0) while (*ip <= end_ip && *addr < end_addr && ret >= 0)
{ {
unw_word_t operand = 0, regnum, val, len; unw_word_t operand = 0, regnum, val, len;
@ -289,10 +286,8 @@ run_cfi_program (struct dwarf_cursor *c, dwarf_state_record_t *sr,
ret = -UNW_EINVAL; ret = -UNW_EINVAL;
break; break;
} }
if (*ip < end_ip) {
memcpy (&sr->rs_current, &(*rs_stack)->state, sizeof (sr->rs_current)); memcpy (&sr->rs_current, &(*rs_stack)->state, sizeof (sr->rs_current));
pop_rstate_stack(rs_stack); pop_rstate_stack(rs_stack);
}
Debug (15, "CFA_restore_state\n"); Debug (15, "CFA_restore_state\n");
break; break;
@ -384,11 +379,8 @@ run_cfi_program (struct dwarf_cursor *c, dwarf_state_record_t *sr,
case DW_CFA_GNU_args_size: case DW_CFA_GNU_args_size:
if ((ret = dwarf_read_uleb128 (as, a, addr, &val, arg)) < 0) if ((ret = dwarf_read_uleb128 (as, a, addr, &val, arg)) < 0)
break; break;
if (*ip < end_ip)
{
sr->args_size = val; sr->args_size = val;
Debug (15, "CFA_GNU_args_size %lu\n", (long) val); Debug (15, "CFA_GNU_args_size %lu\n", (long) val);
}
break; break;
case DW_CFA_GNU_negative_offset_extended: case DW_CFA_GNU_negative_offset_extended:
@ -429,7 +421,7 @@ run_cfi_program (struct dwarf_cursor *c, dwarf_state_record_t *sr,
} }
static int static int
fetch_proc_info (struct dwarf_cursor *c, unw_word_t ip, int need_unwind_info) fetch_proc_info (struct dwarf_cursor *c, unw_word_t ip)
{ {
int ret, dynamic = 1; int ret, dynamic = 1;
@ -443,7 +435,7 @@ fetch_proc_info (struct dwarf_cursor *c, unw_word_t ip, int need_unwind_info)
and b) so that run_cfi_program() runs locations up to the call and b) so that run_cfi_program() runs locations up to the call
but not more. but not more.
For execution resume, we need to do the exact opposite and look For signal frame, we need to do the exact opposite and look
up using the current 'ip' value. That is where execution will up using the current 'ip' value. That is where execution will
continue, and it's important we get this right, as 'ip' could be continue, and it's important we get this right, as 'ip' could be
right at the function entry and hence FDE edge, or at instruction right at the function entry and hence FDE edge, or at instruction
@ -475,14 +467,6 @@ fetch_proc_info (struct dwarf_cursor *c, unw_word_t ip, int need_unwind_info)
if (ret >= 0) if (ret >= 0)
tdep_fetch_frame (c, ip, 1); tdep_fetch_frame (c, ip, 1);
/* Update use_prev_instr for the next frame. */
if (need_unwind_info)
{
assert(c->pi.unwind_info);
struct dwarf_cie_info *dci = c->pi.unwind_info;
c->use_prev_instr = ! dci->signal_frame;
}
return ret; return ret;
} }
@ -541,7 +525,9 @@ parse_fde (struct dwarf_cursor *c, unw_word_t ip, dwarf_state_record_t *sr)
unw_word_t addr = dci->fde_instr_start; unw_word_t addr = dci->fde_instr_start;
unw_word_t curr_ip = c->pi.start_ip; unw_word_t curr_ip = c->pi.start_ip;
dwarf_stackable_reg_state_t *rs_stack = NULL; dwarf_stackable_reg_state_t *rs_stack = NULL;
ret = run_cfi_program (c, sr, &curr_ip, ip, &addr, dci->fde_instr_end, /* Process up to current `ip` for signal frame and `ip - 1` for normal call frame
See `c->use_prev_instr` use in `fetch_proc_info` for details. */
ret = run_cfi_program (c, sr, &curr_ip, ip - c->use_prev_instr, &addr, dci->fde_instr_end,
&rs_stack, dci); &rs_stack, dci);
empty_rstate_stack(&rs_stack); empty_rstate_stack(&rs_stack);
if (ret < 0) if (ret < 0)
@ -914,10 +900,18 @@ find_reg_state (struct dwarf_cursor *c, dwarf_state_record_t *sr)
} }
else else
{ {
ret = fetch_proc_info (c, c->ip, 1); ret = fetch_proc_info (c, c->ip);
int next_use_prev_instr = c->use_prev_instr;
if (ret >= 0) if (ret >= 0)
{
/* Update use_prev_instr for the next frame. */
assert(c->pi.unwind_info);
struct dwarf_cie_info *dci = c->pi.unwind_info;
next_use_prev_instr = ! dci->signal_frame;
ret = create_state_record_for (c, sr, c->ip); ret = create_state_record_for (c, sr, c->ip);
}
put_unwind_info (c, &c->pi); put_unwind_info (c, &c->pi);
c->use_prev_instr = next_use_prev_instr;
if (cache && ret >= 0) if (cache && ret >= 0)
{ {
@ -972,7 +966,7 @@ dwarf_make_proc_info (struct dwarf_cursor *c)
int ret; int ret;
/* Lookup it up the slow way... */ /* Lookup it up the slow way... */
ret = fetch_proc_info (c, c->ip, 0); ret = fetch_proc_info (c, c->ip);
if (ret >= 0) if (ret >= 0)
ret = create_state_record_for (c, &sr, c->ip); ret = create_state_record_for (c, &sr, c->ip);
put_unwind_info (c, &c->pi); put_unwind_info (c, &c->pi);
@ -1029,8 +1023,14 @@ dwarf_reg_states_iterate(struct dwarf_cursor *c,
unw_reg_states_callback cb, unw_reg_states_callback cb,
void *token) void *token)
{ {
int ret = fetch_proc_info (c, c->ip, 1); int ret = fetch_proc_info (c, c->ip);
int next_use_prev_instr = c->use_prev_instr;
if (ret >= 0) if (ret >= 0)
{
/* Update use_prev_instr for the next frame. */
assert(c->pi.unwind_info);
struct dwarf_cie_info *dci = c->pi.unwind_info;
next_use_prev_instr = ! dci->signal_frame;
switch (c->pi.format) switch (c->pi.format)
{ {
case UNW_INFO_FORMAT_TABLE: case UNW_INFO_FORMAT_TABLE:
@ -1046,7 +1046,9 @@ dwarf_reg_states_iterate(struct dwarf_cursor *c,
Debug (1, "Unexpected unwind-info format %d\n", c->pi.format); Debug (1, "Unexpected unwind-info format %d\n", c->pi.format);
ret = -UNW_EINVAL; ret = -UNW_EINVAL;
} }
}
put_unwind_info (c, &c->pi); put_unwind_info (c, &c->pi);
c->use_prev_instr = next_use_prev_instr;
return ret; return ret;
} }