-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #24379 from JuliaLang/yyc/unw
Add libunwind patch to fix dwarf unwinding
- Loading branch information
Showing
2 changed files
with
205 additions
and
1 deletion.
There are no files selected for viewing
200 changes: 200 additions & 0 deletions
200
deps/patches/libunwind-dwarf-Fix-incorrect-cfi-execution.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| From 73b106c6a263fb548d4d6e0c783148df9806fd0b Mon Sep 17 00:00:00 2001 | ||
| From: Yichao Yu <[email protected]> | ||
| Date: Fri, 27 Oct 2017 13:50:58 +0000 | ||
| Subject: [PATCH] dwarf: Fix incorrect cfi execution | ||
|
|
||
| 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. | ||
|
|
||
| 3d9a694de85f2ba10368b4fbc2aff1c6b8b76f58 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> | ||
| ``` | ||
|
|
||
| e9e8ed73e34a2d65c7ec71c296156637763ffd5c 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. | ||
| --- | ||
| src/dwarf/Gparser.c | 53 +++++++++++++++++++++++++++-------------------------- | ||
| 1 file changed, 27 insertions(+), 26 deletions(-) | ||
|
|
||
| diff --git a/src/dwarf/Gparser.c b/src/dwarf/Gparser.c | ||
| index 3a47255..8ffc3f4 100644 | ||
| --- a/src/dwarf/Gparser.c | ||
| +++ b/src/dwarf/Gparser.c | ||
| @@ -82,9 +82,6 @@ run_cfi_program (struct dwarf_cursor *c, dwarf_state_record_t *sr, | ||
| a = unw_get_accessors (as); | ||
| curr_ip = c->pi.start_ip; | ||
|
|
||
| - /* Process everything up to and including the current 'ip', | ||
| - including all the DW_CFA_advance_loc instructions. See | ||
| - 'c->use_prev_instr' use in 'fetch_proc_info' for details. */ | ||
| while (curr_ip <= ip && *addr < end_addr) | ||
| { | ||
| if ((ret = dwarf_readu8 (as, a, addr, &op, arg)) < 0) | ||
| @@ -401,7 +398,7 @@ run_cfi_program (struct dwarf_cursor *c, dwarf_state_record_t *sr, | ||
| } | ||
|
|
||
| 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; | ||
|
|
||
| @@ -415,7 +412,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 | ||
| 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 | ||
| 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 | ||
| @@ -423,18 +420,14 @@ fetch_proc_info (struct dwarf_cursor *c, unw_word_t ip, int need_unwind_info) | ||
| if (c->use_prev_instr) | ||
| --ip; | ||
|
|
||
| - if (c->pi_valid && !need_unwind_info) | ||
| - return 0; | ||
| - | ||
| memset (&c->pi, 0, sizeof (c->pi)); | ||
|
|
||
| /* check dynamic info first --- it overrides everything else */ | ||
| - ret = unwi_find_dynamic_proc_info (c->as, ip, &c->pi, need_unwind_info, | ||
| - c->as_arg); | ||
| + ret = unwi_find_dynamic_proc_info (c->as, ip, &c->pi, 1, c->as_arg); | ||
| if (ret == -UNW_ENOINFO) | ||
| { | ||
| dynamic = 0; | ||
| - if ((ret = tdep_find_proc_info (c, ip, need_unwind_info)) < 0) | ||
| + if ((ret = tdep_find_proc_info (c, ip, 1)) < 0) | ||
| return ret; | ||
| } | ||
|
|
||
| @@ -448,15 +441,7 @@ fetch_proc_info (struct dwarf_cursor *c, unw_word_t ip, int need_unwind_info) | ||
|
|
||
| /* Let system/machine-dependent code determine frame-specific attributes. */ | ||
| if (ret >= 0) | ||
| - tdep_fetch_frame (c, ip, need_unwind_info); | ||
| - | ||
| - /* 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; | ||
| - } | ||
| + tdep_fetch_frame (c, ip, 1); | ||
|
|
||
| return ret; | ||
| } | ||
| @@ -502,7 +487,7 @@ parse_fde (struct dwarf_cursor *c, unw_word_t ip, dwarf_state_record_t *sr) | ||
| memcpy (&sr->rs_initial, &sr->rs_current, sizeof (sr->rs_initial)); | ||
|
|
||
| addr = dci->fde_instr_start; | ||
| - if ((ret = run_cfi_program (c, sr, ip, &addr, dci->fde_instr_end, dci)) < 0) | ||
| + if ((ret = run_cfi_program (c, sr, ip - c->use_prev_instr, &addr, dci->fde_instr_end, dci)) < 0) | ||
| return ret; | ||
|
|
||
| return 0; | ||
| @@ -842,19 +827,30 @@ uncached_dwarf_find_save_locs (struct dwarf_cursor *c) | ||
| dwarf_state_record_t sr; | ||
| int ret; | ||
|
|
||
| - if ((ret = fetch_proc_info (c, c->ip, 1)) < 0) | ||
| + if ((ret = fetch_proc_info (c, c->ip)) < 0) | ||
| { | ||
| put_unwind_info (c, &c->pi); | ||
| return ret; | ||
| } | ||
| + /* Update use_prev_instr for the next frame. */ | ||
| + assert(c->pi.unwind_info); | ||
| + struct dwarf_cie_info *dci = c->pi.unwind_info; | ||
| + int next_use_prev_instr = ! dci->signal_frame; | ||
|
|
||
| if ((ret = create_state_record_for (c, &sr, c->ip)) < 0) | ||
| - return ret; | ||
| + { | ||
| + c->use_prev_instr = next_use_prev_instr; | ||
| + return ret; | ||
| + } | ||
|
|
||
| if ((ret = apply_reg_state (c, &sr.rs_current)) < 0) | ||
| - return ret; | ||
| + { | ||
| + c->use_prev_instr = next_use_prev_instr; | ||
| + return ret; | ||
| + } | ||
|
|
||
| put_unwind_info (c, &c->pi); | ||
| + c->use_prev_instr = next_use_prev_instr; | ||
| return 0; | ||
| } | ||
|
|
||
| @@ -882,13 +878,17 @@ dwarf_find_save_locs (struct dwarf_cursor *c) | ||
| } | ||
| else | ||
| { | ||
| - if ((ret = fetch_proc_info (c, c->ip, 1)) < 0 || | ||
| + if ((ret = fetch_proc_info (c, c->ip)) < 0 || | ||
| (ret = create_state_record_for (c, &sr, c->ip)) < 0) | ||
| { | ||
| put_rs_cache (c->as, cache, &saved_mask); | ||
| put_unwind_info (c, &c->pi); | ||
| return ret; | ||
| } | ||
| + /* Update use_prev_instr for the next frame. */ | ||
| + assert(c->pi.unwind_info); | ||
| + struct dwarf_cie_info *dci = c->pi.unwind_info; | ||
| + int next_use_prev_instr = ! dci->signal_frame; | ||
|
|
||
| rs = rs_new (cache, c); | ||
| memcpy(rs, &sr.rs_current, offsetof(struct dwarf_reg_state, ip)); | ||
| @@ -898,6 +898,7 @@ dwarf_find_save_locs (struct dwarf_cursor *c) | ||
| c->prev_rs = rs - cache->buckets; | ||
|
|
||
| put_unwind_info (c, &c->pi); | ||
| + c->use_prev_instr = next_use_prev_instr; | ||
| } | ||
|
|
||
| memcpy (&rs_copy, rs, sizeof (rs_copy)); | ||
| @@ -926,6 +927,6 @@ dwarf_make_proc_info (struct dwarf_cursor *c) | ||
| || get_cached_proc_info (c) < 0) | ||
| #endif | ||
| /* Lookup it up the slow way... */ | ||
| - return fetch_proc_info (c, c->ip, 0); | ||
| + return fetch_proc_info (c, c->ip); | ||
| return 0; | ||
| } | ||
| -- | ||
| 2.14.3 | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters