From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Tom Tromey <tromey@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
Date: Tue, 27 Aug 2019 09:54:00 -0000 [thread overview]
Message-ID: <20190827095357.GO6076@embecosm.com> (raw)
In-Reply-To: <20190821103225.GF6076@embecosm.com>
* Andrew Burgess <andrew.burgess@embecosm.com> [2019-08-21 11:32:25 +0100]:
> * Tom Tromey <tromey@adacore.com> [2019-08-01 11:04:08 -0600]:
>
> > This changes basic_lookup_symbol_nonlocal to look in the global block
> > of the passed-in block. If no block was passed in, it reverts to the
> > previous behavior.
> >
> > This change is needed to ensure that 'FILENAME'::NAME lookups work
> > properly. As debugging Pedro's test case showed, this was not working
> > properly in the case where multiple identical names could be found
> > (the one situation where this feature is truly needed :-).
> >
> > This also removes some old comments from basic_lookup_symbol_nonlocal
> > that no longer apply once this patch goes in.
>
> So I guess the tests for this are going to be in the
> gdb.base/print-file-var.exp changes that are part of patch #8. It
> would be great if the commit message could mention this - it just
> makes life easier later on.
>
> I wonder if we need to update other *_lookup_symbol_nonlocal functions
> in a similar way? For example can the C tests be compiled as C++,
> which should cause GDB to use cp_lookup_symbol_nonlocal.
>
> Looking at both basic_lookup_symbol_nonlocal and
> cp_lookup_symbol_nonlocal, I wonder if your fix could be moved into
> lookup_global_symbol? And just have 'block_global_block (block)'
> checked before the search of all global blocks?
The patch below applies on top of this series and extends
gdb.base/print-file-var.exp to compile as both C++ and C. The C++
will fail initially.
The patch also includes a proposed fix to move the searching of the
"current" global block into lookup_global_symbol, after this both the
C++ and C versions of the test pass, and there are no other test
regressions.
This patch is going to clash with Christian Biesinger's patch to
refactor lookup_global_symbol, but hopefully merging these two
shouldn't be that hard.
I also tweaked the test to remove some duplicate test names.
What do you think?
Thanks,
Andrew
---
diff --git a/gdb/symtab.c b/gdb/symtab.c
index bd6fa35db6a..63a39e2996a 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2426,22 +2426,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
@@ -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)
{
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-27 9:54 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 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:04 ` [PATCH v2 1/8] Change SYMBOL_VALUE_ADDRESS to be an rvalue 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 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 3/8] Back out earlier Ada exception change 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:12 ` [PATCH v2 5/8] Don't call decode_line_with_current_source from select_source_symtab Tom Tromey
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 [this message]
2019-08-27 18:17 ` Christian Biesinger via gdb-patches
2019-08-28 12:34 ` Andrew Burgess
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=20190827095357.GO6076@embecosm.com \
--to=andrew.burgess@embecosm.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