Re: [PATCH] Some tracepoint fixes

[Nathan Sidwell <nathan@codesourcery.com>]

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

Daniel Jacobowitz wrote:
> On Wed, Feb 09, 2005 at 09:19:17AM +0000, Nathan Sidwell wrote:
> 
>>In porting gdb to a new architecture, I came across a number of core gdb
>>bugs.   Here is the first set of them and addresses the following issues,
>>
>>1) we did not allow 'extended-remote' targets to use tracepoints.
>>
>>2) We could only trace architectures with 64 registers, not 256 like
>>a comment suggested.
>>
>>3) There was an erroneous comment about tracing memory ranges
>>
>>4) If a ^D was entered when entering the 'actions' list, we'd create
>>a NULL action, which would cause a segfault when tracing started.

> Some bits of this are OK, some aren't (and I'd want Michael's opinion
> on them).  In particular, the tracepoint remote protocol packets don't
> appear to be documented; so I'm not sure about the strtol change.  Your
> change is definitely wrong one way or another, because it depends on
> the size of "long" on the host.  You're passing a long* to sscanf where
> it expects an unsigned long*.  If we expect a hex-encoded 32-bit
> result, then let's parse it that way explicitly.

ok. I've installed the attached patch, which are the uncontraversial
bits.  I'll address the strtol one separately.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk


[-- Attachment #2: tracepoint1a.patch --]
[-- Type: text/plain, Size: 2388 bytes --]

2005-03-08  Nathan Sidwell  <nathan@codesourcery.com>

	* tracepoint.c (target_is_remote): Allow extended-remote.
	(struct collection_list): Allow 256 registers, like the comment
	said.
	(add_memrange): Fix comment.
	(read_actions): Turn EOF into 'end'.

Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.68
diff -c -3 -p -r1.68 tracepoint.c
*** tracepoint.c	2 Feb 2005 00:20:05 -0000	1.68
--- tracepoint.c	8 Feb 2005 11:31:20 -0000
*************** static int
*** 164,170 ****
  target_is_remote (void)
  {
    if (current_target.to_shortname &&
!       strcmp (current_target.to_shortname, "remote") == 0)
      return 1;
    else
      return 0;
--- 164,171 ----
  target_is_remote (void)
  {
    if (current_target.to_shortname &&
!       (strcmp (current_target.to_shortname, "remote") == 0
!        || strcmp (current_target.to_shortname, "extended-remote") == 0))
      return 1;
    else
      return 0;
*************** read_actions (struct tracepoint *t)
*** 860,865 ****
--- 861,869 ----
        else
  	line = gdb_readline (0);
  
+       if (!line)
+ 	line = "end";
+       
        linetype = validate_actionline (&line, t);
        if (linetype == BADLINE)
  	continue;		/* already warned -- collect another line */
*************** struct memrange
*** 1074,1080 ****
  
  struct collection_list
    {
!     unsigned char regs_mask[8];	/* room for up to 256 regs */
      long listsize;
      long next_memrange;
      struct memrange *list;
--- 1078,1084 ----
  
  struct collection_list
    {
!     unsigned char regs_mask[32];	/* room for up to 256 regs */
      long listsize;
      long next_memrange;
      struct memrange *list;
*************** add_memrange (struct collection_list *me
*** 1171,1177 ****
        printf_filtered (",%ld)\n", len);
      }
  
!   /* type: 0 == memory, n == basereg */
    memranges->list[memranges->next_memrange].type = type;
    /* base: addr if memory, offset if reg relative.  */
    memranges->list[memranges->next_memrange].start = base;
--- 1175,1181 ----
        printf_filtered (",%ld)\n", len);
      }
  
!   /* type: -1 == memory, n == basereg */
    memranges->list[memranges->next_memrange].type = type;
    /* base: addr if memory, offset if reg relative.  */
    memranges->list[memranges->next_memrange].start = base;