Re: Assigning to BASHPID fails silently

[Chet Ramey]

On 10/20/16 3:57 PM, Dan Douglas wrote:

>> No, it's trivial.  It's an assignment failure.  The fix is to remove
>> the readonly attribute.
> 
> Makes sense to me. I noticed the problem on the same day this was
> reported while trying to compare two environments while eliminating all
> the non-constant dynamic variables from the comparison, that I could
> only override BASHPID by calling bash with:

It's interesting how the reports cluster like that.  The behavior has
been the same since December, 2006 (development implementation, very
soon after bash-3.2 released) and February, 2009 (bash-4.0 released).

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Assigning to BASHPID fails silently

[Dan Douglas]

On Thu, Oct 20, 2016 at 2:35 PM, Chet Ramey  wrote:
> On 10/20/16 11:32 AM, Martijn Dekker wrote:
>
>> So, in some contexts this bug causes a premature exit of the shell, in
>> others it causes a premature exit of a loop. This bug hunt could get
>> interesting.
>
> No, it's trivial.  It's an assignment failure.  The fix is to remove
> the readonly attribute.

Makes sense to me. I noticed the problem on the same day this was
reported while trying to compare two environments while eliminating all
the non-constant dynamic variables from the comparison, that I could
only override BASHPID by calling bash with:

env BASHPID= -- bash -c ...

Seems removing readonly is the thing to do to make it work as
described in variables.c without having to call bash through env.

Re: Assigning to BASHPID fails silently

[Chet Ramey]

On 10/20/16 11:32 AM, Martijn Dekker wrote:

> So, in some contexts this bug causes a premature exit of the shell, in
> others it causes a premature exit of a loop. This bug hunt could get
> interesting.

No, it's trivial.  It's an assignment failure.  The fix is to remove
the readonly attribute.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Assigning to BASHPID fails silently

[lolilolicon]

On Fri, Oct 21, 2016 at 1:31 AM, lolilolicon  wrote:
> bash simply aborts this command.

I think I mean "list".

Re: Assigning to BASHPID fails silently

[lolilolicon]

On Thu, Oct 20, 2016 at 11:32 PM, Martijn Dekker  wrote:
> #! /usr/local/bin/bash
> insubshell() {
> return "$((BASHPID=$$))"
> # ^ fatal typo
> }
> for ((i=0; i<1; i++)); do insubshell; done
> echo $i
> insubshell || echo ok
> echo continuing
>
> The output of this script on my copy of bash-4.4.0 is consistently:
>
> | 0
> | continuing
>
> Clearly, that 0 should be 1. The 'insubshell' function somehow
> manages to interrupt the 'for' loop.

It's not so much the for loop. See,

$ BASHPID= echo k
bash: BASHPID: readonly variable
k
$ BASHPID=; echo k
$ echo k
k

See the second k is not printed. bash simply aborts this command. In
your case, the for compound command.
The bug probably is very clear to Chet already.

Re: Assigning to BASHPID fails silently

[Martijn Dekker]

Op 20-10-16 om 14:22 schreef Greg Wooledge:
> On Wed, Oct 19, 2016 at 11:53:37PM +0200, Martijn Dekker wrote:
>> Assigning to BASHPID most certainly does have an effect. Since you
>> didn't quote that part, I think you might have missed my point that
>> attempting this will silently exit the shell without any error message,
>> causing the problem to be hard to track down.
> 
> Cannot reproduce:
> 
> imadev:~$ bash
> imadev:~$ echo $$
> 5659
> imadev:~$ BASHPID=923
> imadev:~$ echo $$
> 5659
> imadev:~$ exit
> imadev:~$ 

Urgh...

#! /usr/local/bin/bash
insubshell() {
return "$((BASHPID=$$))"
# ^ fatal typo
}
insubshell
echo continuing

The above script failed to produce output *once* (when run as 'bash
test.sh' with bash 4.4) and then output "continuing" ever since, so I
was suspecting a race condition. I can't reproduce it that way now for
the life of me, though.

However, a tweak proves I didn't lose my mind after all:

#! /usr/local/bin/bash
insubshell() {
return "$((BASHPID=$$))"
# ^ fatal typo
}
for ((i=0; i<1; i++)); do insubshell; done
echo $i
insubshell || echo ok
echo continuing

The output of this script on my copy of bash-4.4.0 is consistently:

| 0
| continuing

Clearly, that 0 should be 1. The 'insubshell' function somehow
manages to interrupt the 'for' loop.

So, in some contexts this bug causes a premature exit of the shell, in
others it causes a premature exit of a loop. This bug hunt could get
interesting.

This is on bash 4.4.0-release compiled by myself with default options on
Mac OS X 10.11.6 using "Apple LLVM version 7.3.0 (clang-703.0.31)",
architecture x86_64.

HTH,

- M.

Re: Assigning to BASHPID fails silently

[Chet Ramey]

On 10/19/16 5:53 PM, Martijn Dekker wrote:
> Op 19-10-16 om 15:18 schreef Chet Ramey:
>> On 10/17/16 2:38 PM, Martijn Dekker wrote:
>>> bash 4.4.0 (I did not investigate other versions) does not produce an
>>> error message if you try to assign something to the BASHPID readonly
>>> using either arithmetic or normal assignment. Other readonlies produce a
>>> message on an assignment attempt. BASHPID seems to be an exception.
>>
>> BASHPID is a dynamic variable.  There should be a sentence in the man
>> page that says assignments to it have no effect (as it does for GROUPS
>> and FUNCNAME, for example).
> 
> Assigning to BASHPID most certainly does have an effect. Since you
> didn't quote that part, I think you might have missed my point that
> attempting this will silently exit the shell without any error message,

This is a consequence of it being marked as readonly, and you running it
in a configuration where assignment errors cause the shell to exit.  It
shouldn't be marked as readonly, and it should be documented that
assignments to it are discarded.

> In what possible context would assigning to any of these variables make
> sense, or be an indication of anything other than a fatal bug in the
> script? I think they should all be readonly, and produce a proper
> diagnostic error message upon exit if assigning is attempted.

OK, you've made your position clear.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Assigning to BASHPID fails silently

[PePa]

Picking 2 allows old scripts that work to keep working. Changing to 1
would change the functionality of formerly working scripts in very
undesirable ways. ;-)

> 1. BASHPID is readonly, therefore assignment to it is fatal and the script 
> exits
> (with an error message printed). That's what my previous patch did.
> 
> 2. BASHPID is not read-only, but changes to it are discarded (with the null
> assignement function). Assignments to BASHPID are non-fatal, and it's possible
> to unset it. Once it's unset, it's magical meaning is lost. (I think this is
> what Chet is proposing). noro_bashpid.patch
> 
>> In what possible context would assigning to any of these variables make
>> sense, or be an indication of anything other than a fatal bug in the
>> script? I think they should all be readonly, and produce a proper
>> diagnostic error message upon exit if assigning is attempted.
> [...]
> 
> I wonder the same thing. I don't understand the reasoning for picking (2).
> 

Re: Assigning to BASHPID fails silently

[Greg Wooledge]

On Wed, Oct 19, 2016 at 11:53:37PM +0200, Martijn Dekker wrote:
> Assigning to BASHPID most certainly does have an effect. Since you
> didn't quote that part, I think you might have missed my point that
> attempting this will silently exit the shell without any error message,
> causing the problem to be hard to track down.

Cannot reproduce:

imadev:~$ bash
imadev:~$ echo $$
5659
imadev:~$ BASHPID=923
imadev:~$ echo $$
5659
imadev:~$ exit
imadev:~$ 

Re: Assigning to BASHPID fails silently

[Eduardo A . Bustamante López]

On Wed, Oct 19, 2016 at 11:53:37PM +0200, Martijn Dekker wrote:
[...]
> Assigning to BASHPID most certainly does have an effect. Since you
> didn't quote that part, I think you might have missed my point that
> attempting this will silently exit the shell without any error message,
> causing the problem to be hard to track down. This is different from
> GROUPS and FUNCNAME, where the shell silently continues (causing the
> problem to be hard to track down in a completely different way -- if
> anything, that's worse!).

I think he did get your point. There's definitely a bug here. It should be
either:

1. BASHPID is readonly, therefore assignment to it is fatal and the script exits
(with an error message printed). That's what my previous patch did.

2. BASHPID is not read-only, but changes to it are discarded (with the null
assignement function). Assignments to BASHPID are non-fatal, and it's possible
to unset it. Once it's unset, it's magical meaning is lost. (I think this is
what Chet is proposing). noro_bashpid.patch

> In what possible context would assigning to any of these variables make
> sense, or be an indication of anything other than a fatal bug in the
> script? I think they should all be readonly, and produce a proper
> diagnostic error message upon exit if assigning is attempted.
[...]

I wonder the same thing. I don't understand the reasoning for picking (2).

-- 
Eduardo Bustamante
https://dualbus.me/
diff --git a/variables.c b/variables.c
index 2e8b38c..5120f2e 100644
--- a/variables.c
+++ b/variables.c
@@ -1462,7 +1462,7 @@ get_bashpid (var)
   p = itos (pid);
 
   FREE (value_cell (var));
-  VSETATTR (var, att_integer|att_readonly);
+  VSETATTR (var, att_integer);
   var_setvalue (var, p);
   return (var);
 }
@@ -1767,7 +1767,7 @@ initialize_dynamic_variables ()
   VSETATTR (v, att_integer);
 
   INIT_DYNAMIC_VAR ("BASHPID", (char *)NULL, get_bashpid, null_assign);
-  VSETATTR (v, att_integer|att_readonly);
+  VSETATTR (v, att_integer);
 
 #if defined (HISTORY)
   INIT_DYNAMIC_VAR ("HISTCMD", (char *)NULL, get_histcmd, 
(sh_var_assign_func_t *)NULL);

Re: Assigning to BASHPID fails silently

[Martijn Dekker]

Op 19-10-16 om 15:18 schreef Chet Ramey:
> On 10/17/16 2:38 PM, Martijn Dekker wrote:
>> bash 4.4.0 (I did not investigate other versions) does not produce an
>> error message if you try to assign something to the BASHPID readonly
>> using either arithmetic or normal assignment. Other readonlies produce a
>> message on an assignment attempt. BASHPID seems to be an exception.
> 
> BASHPID is a dynamic variable.  There should be a sentence in the man
> page that says assignments to it have no effect (as it does for GROUPS
> and FUNCNAME, for example).

Assigning to BASHPID most certainly does have an effect. Since you
didn't quote that part, I think you might have missed my point that
attempting this will silently exit the shell without any error message,
causing the problem to be hard to track down. This is different from
GROUPS and FUNCNAME, where the shell silently continues (causing the
problem to be hard to track down in a completely different way -- if
anything, that's worse!).

> It probably should not be readonly, though
> it has been marked as such in previous versions of bash.

In what possible context would assigning to any of these variables make
sense, or be an indication of anything other than a fatal bug in the
script? I think they should all be readonly, and produce a proper
diagnostic error message upon exit if assigning is attempted.

If you're keeping them non-readonly for backwards compatibility reasons,
then IMHO that is misguided; assignments silently fail, which is far
worse than erroring out. Unsetting GROUPS and FUNCNAME first does work,
but it's a rare script that unsets variables before use. Maybe a
compromise would be to introduce a semi-readonly state, where unset
works but assignments fail.

Thanks,

- Martijn

Re: Assigning to BASHPID fails silently

[Chet Ramey]

On 10/17/16 2:38 PM, Martijn Dekker wrote:
> bash 4.4.0 (I did not investigate other versions) does not produce an
> error message if you try to assign something to the BASHPID readonly
> using either arithmetic or normal assignment. Other readonlies produce a
> message on an assignment attempt. BASHPID seems to be an exception.

BASHPID is a dynamic variable.  There should be a sentence in the man
page that says assignments to it have no effect (as it does for GROUPS
and FUNCNAME, for example).  It probably should not be readonly, though
it has been marked as such in previous versions of bash.  I will fix
the man page first.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Assigning to BASHPID fails silently

[Eduardo A . Bustamante López]

On Mon, Oct 17, 2016 at 08:38:29PM +0200, Martijn Dekker wrote:
[...]

The BASHPID variable is initialized with null_assign as its "assign_func". The
default value for this attribute in newly created variables is NULL.

The problem here is caused by the following code from variables.c (function is
bind_variable_internal):

2753   else if (entry->assign_func)  /* array vars have assign functions 
now */
2754 {
2755   INVALIDATE_EXPORTSTR (entry);
2756   newval = (aflags & ASS_APPEND) ? make_variable_value (entry, 
value, aflags) : value;
2757   if (assoc_p (entry))
2758 entry = (*(entry->assign_func)) (entry, newval, -1, savestring 
("0"));
2759   else if (array_p (entry))
2760 entry = (*(entry->assign_func)) (entry, newval, 0, 0);
2761   else
2762 entry = (*(entry->assign_func)) (entry, newval, -1, 0);
2763   if (newval != value)
2764 free (newval);
2765   return (entry);
2766 }
2767   else
2768 {
2769 assign_value:
2770   if ((readonly_p (entry) && (aflags & ASS_FORCE) == 0) || 
noassign_p (entry))
2771 {
2772   if (readonly_p (entry))
2773 err_readonly (name_cell (entry));
2774   return (entry);
2775 }
2776 
2777   /* Variables which are bound are visible. */
2778   VUNSETATTR (entry, att_invisible);

If a variable has a non-NULL assign_func, it will not run the readonly_p
(entry) check, and therefore, no error message is printed. I'm not sure why the
code exits, but it seems clear to me that the assign_func should be NULL in
this case.

I attach a patch with the "fix" (disclaimer, I'm not sure this is the proper
fix, that's for Chet to decide).

Patched bash:

dualbus@yaqui:~/local/src/gnu/bash$ ./bash -c 'BASHPID=1; echo $?'
./bash: BASHPID: readonly variable

-- 
Eduardo Bustamante
https://dualbus.me/
diff --git a/variables.c b/variables.c
index 2e8b38c..8cd5f58 100644
--- a/variables.c
+++ b/variables.c
@@ -1766,7 +1766,7 @@ initialize_dynamic_variables ()
   INIT_DYNAMIC_VAR ("LINENO", (char *)NULL, get_lineno, assign_lineno);
   VSETATTR (v, att_integer);
 
-  INIT_DYNAMIC_VAR ("BASHPID", (char *)NULL, get_bashpid, null_assign);
+  INIT_DYNAMIC_VAR ("BASHPID", (char *)NULL, get_bashpid, 
(sh_var_assign_func_t *)NULL);
   VSETATTR (v, att_integer|att_readonly);
 
 #if defined (HISTORY)

Assigning to BASHPID fails silently

[Martijn Dekker]

bash 4.4.0 (I did not investigate other versions) does not produce an
error message if you try to assign something to the BASHPID readonly
using either arithmetic or normal assignment. Other readonlies produce a
message on an assignment attempt. BASHPID seems to be an exception.


Particularly annoying is that a non-interactive shell will exit silently
on a mistaken attempt to assign to BASHPID. I was making a bash function
to determine if we're in a subshell. Because of the lack of error
message, it took some doing to find my typo in
return "$((BASHPID = $$))"
(of course, comparison is '==' and not '='). Not even 'set -x' will give
a clue; it exits before producing relevant tracing output.


Proof below. (The exit status of the previous command is included in the
prompt in the output below.)

[0]$ UID=0
bash: UID: readonly variable
[1]$ BASHPID=0
[1]$ ((UID=0))
bash: UID: readonly variable
[1]$ ((BASHPID=0))
[1]$

Thanks,

- Martijn