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

[Pedro Alves <palves at redhat dot com>]

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