Message ID | 24262.1395849610@usendtaylorx2l |
---|---|
State | Superseded |
Headers |
Return-Path: <x14314964@homiemail-mx21.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx21.g.dreamhost.com (caibbdcaaahb.dreamhost.com [208.113.200.71]) by wilcox.dreamhost.com (Postfix) with ESMTP id 2682F3601AC for <siddhesh@wilcox.dreamhost.com>; Wed, 26 Mar 2014 09:00:29 -0700 (PDT) Received: by homiemail-mx21.g.dreamhost.com (Postfix, from userid 14314964) id C4A2013B691F; Wed, 26 Mar 2014 09:00:28 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx21.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx21.g.dreamhost.com (Postfix) with ESMTPS id 943281330892 for <gdb@patchwork.siddhesh.in>; Wed, 26 Mar 2014 09:00:28 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; q=dns; s= default; b=nWWlcRVPGflMSbfdw/G6/d7lJKHtmQQI4GIP6wlZQOANRaLGu/IYB 5lFxMLtLmiH8Sj5uEghDPHW4YzYQQl3PCaikvfnwXNCryBKjA+XER2dbJ5++0y10 uOwS1FLZNtjfOFu/jd8c+G9Mhq/RRY2aYD8fOP1r7H17A+Tq6ZNBgQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; s=default; bh=RPIB1LJb72zxC3krs9KTaE1kM6w=; b=tl3yyhq/CcIjmuES+jhjswi/KlM+ mWKxhKYk3gu6DLt1gXsc/QUOT/oa7bBq6f8Oze+2Rk9bBm0XZ2Aj1JV9y+di3cDT nD0JW1shAB6+aSAki2qw1z4a/39YE2mw8L6HSNSZJzCsl8BRNTMursydYqvhwpjR ZZITLtMwlRTqfVo= Received: (qmail 2143 invoked by alias); 26 Mar 2014 16:00:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-gdb=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 2125 invoked by uid 89); 26 Mar 2014 16:00:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mexforward.lss.emc.com Received: from mailuogwhop.emc.com (HELO mexforward.lss.emc.com) (168.159.213.141) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 26 Mar 2014 16:00:23 +0000 Received: from hop04-l1d11-si01.isus.emc.com (hop04-l1d11-si01.isus.emc.com [10.254.111.54]) by mexforward.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id s2QG0KXg015216 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for <gdb-patches@sourceware.org>; Wed, 26 Mar 2014 12:00:20 -0400 Received: from mailhub.lss.emc.com (mailhubhoprd05.lss.emc.com [10.254.222.129]) by hop04-l1d11-si01.isus.emc.com (RSA Interceptor) for <gdb-patches@sourceware.org>; Wed, 26 Mar 2014 12:00:11 -0400 Received: from usendtaylorx2l.lss.emc.com (usendtaylorx2l.lss.emc.com [10.243.10.188]) by mailhub.lss.emc.com (Switch-3.4.3/Switch-3.4.3) with ESMTP id s2QG0Bgt017166 for <gdb-patches@sourceware.org>; Wed, 26 Mar 2014 12:00:11 -0400 Received: by usendtaylorx2l.lss.emc.com (Postfix, from userid 26043) id D748F5C7D55; Wed, 26 Mar 2014 12:00:10 -0400 (EDT) Received: from usendtaylorx2l (localhost [127.0.0.1]) by usendtaylorx2l.lss.emc.com (Postfix) with ESMTP id D56535C6D07 for <gdb-patches@sourceware.org>; Wed, 26 Mar 2014 12:00:10 -0400 (EDT) From: David Taylor <dtaylor@emc.com> To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> Subject: RFA/remote: compare-sections Date: Wed, 26 Mar 2014 12:00:10 -0400 Message-ID: <24262.1395849610@usendtaylorx2l> X-EMM-MHVC: 1 X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in |
Commit Message
David Taylor
March 26, 2014, 4 p.m. UTC
Motivation: When connecting to a remote system, we use the compare-sections command to verify that the box is running the code that we think it is running. Since the system is up and running and *NOT* 'freshly downloaded without yet executing anything', read-write sections, of course, differ from what they were in the executable file. Comparing read-write sections takes time and more importantly the MIS-MATCHED output is confusing to some users. The compare-sections command compares all loadable sections including read-write sections. This patch gives the user the option to compare just the loadable read-only sections. For gdb/ChangeLog: 2014-03-26 David Taylor <dtaylor@emc.com> * remote.c (compare_sections_command): Add -r option to compare all loadable read-only sections. For gdb/doc/ChangeLog: 2014-03-26 David Taylor <dtaylor@emc.com> * gdb.texinfo (compare-sections): Document the new -r (read-only) option. I'm not sure that this patch is big enough to require a copyright assignment, but regardless, EMC has a copyright assignment on file for GDB (and GCC and BINUTILS as well). If it is approved, I will need someone else to commit it as I don't have write access. Patch:
Comments
> From: David Taylor <dtaylor@emc.com> > Date: Wed, 26 Mar 2014 12:00:10 -0400 > > For gdb/ChangeLog: > > 2014-03-26 David Taylor <dtaylor@emc.com> > > * remote.c (compare_sections_command): Add -r option to compare > all loadable read-only sections. > > For gdb/doc/ChangeLog: > > 2014-03-26 David Taylor <dtaylor@emc.com> > > * gdb.texinfo (compare-sections): Document the new -r (read-only) > option. OK for the documentation part, but I think this warrants a NEWS entry. Thanks.
ping. David Taylor <dtaylor@emc.com> wrote: > Motivation: > > When connecting to a remote system, we use the compare-sections command > to verify that the box is running the code that we think it is running. > Since the system is up and running and *NOT* 'freshly downloaded without > yet executing anything', read-write sections, of course, differ from > what they were in the executable file. > > Comparing read-write sections takes time and more importantly the > MIS-MATCHED output is confusing to some users. > > The compare-sections command compares all loadable sections including > read-write sections. This patch gives the user the option to compare > just the loadable read-only sections. > > For gdb/ChangeLog: > > 2014-03-26 David Taylor <dtaylor@emc.com> > > * remote.c (compare_sections_command): Add -r option to compare > all loadable read-only sections. > > For gdb/doc/ChangeLog: > > 2014-03-26 David Taylor <dtaylor@emc.com> > > * gdb.texinfo (compare-sections): Document the new -r (read-only) > option. > > I'm not sure that this patch is big enough to require a copyright > assignment, but regardless, EMC has a copyright assignment on file for > GDB (and GCC and BINUTILS as well). > > If it is approved, I will need someone else to commit it as I don't have > write access. > > Patch: > > Index: gdb/remote.c > =================================================================== > RCS file: /home/cvsroot/GDB/gdb/remote.c,v > retrieving revision 1.8 > diff -u -r1.8 remote.c > --- gdb/remote.c 26 Mar 2014 14:12:34 -0000 1.8 > +++ gdb/remote.c 26 Mar 2014 15:49:40 -0000 > @@ -8664,6 +8664,7 @@ > int matched = 0; > int mismatched = 0; > int res; > + int read_only = 0; > > if (!exec_bfd) > error (_("command cannot be used without an exec file")); > @@ -8671,11 +8672,20 @@ > /* Make sure the remote is pointing at the right process. */ > set_general_process (); > > + if (args && (strcmp (args, "-r") == 0)) > + { > + read_only = 1; > + args = NULL; > + } > + > for (s = exec_bfd->sections; s; s = s->next) > { > if (!(s->flags & SEC_LOAD)) > continue; /* Skip non-loadable section. */ > > + if (read_only && ((s->flags & SEC_READONLY) == 0)) > + continue; /* Skip writeable sections */ > + > size = bfd_get_section_size (s); > if (size == 0) > continue; /* Skip zero-length section. */ > @@ -12046,7 +12056,8 @@ > > add_cmd ("compare-sections", class_obscure, compare_sections_command, _("\ > Compare section data on target to the exec file.\n\ > -Argument is a single section name (default: all loaded sections)."), > +Argument is a single section name (default: all loaded sections).\n\ > +To compare only read-only loaded sections, specify the -r option."), > &cmdlist); > > add_cmd ("packet", class_maintenance, packet_command, _("\ > Index: gdb/doc/gdb.texinfo > =================================================================== > RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v > retrieving revision 1.1.1.2 > diff -u -r1.1.1.2 gdb.texinfo > --- gdb/doc/gdb.texinfo 18 Feb 2014 15:36:03 -0000 1.1.1.2 > +++ gdb/doc/gdb.texinfo 26 Mar 2014 15:49:40 -0000 > @@ -8760,11 +8760,12 @@ > > @table @code > @kindex compare-sections > -@item compare-sections @r{[}@var{section-name}@r{]} > +@item compare-sections @r{[}@var{section-name}@r{|}@code{-r}@r{]} > Compare the data of a loadable section @var{section-name} in the > executable file of the program being debugged with the same section in > the remote machine's memory, and report any mismatches. With no > -arguments, compares all loadable sections. This command's > +arguments, compares all loadable sections. With an argument of > +@code{-r}, compares all loadable read-only sections. This command's > availability depends on the target's support for the @code{"qCRC"} > remote request. > @end table >
> Date: Thu, 03 Apr 2014 16:38:49 -0400 > From: David Taylor <dtaylor@emc.com> > > ping. I already approved the documentation part.
Eli Zaretskii <eliz@gnu.org> wrote: > > Date: Thu, 03 Apr 2014 16:38:49 -0400 > > From: David Taylor <dtaylor@emc.com> > > > > ping. > > I already approved the documentation part. Yes, thank you. I have never heard from anyone about the non-documentation parts.
Final ping. I initially posted this three weeks ago. Eli Zaretskii immediately approved the doc part. The remainder has been ignored. I pinged two weeks ago. The remainder continues to be unreviewed. We (at EMC) find it useful. I will not ping further on it. I hope it goes in eventually, but have resigned myself to having to maintain it as a local change. Sigh. EMC has a copyright assignment on file with the FSF (plus this is small enought that I'm not sure it is copyrightable), so anyone who wishes to do so should feel free to incorporate it into their local versions of GDB. David Taylor <dtaylor@emc.com> wrote: > ping. > > David Taylor <dtaylor@emc.com> wrote: > > > Motivation: > > > > When connecting to a remote system, we use the compare-sections command > > to verify that the box is running the code that we think it is running. > > Since the system is up and running and *NOT* 'freshly downloaded without > > yet executing anything', read-write sections, of course, differ from > > what they were in the executable file. > > > > Comparing read-write sections takes time and more importantly the > > MIS-MATCHED output is confusing to some users. > > > > The compare-sections command compares all loadable sections including > > read-write sections. This patch gives the user the option to compare > > just the loadable read-only sections. > > > > For gdb/ChangeLog: > > > > 2014-03-26 David Taylor <dtaylor@emc.com> > > > > * remote.c (compare_sections_command): Add -r option to compare > > all loadable read-only sections. > > > > For gdb/doc/ChangeLog: > > > > 2014-03-26 David Taylor <dtaylor@emc.com> > > > > * gdb.texinfo (compare-sections): Document the new -r (read-only) > > option. > > > > I'm not sure that this patch is big enough to require a copyright > > assignment, but regardless, EMC has a copyright assignment on file for > > GDB (and GCC and BINUTILS as well). > > > > If it is approved, I will need someone else to commit it as I don't have > > write access. > > > > Patch: > > > > Index: gdb/remote.c > > =================================================================== > > RCS file: /home/cvsroot/GDB/gdb/remote.c,v > > retrieving revision 1.8 > > diff -u -r1.8 remote.c > > --- gdb/remote.c 26 Mar 2014 14:12:34 -0000 1.8 > > +++ gdb/remote.c 26 Mar 2014 15:49:40 -0000 > > @@ -8664,6 +8664,7 @@ > > int matched = 0; > > int mismatched = 0; > > int res; > > + int read_only = 0; > > > > if (!exec_bfd) > > error (_("command cannot be used without an exec file")); > > @@ -8671,11 +8672,20 @@ > > /* Make sure the remote is pointing at the right process. */ > > set_general_process (); > > > > + if (args && (strcmp (args, "-r") == 0)) > > + { > > + read_only = 1; > > + args = NULL; > > + } > > + > > for (s = exec_bfd->sections; s; s = s->next) > > { > > if (!(s->flags & SEC_LOAD)) > > continue; /* Skip non-loadable section. */ > > > > + if (read_only && ((s->flags & SEC_READONLY) == 0)) > > + continue; /* Skip writeable sections */ > > + > > size = bfd_get_section_size (s); > > if (size == 0) > > continue; /* Skip zero-length section. */ > > @@ -12046,7 +12056,8 @@ > > > > add_cmd ("compare-sections", class_obscure, compare_sections_command, _("\ > > Compare section data on target to the exec file.\n\ > > -Argument is a single section name (default: all loaded sections)."), > > +Argument is a single section name (default: all loaded sections).\n\ > > +To compare only read-only loaded sections, specify the -r option."), > > &cmdlist); > > > > add_cmd ("packet", class_maintenance, packet_command, _("\ > > Index: gdb/doc/gdb.texinfo > > =================================================================== > > RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v > > retrieving revision 1.1.1.2 > > diff -u -r1.1.1.2 gdb.texinfo > > --- gdb/doc/gdb.texinfo 18 Feb 2014 15:36:03 -0000 1.1.1.2 > > +++ gdb/doc/gdb.texinfo 26 Mar 2014 15:49:40 -0000 > > @@ -8760,11 +8760,12 @@ > > > > @table @code > > @kindex compare-sections > > -@item compare-sections @r{[}@var{section-name}@r{]} > > +@item compare-sections @r{[}@var{section-name}@r{|}@code{-r}@r{]} > > Compare the data of a loadable section @var{section-name} in the > > executable file of the program being debugged with the same section in > > the remote machine's memory, and report any mismatches. With no > > -arguments, compares all loadable sections. This command's > > +arguments, compares all loadable sections. With an argument of > > +@code{-r}, compares all loadable read-only sections. This command's > > availability depends on the target's support for the @code{"qCRC"} > > remote request. > > @end table > > >
On 04/16/2014 09:43 AM, David Taylor wrote: > Final ping. Hi, David, I am really sorry that patch reviewing is going so slowly right now. It appears the few active maintainers we have are (extremely) busy with "work." As it is, I think maintainers are probably several /months/ behind on patch reviews. There are neither enough people to review patches nor enough maintainers to approve patches after random folk (like me) make recommendations. So please don't feel that you've been singled out. It is a growing pain that gdb is experiencing right now. >>> Motivation: >>> >>> When connecting to a remote system, we use the compare-sections command >>> to verify that the box is running the code that we think it is running. >>> Since the system is up and running and *NOT* 'freshly downloaded without >>> yet executing anything', read-write sections, of course, differ from >>> what they were in the executable file. >>> >>> Comparing read-write sections takes time and more importantly the >>> MIS-MATCHED output is confusing to some users. >>> >>> The compare-sections command compares all loadable sections including >>> read-write sections. This patch gives the user the option to compare >>> just the loadable read-only sections. This is excellent. Thank you for providing this information. I found it very useful. >>> Index: gdb/remote.c >>> =================================================================== >>> RCS file: /home/cvsroot/GDB/gdb/remote.c,v >>> retrieving revision 1.8 >>> diff -u -r1.8 remote.c >>> --- gdb/remote.c 26 Mar 2014 14:12:34 -0000 1.8 >>> +++ gdb/remote.c 26 Mar 2014 15:49:40 -0000 >>> @@ -8664,6 +8664,7 @@ >>> int matched = 0; >>> int mismatched = 0; >>> int res; >>> + int read_only = 0; >>> >>> if (!exec_bfd) >>> error (_("command cannot be used without an exec file")); >>> @@ -8671,11 +8672,20 @@ >>> /* Make sure the remote is pointing at the right process. */ >>> set_general_process (); >>> >>> + if (args && (strcmp (args, "-r") == 0)) >>> + { >>> + read_only = 1; >>> + args = NULL; >>> + } Two (and one-half) things: 1) It was recently agreed that we would now explicitly check pointers against NULL. 2) I think it better/more consistent to use check_for_argument (in cli/cli-utils.[ch]) for this: if (args != NULL && check_for_argument (&args, "-r", sizeof ("-r") - 1)) read_only = 1; 2.5) I'm a testing freak, so I'd really like to see is a test case. Alas, I see that there are no tests /at all/ for compare-sections, so that may be heaping more hardship onto this than is either necessary or plausible. I am not going to ask you to provide one because of this, but a global maintainer might. In any case, with the two changes above, I recommend that this patch be approved by a maintainer. Keith >>> + >>> for (s = exec_bfd->sections; s; s = s->next) >>> { >>> if (!(s->flags & SEC_LOAD)) >>> continue; /* Skip non-loadable section. */ >>> >>> + if (read_only && ((s->flags & SEC_READONLY) == 0)) >>> + continue; /* Skip writeable sections */ >>> + >>> size = bfd_get_section_size (s); >>> if (size == 0) >>> continue; /* Skip zero-length section. */ >>> @@ -12046,7 +12056,8 @@ >>> >>> add_cmd ("compare-sections", class_obscure, compare_sections_command, _("\ >>> Compare section data on target to the exec file.\n\ >>> -Argument is a single section name (default: all loaded sections)."), >>> +Argument is a single section name (default: all loaded sections).\n\ >>> +To compare only read-only loaded sections, specify the -r option."), >>> &cmdlist); >>> >>> add_cmd ("packet", class_maintenance, packet_command, _("\ >>> Index: gdb/doc/gdb.texinfo >>> =================================================================== >>> RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v >>> retrieving revision 1.1.1.2 >>> diff -u -r1.1.1.2 gdb.texinfo >>> --- gdb/doc/gdb.texinfo 18 Feb 2014 15:36:03 -0000 1.1.1.2 >>> +++ gdb/doc/gdb.texinfo 26 Mar 2014 15:49:40 -0000 >>> @@ -8760,11 +8760,12 @@ >>> >>> @table @code >>> @kindex compare-sections >>> -@item compare-sections @r{[}@var{section-name}@r{]} >>> +@item compare-sections @r{[}@var{section-name}@r{|}@code{-r}@r{]} >>> Compare the data of a loadable section @var{section-name} in the >>> executable file of the program being debugged with the same section in >>> the remote machine's memory, and report any mismatches. With no >>> -arguments, compares all loadable sections. This command's >>> +arguments, compares all loadable sections. With an argument of >>> +@code{-r}, compares all loadable read-only sections. This command's >>> availability depends on the target's support for the @code{"qCRC"} >>> remote request. >>> @end table >>> >>
Index: gdb/remote.c =================================================================== RCS file: /home/cvsroot/GDB/gdb/remote.c,v retrieving revision 1.8 diff -u -r1.8 remote.c --- gdb/remote.c 26 Mar 2014 14:12:34 -0000 1.8 +++ gdb/remote.c 26 Mar 2014 15:49:40 -0000 @@ -8664,6 +8664,7 @@ int matched = 0; int mismatched = 0; int res; + int read_only = 0; if (!exec_bfd) error (_("command cannot be used without an exec file")); @@ -8671,11 +8672,20 @@ /* Make sure the remote is pointing at the right process. */ set_general_process (); + if (args && (strcmp (args, "-r") == 0)) + { + read_only = 1; + args = NULL; + } + for (s = exec_bfd->sections; s; s = s->next) { if (!(s->flags & SEC_LOAD)) continue; /* Skip non-loadable section. */ + if (read_only && ((s->flags & SEC_READONLY) == 0)) + continue; /* Skip writeable sections */ + size = bfd_get_section_size (s); if (size == 0) continue; /* Skip zero-length section. */ @@ -12046,7 +12056,8 @@ add_cmd ("compare-sections", class_obscure, compare_sections_command, _("\ Compare section data on target to the exec file.\n\ -Argument is a single section name (default: all loaded sections)."), +Argument is a single section name (default: all loaded sections).\n\ +To compare only read-only loaded sections, specify the -r option."), &cmdlist); add_cmd ("packet", class_maintenance, packet_command, _("\ Index: gdb/doc/gdb.texinfo =================================================================== RCS file: /home/cvsroot/GDB/gdb/doc/gdb.texinfo,v retrieving revision 1.1.1.2 diff -u -r1.1.1.2 gdb.texinfo --- gdb/doc/gdb.texinfo 18 Feb 2014 15:36:03 -0000 1.1.1.2 +++ gdb/doc/gdb.texinfo 26 Mar 2014 15:49:40 -0000 @@ -8760,11 +8760,12 @@ @table @code @kindex compare-sections -@item compare-sections @r{[}@var{section-name}@r{]} +@item compare-sections @r{[}@var{section-name}@r{|}@code{-r}@r{]} Compare the data of a loadable section @var{section-name} in the executable file of the program being debugged with the same section in the remote machine's memory, and report any mismatches. With no -arguments, compares all loadable sections. This command's +arguments, compares all loadable sections. With an argument of +@code{-r}, compares all loadable read-only sections. This command's availability depends on the target's support for the @code{"qCRC"} remote request. @end table