[PATCH] Infer $pc in a file's trace frame

[PATCH] Infer $pc in a file's trace frame

[Stan Shebs]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

This patch is a small usability enhancement from trace frames coming 
from a trace file; if registers have not been collected, then clear them 
all, and guess that $pc must be the same as the tracepoint's address.  
This isn't a good idea for either multi-location tracepoints or stepping 
frames though, and we want to warn the user about those cases.

Stan

2010-04-01  Stan Shebs  <stan@codesourcery.com>

    * tracepoint.c (tfile_fetch_registers): Add fallback case.

    * gdb.texinfo (Tracepoint Restrictions): Document PC inference.

    * gdb.trace/tfile.exp: Sharpen tfind test.


[-- Attachment #2: pcinfer-patch-1 --]
[-- Type: text/plain, Size: 4514 bytes --]

Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.167
diff -p -r1.167 tracepoint.c
*** tracepoint.c	1 Apr 2010 20:30:56 -0000	1.167
--- tracepoint.c	1 Apr 2010 22:03:06 -0000
*************** tfile_fetch_registers (struct target_ops
*** 3664,3670 ****
  {
    struct gdbarch *gdbarch = get_regcache_arch (regcache);
    char block_type;
!   int i, pos, offset, regn, regsize, gotten;
    unsigned short mlen;
    char *regs;
  
--- 3664,3670 ----
  {
    struct gdbarch *gdbarch = get_regcache_arch (regcache);
    char block_type;
!   int i, pos, offset, regn, regsize, gotten, pc_regno;
    unsigned short mlen;
    char *regs;
  
*************** tfile_fetch_registers (struct target_ops
*** 3739,3744 ****
--- 3739,3782 ----
  	  break;
  	}
      }
+ 
+   /* We get here if no register data has been found.  Although we
+      don't like making up numbers, GDB has all manner of troubles when
+      the target says some register is not available.  Filling in with
+      zeroes is a reasonable fallback.  */
+   for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
+     regcache_raw_supply (regcache, regn, NULL);
+ 
+   /* We can often usefully guess that the PC is going to be the same
+      as the address of the tracepoint.  */
+   pc_regno = gdbarch_pc_regnum (gdbarch);
+   if (pc_regno >= 0 && (regno == -1 || regno == pc_regno))
+     {
+       struct breakpoint *tp = get_tracepoint (tracepoint_number);
+ 
+       if (tp && tp->loc)
+ 	{
+ 	  /* But don't try to guess if tracepoint is multi-location...  */
+ 	  if (tp->loc->next)
+ 	    {
+ 	      warning ("Tracepoint %d has multiple locations, cannot infer $pc",
+ 		       tp->number);
+ 	      return;
+ 	    }
+ 	  /* ... or does while-stepping.  */
+ 	  if (tp->step_count > 0)
+ 	    {
+ 	      warning ("Tracepoint %d does while-stepping, cannot infer $pc",
+ 		       tp->number);
+ 	      return;
+ 	    }
+ 
+ 	  store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
+ 				  gdbarch_byte_order (gdbarch),
+ 				  tp->loc->address);
+ 	  regcache_raw_supply (regcache, pc_regno, regs);
+ 	}
+     }
  }
  
  static LONGEST
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.693
diff -p -r1.693 gdb.texinfo
*** doc/gdb.texinfo	1 Apr 2010 14:11:23 -0000	1.693
--- doc/gdb.texinfo	1 Apr 2010 22:03:07 -0000
*************** using the @code{finish} command.  This i
*** 9032,9037 ****
--- 9032,9050 ----
  debugging information; after @code{finish}, you can step to the next line
  and print a variable where your program stored the return value.
  
+ @item
+ If you do not collect registers at a tracepoint, @value{GDBN} can
+ infer that the value of the PC is the address of the tracepoint and
+ display that when you are looking at a trace frame for that
+ tracepoint.  However, this cannot work if the tracepoint has multiple
+ locations (for instance if it was set in a function that was inlined),
+ or if it has a @code{while-stepping} loop.  In those cases
+ @value{GDBN} will warn you that it can't infer the PC, and default it
+ to zero.  Also, @code{tdump} will use the list of collections for the
+ tracepoint proper, and not its stepping list, although the values
+ displayed will be correct for the stepping frame.  Explicit print
+ commands will always work correctly.
+ 
  @end itemize
  
  
Index: testsuite/gdb.trace/tfile.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/tfile.exp,v
retrieving revision 1.4
diff -p -r1.4 tfile.exp
*** testsuite/gdb.trace/tfile.exp	26 Mar 2010 01:46:29 -0000	1.4
--- testsuite/gdb.trace/tfile.exp	1 Apr 2010 22:03:07 -0000
*************** gdb_test "target tfile basic.tf" "Create
*** 69,75 ****
  gdb_test "info trace" ".*tracepoint.*in write_basic_trace_file.*" \
      "info tracepoints on trace file"
  
! gdb_test "tfind 0" "Found trace frame 0.*" "tfind 0 on trace file"
  
  gdb_test "print testglob" " = 31415" "print testglob on trace file"
  
--- 69,78 ----
  gdb_test "info trace" ".*tracepoint.*in write_basic_trace_file.*" \
      "info tracepoints on trace file"
  
! gdb_test "tfind 0" \
!     "Found trace frame 0, tracepoint \[0-9\]+.
! \#0  write_basic_trace_file ().*" \
!     "tfind 0 on trace file"
  
  gdb_test "print testglob" " = 31415" "print testglob on trace file"
  

Re: [PATCH] Infer $pc in a file's trace frame

[Eli Zaretskii]

> Date: Thu, 01 Apr 2010 15:15:24 -0700
> From: Stan Shebs <stan@codesourcery.com>
> 
> This patch is a small usability enhancement from trace frames coming 
> from a trace file; if registers have not been collected, then clear them 
> all, and guess that $pc must be the same as the tracepoint's address.  
> This isn't a good idea for either multi-location tracepoints or stepping 
> frames though, and we want to warn the user about those cases.

Thanks.

> + @item
> + If you do not collect registers at a tracepoint, @value{GDBN} can
> + infer that the value of the PC is the address of the tracepoint and
> + display that when you are looking at a trace frame for that
> + tracepoint.  However, this cannot work if the tracepoint has multiple
> + locations (for instance if it was set in a function that was inlined),
> + or if it has a @code{while-stepping} loop.  In those cases
> + @value{GDBN} will warn you that it can't infer the PC, and default it
> + to zero.  Also, @code{tdump} will use the list of collections for the
> + tracepoint proper, and not its stepping list, although the values
> + displayed will be correct for the stepping frame.  Explicit print
> + commands will always work correctly.

This is okay, but the last two sentences leave so much stuff
unexplained that I doubt the reader will be able to understand their
meaning, unless she is a tracepoint hacker.  At the very least, I
think you should:

  . add cross-references to where `tdump' and `while-stepping' are
    described

  . use the same terminology as you do there, and if the terminology
    is not explained there  ("stepping list"? what's that?), introduce
    it in those places

  . add an example to show the issues you are describing.

Btw, the description of `tdump' has nothing to say about any special
behavior in the presence of `while-stepping', whereas the above seems
to hint that there's something we should say about that.

Re: [PATCH] Infer $pc in a file's trace frame

[Stan Shebs]

Eli Zaretskii wrote:
> This is okay, but the last two sentences leave so much stuff
> unexplained that I doubt the reader will be able to understand their
> meaning, unless she is a tracepoint hacker.  [...]

It is a little obscure to explain, in fact it's a fundamental flaw of 
the tdump implementation that I didn't fully understand myself until 
recently.

What I did for now was to whack those last two sentences, and add a 
couple more paragraphs in the tdump description.  After writing them, my 
main thought is "what a lame design, can't we do tdump better?".  :-)  
So perhaps after gdbserver tracepoints go in and more people get some 
familiarity, we should revisit both code and  documentation.

Stan