Re: [PATCH v3] Make sure GDB uses a valid shell when starting the inferior and to perform the "shell" command

[Doug Evans <xdje42@gmail.com>]

On Sun, Jul 26, 2015 at 12:26 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Sunday, July 26 2015, Doug Evans wrote:
>> ...
>> Hi.
>>
>> I'd like to not have this patch checked in, at least not yet.
>>
>> I'm going to leave security as a separate thread.
>> The topic here is just convenience and assistance (IIUC -
>> please correct me if I'm wrong).
>
> It is just assistance, indeed..  Security is definitely not the focus
> here.
>
>> Having an internally hardcoded list of shells (good or bad) suggests
>> to me there's got to be a better way.
>
> I'm definitely open to suggestions.
>
>> Another thing that bothers me is that if SHELL
>> is set to something gdb thinks is bad, gdb will
>> try to be "clever" and override that setting.
>> If a tool is going to be helpful, I think it
>> also needs a mode to not be. It's hard to
>> work around hardwired cleverness when
>> you don't want it. Hopefully in this instance
>> we can avoid adding an option though.
>
> Yeah.  This can be easily fixed with (yet another) setting.  'set
> use-valid-shell on/off', maybe?
>
>> As a strawman, what if gdb first tests $SHELL
>> (e.g., $SHELL -c 'exit 42' or some such)
>> and if that doesn't work warn the user,
>> but otherwise leave things as is?
>> One could defer doing the test until the first
>> need for $SHELL.
>> And if $SHELL isn't usable, leave it to the
>> user to fix the problem.
>
> So you're suggesting that we only warn the user about the invalid shell,
> instead of deciding to use /bin/sh without asking her?
>
> As much as I think it *is* useful to have GDB default to /bin/sh if
> $SHELL is /sbin/nologin (for example), I am OK with just warning the
> user without taking any action.
>
> So, to summarize: what would you think of a patch that:
>
> - tested $SHELL (as you proposed; $SHELL -c 'exit 42').
>
> - if the test fails, warn the user about it.  If 'set use-valid-shell'
>   is on, continue using /bin/sh; otherwise, just error out.
>
> ?
>
> Thanks,

Assuming others are ok with it, I'd say let's go with the test,
and leave use-valid-shell for another day.
IIUC we tripped over this because of a misconfigured build-bot,
which we can easily fix. It's not clear to me that a new user option
is warranted. They're using gdb. If they don't know about $SHELL
and /bin/sh we can educate them - and one place we can do that
is in the warning we print if the test fails.
[I'm all for having more descriptive/explanatory warnings/errors
that assist users in fixing the issue.]