From patchwork Sun Sep 1 16:08:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 34366 Received: (qmail 42752 invoked by alias); 1 Sep 2019 16:08:59 -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 42700 invoked by uid 89); 1 Sep 2019 16:08:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=guard, somewhere X-HELO: mail-wm1-f68.google.com Received: from mail-wm1-f68.google.com (HELO mail-wm1-f68.google.com) (209.85.128.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 01 Sep 2019 16:08:56 +0000 Received: by mail-wm1-f68.google.com with SMTP id k1so12071441wmi.1 for ; Sun, 01 Sep 2019 09:08:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=9NSXij2iMGBczueedyYeyi6XvJ/vZFEyNenthrxJSGM=; b=GRo38jxrXy16zHWvGhNeCwZTUHPK3KrWTwiBJzwL/icYmc6bUB4k0C/1h+mbF/cJlb udOiCOHpcAYPsTrgpccx+uhcN7JYjLwTLo+EUb3ptTPch83mzmRs8VbAa983MnnjbTbn LIoYOCwPGAQinGKFiNY2BSHUrdeU/CHr/uRAss/f6il/C/1MR4+Q4OGHWCZ5uPgGvT8a /2JCfUjLFYHIFYF/RIC0/G+rGGCMOmh/8AWFhcI7pkImrxbjq6paEA+OFSx2D6su2XUX YeXUjnFgpud7A4q5VYnaZ3t0ZhdKzxOIBXgBFp2JuSr0XfdM8EK95dNBuZihwZt0x6a2 SLig== Return-Path: Received: from localhost (92.40.249.158.threembb.co.uk. [92.40.249.158]) by smtp.gmail.com with ESMTPSA id f186sm15593804wmg.21.2019.09.01.09.08.53 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 01 Sep 2019 09:08:54 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Tom de Vries , Andrew Burgess Subject: [PATCH 1/3] gdb: Don't fault for 'maint print psymbols' when using an index Date: Sun, 1 Sep 2019 17:08:45 +0100 Message-Id: In-Reply-To: References: In-Reply-To: References: X-IsSubscribed: yes I found that these tests: make check-gdb RUNTESTFLAGS="--target_board=cc-with-gdb-index gdb.base/maint.exp" make check-gdb RUNTESTFLAGS="--target_board=cc-with-debug-names gdb.base/maint.exp" were causing GDB to segfault. It turns out that this test runs this command: maint print psymbols -pc main /path/to/some/file which tries to lookup the partial_symtab for 'main'. The problem is that there is no partial_symtab for 'main' as we are using the .gdb_index or .debug_names instead of partial_symtabs. What happens is that maintenance_print_symbols calls find_pc_sect_psymtab, which looks for the partial_symtab in the objfile's objfile->partial_symtabs->psymtabs_addrmap. This is a problem because when we are using the indexes psymtabs_addrmap is reused to hold things other than partial_symtabs, this can be seen in dwarf2read.c in create_addrmap_from_index and create_addrmap_from_aranges. If we then lookup in psymtabs_addrmap we end up returning a pointer to something that isn't really a partial_symtab, after which everything goes wrong. Initially I simply added a check at the start of find_pc_sect_psymtab that the objfile had some partial_symtabs, like: if (objfile->partial_symtabs->psymtabs == NULL) return NULL; Figuring that if there were no partial_symtabs at all then this function should always return NULL, however, this caused a failure in the test gdb.python/py-event.exp which I didn't dig into too deeply, but seems to be that in this tests there are initially no psymtabs, but the second part of find_pc_sect_psymtab does manage to read some in from somewhere, with the check I added the test fails as we returned NULL here and this caused GDB to load in the full symtabs earlier than was expected. Instead I chose to guard only the access to psymtabs_addrmap with a check that the function has some psymtabs. This allows my original tests to pass, and the py-event.exp test to pass too. Now, a good argument can be made that we simply should never call find_pc_sect_psymtab on an objfile that is using indexes instead of partial_symtabs. I did consider this approach, we could easily add an assert into find_pc_sect_psymtab that if we find a partial_symtab in psymtabs_addrmap then the psymtabs pointer must be non-null. The responsibility would then be on the user of find_pc_sect_psymtab to ensure that the objfile being checked is suitable. In the end I didn't take this approach as the check in find_pc_sect_psymtab is cheap and this ensures that any future miss-uses of the function will not cause problems. I also extended the comment on psymtabs_addrmap to indicate that it holds more than just partial_symtabs as this was not at all clear from the original comment, and caused me some confusion when I was initially debugging this problem. gdb/ChangeLog: * psymtab.c (find_pc_sect_psymtab): Move baseaddr local into more inner scope, add check that the objfile has psymtabs before checking psymtabs_addrmap. * psymtab.h (psymtab_storage) : Extend comment. --- gdb/ChangeLog | 7 +++++++ gdb/psymtab.c | 24 +++++++++++++++++------- gdb/psymtab.h | 6 +++++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 6cc7566580a..bc0f1d17783 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -308,14 +308,24 @@ find_pc_sect_psymtab (struct objfile *objfile, CORE_ADDR pc, struct obj_section *section, struct bound_minimal_symbol msymbol) { - CORE_ADDR baseaddr = ANOFFSET (objfile->section_offsets, - SECT_OFF_TEXT (objfile)); - - /* Try just the PSYMTABS_ADDRMAP mapping first as it has better granularity - than the later used TEXTLOW/TEXTHIGH one. */ - - if (objfile->partial_symtabs->psymtabs_addrmap != NULL) + /* Try just the PSYMTABS_ADDRMAP mapping first as it has better + granularity than the later used TEXTLOW/TEXTHIGH one. However, we need + to take care as the PSYMTABS_ADDRMAP can hold things other than partial + symtabs in some cases. + + This function should only be called for objfiles that are using partial + symtabs, not for objfiles that are using indexes (.gdb_index or + .debug_names), however 'maintenance print psymbols' calls this function + directly for all objfiles. If we assume that PSYMTABS_ADDRMAP contains + partial symtabs then we will end up returning a pointer to an object + that is not a partial_symtab, which doesn't end well. */ + + if (objfile->partial_symtabs->psymtabs != NULL + && objfile->partial_symtabs->psymtabs_addrmap != NULL) { + CORE_ADDR baseaddr = ANOFFSET (objfile->section_offsets, + SECT_OFF_TEXT (objfile)); + struct partial_symtab *pst = ((struct partial_symtab *) addrmap_find (objfile->partial_symtabs->psymtabs_addrmap, diff --git a/gdb/psymtab.h b/gdb/psymtab.h index aed686258d4..0ad2b49d9a5 100644 --- a/gdb/psymtab.h +++ b/gdb/psymtab.h @@ -109,7 +109,11 @@ public: /* Map addresses to the entries of PSYMTABS. It would be more efficient to have a map per the whole process but ADDRMAP cannot selectively remove its items during FREE_OBJFILE. This mapping is already present even for - PARTIAL_SYMTABs which still have no corresponding full SYMTABs read. */ + PARTIAL_SYMTABs which still have no corresponding full SYMTABs read. + + The DWARF parser reuses this addrmap to store things other than + psymtabs in the cases where debug information is being read from, for + example, the .debug-names section. */ struct addrmap *psymtabs_addrmap = nullptr;