elf: Fix tst-nodeps2 test failure.

Message ID 20240110155011.3930473-1-carlos@redhat.com
State Committed
Commit 94aa256a47253d2202240c8696cbe824a969d3cc
Headers
Series elf: Fix tst-nodeps2 test failure. |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm warning Patch is already merged

Commit Message

Carlos O'Donell Jan. 10, 2024, 3:50 p.m. UTC
  When building the testsuite with --enable-hard-coded-path-in-tests
the tst-nodeps2-mod.so is not built with the required DT_RUNPATH
values and the test escapes the test framework and loads the system
libraries and aborts. The fix is to use the existing
$(link-test-modules-rpath-link) variable to set DT_RUNPATH correctly.

No regressions on x86_64.
---
 elf/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
  

Comments

Florian Weimer Jan. 10, 2024, 4:09 p.m. UTC | #1
* Carlos O'Donell:

> When building the testsuite with --enable-hard-coded-path-in-tests
> the tst-nodeps2-mod.so is not built with the required DT_RUNPATH
> values and the test escapes the test framework and loads the system
> libraries and aborts. The fix is to use the existing
> $(link-test-modules-rpath-link) variable to set DT_RUNPATH correctly.
>
> No regressions on x86_64.
> ---
>  elf/Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 600812e573..5d78b659ce 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -3022,6 +3022,8 @@ $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
>  $(objpfx)tst-env-setuid-static.out: $(objpfx)tst-sonamemove-runmod1.so
>  
>  # The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
> +# We do not use $(link-test-modules-rpath-link) since the object has no
> +# DT_NEEDED.
>  $(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os
>  	$(LINK.o) -nostartfiles -nostdlib -shared -o $@ $^
>  tst-nodeps1.so-no-z-defs = yes
> @@ -3032,7 +3034,8 @@ $(objpfx)tst-nodeps1: $(objpfx)tst-nodeps1-mod.so
>  # with the IFUNC resolver reference.
>  $(objpfx)tst-nodeps2-mod.so: $(common-objpfx)libc.so \
>    $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.os
> -	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^
> +	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^ \
> +	$(link-test-modules-rpath-link)
>  $(objpfx)tst-nodeps2.out: \
>    $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.so

Looks good.  Maybe reference the faulty commit
78ca44da0160a0b442f0ca1f253e3360f044b2ec ("elf: Relocate libc.so early
during startup and dlmopen (bug 31083)")?

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian
  
Carlos O'Donell Jan. 10, 2024, 6:06 p.m. UTC | #2
On 1/10/24 11:09, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> When building the testsuite with --enable-hard-coded-path-in-tests
>> the tst-nodeps2-mod.so is not built with the required DT_RUNPATH
>> values and the test escapes the test framework and loads the system
>> libraries and aborts. The fix is to use the existing
>> $(link-test-modules-rpath-link) variable to set DT_RUNPATH correctly.
>>
>> No regressions on x86_64.
>> ---
>>  elf/Makefile | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/elf/Makefile b/elf/Makefile
>> index 600812e573..5d78b659ce 100644
>> --- a/elf/Makefile
>> +++ b/elf/Makefile
>> @@ -3022,6 +3022,8 @@ $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
>>  $(objpfx)tst-env-setuid-static.out: $(objpfx)tst-sonamemove-runmod1.so
>>  
>>  # The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
>> +# We do not use $(link-test-modules-rpath-link) since the object has no
>> +# DT_NEEDED.
>>  $(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os
>>  	$(LINK.o) -nostartfiles -nostdlib -shared -o $@ $^
>>  tst-nodeps1.so-no-z-defs = yes
>> @@ -3032,7 +3034,8 @@ $(objpfx)tst-nodeps1: $(objpfx)tst-nodeps1-mod.so
>>  # with the IFUNC resolver reference.
>>  $(objpfx)tst-nodeps2-mod.so: $(common-objpfx)libc.so \
>>    $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.os
>> -	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^
>> +	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^ \
>> +	$(link-test-modules-rpath-link)
>>  $(objpfx)tst-nodeps2.out: \
>>    $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.so
> 
> Looks good.  Maybe reference the faulty commit
> 78ca44da0160a0b442f0ca1f253e3360f044b2ec ("elf: Relocate libc.so early
> during startup and dlmopen (bug 31083)")?

Good point. Will do.

> Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks!

> Thanks,
> Florian
>
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 600812e573..5d78b659ce 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -3022,6 +3022,8 @@  $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
 $(objpfx)tst-env-setuid-static.out: $(objpfx)tst-sonamemove-runmod1.so
 
 # The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
+# We do not use $(link-test-modules-rpath-link) since the object has no
+# DT_NEEDED.
 $(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os
 	$(LINK.o) -nostartfiles -nostdlib -shared -o $@ $^
 tst-nodeps1.so-no-z-defs = yes
@@ -3032,7 +3034,8 @@  $(objpfx)tst-nodeps1: $(objpfx)tst-nodeps1-mod.so
 # with the IFUNC resolver reference.
 $(objpfx)tst-nodeps2-mod.so: $(common-objpfx)libc.so \
   $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.os
-	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^
+	$(LINK.o) -Wl,--no-as-needed -nostartfiles -nostdlib -shared -o $@ $^ \
+	$(link-test-modules-rpath-link)
 $(objpfx)tst-nodeps2.out: \
   $(objpfx)tst-nodeps1-mod.so $(objpfx)tst-nodeps2-mod.so