| Message ID | 20240102105415.516313-1-yanzhang.wang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] RISC-V: Enable static-pie. | expand |
On 02/01/24 07:54, yanzhang.wang@intel.com wrote: > From: Yanzhang Wang <yanzhang.wang@intel.com> > > This patch referents the commit 374cef3 to add static-pie support. And > because the dummy link map is used when relocating ourselves, so need > not to set __global_pointer$ at this time. > > It will also check whether toolchain supports to build static-pie. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > sysdeps/riscv/configure | 49 ++++++++++++++++++++++++++++++++++++++ > sysdeps/riscv/configure.ac | 27 +++++++++++++++++++++ > sysdeps/riscv/dl-machine.h | 2 +- > 3 files changed, 77 insertions(+), 1 deletion(-) > > diff --git a/sysdeps/riscv/configure b/sysdeps/riscv/configure > index acd1f5e743..c8f01709f8 100644 > --- a/sysdeps/riscv/configure > +++ b/sysdeps/riscv/configure > @@ -31,3 +31,52 @@ printf "%s\n" "$libc_cv_riscv_r_align" >&6; } > config_vars="$config_vars > riscv-r-align = $libc_cv_riscv_r_align" > > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking if the toolchain is sufficient to build static PIE on RISC-V" >&5 > +printf %s "checking if the toolchain is sufficient to build static PIE on RISC-V... " >&6; } > +if test ${libc_cv_static_pie_on_riscv+y} > +then : > + printf %s "(cached) " >&6 > +else $as_nop > + > + cat > conftest1.S <<\EOF > + .globl _start > + .type _start, @function > +_start: > + nop > + > + .data > + /* This should produce an R_RISCV_RELATIVE in the static PIE. */ > + .dword _start > +EOF > + > + libc_cv_static_pie_on_riscv=no > + if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie -nostdlib -fPIE -o conftest1 conftest1.S' > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 > + (eval $ac_try) 2>&5 > + ac_status=$? > + printf "%s\n" "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 > + test $ac_status = 0; }; } \ > + && { ac_try='LC_ALL=C $READELF -Wr conftest1 | grep -q R_RISCV_RELATIVE' > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 > + (eval $ac_try) 2>&5 > + ac_status=$? > + printf "%s\n" "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 > + test $ac_status = 0; }; } \ > + && ! { ac_try='LC_ALL=C $READELF -Wl conftest1 | grep -q INTERP' > + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 > + (eval $ac_try) 2>&5 > + ac_status=$? > + printf "%s\n" "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 > + test $ac_status = 0; }; } > + then > + libc_cv_static_pie_on_riscv=yes > + fi > + rm -rf conftest* > +fi > +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv_static_pie_on_riscv" >&5 > +printf "%s\n" "$libc_cv_static_pie_on_riscv" >&6; } > + > +if test "$libc_cv_static_pie_on_riscv" = yes; then > + printf "%s\n" "#define SUPPORT_STATIC_PIE 1" >>confdefs.h > + > +fi > diff --git a/sysdeps/riscv/configure.ac b/sysdeps/riscv/configure.ac > index dbcc216689..ee3d1ed014 100644 > --- a/sysdeps/riscv/configure.ac > +++ b/sysdeps/riscv/configure.ac > @@ -16,3 +16,30 @@ EOF > fi > rm -rf conftest.*]) > LIBC_CONFIG_VAR([riscv-r-align], [$libc_cv_riscv_r_align]) > + > +dnl Test if the toolchain is new enough for static PIE. > +AC_CACHE_CHECK([if the toolchain is sufficient to build static PIE on RISC-V], > +libc_cv_static_pie_on_riscv, [ > + cat > conftest1.S <<\EOF > + .globl _start > + .type _start, @function > +_start: > + nop > + > + .data > + /* This should produce an R_RISCV_RELATIVE in the static PIE. */ > + .dword _start > +EOF > + > + libc_cv_static_pie_on_riscv=no > + if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie -nostdlib -fPIE -o conftest1 conftest1.S]) \ > + && AC_TRY_COMMAND([LC_ALL=C $READELF -Wr conftest1 | grep -q R_RISCV_RELATIVE]) \ > + && ! AC_TRY_COMMAND([LC_ALL=C $READELF -Wl conftest1 | grep -q INTERP]) > + then > + libc_cv_static_pie_on_riscv=yes > + fi > + rm -rf conftest* ]) > + > +if test "$libc_cv_static_pie_on_riscv" = yes; then > + AC_DEFINE(SUPPORT_STATIC_PIE) > +fi > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > index ffb8c4aaa0..0cbb476c05 100644 > --- a/sysdeps/riscv/dl-machine.h > +++ b/sysdeps/riscv/dl-machine.h > @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[], > gotplt[1] = (ElfW(Addr)) l; > } > > - if (l->l_type == lt_executable) > + if (l->l_type == lt_executable && l->l_scope != NULL) > { > /* The __global_pointer$ may not be defined by the linker if the > $gp register does not be used to access the global variable
On Jan 02 2024, yanzhang.wang@intel.com wrote: > From: Yanzhang Wang <yanzhang.wang@intel.com> > > This patch referents the commit 374cef3 to add static-pie support. And > because the dummy link map is used when relocating ourselves, so need > not to set __global_pointer$ at this time. > > It will also check whether toolchain supports to build static-pie. How did you check that? It doesn't work here: Breakpoint 1, _start () at ../sysdeps/riscv/start.S:50 50 call load_gp 1: x/i $pc => 0x3ff7f59ea4 <_start>: jal 0x3ff7f59ec6 <load_gp> (gdb) ni _start () at ../sysdeps/riscv/start.S:74 74 lla gp, __global_pointer$ 1: x/i $pc => 0x3ff7f59ec6 <load_gp>: auipc gp,0xa0 (gdb) 0x0000003ff7f59eca 74 lla gp, __global_pointer$ 1: x/i $pc => 0x3ff7f59eca <_start+38>: add gp,gp,706 (gdb) load_gp () at ../sysdeps/riscv/start.S:76 76 ret 1: x/i $pc => 0x3ff7f59ece <load_gp+8>: ret (gdb) _start () at ../sysdeps/riscv/start.S:51 51 mv a5, a0 /* rtld_fini. */ 1: x/i $pc => 0x3ff7f59ea8 <_start+4>: mv a5,a0 (gdb) 53 la a0, main 1: x/i $pc => 0x3ff7f59eaa <_start+6>: auipc a0,0x9e (gdb) 0x0000003ff7f59eae 53 la a0, main 1: x/i $pc => 0x3ff7f59eae <_start+10>: ld a0,-74(a0) (gdb) 54 REG_L a1, 0(sp) /* argc. */ 1: x/i $pc => 0x3ff7f59eb2 <_start+14>: ld a1,0(sp) (gdb) i reg a0 a0 0x0 0
On Jan 02 2024, yanzhang.wang@intel.com wrote: > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > index ffb8c4aaa0..0cbb476c05 100644 > --- a/sysdeps/riscv/dl-machine.h > +++ b/sysdeps/riscv/dl-machine.h > @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[], > gotplt[1] = (ElfW(Addr)) l; > } > > - if (l->l_type == lt_executable) > + if (l->l_type == lt_executable && l->l_scope != NULL) This is not the right way to test for static PIE, it tries to access a non-relocated non-zero pointer. The mold linker, when producing relative relocations, puts the addend both in the relocation addend and the relocated field, which is perfectly valid to do, so this is non-zero here.
Hi Andreas, Sorry for the late response. Thanks your comments! And I don't quite follow your comments. Could you please give some further information? > (gdb) ni > _start () at ../sysdeps/riscv/start.S:74 > 74 lla gp, __global_pointer$ > 1: x/i $pc > => 0x3ff7f59ec6 <load_gp>: auipc gp,0xa0 > (gdb) > 0x0000003ff7f59eca 74 lla gp, __global_pointer$ > 1: x/i $pc > => 0x3ff7f59eca <_start+38>: add gp,gp,706 > (gdb) > load_gp () at ../sysdeps/riscv/start.S:76 > 76 ret > 1: x/i $pc > => 0x3ff7f59ece <load_gp+8>: ret > (gdb) > _start () at ../sysdeps/riscv/start.S:51 > 51 mv a5, a0 /* rtld_fini. */ > 1: x/i $pc > => 0x3ff7f59ea8 <_start+4>: mv a5,a0 > (gdb) > 53 la a0, main > 1: x/i $pc > => 0x3ff7f59eaa <_start+6>: auipc a0,0x9e > (gdb) > 0x0000003ff7f59eae 53 la a0, main > 1: x/i $pc > => 0x3ff7f59eae <_start+10>: ld a0,-74(a0) > (gdb) > 54 REG_L a1, 0(sp) /* argc. */ > 1: x/i $pc > => 0x3ff7f59eb2 <_start+14>: ld a1,0(sp) > (gdb) i reg a0 > a0 0x0 0 The first, it seems you have constructed a test case which should fail this patch. But based on your gdb comment, I'm not sure which step is not correct. Should the a0 register be non-zero or the gp register be wrong? --- The code in the if will try to lookup the __global_pointer$ and move to the gp register. It will be failed because l->l_scope is NULL when static-pie. So I added this checking to avoid this failure. > > + if (l->l_type == lt_executable && l->l_scope != NULL) > This is not the right way to test for static PIE, it tries to access a > non-relocated non-zero pointer. The mold linker, when producing relative > relocations, puts the addend both in the relocation addend and the > relocated field, which is perfectly valid to do, so this is non-zero here. The second, do you mean there's a case that l->l_scope is not NULL when built with static-pie? Or we should not access the members in link_map ? Thanks, Yanzhang > -----Original Message----- > From: Andreas Schwab <schwab@suse.de> > Sent: Monday, January 29, 2024 8:46 PM > To: Wang, Yanzhang <yanzhang.wang@intel.com> > Cc: libc-alpha@sourceware.org; adhemerval.zanella@linaro.org > Subject: Re: [PATCH v3] RISC-V: Enable static-pie. > > On Jan 02 2024, yanzhang.wang@intel.com wrote: > > > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > > index ffb8c4aaa0..0cbb476c05 100644 > > --- a/sysdeps/riscv/dl-machine.h > > +++ b/sysdeps/riscv/dl-machine.h > > @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, > struct r_scope_elem *scope[], > > gotplt[1] = (ElfW(Addr)) l; > > } > > > > - if (l->l_type == lt_executable) > > + if (l->l_type == lt_executable && l->l_scope != NULL) > > This is not the right way to test for static PIE, it tries to access a > non-relocated non-zero pointer. The mold linker, when producing relative > relocations, puts the addend both in the relocation addend and the > relocated field, which is perfectly valid to do, so this is non-zero here. > > -- > Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 > 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something > completely different."
On Feb 01 2024, Wang, Yanzhang wrote: > The code in the if will try to lookup the __global_pointer$ and move to the > gp register. It will be failed because l->l_scope is NULL when static-pie. No, l->l_scope is an unrelocated pointer. Since ld.mold puts the addend also in the relocated field, it is read as a nonzero, but invalid pointer. See the static-pie tests in the mold testsuite.
On Tue, 2 Jan 2024, yanzhang.wang@intel.com wrote: > This patch referents the commit 374cef3 to add static-pie support. And > because the dummy link map is used when relocating ourselves, so need > not to set __global_pointer$ at this time. > > It will also check whether toolchain supports to build static-pie. Somehow the check fails to trigger here, causing a build error: riscv64-linux-gnu-gcc -o .../support/test-run-command -nostdlib -nostartfiles -static -static-pie .../csu/rcrt1.o .../csu/crti.o `riscv64-linux-gnu-gcc --print-file-name=crtbeginS.o` .../support/test-run-command.o .../elf/static-stubs.o .../support/libsupport_nonshared.a -Wl,--start-group .../libc.a -lgcc -Wl,--end-group `riscv64-linux-gnu-gcc --print-file-name=crtendS.o` .../csu/crtn.o.../bin/../lib/gcc/riscv64-linux-gnu/14.0.0/../../../../riscv64-linux-gnu/bin/ld: read-only segment has dynamic relocations collect2: error: ld returned 1 exit status make[2]: *** [../Rules:290: .../support/test-run-command] Error 1 I have filed BZ #31773. Maciej
diff --git a/sysdeps/riscv/configure b/sysdeps/riscv/configure index acd1f5e743..c8f01709f8 100644 --- a/sysdeps/riscv/configure +++ b/sysdeps/riscv/configure @@ -31,3 +31,52 @@ printf "%s\n" "$libc_cv_riscv_r_align" >&6; } config_vars="$config_vars riscv-r-align = $libc_cv_riscv_r_align" +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking if the toolchain is sufficient to build static PIE on RISC-V" >&5 +printf %s "checking if the toolchain is sufficient to build static PIE on RISC-V... " >&6; } +if test ${libc_cv_static_pie_on_riscv+y} +then : + printf %s "(cached) " >&6 +else $as_nop + + cat > conftest1.S <<\EOF + .globl _start + .type _start, @function +_start: + nop + + .data + /* This should produce an R_RISCV_RELATIVE in the static PIE. */ + .dword _start +EOF + + libc_cv_static_pie_on_riscv=no + if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie -nostdlib -fPIE -o conftest1 conftest1.S' + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 + (eval $ac_try) 2>&5 + ac_status=$? + printf "%s\n" "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; } \ + && { ac_try='LC_ALL=C $READELF -Wr conftest1 | grep -q R_RISCV_RELATIVE' + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 + (eval $ac_try) 2>&5 + ac_status=$? + printf "%s\n" "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; } \ + && ! { ac_try='LC_ALL=C $READELF -Wl conftest1 | grep -q INTERP' + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 + (eval $ac_try) 2>&5 + ac_status=$? + printf "%s\n" "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; } + then + libc_cv_static_pie_on_riscv=yes + fi + rm -rf conftest* +fi +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $libc_cv_static_pie_on_riscv" >&5 +printf "%s\n" "$libc_cv_static_pie_on_riscv" >&6; } + +if test "$libc_cv_static_pie_on_riscv" = yes; then + printf "%s\n" "#define SUPPORT_STATIC_PIE 1" >>confdefs.h + +fi diff --git a/sysdeps/riscv/configure.ac b/sysdeps/riscv/configure.ac index dbcc216689..ee3d1ed014 100644 --- a/sysdeps/riscv/configure.ac +++ b/sysdeps/riscv/configure.ac @@ -16,3 +16,30 @@ EOF fi rm -rf conftest.*]) LIBC_CONFIG_VAR([riscv-r-align], [$libc_cv_riscv_r_align]) + +dnl Test if the toolchain is new enough for static PIE. +AC_CACHE_CHECK([if the toolchain is sufficient to build static PIE on RISC-V], +libc_cv_static_pie_on_riscv, [ + cat > conftest1.S <<\EOF + .globl _start + .type _start, @function +_start: + nop + + .data + /* This should produce an R_RISCV_RELATIVE in the static PIE. */ + .dword _start +EOF + + libc_cv_static_pie_on_riscv=no + if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -static-pie -nostdlib -fPIE -o conftest1 conftest1.S]) \ + && AC_TRY_COMMAND([LC_ALL=C $READELF -Wr conftest1 | grep -q R_RISCV_RELATIVE]) \ + && ! AC_TRY_COMMAND([LC_ALL=C $READELF -Wl conftest1 | grep -q INTERP]) + then + libc_cv_static_pie_on_riscv=yes + fi + rm -rf conftest* ]) + +if test "$libc_cv_static_pie_on_riscv" = yes; then + AC_DEFINE(SUPPORT_STATIC_PIE) +fi diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h index ffb8c4aaa0..0cbb476c05 100644 --- a/sysdeps/riscv/dl-machine.h +++ b/sysdeps/riscv/dl-machine.h @@ -348,7 +348,7 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[], gotplt[1] = (ElfW(Addr)) l; } - if (l->l_type == lt_executable) + if (l->l_type == lt_executable && l->l_scope != NULL) { /* The __global_pointer$ may not be defined by the linker if the $gp register does not be used to access the global variable
From: Yanzhang Wang <yanzhang.wang@intel.com> This patch referents the commit 374cef3 to add static-pie support. And because the dummy link map is used when relocating ourselves, so need not to set __global_pointer$ at this time. It will also check whether toolchain supports to build static-pie. --- sysdeps/riscv/configure | 49 ++++++++++++++++++++++++++++++++++++++ sysdeps/riscv/configure.ac | 27 +++++++++++++++++++++ sysdeps/riscv/dl-machine.h | 2 +- 3 files changed, 77 insertions(+), 1 deletion(-)