Re: [PATCH 2/4] Handle copy relocations

[Pedro Alves <palves@redhat.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 <palves@redhat.com>
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  <palves@redhat.com>

	* gdb.base/print-file-var-lib1.c: Include <stdio.h> 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 <stdio.h> 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 <dlfcn.h>, <assert.h>,
	<stddef.h> 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 <http://www.gnu.org/licenses/>.  */
 
-int this_version_id = 104;
+#include <stdio.h>
+#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 <http://www.gnu.org/licenses/>.  */
 
-int this_version_id = 203;
+#include <stdio.h>
+#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 <http://www.gnu.org/licenses/>.  */
 
+#ifdef SHLIB_NAME
+# include <dlfcn.h>
+#endif
+
+#include <assert.h>
+#include <stddef.h>
+
+#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 <http://www.gnu.org/licenses/>.  */
+
+#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 */
-- 
2.14.5