From patchwork Wed Jun 24 23:02:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 7338 Received: (qmail 41590 invoked by alias); 24 Jun 2015 23:02:26 -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 41577 invoked by uid 89); 24 Jun 2015 23:02:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, KAM_STOCKGEN, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f73.google.com Received: from mail-pa0-f73.google.com (HELO mail-pa0-f73.google.com) (209.85.220.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 24 Jun 2015 23:02:23 +0000 Received: by pabli10 with SMTP id li10so4647377pab.1 for ; Wed, 24 Jun 2015 16:02:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:message-id:date:subject:from:to:cc :content-type; bh=fbX4Jzuwuf2Pwn07MhoNHXCPsblfnptiKI8zK5encTE=; b=bW2Gw9ydJp8DoMJ7iNTAeN26SxeJzv64rTTb7vsFzyXNebhaSEPUNiLWuzhWecmsII Y7ot6EUVh1EN7vQA6YwsxwaARPjzAp681iSPRNOcdqtmwF4ccwcFtfwrHOe/BBw1Vw9Q /E1Y6w2oij4+5YLRK9OLaNQwR3nN51Lmtr7UoyhROTiQsuoR24nzvkYmniK0/idJ7D9R iNJDyAcsUNXXSRb1rxnlq4LA4S9JAwtn9pFppVQx5vl2ifcv8zErhiHTS0j/K+CC8E4/ qYTUoapmH5x6VWbVbff8MVDDrQDURDH8DJGsX06PS2ILc4E/+W6whRig5SKyeWnFRdPY nGjw== X-Gm-Message-State: ALoCoQlG7PXqktZUY0Y18fpTmRqR29YIQ8Zxq9LDC4ARWAQQpTMEve8AMf20PxKBScS0yZOYVwQ6 MIME-Version: 1.0 X-Received: by 10.66.122.70 with SMTP id lq6mr54251902pab.48.1435186941254; Wed, 24 Jun 2015 16:02:21 -0700 (PDT) Message-ID: <047d7b2e4e86d2189d05194b7fc3@google.com> Date: Wed, 24 Jun 2015 23:02:21 +0000 Subject: Re: [RFC] Revisit PR 16253 ("Attempt to use a type name...") From: Doug Evans To: Keith Seitz Cc: gdb-patches@sourceware.org, Jan Kratochvil X-IsSubscribed: yes Keith Seitz writes: > On 06/17/2015 08:50 AM, Keith Seitz wrote: > > On 06/17/2015 05:33 AM, Jan Kratochvil wrote: > >> On Thu, 11 Jun 2015 20:57:18 +0200, Keith Seitz wrote: > >>> PR 16253 > >>> * block.c (block_lookup_symbol_primary): If a symbol is found > >>> which does not exactly match the requested domain, keep searching > >>> for an exact match. Otherwise, return the previously found "best" > >>> symbol. > >> > >> Is there a reason why you haven't patched also block_lookup_symbol? > > > > Two reasons: > > > > 1) I posted this RFC to raise discussion whether this was worth > > pursuing. No need to expend any energy on something that has zero chance > > of being accepted by maintainers. > > > > 2) More important I have not discovered/attempted to coverage test > > lookup_block_symbol to trigger this bug. > > > > I'll be attempting to do that today. > > And I failed. I've not found the "magic" combination of buttons to make > any difference in block_lookup_symbol. A variation of PR 18150? You need to put the symbols in an unexpanded symtab, the catch being that gdb expands the symtab with main() at startup. Lookup in expanded symtabs, which is done before looking up in unexpanded symtabs, is done with block_lookup_symbol_primary. Lookup in unexpanded symtabs is done with block_lookup_symbol. > Nonetheless it is not quite so straight-forward in the BLOCK_FUNCTION > case where we have to decide what is a "better" match: > > SYMBOL_DOMAIN == domain && SYMBOL_IS_ARGUMENT > > or > SYMBOL_DOMAIN != domain (but symbol_matches_domain returns 1) && > !SYMBOL_IS_ARGUMENT I'm not sure either. I'm not sure the BLOCK_FUNCTION case can even exercise this bug. > In that case, I cannot say which is more correct. Moreover I have been > unable to figure out how to test this. I worry that I would simply be > introducing a regression. IMO this is getting into "risky" territory. > [But then my philosophy is "if it ain't broke, don't fix it." As far as > I can tell, block_lookup_symbol is not "broke."] > > So what do maintainers want me to do? How about this? diff --git a/gdb/block.c b/gdb/block.c index 79a8f19..93ed6e7 100644 --- a/gdb/block.c +++ b/gdb/block.c @@ -739,13 +739,21 @@ block_lookup_symbol (const struct block *block, const char *name, if (!BLOCK_FUNCTION (block)) { + struct symbol *other = NULL; + ALL_BLOCK_SYMBOLS_WITH_NAME (block, name, iter, sym) { + if (SYMBOL_DOMAIN (sym) == domain) + return sym; + /* This is a bit of a hack, but symbol_matches_domain might ignore + STRUCT vs VAR domain symbols. So if a matching symbol is found, + make sure there is no "better" matching symbol, i.e., one with + exactly the same domain. PR 16253. */ if (symbol_matches_domain (SYMBOL_LANGUAGE (sym), SYMBOL_DOMAIN (sym), domain)) - return sym; + other = sym; } - return NULL; + return other; } else { @@ -753,7 +761,10 @@ block_lookup_symbol (const struct block *block, const char *name, list; this loop makes sure to take anything else other than parameter symbols first; it only uses parameter symbols as a last resort. Note that this only takes up extra computation - time on a match. */ + time on a match. + It's hard to define types in the parameter list (at least in + C/C++) so we don't do the same PR 16253 hack here that is done + for the !BLOCK_FUNCTION case. */ struct symbol *sym_found = NULL; @@ -779,23 +790,33 @@ struct symbol * block_lookup_symbol_primary (const struct block *block, const char *name, const domain_enum domain) { - struct symbol *sym; + struct symbol *sym, *other; struct dict_iterator dict_iter; /* Verify BLOCK is STATIC_BLOCK or GLOBAL_BLOCK. */ gdb_assert (BLOCK_SUPERBLOCK (block) == NULL || BLOCK_SUPERBLOCK (BLOCK_SUPERBLOCK (block)) == NULL); + other = NULL; for (sym = dict_iter_name_first (block->dict, name, &dict_iter); sym != NULL; sym = dict_iter_name_next (name, &dict_iter)) { + if (SYMBOL_DOMAIN (sym) == domain) + return sym; + + /* This is a bit of a hack, but symbol_matches_domain might ignore + STRUCT vs VAR domain symbols. So if a matching symbol is found, + make sure there is no "better" matching symbol, i.e., one with + exactly the same domain. PR 16253. */ if (symbol_matches_domain (SYMBOL_LANGUAGE (sym), SYMBOL_DOMAIN (sym), domain)) - return sym; + { + other = sym; + } } - return NULL; + return other; } /* See block.h. */ diff --git a/gdb/testsuite/gdb.cp/var-tag-2.cc b/gdb/testsuite/gdb.cp/var-tag-2.cc new file mode 100644 index 0000000..7733473 --- /dev/null +++ b/gdb/testsuite/gdb.cp/var-tag-2.cc @@ -0,0 +1,22 @@ +/* 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 . */ + +/* This object is in a separate file so that its debug info is not + expanded at startup. Once debug info is expanded we are no longer + exercising block_lookup_symbol, and instead are exercising + block_lookup_symbol_primary. */ +enum E2 {a2, b2, c2} E2; diff --git a/gdb/testsuite/gdb.cp/var-tag-3.cc b/gdb/testsuite/gdb.cp/var-tag-3.cc new file mode 100644 index 0000000..7f2133f --- /dev/null +++ b/gdb/testsuite/gdb.cp/var-tag-3.cc @@ -0,0 +1,22 @@ +/* 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 . */ + +/* This object is in a separate file so that its debug info is not + expanded at startup. Once debug info is expanded we are no longer + exercising block_lookup_symbol, and instead are exercising + block_lookup_symbol_primary. */ +struct S2 {} S2; diff --git a/gdb/testsuite/gdb.cp/var-tag-4.cc b/gdb/testsuite/gdb.cp/var-tag-4.cc new file mode 100644 index 0000000..162541c --- /dev/null +++ b/gdb/testsuite/gdb.cp/var-tag-4.cc @@ -0,0 +1,22 @@ +/* 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 . */ + +/* This object is in a separate file so that its debug info is not + expanded at startup. Once debug info is expanded we are no longer + exercising block_lookup_symbol, and instead are exercising + block_lookup_symbol_primary. */ +union U2 {int a; char b;} U2; diff --git a/gdb/testsuite/gdb.cp/var-tag.cc b/gdb/testsuite/gdb.cp/var-tag.cc new file mode 100644 index 0000000..93b9caf --- /dev/null +++ b/gdb/testsuite/gdb.cp/var-tag.cc @@ -0,0 +1,44 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2014 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 global = 3; + +class C { +public: + struct C1 {} C1; + enum E1 {a1, b1, c1} E1; + union U1 {int a1; char b1;} U1; + + C () : E1 (b1) {} + void global (void) const {} + int f (void) const { global (); return 0; } +} C; + +struct S {} S; +enum E {a, b, c} E; +union U {int a; char b;} U; + +class CC {} cc; +struct SS {} ss; +enum EE {ea, eb, ec} ee; +union UU {int aa; char bb;} uu; + +int +main (void) +{ + return C.f (); +} diff --git a/gdb/testsuite/gdb.cp/var-tag.exp b/gdb/testsuite/gdb.cp/var-tag.exp new file mode 100644 index 0000000..9854c31 --- /dev/null +++ b/gdb/testsuite/gdb.cp/var-tag.exp @@ -0,0 +1,105 @@ +# Copyright 2014, 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 . + +# This file is part of the gdb testsuite + +# Test expressions in which variable names shadow tag names. + +if {[skip_cplus_tests]} { continue } + +standard_testfile var-tag.cc var-tag-2.cc var-tag-3.cc var-tag-4.cc + +if {[prepare_for_testing $testfile.exp $testfile \ + [list $srcfile $srcfile2 $srcfile3 $srcfile4] {debug c++}]} { + return -1 +} + +proc do_global_tests {lang} { + set invalid_print "Attempt to use a type name as an expression" + set ptypefmt "type = (class|enum|union|struct) %s {.*}" + + with_test_prefix $lang { + gdb_test_no_output "set language $lang" + gdb_test "ptype C" "type = class C {.*}" + gdb_test "print E" "= a" + gdb_test "ptype E" "type = enum E {.*}" + gdb_test "print S" "= {}" + gdb_test "ptype S" "type = struct S {.*}" + gdb_test "print U" "= {.*}" + gdb_test "ptype U" "type = union U {.*}" + gdb_test "print cc" "= {.*}" + gdb_test "ptype cc" "type = class CC {.*}" + gdb_test "print CC" [format $invalid_print "CC"] + gdb_test "ptype CC" [format $ptypefmt "CC"] + gdb_test "print ss" "= {}" + gdb_test "ptype ss" "type = struct SS {.*}" + gdb_test "print SS" [format $invalid_print "SS"] + gdb_test "ptype SS" [format $ptypefmt "SS"] + gdb_test "print ee" "= .*" + gdb_test "ptype ee" "type = enum EE {.*}" + gdb_test "print EE" [format $invalid_print "EE"] + gdb_test "ptype EE" [format $ptypefmt "EE"] + gdb_test "print uu" "= {.*}" + gdb_test "ptype uu" "type = union UU {.*}" + gdb_test "print UU" [format $invalid_print "UU"] + gdb_test "ptype UU" [format $ptypefmt "UU"] + + # These tests exercise lookup of symbols using the "quick fns" API. + # Each of them is in a separate CU as once its CU is expanded, + # we're no longer using the quick fns API. + gdb_test "print E2" "= a2" + gdb_test "ptype E2" "type = enum E2 {.*}" + gdb_test "print S2" "= {}" + gdb_test "ptype S2" "type = struct S2 {.*}" + gdb_test "print U2" "= {.*}" + gdb_test "ptype U2" "type = union U2 {.*}" + } +} + +# First test expressions when there is no context. +with_test_prefix "before start" { + do_global_tests c++ + do_global_tests c +} + +# Run to main and test again. +if {![runto_main]} { + perror "couldn't run to main" + continue +} + +with_test_prefix "in main" { + do_global_tests c++ + do_global_tests c +} + +# Run to C::f and test again. +gdb_breakpoint "C::f" +gdb_continue_to_breakpoint "continue to C::f" +with_test_prefix "in C::f" { + do_global_tests c++ + do_global_tests c +} + +# Another hard-to-guess-the-users-intent bug... +# It would be really nice if we could query the user! +with_test_prefix "global collision" { + gdb_test_no_output "set language c++" + setup_kfail "c++/16463" "*-*-*" + gdb_test "print global" "= 3" + + # ... with a simple workaround: + gdb_test "print ::global" "= 3" +}