tst-tls6.sh: Replace space with ':' in LD_PRELOAD

Message ID 20240206124208.2428029-1-hjl.tools@gmail.com
State Rejected
Headers
Series tst-tls6.sh: Replace space with ':' in LD_PRELOAD |

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-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

H.J. Lu Feb. 6, 2024, 12:42 p.m. UTC
  Replace space with ':' in LD_PRELOAD.  This fixes [BZ #31344].
---
 nptl/tst-tls6.sh | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
  

Comments

Andreas Schwab Feb. 6, 2024, 12:58 p.m. UTC | #1
On Feb 06 2024, H.J. Lu wrote:

> Replace space with ':' in LD_PRELOAD.  This fixes [BZ #31344].

What does it fix?
  
H.J. Lu Feb. 6, 2024, 1:11 p.m. UTC | #2
On Tue, Feb 6, 2024 at 4:58 AM Andreas Schwab <schwab@suse.de> wrote:
>
> On Feb 06 2024, H.J. Lu wrote:
>
> > Replace space with ':' in LD_PRELOAD.  This fixes [BZ #31344].
>
> What does it fix?

Intel SDE only supports ':' in LD_PRELOAD.
  
Andreas Schwab Feb. 6, 2024, 1:17 p.m. UTC | #3
On Feb 06 2024, H.J. Lu wrote:

> Intel SDE only supports ':' in LD_PRELOAD.

Then fix it.
  
Florian Weimer Feb. 6, 2024, 2 p.m. UTC | #4
* H. J. Lu:

> Replace space with ':' in LD_PRELOAD.  This fixes [BZ #31344].

I think the test currently passes because we use an ELF constructor in
the LD_PRELOAD module to register test cases.  So we just lose some test
coverage by ignoring the LD_PRELOAD modules.  In other cases we rely on
explicit symbol bindings/lookups to make sure that everythin has been
loaded as expected.  But as a minimal fix, this looks okay.

Regarding the actual change:

   LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{$aligned,b,c,d}.so \
+	      | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
+  || fail=1

I believe this could be:

+  LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{$aligned,b,c,d}.so \
+	      | tr ' ' :`" ${tst_tls5} >> $logfile || fail=1

You could use set -- and IFS, but that's probably too obscure.

Thanks,
Florian
  
Jakub Jelinek Feb. 6, 2024, 2:02 p.m. UTC | #5
On Tue, Feb 06, 2024 at 03:00:12PM +0100, Florian Weimer wrote:
> * H. J. Lu:
> 
> > Replace space with ':' in LD_PRELOAD.  This fixes [BZ #31344].
> 
> I think the test currently passes because we use an ELF constructor in
> the LD_PRELOAD module to register test cases.  So we just lose some test
> coverage by ignoring the LD_PRELOAD modules.  In other cases we rely on
> explicit symbol bindings/lookups to make sure that everythin has been
> loaded as expected.  But as a minimal fix, this looks okay.
> 
> Regarding the actual change:
> 
>    LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{$aligned,b,c,d}.so \
> +	      | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
> +  || fail=1
> 
> I believe this could be:
> 
> +  LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{$aligned,b,c,d}.so \
> +	      | tr ' ' :`" ${tst_tls5} >> $logfile || fail=1
> 
> You could use set -- and IFS, but that's probably too obscure.

At least ld.so manpage documents for LD_PRELOAD:
"The items of the list can be separated by spaces or colons, and there is no
support for escaping either separator."
So, is that not true?

	Jakub
  
Adhemerval Zanella Netto Feb. 6, 2024, 2:10 p.m. UTC | #6
On 06/02/24 11:02, Jakub Jelinek wrote:
> On Tue, Feb 06, 2024 at 03:00:12PM +0100, Florian Weimer wrote:
>> * H. J. Lu:
>>
>>> Replace space with ':' in LD_PRELOAD.  This fixes [BZ #31344].
>>
>> I think the test currently passes because we use an ELF constructor in
>> the LD_PRELOAD module to register test cases.  So we just lose some test
>> coverage by ignoring the LD_PRELOAD modules.  In other cases we rely on
>> explicit symbol bindings/lookups to make sure that everythin has been
>> loaded as expected.  But as a minimal fix, this looks okay.
>>
>> Regarding the actual change:
>>
>>    LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{$aligned,b,c,d}.so \
>> +	      | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
>> +  || fail=1
>>
>> I believe this could be:
>>
>> +  LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{$aligned,b,c,d}.so \
>> +	      | tr ' ' :`" ${tst_tls5} >> $logfile || fail=1
>>
>> You could use set -- and IFS, but that's probably too obscure.
> 
> At least ld.so manpage documents for LD_PRELOAD:
> "The items of the list can be separated by spaces or colons, and there is no
> support for escaping either separator."
> So, is that not true?

It is true, LD_PRELOAD accepts both ':' and whitespace:

elf/rtld.c

 866 unsigned int
 867 handle_preload_list (const char *preloadlist, struct link_map *main_map,
 868                      const char *where)
 869 {
[...]
 876       /* Split preload list at space/colon.  */
 877       size_t len = strcspn (p, " :");
[...]
  

Patch

diff --git a/nptl/tst-tls6.sh b/nptl/tst-tls6.sh
index 450ee7870a..9dbb9495c1 100755
--- a/nptl/tst-tls6.sh
+++ b/nptl/tst-tls6.sh
@@ -38,7 +38,8 @@  for aligned in a e f; do
   ${test_wrapper_env} \
   ${run_program_env} \
   LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{$aligned,b,c,d}.so \
-	      | sed 's/:$//;s/: /:/g'`" ${tst_tls5} >> $logfile || fail=1
+	      | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
+  || fail=1
   echo >> $logfile
 
   echo "preload tst-tls5mod{b,$aligned,c,d}.so" >> $logfile
@@ -46,7 +47,8 @@  for aligned in a e f; do
   ${test_wrapper_env} \
   ${run_program_env} \
   LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{b,$aligned,c,d}.so \
-	      | sed 's/:$//;s/: /:/g'`" ${tst_tls5} >> $logfile || fail=1
+	      | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
+  || fail=1
   echo >> $logfile
 
   echo "preload tst-tls5mod{b,c,d,$aligned}.so" >> $logfile
@@ -54,7 +56,8 @@  for aligned in a e f; do
   ${test_wrapper_env} \
   ${run_program_env} \
   LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{b,c,d,$aligned}.so \
-	      | sed 's/:$//;s/: /:/g'`" ${tst_tls5} >> $logfile || fail=1
+	      | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
+  || fail=1
   echo >> $logfile
 done
 
@@ -63,7 +66,8 @@  echo "===============" >> $logfile
 ${test_wrapper_env} \
 ${run_program_env} \
 LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{d,a,b,c,e}.so \
-	    | sed 's/:$//;s/: /:/g'`" ${tst_tls5} >> $logfile || fail=1
+	    | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
+|| fail=1
 echo >> $logfile
 
 echo "preload tst-tls5mod{d,a,b,e,f}" >> $logfile
@@ -71,7 +75,8 @@  echo "===============" >> $logfile
 ${test_wrapper_env} \
 ${run_program_env} \
 LD_PRELOAD="`echo ${common_objpfx}nptl/tst-tls5mod{d,a,b,e,f}.so \
-	    | sed 's/:$//;s/: /:/g'`" ${tst_tls5} >> $logfile || fail=1
+	    | sed 's/:$//;s/: /:/g;s/ /:/g'`" ${tst_tls5} >> $logfile \
+|| fail=1
 echo >> $logfile
 
 exit $fail