elf: Properly remove the initial 'env' command
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_check--master-arm |
fail
|
Patch failed to apply
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
fail
|
Patch failed to apply
|
Commit Message
tst-rtld-list-diagnostics.py is called by
"$(test-wrapper-env) $(objpfx)$(rtld-installed-name) --list-diagnostics"
and $(test-wrapper-env) is set to "$(test-wrapper) env". When there is
a test wrapper, it is incorrect to use:
# Remove the initial 'env' command.
parse_diagnostics(opts.command.split()[1:])
to remove 'env'. Change tst-rtld-list-diagnostics.py to explicitly check
'env' instead. This fixes [BZ #31357].
---
elf/tst-rtld-list-diagnostics.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Comments
* H. J. Lu:
> index 9e70e74bf8..dfba94de64 100644
> --- a/elf/tst-rtld-list-diagnostics.py
> +++ b/elf/tst-rtld-list-diagnostics.py
> @@ -294,7 +294,11 @@ def main(argv):
> check_consistency_with_manual(opts.manual)
>
> # Remove the initial 'env' command.
> - parse_diagnostics(opts.command.split()[1:])
> + options = []
> + for o in opts.command.split()[0:]:
> + if o != 'env':
> + options.append(o)
> + parse_diagnostics(options)
I think you can write this as:
options = opts.command.split()[:]
options.remove('env')
It only removes the first occurrence, but I think that is more correct
anyway.
Thanks,
Florian
On Feb 09 2024, H.J. Lu wrote:
> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> index 9e70e74bf8..dfba94de64 100644
> --- a/elf/tst-rtld-list-diagnostics.py
> +++ b/elf/tst-rtld-list-diagnostics.py
> @@ -294,7 +294,11 @@ def main(argv):
> check_consistency_with_manual(opts.manual)
>
> # Remove the initial 'env' command.
> - parse_diagnostics(opts.command.split()[1:])
> + options = []
> + for o in opts.command.split()[0:]:
> + if o != 'env':
> + options.append(o)
Why does it need to do that in the first place?
On Fri, Feb 9, 2024 at 5:00 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > index 9e70e74bf8..dfba94de64 100644
> > --- a/elf/tst-rtld-list-diagnostics.py
> > +++ b/elf/tst-rtld-list-diagnostics.py
> > @@ -294,7 +294,11 @@ def main(argv):
> > check_consistency_with_manual(opts.manual)
> >
> > # Remove the initial 'env' command.
> > - parse_diagnostics(opts.command.split()[1:])
> > + options = []
> > + for o in opts.command.split()[0:]:
> > + if o != 'env':
> > + options.append(o)
> > + parse_diagnostics(options)
>
> I think you can write this as:
>
> options = opts.command.split()[:]
> options.remove('env')
>
> It only removes the first occurrence, but I think that is more correct
> anyway.
>
Fixed in v2.
Thanks.
* Andreas Schwab:
> On Feb 09 2024, H.J. Lu wrote:
>
>> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
>> index 9e70e74bf8..dfba94de64 100644
>> --- a/elf/tst-rtld-list-diagnostics.py
>> +++ b/elf/tst-rtld-list-diagnostics.py
>> @@ -294,7 +294,11 @@ def main(argv):
>> check_consistency_with_manual(opts.manual)
>>
>> # Remove the initial 'env' command.
>> - parse_diagnostics(opts.command.split()[1:])
>> + options = []
>> + for o in opts.command.split()[0:]:
>> + if o != 'env':
>> + options.append(o)
>
> Why does it need to do that in the first place?
Good question. I must have copied it from scripts/tst-ld-trace.py. I
think we can remove the .split() and run the command string with
shell=True.
Thanks,
Florian
On Fri, Feb 9, 2024 at 5:30 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Andreas Schwab:
>
> > On Feb 09 2024, H.J. Lu wrote:
> >
> >> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> >> index 9e70e74bf8..dfba94de64 100644
> >> --- a/elf/tst-rtld-list-diagnostics.py
> >> +++ b/elf/tst-rtld-list-diagnostics.py
> >> @@ -294,7 +294,11 @@ def main(argv):
> >> check_consistency_with_manual(opts.manual)
> >>
> >> # Remove the initial 'env' command.
> >> - parse_diagnostics(opts.command.split()[1:])
> >> + options = []
> >> + for o in opts.command.split()[0:]:
> >> + if o != 'env':
> >> + options.append(o)
> >
> > Why does it need to do that in the first place?
>
> Good question. I must have copied it from scripts/tst-ld-trace.py. I
> think we can remove the .split() and run the command string with
How does removing .split() work? The argument is
"/export/build/gnu/tools-build/glibc-apx-sde/test-sde.sh env
/export/build/gnu/tools-build/glibc-apx-sde/build-x86_64-linux/elf/ld-linux-x86-64.so.2
--list-diagnostics"
We need .split() to change it to proper arguments.
> shell=True.
>
> Thanks,
> Florian
>
* H. J. Lu:
> On Fri, Feb 9, 2024 at 5:30 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Andreas Schwab:
>>
>> > On Feb 09 2024, H.J. Lu wrote:
>> >
>> >> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
>> >> index 9e70e74bf8..dfba94de64 100644
>> >> --- a/elf/tst-rtld-list-diagnostics.py
>> >> +++ b/elf/tst-rtld-list-diagnostics.py
>> >> @@ -294,7 +294,11 @@ def main(argv):
>> >> check_consistency_with_manual(opts.manual)
>> >>
>> >> # Remove the initial 'env' command.
>> >> - parse_diagnostics(opts.command.split()[1:])
>> >> + options = []
>> >> + for o in opts.command.split()[0:]:
>> >> + if o != 'env':
>> >> + options.append(o)
>> >
>> > Why does it need to do that in the first place?
>>
>> Good question. I must have copied it from scripts/tst-ld-trace.py. I
>> think we can remove the .split() and run the command string with
>
> How does removing .split() work? The argument is
>
> "/export/build/gnu/tools-build/glibc-apx-sde/test-sde.sh env
> /export/build/gnu/tools-build/glibc-apx-sde/build-x86_64-linux/elf/ld-linux-x86-64.so.2
> --list-diagnostics"
>
> We need .split() to change it to proper arguments.
Can you test this?
diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
index 9e70e74bf8..024bd8c320 100644
--- a/elf/tst-rtld-list-diagnostics.py
+++ b/elf/tst-rtld-list-diagnostics.py
@@ -222,7 +222,7 @@ else:
def parse_diagnostics(cmd):
global errors
diag_out = subprocess.run(cmd, stdout=subprocess.PIPE, check=True,
- universal_newlines=True).stdout
+ universal_newlines=True, shell=True).stdout
if diag_out[-1] != '\n':
print('error: ld.so output does not end in newline')
errors += 1
@@ -293,8 +293,7 @@ def main(argv):
if opts.manual:
check_consistency_with_manual(opts.manual)
- # Remove the initial 'env' command.
- parse_diagnostics(opts.command.split()[1:])
+ parse_diagnostics(opts.command)
if errors:
sys.exit(1)
Thanks,
Florian
On Fri, Feb 9, 2024 at 6:53 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Feb 9, 2024 at 5:30 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * Andreas Schwab:
> >>
> >> > On Feb 09 2024, H.J. Lu wrote:
> >> >
> >> >> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> >> >> index 9e70e74bf8..dfba94de64 100644
> >> >> --- a/elf/tst-rtld-list-diagnostics.py
> >> >> +++ b/elf/tst-rtld-list-diagnostics.py
> >> >> @@ -294,7 +294,11 @@ def main(argv):
> >> >> check_consistency_with_manual(opts.manual)
> >> >>
> >> >> # Remove the initial 'env' command.
> >> >> - parse_diagnostics(opts.command.split()[1:])
> >> >> + options = []
> >> >> + for o in opts.command.split()[0:]:
> >> >> + if o != 'env':
> >> >> + options.append(o)
> >> >
> >> > Why does it need to do that in the first place?
> >>
> >> Good question. I must have copied it from scripts/tst-ld-trace.py. I
> >> think we can remove the .split() and run the command string with
> >
> > How does removing .split() work? The argument is
> >
> > "/export/build/gnu/tools-build/glibc-apx-sde/test-sde.sh env
> > /export/build/gnu/tools-build/glibc-apx-sde/build-x86_64-linux/elf/ld-linux-x86-64.so.2
> > --list-diagnostics"
> >
> > We need .split() to change it to proper arguments.
>
> Can you test this?
>
> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> index 9e70e74bf8..024bd8c320 100644
> --- a/elf/tst-rtld-list-diagnostics.py
> +++ b/elf/tst-rtld-list-diagnostics.py
> @@ -222,7 +222,7 @@ else:
> def parse_diagnostics(cmd):
> global errors
> diag_out = subprocess.run(cmd, stdout=subprocess.PIPE, check=True,
> - universal_newlines=True).stdout
> + universal_newlines=True, shell=True).stdout
> if diag_out[-1] != '\n':
> print('error: ld.so output does not end in newline')
> errors += 1
> @@ -293,8 +293,7 @@ def main(argv):
> if opts.manual:
> check_consistency_with_manual(opts.manual)
>
> - # Remove the initial 'env' command.
> - parse_diagnostics(opts.command.split()[1:])
> + parse_diagnostics(opts.command)
>
> if errors:
> sys.exit(1)
>
> Thanks,
> Florian
>
It works. Can you check it in?
Thanks.
* H. J. Lu:
> It works. Can you check it in?
Thanks for checking, pushed.
Florian
@@ -294,7 +294,11 @@ def main(argv):
check_consistency_with_manual(opts.manual)
# Remove the initial 'env' command.
- parse_diagnostics(opts.command.split()[1:])
+ options = []
+ for o in opts.command.split()[0:]:
+ if o != 'env':
+ options.append(o)
+ parse_diagnostics(options)
if errors:
sys.exit(1)