|
From: | Saleem Abdulrasool |
Subject: | Re: [Libunwind-devel] [PATCH] Correct handling of DW_CFA_GNU_args_size |
Date: | Mon, 5 Dec 2016 21:55:58 +0000 |
From: Dave Watson
Sent: Wednesday, November 30, 2016 1:31 PM To: address@hidden; address@hidden Cc: Saleem Abdulrasool Subject: [PATCH] Correct handling of DW_CFA_GNU_args_size When resuming execution, DW_CFA_GNU_args_size from the current frame
must be added back to the stack pointer. Clang now generates these frequently at -O3. A simple repro for x86_64, that will crash with clang ~3.9 or newer: void f(int, int,int,int,int,int,int,int,int); int main() { try { f(0,1,2,3,4,5,6,7,8); } catch (int) { return 0; } return 1; } Where f is something that throws an int, but in a different translation unit to prevent optimization. This results in cfi instructions before the call: .cfi_escape 0x2e, 0x20 Grabbing the args_size means fully parsing the cfi in the current frame, which is unfortunate because it means nearly twice the work at each step. The logic to grab args_size can be in unw_step or get_proc_info (since this is always called before resuming in stack unwinding). Putting it in get_proc_info allows the more common unw_step code to remain fast. It would potentially fit in nicely with a proc info cache (as mentioned in the if0 comment block) --- src/dwarf/Gparser.c | 16 ++++++++++++++-- src/x86/Gresume.c | 9 +++++++++ src/x86_64/Gresume.c | 9 +++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/dwarf/Gparser.c b/src/dwarf/Gparser.c index 3a47255..5336a2a 100644 --- a/src/dwarf/Gparser.c +++ b/src/dwarf/Gparser.c @@ -925,7 +925,19 @@ dwarf_make_proc_info (struct dwarf_cursor *c) if (c->as->caching_policy == UNW_CACHE_NONE || get_cached_proc_info (c) < 0) #endif - /* Lookup it up the slow way... */ - return fetch_proc_info (c, c->ip, 0); + dwarf_state_record_t sr; + int ret; + + /* Lookup it up the slow way... */ + if ((ret = fetch_proc_info (c, c->ip, 1)) < 0) + return ret; + /* Also need to check if current frame contains + args_size, and set cursor appropriatly. Only + needed for unw_resume */ + if ((ret = dwarf_create_state_record (c, &sr)) < 0) + return ret; + put_unwind_info (c, &c->pi); + c->args_size = sr.args_size; + return 0; } diff --git a/src/x86/Gresume.c b/src/x86/Gresume.c index cb3fb31..9bec497 100644 --- a/src/x86/Gresume.c +++ b/src/x86/Gresume.c @@ -63,6 +63,15 @@ establish_machine_state (struct cursor *c) (*access_reg) (as, reg, &val, 1, arg); } } + + if (c->dwarf.args_size) + { + if (tdep_access_reg (c, RSP, &val, 0) >= 0) Shouldnt this be ESP?
+ { + val += c->dwarf.args_size; + (*access_reg) (as, RSP, &val, 1, arg); And this.
+ } + } return 0; } diff --git a/src/x86_64/Gresume.c b/src/x86_64/Gresume.c index 6880d94..b880b01 100644 --- a/src/x86_64/Gresume.c +++ b/src/x86_64/Gresume.c @@ -95,6 +95,15 @@ establish_machine_state (struct cursor *c) (*access_reg) (as, reg, &val, 1, arg); } } + + if (c->dwarf.args_size) + { + if (tdep_access_reg (c, RSP, &val, 0) >= 0) + { + val += c->dwarf.args_size; + (*access_reg) (as, RSP, &val, 1, arg); + } + } return 0; } -- 2.8.0-rc2 |
[Prev in Thread] | Current Thread | [Next in Thread] |