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

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

[Walfred Tedeschi]

The provided patch adds a scope lookup with higher priority in the case
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, 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.  Without using this patch the result of the evaluations
are:

	print 'print-file-var-lib1.c'::this_version_id
	$1 = 104

and

	print 'print-file-var-lib2.c'::this_version_id
	$2 = 104  # This value is wrong we have set 203 in the code


With the patch applied we get:

	print 'print-file-var-lib1.c'::this_version_id
	$1 = 104

and

	print 'print-file-var-lib2.c'::this_version_id
	$2 = 203  # Now value is correct



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

in this case v1 and v2 are the same and equal to the this_version_id
defined in print-file-var-lib1.c.  Test is changed to assure that
when the scope operator is used the value seen in print-file-var-lib2.c
is the one GDB evaluates, i.e. 203.

*  Reading again Simon's comments I have decided to take a quick look on
the failing test.  I have fixed it according to the logic of the current
patch.  

2017-10-18  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.base/print-file-var.exp: Modify expected result for
	evaluating 'print-file-var-lib2.c'::this_version_id.

---
 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   | 90 ++++++++++++++++++++++
 gdb/testsuite/gdb.base/print-file-var.exp          |  4 +-
 6 files changed, 208 insertions(+), 1 deletion(-)
 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.
+
+   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..9c3c0e6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
@@ -0,0 +1,90 @@
+# 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
+}
+
+clean_restart $executable
+
+if ![runto_main] {
+    untested "could not run to main"
+    return -1
+}
+
+# Create to shared libraries having the symbol "this_version_id" defined in both
+# libraries global scope.  Main program will dllopen one by one and evaluate the
+# symbol "this_version_id" via a function provided by the library assigning the value of
+# "this_version_id" to V1 and V2.
+# Using the scope to perform the evaluations should return the value
+# defined in the c file.
+
+# 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_continue_to_breakpoint "continue to the STOP"
+
+# 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"
+
diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index 223a67d..ae5071a 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -88,5 +88,7 @@ gdb_test "continue" \
 gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \
          " = 1"
 
-gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
+# Independent of the linker value seen in the second library should
+# be 203.
+gdb_test "print 'print-file-var-lib2.c'::this_version_id == 203" \
          " = 1"
-- 
2.5.5

Re: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Simon Marchi]

Hi,

Just a nit, git shows this when applying your patch:

Applying: symlookup: improves symbol lookup when a file is specified.
.git/rebase-apply/patch:157: new blank line at EOF.
+
.git/rebase-apply/patch:253: new blank line at EOF.
+
warning: 2 lines add whitespace errors.

If there are extra blank lines at the end of the new files, you can 
remove them.

On 2017-10-18 05:01, Walfred Tedeschi wrote:
> The provided patch adds a scope lookup with higher priority in the case
> 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, 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.  Without using this patch the result of the 
> evaluations
> are:
> 
> 	print 'print-file-var-lib1.c'::this_version_id
> 	$1 = 104
> 
> and
> 
> 	print 'print-file-var-lib2.c'::this_version_id
> 	$2 = 104  # This value is wrong we have set 203 in the code
> 
> 
> With the patch applied we get:
> 
> 	print 'print-file-var-lib1.c'::this_version_id
> 	$1 = 104
> 
> and
> 
> 	print 'print-file-var-lib2.c'::this_version_id
> 	$2 = 203  # Now value is correct
> 
> 
> 
> 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
> 
> in this case v1 and v2 are the same and equal to the this_version_id
> defined in print-file-var-lib1.c.  Test is changed to assure that
> when the scope operator is used the value seen in print-file-var-lib2.c
> is the one GDB evaluates, i.e. 203.
> 
> *  Reading again Simon's comments I have decided to take a quick look 
> on
> the failing test.  I have fixed it according to the logic of the 
> current
> patch.
> 
> 2017-10-18  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.base/print-file-var.exp: Modify expected result for
> 	evaluating 'print-file-var-lib2.c'::this_version_id.
> 
> ---
>  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   | 90 
> ++++++++++++++++++++++
>  gdb/testsuite/gdb.base/print-file-var.exp          |  4 +-
>  6 files changed, 208 insertions(+), 1 deletion(-)
>  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.
> +
> +   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..9c3c0e6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
> @@ -0,0 +1,90 @@
> +# 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/>.  */
> +

Could you add here a one sentence description of what this test file is 
about?  When looking at test files, it is nice to have a quick summary 
to know what it is testing.

> +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 {}]

shlibdir seems unused.

> +
> +set lib_opts debug
> +set exec_opts [list debug shlib_load
> additional_flags=-DSHLIB_NAME=\"${lib_dlopen1}\" \
> +additional_flags=-DSHLIB_NAME2=\"${lib_dlopen2}\"]

The flags definied SHLIB_NAME and SHLIB_NAME2 are not useful, I think.

> +
> +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
> +}
> +
> +clean_restart $executable
> +
> +if ![runto_main] {
> +    untested "could not run to main"
> +    return -1
> +}
> +
> +# Create to shared libraries having the symbol "this_version_id"
> defined in both

"to" -> "two"

> +# libraries global scope.  Main program will dllopen one by one and
> evaluate the

"dllopen" -> "dlopen"

> +# symbol "this_version_id" via a function provided by the library
> assigning the value of
> +# "this_version_id" to V1 and V2.
> +# Using the scope to perform the evaluations should return the value
> +# defined in the c file.
> +
> +# 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_continue_to_breakpoint "continue to the STOP"
> +
> +# Now check the value of this_version_id in both print-file-var-lib1.c
> +# and print-file-var-lib2.c.

The filenames mentioned here are wrong.  It's probably simpler to say 
"... in both libraries".

> +
> +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"
> +
> diff --git a/gdb/testsuite/gdb.base/print-file-var.exp
> b/gdb/testsuite/gdb.base/print-file-var.exp
> index 223a67d..ae5071a 100644
> --- a/gdb/testsuite/gdb.base/print-file-var.exp
> +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> @@ -88,5 +88,7 @@ gdb_test "continue" \
>  gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \
>           " = 1"
> 
> -gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
> +# Independent of the linker value seen in the second library should
> +# be 203.
> +gdb_test "print 'print-file-var-lib2.c'::this_version_id == 203" \
>           " = 1"

I am not convinced about changing this test.  Normally, we want the 
debugger, when evaluating expressions to act as close as possible to the 
target program.  If the value of this_version_id read in lib2 was 104, 
then that's probably what print 'print-file-var-lib2.c'::this_version_id 
should show as well, don't you think?  This test is checking that this 
is the case.

Simon

RE: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Tedeschi, Walfred]

> -----Original Message-----
> From: Simon Marchi [mailto:simon.marchi@polymtl.ca]
> Sent: Thursday, October 19, 2017 9:07 PM
> To: Tedeschi, Walfred <walfred.tedeschi@intel.com>
> Cc: palves@redhat.com; gdb-patches@sourceware.org
> Subject: Re: [PATCH V4] symlookup: improves symbol lookup when a file is
> specified.
> 
> Hi,
> 
> Just a nit, git shows this when applying your patch:
> 
> Applying: symlookup: improves symbol lookup when a file is specified.
> .git/rebase-apply/patch:157: new blank line at EOF.
> +
> .git/rebase-apply/patch:253: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.
> 
> If there are extra blank lines at the end of the new files, you can remove
> them.
> 
> On 2017-10-18 05:01, Walfred Tedeschi wrote:
> > The provided patch adds a scope lookup with higher priority in the
> > case 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, 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.  Without using this patch the result of the
> > evaluations
> > are:
> >
> > 	print 'print-file-var-lib1.c'::this_version_id
> > 	$1 = 104
> >
> > and
> >
> > 	print 'print-file-var-lib2.c'::this_version_id
> > 	$2 = 104  # This value is wrong we have set 203 in the code
> >
> >
> > With the patch applied we get:
> >
> > 	print 'print-file-var-lib1.c'::this_version_id
> > 	$1 = 104
> >
> > and
> >
> > 	print 'print-file-var-lib2.c'::this_version_id
> > 	$2 = 203  # Now value is correct
> >
> >
> >
> > 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
> >
> > in this case v1 and v2 are the same and equal to the this_version_id
> > defined in print-file-var-lib1.c.  Test is changed to assure that when
> > the scope operator is used the value seen in print-file-var-lib2.c is
> > the one GDB evaluates, i.e. 203.
> >
> > *  Reading again Simon's comments I have decided to take a quick look
> > on the failing test.  I have fixed it according to the logic of the
> > current patch.
> >
> > 2017-10-18  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.base/print-file-var.exp: Modify expected result for
> > 	evaluating 'print-file-var-lib2.c'::this_version_id.
> >
> > ---
> >  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   | 90
> > ++++++++++++++++++++++
> >  gdb/testsuite/gdb.base/print-file-var.exp          |  4 +-
> >  6 files changed, 208 insertions(+), 1 deletion(-)  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.
> > +
> > +   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..9c3c0e6
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
> > @@ -0,0 +1,90 @@
> > +# 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/>.  */
> > +
> 
> Could you add here a one sentence description of what this test file is about?
> When looking at test files, it is nice to have a quick summary to know what it
> is testing.
> 
> > +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
> > +{}]
> 
> shlibdir seems unused.
> 
> > +
> > +set lib_opts debug
> > +set exec_opts [list debug shlib_load
> > additional_flags=-DSHLIB_NAME=\"${lib_dlopen1}\" \
> > +additional_flags=-DSHLIB_NAME2=\"${lib_dlopen2}\"]
> 
> The flags definied SHLIB_NAME and SHLIB_NAME2 are not useful, I think.
> 
> > +
> > +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
> > +}
> > +
> > +clean_restart $executable
> > +
> > +if ![runto_main] {
> > +    untested "could not run to main"
> > +    return -1
> > +}
> > +
> > +# Create to shared libraries having the symbol "this_version_id"
> > defined in both
> 
> "to" -> "two"
> 
> > +# libraries global scope.  Main program will dllopen one by one and
> > evaluate the
> 
> "dllopen" -> "dlopen"
> 
> > +# symbol "this_version_id" via a function provided by the library
> > assigning the value of
> > +# "this_version_id" to V1 and V2.
> > +# Using the scope to perform the evaluations should return the value
> > +# defined in the c file.
> > +
> > +# 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_continue_to_breakpoint "continue to the STOP"
> > +
> > +# Now check the value of this_version_id in both
> > +print-file-var-lib1.c # and print-file-var-lib2.c.
> 
> The filenames mentioned here are wrong.  It's probably simpler to say "... in
> both libraries".
> 
> > +
> > +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"
> > +
> > diff --git a/gdb/testsuite/gdb.base/print-file-var.exp
> > b/gdb/testsuite/gdb.base/print-file-var.exp
> > index 223a67d..ae5071a 100644
> > --- a/gdb/testsuite/gdb.base/print-file-var.exp
> > +++ b/gdb/testsuite/gdb.base/print-file-var.exp
> > @@ -88,5 +88,7 @@ gdb_test "continue" \  gdb_test "print
> > 'print-file-var-lib1.c'::this_version_id == v1" \
> >           " = 1"
> >
> > -gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
> > +# Independent of the linker value seen in the second library should #
> > +be 203.
> > +gdb_test "print 'print-file-var-lib2.c'::this_version_id == 203" \
> >           " = 1"
> 
Hi Simon,

Thanks for your review!
For all the comment above I agree, Thanks again!

For the one below there are different point of views. 
How I see it: Very few sane people will add a symbols in a shared library that
will collide like the case we presented here.  If one does so how can the debugger
help?

Providing the same value as the runtime or linker does? 
This one user already knows. 
Or providing what the debug information provides as value created by the library itself.
In final end both are right. :|

But when specifying the scope if user is provided the value of the debug info it should
be easier to spot that there is something weird going on in the code.

That was my rationale.

Perhaps, the debugger could here even provide a new command to analyze those colliding symbols.

/Fred

> I am not convinced about changing this test.  Normally, we want the
> debugger, when evaluating expressions to act as close as possible to the
> target program.  If the value of this_version_id read in lib2 was 104, then
> that's probably what print 'print-file-var-lib2.c'::this_version_id
> should show as well, don't you think?  This test is checking that this is the
> case.



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

Re: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Simon Marchi]

On 2017-10-20 03:45 AM, Tedeschi, Walfred wrote:
> Hi Simon,
> 
> Thanks for your review!
> For all the comment above I agree, Thanks again!
> 
> For the one below there are different point of views. 
> How I see it: Very few sane people will add a symbols in a shared library that
> will collide like the case we presented here.  If one does so how can the debugger
> help?

I think one usual use case is plugins implemented with shared library.  Although
the data symbols will commonly be static, and the plugin will only expose some
function symbols.

> Providing the same value as the runtime or linker does? 
> This one user already knows. 
> Or providing what the debug information provides as value created by the library itself.
> In final end both are right. :|
> 
> But when specifying the scope if user is provided the value of the debug info it should
> be easier to spot that there is something weird going on in the code.

I think what you just said summarizes the problem well and I think it makes sense.
I just don't think I have enough experience about symbol handling to understand
the situation fully.  Could another maintainer with more experience about symbols
give the final ok?

> That was my rationale.
> 
> Perhaps, the debugger could here even provide a new command to analyze those colliding symbols.

Yes, and maybe a way to print all symbols with a given name, with some info
about where they come from.

Simon

Re: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Pedro Alves]

On 10/20/2017 03:28 PM, Simon Marchi wrote:
> On 2017-10-20 03:45 AM, Tedeschi, Walfred wrote:
>> Hi Simon,
>>
>> Thanks for your review!
>> For all the comment above I agree, Thanks again!
>>
>> For the one below there are different point of views. 
>> How I see it: Very few sane people will add a symbols in a shared library that
>> will collide like the case we presented here.  If one does so how can the debugger
>> help?
> 
> I think one usual use case is plugins implemented with shared library.  Although
> the data symbols will commonly be static, and the plugin will only expose some
> function symbols.
> 
>> Providing the same value as the runtime or linker does? 
>> This one user already knows. 
>> Or providing what the debug information provides as value created by the library itself.
>> In final end both are right. :|
>>
>> But when specifying the scope if user is provided the value of the debug info it should
>> be easier to spot that there is something weird going on in the code.
> 
> I think what you just said summarizes the problem well and I think it makes sense.
> I just don't think I have enough experience about symbol handling to understand
> the situation fully.  Could another maintainer with more experience about symbols
> give the final ok?

I disagree.  Having
 (gdb) frame
 #0 0x000000000040073b in function () at source.c:22
 (gdb) print foo
and:
 (gdb) print 'source.c':foo

show different values when you're stopped in a function in
the source.c file would look inconsistent to me.

Actually, the patch introduces what looks like a related clear
regression to me.  With the print-file-var.exp test program, try
stepping into get_version_2, and printing the this_version_id
global.  And then type finish.  Vis:

 (gdb) s
 get_version_2 () at gdb.base/print-file-var-lib2.c:22
 22        return this_version_id;
 (gdb) p this_version_id
 $1 = 203
 (gdb) finish
 Run till exit from #0  get_version_2 () at gdb.base/print-file-var-lib2.c:22
 0x000000000040073b in main () at gdb.base/print-file-var-main.c:24
 24        int v2 = get_version_2 ();
 Value returned is $2 = 104
 (gdb) 

GDB says "203", while the program returns "104".
That looks like a bug to me.  I'd expect the print
to show me the current value of the variable in scope.

In current master (without the patch), we get:

 (gdb) s
 get_version_2 () at gdb.base/print-file-var-lib2.c:22
 22        return this_version_id;
 (gdb) p this_version_id
 $1 = 104
 (gdb) finish
 Run till exit from #0  get_version_2 () at gdb.base/print-file-var-lib2.c:22
 0x000000000040073b in main () at gdb.base/print-file-var-main.c:24
 24        int v2 = get_version_2 ();
 Value returned is $2 = 104

Thanks,
Pedro Alves

[PATCH] Enhance gdb.base/print-file-var.exp testcase (Re: [PATCH V4] symlookup: improves symbol lookup when a file is specified.)

[Pedro Alves]

On 10/20/2017 04:28 PM, Pedro Alves wrote:

> Actually, the patch introduces what looks like a related clear
> regression to me.  With the print-file-var.exp test program, try
> stepping into get_version_2, and printing the this_version_id
> global.  And then type finish.  Vis:
> 
>  (gdb) s
>  get_version_2 () at gdb.base/print-file-var-lib2.c:22
>  22        return this_version_id;
>  (gdb) p this_version_id
>  $1 = 203
>  (gdb) finish
>  Run till exit from #0  get_version_2 () at gdb.base/print-file-var-lib2.c:22
>  0x000000000040073b in main () at gdb.base/print-file-var-main.c:24
>  24        int v2 = get_version_2 ();
>  Value returned is $2 = 104
>  (gdb) 
> 
> GDB says "203", while the program returns "104".
> That looks like a bug to me.  I'd expect the print
> to show me the current value of the variable in scope.
> 
> In current master (without the patch), we get:
> 
>  (gdb) s
>  get_version_2 () at gdb.base/print-file-var-lib2.c:22
>  22        return this_version_id;
>  (gdb) p this_version_id
>  $1 = 104
>  (gdb) finish
>  Run till exit from #0  get_version_2 () at gdb.base/print-file-var-lib2.c:22
>  0x000000000040073b in main () at gdb.base/print-file-var-main.c:24
>  24        int v2 = get_version_2 ();
>  Value returned is $2 = 104

Now in testsuite patch form...  This is against master, where it passes
cleanly.  Walfred's patch results in:

  FAIL: gdb.base/print-file-var.exp: GDB sees same value as inferior sees while stopped in lib2

(Note: I haven't really looked deep in detail on why that is so.)

From c389985174ac2b87d14e864d1eff9b5a7e9051db Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 20 Oct 2017 16:30:46 +0100
Subject: [PATCH] Enhance gdb.base/print-file-var.exp testcase

Make sure that GDB sees the same value the inferior sees when stopped
in each library.

gdb/testsuite/ChangeLog:
2017-10-20  Pedro Alves  <palves@redhat.com>

	* gdb.base/print-file-var.exp: Check that GDB sees same value as
	inferior in each library.
---
 gdb/testsuite/gdb.base/print-file-var.exp | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index 223a67d..60b8e08 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -74,6 +74,17 @@ if ![runto_main] {
 
 set bp_location \
     [gdb_get_line_number "STOP" "${executable}.c"]
+
+# Stop in both libraries and save the value of 'this_version_id' as
+# seen from those contexts.
+gdb_breakpoint "get_version_1"
+gdb_test "continue" "Breakpoint .* get_version_1.*"
+set v1_lib1 [get_integer_valueof "this_version_id" -1]
+
+gdb_breakpoint "get_version_2"
+gdb_test "continue" "Breakpoint .* get_version_2.*"
+set v2_lib2 [get_integer_valueof "this_version_id" -1]
+
 gdb_test "break $executable.c:$bp_location" \
          "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
          "breapoint past v1 & v2 initialization"
@@ -82,6 +93,16 @@ gdb_test "continue" \
          "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
          "continue to STOP marker"
 
+# Check that the values of 'this_version_id' that we saved above when
+# stopped in both print-file-var-lib1.c:get_version_1 and
+# print-file-var-lib2.c:get_version_2 match what those functions
+# actually returned.
+
+gdb_test "print v1" " = $v1_lib1" \
+    "GDB sees same value as inferior sees while stopped in lib1"
+gdb_test "print v2" " = $v2_lib2" \
+    "GDB sees same value as inferior sees while stopped in lib2"
+
 # Now check the value of this_version_id in both print-file-var-lib1.c
 # and print-file-var-lib2.c.
 
-- 
2.5.5

RE: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Tedeschi, Walfred]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, October 20, 2017 5:29 PM
> To: Simon Marchi <simon.marchi@ericsson.com>; Tedeschi, Walfred
> <walfred.tedeschi@intel.com>; Simon Marchi <simon.marchi@polymtl.ca>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH V4] symlookup: improves symbol lookup when a file is
> specified.
> 
> On 10/20/2017 03:28 PM, Simon Marchi wrote:
> > On 2017-10-20 03:45 AM, Tedeschi, Walfred wrote:
> >> Hi Simon,
> >>
> >> Thanks for your review!
> >> For all the comment above I agree, Thanks again!
> >>
> >> For the one below there are different point of views.
> >> How I see it: Very few sane people will add a symbols in a shared
> >> library that will collide like the case we presented here.  If one
> >> does so how can the debugger help?
> >
> > I think one usual use case is plugins implemented with shared library.
> > Although the data symbols will commonly be static, and the plugin will
> > only expose some function symbols.
> >
> >> Providing the same value as the runtime or linker does?
> >> This one user already knows.
> >> Or providing what the debug information provides as value created by the
> library itself.
> >> In final end both are right. :|
> >>
> >> But when specifying the scope if user is provided the value of the
> >> debug info it should be easier to spot that there is something weird going on
> in the code.
> >
> > I think what you just said summarizes the problem well and I think it makes
> sense.
> > I just don't think I have enough experience about symbol handling to
> > understand the situation fully.  Could another maintainer with more
> > experience about symbols give the final ok?
> 
> I disagree.  Having
>  (gdb) frame
>  #0 0x000000000040073b in function () at source.c:22
>  (gdb) print foo
> and:
>  (gdb) print 'source.c':foo
> 
> show different values when you're stopped in a function in the source.c file
> would look inconsistent to me.
> 
> Actually, the patch introduces what looks like a related clear regression to me.
> With the print-file-var.exp test program, try stepping into get_version_2, and
> printing the this_version_id global.  And then type finish.  Vis:
> 
>  (gdb) s
>  get_version_2 () at gdb.base/print-file-var-lib2.c:22
>  22        return this_version_id;
>  (gdb) p this_version_id
>  $1 = 203
>  (gdb) finish
>  Run till exit from #0  get_version_2 () at gdb.base/print-file-var-lib2.c:22
> 0x000000000040073b in main () at gdb.base/print-file-var-main.c:24
>  24        int v2 = get_version_2 ();
>  Value returned is $2 = 104
>  (gdb)
> 
> GDB says "203", while the program returns "104".
> That looks like a bug to me.  I'd expect the print to show me the current value of
> the variable in scope.
> 
> In current master (without the patch), we get:
> 
>  (gdb) s
>  get_version_2 () at gdb.base/print-file-var-lib2.c:22
>  22        return this_version_id;
>  (gdb) p this_version_id
>  $1 = 104
>  (gdb) finish
>  Run till exit from #0  get_version_2 () at gdb.base/print-file-var-lib2.c:22
> 0x000000000040073b in main () at gdb.base/print-file-var-main.c:24
>  24        int v2 = get_version_2 ();
>  Value returned is $2 = 104

Hello Pedro,

Thanks a lot for reviewing that!
I will bring more information related to what debug information provide and so on.
The linker is the cause of this disconnection.  In a previous test patch I have added a set/show variable switch between.
Dlopen and linker behavior.  I also found that wrong and looking at it again.  

I will try to bring more facts with the debug info and memory examination.

In any case actual behavior of master is also wrong for dlopen case!

Thanks and regards,
/Fred
> 
> Thanks,
> Pedro Alves
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

RE: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Tedeschi, Walfred]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Tedeschi, Walfred
> Sent: Saturday, October 21, 2017 12:47 PM
> To: Pedro Alves <palves@redhat.com>; Simon Marchi
> <simon.marchi@ericsson.com>; Simon Marchi <simon.marchi@polymtl.ca>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH V4] symlookup: improves symbol lookup when a file is
> specified.
> 
> > -----Original Message-----
> > From: Pedro Alves [mailto:palves@redhat.com]
> > Sent: Friday, October 20, 2017 5:29 PM
> > To: Simon Marchi <simon.marchi@ericsson.com>; Tedeschi, Walfred
> > <walfred.tedeschi@intel.com>; Simon Marchi <simon.marchi@polymtl.ca>
> > Cc: gdb-patches@sourceware.org
> > Subject: Re: [PATCH V4] symlookup: improves symbol lookup when a file
> > is specified.
> >
> > On 10/20/2017 03:28 PM, Simon Marchi wrote:
> > > On 2017-10-20 03:45 AM, Tedeschi, Walfred wrote:
> > >> Hi Simon,
> > >>
> > >> Thanks for your review!
> > >> For all the comment above I agree, Thanks again!
> > >>
> > >> For the one below there are different point of views.
> > >> How I see it: Very few sane people will add a symbols in a shared
> > >> library that will collide like the case we presented here.  If one
> > >> does so how can the debugger help?
> > >
> > > I think one usual use case is plugins implemented with shared library.
> > > Although the data symbols will commonly be static, and the plugin
> > > will only expose some function symbols.
> > >
> > >> Providing the same value as the runtime or linker does?
> > >> This one user already knows.
> > >> Or providing what the debug information provides as value created
> > >> by the
> > library itself.
> > >> In final end both are right. :|
> > >>
> > >> But when specifying the scope if user is provided the value of the
> > >> debug info it should be easier to spot that there is something
> > >> weird going on
> > in the code.
> > >
> > > I think what you just said summarizes the problem well and I think
> > > it makes
> > sense.
> > > I just don't think I have enough experience about symbol handling to
> > > understand the situation fully.  Could another maintainer with more
> > > experience about symbols give the final ok?
> >
> > I disagree.  Having
> >  (gdb) frame
> >  #0 0x000000000040073b in function () at source.c:22
> >  (gdb) print foo
> > and:
> >  (gdb) print 'source.c':foo
> >
> > show different values when you're stopped in a function in the
> > source.c file would look inconsistent to me.
> >
> > Actually, the patch introduces what looks like a related clear regression to
> me.
> > With the print-file-var.exp test program, try stepping into
> > get_version_2, and printing the this_version_id global.  And then type
> finish.  Vis:
> >
> >  (gdb) s
> >  get_version_2 () at gdb.base/print-file-var-lib2.c:22
> >  22        return this_version_id;
> >  (gdb) p this_version_id
> >  $1 = 203
> >  (gdb) finish
> >  Run till exit from #0  get_version_2 () at
> > gdb.base/print-file-var-lib2.c:22 0x000000000040073b in main () at
> gdb.base/print-file-var-main.c:24
> >  24        int v2 = get_version_2 ();
> >  Value returned is $2 = 104
> >  (gdb)
> >
> > GDB says "203", while the program returns "104".
> > That looks like a bug to me.  I'd expect the print to show me the
> > current value of the variable in scope.
> >
> > In current master (without the patch), we get:
> >
> >  (gdb) s
> >  get_version_2 () at gdb.base/print-file-var-lib2.c:22
> >  22        return this_version_id;
> >  (gdb) p this_version_id
> >  $1 = 104
> >  (gdb) finish
> >  Run till exit from #0  get_version_2 () at
> > gdb.base/print-file-var-lib2.c:22 0x000000000040073b in main () at
> gdb.base/print-file-var-main.c:24
> >  24        int v2 = get_version_2 ();
> >  Value returned is $2 = 104
> 
> Hello Pedro,
> 
> Thanks a lot for reviewing that!
> I will bring more information related to what debug information provide and
> so on.
> The linker is the cause of this disconnection.  In a previous test patch I have
> added a set/show variable switch between.
> Dlopen and linker behavior.  I also found that wrong and looking at it again.
> 
> I will try to bring more facts with the debug info and memory examination.
> 
> In any case actual behavior of master is also wrong for dlopen case!
> 

Hello all,

Here we go:

With mainline gdb we have the following scenario:

Stopped at main line 32 in print-file-var-main(testcase)

	(gdb) print this_version_id 
	$1 = 104

This was expected, we are looking at the symbol that the linker and main can see as specified in the global scope.

	(gdb) print &this_version_id 
	$2 = (int *) 0x7ffff7ddb028 <this_version_id>

	(gdb) info symbol 0x7ffff7ddb028
	this_version_id in section .data of /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/testsuite/outputs/gdb.base/print-file-var/print-file-var-lib1.so

Those two last evaluations prove that we are seem a symbol coming from the first library as expected and done by the linker.

However interestingly if I specify the scope, I get:

	(gdb) print  &('print-file-var-lib2.c'::this_version_id)
	$3 = (int *) 0x7ffff7ddb028  <this_version_id>

	(gdb) info symbol 0x7ffff7ddb028  
	this_version_id in section .data of /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/testsuite/outputs/gdb.base/print-file-var/print-file-var-lib1.so
	
But I have specified the scope! I am asking specifically that I want the symbol defined in the second library not in the first.

If I use the patch provided than I get:

	(gdb) print  &('print-file-var-lib2.c'::this_version_id)
	$4 = (int *) 0x7ffff7ddb028 <this_version_id>

	(gdb) info symbol 0x7ffff7ddb028
	this_version_id in section .data of /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/testsuite/outputs/gdb.base/print-file-var/print-file-var-lib1.so

_This was what I've expected. _

With that I _cannot_ say that GDB is doing right in the main line. Basically the test is weak!

The issue is in the symtab.c ~2603:
     gdbarch_iterate_over_objfiles_in_search_order
 	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
 	 lookup_symbol_global_iterator_cb, &lookup_data, objfile);

The lookup here is done in the order of the library load, what is right for global symbols, If and only if no scope is provided.
Again user will specify the scope if he has a doubt on how in earth his value is not what is seen in the execution!


I hope this sample run helps in elucidating the problem!

Thanks and regards,
/Fred

> Thanks and regards,
> /Fred
> >
> > Thanks,
> > Pedro Alves
> 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
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

Re: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Pedro Alves]

On 10/23/2017 10:03 AM, Tedeschi, Walfred wrote:

> Here we go:
> 
> With mainline gdb we have the following scenario:
> 
> Stopped at main line 32 in print-file-var-main(testcase)
> 
> 	(gdb) print this_version_id 
> 	$1 = 104
> 
> This was expected, we are looking at the symbol that the linker and main can see as specified in the global scope.
> 
> 	(gdb) print &this_version_id 
> 	$2 = (int *) 0x7ffff7ddb028 <this_version_id>
> 
> 	(gdb) info symbol 0x7ffff7ddb028
> 	this_version_id in section .data of /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/testsuite/outputs/gdb.base/print-file-var/print-file-var-lib1.so
> 
> Those two last evaluations prove that we are seem a symbol coming from the first library as expected and done by the linker.
> 
> However interestingly if I specify the scope, I get:
> 
> 	(gdb) print  &('print-file-var-lib2.c'::this_version_id)
> 	$3 = (int *) 0x7ffff7ddb028  <this_version_id>
> 
> 	(gdb) info symbol 0x7ffff7ddb028  
> 	this_version_id in section .data of /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/testsuite/outputs/gdb.base/print-file-var/print-file-var-lib1.so
> 	
> But I have specified the scope! I am asking specifically that I want the symbol defined in the second library not in the first.

In the running process, there is no such thing as two different
addresses for 'this_version_id'.  The linker arranges for all references to
refer to the same symbol.  This is symbol interposition at work.  Therefore,
it does not make sense for print  &('print-file-var-lib2.c'::this_version_id)
to print any other address because all the code in print-file-var-lib2.c
_is_ referencing the symbol in the first library.

Try this:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
From 9cbfd5d4985ab1bbc6a1d4e95fc965db00f807a8 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 23 Oct 2017 10:37:48 +0100
Subject: [PATCH] print addresses

---
 gdb/testsuite/gdb.base/print-file-var-lib1.c | 4 ++++
 gdb/testsuite/gdb.base/print-file-var-lib2.c | 3 +++
 gdb/testsuite/gdb.base/print-file-var-main.c | 1 -
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c
index 033f9e4..a284003 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
@@ -14,10 +14,14 @@
    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 <stdio.h>
+
 int this_version_id = 104;
 
 int
 get_version_1 (void)
 {
+  printf ("get_version_1: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
+
   return this_version_id;
 }
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c
index d94a792..2cb210f 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
@@ -14,10 +14,13 @@
    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 <stdio.h>
+
 int this_version_id = 203;
 
 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;
 }
diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c
index a19f6f7..3f4a5dc 100644
--- a/gdb/testsuite/gdb.base/print-file-var-main.c
+++ b/gdb/testsuite/gdb.base/print-file-var-main.c
@@ -31,4 +31,3 @@ main (void)
 
   return 0; /* STOP */
 }
-
-- 
2.5.5
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and you'll see:

 $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main 
 get_version_1: &this_version_id=0x7f2fcc3d2030, this_version_id=104
 get_version_2: &this_version_id=0x7f2fcc3d2030, this_version_id=104
                                 ^^^^^^^^^^^^^^                  ^^^

As you can see, the _same address_ is printed twice.  So it makes
sense for GDB to do the same thing.


And if you define the same symbol in the main program, you'll see
the linker making sure that the version in the main program is
used by both libraries.  I.e., try this:

~~~
diff --git c/gdb/testsuite/gdb.base/print-file-var-main.c w/gdb/testsuite/gdb.base/print-file-var-main.c
index 3f4a5dc..60f277a 100644
--- c/gdb/testsuite/gdb.base/print-file-var-main.c
+++ w/gdb/testsuite/gdb.base/print-file-var-main.c
@@ -31,3 +31,5 @@ main (void)
 
   return 0; /* STOP */
 }
+
+int this_version_id = 55;
~~~

and now you get:

 $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main 
 get_version_1: &this_version_id=0x60103c, this_version_id=55
 get_version_2: &this_version_id=0x60103c, this_version_id=55
                                 ^^^^^^^^                  ^^

Again, that's symbol interposition for you.


If you mark the "this_version_id" symbols in the libraries with
hidden or protected visibility, like:

lib1:
 __attribute__((visibility("hidden"))) int this_version_id = 104;
lib2:
 __attribute__((visibility("hidden"))) int this_version_id = 203;

then you'll see that that disables the interposition/overriding:

 $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main 
 get_version_1: &this_version_id=0x7f343c96c030, this_version_id=104
 get_version_2: &this_version_id=0x7f343c76a030, this_version_id=203
                                 ^^^^^^^^^^^^^^                  ^^^

And in the dlopen case, you'll get the same / non-override result.

So to me, a proper fix for this whole shebang makes GDB follow the
same rules the linker does (and ideally convert the variants above
to testsuite tests).  How exactly to make that work, I'm not
sure, off hand.  But we do already have some logic in place
for something like this in

  solib.c:solib_global_lookup
    -> ops->lookup_lib_global_symbol
      -> solib-svr4.c:elf_lookup_lib_symbol.

Please investigate more into this.

> 
> If I use the patch provided than I get:
> 
> 	(gdb) print  &('print-file-var-lib2.c'::this_version_id)
> 	$4 = (int *) 0x7ffff7ddb028 <this_version_id>
> 
> 	(gdb) info symbol 0x7ffff7ddb028
> 	this_version_id in section .data of /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/testsuite/outputs/gdb.base/print-file-var/print-file-var-lib1.so
> 
> _This was what I've expected. _

I disagree with your expectation.

Thanks,
Pedro Alves

> 
> With that I _cannot_ say that GDB is doing right in the main line. Basically the test is weak!
> 
> The issue is in the symtab.c ~2603:
>      gdbarch_iterate_over_objfiles_in_search_order
>  	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
>  	 lookup_symbol_global_iterator_cb, &lookup_data, objfile);
> 
> The lookup here is done in the order of the library load, what is right for global symbols, If and only if no scope is provided.
> Again user will specify the scope if he has a doubt on how in earth his value is not what is seen in the execution!
> 
> 
> I hope this sample run helps in elucidating the problem!
> 
> Thanks and regards,
> /Fred

RE: [PATCH V4] symlookup: improves symbol lookup when a file is specified.

[Tedeschi, Walfred]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Monday, October 23, 2017 12:07 PM
> To: Tedeschi, Walfred <walfred.tedeschi@intel.com>; Simon Marchi
> <simon.marchi@ericsson.com>; Simon Marchi <simon.marchi@polymtl.ca>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH V4] symlookup: improves symbol lookup when a file is
> specified.
> 
> On 10/23/2017 10:03 AM, Tedeschi, Walfred wrote:
> 
> > Here we go:
> >
> > With mainline gdb we have the following scenario:
> >
> > Stopped at main line 32 in print-file-var-main(testcase)
> >
> > 	(gdb) print this_version_id
> > 	$1 = 104
> >
> > This was expected, we are looking at the symbol that the linker and main can
> see as specified in the global scope.
> >
> > 	(gdb) print &this_version_id
> > 	$2 = (int *) 0x7ffff7ddb028 <this_version_id>
> >
> > 	(gdb) info symbol 0x7ffff7ddb028
> > 	this_version_id in section .data of
> > /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/test
> > suite/outputs/gdb.base/print-file-var/print-file-var-lib1.so
> >
> > Those two last evaluations prove that we are seem a symbol coming from the
> first library as expected and done by the linker.
> >
> > However interestingly if I specify the scope, I get:
> >
> > 	(gdb) print  &('print-file-var-lib2.c'::this_version_id)
> > 	$3 = (int *) 0x7ffff7ddb028  <this_version_id>
> >
> > 	(gdb) info symbol 0x7ffff7ddb028
> > 	this_version_id in section .data of
> > /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/test
> > suite/outputs/gdb.base/print-file-var/print-file-var-lib1.so
> >
> > But I have specified the scope! I am asking specifically that I want the symbol
> defined in the second library not in the first.
> 
> In the running process, there is no such thing as two different addresses for
> 'this_version_id'.  The linker arranges for all references to refer to the same
> symbol.  This is symbol interposition at work.  Therefore, it does not make sense
> for print  &('print-file-var-lib2.c'::this_version_id)
> to print any other address because all the code in print-file-var-lib2.c _is_
> referencing the symbol in the first library.
> 
> Try this:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> From 9cbfd5d4985ab1bbc6a1d4e95fc965db00f807a8 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 23 Oct 2017 10:37:48 +0100
> Subject: [PATCH] print addresses
> 
> ---
>  gdb/testsuite/gdb.base/print-file-var-lib1.c | 4 ++++
> gdb/testsuite/gdb.base/print-file-var-lib2.c | 3 +++
> gdb/testsuite/gdb.base/print-file-var-main.c | 1 -
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c
> b/gdb/testsuite/gdb.base/print-file-var-lib1.c
> index 033f9e4..a284003 100644
> --- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
> +++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
> @@ -14,10 +14,14 @@
>     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 <stdio.h>
> +
>  int this_version_id = 104;
> 
>  int
>  get_version_1 (void)
>  {
> +  printf ("get_version_1: &this_version_id=%p, this_version_id=%d\n",
> + &this_version_id, this_version_id);
> +
>    return this_version_id;
>  }
> diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c
> b/gdb/testsuite/gdb.base/print-file-var-lib2.c
> index d94a792..2cb210f 100644
> --- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
> +++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
> @@ -14,10 +14,13 @@
>     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 <stdio.h>
> +
>  int this_version_id = 203;
> 
>  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;
>  }
> diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c
> b/gdb/testsuite/gdb.base/print-file-var-main.c
> index a19f6f7..3f4a5dc 100644
> --- a/gdb/testsuite/gdb.base/print-file-var-main.c
> +++ b/gdb/testsuite/gdb.base/print-file-var-main.c
> @@ -31,4 +31,3 @@ main (void)
> 
>    return 0; /* STOP */
>  }
> -
> --
> 2.5.5
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> and you'll see:
> 
>  $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main
>  get_version_1: &this_version_id=0x7f2fcc3d2030, this_version_id=104
>  get_version_2: &this_version_id=0x7f2fcc3d2030, this_version_id=104
>                                  ^^^^^^^^^^^^^^                  ^^^
> 
> As you can see, the _same address_ is printed twice.  So it makes sense for GDB
> to do the same thing.
> 
> 
> And if you define the same symbol in the main program, you'll see the linker
> making sure that the version in the main program is used by both libraries.  I.e.,
> try this:
> 
> ~~~
> diff --git c/gdb/testsuite/gdb.base/print-file-var-main.c
> w/gdb/testsuite/gdb.base/print-file-var-main.c
> index 3f4a5dc..60f277a 100644
> --- c/gdb/testsuite/gdb.base/print-file-var-main.c
> +++ w/gdb/testsuite/gdb.base/print-file-var-main.c
> @@ -31,3 +31,5 @@ main (void)
> 
>    return 0; /* STOP */
>  }
> +
> +int this_version_id = 55;
> ~~~
> 
> and now you get:
> 
>  $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main
>  get_version_1: &this_version_id=0x60103c, this_version_id=55
>  get_version_2: &this_version_id=0x60103c, this_version_id=55
>                                  ^^^^^^^^                  ^^
> 
> Again, that's symbol interposition for you.
> 
> 
> If you mark the "this_version_id" symbols in the libraries with hidden or
> protected visibility, like:
> 
> lib1:
>  __attribute__((visibility("hidden"))) int this_version_id = 104;
> lib2:
>  __attribute__((visibility("hidden"))) int this_version_id = 203;
> 
> then you'll see that that disables the interposition/overriding:
> 
>  $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main
>  get_version_1: &this_version_id=0x7f343c96c030, this_version_id=104
>  get_version_2: &this_version_id=0x7f343c76a030, this_version_id=203
>                                  ^^^^^^^^^^^^^^                  ^^^
> 
> And in the dlopen case, you'll get the same / non-override result.
> 
> So to me, a proper fix for this whole shebang makes GDB follow the same rules
> the linker does (and ideally convert the variants above to testsuite tests).  How
> exactly to make that work, I'm not sure, off hand.  But we do already have some
> logic in place for something like this in
> 
>   solib.c:solib_global_lookup
>     -> ops->lookup_lib_global_symbol
>       -> solib-svr4.c:elf_lookup_lib_symbol.
> 
> Please investigate more into this.
> 
> >
> > If I use the patch provided than I get:
> >
> > 	(gdb) print  &('print-file-var-lib2.c'::this_version_id)
> > 	$4 = (int *) 0x7ffff7ddb028 <this_version_id>
> >
> > 	(gdb) info symbol 0x7ffff7ddb028
> > 	this_version_id in section .data of
> > /nfs/iul/disks/iul_team2/wtedesch/_gdb_/extern_gdb/bin/dlopen/gdb/test
> > suite/outputs/gdb.base/print-file-var/print-file-var-lib1.so
> >
> > _This was what I've expected. _
> 
> I disagree with your expectation.

Pedro and All,

I was considering all of that and sleeping a bit over it.
In summary I see the problem as:
1. For application that do not rely on dlopen GDB is doing the right job.
2. For dlopen applications there is improvement to be made.
3.  dlopen scenario is an exception in terms of usage.
4. User should have more control about the libraries that was dlopened.
5. Considering that from the system side there is no way to distinguish if the shared object was dlopened added with the linker.

Based on this scenario; I was considering in adding a flag in loaded object, so symbol lookup could profit
of the flag and provide right evaluations on both cases.

I do not know how implementation would look like.  By now this is only a proposal to solve the dlopen case.

Thanks and regards,
/Fred
> 
> Thanks,
> Pedro Alves
> 
> >
> > With that I _cannot_ say that GDB is doing right in the main line. Basically the
> test is weak!
> >
> > The issue is in the symtab.c ~2603:
> >      gdbarch_iterate_over_objfiles_in_search_order
> >  	(objfile != NULL ? get_objfile_arch (objfile) : target_gdbarch (),
> >  	 lookup_symbol_global_iterator_cb, &lookup_data, objfile);
> >
> > The lookup here is done in the order of the library load, what is right for global
> symbols, If and only if no scope is provided.
> > Again user will specify the scope if he has a doubt on how in earth his value is
> not what is seen in the execution!
> >
> >
> > I hope this sample run helps in elucidating the problem!
> >
> > 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