[v2] symlookup: improves symbol lookup when a file is specified.

Message ID 1507727728-30540-1-git-send-email-walfred.tedeschi@intel.com
State New, archived
Headers

Commit Message

Walfred Tedeschi Oct. 11, 2017, 1:15 p.m. UTC
  The provided patch adds a scope lookup with higher priority in the case a
a file is provided for the evaluation.

Usual symbol lookup from GDB follows linker logic.  This is what is
desired most of the time.  In the usage case presented a full qualified scope
is provided provided, so the lookup has to follow this priority.  Failing in
finding the symbol at this scope usual path is followed.

As use and test case it is presented two shared objects having a global
variable with the same name but comming from different source files.
Without the patch evaluating those variables providing the file scope
returns the wrong value;  It returns the value seen in the first loaded
shared object. Using the patch the value defined in the file scope is
the one returned.

One of the already existing test cases in print-file-var.exp starts to
fail after aplying this patch.  In fact the value that the linker
sees is different then the one debugger can read from the shared object.
In this sense it looks like to me that the test has to be changed.
The fail is in the call:
  print 'print-file-var-lib2.c'::this_version_id == v2

During load of the libraries linker resolves the symbol as the first one
loaded and independent of the scope the symbol will have the same value,
as defined in the first library loaded.
However, when defining the scope the value should represent the value
in that context. Diferent evaluations of the same symbols might also better
spot the issue in users code.

- I haven't changed the test because I wanted to hear the community
thought on the subject.



2017-07-13  Walfred Tedeschi  <walfred.tedeschi@intel.com>

gdb/ChangeLog:

	* symtab.c (lookup_global_symbol): Add new lookup to ensure
	priority on given block.

gdb/testsuite/ChangeLog:

	* gdb.base/print-file-var-dlopen-main.c: New file.
	* gdb.base/print-file-var-dlopen.exp: New test based on
	print-file-var.exp.
	* gdb.base/print-file-var-dlopen-lib1.c: New file.
	* gdb.base/print-file-var-dlopen-lib2.c: New file.


---
 gdb/symtab.c                                       |   4 +
 .../gdb.base/print-file-var-dlopen-lib1.c          |  25 +++++
 .../gdb.base/print-file-var-dlopen-lib2.c          |  25 +++++
 .../gdb.base/print-file-var-dlopen-main.c          |  61 +++++++++++
 gdb/testsuite/gdb.base/print-file-var-dlopen.exp   | 113 +++++++++++++++++++++
 5 files changed, 228 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
 create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
 create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
 create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen.exp
  

Comments

Walfred Tedeschi Oct. 16, 2017, 1:23 p.m. UTC | #1
Hi all,

> +set lib_opts debug
> +set exec_opts [list debug shlib_load
> +additional_flags=-DSHLIB_NAME=\"${lib_dlopen1}\"
> +additional_flags=-DSHLIB_NAME2=\"${lib_dlopen2}\"]
> +
> +if { [gdb_compile_shlib $libsrc1 $libobj1 $lib_opts] != ""
> +     || [gdb_compile_shlib $libsrc2 $libobj2 $lib_opts] != ""
> +     || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
> +    untested "failed to compile"
> +    return -1
> +}

I have revmoved the code below from my sandbox.
Next review will not have that.

> +
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
> +                        ${libobj1} \
> +                        ${lib_opts} ] != "" } {
> +    return -1
> +}
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
> +                        ${libobj2} \
> +                        ${lib_opts} ] != "" } {
> +    return -1
> +}
> +if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
> +                  [standard_output_file ${executable}] \
> +                  executable \
> +                  [list debug shlib=-ldl]]
> +     != ""} {
> +    return -1
> +}
> +

Would it help to have a branch with those changes?

Thanks and regards,
/Fred

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  
Simon Marchi Oct. 16, 2017, 5:06 p.m. UTC | #2
On 2017-10-11 09:15 AM, Walfred Tedeschi wrote:
> The provided patch adds a scope lookup with higher priority in the case a
> a file is provided for the evaluation.
> 
> Usual symbol lookup from GDB follows linker logic.  This is what is
> desired most of the time.  In the usage case presented a full qualified scope
> is provided provided, so the lookup has to follow this priority.  Failing in

"provided provided"

> finding the symbol at this scope usual path is followed.
> 
> As use and test case it is presented two shared objects having a global
> variable with the same name but comming from different source files.
> Without the patch evaluating those variables providing the file scope
> returns the wrong value;  It returns the value seen in the first loaded
> shared object. Using the patch the value defined in the file scope is
> the one returned.
> 
> One of the already existing test cases in print-file-var.exp starts to
> fail after aplying this patch.  In fact the value that the linker
> sees is different then the one debugger can read from the shared object.
> In this sense it looks like to me that the test has to be changed.
> The fail is in the call:
>   print 'print-file-var-lib2.c'::this_version_id == v2
> 
> During load of the libraries linker resolves the symbol as the first one
> loaded and independent of the scope the symbol will have the same value,
> as defined in the first library loaded.
> However, when defining the scope the value should represent the value
> in that context. Diferent evaluations of the same symbols might also better
> spot the issue in users code.
> 
> - I haven't changed the test because I wanted to hear the community
> thought on the subject.

I can't really comment on how right the fix is, but I can see that the current
behavior is clearly wrong:

(gdb) p 'print-file-var-dlopen-lib1.c'::this_version_id
$1 = 104
(gdb) p 'print-file-var-dlopen-lib2.c'::this_version_id
$2 = 104

And that your patch fixes it:

(gdb) p 'print-file-var-dlopen-lib1.c'::this_version_id
$1 = 104
(gdb) p 'print-file-var-dlopen-lib2.c'::this_version_id
$2 = 203

> 2017-07-13  Walfred Tedeschi  <walfred.tedeschi@intel.com>
> 
> gdb/ChangeLog:
> 
> 	* symtab.c (lookup_global_symbol): Add new lookup to ensure
> 	priority on given block.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/print-file-var-dlopen-main.c: New file.
> 	* gdb.base/print-file-var-dlopen.exp: New test based on
> 	print-file-var.exp.
> 	* gdb.base/print-file-var-dlopen-lib1.c: New file.
> 	* gdb.base/print-file-var-dlopen-lib2.c: New file.
> 
> 
> ---
>  gdb/symtab.c                                       |   4 +
>  .../gdb.base/print-file-var-dlopen-lib1.c          |  25 +++++
>  .../gdb.base/print-file-var-dlopen-lib2.c          |  25 +++++
>  .../gdb.base/print-file-var-dlopen-main.c          |  61 +++++++++++
>  gdb/testsuite/gdb.base/print-file-var-dlopen.exp   | 113 +++++++++++++++++++++
>  5 files changed, 228 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
>  create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
>  create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
>  create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen.exp
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 16a6b2e..a2c307f 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2589,6 +2589,10 @@ lookup_global_symbol (const char *name,
>    if (objfile != NULL)
>      result = solib_global_lookup (objfile, name, domain);
>  
> +  /* We still need to look on the global scope of current object file.  */
> +  if (result.symbol == NULL && objfile != NULL)
> +    result = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK, name, domain);
> +
>    /* 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-dlopen-lib1.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
> new file mode 100644
> index 0000000..09ec947
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +   Copyright 2012-2017 Free Software Foundation, Inc.

Are the years right here (the 2012)?

> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int this_version_id = 104;
> +
> +int
> +get_version (void)
> +{
> +  static int test;
> +  test = this_version_id;
> +  return test;
> +}
> diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
> new file mode 100644
> index 0000000..b097cd2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +   Copyright 2012-2017 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +int this_version_id = 203;
> +
> +int
> +get_version (void)
> +{
> +  static int test;
> +  test = this_version_id;
> +  return test;
> +}
> diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
> new file mode 100644
> index 0000000..954a64e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
> @@ -0,0 +1,61 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +   Copyright 2017 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <dlfcn.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +int
> +dummy (void)
> +{
> +  return 1;
> +}
> +
> +int
> +main (void)
> +{
> +  int (*get_version1) (void);
> +  int (*get_version2) (void);
> +  int v1, v2;
> +
> +  void *lib1 = dlopen ("print-file-var-dlopen-lib1.so", RTLD_LAZY);
> +  void *lib2 = dlopen ("print-file-var-dlopen-lib2.so", RTLD_LAZY);
> +
> +  if (lib1 == NULL || lib2 == NULL)
> +    return 1;
> +
> +  *(int **) (&get_version1) = dlsym (lib1, "get_version");
> +  *(int **) (&get_version2) = dlsym (lib2, "get_version");
> +
> +  if (get_version1 != NULL
> +      && get_version2 != NULL)
> +    {
> +      v1 = get_version1();
> +      v2 = get_version2();
> +    }
> +
> +
> +  if (v1 != 104 || v2 != 203)
> +    return 1;
> +
> +  dummy (); /* STOP  */
> +
> +  dlclose (lib1);
> +  dlclose (lib2);
> +
> +  return 0;
> +}
> +
> diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen.exp b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
> new file mode 100644
> index 0000000..1c331ba
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
> @@ -0,0 +1,113 @@
> +# Copyright 2017 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +if {[skip_shlib_tests]} {
> +    return 0
> +}
> +
> +set executable print-file-var-dlopen-main
> +
> +set lib1 "print-file-var-dlopen-lib1"
> +set lib2 "print-file-var-dlopen-lib2"
> +set libsrc1 $srcdir/$subdir/${lib1}.c
> +set libsrc2 $srcdir/$subdir/${lib2}.c
> +set libobj1 [standard_output_file ${lib1}.so]
> +set libobj2 [standard_output_file ${lib2}.so]
> +set lib_dlopen1 [shlib_target_file ${libobj1}]
> +set lib_dlopen2 [shlib_target_file ${libobj2}]
> +
> +set srcfile $srcdir/$subdir/$executable.c
> +set binfile [standard_output_file $executable]
> +set shlibdir [standard_output_file {}]
> +
> +set lib_opts debug
> +set exec_opts [list debug shlib_load additional_flags=-DSHLIB_NAME=\"${lib_dlopen1}\" additional_flags=-DSHLIB_NAME2=\"${lib_dlopen2}\"]
> +
> +if { [gdb_compile_shlib $libsrc1 $libobj1 $lib_opts] != ""
> +     || [gdb_compile_shlib $libsrc2 $libobj2 $lib_opts] != ""
> +     || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
> +    untested "failed to compile"
> +    return -1
> +}
> +
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
> +                        ${libobj1} \
> +                        ${lib_opts} ] != "" } {
> +    return -1
> +}
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
> +                        ${libobj2} \
> +                        ${lib_opts} ] != "" } {
> +    return -1
> +}
> +if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
> +                  [standard_output_file ${executable}] \
> +                  executable \
> +                  [list debug shlib=-ldl]]
> +     != ""} {
> +    return -1
> +}
> +
> +
> +clean_restart $executable
> +
> +if ![runto_main] {
> +    untested "could not run to main"
> +    return -1
> +}
> +
> +# Try printing "this_version_num" qualified with the name of the file
> +# where the variables are defined.  There are two global variables
> +# with that name, and some systems such as GNU/Linux merge them
> +# into one single entity, while some other systems such as Windows
> +# keep them separate.

I don't really understand what you mean by "GNU/Linux merge them into one
single entity".  There are still two distinct variables, no?

> +gdb_test "continue" \
> +         "Breakpoint \[0-9\]+, main \\(\\) at.*" \
> +         "continue to STOP marker"

You could use gdb_continue_to_breakpoint.

Simon
  
Walfred Tedeschi Oct. 17, 2017, 7:57 a.m. UTC | #3
Simon,

Thanks for your review!

Some comments below...

> From: Simon Marchi [mailto:simon.marchi@ericsson.com]

> Sent: Monday, October 16, 2017 7:06 PM

> To: Tedeschi, Walfred <walfred.tedeschi@intel.com>; palves@redhat.com;

> simon.marchi@polymtl.ca

> Cc: gdb-patches@sourceware.org

> Subject: Re: [PATCH v2] symlookup: improves symbol lookup when a file is

> specified.

> 

> On 2017-10-11 09:15 AM, Walfred Tedeschi wrote:

> > The provided patch adds a scope lookup with higher priority in the

> > case a a file is provided for the evaluation.

Looks like we have also here a double "a"

> >

> > Usual symbol lookup from GDB follows linker logic.  This is what is

> > desired most of the time.  In the usage case presented a full

> > qualified scope is provided provided, so the lookup has to follow this

> > priority.  Failing in

> 

> "provided provided"


Ok!

> 

> > finding the symbol at this scope usual path is followed.

> >

> > As use and test case it is presented two shared objects having a

> > global variable with the same name but comming from different source

> files.

> > Without the patch evaluating those variables providing the file scope

> > returns the wrong value;  It returns the value seen in the first

> > loaded shared object. Using the patch the value defined in the file

> > scope is the one returned.

> >

> > One of the already existing test cases in print-file-var.exp starts to

> > fail after aplying this patch.  In fact the value that the linker sees

> > is different then the one debugger can read from the shared object.

> > In this sense it looks like to me that the test has to be changed.

> > The fail is in the call:

> >   print 'print-file-var-lib2.c'::this_version_id == v2

> >

> > During load of the libraries linker resolves the symbol as the first

> > one loaded and independent of the scope the symbol will have the same

> > value, as defined in the first library loaded.

> > However, when defining the scope the value should represent the value

> > in that context. Diferent evaluations of the same symbols might also

> > better spot the issue in users code.

> >

> > - I haven't changed the test because I wanted to hear the community

> > thought on the subject.

> 

> I can't really comment on how right the fix is, but I can see that the current

> behavior is clearly wrong:

> 


I will add that in the commit message, I think it might make it clearer.

> (gdb) p 'print-file-var-dlopen-lib1.c'::this_version_id

> $1 = 104

> (gdb) p 'print-file-var-dlopen-lib2.c'::this_version_id

> $2 = 104

> 

> And that your patch fixes it:

> 

> (gdb) p 'print-file-var-dlopen-lib1.c'::this_version_id

> $1 = 104

> (gdb) p 'print-file-var-dlopen-lib2.c'::this_version_id

> $2 = 203

> 

> > 2017-07-13  Walfred Tedeschi  <walfred.tedeschi@intel.com>

> >

> > gdb/ChangeLog:

> >

> > 	* symtab.c (lookup_global_symbol): Add new lookup to ensure

> > 	priority on given block.

> >

> > gdb/testsuite/ChangeLog:

> >

> > 	* gdb.base/print-file-var-dlopen-main.c: New file.

> > 	* gdb.base/print-file-var-dlopen.exp: New test based on

> > 	print-file-var.exp.

> > 	* gdb.base/print-file-var-dlopen-lib1.c: New file.

> > 	* gdb.base/print-file-var-dlopen-lib2.c: New file.

> >

> >

> > ---

> >  gdb/symtab.c                                       |   4 +

> >  .../gdb.base/print-file-var-dlopen-lib1.c          |  25 +++++

> >  .../gdb.base/print-file-var-dlopen-lib2.c          |  25 +++++

> >  .../gdb.base/print-file-var-dlopen-main.c          |  61 +++++++++++

> >  gdb/testsuite/gdb.base/print-file-var-dlopen.exp   | 113

> +++++++++++++++++++++

> >  5 files changed, 228 insertions(+)

> >  create mode 100644

> > gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c

> >  create mode 100644

> > gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c

> >  create mode 100644

> > gdb/testsuite/gdb.base/print-file-var-dlopen-main.c

> >  create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen.exp

> >

> > diff --git a/gdb/symtab.c b/gdb/symtab.c index 16a6b2e..a2c307f 100644

> > --- a/gdb/symtab.c

> > +++ b/gdb/symtab.c

> > @@ -2589,6 +2589,10 @@ lookup_global_symbol (const char *name,

> >    if (objfile != NULL)

> >      result = solib_global_lookup (objfile, name, domain);

> >

> > +  /* We still need to look on the global scope of current object

> > + file.  */  if (result.symbol == NULL && objfile != NULL)

> > +    result = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK, name,

> > + domain);

> > +

> >    /* 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-dlopen-lib1.c

> > b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c

> > new file mode 100644

> > index 0000000..09ec947

> > --- /dev/null

> > +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c

> > @@ -0,0 +1,25 @@

> > +/* This testcase is part of GDB, the GNU debugger.

> > +   Copyright 2012-2017 Free Software Foundation, Inc.

> 

> Are the years right here (the 2012)?



I have improved a test case from 2012 to do this one.

> 

> > +

> > +   This program is free software; you can redistribute it and/or modify

> > +   it under the terms of the GNU General Public License as published by

> > +   the Free Software Foundation; either version 3 of the License, or

> > +   (at your option) any later version.

> > +

> > +   This program is distributed in the hope that it will be useful,

> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> > +   GNU General Public License for more details.

> > +

> > +   You should have received a copy of the GNU General Public License

> > +   along with this program.  If not, see

> > + <http://www.gnu.org/licenses/>.  */

> > +

> > +int this_version_id = 104;

> > +

> > +int

> > +get_version (void)

> > +{

> > +  static int test;

> > +  test = this_version_id;

> > +  return test;

> > +}

> > diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c

> > b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c

> > new file mode 100644

> > index 0000000..b097cd2

> > --- /dev/null

> > +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c

> > @@ -0,0 +1,25 @@

> > +/* This testcase is part of GDB, the GNU debugger.

> > +   Copyright 2012-2017 Free Software Foundation, Inc.

> > +

> > +   This program is free software; you can redistribute it and/or modify

> > +   it under the terms of the GNU General Public License as published by

> > +   the Free Software Foundation; either version 3 of the License, or

> > +   (at your option) any later version.

> > +

> > +   This program is distributed in the hope that it will be useful,

> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> > +   GNU General Public License for more details.

> > +

> > +   You should have received a copy of the GNU General Public License

> > +   along with this program.  If not, see

> > + <http://www.gnu.org/licenses/>.  */

> > +

> > +int this_version_id = 203;

> > +

> > +int

> > +get_version (void)

> > +{

> > +  static int test;

> > +  test = this_version_id;

> > +  return test;

> > +}

> > diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c

> > b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c

> > new file mode 100644

> > index 0000000..954a64e

> > --- /dev/null

> > +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c

> > @@ -0,0 +1,61 @@

> > +/* This testcase is part of GDB, the GNU debugger.

> > +   Copyright 2017 Free Software Foundation, Inc.

> > +

> > +   This program is free software; you can redistribute it and/or modify

> > +   it under the terms of the GNU General Public License as published by

> > +   the Free Software Foundation; either version 3 of the License, or

> > +   (at your option) any later version.

> > +

> > +   This program is distributed in the hope that it will be useful,

> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

> > +   GNU General Public License for more details.

> > +

> > +   You should have received a copy of the GNU General Public License

> > +   along with this program.  If not, see

> > + <http://www.gnu.org/licenses/>.  */

> > +

> > +#include <dlfcn.h>

> > +#include <stdio.h>

> > +#include <stdlib.h>

> > +

> > +int

> > +dummy (void)

> > +{

> > +  return 1;

> > +}

> > +

> > +int

> > +main (void)

> > +{

> > +  int (*get_version1) (void);

> > +  int (*get_version2) (void);

> > +  int v1, v2;

> > +

> > +  void *lib1 = dlopen ("print-file-var-dlopen-lib1.so", RTLD_LAZY);

> > + void *lib2 = dlopen ("print-file-var-dlopen-lib2.so", RTLD_LAZY);

> > +

> > +  if (lib1 == NULL || lib2 == NULL)

> > +    return 1;

> > +

> > +  *(int **) (&get_version1) = dlsym (lib1, "get_version");  *(int **)

> > + (&get_version2) = dlsym (lib2, "get_version");

> > +

> > +  if (get_version1 != NULL

> > +      && get_version2 != NULL)

> > +    {

> > +      v1 = get_version1();

> > +      v2 = get_version2();

> > +    }

> > +

> > +

> > +  if (v1 != 104 || v2 != 203)

> > +    return 1;

> > +

> > +  dummy (); /* STOP  */

> > +

> > +  dlclose (lib1);

> > +  dlclose (lib2);

> > +

> > +  return 0;

> > +}

> > +

> > diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen.exp

> > b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp

> > new file mode 100644

> > index 0000000..1c331ba

> > --- /dev/null

> > +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp

> > @@ -0,0 +1,113 @@

> > +# Copyright 2017 Free Software Foundation, Inc.

> > +

> > +# This program is free software; you can redistribute it and/or

> > +modify # it under the terms of the GNU General Public License as

> > +published by # the Free Software Foundation; either version 3 of the

> > +License, or # (at your option) any later version.

> > +#

> > +# This program is distributed in the hope that it will be useful, #

> > +but WITHOUT ANY WARRANTY; without even the implied warranty of #

> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the #

> GNU

> > +General Public License for more details.

> > +#

> > +# You should have received a copy of the GNU General Public License #

> > +along with this program.  If not, see <http://www.gnu.org/licenses/>.

> > +*/

> > +

> > +if {[skip_shlib_tests]} {

> > +    return 0

> > +}

> > +

> > +set executable print-file-var-dlopen-main

> > +

> > +set lib1 "print-file-var-dlopen-lib1"

> > +set lib2 "print-file-var-dlopen-lib2"

> > +set libsrc1 $srcdir/$subdir/${lib1}.c set libsrc2

> > +$srcdir/$subdir/${lib2}.c set libobj1 [standard_output_file

> > +${lib1}.so] set libobj2 [standard_output_file ${lib2}.so] set

> > +lib_dlopen1 [shlib_target_file ${libobj1}] set lib_dlopen2

> > +[shlib_target_file ${libobj2}]

> > +

> > +set srcfile $srcdir/$subdir/$executable.c set binfile

> > +[standard_output_file $executable] set shlibdir [standard_output_file

> > +{}]

> > +

> > +set lib_opts debug

> > +set exec_opts [list debug shlib_load

> > +additional_flags=-DSHLIB_NAME=\"${lib_dlopen1}\"

> > +additional_flags=-DSHLIB_NAME2=\"${lib_dlopen2}\"]

> > +

> > +if { [gdb_compile_shlib $libsrc1 $libobj1 $lib_opts] != ""

> > +     || [gdb_compile_shlib $libsrc2 $libobj2 $lib_opts] != ""

> > +     || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {

> > +    untested "failed to compile"

> > +    return -1

> > +}

> > +

> > +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \

> > +                        ${libobj1} \

> > +                        ${lib_opts} ] != "" } {

> > +    return -1

> > +}

> > +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \

> > +                        ${libobj2} \

> > +                        ${lib_opts} ] != "" } {

> > +    return -1

> > +}

> > +if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \

> > +                  [standard_output_file ${executable}] \

> > +                  executable \

> > +                  [list debug shlib=-ldl]]

> > +     != ""} {

> > +    return -1

> > +}

> > +

> > +

> > +clean_restart $executable

> > +

> > +if ![runto_main] {

> > +    untested "could not run to main"

> > +    return -1

> > +}

> > +

> > +# Try printing "this_version_num" qualified with the name of the file

> > +# where the variables are defined.  There are two global variables #

> > +with that name, and some systems such as GNU/Linux merge them # into

> > +one single entity, while some other systems such as Windows # keep

> > +them separate.

> 

> I don't really understand what you mean by "GNU/Linux merge them into

> one single entity".  There are still two distinct variables, no?


This comment comes from the other test, we do not need it anymore.

> 

> > +gdb_test "continue" \

> > +         "Breakpoint \[0-9\]+, main \\(\\) at.*" \

> > +         "continue to STOP marker"

> 

> You could use gdb_continue_to_breakpoint.


Thank you!

> 

> Simon


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 16a6b2e..a2c307f 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2589,6 +2589,10 @@  lookup_global_symbol (const char *name,
   if (objfile != NULL)
     result = solib_global_lookup (objfile, name, domain);
 
+  /* We still need to look on the global scope of current object file.  */
+  if (result.symbol == NULL && objfile != NULL)
+    result = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK, name, domain);
+
   /* 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-dlopen-lib1.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
new file mode 100644
index 0000000..09ec947
--- /dev/null
+++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
@@ -0,0 +1,25 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+   Copyright 2012-2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int this_version_id = 104;
+
+int
+get_version (void)
+{
+  static int test;
+  test = this_version_id;
+  return test;
+}
diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
new file mode 100644
index 0000000..b097cd2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
@@ -0,0 +1,25 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+   Copyright 2012-2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int this_version_id = 203;
+
+int
+get_version (void)
+{
+  static int test;
+  test = this_version_id;
+  return test;
+}
diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
new file mode 100644
index 0000000..954a64e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
@@ -0,0 +1,61 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+   Copyright 2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <dlfcn.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int
+dummy (void)
+{
+  return 1;
+}
+
+int
+main (void)
+{
+  int (*get_version1) (void);
+  int (*get_version2) (void);
+  int v1, v2;
+
+  void *lib1 = dlopen ("print-file-var-dlopen-lib1.so", RTLD_LAZY);
+  void *lib2 = dlopen ("print-file-var-dlopen-lib2.so", RTLD_LAZY);
+
+  if (lib1 == NULL || lib2 == NULL)
+    return 1;
+
+  *(int **) (&get_version1) = dlsym (lib1, "get_version");
+  *(int **) (&get_version2) = dlsym (lib2, "get_version");
+
+  if (get_version1 != NULL
+      && get_version2 != NULL)
+    {
+      v1 = get_version1();
+      v2 = get_version2();
+    }
+
+
+  if (v1 != 104 || v2 != 203)
+    return 1;
+
+  dummy (); /* STOP  */
+
+  dlclose (lib1);
+  dlclose (lib2);
+
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen.exp b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
new file mode 100644
index 0000000..1c331ba
--- /dev/null
+++ b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
@@ -0,0 +1,113 @@ 
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+set executable print-file-var-dlopen-main
+
+set lib1 "print-file-var-dlopen-lib1"
+set lib2 "print-file-var-dlopen-lib2"
+set libsrc1 $srcdir/$subdir/${lib1}.c
+set libsrc2 $srcdir/$subdir/${lib2}.c
+set libobj1 [standard_output_file ${lib1}.so]
+set libobj2 [standard_output_file ${lib2}.so]
+set lib_dlopen1 [shlib_target_file ${libobj1}]
+set lib_dlopen2 [shlib_target_file ${libobj2}]
+
+set srcfile $srcdir/$subdir/$executable.c
+set binfile [standard_output_file $executable]
+set shlibdir [standard_output_file {}]
+
+set lib_opts debug
+set exec_opts [list debug shlib_load additional_flags=-DSHLIB_NAME=\"${lib_dlopen1}\" additional_flags=-DSHLIB_NAME2=\"${lib_dlopen2}\"]
+
+if { [gdb_compile_shlib $libsrc1 $libobj1 $lib_opts] != ""
+     || [gdb_compile_shlib $libsrc2 $libobj2 $lib_opts] != ""
+     || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
+    untested "failed to compile"
+    return -1
+}
+
+if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
+                        ${libobj1} \
+                        ${lib_opts} ] != "" } {
+    return -1
+}
+if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
+                        ${libobj2} \
+                        ${lib_opts} ] != "" } {
+    return -1
+}
+if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
+                  [standard_output_file ${executable}] \
+                  executable \
+                  [list debug shlib=-ldl]]
+     != ""} {
+    return -1
+}
+
+
+clean_restart $executable
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+# Try printing "this_version_num" qualified with the name of the file
+# where the variables are defined.  There are two global variables
+# with that name, and some systems such as GNU/Linux merge them
+# into one single entity, while some other systems such as Windows
+# keep them separate.  In the first situation, we have to verify
+# that GDB does not randomly select the wrong instance, even when
+# a specific filename is used to qualified the lookup.  And in the
+# second case, we have to verify that GDB does select the instance
+# defined in the given filename.
+#
+# To avoid adding target-specific code in this testcase, the program
+# sets two local variable named 'v1' and 'v2' with the value of
+# our global variables.  This allows us to compare the value that
+# GDB returns for each query against the actual value seen by
+# the program itself.
+
+# Get past the initialization of variables 'v1' and 'v2'.
+
+set bp_location \
+    [gdb_get_line_number "STOP" "${executable}.c"]
+gdb_test "break $executable.c:$bp_location" \
+         "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
+         "breapoint past v1 & v2 initialization"
+
+gdb_test "continue" \
+         "Breakpoint \[0-9\]+, main \\(\\) at.*" \
+         "continue to STOP marker"
+
+# Now check the value of this_version_id in both print-file-var-lib1.c
+# and print-file-var-lib2.c.
+
+gdb_test "print 'print-file-var-dlopen-lib1.c'::this_version_id == v1" \
+         " = 1"
+
+gdb_test "print 'print-file-var-dlopen-lib2.c'::this_version_id == v2" \
+         " = 1"
+
+gdb_test "print 'print-file-var-dlopen-lib2.c'::get_version::test == v2" \
+         " = 1"
+
+gdb_test "print 'print-file-var-dlopen-lib1.c'::get_version::test == v1" \
+         " = 1"
+