This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [gdb-7.0/doco] Re: RFC: Mark outer frames
[with the patch, this time...]
Eli,
This patch adds some documentation back about null_frame_id, as you
asked a while back. Does this looks good to you?
--
Joel
commit dbcfda80ee04f0aba772bc7b58f10969243fddf7
Author: Joel Brobecker <brobecker@adacore.com>
Date: Sat Sep 12 14:10:27 2009 -0700
* 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.
diff --git a/gdb/doc/gdbint.texinfo b/gdb/doc/gdbint.texinfo
index e706caa..52155c1 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/amd64obsd-tdep.c b/gdb/amd64obsd-tdep.c
index 1938bdd..ed57cb8 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/frame.c b/gdb/frame.c
index 9ca69bf..7932b48 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 611c6d3..994b529 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 072e687..03aa6c6 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 8621838..c33a24e 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 bc72e40..674204a 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 c907635..a6ca2e3 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 731d3ad..0e6dae3 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 b1896dc..921d00e 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 a7ac3c8..55b4b96 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);