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

Message ID 20190801170412.5553-5-tromey@adacore.com
State New
Headers show
Series
  • Handle copy relocations and add $_ada_exception
Related show

Commit Message

Tom Tromey Aug. 1, 2019, 5:04 p.m.
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.

gdb/ChangeLog
2019-08-01  Tom Tromey  <tromey@adacore.com>

	* symtab.c (basic_lookup_symbol_nonlocal): Search global block.
	Remove old comments.
---
 gdb/ChangeLog |  5 +++++
 gdb/symtab.c  | 44 ++++++++++++++++----------------------------
 2 files changed, 21 insertions(+), 28 deletions(-)

-- 
2.20.1

Comments

Andrew Burgess Aug. 21, 2019, 10:32 a.m. | #1
* 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?

Some other languages provide their own *_lookup_symbol_nonlocal, I
don't know if these would also need fixing.

Thanks,
Andrew


> 

> gdb/ChangeLog

> 2019-08-01  Tom Tromey  <tromey@adacore.com>

> 

> 	* symtab.c (basic_lookup_symbol_nonlocal): Search global block.

> 	Remove old comments.

> ---

>  gdb/ChangeLog |  5 +++++

>  gdb/symtab.c  | 44 ++++++++++++++++----------------------------

>  2 files changed, 21 insertions(+), 28 deletions(-)

> 

> diff --git a/gdb/symtab.c b/gdb/symtab.c

> index 0ff212e0d97..b8f33509c09 100644

> --- a/gdb/symtab.c

> +++ b/gdb/symtab.c

> @@ -2417,34 +2417,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,

>  {

>    struct block_symbol result;

>  

> -  /* NOTE: carlton/2003-05-19: The comments below were written when

> -     this (or what turned into this) was part of lookup_symbol_aux;

> -     I'm much less worried about these questions now, since these

> -     decisions have turned out well, but I leave these comments here

> -     for posterity.  */

> -

> -  /* NOTE: carlton/2002-12-05: There is a question as to whether or

> -     not it would be appropriate to search the current global block

> -     here as well.  (That's what this code used to do before the

> -     is_a_field_of_this check was moved up.)  On the one hand, it's

> -     redundant with the lookup in all objfiles search that happens

> -     next.  On the other hand, if decode_line_1 is passed an argument

> -     like filename:var, then the user presumably wants 'var' to be

> -     searched for in filename.  On the third hand, there shouldn't be

> -     multiple global variables all of which are named 'var', and it's

> -     not like decode_line_1 has ever restricted its search to only

> -     global variables in a single filename.  All in all, only

> -     searching the static block here seems best: it's correct and it's

> -     cleanest.  */

> -

> -  /* NOTE: carlton/2002-12-05: There's also a possible performance

> -     issue here: if you usually search for global symbols in the

> -     current file, then it would be slightly better to search the

> -     current global block before searching all the symtabs.  But there

> -     are other factors that have a much greater effect on performance

> -     than that one, so I don't think we should worry about that for

> -     now.  */

> -

>    /* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip

>       the current objfile.  Searching the current objfile first is useful

>       for both matching user expectations as well as performance.  */

> @@ -2453,6 +2425,22 @@ 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

> -- 

> 2.20.1

>
Andrew Burgess Aug. 27, 2019, 9:53 a.m. | #2
* 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 */
Ali Tamur via gdb-patches Aug. 27, 2019, 6:16 p.m. | #3
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/)

Christian
Andrew Burgess Aug. 28, 2019, 12:34 p.m. | #4
* 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 */

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0ff212e0d97..b8f33509c09 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2417,34 +2417,6 @@  basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
 {
   struct block_symbol result;
 
-  /* NOTE: carlton/2003-05-19: The comments below were written when
-     this (or what turned into this) was part of lookup_symbol_aux;
-     I'm much less worried about these questions now, since these
-     decisions have turned out well, but I leave these comments here
-     for posterity.  */
-
-  /* NOTE: carlton/2002-12-05: There is a question as to whether or
-     not it would be appropriate to search the current global block
-     here as well.  (That's what this code used to do before the
-     is_a_field_of_this check was moved up.)  On the one hand, it's
-     redundant with the lookup in all objfiles search that happens
-     next.  On the other hand, if decode_line_1 is passed an argument
-     like filename:var, then the user presumably wants 'var' to be
-     searched for in filename.  On the third hand, there shouldn't be
-     multiple global variables all of which are named 'var', and it's
-     not like decode_line_1 has ever restricted its search to only
-     global variables in a single filename.  All in all, only
-     searching the static block here seems best: it's correct and it's
-     cleanest.  */
-
-  /* NOTE: carlton/2002-12-05: There's also a possible performance
-     issue here: if you usually search for global symbols in the
-     current file, then it would be slightly better to search the
-     current global block before searching all the symtabs.  But there
-     are other factors that have a much greater effect on performance
-     than that one, so I don't think we should worry about that for
-     now.  */
-
   /* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip
      the current objfile.  Searching the current objfile first is useful
      for both matching user expectations as well as performance.  */
@@ -2453,6 +2425,22 @@  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