From patchwork Thu Aug 21 04:53:58 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 2479 Received: (qmail 27993 invoked by alias); 21 Aug 2014 04:57:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27980 invoked by uid 89); 21 Aug 2014 04:57:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 21 Aug 2014 04:57:54 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1XKKRa-00056E-8Y from Yao_Qi@mentor.com ; Wed, 20 Aug 2014 21:57:50 -0700 Received: from SVR-ORW-FEM-06.mgc.mentorg.com ([147.34.97.120]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 20 Aug 2014 21:57:50 -0700 Received: from qiyao.dyndns.org (147.34.91.1) by SVR-ORW-FEM-06.mgc.mentorg.com (147.34.97.120) with Microsoft SMTP Server id 14.2.247.3; Wed, 20 Aug 2014 21:57:48 -0700 Message-ID: <53F57B66.1030602@codesourcery.com> Date: Thu, 21 Aug 2014 12:53:58 +0800 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Pedro Alves , Jan Kratochvil CC: "gdb-patches@sourceware.org" Subject: Re: --with-babeltrace generates many FAILs References: <20140816204614.GA7000@host2.jankratochvil.net> <53F3457E.5030205@codesourcery.com> <20140819140755.GA30208@host2.jankratochvil.net> <53F41DE5.1010406@codesourcery.com> <53F46D48.2060200@redhat.com> <53F487AC.7070606@codesourcery.com> <53F4A907.9080504@redhat.com> In-Reply-To: <53F4A907.9080504@redhat.com> X-IsSubscribed: yes 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. diff --git a/gdb/ctf.c b/gdb/ctf.c index df645c0..9fd8c04 100644 --- a/gdb/ctf.c +++ b/gdb/ctf.c @@ -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