--with-babeltrace generates many FAILs
Commit Message
On 08/20/2014 09:56 PM, Pedro Alves wrote:
> Note that there's a fundamental issue with the workaround -- it
> assumes that the gdb that generates CTF is the same that consumes it.
> It's certainly easy to imagine a CTF file generated by a gdb not
> affected by the bug be consumed by a gdb with the bug. Or the other way
> around.
No, the workaround doesn't have such assumption. The trace file
generated by GDB with 1.1.0 has a faked packet, and the trace file can
be read by GDB with 1.1.1. Although the faked packet is not necessary,
it doesn't make any error.
Since 1.1.2, babeltrace becomes more restrict on CTF, and starts to
complain the faked packet GDB adds. The code in GDB was written when
1.1.0 was released, at that moment, nobody knows how 1.1.2 does.
Likewise, we don't know how does babeltrace behave in 2015. We
followed the CTF spec to generate CTF data, and ideally babeltrace of
any version can parse them. However, it is not surprise to me that
two implementations (producer and consumer) have different
understandings on some parts of the specification.
>
>>> >> IOW, why do we still need to support 1.1.0?
>> >
>> > No special reason, 1.1.0 was just used when I did the CTF work in GDB,
>> > and was used on my laptop since then. IIRC, 1.1.0 was released in 2013
>> > March, so it isn't very old and it might be used somewhere.
>> > Shouldn't we be conservative in this case?
> My point is exactly that this is new-ish code, and a moving target.
> If bugs are fixed promptly, why go through trouble working around
> them in gdb instead of just updating the version in the distro?
I prefer the latter, but I am not a distro integrator.
> It'd be different if we were talking about a one liner instead of
> a good chunk of autoconf/pkg-config glue.
>
> I'm not strictly objecting your patch, but it does look like
> the kind of code that: #1 will need further tweaking in the future;
> #2 will become obsolete anyway as time passes anyway. Couple that
> with the generator != consumer issue, and it raises eyebrows.
>
Yeah, the patch isn't perfect. I am fine with your suggestion, and the
patch below removes the workaround.
>> > In general, GDB and GDBserver uses a set of libraries, what are the
>> > criteria of
>> >
>> > 1. stop supporting a version of a library, such as libbabeltrace 1.1.0
> When the burden of supporting it outweighs the benefits?
>
That is still too abstract to operate, IMO.
>> > 2. stop supporting or using a library, such as the UST stuff in GDBserver,
> When nobody wants to maintain it anymore (I personally don't)?
OK, GDBserver UST support depends on a quite old ust library, I'll
take a look further.
Comments
On 08/21/2014 05:53 AM, Yao Qi wrote:
> On 08/20/2014 09:56 PM, Pedro Alves wrote:
>> > Note that there's a fundamental issue with the workaround -- it
>> > assumes that the gdb that generates CTF is the same that consumes it.
>> > It's certainly easy to imagine a CTF file generated by a gdb not
>> > affected by the bug be consumed by a gdb with the bug. Or the other way
>> > around.
> No, the workaround doesn't have such assumption. The trace file
> generated by GDB with 1.1.0 has a faked packet, and the trace file can
> be read by GDB with 1.1.1. Although the faked packet is not necessary,
> it doesn't make any error.
>
> Since 1.1.2, babeltrace becomes more restrict on CTF, and starts to
> complain the faked packet GDB adds. The code in GDB was written when
> 1.1.0 was released, at that moment, nobody knows how 1.1.2 does.
> Likewise, we don't know how does babeltrace behave in 2015. We
> followed the CTF spec to generate CTF data, and ideally babeltrace of
> any version can parse them. However, it is not surprise to me that
> two implementations (producer and consumer) have different
> understandings on some parts of the specification.
Right. From your description, newer babeltrace rejects the workaround
because it's more strict now, which I read as the workaround
somehow knowingly violating the CTF spec, hence, the workaround
assuming babeltrace or other CTF consumers would remain lax. The
point was that CTF generated by a GDB with 1.1.0 wouldn't be readable
by a GDB with 1.1.2, and vice-versa. If it remains in place, the
assumption that we can infer whether a workaround will be necessary
or usable from the babeltrace version that is used to build the
producer GDB isn't strictly correct. But maybe "assumption" was a
too strong word. (I hope you didn't feel I was pointing fingers at
you or something. Certainly not intended!)
>> > I'm not strictly objecting your patch, but it does look like
>> > the kind of code that: #1 will need further tweaking in the future;
>> > #2 will become obsolete anyway as time passes anyway. Couple that
>> > with the generator != consumer issue, and it raises eyebrows.
>> >
> Yeah, the patch isn't perfect. I am fine with your suggestion, and the
> patch below removes the workaround.
Alright, onward! Thanks.
>>>> >> > In general, GDB and GDBserver uses a set of libraries, what are the
>>>> >> > criteria of
>>>> >> >
>>>> >> > 1. stop supporting a version of a library, such as libbabeltrace 1.1.0
>> > When the burden of supporting it outweighs the benefits?
>> >
> That is still too abstract to operate, IMO.
Kind of just as abstract as the question. :-)
The "benefit" is being able to take advantage of the library's features.
The "burden" is the cost/effort required to use the library and workaround
any issues it might have. When there are bugfix releases that postdate
a given version, I think the effort to support the older unfixed version
isn't so much worth it. The bugs have been fixed at the source, and we
can simply require builders/integrators upgrade to the latest bugfix
release (that's what bugfix/stable releases are for!). Thus, IMO, the
existence of bugfix releases makes the burden of maintaining support
for old unfixed versions outweigh the benefits, as we can alternatively
just say "not our problem: build against the latest fixed version
instead, please".
> Subject: [PATCH] Remove workaround to libbabeltrace 1.1.0 issue
This looks good to me.
Thanks,
Pedro Alves
Pedro Alves <palves@redhat.com> writes:
> Right. From your description, newer babeltrace rejects the workaround
> because it's more strict now, which I read as the workaround
> somehow knowingly violating the CTF spec, hence, the workaround
> assuming babeltrace or other CTF consumers would remain lax. The
> point was that CTF generated by a GDB with 1.1.0 wouldn't be readable
> by a GDB with 1.1.2, and vice-versa. If it remains in place, the
> assumption that we can infer whether a workaround will be necessary
> or usable from the babeltrace version that is used to build the
> producer GDB isn't strictly correct. But maybe "assumption" was a
> too strong word. (I hope you didn't feel I was pointing fingers at
> you or something. Certainly not intended!)
No, I didn't feel that :) I'd like people know the status and problem
clearly and make people on the same page.
>>>>> >> > In general, GDB and GDBserver uses a set of libraries, what are the
>>>>> >> > criteria of
>>>>> >> >
>>>>> >> > 1. stop supporting a version of a library, such as libbabeltrace 1.1.0
>>> > When the burden of supporting it outweighs the benefits?
>>> >
>> That is still too abstract to operate, IMO.
>
> Kind of just as abstract as the question. :-)
>
> The "benefit" is being able to take advantage of the library's features.
> The "burden" is the cost/effort required to use the library and workaround
> any issues it might have. When there are bugfix releases that postdate
> a given version, I think the effort to support the older unfixed version
> isn't so much worth it. The bugs have been fixed at the source, and we
> can simply require builders/integrators upgrade to the latest bugfix
> release (that's what bugfix/stable releases are for!). Thus, IMO, the
> existence of bugfix releases makes the burden of maintaining support
> for old unfixed versions outweigh the benefits, as we can alternatively
> just say "not our problem: build against the latest fixed version
> instead, please".
This is clear to me.
>
>> Subject: [PATCH] Remove workaround to libbabeltrace 1.1.0 issue
>
> This looks good to me.
>
Patch is pushed in. Thanks.
On Fri, 22 Aug 2014 05:48:30 +0200, Yao Qi wrote:
> Patch is pushed in. Thanks.
Maybe could you push it also for 7.8.1?
https://sourceware.org/gdb/wiki/GDB_7.8_Release
At least I have pushed there first the CFLAGS=-Wall fix which disclosed this
bug a bit more.
Jan
On 08/27/2014 04:27 AM, Jan Kratochvil wrote:
> Maybe could you push it also for 7.8.1?
> https://sourceware.org/gdb/wiki/GDB_7.8_Release
Yeah, that is reasonable to me. Patch is pushed in to 7.8 branch. The
wikipage is updated too.
@@ -623,11 +623,6 @@ ctf_write_definition_end (struct trace_file_writer *self)
self->ops->frame_ops->end (self);
}
-/* The minimal file size of data stream. It is required by
- babeltrace. */
-
-#define CTF_FILE_MIN_SIZE 4096
-
/* This is the implementation of trace_file_write_ops method
end. */
@@ -637,50 +632,6 @@ ctf_end (struct trace_file_writer *self)
struct ctf_trace_file_writer *writer = (struct ctf_trace_file_writer *) self;
gdb_assert (writer->tcs.content_size == 0);
- /* The babeltrace requires or assumes that the size of datastream
- file is greater than 4096 bytes. If we don't generate enough
- packets and events, create a fake packet which has zero event,
- to use up the space. */
- if (writer->tcs.packet_start < CTF_FILE_MIN_SIZE)
- {
- uint32_t u32;
-
- /* magic. */
- u32 = CTF_MAGIC;
- ctf_save_write_uint32 (&writer->tcs, u32);
-
- /* content_size. */
- u32 = 0;
- ctf_save_write_uint32 (&writer->tcs, u32);
-
- /* packet_size. */
- u32 = 12;
- if (writer->tcs.packet_start + u32 < CTF_FILE_MIN_SIZE)
- u32 = CTF_FILE_MIN_SIZE - writer->tcs.packet_start;
-
- u32 *= TARGET_CHAR_BIT;
- ctf_save_write_uint32 (&writer->tcs, u32);
-
- /* tpnum. */
- u32 = 0;
- ctf_save_write (&writer->tcs, (gdb_byte *) &u32, 2);
-
- /* Enlarge the file to CTF_FILE_MIN_SIZE is it is still less
- than that. */
- if (CTF_FILE_MIN_SIZE
- > (writer->tcs.packet_start + writer->tcs.content_size))
- {
- gdb_byte b = 0;
-
- /* Fake the content size to avoid assertion failure in
- ctf_save_fseek. */
- writer->tcs.content_size = (CTF_FILE_MIN_SIZE
- - 1 - writer->tcs.packet_start);
- ctf_save_fseek (&writer->tcs, CTF_FILE_MIN_SIZE - 1,
- SEEK_SET);
- ctf_save_write (&writer->tcs, &b, 1);
- }
- }
}
/* This is the implementation of trace_frame_write_ops method