From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Christian Biesinger <cbiesinger@google.com>
Cc: Tom Tromey <tromey@adacore.com>,
gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
Date: Wed, 28 Aug 2019 12:34:00 -0000 [thread overview]
Message-ID: <20190828123440.GU6076@embecosm.com> (raw)
In-Reply-To: <CAPTJ0XF2shJX6PJOMd9se3rVWn1ZMr0Vcwi0KPSGH6e-idWiNg@mail.gmail.com>
* Christian Biesinger <cbiesinger@google.com> [2019-08-27 13:16:37 -0500]:
> On Tue, Aug 27, 2019 at 4:54 AM Andrew Burgess
> <andrew.burgess@embecosm.com> 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 <andrew.burgess@embecosm.com>
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 <andrew.burgess@embecosm.com>
+
+ * symtab.c (basic_lookup_symbol_nonlocal): Move check of global
+ block from here, to...
+ (lookup_global_symbol): ...here.
+
2019-08-21 Tom Tromey <tromey@adacore.com>
* 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 <andrew.burgess@embecosm.com>
+
+ * 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 <andrew.burgess@embecosm.com>
* 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 */
next prev parent reply other threads:[~2019-08-28 12:34 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
2019-08-01 17:04 ` [PATCH v2 7/8] Add $_ada_exception convenience variable Tom Tromey
2019-08-01 17:56 ` Eli Zaretskii
2019-09-20 19:16 ` Tom Tromey
2019-08-27 9:47 ` Andrew Burgess
2019-09-20 19:15 ` Tom Tromey
2019-08-01 17:04 ` [PATCH v2 6/8] Make current_source_* per-program-space Tom Tromey
2019-08-20 15:57 ` Andrew Burgess
2019-08-21 19:05 ` Tom Tromey
2019-08-01 17:04 ` [PATCH v2 3/8] Back out earlier Ada exception change Tom Tromey
2019-08-01 17:04 ` [PATCH v2 2/8] Handle copy relocations Tom Tromey
2019-08-21 15:35 ` Andrew Burgess
2019-08-21 19:11 ` Tom Tromey
2019-08-22 8:41 ` Andrew Burgess
2019-09-19 17:45 ` Tom Tromey
2019-09-19 18:28 ` Tom Tromey
2019-09-19 18:40 ` Andrew Burgess
2019-08-01 17:04 ` [PATCH v2 1/8] Change SYMBOL_VALUE_ADDRESS to be an rvalue Tom Tromey
2019-08-01 17:04 ` [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol Tom Tromey
2019-08-20 13:41 ` Andrew Burgess
2019-08-21 14:35 ` Andrew Burgess
2019-08-01 17:12 ` [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal Tom Tromey
2019-08-21 10:32 ` Andrew Burgess
2019-08-27 9:54 ` Andrew Burgess
2019-08-27 18:17 ` Christian Biesinger via gdb-patches
2019-08-28 12:34 ` Andrew Burgess [this message]
2019-08-01 17:12 ` [PATCH v2 5/8] Don't call decode_line_with_current_source from select_source_symtab Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190828123440.GU6076@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=cbiesinger@google.com \
--cc=gdb-patches@sourceware.org \
--cc=tromey@adacore.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox