From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26655 invoked by alias); 22 Apr 2010 12:20:29 -0000 Received: (qmail 26607 invoked by uid 22791); 22 Apr 2010 12:20:25 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 22 Apr 2010 12:20:14 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id A8E742BAD0E; Thu, 22 Apr 2010 08:20:12 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ejJLY0i2KWA9; Thu, 22 Apr 2010 08:20:12 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 747542BAD10; Thu, 22 Apr 2010 08:20:12 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 2B658F5895; Thu, 22 Apr 2010 05:20:12 -0700 (PDT) Date: Thu, 22 Apr 2010 12:20:00 -0000 From: Joel Brobecker To: Pierre Muller Cc: gdb-patches@sourceware.org Subject: Re: [RFA-v2] Fix bug report 11479 Message-ID: <20100422122012.GI19194@adacore.com> References: <000f01cad75b$b53278a0$1f9769e0$@muller@ics-cnrs.unistra.fr> <20100412155453.GW19194@adacore.com> <000001cada5c$49b8e1a0$dd2aa4e0$@muller@ics-cnrs.unistra.fr> <20100413152823.GG19194@adacore.com> <20100421151117.GA13675@adacore.com> <001501cae197$280ca8b0$7825fa10$@muller@ics-cnrs.unistra.fr> <20100422011119.GF19194@adacore.com> <001001cae205$59a6fbf0$0cf4f3d0$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <001001cae205$59a6fbf0$0cf4f3d0$@muller@ics-cnrs.unistra.fr> User-Agent: Mutt/1.5.20 (2009-06-14) 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-04/txt/msg00745.txt.bz2 > 2010-04-22 Pierre Muller > > PR stabs/11479. > * stabsread.c (set_length_in_type_chain): New function. > (read_struct_type): Call set_length_in_type_chain function. > (read_enum_type): Idem. The code portion is pre-approved with the comments below. Just go ahead and check it in. > 2010-04-22 Pierre Muller > > PR stabs/11479. > * gdb.stabs/gdb11479.exp: New file. > * gdb.stabs/gdb11479.c: New file. This portion can be checked in separately after we answer the few questions I have (see below). > +/* Set the length for all variants of a same main_type, which are > + connected in the closed chain. */ We're missing the "why" this is useful. How about the following? /* Set the length for all variants of a same main_type, which are connected in the closed chain. This is something that needs to be done when a type is defined *after* some cross references to this type have already been read. Consider for instance the following scenario where we have the following two stabs entries: .stabs "t:p(0,21)=*(0,22)=k(0,23)=xsdummy:",160,0,28,-24 .stabs "dummy:T(0,23)=s16x:(0,1),0,3[...]" A stubbed version of type dummy is created while processing the first stabs entry. The length of that type is initially set to zero, since it is unknown at this point. Also, a "constant" variation of type "dummy" is created as well (this is the "(0,22)=k(0,23)" section of the stabs line). The second stabs entry allows us to replace the stubbed definition with the real definition. However, we still need to adjust the length of the "constant" variation of that type, as its length was left untouched during the main type replacement... */ > +static void > +set_length_in_type_chain (struct type * type) ^^ No space after the * > + Please email any bugs, comments, and/or additions to this file to: > + bug-gdb@gnu.org */ Can you remove this bit? > +# Please email any bugs, comments, and/or additions to this file to: > +# bug-gdb@gnu.org Same for this one... > +set testfile "gdb11479" > +set srcfile ${testfile}.c > +set binfile ${objdir}/${subdir}/${testfile} > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable > {debug additional_flags=-gstabs}] != "" } { > + if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" > executable {debug}] != "" } { > + untested "couldn't compile ${srcdir}/${subdir}/${srcfile}" > + return -1 > + } > +} > + > +# Start with a fresh gdb. > +gdb_exit > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir > +gdb_load ${binfile} Can you actually use "prepare_for_testing" here? The only thing that is a bit out of the ordinary is request the -gstabs debug flag. I am actually wondering if we really want that, since this may not work on certain platforms while we might still want to use the test with the default debug format. On the other hand, you probably want to make sure that your test gets run with -gstabs on Windows, so we have two incompatible objectives... How's about we do 2 builds? One with standard debug, and another with -gstabs? We can repeat the testing by putting the gdb_test calls inside a proc and then call it twice? I think that prepare_for_testing should be able to handle the case where you need to add additional_flags as well. In fact: gdb.base/commands.exp:if { [prepare_for_testing commands.exp commands run.c {debug additional_flags=-DFAKEARGV}] } { > +# Regression test for a cleanup bug in the charset code. ? > +gdb_exit Harmless, but unnecessary. Let's toss it. -- Joel