From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 9F8C0385E002 for ; Tue, 24 Mar 2020 22:41:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9F8C0385E002 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 1EF761E5F8; Tue, 24 Mar 2020 18:41:55 -0400 (EDT) Subject: Re: [PATCH][gdb] Print user/includes fields for maint commands To: Tom de Vries , gdb-patches@sourceware.org References: <20200324112241.GA23197@delia> <0e3c47b7-9cec-5efa-c3cf-8bff63042886@simark.ca> <2b576b9e-82c4-a76b-ce54-b6828d1f49af@suse.de> From: Simon Marchi Message-ID: <9dcaf2a7-e591-5a5a-9464-b3913f435700@simark.ca> Date: Tue, 24 Mar 2020 18:41:54 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <2b576b9e-82c4-a76b-ce54-b6828d1f49af@suse.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-29.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Mar 2020 22:41:56 -0000 On 2020-03-24 6:28 p.m., Tom de Vries wrote: > On 24-03-2020 18:04, Simon Marchi wrote: > > Hi, > > thanks for the review, I've followed up on all the comments. > > Updated patch attached below. > > Thanks, > - Tom > > > 0001-gdb-Print-user-includes-fields-for-maint-commands.patch > > [gdb] Print user/includes fields for maint commands > > The type struct compunit_symtab contains two fields (disregarding field next) > that express relations with other compunit_symtabs: user and includes. > > These fields are currently not printed with "maint info symtabs" and > "maint print symbols". > > Fix this such that for "maint info symtabs" we print: > ... > { ((struct compunit_symtab *) 0x23e8450) > debugformat DWARF 2 > producer (null) > dirname (null) > blockvector ((struct blockvector *) 0x23e8590) > + user ((struct compunit_symtab *) 0x2336280) > + ( includes > + ((struct compunit_symtab *) 0x23e85e0) > + ((struct compunit_symtab *) 0x23e8960) > + ) > { symtab ((struct symtab *) 0x23e85b0) > fullname (null) > linetable ((struct linetable *) 0x0) > } > } > ... > > And for "maint print symbols" we print: > ... > -Symtab for file > +Symtab for file at 0x23e85b0 > Read from object file /data/gdb_versions/devel/a.out (0x233ccf0) > Language: c > > Blockvector: > > block #000, object at 0x23e8530, 0 syms/buckets in 0x0..0x0 > block #001, object at 0x23e84d0 under 0x23e8530, 0 syms/buckets in 0x0..0x0 > > +Compunit user: 0x2336300 > +Compunit include: 0x23e8900 > +Compunit include: 0x23dd970 > ... > Note: for user and includes we don't list the actual compunit_symtab address, > but instead the corresponding symtab address, which allows us to find that > symtab elsewhere in the output (given that we also now print the address of > symtabs). > > gdb/ChangeLog: > > 2020-03-24 Tom de Vries > > * symtab.h (is_main_symtab_of_compunit_symtab): New function. > * symmisc.c (dump_symtab_1): Print user and includes fields. > (maintenance_info_symtabs): Same. > > --- > gdb/symmisc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++--- > gdb/symtab.h | 7 +++++++ > 2 files changed, 56 insertions(+), 3 deletions(-) > > diff --git a/gdb/symmisc.c b/gdb/symmisc.c > index 3df526bddb..6ee1b5edfd 100644 > --- a/gdb/symmisc.c > +++ b/gdb/symmisc.c > @@ -279,8 +279,10 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile) > const struct block *b; > int depth; > > - fprintf_filtered (outfile, "\nSymtab for file %s\n", > - symtab_to_filename_for_display (symtab)); > + fprintf_filtered (outfile, "\nSymtab for file %s at %s\n", > + symtab_to_filename_for_display (symtab), > + host_address_to_string (symtab)); > + > if (SYMTAB_DIRNAME (symtab) != NULL) > fprintf_filtered (outfile, "Compilation directory is %s\n", > SYMTAB_DIRNAME (symtab)); > @@ -308,7 +310,7 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile) > } > /* Now print the block info, but only for compunit symtabs since we will > print lots of duplicate info otherwise. */ > - if (symtab == COMPUNIT_FILETABS (SYMTAB_COMPUNIT (symtab))) > + if (is_main_symtab_of_compunit_symtab (symtab)) > { > fprintf_filtered (outfile, "\nBlockvector:\n\n"); > bv = SYMTAB_BLOCKVECTOR (symtab); > @@ -371,6 +373,28 @@ dump_symtab_1 (struct symtab *symtab, struct ui_file *outfile) > "\nBlockvector same as owning compunit: %s\n\n", > compunit_filename); > } > + > + if (is_main_symtab_of_compunit_symtab (symtab)) > + { Hmm, if this is the exact same condition as the previous `if`, what's the point of having a separate `if`? Now that I read this code a bit more, I realize that it's written in this way because it dates from the time where there wasn't a distinction between compunit_symtabs and symtabs. Nowadays, it would probably be preferable to print the compunit_symtab information at some level higher, in maintenance_print_symbols. If you want to take the time to do things right, you can try do to that, otherwise I'm also fine with this patch (one the question above about the `if` is answered). Simon