un-nice things in “set -x”

Reported by Thorsten Glaser on 2013-05-12
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
mksh
Medium
Unassigned
PLD Linux
Undecided
Unassigned

Bug Description

One:

tg@blau:~ $ mksh-R41c -xc $'cat <<"EOF"\nfoo\nEOF\n'
+ cat
+ <<"EOF"
foo
tg@blau:~ $ mksh -xc $'cat <<"EOF"\nfoo\nEOF\n'
+ <<"EOF"cat
foo

Problem: no whitespace after here document marker.

Two:

tg@blau:~ $ mksh -xc 'foo=$(bash --version 2>&1 | head -1); echo "=$foo="'
+ + head -1
+ foo='+ + 2>&1 bash --version'
+ echo '=+ + 2>&1 bash --version='
=+ + 2>&1 bash --version=
tg@blau:~ $ mksh-R41c -xc 'foo=$(bash --version 2>&1 | head -1); echo "=$foo="'
+ bash --version
+ head -1
+ 2>&1
+ foo=GNU bash, version 2.05b.0(1)-release (i386-ecce-mirbsd10)
+ echo =GNU bash, version 2.05b.0(1)-release (i386-ecce-mirbsd10)=
=GNU bash, version 2.05b.0(1)-release (i386-ecce-mirbsd10)=
tg@blau:~ $ mksh -xc 'foo=$(bash --version 2>&1 | head -1); echo "=$foo="'
+ + head -1
+ foo='+ + 2>&1 bash --version'
+ echo '=+ + 2>&1 bash --version='
=+ + 2>&1 bash --version=

Questionable: redirection in effect while printing trace.
Maybe not so nice: PS4 shown twice (what happens with subsequent things?)

Given: stderr redirections/parsing never safe while “set -x” is in effect.
But maybe we can do better? Something like a trace fd?

tg@blau:~ $ mksh-R41c -xc 'foo=$((echo foo; bash --version) 2>&1 | head -1); echo "=$foo="'
+ 2>&1
+ head -1
+ foo=+ echo foo
+ echo =+ echo foo=
=+ echo foo=
tg@blau:~ $ mksh -xc 'foo=$((echo foo; bash --version) 2>&1 | head -1); echo "=$foo="'
+ + head -1
+ foo='+ 2>&1 + echo foo'
+ echo '=+ 2>&1 + echo foo='
=+ 2>&1 + echo foo=

This clearly shows that having the redirection in effect is not a new thing,
it just affects more cases now. (The “+ 2>&1 +” is also a cosmetic.)

Maybe some sort of buffering or pre-preparing the output? Have to look at this again.

Thanks to arekm from PLD Linux for reporting these issues.

Thorsten Glaser (mirabilos) wrote :

Nice DoS:

{{{
tg@blau:~ $ PS4='$(echo foo) + '
tg@blau:~ $ set -x

^C/bin/mksh: can't fork - try again
 + echo foo
foo + echo foo
[…]
}}}

Solution: disable Flag(XTRACE) while expanding PS4, hard (possibly make Flag(XTRACE) a bitfield).

Thorsten Glaser (mirabilos) wrote :

POSIX:

   -x
          The shell shall write to standard error a trace for each command after it expands the command and
          before it executes it. It is unspecified whether the command that turns tracing off is traced.

GNU bash (master 2013-05-26 4.2 4.037):

manpage ⇒ -x
                    After expanding each simple command, for command, case
                    command, select command, or arithmetic for command,
                    display the expanded value of PS4, followed by the
                    command and its expanded arguments or associated word
                    list.

info ⇒ -x
               Print a trace of simple commands, for commands, case
               commands, select commands, and arithmetic for commands and
               their arguments or associated word lists after they are
               expanded and before they are executed. The value of the PS4
               variable is expanded and the resultant value is printed
               before the command and its expanded arguments.

   BASH_XTRACEFD
          If set to an integer corresponding to a valid file descriptor,
          Bash will write the trace output generated when `set -x' is
          enabled to that file descriptor. This allows tracing output to
          be separated from diagnostic and error messages. The file
          descriptor is closed when BASH_XTRACEFD is unset or assigned a
          new value. Unsetting BASH_XTRACEFD or assigning it the empty
          string causes the trace output to be sent to the standard error.
          Note that setting BASH_XTRACEFD to 2 (the standard error file
          descriptor) and then unsetting it will result in the standard
          error being closed.

Thorsten Glaser (mirabilos) wrote :

This makes me think a bit.

We could do:

“as soon as ‘set -x’ is run, clone stderr, and then use that for any future traces”

We used to do:

“print some parts, do the I/O redir, print some more parts, run the command”, rinse repeat

We now do:

“take note of some parts, do the I/O redir, print everything at once, run the command”, rinse repeat, with minor bugs

We could do either when implementing BASH_XTRACEFD though… (also, just displaying TCOM is probably wrong). This needs serious thought.

Thorsten Glaser (mirabilos) wrote :

This is a bit tricky. I don’t quite like how BASH_XTRACEFD works.

I personally like the idea to dup stderr at the time of
running “set -x” the best (any subsequent “set +x” will
close the dup; any subsequent “set -x” will close the
dup if open, then dup whatever stderr currently is anew).

However, none of these scenarios work well with subshells.
We could do something environment-y to pass the fd number
of the dup down to the subshell, but that’s get funny to
decide who dups or closes what when. Consider:

parentshell: FOO_DUPFD=25 mksh subshell

subshell: exec 25>foo; set -x

This would either mean that the shell has to dup FOO_DUPFD
upon startup if it’s set, or trace all redirections (which
we could use to avoid the dup in the first place… except
if stderr was actually closed…), or that the set -x in the
subshell would go to the file foo instead of to what the
parent’s goes.

Considering all this, and that XTRACE is not inherited by
subshells anyway (just sub-environments in the same shell),
I’d say ignore the fact and just dup on “set -x”?

Does anyone else have any ideas/comments on this?

Thorsten Glaser (mirabilos) wrote :

New option -o inherit-xtrace requested in IRC by
18:41⎜*** glen is ~<email address hidden> (i see dead processes)
(also apparently PLD guy).

Thorsten Glaser (mirabilos) wrote :

Oh, and how about doing something like 'exec 3<&x' to redirect xtrace to fd#3?

Doesn’t mix well with 'mksh -x foo' though.

Also, 'exec x>&3' can't be extended that way, or 'exec file<&x'…
but one could do 'exec 3>foo; exec 3<&x'.

Also, need to think about whether this should dup or not.

affects: mksh → pld-linux
Changed in pld-linux:
status: New → Confirmed
affects: pld-linux → mksh
Changed in mksh:
status: Confirmed → New
Changed in pld-linux:
status: New → Confirmed
Thorsten Glaser (mirabilos) wrote :

Fix committed, will be in R47.

*** Please test! ***

Changed in mksh:
importance: Undecided → Medium
status: New → Fix Committed
Changed in mksh:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers