Re: [patch] circ.exp

Pedro Alves
2013-04-17T21:20:17Z

On 04/17/2013 05:06 PM, Abid, Hafiz wrote:
>
>> > 4) Many tests were expecting current stack frame to be printed as result of
>> > tfind command e.g. "#0 func9 .*". But this only happens when traceframe_number
>> > is -1. In the following code, old_frame_id equal to current frame id unless
>> > traceframe_number < 0.
>> >
>>
>> I'm confused. Does the tfind in question switch to a different
>> frame or not?
> I have observed that location is only printed first time we give tfind command (or give it after doing a tfind none).
> If you see the example session below, you will see that 'tfind start' prints the following line.
> #0 func0 () at ../.././../git/gdb/testsuite/gdb.trace/circ.c:28
>
> But next tfind does not print any such line. The test was assuming the presence of this line every time. I am not sure what is expected behavior is though.
>
> (gdb) tstart
> (gdb) info tracepoints
> Num Type Disp Enb Address What
> 1 tracepoint keep y 0x080483b7 in func0
> at ../.././../git/gdb/testsuite/gdb.trace/circ.c:28
> collect testload
> installed on target
> 2 tracepoint keep y 0x080483bc in func1
> at ../.././../git/gdb/testsuite/gdb.trace/circ.c:32
> collect testload
> installed on target
> (gdb) c
> Continuing.
>
> Breakpoint 3, end () at ../.././../git/gdb/testsuite/gdb.trace/circ.c:72
> 72 }
> (gdb) tstop
> (gdb) tfind start
> Found trace frame 0, tracepoint 1
> #0 func0 () at ../.././../git/gdb/testsuite/gdb.trace/circ.c:28
> 28 }
> (gdb) tfind
> Found trace frame 1, tracepoint 2
> 32 }
> (gdb)

I see. The issue is that when haven't collected any registers in the
the tracepoint (collect $regs), we end up with a frames that are
un-unwindable (UNWIND_UNAVAILABLE). All frames in that case have the same
id (outer_frame_id), even if they represent different functions. I started
out with a local fix in tfind_1, but that got ugly quick. I think we
should fix this in the frame machinery itself. Here's a WIP patch for that.
It makes us build a frame id that represents a function, with an <unavailable>
stack address. I haven't thought every detail through, but I think it's
on the right path.

---

gdb/frame.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++--------
gdb/frame.h | 8 +++++---
2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index 8d4e2c8..c62202c 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -47,6 +47,8 @@
static struct frame_info *get_prev_frame_1 (struct frame_info *this_frame);
static struct frame_info *get_prev_frame_raw (struct frame_info *this_frame);

+static struct frame_id frame_id_build_unavailable_stack (CORE_ADDR code_addr);
+
/* We keep a cache of stack frames, each of which is a "struct
frame_info". The innermost one gets allocated (in
wait_for_inferior) each time the inferior stops; current_frame
@@ -210,7 +212,9 @@ show_backtrace_limit (struct ui_file *file, int from_tty,
static void
fprint_field (struct ui_file *file, const char *name, int p, CORE_ADDR addr)
{
- if (p)
+ if (p < 0)
+ fprintf_unfiltered (file, "%s=<unavailable>", name);
+ else if (p > 0)
fprintf_unfiltered (file, "%s=%s", name, hex_string (addr));
else
fprintf_unfiltered (file, "!%s", name);
@@ -333,10 +337,27 @@ get_frame_id (struct frame_info *fi)
/* Find the unwinder. */
if (fi->unwind == NULL)
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);
+
+ fi->stop_reason = fi->unwind->stop_reason (fi,&fi->prologue_cache);
+
+ /* Find THIS frame's ID. If the stack is unavailable, build a
+ frame id that identifies the function (and that only matches
+ equal with another stackless frame id of the same
+ function). */
+ if (fi->stop_reason == UNWIND_UNAVAILABLE)
+ {
+ CORE_ADDR pc;
+
+ pc = get_frame_func (fi);
+ fi->this_id.value = frame_id_build_unavailable_stack (pc);
+ }
+ else
+ {
+ /* 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)
@@ -394,6 +415,22 @@ frame_id_build_special (CORE_ADDR stack_addr, CORE_ADDR code_addr,
return id;
}

+/* Construct a frame ID representing a frame where the stack address
+ is unavailable (but exists). The first parameter is the frame's
+ constant code address (typically the entry point). The special
+ identifier address is set to indicate a wild card. */
+
+static struct frame_id
+frame_id_build_unavailable_stack (CORE_ADDR code_addr)
+{
+ struct frame_id id = null_frame_id;
+
+ id.stack_addr_p = -1;
+ id.code_addr = code_addr;
+ id.code_addr_p = 1;
+ return id;
+}
+
struct frame_id
frame_id_build (CORE_ADDR stack_addr, CORE_ADDR code_addr)
{
@@ -461,7 +498,7 @@ frame_id_eq (struct frame_id l, struct frame_id r)
/* 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;
- else if (l.stack_addr != r.stack_addr)
+ else if (l.stack_addr_p != r.stack_addr_p || l.stack_addr != r.stack_addr)
/* If .stack addresses are different, the frames are different. */
eq = 0;
else if (l.code_addr_p && r.code_addr_p && l.code_addr != r.code_addr)
@@ -528,8 +565,9 @@ frame_id_inner (struct gdbarch *gdbarch, struct frame_id l, struct frame_id r)
{
int inner;

- if (!l.stack_addr_p || !r.stack_addr_p)
- /* Like NaN, any operation involving an invalid ID always fails. */
+ if (l.stack_addr_p != 1 || r.stack_addr_p != 1)
+ /* Like NaN, any operation involving an invalid ID always fails.
+ Likewise if either ID has an unavailable stack address. */
inner = 0;
else if (l.artificial_depth > r.artificial_depth
&& l.stack_addr == r.stack_addr
diff --git a/gdb/frame.h b/gdb/frame.h
index 31b9cb7..66226d2 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -97,8 +97,10 @@ struct frame_id
function pointer register or stack pointer register. They are
wrong.

- This field is valid only if stack_addr_p is true. Otherwise, this
- frame represents the null frame. */
+ This field is valid only if stack_addr_p is positive. If 0, this
+ frame represents the null frame. If -1, we don't have the stack
+ address available -- e.g., we're inspecting a trace frame where
+ enough registers hadn't been collected. */
CORE_ADDR stack_addr;

/* The frame's code address. This shall be constant through out the
@@ -129,7 +131,7 @@ struct frame_id
CORE_ADDR special_addr;

/* Flags to indicate the above fields have valid contents. */
- unsigned int stack_addr_p : 1;
+ int stack_addr_p : 2;
unsigned int code_addr_p : 1;
unsigned int special_addr_p : 1;