[PATCH/commit] Handle errors in tracepoint target agent

Stan Shebs
2010-03-26T01:56:07Z

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

For the record, here's what I ended up committing. The hex2bin sniffing
idea is not a winner, because it calls error() if it sees a non-hex
character, so I just added a prefix character 'X' that signals that the
error message is hex-encoded instead of plain text; targets can do
either as they prefer. The error message is never NULL, I added the MI
status bit overlooked before, and improved the trace file generator's
portability, although it could probably use more. (A bit of a tricky
program, but a useful investment, as independent implementation of file
format.)

Stan

2010-03-25 Stan Shebs <stan@codesourcery.com>

* tracepoint.h (trace_stop_reason): Add tracepoint_error.
(struct trace_status): New field error_desc.
* tracepoint.c (stop_reason_names): Add terror.
(current_trace_status): Ensure non-NULL error description.
(trace_status_command): Add error report.
(trace_status_mi): Ditto.
(trace_save): Add special case for error description.
(parse_trace_status): Add case for errors.

* gdb.texinfo (Tracepoint Packets): Document trace error status.

* gdb.trace/tfile.c: Generate an additional trace file, improve
portability.
* gdb.trace/tfile.exp: Test trace file with an error stop, delete
files in a better way.

[-- Attachment #2: terror-patch-2 --]
[-- Type: text/plain, Size: 13342 bytes --]

Index: tracepoint.c
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.c,v
retrieving revision 1.155
diff -p -r1.155 tracepoint.c
*** tracepoint.c 24 Mar 2010 21:12:18 -0000 1.155
--- tracepoint.c 26 Mar 2010 01:43:59 -0000
*************** char *stop_reason_names[] = {
*** 195,206 ****
"tstop",
"tfull",
"tdisconnected",
! "tpasscount"
};

struct trace_status *
current_trace_status ()
{
return &trace_status;
}

--- 195,210 ----
"tstop",
"tfull",
"tdisconnected",
! "tpasscount",
! "terror"
};

struct trace_status *
current_trace_status ()
{
+ /* Ensure this is never NULL. */
+ if (!trace_status.error_desc)
+ trace_status.error_desc = "";
return &trace_status;
}

*************** trace_status_command (char *args, int fr
*** 1570,1575 ****
--- 1574,1587 ----
printf_filtered (_("Trace stopped by tracepoint %d.\n"),
ts->stopping_tracepoint);
break;
+ case tracepoint_error:
+ if (ts->stopping_tracepoint)
+ printf_filtered (_("Trace stopped by an error (%s, tracepoint %d).\n"),
+ ts->error_desc, ts->stopping_tracepoint);
+ else
+ printf_filtered (_("Trace stopped by an error (%s).\n"),
+ ts->error_desc);
+ break;
case trace_stop_reason_unknown:
printf_filtered (_("Trace stopped for an unknown reason.\n"));
break;
*************** trace_status_mi (int on_stop)
*** 1684,1689 ****
--- 1696,1705 ----
stop_reason = "passcount";
stopping_tracepoint = ts->stopping_tracepoint;
break;
+ case tracepoint_error:
+ stop_reason = "error";
+ stopping_tracepoint = ts->stopping_tracepoint;
+ break;
}

if (stop_reason)
*************** trace_status_mi (int on_stop)
*** 1692,1697 ****
--- 1708,1716 ----
if (stopping_tracepoint != -1)
ui_out_field_int (uiout, "stopping-tracepoint",
stopping_tracepoint);
+ if (ts->stop_reason == tracepoint_error)
+ ui_out_field_string (uiout, "error-description",
+ ts->error_desc);
}
}
}
*************** trace_save (const char *filename, int ta
*** 2463,2471 ****
fprintf (fp, "R %x\n", trace_regblock_size);

/* Write out status of the tracing run (aka "tstatus" info). */
! fprintf (fp, "status %c;%s:%x",
! (ts->running ? '1' : '0'),
! stop_reason_names[ts->stop_reason], ts->stopping_tracepoint);
if (ts->traceframe_count >= 0)
fprintf (fp, ";tframes:%x", ts->traceframe_count);
if (ts->traceframes_created >= 0)
--- 2482,2497 ----
fprintf (fp, "R %x\n", trace_regblock_size);

/* Write out status of the tracing run (aka "tstatus" info). */
! fprintf (fp, "status %c;%s",
! (ts->running ? '1' : '0'), stop_reason_names[ts->stop_reason]);
! /* Encode the error message in hex, might have weird chars. */
! if (ts->stop_reason == tracepoint_error)
! {
! char *buf = (char *) alloca (strlen (ts->error_desc) * 2 + 1);
! bin2hex ((gdb_byte *) ts->error_desc, buf, 0);
! fprintf (fp, ":X%s", buf);
! }
! fprintf (fp, ":%x", ts->stopping_tracepoint);
if (ts->traceframe_count >= 0)
fprintf (fp, ";tframes:%x", ts->traceframe_count);
if (ts->traceframes_created >= 0)
*************** extern char *unpack_varlen_hex (char *bu
*** 3126,3137 ****
void
parse_trace_status (char *line, struct trace_status *ts)
{
! char *p = line, *p1, *p_temp;
ULONGEST val;

ts->running_known = 1;
ts->running = (*p++ == '1');
ts->stop_reason = trace_stop_reason_unknown;
ts->traceframe_count = -1;
ts->traceframes_created = -1;
ts->buffer_free = -1;
--- 3152,3164 ----
void
parse_trace_status (char *line, struct trace_status *ts)
{
! char *p = line, *p1, *p2, *p_temp;
ULONGEST val;

ts->running_known = 1;
ts->running = (*p++ == '1');
ts->stop_reason = trace_stop_reason_unknown;
+ ts->error_desc = "";
ts->traceframe_count = -1;
ts->traceframes_created = -1;
ts->buffer_free = -1;
*************** Status line: '%s'\n"), p, line);
*** 3164,3169 ****
--- 3191,3220 ----
p = unpack_varlen_hex (++p1, &val);
ts->stop_reason = tstop_command;
}
+ else if (strncmp (p, stop_reason_names[tracepoint_error], p1 - p) == 0)
+ {
+ p2 = strchr (++p1, ':');
+ if (p2 != p1)
+ {
+ int end;
+ ts->error_desc = (char *) xmalloc (p2 - p1 + 1);
+ /* See if we're doing plain text or hex encoding. */
+ if (*p1 == 'X')
+ {
+ ++p1;
+ end = hex2bin (p1, ts->error_desc, (p2 - p1) / 2);
+ }
+ else
+ {
+ memcpy (ts->error_desc, p1, p2 - p1);
+ end = p2 - p1;
+ }
+ ts->error_desc[end] = '\0';
+ }
+ p = unpack_varlen_hex (++p2, &val);
+ ts->stopping_tracepoint = val;
+ ts->stop_reason = tracepoint_error;
+ }
else if (strncmp (p, "tframes", p1 - p) == 0)
{
p = unpack_varlen_hex (++p1, &val);
Index: tracepoint.h
===================================================================
RCS file: /cvs/src/src/gdb/tracepoint.h,v
retrieving revision 1.29
diff -p -r1.29 tracepoint.h
*** tracepoint.h 23 Mar 2010 22:05:45 -0000 1.29
--- tracepoint.h 26 Mar 2010 01:43:59 -0000
*************** enum trace_stop_reason
*** 73,79 ****
tstop_command,
trace_buffer_full,
trace_disconnected,
! tracepoint_passcount
};

struct trace_status
--- 73,80 ----
tstop_command,
trace_buffer_full,
trace_disconnected,
! tracepoint_passcount,
! tracepoint_error
};

struct trace_status
*************** struct trace_status
*** 89,98 ****

enum trace_stop_reason stop_reason;

! /* If stop_reason == tracepoint_passcount, the on-target number
! of the tracepoint which caused the stop. */
int stopping_tracepoint;

/* Number of traceframes currently in the buffer. */

int traceframe_count;
--- 90,104 ----

enum trace_stop_reason stop_reason;

! /* If stop_reason is tracepoint_passcount or tracepoint_error, this
! is the (on-target) number of the tracepoint which caused the
! stop. */
int stopping_tracepoint;

+ /* If stop_reason is tracepoint_error, this is a human-readable
+ string that describes the error that happened on the target. */
+ char *error_desc;
+
/* Number of traceframes currently in the buffer. */

int traceframe_count;
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.683
diff -p -r1.683 gdb.texinfo
*** doc/gdb.texinfo 24 Mar 2010 21:24:08 -0000 1.683
--- doc/gdb.texinfo 26 Mar 2010 01:44:00 -0000
*************** The trace stopped because @value{GDBN} d
*** 31362,31367 ****
--- 31362,31376 ----
@item tpasscount:@var{tpnum}
The trace stopped because tracepoint @var{tpnum} exceeded its pass count.

+ @item terror:@var{text}:@var{tpnum}
+ The trace stopped because tracepoint @var{tpnum} had an error. The
+ string @var{text} is available to describe the nature of the error
+ (for instance, a divide by zero in the condition expression).
+ @var{text} may take either of two forms; it may be plain text, but
+ with the restriction that no colons or other special characters are
+ allowed, or it may be an @code{X} followed by hex digits encoding the
+ text string.
+
@item tunknown:0
The trace stopped for some other reason.

Index: testsuite/gdb.trace/tfile.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/tfile.c,v
retrieving revision 1.1
diff -p -r1.1 tfile.c
*** testsuite/gdb.trace/tfile.c 15 Jan 2010 22:37:20 -0000 1.1
--- testsuite/gdb.trace/tfile.c 26 Mar 2010 01:44:00 -0000
*************** finish_trace_file (int fd)
*** 41,49 ****
}

void
! write_basic_trace_file ()
{
! int fd;

fd = start_trace_file ("basic.tf");

--- 41,51 ----
}

void
! write_basic_trace_file (void)
{
! int fd, int_x;
! short short_x;
! long long ll_x;

fd = start_trace_file ("basic.tf");

*************** write_basic_trace_file ()
*** 61,66 ****
--- 63,69 ----

/* Dump tracepoint definitions, in syntax similar to that used
for reconnection uploads. */
+ /* FIXME need a portable way to print function address in hex */
snprintf (spbuf, sizeof spbuf, "tp T1:%lx:E:0:0\n",
(long) &write_basic_trace_file);
write (fd, spbuf, strlen (spbuf));
*************** write_basic_trace_file ()
*** 71,98 ****
write (fd, "\n", 1);

/* Make up a simulated trace buffer. */
! /* (Encapsulate better if we're going to do lots of this.) */
trptr = trbuf;
! *((short *) trptr) = 1;
! trptr += sizeof (short);
tfsizeptr = trptr;
! trptr += sizeof (int);
*((char *) trptr) = 'M';
trptr += 1;
! *((long long *) trptr) = (long) &testglob;
trptr += sizeof (long long);
! *((short *) trptr) = sizeof (testglob);
! trptr += sizeof (short);
! *((int *) trptr) = testglob;
trptr += sizeof (testglob);
/* Go back and patch in the frame size. */
! *((int *) tfsizeptr) = trptr - tfsizeptr - sizeof (int);

/* Write end of tracebuffer marker. */
! *((short *) trptr) = 0;
! trptr += sizeof (short);
! *((int *) trptr) = 0;
! trptr += sizeof (int);

write (fd, trbuf, trptr - trbuf);

--- 74,146 ----
write (fd, "\n", 1);

/* Make up a simulated trace buffer. */
! /* (Encapsulate better if we're going to do lots of this; note that
! buffer endianness is the target program's enddianness.) */
trptr = trbuf;
! short_x = 1;
! memcpy (trptr, &short_x, 2);
! trptr += 2;
tfsizeptr = trptr;
! trptr += 4;
*((char *) trptr) = 'M';
trptr += 1;
! ll_x = (long) &testglob;
! memcpy (trptr, &ll_x, sizeof (long long));
trptr += sizeof (long long);
! short_x = sizeof (testglob);
! memcpy (trptr, &short_x, 2);
! trptr += 2;
! memcpy (trptr, &testglob, sizeof (testglob));
trptr += sizeof (testglob);
/* Go back and patch in the frame size. */
! int_x = trptr - tfsizeptr - sizeof (int);
! memcpy (tfsizeptr, &int_x, 4);
!
! /* Write end of tracebuffer marker. */
! memset (trptr, 0, 6);
! trptr += 6;
!
! write (fd, trbuf, trptr - trbuf);
!
! finish_trace_file (fd);
! }
!
! void
! write_error_trace_file (void)
! {
! int fd;
!
! fd = start_trace_file ("error.tf");
!
! /* The next part of the file consists of newline-separated lines
! defining status, tracepoints, etc. The section is terminated by
! an empty line. */
!
! /* Dump the size of the R (register) blocks in traceframes. */
! snprintf (spbuf, sizeof spbuf, "R %x\n", 500 /* FIXME get from arch */);
! write (fd, spbuf, strlen (spbuf));
!
! /* Dump trace status, in the general form of the qTstatus reply. */
! snprintf (spbuf, sizeof spbuf, "status 0;terror:made-up error:1;tframes:0;tcreated:0;tfree:100;tsize:1000\n");
! write (fd, spbuf, strlen (spbuf));
!
! /* Dump tracepoint definitions, in syntax similar to that used
! for reconnection uploads. */
! /* FIXME need a portable way to print function address in hex */
! snprintf (spbuf, sizeof spbuf, "tp T1:%lx:E:0:0\n",
! (long) &write_basic_trace_file);
! write (fd, spbuf, strlen (spbuf));
! /* (Note that we would only need actions defined if we wanted to
! test tdump.) */
!
! /* Empty line marks the end of the definition section. */
! write (fd, "\n", 1);
!
! trptr = trbuf;

/* Write end of tracebuffer marker. */
! memset (trptr, 0, 6);
! trptr += 6;

write (fd, trbuf, trptr - trbuf);

*************** main (int argc, char **argv, char **envp
*** 109,114 ****
--- 157,164 ----
{
write_basic_trace_file ();

+ write_error_trace_file ();
+
done_making_trace_files ();

return 0;
Index: testsuite/gdb.trace/tfile.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/tfile.exp,v
retrieving revision 1.3
diff -p -r1.3 tfile.exp
*** testsuite/gdb.trace/tfile.exp 24 Mar 2010 21:11:06 -0000 1.3
--- testsuite/gdb.trace/tfile.exp 26 Mar 2010 01:44:00 -0000
*************** if { [gdb_compile "$srcdir/$subdir/$srcf
*** 47,53 ****
gdb_reinitialize_dir $srcdir/$subdir

# Make sure we are starting fresh.
! remote_exec build {sh -xc rm\ -f\ basic.tf}

gdb_load $binfile

--- 47,54 ----
gdb_reinitialize_dir $srcdir/$subdir

# Make sure we are starting fresh.
! remote_file host delete basic.tf
! remote_file host delete error.tf

gdb_load $binfile

*************** Trace buffer has 256 bytes of 4096 bytes
*** 83,89 ****
--- 84,102 ----
Looking at trace frame 0, tracepoint .*" \
"tstatus on trace file"

+ # Now start afresh, using only a trace file.

+ gdb_exit
+ gdb_start

+ gdb_load $binfile

+ gdb_test "target tfile error.tf" "Created tracepoint.*" "target tfile"

+ gdb_test "tstatus" \
+ "Using a trace file.*
+ Trace stopped by an error \\(made-up error, tracepoint 1\\).*
+ Collected 0 trace frame.*
+ Trace buffer has 256 bytes of 4096 bytes free \\(93% full\\).*
+ Not looking at any trace frame.*" \
+ "tstatus on error trace file"