This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/5] Refactor 'tsave'
On 03/01/2013 11:25 AM, Yao Qi wrote:
> On 03/01/2013 06:05 PM, Pedro Alves wrote:
>> And what is the format of that "raw" data? Exactly the format
>
> "raw data" doesn't mean format-less data. The format here is how data are organized in trace *buffer*. Data stored in trace buffer are treated as "raw".
I'm about as familiar with gdb's tracing code as they get, and
I _still_ got confused with reading this part of the patch. I'd
think that'd indicate something wasn't as clear as possible.
>
>> described in the manual, in the "Trace File Format" node:
>>
>> "The trace frame section consists of a number of consecutive frames.
>> Each frame begins with a two-byte tracepoint number, followed by a
>> four-byte size giving the amount of data in the frame. The data in
>> the frame consists of a number of blocks, each introduced by a
>> character indicating its type (at least register, memory, and trace
>> state variable). The data in this section is raw binary, not a
>> hexadecimal or other encoding; its endianness matches the target's
>> endianness."
>
> Yes, this is only about TFILE format, instead of how data are stored in trace buffer, which is GDB internal.
Huh. No. Really. See:
@item qTBuffer:@var{offset},@var{len}
@cindex @samp{qTBuffer} packet
Return up to @var{len} bytes of the current contents of trace buffer,
starting at @var{offset}. The trace buffer is treated as if it were
a contiguous collection of traceframes, as per the trace file format.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It's not any random internal detail at all. It's a defined format.
>>
>> So it's not really "raw" binary data -- there's a format, with
>> headers and all. And it can't be any other format, otherwise
>> the patch's design won't work... (**)
>>
>
> I never say "raw data is binary data or raw data is format-less data".
> My point is raw data has its own format, and GDB is free to store them
> directly by write_raw_data or parse them (according to how data are stored in trace buffers) first and store them in some
> format (CTF and even TFILE).
The patch clearly assumes the data stored in the trace buffers
is as described in the manual, in the "Trace File Format" node,
as I pointed out above. ("The trace frame section ...").
>
>>> >
>>> >TFILE is a format that composed by ascii definition part and trace frames dumped from the raw data directly. There could be another trace file format FOO that stores raw data as well.
>> It's not "raw". It's trace frames in gdb's trace file format
>> as defined in the manual.
>>
>
> It is just because TFILE format use the same format that data are
> stored in trace buffers. One day, we can use another way to organize
> the trace buffers, but still output TFILE format in default. For example, we may
> add checksum for each block in trace buffers in GDBserver in the future,
> but still generate TFILE format trace file in GDB side. When we get raw
> data (with checksum in each block) from the remote target, can we call
> them "tfile format data"?. Another example is GDBserver can send the format
> of trace buffers to GDB first via xml, and GDB can parse the data got
> from GDBserver according the xml description, and save them into CTF or
> TFILE format. We can't call the data from the remote target "tfile format data".
At the point there's more than one format that GDB can fetch from the
target, then the code that fetches it will need to know what format it is
getting. Say, nothing would change in the patch, then ...
LONGEST gotten = 0;
if (writer->ops->write_raw_data != NULL)
{
gotten = target_get_raw_trace_data (buf, offset,
MAX_TRACE_UPLOAD);
^^^^^^^^^^^^^^^^^^^^^^^^
say this sometimes returns XML.
if (gotten == 0)
break;
writer->ops->write_raw_data (writer, buf, gotten);
^^^^^^^^^^^^^^
what format is the writer getting? How can it tell?
}
... this leaves gdb or the writer with no idea what "kind"
of "raw" data it is getting. It may not understand or
want XML at all!
Clearly, more code will need to be added here to handle
the multiple types, and the possibility that the format
we get from the target is not the same as the writer wants.
Maybe we'll have then:
/* See if we can pass down raw trace buffer trace frames
directly, as per FOO's trace file format, if both the target
and the writer support that format. */
if (writer->ops->write_raw_FOO_data != NULL)
{
gotten = target_get_raw_FOO_trace_data (buf, offset,
MAX_TRACE_UPLOAD);
if (gotten == 0)
break;
writer->ops->write_raw_FOO_data (writer, buf, gotten);
}
/* Nope. See if we can pass down raw trace buffer trace frames
directly, as per GDB's trace file format, if both the target
and the writer support that format. */
else if (writer->ops->write_raw_data != NULL)
{
gotten = target_get_raw_trace_data (buf, offset,
MAX_TRACE_UPLOAD);
say this returns XML.
if (gotten == 0)
break;
writer->ops->write_raw_data (writer, buf, gotten);
}
/* Nope. We'll need to do trace buffer format conversion. Try
format foo first, as it is more complete. */
else
{
... if possible, fetch data from the target in foo format,
parse it, and call writer hooks with the pieces ...
...
... else if possible, fetch frame data from the target in gdb
trace file format, parse it, and call writer hooks
with the pieces ...
}
See, the comments should make all this much clearer. That's really
all I'm asking -- to make this bit of code a bit clearer.
>
>>> >
>>>>>> >>> >+ {
>>>>>> >>> >+ uint16_t tp_num;
>>>>>> >>> >+ uint32_t tf_size;
>>>>>> >>> >+ unsigned int read_length;
>>>>>> >>> >+ unsigned int block;
>>>>>> >>> >+
>>>>>> >>> >+ /* Read the first six bytes in, which is the tracepoint
>>>>>> >>> >+ number and trace frame size. */
>>>>>> >>> >+ gotten = target_get_raw_trace_data (buf, offset, 6);
>>>>>> >>> >+
>>>>>> >>> >+ if (gotten == 0)
>>>>>> >>> >+ break;
>>>>>> >>> >+ tp_num = (uint16_t)
>>>>>> >>> >+ extract_unsigned_integer (&buf[0], 2, byte_order);
>>>>>> >>> >+
>>>>>> >>> >+ tf_size = (uint32_t)
>>>>>> >>> >+ extract_unsigned_integer (&buf[2], 4, byte_order);
>> (**) ... as can be seen here and below with
>>
>> + switch (block_type)
>> + {
>> + case 'R':
>> ...
>> + case 'M':
>> ...
>>
>> etc.
>>
>> This is parsing the "raw"'s headers, in gdb's trace file format.
>> This as the else branch. The "then" branch only works if the target
>> can interpret "buf" as trace frames in gdb's trace file format.
>>
> Again, it is not trace file format.
Again, it is.
> It is how data are stored in trace buffers.
It is, because the the target passes the trace buffers in
trace file format back to gdb. gdbserver uses the same format
natively, for its own buffers, as naturally this avoid any
conversion.
(actually, historically, the tfile format was designed as
reusing gdbserver's, but anyway, same difference).
--
Pedro Alves