From patchwork Wed Aug 28 12:34:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 34294 Received: (qmail 13350 invoked by alias); 28 Aug 2019 12:34: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 13341 invoked by uid 89); 28 Aug 2019 12:34:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.2 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=19, 9 X-HELO: mail-wm1-f66.google.com Received: from mail-wm1-f66.google.com (HELO mail-wm1-f66.google.com) (209.85.128.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 28 Aug 2019 12:34:44 +0000 Received: by mail-wm1-f66.google.com with SMTP id l2so2697126wmg.0 for ; Wed, 28 Aug 2019 05:34:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VsPHQBozVw8xALC3hGUqnMnFUmMz0MaliDvIusX71Wo=; b=YAdReI3v4xvyweR6iR3l4/1+PvbJJjrEZotmaBj4c5rT9jD0GMLX7tkqsuk8kqHagl Dfz69sQhKh4zbvtrEhA2uytmkQ4k+rZybc0UlgOZ177Zy3pWGl6RjA8mbJKvBXp2PeMn mRXyMVEUsK7Q4xZarLdYgKfPcJYlCL9cu4M9DO8PsXGs7T3dUyJm7Eo90qyrDmJQfQq4 KfezUivSNzm+01x9bnIY++FWb7rQlczGk0s5WWKaUwNIpme0FT4ZAXVXxZaHxZki5l94 qk6FLZqRPaWR3O/K5pwGJTaBMzn1O8MUUkbCji2pSr+6yBD14E+QZvxneotSeqsh4OVU Npiw== Return-Path: Received: from localhost (host86-161-16-231.range86-161.btcentralplus.com. [86.161.16.231]) by smtp.gmail.com with ESMTPSA id e14sm1735130wme.35.2019.08.28.05.34.41 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 28 Aug 2019 05:34:41 -0700 (PDT) Date: Wed, 28 Aug 2019 13:34:40 +0100 From: Andrew Burgess To: Christian Biesinger Cc: Tom Tromey , gdb-patches Subject: Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal Message-ID: <20190828123440.GU6076@embecosm.com> References: <20190801170412.5553-1-tromey@adacore.com> <20190801170412.5553-5-tromey@adacore.com> <20190821103225.GF6076@embecosm.com> <20190827095357.GO6076@embecosm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Fortune: What happens when you cut back the jungle? It recedes. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Christian Biesinger [2019-08-27 13:16:37 -0500]: > On Tue, Aug 27, 2019 at 4:54 AM Andrew Burgess > wrote: > > @@ -2662,6 +2646,22 @@ lookup_global_symbol (const char *name, > > if (objfile != NULL) > > result = solib_global_lookup (objfile, name, domain); > > > > + /* If a block was passed in, we want to search the corresponding > > + global block first. This yields "more expected" behavior, and is > > + needed to support 'FILENAME'::VARIABLE lookups. */ > > + const struct block *global_block = block_global_block (block); > > + if (global_block != nullptr) > > + { > > + result.symbol = lookup_symbol_in_block (name, > > + symbol_name_match_type::FULL, > > + global_block, domain); > > + if (result.symbol != nullptr) > > + { > > + result.block = global_block; > > + return result; > > + } > > + } > > + > > /* If that didn't work go a global search (of global blocks, heh). */ > > if (result.symbol == NULL) > > { > > I like this change but shouldn't this call happen before solib_global_lookup? > > (That said, I would like to remove that function... > https://patches-gcc.linaro.org/patch/21673/) Here is a version of my patch that applies when Tom's series is rebased onto current upstream HEAD. As Christian suggests, this moves the check before the call to solib_global_lookup. I regression tested this again and see no failures. --- commit ba3a8d0bf2863ec9fede2901b119bfdeb67c9434 Author: Andrew Burgess Date: Tue Aug 27 10:47:19 2019 +0100 gdb: Fix FILE::VAR symbol lookup for C++ Extend the recently added test case to compile as C++ and C, then check that we can find all of the symbols. Move a recently added check of a global block from basic_lookup_symbol_nonlocal into lookup_global_symbol, fixing the C++ case. I've also made test names in gdb.base/print-file-var.exp unique. gdb/ChangeLog: * symtab.c (basic_lookup_symbol_nonlocal): Move check of global block from here, to... (lookup_global_symbol): ...here. gdb/testsuite/ChangeLog: * gdb.base/print-file-var-lib1.c: Add extern "C" wrappers around interface functions. * gdb.base/print-file-var-lib2.c: Likewise. * gdb.base/print-file-var-main.c: Likewise around declarations. * gdb.base/print-file-var.exp: Compile tests as both C++ and C, make test names unique. * gdb.base/print-file-var.h: Add some #defines to simplify setting up extern "C" blocks. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index c6351403819..3eb9d487e98 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2019-08-27 Andrew Burgess + + * symtab.c (basic_lookup_symbol_nonlocal): Move check of global + block from here, to... + (lookup_global_symbol): ...here. + 2019-08-21 Tom Tromey * NEWS: Add $_ada_exception entry. diff --git a/gdb/symtab.c b/gdb/symtab.c index f05bebfbcbb..49bcef14305 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -2425,22 +2425,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef, if (result.symbol != NULL) return result; - /* If a block was passed in, we want to search the corresponding - global block now. This yields "more expected" behavior, and is - needed to support 'FILENAME'::VARIABLE lookups. */ - const struct block *global_block = block_global_block (block); - if (global_block != nullptr) - { - result.symbol = lookup_symbol_in_block (name, - symbol_name_match_type::FULL, - global_block, domain); - if (result.symbol != nullptr) - { - result.block = global_block; - return result; - } - } - /* If we didn't find a definition for a builtin type in the static block, search for it now. This is actually the right thing to do and can be a massive performance win. E.g., when debugging a program with lots of @@ -2665,6 +2649,19 @@ lookup_global_symbol (const char *name, const struct block *block, const domain_enum domain) { + /* If a block was passed in, we want to search the corresponding + global block first. This yields "more expected" behavior, and is + needed to support 'FILENAME'::VARIABLE lookups. */ + const struct block *global_block = block_global_block (block); + if (global_block != nullptr) + { + symbol *sym = lookup_symbol_in_block (name, + symbol_name_match_type::FULL, + global_block, domain); + if (sym != nullptr) + return { sym, global_block }; + } + struct objfile *objfile = lookup_objfile_from_block (block); return lookup_global_or_static_symbol (name, GLOBAL_BLOCK, objfile, domain); } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index e0b0efc6cfd..69effa30f0f 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,14 @@ +2019-08-27 Andrew Burgess + + * gdb.base/print-file-var-lib1.c: Add extern "C" wrappers around + interface functions. + * gdb.base/print-file-var-lib2.c: Likewise. + * gdb.base/print-file-var-main.c: Likewise around declarations. + * gdb.base/print-file-var.exp: Compile tests as both C++ and C, + make test names unique. + * gdb.base/print-file-var.h: Add some #defines to simplify setting + up extern "C" blocks. + 2019-08-27 Andrew Burgess * gdb.ada/catch_ex_std.exp: Handle case where exceptions can't be diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c index aec04a9b02b..d172c15bc7d 100644 --- a/gdb/testsuite/gdb.base/print-file-var-lib1.c +++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c @@ -19,6 +19,8 @@ ATTRIBUTE_VISIBILITY int this_version_id = 104; +START_EXTERN_C + int get_version_1 (void) { @@ -26,3 +28,5 @@ get_version_1 (void) return this_version_id; } + +END_EXTERN_C diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c index 4dfdfa04c99..b392aff9f3d 100644 --- a/gdb/testsuite/gdb.base/print-file-var-lib2.c +++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c @@ -19,9 +19,13 @@ ATTRIBUTE_VISIBILITY int this_version_id = 203; +START_EXTERN_C + int get_version_2 (void) { printf ("get_version_2: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id); return this_version_id; } + +END_EXTERN_C diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c index 29d4fed22d1..1472bd44883 100644 --- a/gdb/testsuite/gdb.base/print-file-var-main.c +++ b/gdb/testsuite/gdb.base/print-file-var-main.c @@ -23,9 +23,13 @@ #include "print-file-var.h" +START_EXTERN_C + extern int get_version_1 (void); extern int get_version_2 (void); +END_EXTERN_C + #if VERSION_ID_MAIN ATTRIBUTE_VISIBILITY int this_version_id = 55; #endif diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp index a37cca70de6..1a065cf568b 100644 --- a/gdb/testsuite/gdb.base/print-file-var.exp +++ b/gdb/testsuite/gdb.base/print-file-var.exp @@ -17,7 +17,7 @@ if {[skip_shlib_tests]} { return -1 } -proc test {hidden dlopen version_id_main} { +proc test {hidden dlopen version_id_main lang} { global srcdir subdir set main "print-file-var-main" @@ -32,7 +32,7 @@ proc test {hidden dlopen version_id_main} { set libobj1 [standard_output_file ${lib1}$suffix.so] set libobj2 [standard_output_file ${lib2}$suffix.so] - set lib_opts { debug additional_flags=-fPIC } + set lib_opts { debug additional_flags=-fPIC $lang } lappend lib_opts "additional_flags=-DHIDDEN=$hidden" if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \ @@ -46,7 +46,7 @@ proc test {hidden dlopen version_id_main} { return -1 } - set main_opts [list debug shlib=${libobj1}] + set main_opts [list debug shlib=${libobj1} $lang] if {$dlopen} { lappend main_opts "shlib_load" \ @@ -108,12 +108,14 @@ proc test {hidden dlopen version_id_main} { # Compare the values of $sym1 and $sym2. proc compare {sym1 sym2} { - # Done this way instead of comparing the symbols with "print $sym1 - # == sym2" in GDB directly so that the values of the symbols end - # up visible in the logs, for debug purposes. - set vsym1 [get_integer_valueof $sym1 -1] - set vsym2 [get_integer_valueof $sym2 -1] - gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2" + with_test_prefix "sym1=$sym1,sym2=$sym2" { + # Done this way instead of comparing the symbols with "print $sym1 + # == sym2" in GDB directly so that the values of the symbols end + # up visible in the logs, for debug purposes. + set vsym1 [get_integer_valueof $sym1 -1] + set vsym2 [get_integer_valueof $sym2 -1] + gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2" + } } if $version_id_main { @@ -123,13 +125,14 @@ proc test {hidden dlopen version_id_main} { compare "'print-file-var-lib1.c'::this_version_id" "v1" compare "'print-file-var-lib2.c'::this_version_id" "v2" - } -foreach_with_prefix hidden {0 1} { - foreach_with_prefix dlopen {0 1} { - foreach_with_prefix version_id_main {0 1} { - test $hidden $dlopen $version_id_main +foreach_with_prefix lang { c c++ } { + foreach_with_prefix hidden {0 1} { + foreach_with_prefix dlopen {0 1} { + foreach_with_prefix version_id_main {0 1} { + test $hidden $dlopen $version_id_main $lang + } } } } diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h index fe7a3460edb..c44e4848b4a 100644 --- a/gdb/testsuite/gdb.base/print-file-var.h +++ b/gdb/testsuite/gdb.base/print-file-var.h @@ -23,4 +23,12 @@ # define ATTRIBUTE_VISIBILITY #endif +#ifdef __cplusplus +# define START_EXTERN_C extern "C" { +# define END_EXTERN_C } +#else +# define START_EXTERN_C +# define END_EXTERN_C +#endif + #endif /* PRINT_FILE_VAR_H */