From patchwork Sat Jul 20 01:10:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 33744 Received: (qmail 30100 invoked by alias); 20 Jul 2019 01:11:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 30087 invoked by uid 89); 20 Jul 2019 01:11:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=surprised, STOP X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 20 Jul 2019 01:11:02 +0000 Received: by mail-wr1-f68.google.com with SMTP id n9so33893769wru.0 for ; Fri, 19 Jul 2019 18:11:01 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:4eeb:42ff:feef:f164? ([2001:8a0:f913:f700:4eeb:42ff:feef:f164]) by smtp.gmail.com with ESMTPSA id g11sm37261605wrq.92.2019.07.19.18.10.58 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Fri, 19 Jul 2019 18:10:59 -0700 (PDT) Subject: Re: [PATCH 2/4] Handle copy relocations To: Tom Tromey , gdb-patches@sourceware.org References: <20190627145235.21222-1-tromey@adacore.com> <20190627145235.21222-3-tromey@adacore.com> From: Pedro Alves Message-ID: Date: Sat, 20 Jul 2019 02:10:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20190627145235.21222-3-tromey@adacore.com> On 6/27/19 3:52 PM, Tom Tromey wrote: > In ELF, if a data symbol is defined in a shared library and used by > the main program, it will be subject to a "copy relocation". To be more correct, this depends on ABI, or how you compile your binary. Compiling the main binary as PIC/PIE may or may not use copy relocations and use GOT accesses instead, for example. > In this > scenario, the main program has a copy of the symbol in question, and a > relocation that tells ld.so to copy the data from the shared library. > Then the symbol in the main program is used to satisfy all references. > > This patch changes gdb to handle this scenario. Data symbols coming > from ELF shared libraries get a special flag that indicates that the > symbol's address may be subject to copy relocation. > > I looked briefly into handling copy relocations by looking at the > actual relocations in the main program, but this seemed difficult to > do with BFD. > > Note that no caching is done here. Perhaps this could be changed if > need be; I wanted to avoid possible problems with either objfile > lifetimes and changes, or conflicts with the long-term (vapor-ware) > objfile splitting project. I'm surprised to find no tests in this patch? I'm not sure whether this is the right approach, since it seems to assume that interposition/preemption happens, while it may not, given possibility of attribute visibility hidden/protected. Ideally whatever design we take considers how no-preemption would be addressed. This came up earlier here: https://sourceware.org/ml/gdb-patches/2017-10/msg00710.html I've taken the examples from that email and converted them to a proper testcase today. See below. With current master, we get: FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=0: 'print-file-var-lib2.c'::this_version_id == v2 FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=1: 'print-file-var-lib1.c'::this_version_id == v1 FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=0: version_id_main=1: 'print-file-var-lib2.c'::this_version_id == v2 FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=1: version_id_main=0: 'print-file-var-lib2.c'::this_version_id == v2 FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=1: version_id_main=1: 'print-file-var-lib1.c'::this_version_id == v1 FAIL: gdb.base/print-file-var.exp: hidden=1: dlopen=1: version_id_main=1: 'print-file-var-lib2.c'::this_version_id == v2 Your patch curiously doesn't make a difference here. Could we add something here to show what your patch improves? But I'm wondering whether fixing the hidden=1 cases would fit with the design of your patch, or does that scenario require thinking about all this differently? From ffe918c15e9dc10540ed1590d9d37fee3fcea844 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Sat, 20 Jul 2019 01:20:35 +0100 Subject: [PATCH] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol Make gdb.base/print-file-var.exp test all combinations of: - attribute hidden in the this_version_id symbols or not - dlopen or not - this_version_id symbol in main file or not I did not reindent the body of "test" in order to keep the patch readable. To be reindended before pushing. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * gdb.base/print-file-var-lib1.c: Include and "print-file-var.h". (this_version_id) Use ATTRIBUTE_VISIBILITY. (get_version_1): Print this_version_id and its address. * gdb.base/print-file-var-lib2.c: Include and "print-file-var.h". (this_version_id) Use ATTRIBUTE_VISIBILITY. (get_version_2): Print this_version_id and its address. * gdb.base/print-file-var-main.c: Include , , and "print-file-var.h". [VERSION_ID_MAIN] (this_version_id): Define. (main): Define v0. Use dlopen if SHLIB_NAME is defined. * gdb.base/print-file-var.exp (test): New, factored out from top level. (top level): Test all combinations of attribute hidden or not, dlopen or not, and this_version_id symbol in main file or not. --- gdb/testsuite/gdb.base/print-file-var-lib1.c | 7 ++- gdb/testsuite/gdb.base/print-file-var-lib2.c | 6 ++- gdb/testsuite/gdb.base/print-file-var-main.c | 38 +++++++++++--- gdb/testsuite/gdb.base/print-file-var.exp | 76 +++++++++++++++++++++------- gdb/testsuite/gdb.base/print-file-var.h | 26 ++++++++++ 5 files changed, 127 insertions(+), 26 deletions(-) create mode 100644 gdb/testsuite/gdb.base/print-file-var.h diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c index b5f4fb90b39..aec04a9b02b 100644 --- a/gdb/testsuite/gdb.base/print-file-var-lib1.c +++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c @@ -14,10 +14,15 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ -int this_version_id = 104; +#include +#include "print-file-var.h" + +ATTRIBUTE_VISIBILITY 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 28bd1acb17f..4dfdfa04c99 100644 --- a/gdb/testsuite/gdb.base/print-file-var-lib2.c +++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c @@ -14,10 +14,14 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ -int this_version_id = 203; +#include +#include "print-file-var.h" + +ATTRIBUTE_VISIBILITY 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 ddc54f14d98..29d4fed22d1 100644 --- a/gdb/testsuite/gdb.base/print-file-var-main.c +++ b/gdb/testsuite/gdb.base/print-file-var-main.c @@ -14,21 +14,45 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +#ifdef SHLIB_NAME +# include +#endif + +#include +#include + +#include "print-file-var.h" + extern int get_version_1 (void); extern int get_version_2 (void); +#if VERSION_ID_MAIN +ATTRIBUTE_VISIBILITY int this_version_id = 55; +#endif + int main (void) { +#if VERSION_ID_MAIN + int vm = this_version_id; +#endif int v1 = get_version_1 (); - int v2 = get_version_2 (); + int v2; - if (v1 != 104) - return 1; - /* The value returned by get_version_2 depends on the target system. */ - if (v2 != 104 && v2 != 203) - return 2; +#ifdef SHLIB_NAME + { + void *handle = dlopen (SHLIB_NAME, RTLD_LAZY); + int (*getver2) (void); + + assert (handle != NULL); + + getver2 = (int (*)(void)) dlsym (handle, "get_version_2"); + + v2 = getver2 (); + } +#else + v2 = get_version_2 (); +#endif return 0; /* STOP */ } - diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp index 1f733fb4dee..4f4afc161e2 100644 --- a/gdb/testsuite/gdb.base/print-file-var.exp +++ b/gdb/testsuite/gdb.base/print-file-var.exp @@ -17,15 +17,23 @@ if {[skip_shlib_tests]} { return -1 } -set executable print-file-var-main +proc test {hidden dlopen version_id_main} { + global srcdir subdir + +set main "print-file-var-main" + +set suffix "-hidden$hidden-dlopen$dlopen-version_id_main$version_id_main" + +set executable $main$suffix set lib1 "print-file-var-lib1" set lib2 "print-file-var-lib2" -set libobj1 [standard_output_file ${lib1}.so] -set libobj2 [standard_output_file ${lib2}.so] +set libobj1 [standard_output_file ${lib1}$suffix.so] +set libobj2 [standard_output_file ${lib2}$suffix.so] set lib_opts { debug additional_flags=-fPIC } +lappend lib_opts "additional_flags=-DHIDDEN=$hidden" if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \ ${libobj1} \ @@ -37,10 +45,21 @@ if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \ ${lib_opts} ] != "" } { return -1 } -if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \ + +set main_opts [list debug shlib=${libobj1}] + +if {$dlopen} { + lappend main_opts "shlib_load" "additional_flags=-DSHLIB_NAME=\"$libobj2\"" +} else { + lappend main_opts "shlib=${libobj2}" +} + +lappend main_opts "additional_flags=-DVERSION_ID_MAIN=$version_id_main" + +if { [gdb_compile "${srcdir}/${subdir}/${main}.c" \ [standard_output_file ${executable}] \ executable \ - [list debug shlib=${libobj1} shlib=${libobj2}]] + $main_opts] != ""} { return -1 } @@ -55,7 +74,7 @@ if ![runto_main] { } # Try printing "this_version_num" qualified with the name of the file -# where the variables are defined. There are two global variables +# where the variables are defined. There are three global variables # with that name, and some systems such as GNU/Linux merge them # into one single entity, while some other systems such as Windows # keep them separate. In the first situation, we have to verify @@ -65,28 +84,51 @@ if ![runto_main] { # defined in the given filename. # # To avoid adding target-specific code in this testcase, the program -# sets two local variable named 'v1' and 'v2' with the value of +# sets three local variables named 'vm', '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'. +# Get past the initialization of the v* variables. set bp_location \ - [gdb_get_line_number "STOP" "${executable}.c"] -gdb_test "break $executable.c:$bp_location" \ + [gdb_get_line_number "STOP" "${main}.c"] +gdb_test "break $main.c:$bp_location" \ "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \ - "breapoint past v1 & v2 initialization" + "breapoint at STOP marker" gdb_test "continue" \ "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \ "continue to STOP marker" -# Now check the value of this_version_id in both print-file-var-lib1.c -# and print-file-var-lib2.c. +# Now check the value of this_version_id in all of +# print-file-var-main.c, print-file-var-lib1.c and +# print-file-var-lib2.c. + +# Compare the values of $sym1 and $sym2. +proc compare {sym1 sym2} { + # Done this way instead of comparing the symbols with "print $sym1 + # == sym2" in GDB directly so that the values of the symbols end + # up visible in the logs, for debug purposes. + set vsym1 [get_integer_valueof $sym1 -1] + set vsym2 [get_integer_valueof $sym2 -1] + gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2" +} + +if $version_id_main { + compare "'print-file-var-main.c'::this_version_id" "vm" + compare "this_version_id" "vm" +} -gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \ - " = 1" +compare "'print-file-var-lib1.c'::this_version_id" "v1" +compare "'print-file-var-lib2.c'::this_version_id" "v2" -gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \ - " = 1" +} + +foreach_with_prefix hidden {0 1} { + foreach_with_prefix dlopen {0 1} { + foreach_with_prefix version_id_main {0 1} { + test $hidden $dlopen $version_id_main + } + } +} diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h new file mode 100644 index 00000000000..fe7a3460edb --- /dev/null +++ b/gdb/testsuite/gdb.base/print-file-var.h @@ -0,0 +1,26 @@ +/* This testcase is part of GDB, the GNU debugger. + Copyright 2019 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 . */ + +#ifndef PRINT_FILE_VAR_H +#define PRINT_FILE_VAR_H + +#if HIDDEN +# define ATTRIBUTE_VISIBILITY __attribute__((visibility ("hidden"))) +#else +# define ATTRIBUTE_VISIBILITY +#endif + +#endif /* PRINT_FILE_VAR_H */