From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 101806 invoked by alias); 9 Oct 2017 14:06:41 -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 101792 invoked by uid 89); 9 Oct 2017 14:06:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=fred X-HELO: mga05.intel.com Received: from mga05.intel.com (HELO mga05.intel.com) (192.55.52.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Oct 2017 14:06:38 +0000 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP; 09 Oct 2017 07:06:37 -0700 X-ExtLoop1: 1 Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by orsmga003.jf.intel.com with ESMTP; 09 Oct 2017 07:06:36 -0700 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.248]) by IRSMSX152.ger.corp.intel.com ([169.254.6.87]) with mapi id 14.03.0319.002; Mon, 9 Oct 2017 15:05:05 +0100 From: "Tedeschi, Walfred" To: Pedro Alves , "qiyaoltc@gmail.com" , "Keith Seitz (keiths@redhat.com)" CC: "gdb-patches@sourceware.org" Subject: RE: [PATCH] symlookup: improves symbol lookup when a file is specified. Date: Mon, 09 Oct 2017 14:06:00 -0000 Message-ID: References: <1504687613-14649-1-git-send-email-walfred.tedeschi@intel.com> <1504687613-14649-3-git-send-email-walfred.tedeschi@intel.com> In-Reply-To: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action 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/msg00201.txt.bz2 -----Original Message----- From: Pedro Alves [mailto:palves@redhat.com]=20 Sent: Monday, October 9, 2017 2:42 PM To: Tedeschi, Walfred ; qiyaoltc@gmail.com Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] symlookup: improves symbol lookup when a file is speci= fied. On 09/06/2017 09:46 AM, Walfred Tedeschi wrote: > The provided patch adds a global lookup with higher priority for the=20 > provided file. >=20 > Usual symbol lookup from GDB follows linker logic. This is what is=20 > desired most of the time. In the usage case a file is provided as=20 > scope, so the lookup has to follow this priority. Failing in finding=20 > the symbol at this scope usual path is followed. >=20 > As test case it is presented two shared objects having a global=20 > variable with the same name but comming from different source files.=20=20 > Without the patch accessing them via the file scope returns the value=20 > seen in the first loaded shared object. Using the patch the value=20 > defined in the file scope is the one returned. >=20 > One of the already existing test cases in print-file-var.exp starts to=20 > fail after using this patch. In fact the value that the linker sees is=20 > different then the one debugger can read from the shared object. In=20 > 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 =3D=3D v2 In this case=20 > there also in both libraries the symbol this_version_id. > During load of the libraries linker resolves the symbol as the first=20 > one loaded and independent of the scope symbol will have the same=20 > value, as defined in the first library loaded. > However, when defining the scope the value should represent the value=20 > in that context. Diferent evaluations of the same symbols might also=20 > better spot the issue in users code. >=20 > - I haven't changed the test because I wanted to hear the community=20 > thought on the subject. I'm not sure I understand the description above, so I'd like to try this lo= cally to try to understand it better, but the test fails for me: (gdb) break print-file-var-dlopen-main.c:51 Breakpoint 2 at 0x400751: fil= e /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/print-file-var-dlopen-ma= in.c, line 51. (gdb) PASS: gdb.base/print-file-var-dlopen.exp: breapoint past v1 & v2 ini= tialization continue Continuing. [Inferior 1 (process 11534) exited with code 01] (gdb) FAIL: gdb.base/print-file-var-dlopen.exp: continue to STOP marker (t= he program exited) The program exits because dlopen failed. Is there a reason the test is try= ing to use LD_LIBRARY_PATH instead of compiling the program with the shared= library file name defined as an absolute path via additional_flags=3D-DXXX= X like seemingly every other dlopen test does? > + > + dummy (); /* STOP */ > + dlclose (lib1); > + dlclose (lib2); > + if (v1 !=3D 104) > + return 1; > + /* The value returned by get_version_2 depends on the target=20 > + system. */ if (v2 !=3D 104 || v2 !=3D 203) > + return 2; > + > + return 0; > +} > + I tried to fix the above to get it running for me, but then noticed other t= hings clearly bad with the test. This certainly can't have passed as is fo= r you. For example, the test reuses these source files from the print-file-var.exp= test: +set lib1 "print-file-var-lib1" +set lib2 "print-file-var-lib2" But then looks for "get_version" symbols in them, but there's no such symbo= ls...: + *(int **) (&get_version1) =3D dlsym (lib1, "get_version"); + *(int **= ) (&get_version2) =3D dlsym (lib2, "get_version"); While here: > + > +gdb_test "print 'print-file-var-dlopen-lib1.c'::this_version_id =3D=3D v= 1" \ > + " =3D 1" it looks like the test actually wanted to use some different source files, = but there's no print-file-var-dlopen-lib1.c included in the patch. Walfred, please double check what you intend to submit. Pedro Alves Pedro, Thanks a lot for your review and sorry. I should have started patching the test that was already existing in GDB an= d then moved to a new one. I believe that a fixup was missed in the process. I will prepare the patch = again! 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