[PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal
Andrew Burgess
andrew.burgess@embecosm.com
Tue Aug 27 09:54:00 GMT 2019
* 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 */
More information about the Gdb-patches
mailing list