From patchwork Fri Apr 17 00:02:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 6270 Received: (qmail 12236 invoked by alias); 17 Apr 2015 00:02:46 -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 12224 invoked by uid 89); 17 Apr 2015 00:02:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, KAM_STOCKGEN, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mail-pd0-f202.google.com Received: from mail-pd0-f202.google.com (HELO mail-pd0-f202.google.com) (209.85.192.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 17 Apr 2015 00:02:42 +0000 Received: by pdjg10 with SMTP id g10so6879254pdj.0 for ; Thu, 16 Apr 2015 17:02:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-type; bh=JsIfGp88r8/ekZCEnnIQYNzJwPaJFhmRVxEDcutqUZE=; b=aJ8k3MnOWu/jxbuNu8VHiRSz2/beiBN1lER/o05OlplaeTwJEp/2ElLuWWoKko7wgC JgWVQGL9/hwraBfyLixVHE2GUEhuq8P4aKYBLNxUSDdKWr+sQlYPnal8Bp4zFRb7ryTr a+zHc8otgrz+aGEY9CGp2WG6InrMVj2LwDuWuge50DnNwNBecvgtqUVD9IY2gz4MsISL xcwOR4E8a+wospIw/kJCUHBOZmM/gB+fwmXF5EFbWyYCTMAwUbrrwalJginHCjhPDtF5 DPGHnEDQ2yqv9poq9WpY5fBwajsBWxKNYiTJSzggZpnRl6E7j7FPD3oATlr8MLmY4l3V LsFQ== X-Gm-Message-State: ALoCoQlH1LrIluoCQF2zWNEeI4418cgC/1ckkVREJsvVrMrpJzvduCqzM9E277XrXRbaMrsIKvp35gQV+tOyb7x99IS2UwMWD/GzzQS4XMUR0APFT2FJ8dfbNc55sKTVwHcs5cm5vIC2M2sXFMymYXqN1IceM3Ni1HNbkYVoZyHaM13GJ6b3C/w= X-Received: by 10.66.159.227 with SMTP id xf3mr402658pab.27.1429228960158; Thu, 16 Apr 2015 17:02:40 -0700 (PDT) Received: from corpmail-nozzle1-2.hot.corp.google.com ([100.108.1.103]) by gmr-mx.google.com with ESMTPS id i27si275995yha.6.2015.04.16.17.02.39 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Apr 2015 17:02:40 -0700 (PDT) Received: from ruffy.mtv.corp.google.com ([172.17.128.44]) by corpmail-nozzle1-2.hot.corp.google.com with ESMTPS id hqAG2iGJ.1; Thu, 16 Apr 2015 17:02:39 -0700 From: Doug Evans To: gdb-patches@sourceware.org Subject: [PATCH] [PR symtab/18258] Fix. Date: Thu, 16 Apr 2015 17:02:38 -0700 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes Hi. This patch fixes pr 18258. https://sourceware.org/bugzilla/show_bug.cgi?id=18258 The problem is described pretty well in the PR. Basically, we need to include the opaque-type test while we're iterating over compunit_symtab->includes. Regression tested on amd64-linux. 2015-04-16 Doug Evans PR symtab/18258 * block.c (block_find_symbol): New function. (block_find_non_opaque_type): Ditto. (block_find_non_opaque_type_preferred): Ditto. * block.h (block_symbol_matcher_ftype): New typedef. (block_find_symbol): Declare. (block_find_non_opaque_type): Ditto. (block_find_non_opaque_type_preferred): Ditto. * dwarf2read.c (dw2_lookup_symbol): Call block_find_symbol. * psymtab.c (psym_lookup_symbol): Ditto. * symtab.c (basic_lookup_transparent_type_1): New function. (basic_lookup_transparent_type): Call it. testsuite/ PR symtab/18258 * gdb.dwarf2/opaque-type-lookup-2.c: New file. * gdb.dwarf2/opaque-type-lookup.c: New file. * gdb.dwarf2/opaque-type-lookup.exp: New file. diff --git a/gdb/block.c b/gdb/block.c index 00a7012..79a8f19 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -797,3 +797,50 @@ block_lookup_symbol_primary (const struct block *block, const char *name, return NULL; } + +/* See block.h. */ + +struct symbol * +block_find_symbol (const struct block *block, const char *name, + const domain_enum domain, + block_symbol_matcher_ftype *matcher, void *data) +{ + struct block_iterator iter; + struct symbol *sym; + + /* Verify BLOCK is STATIC_BLOCK or GLOBAL_BLOCK. */ + gdb_assert (BLOCK_SUPERBLOCK (block) == NULL + || BLOCK_SUPERBLOCK (BLOCK_SUPERBLOCK (block)) == NULL); + + ALL_BLOCK_SYMBOLS_WITH_NAME (block, name, iter, sym) + { + /* MATCHER is deliberately called second here so that it never sees + a non-domain-matching symbol. */ + if (symbol_matches_domain (SYMBOL_LANGUAGE (sym), + SYMBOL_DOMAIN (sym), domain) + && matcher (sym, data)) + return sym; + } + return NULL; +} + +/* See block.h. */ + +int +block_find_non_opaque_type (struct symbol *sym, void *data) +{ + return !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)); +} + +/* See block.h. */ + +int +block_find_non_opaque_type_preferred (struct symbol *sym, void *data) +{ + struct symbol **best = data; + + if (!TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))) + return 1; + *best = sym; + return 0; +} diff --git a/gdb/block.h b/gdb/block.h index bdc5888..d8ad343 100644 --- a/gdb/block.h +++ b/gdb/block.h @@ -292,6 +292,40 @@ extern struct symbol *block_lookup_symbol_primary (const struct block *block, const char *name, const domain_enum domain); +/* The type of the MATCHER argument to block_find_symbol. */ + +typedef int (block_symbol_matcher_ftype) (struct symbol *, void *); + +/* Find symbol NAME in BLOCK and in DOMAIN that satisfies MATCHER. + DATA is passed unchanged to MATCHER. + BLOCK must be STATIC_BLOCK or GLOBAL_BLOCK. */ + +extern struct symbol *block_find_symbol (const struct block *block, + const char *name, + const domain_enum domain, + block_symbol_matcher_ftype *matcher, + void *data); + +/* A matcher function for block_find_symbol to find only symbols with + non-opaque types. */ + +extern int block_find_non_opaque_type (struct symbol *sym, void *data); + +/* A matcher function for block_find_symbol to prefer symbols with + non-opaque types. The way to use this function is as follows: + + struct symbol *with_opaque = NULL; + struct symbol *sym + = block_find_symbol (block, name, domain, + block_find_non_opaque_type_preferred, &with_opaque); + + At this point if SYM is non-NULL then a non-opaque type has been found. + Otherwise, if WITH_OPAQUE is non-NULL then an opaque type has been found. + Otherwise, the symbol was not found. */ + +extern int block_find_non_opaque_type_preferred (struct symbol *sym, + void *data); + /* Macro to loop through all symbols in BLOCK, in no particular order. ITER helps keep track of the iteration, and must be a struct block_iterator. SYM points to the current symbol. */ diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index b91fbf5..85e4b28 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -3663,23 +3663,25 @@ dw2_lookup_symbol (struct objfile *objfile, int block_index, while ((per_cu = dw2_symtab_iter_next (&iter)) != NULL) { - struct symbol *sym = NULL; + struct symbol *sym, *with_opaque = NULL; struct compunit_symtab *stab = dw2_instantiate_symtab (per_cu); const struct blockvector *bv = COMPUNIT_BLOCKVECTOR (stab); struct block *block = BLOCKVECTOR_BLOCK (bv, block_index); + sym = block_find_symbol (block, name, domain, + block_find_non_opaque_type_preferred, + &with_opaque); + /* Some caution must be observed with overloaded functions and methods, since the index will not contain any overload information (but NAME might contain it). */ - sym = block_lookup_symbol (block, name, domain); - - if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0) - { - if (!TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))) - return stab; - stab_best = stab; - } + if (sym != NULL + && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0) + return stab; + if (with_opaque != NULL + && strcmp_iw (SYMBOL_SEARCH_NAME (with_opaque), name) == 0) + stab_best = stab; /* Keep looking through other CUs. */ } diff --git a/gdb/psymtab.c b/gdb/psymtab.c index 383e4c4..9ee6ed1 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -516,7 +516,7 @@ psym_lookup_symbol (struct objfile *objfile, if (!ps->readin && lookup_partial_symbol (objfile, ps, name, psymtab_index, domain)) { - struct symbol *sym = NULL; + struct symbol *sym, *with_opaque = NULL; struct compunit_symtab *stab = psymtab_to_symtab (objfile, ps); /* Note: While psymtab_to_symtab can return NULL if the partial symtab is empty, we can assume it won't here because lookup_partial_symbol @@ -524,18 +524,20 @@ psym_lookup_symbol (struct objfile *objfile, const struct blockvector *bv = COMPUNIT_BLOCKVECTOR (stab); struct block *block = BLOCKVECTOR_BLOCK (bv, block_index); + sym = block_find_symbol (block, name, domain, + block_find_non_opaque_type_preferred, + &with_opaque); + /* Some caution must be observed with overloaded functions - and methods, since the psymtab will not contain any overload + and methods, since the index will not contain any overload information (but NAME might contain it). */ - sym = block_lookup_symbol (block, name, domain); - - if (sym && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0) - { - if (!TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))) - return stab; - stab_best = stab; - } + if (sym != NULL + && strcmp_iw (SYMBOL_SEARCH_NAME (sym), name) == 0) + return stab; + if (with_opaque != NULL + && strcmp_iw (SYMBOL_SEARCH_NAME (with_opaque), name) == 0) + stab_best = stab; /* Keep looking through other psymtabs. */ } diff --git a/gdb/symtab.c b/gdb/symtab.c index 72df872..6693930 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -2804,12 +2804,39 @@ basic_lookup_transparent_type_quick (struct objfile *objfile, int block_index, bv = COMPUNIT_BLOCKVECTOR (cust); block = BLOCKVECTOR_BLOCK (bv, block_index); - sym = block_lookup_symbol (block, name, STRUCT_DOMAIN); - if (!sym) + sym = block_find_symbol (block, name, STRUCT_DOMAIN, + block_find_non_opaque_type, NULL); + if (sym == NULL) error_in_psymtab_expansion (block_index, name, cust); + gdb_assert (!TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))); + return SYMBOL_TYPE (sym); +} + +/* Subroutine of basic_lookup_transparent_type to simplify it. + Look up the non-opaque definition of NAME in BLOCK_INDEX of OBJFILE. + BLOCK_INDEX is either GLOBAL_BLOCK or STATIC_BLOCK. */ - if (!TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))) - return SYMBOL_TYPE (sym); +static struct type * +basic_lookup_transparent_type_1 (struct objfile *objfile, int block_index, + const char *name) +{ + const struct compunit_symtab *cust; + const struct blockvector *bv; + const struct block *block; + const struct symbol *sym; + + ALL_OBJFILE_COMPUNITS (objfile, cust) + { + bv = COMPUNIT_BLOCKVECTOR (cust); + block = BLOCKVECTOR_BLOCK (bv, block_index); + sym = block_find_symbol (block, name, STRUCT_DOMAIN, + block_find_non_opaque_type, NULL); + if (sym != NULL) + { + gdb_assert (!TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))); + return SYMBOL_TYPE (sym); + } + } return NULL; } @@ -2837,16 +2864,9 @@ basic_lookup_transparent_type (const char *name) ALL_OBJFILES (objfile) { - ALL_OBJFILE_COMPUNITS (objfile, cust) - { - bv = COMPUNIT_BLOCKVECTOR (cust); - block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK); - sym = block_lookup_symbol (block, name, STRUCT_DOMAIN); - if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))) - { - return SYMBOL_TYPE (sym); - } - } + t = basic_lookup_transparent_type_1 (objfile, GLOBAL_BLOCK, name); + if (t) + return t; } ALL_OBJFILES (objfile) @@ -2865,16 +2885,9 @@ basic_lookup_transparent_type (const char *name) ALL_OBJFILES (objfile) { - ALL_OBJFILE_COMPUNITS (objfile, cust) - { - bv = COMPUNIT_BLOCKVECTOR (cust); - block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK); - sym = block_lookup_symbol (block, name, STRUCT_DOMAIN); - if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym))) - { - return SYMBOL_TYPE (sym); - } - } + t = basic_lookup_transparent_type_1 (objfile, STATIC_BLOCK, name); + if (t) + return t; } ALL_OBJFILES (objfile) diff --git a/gdb/testsuite/gdb.dwarf2/opaque-type-lookup-2.c b/gdb/testsuite/gdb.dwarf2/opaque-type-lookup-2.c new file mode 100644 index 0000000..948e26d --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/opaque-type-lookup-2.c @@ -0,0 +1,24 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +/* These are actually struct struct_{a,b}, but that's handled by the dwarf + in opaque-type-lookup.exp. + IWBN to give these a different name than what's in the dwarf so that minsym + lookup doesn't interfere with the testing. However, that currently doesn't + work (we don't record the linkage name of the symbol). */ +char variable_a = 'a'; +char variable_b = 'b'; diff --git a/gdb/testsuite/gdb.dwarf2/opaque-type-lookup.c b/gdb/testsuite/gdb.dwarf2/opaque-type-lookup.c new file mode 100644 index 0000000..7cfe08e --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/opaque-type-lookup.c @@ -0,0 +1,23 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +int +main() +{ + asm ("main_label: .globl main_label"); + return 0; +} diff --git a/gdb/testsuite/gdb.dwarf2/opaque-type-lookup.exp b/gdb/testsuite/gdb.dwarf2/opaque-type-lookup.exp new file mode 100644 index 0000000..67f6dbf --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/opaque-type-lookup.exp @@ -0,0 +1,200 @@ +# Copyright 2013-2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test PR 18258. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +if {![dwarf2_support]} { + return 0 +} + +standard_testfile opaque-type-lookup.c opaque-type-lookup-1.S opaque-type-lookup-2.c + +# Create the DWARF. +set asm_file [standard_output_file $srcfile2] +Dwarf::assemble $asm_file { + declare_labels partial_unit_defining_a partial_unit_defining_b + declare_labels partial_unit_with_opaque + declare_labels struct_a_label struct_b_label + declare_labels opaque_struct_a_label opaque_struct_b_label + declare_labels char_type1_label char_type2_label + global srcdir subdir srcfile + + extern main + + # The partial units are laid out so we're not dependent on the order that + # they appear in compunit_symtab.includes. We need the one with the + # opaque definition to appear first to gdb, so we put it in the middle. + # Either the handling of variable_a or variable_b will be broken (in an + # unpatched gdb). + # + # However, once a partial unit with a non-opaque type is read in we can + # no longer see the bug as gdb will keep looking until it eventually gets + # to the defining partial unit, setting aside the symbol lookup cache. + # So heads up! + + cu {} { + compile_unit { + {language @DW_LANG_C} + {name opaque_before} + } { + imported_unit { + {import $partial_unit_with_opaque ref_addr} + } + + imported_unit { + {import $partial_unit_defining_a ref_addr} + } + } + } + + cu {} { + compile_unit { + {language @DW_LANG_C} + {name opaque_after} + } { + imported_unit { + {import $partial_unit_defining_b ref_addr} + } + + imported_unit { + {import $partial_unit_with_opaque ref_addr} + } + } + } + + cu {} { + partial_unit_with_opaque: partial_unit { + {name "partial_unit_with_opaque"} + } { + # Normally gdb doesn't add opaque types to the symbol table + # but there are times when it will, and in order to exercise + # this bug we need this entry in the symbol table. + # By giving it a size of 1 we achieve this. + + opaque_struct_a_label: structure_type { + {name struct_a} + {declaration 1 flag} + {byte_size 1 DW_FORM_sdata} + } + + opaque_struct_b_label: structure_type { + {name struct_b} + {declaration 1 flag} + {byte_size 1 DW_FORM_sdata} + } + + DW_TAG_variable { + {name variable_a} + {type :$opaque_struct_a_label} + {external 1 flag} + {declaration 1 flag} + } + + DW_TAG_variable { + {name variable_b} + {type :$opaque_struct_b_label} + {external 1 flag} + {declaration 1 flag} + } + } + } + + cu {} { + partial_unit_defining_a: partial_unit { + {name "partial_unit_defining_a"} + } { + char_type1_label: base_type { + {name "signed char"} + {encoding @DW_ATE_signed} + {byte_size 1 DW_FORM_sdata} + } + + struct_a_label: structure_type { + {name struct_a} + {byte_size 1 DW_FORM_sdata} + } { + member { + {name xyz} + {type :$char_type1_label} + {data_member_location 0 DW_FORM_sdata} + } + } + + DW_TAG_variable { + {name variable_a} + {type :$struct_a_label} + {external 1 flag} + {linkage_name variable_a} + } + } + } + + cu {} { + partial_unit_defining_b: partial_unit { + {name "partial_unit_defining_b"} + } { + char_type2_label: base_type { + {name "signed char"} + {encoding @DW_ATE_signed} + {byte_size 1 DW_FORM_sdata} + } + + struct_b_label: structure_type { + {name struct_b} + {byte_size 1 DW_FORM_sdata} + } { + member { + {name xyz} + {type :$char_type2_label} + {data_member_location 0 DW_FORM_sdata} + } + } + + DW_TAG_variable { + {name variable_b} + {type :$struct_b_label} + {external 1 flag} + {linkage_name variable_b} + } + } + } + + # GDB expands the symbol table with main at start up, + # so keep this separate. + cu {} { + compile_unit { + {language @DW_LANG_C} + {name main} + } { + subprogram { + {MACRO_AT_func { main ${srcdir}/${subdir}/${srcfile} }} + } + } + } +} + +if [prepare_for_testing ${testfile}.exp $testfile "${asm_file} ${srcfile} ${srcfile3}" {nodebug}] { + return -1 +} + +if ![runto_main] { + return -1 +} + +gdb_test "p variable_a" " = {xyz = 97 'a'}" +gdb_test "p variable_b" " = {xyz = 98 'b'}"