From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49744 invoked by alias); 25 Oct 2017 09:39:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 49726 invoked by uid 89); 25 Oct 2017 09:39:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=Managing, sleeping, GmbH, 10-12 X-HELO: mga01.intel.com Received: from mga01.intel.com (HELO mga01.intel.com) (192.55.52.88) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 25 Oct 2017 09:39:39 +0000 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Oct 2017 02:39:38 -0700 X-ExtLoop1: 1 Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga001.fm.intel.com with ESMTP; 25 Oct 2017 02:39:36 -0700 Received: from irsmsx156.ger.corp.intel.com (10.108.20.68) by IRSMSX104.ger.corp.intel.com (163.33.3.159) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 25 Oct 2017 10:39:35 +0100 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.248]) by IRSMSX156.ger.corp.intel.com ([169.254.3.33]) with mapi id 14.03.0319.002; Wed, 25 Oct 2017 10:39:35 +0100 From: "Tedeschi, Walfred" To: Pedro Alves , Simon Marchi , Simon Marchi CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH V4] symlookup: improves symbol lookup when a file is specified. Date: Wed, 25 Oct 2017 09:39:00 -0000 Message-ID: References: <1508317280-31265-1-git-send-email-walfred.tedeschi@intel.com> <327caaf3429595c07a29d455ea3ed6a0@polymtl.ca> <9220f1a2-9932-a4ab-f896-a5315f701eb4@redhat.com> In-Reply-To: <9220f1a2-9932-a4ab-f896-a5315f701eb4@redhat.com> dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOWFmZTQ3MWEtOGM5My00MzMzLWFkMWItMzI2YjU0ZWY5NjQ5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJERHpTXC9RNFd3cW0yZVJKS1NOb01qeE1zWUlFWGgzM29VUHdTWTNseWc4OEVhVEN1b29ycXNXNjVzVmNldEhmZyJ9 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-10/txt/msg00764.txt.bz2 > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Monday, October 23, 2017 12:07 PM > To: Tedeschi, Walfred ; Simon Marchi > ; Simon Marchi > Cc: gdb-patches@sourceware.org > Subject: Re: [PATCH V4] symlookup: improves symbol lookup when a file is > specified. >=20 > On 10/23/2017 10:03 AM, Tedeschi, Walfred wrote: >=20 > > 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 =3D 104 > > > > This was expected, we are looking at the symbol that the linker and mai= n can > see as specified in the global scope. > > > > (gdb) print &this_version_id > > $2 =3D (int *) 0x7ffff7ddb028 > > > > (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 =3D (int *) 0x7ffff7ddb028 > > > > (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 th= e symbol > defined in the second library not in the first. >=20 > 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 th= e same > symbol. This is symbol interposition at work. Therefore, it does not ma= ke 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. >=20 > Try this: >=20 > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > From 9cbfd5d4985ab1bbc6a1d4e95fc965db00f807a8 Mon Sep 17 00:00:00 2001 > From: Pedro Alves > Date: Mon, 23 Oct 2017 10:37:48 +0100 > Subject: [PATCH] print addresses >=20 > --- > 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(-) >=20 > 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 .= */ >=20 > +#include > + > int this_version_id =3D 104; >=20 > int > get_version_1 (void) > { > + printf ("get_version_1: &this_version_id=3D%p, this_version_id=3D%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 .= */ >=20 > +#include > + > int this_version_id =3D 203; >=20 > int > get_version_2 (void) > { > + printf ("get_version_2: &this_version_id=3D%p, this_version_id=3D%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) >=20 > return 0; /* STOP */ > } > - > -- > 2.5.5 > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >=20 > and you'll see: >=20 > $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main > get_version_1: &this_version_id=3D0x7f2fcc3d2030, this_version_id=3D104 > get_version_2: &this_version_id=3D0x7f2fcc3d2030, this_version_id=3D104 > ^^^^^^^^^^^^^^ ^^^ >=20 > As you can see, the _same address_ is printed twice. So it makes sense f= or GDB > to do the same thing. >=20 >=20 > And if you define the same symbol in the main program, you'll see the lin= ker > making sure that the version in the main program is used by both librarie= s. I.e., > try this: >=20 > ~~~ > 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) >=20 > return 0; /* STOP */ > } > + > +int this_version_id =3D 55; > ~~~ >=20 > and now you get: >=20 > $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main > get_version_1: &this_version_id=3D0x60103c, this_version_id=3D55 > get_version_2: &this_version_id=3D0x60103c, this_version_id=3D55 > ^^^^^^^^ ^^ >=20 > Again, that's symbol interposition for you. >=20 >=20 > If you mark the "this_version_id" symbols in the libraries with hidden or > protected visibility, like: >=20 > lib1: > __attribute__((visibility("hidden"))) int this_version_id =3D 104; > lib2: > __attribute__((visibility("hidden"))) int this_version_id =3D 203; >=20 > then you'll see that that disables the interposition/overriding: >=20 > $ ./testsuite/outputs/gdb.base/print-file-var/print-file-var-main > get_version_1: &this_version_id=3D0x7f343c96c030, this_version_id=3D104 > get_version_2: &this_version_id=3D0x7f343c76a030, this_version_id=3D203 > ^^^^^^^^^^^^^^ ^^^ >=20 > And in the dlopen case, you'll get the same / non-override result. >=20 > So to me, a proper fix for this whole shebang makes GDB follow the same r= ules > the linker does (and ideally convert the variants above to testsuite test= s). How > exactly to make that work, I'm not sure, off hand. But we do already hav= e some > logic in place for something like this in >=20 > solib.c:solib_global_lookup > -> ops->lookup_lib_global_symbol > -> solib-svr4.c:elf_lookup_lib_symbol. >=20 > Please investigate more into this. >=20 > > > > If I use the patch provided than I get: > > > > (gdb) print &('print-file-var-lib2.c'::this_version_id) > > $4 =3D (int *) 0x7ffff7ddb028 > > > > (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. _ >=20 > 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 pr= oposal to solve the dlopen case. Thanks and regards, /Fred >=20 > Thanks, > Pedro Alves >=20 > > > > With that I _cannot_ say that GDB is doing right in the main line. Basi= cally the > test is weak! > > > > The issue is in the symtab.c ~2603: > > gdbarch_iterate_over_objfiles_in_search_order > > (objfile !=3D 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