public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Riscv shared libraries should not export __global_pointer$.
@ 2017-12-05  1:38 Jim Wilson
  2017-12-05  5:56 ` Palmer Dabbelt
  2017-12-05 22:55 ` [PATCH] Really fix riscv shared library __global_pointer$ problem Jim Wilson
  0 siblings, 2 replies; 3+ messages in thread
From: Jim Wilson @ 2017-12-05  1:38 UTC (permalink / raw)
  To: binutils; +Cc: Jim Wilson

This fixes issue #108 in the riscv-binutils-gdb project on github.
    https://github.com/riscv/riscv-binutils-gdb/issues/108
The problem here is that the riscv linker is exporting __global_pointer$ which
is wrong.  This symbol serves the same function as the mips _gp.  It is set by
the linker script in every executable/shared library and should not be
exported.  Exporting it can lead to linker errors as reported in #108.  The
fix is use HIDDEN for __global_pointer$, same as the mips port uses HIDDEN
for _gp.

The mips port has some nice testcases for this, so I borrowed them to use
for the riscv target.  I also noticed that the riscv specific linker tests
weren't being run because of a typo in the target check.  This is fixed by
this patch also.

This was tested with check-binutils/check-gas/check-ld on riscv32 and riscv64.
There were no regressions.  Committed.

	ld/
	* emulparams/elf32lriscv-defs.sh (SDATA_START_SYMBOLS): Mark
	__global_pointer$ as HIDDEN.
	* testsuite/ld-riscv-elf/gp-hidden-64.rd: New.
	* testsuite/ld-riscv-elf/gp-hidden-lib.rd: New.
	* testsuite/ld-riscv-elf/gp-hidden-lib.s: New.
	* testsuite/ld-riscv-elf/gp-hidden-ver-64.rd: New.
	* testsuite/ld-riscv-elf/gp-hidden-ver.rd: New.
	* testsuite/ld-riscv-elf/gp-hidden-ver.s: New.
	* testsuite/ld-riscv-elf/gp-hidder-ver.ver: New.
	* testsuite/ld-riscv-elf/gp-hidden.rd: New.
	* testsuite/ld-riscv-elf/gp-hidden.s: New.
	* testsuite/ld-riscv-elf/gp-hidden.sd: New.
	* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Change riscv to riscv*.
	Run the new tests with run_ld_link_tests.
---
 ld/emulparams/elf32lriscv-defs.sh             |  2 +-
 ld/testsuite/ld-riscv-elf/gp-hidden-64.rd     |  4 ++++
 ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd    |  5 ++++
 ld/testsuite/ld-riscv-elf/gp-hidden-lib.s     |  6 +++++
 ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd |  6 +++++
 ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd    |  6 +++++
 ld/testsuite/ld-riscv-elf/gp-hidden-ver.s     |  7 ++++++
 ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver   |  1 +
 ld/testsuite/ld-riscv-elf/gp-hidden.rd        |  4 ++++
 ld/testsuite/ld-riscv-elf/gp-hidden.s         |  7 ++++++
 ld/testsuite/ld-riscv-elf/gp-hidden.sd        |  9 +++++++
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp    | 34 ++++++++++++++++++++++++++-
 12 files changed, 89 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden.rd
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden.s
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden.sd

diff --git a/ld/emulparams/elf32lriscv-defs.sh b/ld/emulparams/elf32lriscv-defs.sh
index ab80333289..5b41d5cd02 100644
--- a/ld/emulparams/elf32lriscv-defs.sh
+++ b/ld/emulparams/elf32lriscv-defs.sh
@@ -23,7 +23,7 @@ TEXT_START_ADDR=0x10000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
 COMMONPAGESIZE="CONSTANT (COMMONPAGESIZE)"
 
-SDATA_START_SYMBOLS="__global_pointer$ = . + 0x800;
+SDATA_START_SYMBOLS="HIDDEN (__global_pointer$ = . + 0x800);
     *(.srodata.cst16) *(.srodata.cst8) *(.srodata.cst4) *(.srodata.cst2) *(.srodata .srodata.*)"
 
 INITIAL_READONLY_SECTIONS=".interp         : { *(.interp) } ${CREATE_PIE-${INITIAL_READONLY_SECTIONS}}"
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
new file mode 100644
index 0000000000..aaaec93d8e
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
@@ -0,0 +1,4 @@
+
+Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
+ *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
+[0-9a-f]+ * [0-9a-f]+02 * R_RISCV_64 * [0-9a-f]+ * foo \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
new file mode 100644
index 0000000000..59b4442190
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
@@ -0,0 +1,5 @@
+
+Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
+ *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
+# This must be an absolute relocation, there must not be a _gp reference.
+[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
new file mode 100644
index 0000000000..04f0655857
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
@@ -0,0 +1,6 @@
+	.data
+	.globl	bar
+	.type	bar, @object
+bar:
+	.dc.a	__global_pointer$
+	.size	bar, . - bar
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
new file mode 100644
index 0000000000..57cb13fc4a
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
@@ -0,0 +1,6 @@
+
+Relocation section '\.rela\.dyn' at offset .* contains 2 entries:
+ *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
+# This must be an absolute relocation, there must not be a _gp reference.
+[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
+[0-9a-f]+ * [0-9a-f]+02 * R_RISCV_64 * [0-9a-f]+ * bar \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
new file mode 100644
index 0000000000..dbed24f81e
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
@@ -0,0 +1,6 @@
+
+Relocation section '\.rela\.dyn' at offset .* contains 2 entries:
+ *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
+# This must be an absolute relocation, there must not be a _gp reference.
+[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
+[0-9a-f]+ * [0-9a-f]+01 * R_RISCV_32 * [0-9a-f]+ * bar \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
new file mode 100644
index 0000000000..a197bacd9b
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
@@ -0,0 +1,7 @@
+	.data
+	.globl	foo
+	.type	foo, @object
+foo:
+	.dc.a	bar
+	.dc.a	__global_pointer$
+	.size	foo, . - foo
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
new file mode 100644
index 0000000000..b6b2365188
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
@@ -0,0 +1 @@
+{ global: foo; local: *; };
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.rd b/ld/testsuite/ld-riscv-elf/gp-hidden.rd
new file mode 100644
index 0000000000..dc29f7d058
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden.rd
@@ -0,0 +1,4 @@
+
+Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
+ *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
+[0-9a-f]+ * [0-9a-f]+01 * R_RISCV_32 * [0-9a-f]+ * foo \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.s b/ld/testsuite/ld-riscv-elf/gp-hidden.s
new file mode 100644
index 0000000000..18d3ca3d98
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden.s
@@ -0,0 +1,7 @@
+	.data
+	.globl	blah
+	.type	blah, @object
+blah:
+	.dc.a	foo
+	.dc.a	__global_pointer$
+	.size	blah, . - blah
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.sd b/ld/testsuite/ld-riscv-elf/gp-hidden.sd
new file mode 100644
index 0000000000..f4321c2b1f
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-hidden.sd
@@ -0,0 +1,9 @@
+
+Symbol table '.dynsym' contains [0-9]+ entries:
+ * Num: * Value * Size * Type * Bind * Vis * Ndx * Name
+#...
+Symbol table '.symtab' contains [0-9]+ entries:
+ * Num: * Value * Size * Type * Bind * Vis * Ndx * Name
+#...
+ * [0-9a-f]+: * [0-9a-f]+ * 0 * NOTYPE * LOCAL * DEFAULT .* __global_pointer\$
+#pass
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index c219b18159..9f5959cc3b 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -19,6 +19,38 @@
 # MA 02110-1301, USA.
 #
 
-if [istarget "riscv-*-*"] {
+if [istarget "riscv*-*-*"] {
     run_dump_test "c-lui"
+
+    set abis { rv32gc ilp32 elf32lriscv rv64gc lp64 elf64lriscv }
+    foreach { arch abi emul } $abis {
+	# This checks whether our linker scripts get the scope of
+	# __global_pointer$ right, and thus must therefore use default scripts.
+	set suff64 [string map {ilp32 "" lp64 -64} $abi]
+	run_ld_link_tests [list \
+			       [list "gp scope test ($abi shared library)" \
+				    "-m$emul -shared" "" \
+				    "-march=$arch -mabi=$abi -fpic" \
+				    { gp-hidden-lib.s } \
+				    [list \
+					 "readelf --relocs gp-hidden-lib.rd" \
+					 "readelf --syms gp-hidden.sd"] \
+				    "gp-hidden-lib-${abi}.so"] \
+			       [list "gp scope test ($abi versioned lib)" \
+				    "-m$emul -shared -version-script gp-hidden-ver.ver tmpdir/gp-hidden-lib-${abi}.so" "" \
+				    "-march=$arch -mabi=$abi -fpic" \
+				    { gp-hidden-ver.s } \
+				    [list \
+					 "readelf --relocs gp-hidden-ver${suff64}.rd" \
+					 "readelf --syms gp-hidden.sd"] \
+				    "gp-hidden-ver-${abi}.so"] \
+			       [list "gp scope test ($abi executable)" \
+				    "-m$emul -e 0 -rpath-link . tmpdir/gp-hidden-ver-${abi}.so" "" \
+				    "-march=$arch -mabi=$abi" \
+				    {gp-hidden.s} \
+				    [list \
+					 "readelf --relocs gp-hidden${suff64}.rd" \
+					 "readelf --syms gp-hidden.sd"]\
+				    "gp-hidden-${abi}"]]
+    }
 }
-- 
2.14.1

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] Riscv shared libraries should not export __global_pointer$.
  2017-12-05  1:38 [PATCH] Riscv shared libraries should not export __global_pointer$ Jim Wilson
@ 2017-12-05  5:56 ` Palmer Dabbelt
  2017-12-05 22:55 ` [PATCH] Really fix riscv shared library __global_pointer$ problem Jim Wilson
  1 sibling, 0 replies; 3+ messages in thread
From: Palmer Dabbelt @ 2017-12-05  5:56 UTC (permalink / raw)
  To: Jim Wilson; +Cc: Andrew Waterman, binutils, Jim Wilson

On Mon, 04 Dec 2017 17:38:45 PST (-0800), Jim Wilson wrote:
> This fixes issue #108 in the riscv-binutils-gdb project on github.
>     https://github.com/riscv/riscv-binutils-gdb/issues/108
> The problem here is that the riscv linker is exporting __global_pointer$ which
> is wrong.  This symbol serves the same function as the mips _gp.  It is set by
> the linker script in every executable/shared library and should not be
> exported.  Exporting it can lead to linker errors as reported in #108.  The
> fix is use HIDDEN for __global_pointer$, same as the mips port uses HIDDEN
> for _gp.

I (well, it was really Andrew's idea, but I think he's right) think this might 
just be masking another bug: unlike MIPS, we don't save/restore GP when moving 
between shared libraries on RISC-V.  Thus, actually using GP within a shared 
library is almost certainly a bug: how to you tell what the GP offset is to 
load any particular symbol's value?  We don't bother having a dynamic 
relocation for this because the only benefit of GP is during relaxation, and we 
couldn't handle that in the dynamic linker.

For example, in PIC mode we compile this C code

    $ cat test.c
    long global;
    long func(void) { return global; }

to GOT-relative loads.

    $ riscv64-unknown-linux-gnu-objdump -d test.so 
    ...
    0000000000000468 <func>:
     468:	00002797          	auipc	a5,0x2
     46c:	bc07b783          	ld	a5,-1088(a5) # 2028 <global@@Base-0x20>
     470:	6388                	ld	a0,0(a5)
     472:	8082                	ret

I think the real problem here is that I'm seeing `__global_pointer$` defined in 
even the trivial shared library from above

    $ riscv64-unknown-linux-gnu-objdump -t test.so
    ...
    0000000000002840 g       .got	0000000000000000              __global_pointer$

While this commit and the associated test cases aren't wrong, they're just not 
strong enough: gp-relative addressing should never appear in shared libraries 
(it's OK it shared executables), and therefor the presence of 
`__global_pointer$` probably indicates that something else has gone wrong.  

This is probably OK for now, as it'll at least let user's code build, but I 
think the real bug here is: why is `__global_pointer$` showing up in shared 
libraries?  I'd bet that I screwed something up in glibc...

> The mips port has some nice testcases for this, so I borrowed them to use
> for the riscv target.  I also noticed that the riscv specific linker tests
> weren't being run because of a typo in the target check.  This is fixed by
> this patch also.
>
> This was tested with check-binutils/check-gas/check-ld on riscv32 and riscv64.
> There were no regressions.  Committed.
>
> 	ld/
> 	* emulparams/elf32lriscv-defs.sh (SDATA_START_SYMBOLS): Mark
> 	__global_pointer$ as HIDDEN.
> 	* testsuite/ld-riscv-elf/gp-hidden-64.rd: New.
> 	* testsuite/ld-riscv-elf/gp-hidden-lib.rd: New.
> 	* testsuite/ld-riscv-elf/gp-hidden-lib.s: New.
> 	* testsuite/ld-riscv-elf/gp-hidden-ver-64.rd: New.
> 	* testsuite/ld-riscv-elf/gp-hidden-ver.rd: New.
> 	* testsuite/ld-riscv-elf/gp-hidden-ver.s: New.
> 	* testsuite/ld-riscv-elf/gp-hidder-ver.ver: New.
> 	* testsuite/ld-riscv-elf/gp-hidden.rd: New.
> 	* testsuite/ld-riscv-elf/gp-hidden.s: New.
> 	* testsuite/ld-riscv-elf/gp-hidden.sd: New.
> 	* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Change riscv to riscv*.
> 	Run the new tests with run_ld_link_tests.
> ---
>  ld/emulparams/elf32lriscv-defs.sh             |  2 +-
>  ld/testsuite/ld-riscv-elf/gp-hidden-64.rd     |  4 ++++
>  ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd    |  5 ++++
>  ld/testsuite/ld-riscv-elf/gp-hidden-lib.s     |  6 +++++
>  ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd |  6 +++++
>  ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd    |  6 +++++
>  ld/testsuite/ld-riscv-elf/gp-hidden-ver.s     |  7 ++++++
>  ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver   |  1 +
>  ld/testsuite/ld-riscv-elf/gp-hidden.rd        |  4 ++++
>  ld/testsuite/ld-riscv-elf/gp-hidden.s         |  7 ++++++
>  ld/testsuite/ld-riscv-elf/gp-hidden.sd        |  9 +++++++
>  ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp    | 34 ++++++++++++++++++++++++++-
>  12 files changed, 89 insertions(+), 2 deletions(-)
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden.rd
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden.s
>  create mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden.sd
>
> diff --git a/ld/emulparams/elf32lriscv-defs.sh b/ld/emulparams/elf32lriscv-defs.sh
> index ab80333289..5b41d5cd02 100644
> --- a/ld/emulparams/elf32lriscv-defs.sh
> +++ b/ld/emulparams/elf32lriscv-defs.sh
> @@ -23,7 +23,7 @@ TEXT_START_ADDR=0x10000
>  MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
>  COMMONPAGESIZE="CONSTANT (COMMONPAGESIZE)"
>
> -SDATA_START_SYMBOLS="__global_pointer$ = . + 0x800;
> +SDATA_START_SYMBOLS="HIDDEN (__global_pointer$ = . + 0x800);
>      *(.srodata.cst16) *(.srodata.cst8) *(.srodata.cst4) *(.srodata.cst2) *(.srodata .srodata.*)"
>
>  INITIAL_READONLY_SECTIONS=".interp         : { *(.interp) } ${CREATE_PIE-${INITIAL_READONLY_SECTIONS}}"
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
> new file mode 100644
> index 0000000000..aaaec93d8e
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
> @@ -0,0 +1,4 @@
> +
> +Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
> + *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
> +[0-9a-f]+ * [0-9a-f]+02 * R_RISCV_64 * [0-9a-f]+ * foo \+ 0
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
> new file mode 100644
> index 0000000000..59b4442190
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
> @@ -0,0 +1,5 @@
> +
> +Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
> + *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
> +# This must be an absolute relocation, there must not be a _gp reference.
> +[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
> new file mode 100644
> index 0000000000..04f0655857
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
> @@ -0,0 +1,6 @@
> +	.data
> +	.globl	bar
> +	.type	bar, @object
> +bar:
> +	.dc.a	__global_pointer$
> +	.size	bar, . - bar
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
> new file mode 100644
> index 0000000000..57cb13fc4a
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
> @@ -0,0 +1,6 @@
> +
> +Relocation section '\.rela\.dyn' at offset .* contains 2 entries:
> + *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
> +# This must be an absolute relocation, there must not be a _gp reference.
> +[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
> +[0-9a-f]+ * [0-9a-f]+02 * R_RISCV_64 * [0-9a-f]+ * bar \+ 0
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
> new file mode 100644
> index 0000000000..dbed24f81e
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
> @@ -0,0 +1,6 @@
> +
> +Relocation section '\.rela\.dyn' at offset .* contains 2 entries:
> + *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
> +# This must be an absolute relocation, there must not be a _gp reference.
> +[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
> +[0-9a-f]+ * [0-9a-f]+01 * R_RISCV_32 * [0-9a-f]+ * bar \+ 0
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
> new file mode 100644
> index 0000000000..a197bacd9b
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
> @@ -0,0 +1,7 @@
> +	.data
> +	.globl	foo
> +	.type	foo, @object
> +foo:
> +	.dc.a	bar
> +	.dc.a	__global_pointer$
> +	.size	foo, . - foo
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
> new file mode 100644
> index 0000000000..b6b2365188
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
> @@ -0,0 +1 @@
> +{ global: foo; local: *; };
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.rd b/ld/testsuite/ld-riscv-elf/gp-hidden.rd
> new file mode 100644
> index 0000000000..dc29f7d058
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden.rd
> @@ -0,0 +1,4 @@
> +
> +Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
> + *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
> +[0-9a-f]+ * [0-9a-f]+01 * R_RISCV_32 * [0-9a-f]+ * foo \+ 0
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.s b/ld/testsuite/ld-riscv-elf/gp-hidden.s
> new file mode 100644
> index 0000000000..18d3ca3d98
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden.s
> @@ -0,0 +1,7 @@
> +	.data
> +	.globl	blah
> +	.type	blah, @object
> +blah:
> +	.dc.a	foo
> +	.dc.a	__global_pointer$
> +	.size	blah, . - blah
> diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.sd b/ld/testsuite/ld-riscv-elf/gp-hidden.sd
> new file mode 100644
> index 0000000000..f4321c2b1f
> --- /dev/null
> +++ b/ld/testsuite/ld-riscv-elf/gp-hidden.sd
> @@ -0,0 +1,9 @@
> +
> +Symbol table '.dynsym' contains [0-9]+ entries:
> + * Num: * Value * Size * Type * Bind * Vis * Ndx * Name
> +#...
> +Symbol table '.symtab' contains [0-9]+ entries:
> + * Num: * Value * Size * Type * Bind * Vis * Ndx * Name
> +#...
> + * [0-9a-f]+: * [0-9a-f]+ * 0 * NOTYPE * LOCAL * DEFAULT .* __global_pointer\$
> +#pass
> diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> index c219b18159..9f5959cc3b 100644
> --- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> +++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
> @@ -19,6 +19,38 @@
>  # MA 02110-1301, USA.
>  #
>
> -if [istarget "riscv-*-*"] {
> +if [istarget "riscv*-*-*"] {
>      run_dump_test "c-lui"
> +
> +    set abis { rv32gc ilp32 elf32lriscv rv64gc lp64 elf64lriscv }
> +    foreach { arch abi emul } $abis {
> +	# This checks whether our linker scripts get the scope of
> +	# __global_pointer$ right, and thus must therefore use default scripts.
> +	set suff64 [string map {ilp32 "" lp64 -64} $abi]
> +	run_ld_link_tests [list \
> +			       [list "gp scope test ($abi shared library)" \
> +				    "-m$emul -shared" "" \
> +				    "-march=$arch -mabi=$abi -fpic" \
> +				    { gp-hidden-lib.s } \
> +				    [list \
> +					 "readelf --relocs gp-hidden-lib.rd" \
> +					 "readelf --syms gp-hidden.sd"] \
> +				    "gp-hidden-lib-${abi}.so"] \
> +			       [list "gp scope test ($abi versioned lib)" \
> +				    "-m$emul -shared -version-script gp-hidden-ver.ver tmpdir/gp-hidden-lib-${abi}.so" "" \
> +				    "-march=$arch -mabi=$abi -fpic" \
> +				    { gp-hidden-ver.s } \
> +				    [list \
> +					 "readelf --relocs gp-hidden-ver${suff64}.rd" \
> +					 "readelf --syms gp-hidden.sd"] \
> +				    "gp-hidden-ver-${abi}.so"] \
> +			       [list "gp scope test ($abi executable)" \
> +				    "-m$emul -e 0 -rpath-link . tmpdir/gp-hidden-ver-${abi}.so" "" \
> +				    "-march=$arch -mabi=$abi" \
> +				    {gp-hidden.s} \
> +				    [list \
> +					 "readelf --relocs gp-hidden${suff64}.rd" \
> +					 "readelf --syms gp-hidden.sd"]\
> +				    "gp-hidden-${abi}"]]
> +    }
>  }

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [PATCH] Really fix riscv shared library __global_pointer$ problem.
  2017-12-05  1:38 [PATCH] Riscv shared libraries should not export __global_pointer$ Jim Wilson
  2017-12-05  5:56 ` Palmer Dabbelt
@ 2017-12-05 22:55 ` Jim Wilson
  1 sibling, 0 replies; 3+ messages in thread
From: Jim Wilson @ 2017-12-05 22:55 UTC (permalink / raw)
  To: binutils; +Cc: Jim Wilson

Palmer pointed out that I did this wrong, and Andrew gave some info in private
mail too.  I had to read the ABI again, and study the binutils code some more
to see what was going on, and see that I did get it wrong.  So a new fix, which
is to stop defining __global_pointer$ in shared libraries, and now it doesn't
need to be hidden.  This requires completely rewriting the linker tests I just
added.  I verified that the new tests fail with my previous patch removed, and
work with this patch applied.  Committed.

Jim

	ld/
	* emulparams/elf32lriscv-defs.sh (SDATA_START_SYMBOLS): Remove HIDDEN.
	Don't define __global_pointer$ when CREATE_SHLIB.
	* testsuite/ld-riscv-elf/gp-hidden-64.rd,
	* testsuite/ld-riscv-elf/gp-hidden-lib.rd,
	* testsuite/ld-riscv-elf/gp-hidden-lib.s,
	* testsuite/ld-riscv-elf/gp-hidden-ver-64.rd,
	* testsuite/ld-riscv-elf/gp-hidden-ver.rd,
	* testsuite/ld-riscv-elf/gp-hidden-ver.s,
	* testsuite/ld-riscv-elf/gp-hidden-ver.ver,
	* testsuite/ld-riscv-elf/gp-hidden.rd,
	* testsuite/ld-riscv-elf/gp-hidden.s,
	* testsuite/ld-riscv-elf/gp-hidden.sd: Delete.
	* testsuite/ld-riscv-elf/gp-test-lib.sd,
	* testsuite/ld-riscv-elf/gp-test.s,
	* testsuite/ld-riscv-elf/gp-test.sd: New.
	* testsuite/ld-riscv-elf/ld-riscv-elf.exp: Rewrite gp tests.
---
 ld/emulparams/elf32lriscv-defs.sh                  |  3 +-
 ld/testsuite/ld-riscv-elf/gp-hidden-64.rd          |  4 ---
 ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd         |  5 ----
 ld/testsuite/ld-riscv-elf/gp-hidden-lib.s          |  6 ----
 ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd      |  6 ----
 ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd         |  6 ----
 ld/testsuite/ld-riscv-elf/gp-hidden-ver.s          |  7 -----
 ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver        |  1 -
 ld/testsuite/ld-riscv-elf/gp-hidden.rd             |  4 ---
 .../ld-riscv-elf/{gp-hidden.sd => gp-test-lib.sd}  |  6 ++--
 .../ld-riscv-elf/{gp-hidden.s => gp-test.s}        |  3 +-
 ld/testsuite/ld-riscv-elf/gp-test.sd               |  5 ++++
 ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp         | 35 ++++++++--------------
 13 files changed, 23 insertions(+), 68 deletions(-)
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
 delete mode 100644 ld/testsuite/ld-riscv-elf/gp-hidden.rd
 rename ld/testsuite/ld-riscv-elf/{gp-hidden.sd => gp-test-lib.sd} (70%)
 rename ld/testsuite/ld-riscv-elf/{gp-hidden.s => gp-test.s} (65%)
 create mode 100644 ld/testsuite/ld-riscv-elf/gp-test.sd

diff --git a/ld/emulparams/elf32lriscv-defs.sh b/ld/emulparams/elf32lriscv-defs.sh
index 5b41d5cd02..29ed6d8542 100644
--- a/ld/emulparams/elf32lriscv-defs.sh
+++ b/ld/emulparams/elf32lriscv-defs.sh
@@ -23,7 +23,8 @@ TEXT_START_ADDR=0x10000
 MAXPAGESIZE="CONSTANT (MAXPAGESIZE)"
 COMMONPAGESIZE="CONSTANT (COMMONPAGESIZE)"
 
-SDATA_START_SYMBOLS="HIDDEN (__global_pointer$ = . + 0x800);
+SDATA_START_SYMBOLS="__global_pointer$ = . + 0x800;"
+SDATA_START_SYMBOLS="${CREATE_SHLIB-${SDATA_START_SYMBOLS}}
     *(.srodata.cst16) *(.srodata.cst8) *(.srodata.cst4) *(.srodata.cst2) *(.srodata .srodata.*)"
 
 INITIAL_READONLY_SECTIONS=".interp         : { *(.interp) } ${CREATE_PIE-${INITIAL_READONLY_SECTIONS}}"
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
deleted file mode 100644
index aaaec93d8e..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden-64.rd
+++ /dev/null
@@ -1,4 +0,0 @@
-
-Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
- *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
-[0-9a-f]+ * [0-9a-f]+02 * R_RISCV_64 * [0-9a-f]+ * foo \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
deleted file mode 100644
index 59b4442190..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.rd
+++ /dev/null
@@ -1,5 +0,0 @@
-
-Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
- *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
-# This must be an absolute relocation, there must not be a _gp reference.
-[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s b/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
deleted file mode 100644
index 04f0655857..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden-lib.s
+++ /dev/null
@@ -1,6 +0,0 @@
-	.data
-	.globl	bar
-	.type	bar, @object
-bar:
-	.dc.a	__global_pointer$
-	.size	bar, . - bar
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
deleted file mode 100644
index 57cb13fc4a..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden-ver-64.rd
+++ /dev/null
@@ -1,6 +0,0 @@
-
-Relocation section '\.rela\.dyn' at offset .* contains 2 entries:
- *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
-# This must be an absolute relocation, there must not be a _gp reference.
-[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
-[0-9a-f]+ * [0-9a-f]+02 * R_RISCV_64 * [0-9a-f]+ * bar \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
deleted file mode 100644
index dbed24f81e..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.rd
+++ /dev/null
@@ -1,6 +0,0 @@
-
-Relocation section '\.rela\.dyn' at offset .* contains 2 entries:
- *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
-# This must be an absolute relocation, there must not be a _gp reference.
-[0-9a-f]+ * 0+03 * R_RISCV_RELATIVE * [0-9a-f]+
-[0-9a-f]+ * [0-9a-f]+01 * R_RISCV_32 * [0-9a-f]+ * bar \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
deleted file mode 100644
index a197bacd9b..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.s
+++ /dev/null
@@ -1,7 +0,0 @@
-	.data
-	.globl	foo
-	.type	foo, @object
-foo:
-	.dc.a	bar
-	.dc.a	__global_pointer$
-	.size	foo, . - foo
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver b/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
deleted file mode 100644
index b6b2365188..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden-ver.ver
+++ /dev/null
@@ -1 +0,0 @@
-{ global: foo; local: *; };
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.rd b/ld/testsuite/ld-riscv-elf/gp-hidden.rd
deleted file mode 100644
index dc29f7d058..0000000000
--- a/ld/testsuite/ld-riscv-elf/gp-hidden.rd
+++ /dev/null
@@ -1,4 +0,0 @@
-
-Relocation section '\.rela\.dyn' at offset .* contains 1 entry:
- *Offset * Info * Type * Sym\. *Value * Sym\. *Name \+ Addend
-[0-9a-f]+ * [0-9a-f]+01 * R_RISCV_32 * [0-9a-f]+ * foo \+ 0
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.sd b/ld/testsuite/ld-riscv-elf/gp-test-lib.sd
similarity index 70%
rename from ld/testsuite/ld-riscv-elf/gp-hidden.sd
rename to ld/testsuite/ld-riscv-elf/gp-test-lib.sd
index f4321c2b1f..c590e6c712 100644
--- a/ld/testsuite/ld-riscv-elf/gp-hidden.sd
+++ b/ld/testsuite/ld-riscv-elf/gp-test-lib.sd
@@ -1,9 +1,9 @@
-
 Symbol table '.dynsym' contains [0-9]+ entries:
  * Num: * Value * Size * Type * Bind * Vis * Ndx * Name
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
  * Num: * Value * Size * Type * Bind * Vis * Ndx * Name
+#failif
+#...
+ * [0-9a-f]+: * [0-9a-f]+ * 0 * NOTYPE * GLOBAL * DEFAULT .* __global_pointer\$
 #...
- * [0-9a-f]+: * [0-9a-f]+ * 0 * NOTYPE * LOCAL * DEFAULT .* __global_pointer\$
-#pass
diff --git a/ld/testsuite/ld-riscv-elf/gp-hidden.s b/ld/testsuite/ld-riscv-elf/gp-test.s
similarity index 65%
rename from ld/testsuite/ld-riscv-elf/gp-hidden.s
rename to ld/testsuite/ld-riscv-elf/gp-test.s
index 18d3ca3d98..a92c75e946 100644
--- a/ld/testsuite/ld-riscv-elf/gp-hidden.s
+++ b/ld/testsuite/ld-riscv-elf/gp-test.s
@@ -2,6 +2,5 @@
 	.globl	blah
 	.type	blah, @object
 blah:
-	.dc.a	foo
-	.dc.a	__global_pointer$
+	.dc.a	0
 	.size	blah, . - blah
diff --git a/ld/testsuite/ld-riscv-elf/gp-test.sd b/ld/testsuite/ld-riscv-elf/gp-test.sd
new file mode 100644
index 0000000000..30c148255f
--- /dev/null
+++ b/ld/testsuite/ld-riscv-elf/gp-test.sd
@@ -0,0 +1,5 @@
+Symbol table '.symtab' contains [0-9]+ entries:
+ * Num: * Value * Size * Type * Bind * Vis * Ndx * Name
+#...
+ * [0-9a-f]+: * [0-9a-f]+ * 0 * NOTYPE * GLOBAL * DEFAULT .* __global_pointer\$
+#pass
diff --git a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
index 9f5959cc3b..ea09a165ae 100644
--- a/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
+++ b/ld/testsuite/ld-riscv-elf/ld-riscv-elf.exp
@@ -24,33 +24,22 @@ if [istarget "riscv*-*-*"] {
 
     set abis { rv32gc ilp32 elf32lriscv rv64gc lp64 elf64lriscv }
     foreach { arch abi emul } $abis {
-	# This checks whether our linker scripts get the scope of
-	# __global_pointer$ right, and thus must therefore use default scripts.
+	# This checks whether our linker scripts handle __global_pointer$
+	# correctly.  It should be defined in executables and PIE, but not
+	# in shared libraries.
 	set suff64 [string map {ilp32 "" lp64 -64} $abi]
 	run_ld_link_tests [list \
-			       [list "gp scope test ($abi shared library)" \
+			       [list "gp test ($abi shared library)" \
 				    "-m$emul -shared" "" \
 				    "-march=$arch -mabi=$abi -fpic" \
-				    { gp-hidden-lib.s } \
-				    [list \
-					 "readelf --relocs gp-hidden-lib.rd" \
-					 "readelf --syms gp-hidden.sd"] \
-				    "gp-hidden-lib-${abi}.so"] \
-			       [list "gp scope test ($abi versioned lib)" \
-				    "-m$emul -shared -version-script gp-hidden-ver.ver tmpdir/gp-hidden-lib-${abi}.so" "" \
-				    "-march=$arch -mabi=$abi -fpic" \
-				    { gp-hidden-ver.s } \
-				    [list \
-					 "readelf --relocs gp-hidden-ver${suff64}.rd" \
-					 "readelf --syms gp-hidden.sd"] \
-				    "gp-hidden-ver-${abi}.so"] \
-			       [list "gp scope test ($abi executable)" \
-				    "-m$emul -e 0 -rpath-link . tmpdir/gp-hidden-ver-${abi}.so" "" \
+				    { gp-test.s } \
+				    [list "readelf --syms gp-test-lib.sd"] \
+				    "gp-test-lib-${abi}.so"] \
+			       [list "gp test ($abi executable)" \
+				    "-m$emul" "" \
 				    "-march=$arch -mabi=$abi" \
-				    {gp-hidden.s} \
-				    [list \
-					 "readelf --relocs gp-hidden${suff64}.rd" \
-					 "readelf --syms gp-hidden.sd"]\
-				    "gp-hidden-${abi}"]]
+				    { gp-test.s } \
+				    [list "readelf --syms gp-test.sd"] \
+				    "gp-test-${abi}"]]
     }
 }
-- 
2.14.1

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-12-05 22:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-05  1:38 [PATCH] Riscv shared libraries should not export __global_pointer$ Jim Wilson
2017-12-05  5:56 ` Palmer Dabbelt
2017-12-05 22:55 ` [PATCH] Really fix riscv shared library __global_pointer$ problem Jim Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).