[PATCH] Use correct exit status in ldd (bug 24150)

[PATCH] Use correct exit status in ldd (bug 24150)

[Andreas Schwab]

The message "not a dynamic executable" is not an error, so don't exit with
a nonzero status.
---
 elf/ldd.bash.in | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/elf/ldd.bash.in b/elf/ldd.bash.in
index 467cbf44e9..a879ddd640 100644
--- a/elf/ldd.bash.in
+++ b/elf/ldd.bash.in
@@ -166,10 +166,7 @@ warning: you do not have execution permission for" "\`$file'" >&2
     case $ret in
     1)
       # This can be a non-ELF binary or no binary at all.
-      nonelf "$file" || {
-	echo $"	not a dynamic executable" >&2
-	result=1
-      }
+      nonelf "$file" || echo $"	not a dynamic executable"
       ;;
     0|2)
       try_trace "$RTLD" "$file" || result=1
-- 
2.25.0

-- 
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."

Re: [PATCH] Use correct exit status in ldd (bug 24150)

[Carlos O'Donell]

On 2/3/20 6:25 AM, Andreas Schwab wrote:
> The message "not a dynamic executable" is not an error, so don't exit with
> a nonzero status.

I don't consider that a strong enough reason to make this change given the
potential for breaking scripts that use ldd.

Existing scripts can use ldd to test if a file is a valid dynamic executable,
and may be relying on that behaviour.

Florian only just had eu-elfclassify added to elfutils to help classify binaries
so we could avoid needlessly using ldd for such purposes, but this is relatively
new.

In summary:
- I'd like to see a strong reason to change this.
- User scripts should use eu-elfclassify, but may continue to use ldd.

> ---
>  elf/ldd.bash.in | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/elf/ldd.bash.in b/elf/ldd.bash.in
> index 467cbf44e9..a879ddd640 100644
> --- a/elf/ldd.bash.in
> +++ b/elf/ldd.bash.in
> @@ -166,10 +166,7 @@ warning: you do not have execution permission for" "\`$file'" >&2
>      case $ret in
>      1)
>        # This can be a non-ELF binary or no binary at all.
> -      nonelf "$file" || {
> -	echo $"	not a dynamic executable" >&2
> -	result=1
> -      }
> +      nonelf "$file" || echo $"	not a dynamic executable"

If all rtld in ${RTLDLIST} are non-executable this will cause the script to return
success when it should not.

>        ;;
>      0|2)
>        try_trace "$RTLD" "$file" || result=1
> 


-- 
Cheers,
Carlos.

Re: [PATCH] Use correct exit status in ldd (bug 24150)

[Andreas Schwab]

On Feb 03 2020, Carlos O'Donell wrote:

> If all rtld in ${RTLDLIST} are non-executable this will cause the script to return
> success when it should not.

Why should it not?

Andreas.

-- 
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."

Re: [PATCH] Use correct exit status in ldd (bug 24150)

[Carlos O'Donell]

On 2/3/20 10:58 AM, Andreas Schwab wrote:
> On Feb 03 2020, Carlos O'Donell wrote:
> 
>> If all rtld in ${RTLDLIST} are non-executable this will cause the script to return
>> success when it should not.
> 
> Why should it not?

It is an error if the script has an internal error (lack of executable helper program i.e. ld.so) and cannot carry out the requested user function.

-- 
Cheers,
Carlos.

Re: [PATCH] Use correct exit status in ldd (bug 24150)

[Andreas Schwab]

On Feb 03 2020, Carlos O'Donell wrote:

> On 2/3/20 10:58 AM, Andreas Schwab wrote:
>> On Feb 03 2020, Carlos O'Donell wrote:
>> 
>>> If all rtld in ${RTLDLIST} are non-executable this will cause the script to return
>>> success when it should not.
>> 
>> Why should it not?
>
> It is an error if the script has an internal error (lack of executable helper program i.e. ld.so) and cannot carry out the requested user function.

This is not unlike any shared library including ld.so itself where ldd
returns sucessful.
This case wasn't intended to be an error, otherwise the message would
have been formatted as an error.

Andreas.

-- 
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."

Re: [PATCH] Use correct exit status in ldd (bug 24150)

[Carlos O'Donell]

On 2/3/20 11:52 AM, Andreas Schwab wrote:
> On Feb 03 2020, Carlos O'Donell wrote:
> 
>> On 2/3/20 10:58 AM, Andreas Schwab wrote:
>>> On Feb 03 2020, Carlos O'Donell wrote:
>>> 
>>>> If all rtld in ${RTLDLIST} are non-executable this will cause
>>>> the script to return success when it should not.
>>> 
>>> Why should it not?
>> 
>> It is an error if the script has an internal error (lack of
>> executable helper program i.e. ld.so) and cannot carry out the
>> requested user function.
> 
> This is not unlike any shared library including ld.so itself where
> ldd returns sucessful. This case wasn't intended to be an error,
> otherwise the message would have been formatted as an error.

We should derive what is or is not an error from first principles
based on a desire to maintain compatibility with existing scripts
and to do what is logical for ldd.

If all rtld in ${RTLDLIST} are non-executable then I think we should
return a non-zero exit code. We did not do what the user asked and
so this is a failure. It is also a relatively obscure corner case
so I am not that worried.

It is less clear what should happen in the case of your patch where
the user-requested file is not something that ldd can process and so
there is nothing to do. In that case I'm arguing that we should have
a strong reason to change this since it has the potential to impact
existing scripts written to use ldd.

-- 
Cheers,
Carlos.