Re: Command substitution in here-documents
On Fri, Aug 17, 2018 at 09:08:35PM +0800, Herbert Xu wrote:
> Ron Yorston wrote:
> > I've been continuing to worry about this.
> >
> > In the old code (prior to commit 7c245aa) the TNL token that was
> > detected in list() had actually been read. The current code uses
> > peektoken().
> >
> > When parseheredoc() is called in interactive mode the last call to
> > readtoken() is in the code to set the prompt. Even though the text
> > of that token is long gone it's used anyway. Exactly what then happens
> > on a given platform depends on how memory allocation is handled.
> >
> > I can make the error go away by explicitly reading the TNL token when
> > peektoken() detects a newline and we're about to call parseheredoc().
> >
> > Of course, I may be wrong.
>
> The real problem is the fact that we call expandstr which reenters
> the parser and the parser is not reentrant.
>
> We need to save the parser state and restore it around expandstr.
> I'm working on a fix.
This patch should fix the problem:
---8<---
parser: Do not push token back before parseheredoc
When we read the first token in list() we use peektoken instead
of readtoken as the following code needs to use the same token
again. However, this is wrong when we're in a here-document as
it will clobber the saved token without resetting the tokpushback
flag.
This patch fixes it by doing the tokpushback after parseheredoc
and setting lasttoken again if parseheredoc was called.
Reported-by: Ron Yorston
Fixes: 7c245aa8ed33 ("[PARSER] Simplify EOF/newline handling in...")
Fixes: ee5cbe9fd6bc ("[SHELL] Optimize dash -c "command" to avoid a fork")
Signed-off-by: Herbert Xu
diff --git a/src/parser.c b/src/parser.c
index c4e6378..1f9e8ec 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -166,7 +166,7 @@ list(int nlflag)
n1 = NULL;
for (;;) {
- switch (peektoken()) {
+ switch (readtoken()) {
case TNL:
if (!(nlflag & 1))
break;
@@ -177,9 +177,12 @@ list(int nlflag)
if (!n1 && (nlflag & 1))
n1 = NEOF;
parseheredoc();
+ tokpushback++;
+ lasttoken = TEOF;
return n1;
}
+ tokpushback++;
checkkwd = CHKNL | CHKKWD | CHKALIAS;
if (nlflag == 2 && tokendlist[peektoken()])
return n1;
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: Command substitution in here-documents
I've been continuing to worry about this. In the old code (prior to commit 7c245aa) the TNL token that was detected in list() had actually been read. The current code uses peektoken(). When parseheredoc() is called in interactive mode the last call to readtoken() is in the code to set the prompt. Even though the text of that token is long gone it's used anyway. Exactly what then happens on a given platform depends on how memory allocation is handled. I can make the error go away by explicitly reading the TNL token when peektoken() detects a newline and we're about to call parseheredoc(). Of course, I may be wrong. Ron --- src/parser.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/parser.c b/src/parser.c index c4e6378..b9bc0ca 100644 --- a/src/parser.c +++ b/src/parser.c @@ -170,6 +170,7 @@ list(int nlflag) case TNL: if (!(nlflag & 1)) break; + readtoken(); parseheredoc(); return n1; -- 2.17.1
Re: Command substitution in here-documents
Op 13-08-18 om 18:30 schreef Ron Yorston: > Simon Ser wrote: >> On August 13, 2018 4:22 PM, Martijn Dekker wrote: >>> - The latest release is 0.5.10.2. I can't reproduce the bug at all in >>> 0.5.10, 0.5.10.1 or 0.5.10.2. >> >> This is interesting. I can reproduce with the latest commit in master. > > I'm with Simon, the bug is present in current master for me. Yes, interesting. > This is on x86_64 Linux. Which platform are you using Martijn? I compile my test shells on x86_64 Mac OS X 10.11.6. Compiler: Apple LLVM version 8.0.0 (clang-800.0.42.1) Tried it with current master, can't reproduce it there either. - M.
Re: Command substitution in here-documents
Simon Ser wrote: >On August 13, 2018 4:22 PM, Martijn Dekker wrote: >> - The latest release is 0.5.10.2. I can't reproduce the bug at all in >> 0.5.10, 0.5.10.1 or 0.5.10.2. > >This is interesting. I can reproduce with the latest commit in master. I'm with Simon, the bug is present in current master for me. This is on x86_64 Linux. Which platform are you using Martijn? Ron
Re: Command substitution in here-documents
Hi, Thanks both for your replies. On August 13, 2018 4:22 PM, Martijn Dekker wrote: > - The latest release is 0.5.10.2. I can't reproduce the bug at all in > 0.5.10, 0.5.10.1 or 0.5.10.2. This is interesting. I can reproduce with the latest commit in master. > - I can only reproduce the bug on dash 0.5.9 and 0.5.9.1 in interactive > mode, inputting the commands manually. I cannot reproduce it when > running a script (including a dot script sourced using a '.' command in > interactive mode). Yeah, I can confirm this only happens in interactive mode.
Re: Command substitution in here-documents
Op 11-08-18 om 12:08 schreef Ron Yorston: > - Fixing this (other than using the blunt instrument of reverting the > faulty commit) is beyond my pay grade. Someone with a better > understanding of the code will need to take a look. My observations: - The latest release is 0.5.10.2. I can't reproduce the bug at all in 0.5.10, 0.5.10.1 or 0.5.10.2. - I can only reproduce the bug on dash 0.5.9 and 0.5.9.1 in interactive mode, inputting the commands manually. I cannot reproduce it when running a script (including a dot script sourced using a '.' command in interactive mode). - The error message produced can be ... interesting, e.g.: $ dash-0.5.9.1 $ while read -r f; do echo "$f"; done < `echo hi` > EOF dash-0.5.9.1: 1: ad -r f; do echo "$f"; done <
Re: Command substitution in here-documents
Simon Ser wrote: >I'm trying to use command substitution in here-documents. Here's my script: > > cat < `echo hi` > EOF > >This seems not to work in dash 0.5.9.1. It fails with this error: > > dash: 1: > : not found 'git bisect' points to commit 7c245aa8ed33ba5db30eef9369d67036a05b0371 ([PARSER] Simplify EOF/newline handling in list parser) as being at fault. This dates from October 2014 so the last good tagged release is 0.5.8. Reverting the commit makes the problem go away. Some observations: - The error only occurs when the commands are run from the command line; if run from within a script it works. This is probably because... - It appears that the prompt is being injected into the text of the command substitution. If PS2 is an empty string the command works. - Fixing this (other than using the blunt instrument of reverting the faulty commit) is beyond my pay grade. Someone with a better understanding of the code will need to take a look. Ron
