NetBSD Problem Report #50827
From www@NetBSD.org Thu Feb 18 07:31:16 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.NetBSD.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 690D57ACB8
for <gnats-bugs@gnats.NetBSD.org>; Thu, 18 Feb 2016 07:31:16 +0000 (UTC)
Message-Id: <20160218073115.38AAF7ACC5@mollari.NetBSD.org>
Date: Thu, 18 Feb 2016 07:31:15 +0000 (UTC)
From: rhansen@rhansen.org
Reply-To: rhansen@rhansen.org
To: gnats-bugs@NetBSD.org
Subject: sh -c ': "${x=$((1))}"' gives bogus syntax error
X-Send-Pr-Version: www-1.0
>Number: 50827
>Category: bin
>Synopsis: sh -c ': "${x=$((1))}"' gives bogus syntax error
>Confidential: no
>Severity: critical
>Priority: medium
>Responsible: bin-bug-people
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Feb 18 07:35:00 +0000 2016
>Closed-Date: Tue Feb 23 13:04:50 +0000 2016
>Last-Modified: Tue Feb 23 13:04:50 +0000 2016
>Originator: Richard Hansen
>Release: 7.0
>Organization:
>Environment:
NetBSD foo.example.com 7.0 NetBSD 7.0 (GENERIC.201509250726Z) amd64
>Description:
/bin/sh incorrectly reports a syntax error ("Missing '}'") with the following line:
: "${x=$((1))}"
The following work as expected:
: ${x=$((1))}
: ${x="$((1))"}
: "${x="$((1))"}
>How-To-Repeat:
$ sh -c ': "${x=$((1))}"'
sh: Syntax error: Missing '}'
>Fix:
>Release-Note:
>Audit-Trail:
From: Richard Hansen <rhansen@rhansen.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/50827: sh -c ': "${x=$((1))}"' gives bogus syntax error
Date: Thu, 18 Feb 2016 02:59:08 -0500
Apologies -- the last example:
> : "${x="$((1))"}
should have been:
: "${x="$((1))"}"
(I forgot the second close double quote)
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/50827: sh -c ': "${x=$((1))}"' gives bogus syntax error
Date: Fri, 19 Feb 2016 16:31:13 +0700
Date: Thu, 18 Feb 2016 07:35:00 +0000 (UTC)
From: rhansen@rhansen.org
Message-ID: <20160218073500.BFF977ACD0@mollari.NetBSD.org>
| /bin/sh incorrectly reports a syntax error ("Missing '}'") with the following line:
|
| : "${x=$((1))}"
It also mishandles ...
sh -c ': ${x="$(("1"))"}'
sh: Syntax error: Missing '))'
and similar cases.
I believe the following patch might fix both of those problems (the 4
lines deleted fix rhansen's problem, the extra manipulation of varnest
and associated code fixes the $(( )) case.)
These bugs have been there a long time, so once verified, the fix should
be pulled up to the netbsd 6 & netbsd 7 series (all variants.)
kre
--- parser.c.old 2014-08-30 06:38:31.000000000 +0700
+++ parser.c 2016-02-19 16:18:50.000000000 +0700
@@ -1106,13 +1106,11 @@
--parenlevel;
} else {
if (pgetc() == ')') {
+ if (varnest > 0) /*always*/
+ varnest--;
if (--arinest == 0) {
USTPUTC(CTLENDARI, out);
syntax = prevsyntax;
- if (syntax == DQSYNTAX)
- SETDBLQUOTE();
- else
- CLRDBLQUOTE();
} else
USTPUTC(')', out);
} else {
@@ -1562,6 +1560,13 @@
*/
USTPUTC('(', out);
}
+
+ varnest++;
+ if (varnest >= maxnest) {
+ dblquotep = ckrealloc(dblquotep, maxnest / 8);
+ dblquotep[(maxnest / 32) - 1] = 0;
+ maxnest += 32;
+ }
goto parsearith_return;
}
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/50827 CVS commit: src/tests/bin/sh
Date: Fri, 19 Feb 2016 08:48:28 -0500
Module Name: src
Committed By: christos
Date: Fri Feb 19 13:48:28 UTC 2016
Modified Files:
src/tests/bin/sh: t_expand.sh t_varquote.sh
Log Message:
Add a test for PR/50827
To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/tests/bin/sh/t_expand.sh \
src/tests/bin/sh/t_varquote.sh
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/50827 CVS commit: src/bin/sh
Date: Fri, 19 Feb 2016 08:50:37 -0500
Module Name: src
Committed By: christos
Date: Fri Feb 19 13:50:37 UTC 2016
Modified Files:
src/bin/sh: parser.c
Log Message:
PR/50827: Richard Hansen: Fix default variable assignment with arithmetic,
from kre.
To generate a diff of this commit:
cvs rdiff -u -r1.93 -r1.94 src/bin/sh/parser.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Fri, 19 Feb 2016 14:01:59 +0000
State-Changed-Why:
christos committed a fix, does it work for you?
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Fri, 19 Feb 2016 21:49:24 +0700
Date: Fri, 19 Feb 2016 14:02:00 +0000 (UTC)
From: wiz@NetBSD.org
Message-ID: <20160219140200.3F64A7ACAF@mollari.NetBSD.org>
| christos committed a fix, does it work for you?
I know this was not directed at me, so don't take this as an answer,
but it still fails on even more bizarre usages...
./sh -c 'echo ${x:-$(( ${y:-"$((4))"} ))}'
sh: Syntax error: Unterminated quoted string
this fails if the quotes are inside the outer $(( )), but not inside the
inner $(( )) .. doesn't matter if the entire ${y...} is quoted or just
as shown, it fails the same way. But works if it is just the "4" quoted.
And to go even further into the absurd ...
./sh -c 'echo ${x:-"$(( "${y:-$(( ${z:-$(( 9 ))} ))}" ))"}'
sh: Syntax error: Missing '))'
sh is better now than it was, but this stuff ought to be fixed as well.
Without quoting, that works (believe it or not.)
I am looking into it, but maybe the PR can remain open for now.
kre
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
rhansen@rhansen.org
Cc:
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Fri, 19 Feb 2016 09:57:32 -0500
On Feb 19, 2:50pm, kre@munnari.OZ.AU (Robert Elz) wrote:
-- Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
| The following reply was made to PR bin/50827; it has been noted by GNATS.
|
| From: Robert Elz <kre@munnari.OZ.AU>
| To: gnats-bugs@NetBSD.org
| Cc:
| Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
| Date: Fri, 19 Feb 2016 21:49:24 +0700
|
| Date: Fri, 19 Feb 2016 14:02:00 +0000 (UTC)
| From: wiz@NetBSD.org
| Message-ID: <20160219140200.3F64A7ACAF@mollari.NetBSD.org>
|
| | christos committed a fix, does it work for you?
|
| I know this was not directed at me, so don't take this as an answer,
| but it still fails on even more bizarre usages...
|
| ./sh -c 'echo ${x:-$(( ${y:-"$((4))"} ))}'
| sh: Syntax error: Unterminated quoted string
|
| this fails if the quotes are inside the outer $(( )), but not inside the
| inner $(( )) .. doesn't matter if the entire ${y...} is quoted or just
| as shown, it fails the same way. But works if it is just the "4" quoted.
|
| And to go even further into the absurd ...
|
| ./sh -c 'echo ${x:-"$(( "${y:-$(( ${z:-$(( 9 ))} ))}" ))"}'
| sh: Syntax error: Missing '))'
|
| sh is better now than it was, but this stuff ought to be fixed as well.
| Without quoting, that works (believe it or not.)
|
| I am looking into it, but maybe the PR can remain open for now.
If you are looking into quoting, please look at: bin/43469. Also it might
be worthwhile to compare ours with FreeBSD's sh and switch to their code
if it is better.
christos
From: Robert Elz <kre@munnari.OZ.AU>
To: christos@zoulas.com (Christos Zoulas)
Cc: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
rhansen@rhansen.org
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Fri, 19 Feb 2016 22:43:47 +0700
Date: Fri, 19 Feb 2016 09:57:32 -0500
From: christos@zoulas.com (Christos Zoulas)
Message-ID: <20160219145732.1539C17FDAF@rebar.astron.com>
| If you are looking into quoting, please look at: bin/43469.
Will do.
| Also it might be worthwhile to compare ours with FreeBSD's sh
I will look.
| and switch to their code if it is better.
I can offer an opinion (and it is not just a wholesale replacement) offer
some patches perhaps, but the actual switch (if it were to happen) would
need to be done by someone else...
kre
From: Richard Hansen <rhansen@rhansen.org>
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: bin/50827: sh -c ': "${x=$((1))}"' gives bogus syntax error
Date: Fri, 19 Feb 2016 22:45:04 -0500
On 2016-02-19 04:35, Robert Elz wrote:
> It also mishandles ...
>
> sh -c ': ${x="$(("1"))"}'
> sh: Syntax error: Missing '))'
From http://austingroupbugs.net/view.php?id=985#c2885 (which will become
POSIX standard once approved):
The expression shall be expanded as if it were a word in
double-quotes, except that the treatment of a double-quote inside
the expression is unspecified and the result of parameter expansion
of the '@' special parameter is unspecified.
So the NetBSD shell is OK as-is in this regard, though it doesn't hurt
to change the shell to treat "$(("1"))" like "$((1))" because POSIX says
(will say) "unspecified".
From: Richard Hansen <rhansen@rhansen.org>
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
wiz@NetBSD.org
Cc:
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Sat, 20 Feb 2016 02:45:30 -0500
On 2016-02-19 09:02, wiz@NetBSD.org wrote:
> christos committed a fix, does it work for you?
Yes, it works for me. I did not test for any regressions.
Thank you!
State-Changed-From-To: feedback->analyzed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Sat, 20 Feb 2016 10:06:26 +0000
State-Changed-Why:
Original problem fixed; keep this PR open for a bit if kre wants to provide more fixes.
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Sun, 21 Feb 2016 00:22:31 +0700
Date: Sat, 20 Feb 2016 10:06:26 +0000 (UTC)
From: wiz@NetBSD.org
Message-ID: <20160220100626.B4EE07ACCD@mollari.NetBSD.org>
| Original problem fixed; keep this PR open for a bit if kre wants
| to provide more fixes.
An update ... I have a version that works for all I have tested so far
(but that is not yet a lot). It does include the initial subject of
this PR, and the horrid example I found last, and even the example from
PR 43469 that Christos asked me to look at (though that was really a different
problem.)
But I have yet to run (and then add to) the atf tests - and perhaps fix
anything I have broken along the way. Tomorrow sometime I hope.
kre
From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, rhansen@rhansen.org
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Sat, 20 Feb 2016 12:33:30 -0500
> wow the 43469 one was giving me trouble for a while!
christos
From: Robert Elz <kre@munnari.OZ.AU>
To: Christos Zoulas <christos@zoulas.com>
Cc: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
rhansen@rhansen.org
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Sun, 21 Feb 2016 08:19:00 +0700
Date: Sat, 20 Feb 2016 12:33:30 -0500
From: Christos Zoulas <christos@zoulas.com>
Message-ID: <61B42472-7C43-4B93-8185-D902A81727B5@zoulas.com>
| > wow the 43469 one was giving me trouble for a while!
Don't jump for joy just yet. The two expressions in the PR both work
(produce the same, correct, result) but there is potential for much
other breakage I have not yet tested, plus more variants of the expr
in question tha also need to work, also yet untested.
So some progress, but too early to open the champaign...
While here I'll ask another question ... the code in sh is sprinkled
with "volatile" all over the place (particularly in the area I have
been looking.) I gather this is to appease gcc wrt setjmp/longmp
(the FreeBSD code has a comment - or maybe commit log entry - simply
refusing to go that route...) Does anyone mind if I strip out most
of that, and add "just the right" volatile vars, so the trap longjmp
stuff works (at least in the token reading functions) - or should work,
modulo whatever nonsense gcc gets up to, and remove it from all the
rest? (If I do that I'll make it a separate patch.) All that (unnecessary)
volatile nonsense has to be slowing shell execution, and with my fixes,
it will be even worse. Until I remove it so I can measure the effect, I
don't know how much difference it really makes, but ... And of course,
nothing will happen there until the substantive changes are all fully
tested (and corrected if needed.)
kre
From: christos@zoulas.com (Christos Zoulas)
To: Robert Elz <kre@munnari.OZ.AU>
Cc: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
rhansen@rhansen.org
Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
Date: Sat, 20 Feb 2016 20:22:11 -0500
On Feb 21, 8:19am, kre@munnari.OZ.AU (Robert Elz) wrote:
-- Subject: Re: bin/50827 (sh -c ': "${x=$((1))}"' gives bogus syntax error)
| Don't jump for joy just yet. The two expressions in the PR both work
| (produce the same, correct, result) but there is potential for much
| other breakage I have not yet tested, plus more variants of the expr
| in question tha also need to work, also yet untested.
| So some progress, but too early to open the champaign...
Ok, I will put it back in the fridge.
| While here I'll ask another question ... the code in sh is sprinkled
| with "volatile" all over the place (particularly in the area I have
| been looking.) I gather this is to appease gcc wrt setjmp/longmp
| (the FreeBSD code has a comment - or maybe commit log entry - simply
| refusing to go that route...) Does anyone mind if I strip out most
| of that, and add "just the right" volatile vars, so the trap longjmp
| stuff works (at least in the token reading functions) - or should work,
| modulo whatever nonsense gcc gets up to, and remove it from all the
| rest? (If I do that I'll make it a separate patch.) All that (unnecessary)
| volatile nonsense has to be slowing shell execution, and with my fixes,
| it will be even worse. Until I remove it so I can measure the effect, I
| don't know how much difference it really makes, but ... And of course,
| nothing will happen there until the substantive changes are all fully
| tested (and corrected if needed.)
I think that most of the volatile was because gcc wanted it... I don't
mind the cleanup, go for it. But expect transient breakage amongst different
ports. Also our shell does vfork() which is a nother volatile creator, I
don't think that FreeBSD supports that.
christos
From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: bin/50827: sh -c ': "${x=$((1))}"' gives bogus syntax error
Date: Sun, 21 Feb 2016 15:45:39 +0700
Date: Sat, 20 Feb 2016 03:50:01 +0000 (UTC)
From: Richard Hansen <rhansen@rhansen.org>
Message-ID: <20160220035001.44F947ACAF@mollari.NetBSD.org>
| So the NetBSD shell is OK as-is in this regard, though it doesn't hurt
| to change the shell to treat "$(("1"))" like "$((1))" because POSIX says
| (will say) "unspecified".
Yes, something in one of ether the NetBSD or FreeBSD sources or commit
logs says something like (paraphrasing)...
We don't have to do this, but people write code this way
[quoted strings inside $(( ))] and expect it to work, so
we might as well make it work.
From what I can tell, so far, in my as yet un-committed (not submitted)
code, all of this stuff is now working fine. I am still running
tests (and working on the sh atf tests in general.)
kre
State-Changed-From-To: analyzed->closed
State-Changed-By: christos@NetBSD.org
State-Changed-When: Tue, 23 Feb 2016 08:04:50 -0500
State-Changed-Why:
fixed
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home