diff mbox

[v2,4/8] Search global block from basic_lookup_symbol_nonlocal

Message ID 20190828123440.GU6076@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess Aug. 28, 2019, 12:34 p.m. UTC
* 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 mbox

Patch

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 */