From 005ca36a8b302b4d6da415e0cf66d5d9f684a8da Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Sun, 13 Sep 2009 16:28:29 +0000 Subject: [PATCH] gdb/ * frame.c (get_frame_id): Default to outer_frame_id if the this_id method does not supply an ID. Assert that the result is not null_frame_id. (outer_frame_id): New. (frame_id_p): Accept outer_frame_id. (frame_id_eq): Allow outer_frame_id to be equal to itself. (frame_find_by_id): Revert previous local workarounds. (get_prev_frame_1): Adjust end-of-stack check to test outer_frame_id. * frame.h (null_frame_id, frame_id_p): Update comments. (outer_frame_id): Declare. * infrun.c (handle_inferior_event): Do not treat all steps from the outermost frame as subroutine calls. * libunwind-frame.c (libunwind_frame_this_id): Do not clear THIS_ID. * hppa-tdep.c (hppa_stub_frame_this_id): Likewise. * ia64-tdep.c (ia64_frame_this_id): Likewise. (ia64_libunwind_frame_this_id, ia64_libunwind_sigtramp_frame_this_id): Use outer_frame_id instead of null_frame_id. * amd64obsd-tdep.c (amd64obsd_trapframe_cache): Use outer_frame_id. * i386obsd-tdep.c (i386obsd_trapframe_cache): Likewise. * inline-frame.c (inline_frame_this_id): Refuse outer_frame_id. * thread.c (restore_selected_frame): Update comment and remove frame_id_p check. gdb/doc/ * gdbint.texinfo (Unwinding the Frame ID): Reference outer_frame_id. --- gdb/ChangeLog | 26 ++++++++++++++++++++++++++ gdb/amd64obsd-tdep.c | 2 +- gdb/doc/ChangeLog | 4 ++++ gdb/doc/gdbint.texinfo | 10 +++++++++- gdb/frame.c | 18 ++++++++++++++++-- gdb/frame.h | 10 ++++++++-- gdb/hppa-tdep.c | 2 -- gdb/i386obsd-tdep.c | 2 +- gdb/ia64-tdep.c | 17 +++++++---------- gdb/infrun.c | 16 ++++++++++++++-- gdb/inline-frame.c | 4 ++++ gdb/libunwind-frame.c | 2 -- gdb/thread.c | 10 +++------- 13 files changed, 93 insertions(+), 30 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 80d33ac07e3..a1e66e62eae 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,29 @@ +2009-09-13 Daniel Jacobowitz + + * frame.c (get_frame_id): Default to outer_frame_id if the this_id + method does not supply an ID. Assert that the result is not + null_frame_id. + (outer_frame_id): New. + (frame_id_p): Accept outer_frame_id. + (frame_id_eq): Allow outer_frame_id to be equal to itself. + (frame_find_by_id): Revert previous local workarounds. + (get_prev_frame_1): Adjust end-of-stack check to test outer_frame_id. + * frame.h (null_frame_id, frame_id_p): Update comments. + (outer_frame_id): Declare. + * infrun.c (handle_inferior_event): Do not treat all steps from the + outermost frame as subroutine calls. + + * libunwind-frame.c (libunwind_frame_this_id): Do not clear THIS_ID. + * hppa-tdep.c (hppa_stub_frame_this_id): Likewise. + * ia64-tdep.c (ia64_frame_this_id): Likewise. + (ia64_libunwind_frame_this_id, ia64_libunwind_sigtramp_frame_this_id): + Use outer_frame_id instead of null_frame_id. + * amd64obsd-tdep.c (amd64obsd_trapframe_cache): Use outer_frame_id. + * i386obsd-tdep.c (i386obsd_trapframe_cache): Likewise. + * inline-frame.c (inline_frame_this_id): Refuse outer_frame_id. + * thread.c (restore_selected_frame): Update comment and remove + frame_id_p check. + 2009-09-11 Doug Evans * dwarf2expr.c (execute_stack_op, case DW_OP_piece): Delete unused diff --git a/gdb/amd64obsd-tdep.c b/gdb/amd64obsd-tdep.c index 1938bdd7279..ed57cb8c475 100644 --- a/gdb/amd64obsd-tdep.c +++ b/gdb/amd64obsd-tdep.c @@ -380,7 +380,7 @@ amd64obsd_trapframe_cache (struct frame_info *this_frame, void **this_cache) if ((cs & I386_SEL_RPL) == I386_SEL_UPL) { /* Trap from user space; terminate backtrace. */ - trad_frame_set_id (cache, null_frame_id); + trad_frame_set_id (cache, outer_frame_id); } else { diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 595bbe27bf0..8a9bb7b8ab9 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,7 @@ +2009-09-13 Daniel Jacobowitz + + * gdbint.texinfo (Unwinding the Frame ID): Reference outer_frame_id. + 2009-09-10 Michael Snyder * gdb.texinfo (qSupported): Mention new ReverseContinue and diff --git a/gdb/doc/gdbint.texinfo b/gdb/doc/gdbint.texinfo index e706caaf180..c2be3f7bf5d 100644 --- a/gdb/doc/gdbint.texinfo +++ b/gdb/doc/gdbint.texinfo @@ -2042,9 +2042,17 @@ address as its caller. On some platforms, a third address is part of the ID to further disambiguate frames---for instance, on IA-64 the separate register stack address is included in the ID. -An invalid frame ID (@code{null_frame_id}) returned from the +An invalid frame ID (@code{outer_frame_id}) returned from the @code{this_id} method means to stop unwinding after this frame. +@code{null_frame_id} is another invalid frame ID which should be used +when there is no frame. For instance, certain breakpoints are attached +to a specific frame, and that frame is identified through its frame ID +(we use this to implement the "finish" command). Using +@code{null_frame_id} as the frame ID for a given breakpoint means +that the breakpoint is not specific to any frame. The @code{this_id} +method should never return @code{null_frame_id}. + @section Unwinding Registers Each unwinder includes a @code{prev_register} method. This method diff --git a/gdb/frame.c b/gdb/frame.c index 9ca69bf6596..7932b48ee3b 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -324,7 +324,10 @@ get_frame_id (struct frame_info *fi) if (fi->unwind == NULL) fi->unwind = frame_unwind_find_by_frame (fi, &fi->prologue_cache); /* Find THIS frame's ID. */ + /* Default to outermost if no ID is found. */ + fi->this_id.value = outer_frame_id; fi->unwind->this_id (fi, &fi->prologue_cache, &fi->this_id.value); + gdb_assert (frame_id_p (fi->this_id.value)); fi->this_id.p = 1; if (frame_debug) { @@ -364,6 +367,7 @@ frame_unwind_caller_id (struct frame_info *next_frame) } const struct frame_id null_frame_id; /* All zeros. */ +const struct frame_id outer_frame_id = { 0, 0, 0, 0, 0, 1, 0 }; struct frame_id frame_id_build_special (CORE_ADDR stack_addr, CORE_ADDR code_addr, @@ -405,6 +409,9 @@ frame_id_p (struct frame_id l) int p; /* The frame is valid iff it has a valid stack address. */ p = l.stack_addr_p; + /* outer_frame_id is also valid. */ + if (!p && memcmp (&l, &outer_frame_id, sizeof (l)) == 0) + p = 1; if (frame_debug) { fprintf_unfiltered (gdb_stdlog, "{ frame_id_p (l="); @@ -427,7 +434,14 @@ int frame_id_eq (struct frame_id l, struct frame_id r) { int eq; - if (!l.stack_addr_p || !r.stack_addr_p) + if (!l.stack_addr_p && l.special_addr_p && !r.stack_addr_p && r.special_addr_p) + /* The outermost frame marker is equal to itself. This is the + dodgy thing about outer_frame_id, since between execution steps + we might step into another function - from which we can't + unwind either. More thought required to get rid of + outer_frame_id. */ + eq = 1; + else if (!l.stack_addr_p || !r.stack_addr_p) /* Like a NaN, if either ID is invalid, the result is false. Note that a frame ID is invalid iff it is the null frame ID. */ eq = 0; @@ -1425,7 +1439,7 @@ get_prev_frame_1 (struct frame_info *this_frame) unwind to the prev frame. Be careful to not apply this test to the sentinel frame. */ this_id = get_frame_id (this_frame); - if (this_frame->level >= 0 && !frame_id_p (this_id)) + if (this_frame->level >= 0 && frame_id_eq (this_id, outer_frame_id)) { if (frame_debug) { diff --git a/gdb/frame.h b/gdb/frame.h index 611c6d3562e..994b529f9d2 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -142,9 +142,14 @@ struct frame_id /* Methods for constructing and comparing Frame IDs. */ -/* For convenience. All fields are zero. */ +/* For convenience. All fields are zero. This means "there is no frame". */ extern const struct frame_id null_frame_id; +/* This means "there is no frame ID, but there is a frame". It should be + replaced by best-effort frame IDs for the outermost frame, somehow. + The implementation is only special_addr_p set. */ +extern const struct frame_id outer_frame_id; + /* Flag to control debugging. */ extern int frame_debug; @@ -170,7 +175,8 @@ extern struct frame_id frame_id_build_special (CORE_ADDR stack_addr, extern struct frame_id frame_id_build_wild (CORE_ADDR stack_addr); /* Returns non-zero when L is a valid frame (a valid frame has a - non-zero .base). */ + non-zero .base). The outermost frame is valid even without an + ID. */ extern int frame_id_p (struct frame_id l); /* Returns non-zero when L is a valid frame representing an inlined diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c index 072e687c815..03aa6c65c13 100644 --- a/gdb/hppa-tdep.c +++ b/gdb/hppa-tdep.c @@ -2427,8 +2427,6 @@ hppa_stub_frame_this_id (struct frame_info *this_frame, if (info) *this_id = frame_id_build (info->base, get_frame_func (this_frame)); - else - *this_id = null_frame_id; } static struct value * diff --git a/gdb/i386obsd-tdep.c b/gdb/i386obsd-tdep.c index 862183895d5..c33a24e5be4 100644 --- a/gdb/i386obsd-tdep.c +++ b/gdb/i386obsd-tdep.c @@ -376,7 +376,7 @@ i386obsd_trapframe_cache (struct frame_info *this_frame, void **this_cache) if ((cs & I386_SEL_RPL) == I386_SEL_UPL) { /* Trap from user space; terminate backtrace. */ - trad_frame_set_id (cache, null_frame_id); + trad_frame_set_id (cache, outer_frame_id); } else { diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c index bc72e405767..674204a9321 100644 --- a/gdb/ia64-tdep.c +++ b/gdb/ia64-tdep.c @@ -1774,9 +1774,7 @@ ia64_frame_this_id (struct frame_info *this_frame, void **this_cache, ia64_frame_cache (this_frame, this_cache); /* If outermost frame, mark with null frame id. */ - if (cache->base == 0) - (*this_id) = null_frame_id; - else + if (cache->base != 0) (*this_id) = frame_id_build_special (cache->base, cache->pc, cache->bsp); if (gdbarch_debug >= 1) fprintf_unfiltered (gdb_stdlog, @@ -2790,15 +2788,14 @@ ia64_libunwind_frame_this_id (struct frame_info *this_frame, void **this_cache, { struct gdbarch *gdbarch = get_frame_arch (this_frame); enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - struct frame_id id; + struct frame_id id = outer_frame_id; char buf[8]; CORE_ADDR bsp; - libunwind_frame_this_id (this_frame, this_cache, &id); - if (frame_id_eq (id, null_frame_id)) + if (frame_id_eq (id, outer_frame_id)) { - (*this_id) = null_frame_id; + (*this_id) = outer_frame_id; return; } @@ -2923,13 +2920,13 @@ ia64_libunwind_sigtramp_frame_this_id (struct frame_info *this_frame, enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); char buf[8]; CORE_ADDR bsp; - struct frame_id id; + struct frame_id id = outer_frame_id; CORE_ADDR prev_ip; libunwind_frame_this_id (this_frame, this_cache, &id); - if (frame_id_eq (id, null_frame_id)) + if (frame_id_eq (id, outer_frame_id)) { - (*this_id) = null_frame_id; + (*this_id) = outer_frame_id; return; } diff --git a/gdb/infrun.c b/gdb/infrun.c index c907635899f..a6ca2e3adfc 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3796,10 +3796,22 @@ infrun: not switching back to stepped thread, it has vanished\n"); NOTE: frame_id_eq will never report two invalid frame IDs as being equal, so to get into this block, both the current and previous frame must have valid frame IDs. */ + /* The outer_frame_id check is a heuristic to detect stepping + through startup code. If we step over an instruction which + sets the stack pointer from an invalid value to a valid value, + we may detect that as a subroutine call from the mythical + "outermost" function. This could be fixed by marking + outermost frames as !stack_p,code_p,special_p. Then the + initial outermost frame, before sp was valid, would + have code_addr == &_start. See the commend in frame_id_eq + for more. */ if (!frame_id_eq (get_stack_frame_id (frame), ecs->event_thread->step_stack_frame_id) - && frame_id_eq (frame_unwind_caller_id (frame), - ecs->event_thread->step_stack_frame_id)) + && (frame_id_eq (frame_unwind_caller_id (get_current_frame ()), + ecs->event_thread->step_stack_frame_id) + && (!frame_id_eq (ecs->event_thread->step_stack_frame_id, + outer_frame_id) + || step_start_function != find_pc_function (stop_pc)))) { CORE_ADDR real_stop_pc; diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c index 731d3adcaaf..0e6dae323fd 100644 --- a/gdb/inline-frame.c +++ b/gdb/inline-frame.c @@ -148,6 +148,10 @@ inline_frame_this_id (struct frame_info *this_frame, frame"). This will take work. */ gdb_assert (frame_id_p (*this_id)); + /* For now, require we don't match outer_frame_id either (see + comment above). */ + gdb_assert (!frame_id_eq (*this_id, outer_frame_id)); + /* Future work NOTE: Alexandre Oliva applied a patch to GCC 4.3 which generates DW_AT_entry_pc for inlined functions when possible. If this attribute is available, we should use it diff --git a/gdb/libunwind-frame.c b/gdb/libunwind-frame.c index b1896dc9fca..921d00ed40e 100644 --- a/gdb/libunwind-frame.c +++ b/gdb/libunwind-frame.c @@ -278,8 +278,6 @@ libunwind_frame_this_id (struct frame_info *this_frame, void **this_cache, if (cache != NULL) (*this_id) = frame_id_build (cache->base, cache->func_addr); - else - (*this_id) = null_frame_id; } struct value * diff --git a/gdb/thread.c b/gdb/thread.c index a7ac3c8a9ae..55b4b96fc6b 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -885,15 +885,11 @@ restore_selected_frame (struct frame_id a_frame_id, int frame_level) frame = find_relative_frame (get_current_frame (), &count); if (count == 0 && frame != NULL - /* Either the frame ids match, of they're both invalid. The - latter case is not failsafe, but since it's highly unlikely + /* The frame ids must match - either both valid or both outer_frame_id. + The latter case is not failsafe, but since it's highly unlikely the search by level finds the wrong frame, it's 99.9(9)% of the time (for all practical purposes) safe. */ - && (frame_id_eq (get_frame_id (frame), a_frame_id) - /* Note: could be better to check every frame_id - member for equality here. */ - || (!frame_id_p (get_frame_id (frame)) - && !frame_id_p (a_frame_id)))) + && frame_id_eq (get_frame_id (frame), a_frame_id)) { /* Cool, all is fine. */ select_frame (frame); -- 2.30.2