From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10488 invoked by alias); 1 Sep 2010 16:07:30 -0000 Received: (qmail 10461 invoked by uid 22791); 1 Sep 2010 16:07:27 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 01 Sep 2010 16:07:22 +0000 Received: (qmail 20717 invoked from network); 1 Sep 2010 16:07:20 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 1 Sep 2010 16:07:20 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFA] Fix maint translate command Date: Wed, 01 Sep 2010 16:07:00 -0000 User-Agent: KMail/1.13.2 (Linux/2.6.33-29-realtime; KDE/4.4.2; x86_64; ; ) Cc: "Pierre Muller" References: <002101cb49b3$50d60cd0$f2822670$@muller@ics-cnrs.unistra.fr> <000001cb49dc$81074080$8315c180$@muller@ics-cnrs.unistra.fr> <201009011625.27952.pedro@codesourcery.com> In-Reply-To: <201009011625.27952.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009011707.14802.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-09/txt/msg00013.txt.bz2 On Wednesday 01 September 2010 16:25:27, Pedro Alves wrote: > [please don't top-post] > > On Wednesday 01 September 2010 14:49:30, Pierre Muller wrote: > > > On Wednesday 01 September 2010 09:54:40, Pierre Muller wrote: > > > > ALL_OBJSECTIONS is a double for loop, > > > > thus the 'break' statement was only exiting the first loop, > > > > but the second loop was still continuing, leading to > > > > a false error. > > > > > > > > > > It all sounds like we should fix ALL_OBJSECTIONS then. > > > Honestly, this might be a good idea, > > but I have no idea how to do this :( > > With Magic. It would be real simple if the inner loop's end condition > was a check againt NULL, rather than "(osect) < (objfile)->sections_end" > --- you'd just add an osect == NULL check to the outer loop's conditional. > > Something like this is still simple, but clears "objfile" when you break > out of the inner loop: > > #define ALL_OBJSECTIONS(objfile, osect) \ > for ((objfile) = current_program_space->objfiles; \ > (objfile) != NULL; \ > (objfile) = (osect) < (objfile)->sections_end ? NULL : (objfile)->next) \ > for ((osect) = (objfile)->sections; (osect) < (objfile)->sections_end; osect++) > > > It happens that "maintenance translate ..." expects that OBJFILE is preserved > when you "break" within ALL_OBJSECTIONS, which, is probably what most would > expect if they didn't know the innards to the macro, so, we go a step further > to preserve that OBJFILE pointer: > Sorry, I left it half baked. Here's the correct version (I forgot to update "osect" when advancing the objfile in the outer loop). I think it's correct now: #define ALL_OBJSECTIONS(objfile, osect) \ for ((objfile) = current_program_space->objfiles, \ (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0; \ (objfile) != NULL \ && (osect) == (objfile)->sections_end; \ ((osect) == (objfile)->sections_end \ ? ((objfile) = (objfile)->next, \ (objfile) != NULL ? (osect) = (objfile)->sections_end : 0) \ : 0)) \ for ((osect) = (objfile)->sections; \ (osect) < (objfile)->sections_end; \ (osect)++) > I admit it loop ugly, and that may look complicated, but it isn't > (complicated). The outer loop learns about the inner loop's end > condition, and stops iterating if it detects the inner loop didn't > reach it's end. The trick to not clearing "objfile" is to only > advance it in the outer loop, if the inner loop reached it's end. > > It fixes "maint translate .text 0xXXX" for me and has no regressions on > x86_64-unknown-linux-gnu. > > There's a: > > goto keep; /* break out of two nested for loops */ > > in gcore.c that could then be replaced with a "break" ... -- Pedro Alves 2010-09-01 Pedro Alves * objfiles.h (ALL_OBJSECTIONS): Handle breaks in the inner loop. --- gdb/objfiles.h | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) Index: src/gdb/objfiles.h =================================================================== --- src.orig/gdb/objfiles.h 2010-09-01 14:35:08.000000000 +0100 +++ src/gdb/objfiles.h 2010-09-01 16:58:36.000000000 +0100 @@ -611,9 +611,25 @@ extern int gdb_bfd_close_or_warn (struct #define ALL_OBJFILE_OSECTIONS(objfile, osect) \ for (osect = objfile->sections; osect < objfile->sections_end; osect++) -#define ALL_OBJSECTIONS(objfile, osect) \ - ALL_OBJFILES (objfile) \ - ALL_OBJFILE_OSECTIONS (objfile, osect) +/* Traverse all obj_sections in all objfiles in the current program + space. Note that this detects a "break" in the inner loop, and + exits immediately from the outer loop as well, thus, client code + doesn't need to know that this is implemented with a double for. + The extra hair is to make sure that OBJFILE isn't cleared on a + "break". */ + +#define ALL_OBJSECTIONS(objfile, osect) \ + for ((objfile) = current_program_space->objfiles, \ + (objfile) != NULL ? ((osect) = (objfile)->sections_end) : 0; \ + (objfile) != NULL \ + && (osect) == (objfile)->sections_end; \ + ((osect) == (objfile)->sections_end \ + ? ((objfile) = (objfile)->next, \ + (objfile) != NULL ? (osect) = (objfile)->sections_end : 0) \ + : 0)) \ + for ((osect) = (objfile)->sections; \ + (osect) < (objfile)->sections_end; \ + (osect)++) #define SECT_OFF_DATA(objfile) \ ((objfile->sect_index_data == -1) \