NetBSD Problem Report #50852
From www@NetBSD.org Fri Feb 26 10:54:47 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 DB9B37ACCE
for <gnats-bugs@gnats.NetBSD.org>; Fri, 26 Feb 2016 10:54:47 +0000 (UTC)
Message-Id: <20160226105447.073757ACDB@mollari.NetBSD.org>
Date: Fri, 26 Feb 2016 10:54:47 +0000 (UTC)
From: dcb314@hotmail.com
Reply-To: dcb314@hotmail.com
To: gnats-bugs@NetBSD.org
Subject: src/sys/arch/hp300/stand/common/ite.c:239: bad expression
X-Send-Pr-Version: www-1.0
>Number: 50852
>Category: port-hp300
>Synopsis: src/sys/arch/hp300/stand/common/ite.c:239: bad expression
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: port-hp300-maintainer
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Feb 26 10:55:00 +0000 2016
>Closed-Date: Mon Feb 29 15:13:53 +0000 2016
>Last-Modified: Mon Feb 29 15:13:53 +0000 2016
>Originator: David Binderman
>Release: cvs dated 20160226
>Organization:
>Environment:
>Description:
[src/sys/arch/hp300/stand/common/ite.c:239]: (error) Expression 'whichconsole=++whichconsole%(NITE+1)' depends on order of evaluation of side effects
Source code is
whichconsole = ++whichconsole % (NITE+1);
Maybe better code
whichconsole = (whichconsole + 1) % (NITE+1);
>How-To-Repeat:
>Fix:
>Release-Note:
>Audit-Trail:
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: port-hp300-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Fri, 26 Feb 2016 13:58:36 +0100
On Fri, Feb 26, 2016 at 10:55:00AM +0000, dcb314@hotmail.com wrote:
> >Description:
> [src/sys/arch/hp300/stand/common/ite.c:239]: (error) Expression 'whichconsole=++whichconsole%(NITE+1)' depends on order of evaluation of side effects
>
> Source code is
>
> whichconsole = ++whichconsole % (NITE+1);
>
> Maybe better code
>
> whichconsole = (whichconsole + 1) % (NITE+1);
Independent of the question of which code is better, I don't think the
error is correct. The location of the assignment (whichconsole) is not
changed by the right operand of the =, so the evaluation of the left and
right operand is invariant under evaluation order of the side effect.
The side effect of *updating* whichconsole as part of the assignment is
sequenced *after* the evaluation of the right operand (6.5.16 (3)
sentence 4. As such, the value here is perfectly well defined according
to the standard. This is quite different from an expression like
*(++p) = *(++p);
Joerg
From: David Binderman <dcb314@hotmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"port-hp300-maintainer@netbsd.org" <port-hp300-maintainer@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>, "netbsd-bugs@netbsd.org"
<netbsd-bugs@netbsd.org>
Cc:
Subject: RE: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239:
bad expression
Date: Fri, 26 Feb 2016 16:45:21 +0000
Hello there=2C=0A=
=0A=
----------------------------------------=0A=
> On Fri=2C Feb 26=2C 2016 at 10:55:00AM +0000=2C dcb314@hotmail.com wrote:=
=0A=
>>>Description:=0A=
>> [src/sys/arch/hp300/stand/common/ite.c:239]: (error) Expression 'whichco=
nsole=3D++whichconsole%(NITE+1)' depends on order of evaluation of side eff=
ects=0A=
>>=0A=
>> Source code is=0A=
>>=0A=
>> whichconsole =3D ++whichconsole % (NITE+1)=3B=0A=
>>=0A=
>> Maybe better code=0A=
>>=0A=
>> whichconsole =3D (whichconsole + 1) % (NITE+1)=3B=0A=
>=0A=
> Independent of the question of which code is better=2C I don't think the=
=0A=
> error is correct. =0A=
=0A=
False=2C the error is valid.=0A=
=0A=
There are two writes to whichconsole=2C one on the left of the assignment=
=2C=0A=
one on the right.=0A=
=0A=
The ISO standard doesn't define which order those writes occur.=0A=
See Steve Summit's C FAQ for sequence points=2C section 3.8.=0A=
=0A=
=0A=
Regards=0A=
=0A=
David Binderman=0A=
=0A=
31 years a C programmer=2C including C and C++ compiler development.=0A=
=
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/50852 CVS commit: src/sys/arch/hp300/stand/common
Date: Fri, 26 Feb 2016 13:11:11 -0500
Module Name: src
Committed By: christos
Date: Fri Feb 26 18:11:11 UTC 2016
Modified Files:
src/sys/arch/hp300/stand/common: ite.c
Log Message:
PR/50852: David Binderman: make code more readable (although it was correct
anyway)
To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/sys/arch/hp300/stand/common/ite.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: David Binderman <dcb314@hotmail.com>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"port-hp300-maintainer@netbsd.org" <port-hp300-maintainer@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Fri, 26 Feb 2016 21:24:14 +0100
On Fri, Feb 26, 2016 at 04:45:21PM +0000, David Binderman wrote:
> Hello there,
>
> ----------------------------------------
> > On Fri, Feb 26, 2016 at 10:55:00AM +0000, dcb314@hotmail.com wrote:
> >>>Description:
> >> [src/sys/arch/hp300/stand/common/ite.c:239]: (error) Expression 'whichconsole=++whichconsole%(NITE+1)' depends on order of evaluation of side effects
> >>
> >> Source code is
> >>
> >> whichconsole = ++whichconsole % (NITE+1);
> >>
> >> Maybe better code
> >>
> >> whichconsole = (whichconsole + 1) % (NITE+1);
> >
> > Independent of the question of which code is better, I don't think the
> > error is correct.
>
> False, the error is valid.
>
> There are two writes to whichconsole, one on the left of the assignment,
> one on the right.
>
> The ISO standard doesn't define which order those writes occur.
> See Steve Summit's C FAQ for sequence points, section 3.8.
You are wrong. There is no write from the left operand of the
assignment. The write is the result of the assignment and as written
before is explicitly sequenced by the standard.
Joerg
From: Dave Huang <khym@azeotrope.org>
To: Joerg Sonnenberger <joerg@britannica.bec.de>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Fri, 26 Feb 2016 17:04:46 -0600
On Fri, Feb 26, 2016 at 09:24:14PM +0100, Joerg Sonnenberger wrote:
> On Fri, Feb 26, 2016 at 04:45:21PM +0000, David Binderman wrote:
> > >> Source code is
> > >>
> > >> whichconsole = ++whichconsole % (NITE+1);
> > >>
> > >> Maybe better code
> > >>
> > >> whichconsole = (whichconsole + 1) % (NITE+1);
> > >
> > > Independent of the question of which code is better, I don't think the
> > > error is correct.
> >
> > False, the error is valid.
> >
> > There are two writes to whichconsole, one on the left of the assignment,
> > one on the right.
> >
> > The ISO standard doesn't define which order those writes occur.
> > See Steve Summit's C FAQ for sequence points, section 3.8.
>
> You are wrong. There is no write from the left operand of the
> assignment. The write is the result of the assignment and as written
> before is explicitly sequenced by the standard.
How is the code in question different from i = ++i;, which I've always
been told is undefined? The assignment operator does not introduce a
sequence point.
gcc thinks it's wrong too:
% cat t.c
#define NITE 4
int main(void)
{
int whichconsole = 0;
whichconsole = ++whichconsole % (NITE+1);
return 0;
}
% cc -Wall t.c
t.c: In function ‘main’:
t.c:6:16: warning: operation on ‘whichconsole’ may be undefined [-Wsequence-point]
whichconsole = ++whichconsole % (NITE+1);
^
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: Dave Huang <khym@azeotrope.org>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Sat, 27 Feb 2016 00:10:21 +0100
On Fri, Feb 26, 2016 at 05:04:46PM -0600, Dave Huang wrote:
> On Fri, Feb 26, 2016 at 09:24:14PM +0100, Joerg Sonnenberger wrote:
> > On Fri, Feb 26, 2016 at 04:45:21PM +0000, David Binderman wrote:
> > > >> Source code is
> > > >>
> > > >> whichconsole = ++whichconsole % (NITE+1);
> > > >>
> > > >> Maybe better code
> > > >>
> > > >> whichconsole = (whichconsole + 1) % (NITE+1);
> > > >
> > > > Independent of the question of which code is better, I don't think the
> > > > error is correct.
> > >
> > > False, the error is valid.
> > >
> > > There are two writes to whichconsole, one on the left of the assignment,
> > > one on the right.
> > >
> > > The ISO standard doesn't define which order those writes occur.
> > > See Steve Summit's C FAQ for sequence points, section 3.8.
> >
> > You are wrong. There is no write from the left operand of the
> > assignment. The write is the result of the assignment and as written
> > before is explicitly sequenced by the standard.
>
> How is the code in question different from i = ++i;, which I've always
> been told is undefined? The assignment operator does not introduce a
> sequence point.
At least in C11, it does.
Joerg
From: Dave Huang <khym@azeotrope.org>
To: Joerg Sonnenberger <joerg@britannica.bec.de>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Fri, 26 Feb 2016 17:40:11 -0600
On Sat, Feb 27, 2016 at 12:10:21AM +0100, Joerg Sonnenberger wrote:
> On Fri, Feb 26, 2016 at 05:04:46PM -0600, Dave Huang wrote:
> > How is the code in question different from i = ++i;, which I've always
> > been told is undefined? The assignment operator does not introduce a
> > sequence point.
>
> At least in C11, it does.
Hmm, well I'm no language lawyer, but footnote 84 in section 6.5 of
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf (the C1X
draft) says "This paragraph renders undefined statement expressions
such as i = ++i + 1;"
However, I did see someone say that i = ++i + 1 is well-defined
starting in *C++* 11.
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: Dave Huang <khym@azeotrope.org>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Sat, 27 Feb 2016 08:44:19 +0100
On Fri, Feb 26, 2016 at 05:40:11PM -0600, Dave Huang wrote:
> On Sat, Feb 27, 2016 at 12:10:21AM +0100, Joerg Sonnenberger wrote:
> > On Fri, Feb 26, 2016 at 05:04:46PM -0600, Dave Huang wrote:
> > > How is the code in question different from i = ++i;, which I've always
> > > been told is undefined? The assignment operator does not introduce a
> > > sequence point.
> >
> > At least in C11, it does.
>
> Hmm, well I'm no language lawyer, but footnote 84 in section 6.5 of
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf (the C1X
> draft) says "This paragraph renders undefined statement expressions
> such as i = ++i + 1;"
I think that's one is just a left-over from earlier versions, since
sequencing rules are explicit for that case. The second example
"a[i++]=i" on the other hand is still undefined.
Joerg
From: David Binderman <dcb314@hotmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"port-hp300-maintainer@netbsd.org" <port-hp300-maintainer@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>, "netbsd-bugs@netbsd.org"
<netbsd-bugs@netbsd.org>
Cc:
Subject: RE: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239:
bad expression
Date: Sat, 27 Feb 2016 07:52:56 +0000
Hello there=2C=0A=
=0A=
----------------------------------------=0A=
> However=2C I did see someone say that i =3D ++i + 1 is well-defined=0A=
> starting in *C++* 11.=0A=
=0A=
The code in question is C code. Let's leave what happens in C++ aside=2C=0A=
since it is only muddying the issue. =0A=
=0A=
Let's also leave aside what happens in various C language versions and conc=
entrate =0A=
on the language version the compiler uses for the netBSD code.=0A=
=0A=
=0A=
$ more feb27b.c=0A=
=0A=
#define NITE 4=0A=
=0A=
int=0A=
main()=0A=
{=0A=
=A0=A0=A0 int whichconsole =3D 0=3B=0A=
=A0=A0=A0 whichconsole =3D ++whichconsole % (NITE+1)=3B=0A=
=A0=A0=A0 return 0=3B=0A=
}=0A=
=0A=
First=2C with a recent version of clang=2C a well known C compiler.=0A=
=0A=
$ ~/llvm/results/bin/clang -c -g -O2 -Wall -Wextra feb27b.c=0A=
feb27b.c:8:17: warning: multiple unsequenced modifications to 'whichconsole=
'=0A=
=A0=A0=A0=A0=A0 [-Wunsequenced]=0A=
=A0=A0=A0=A0=A0=A0=A0 whichconsole =3D ++whichconsole % (NITE+1)=3B=0A=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 ~ ^=0A=
1 warning generated.=0A=
=0A=
Next a recent version of gcc.=0A=
=0A=
$ ~/gcc/results/bin/gcc -c -g -O2 -Wall -Wextra feb27b.c=0A=
feb27b.c: In function =91main=92:=0A=
feb27b.c:8:15: warning: operation on =91whichconsole=92 may be undefined [-=
Wsequence-point]=0A=
=A0 whichconsole =3D ++whichconsole % (NITE+1)=3B=0A=
=A0 ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~=0A=
$ =0A=
=0A=
Two different compilers=2C in their default language standard=2C think the =
code falls=0A=
foul of the C sequence point rules. =0A=
=0A=
I think it is *much* more likely that the original code is wrong than two s=
eparate compilers=0A=
have the same bug in them.=0A=
=0A=
=0A=
Regards=0A=
=0A=
David Binderman=0A=
=
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: David Binderman <dcb314@hotmail.com>
Cc: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"port-hp300-maintainer@netbsd.org" <port-hp300-maintainer@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Sat, 27 Feb 2016 19:36:09 +0100
On Sat, Feb 27, 2016 at 07:52:56AM +0000, David Binderman wrote:
> Hello there,
>
> ----------------------------------------
> > However, I did see someone say that i = ++i + 1 is well-defined
> > starting in *C++* 11.
>
> The code in question is C code. Let's leave what happens in C++ aside,
> since it is only muddying the issue.
I never talked about C++. I explicitly talke about C11.
> Let's also leave aside what happens in various C language versions and concentrate
> on the language version the compiler uses for the netBSD code.
>
>
> $ more feb27b.c
>
> #define NITE 4
>
> int
> main()
> {
> int whichconsole = 0;
> whichconsole = ++whichconsole % (NITE+1);
> return 0;
> }
>
> First, with a recent version of clang, a well known C compiler.
>
> $ ~/llvm/results/bin/clang -c -g -O2 -Wall -Wextra feb27b.c
> feb27b.c:8:17: warning: multiple unsequenced modifications to 'whichconsole'
> [-Wunsequenced]
> whichconsole = ++whichconsole % (NITE+1);
> ~ ^
> 1 warning generated.
>
> Next a recent version of gcc.
>
> $ ~/gcc/results/bin/gcc -c -g -O2 -Wall -Wextra feb27b.c
> feb27b.c: In function ‘main’:
> feb27b.c:8:15: warning: operation on ‘whichconsole’ may be undefined [-Wsequence-point]
> whichconsole = ++whichconsole % (NITE+1);
> ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
> $
>
> Two different compilers, in their default language standard, think the code falls
> foul of the C sequence point rules.
>
> I think it is *much* more likely that the original code is wrong than two separate compilers
> have the same bug in them.
Actually, it is quite likely that the specific warnings for the sequence
point rules just haven't been updated. You keep ignoring the explicit
reference to the standard I gave. It is quite explicit that a sequence
point exists. Ironically, for the pre-increment version, it is highly
likely that no sane compiler ever did not do the correct thing, but
that's beside the point.
Joerg
From: David Binderman <dcb314@hotmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>,
"port-hp300-maintainer@netbsd.org" <port-hp300-maintainer@netbsd.org>,
"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>, "netbsd-bugs@netbsd.org"
<netbsd-bugs@netbsd.org>
Cc:
Subject: RE: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239:
bad expression
Date: Sun, 28 Feb 2016 17:48:54 +0000
Hello there=2C=0A=
=0A=
----------------------------------------=0A=
> I never talked about C++. I explicitly talke about C11.=0A=
=0A=
I can't seem to find out which version of the language standard the code=0A=
in question in port-hp300 gets compiled to.=0A=
=0A=
There certainly seems to be a lot (> 200=2C000) of uses of -std=3Dgnu99 in=
=0A=
a recent x86 build. All other language settings seem to get mentioned=0A=
< 500 times=2C so to odds are high source code file ite.c gets compiled wit=
h -std=3Dgnu99.=0A=
=0A=
Once we've got that=2C and then we've agreed what the three language=0A=
standards do in this area=2C (C90=2C C99=2C C11)=2C then we might be able t=
o=0A=
agree *finally* whether the code is in error.=0A=
=0A=
Of course=2C the reliable programmer makes it work reliably in all versions=
of C.=0A=
=0A=
The code is certainly in error in C90=2C maybe C99=2C maybe not C11.=0A=
=A0=0A=
I had a look at=0A=
=0A=
http://en.cppreference.com/w/c/language/eval_order=0A=
=0A=
but I couldn't make much sense of it. I think the Rules section might=0A=
have the nitty gritty.=0A=
=0A=
=0A=
Regards=0A=
=0A=
David Binderman=0A=
=
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: port-hp300/50852: src/sys/arch/hp300/stand/common/ite.c:239: bad
expression
Date: Mon, 29 Feb 2016 15:09:10 +0000
On Sat, Feb 27, 2016 at 06:40:01PM +0000, Joerg Sonnenberger wrote:
> Ironically, for the pre-increment version, it is highly
> likely that no sane compiler ever did not do the correct thing, but
> that's beside the point.
That's not true. A reasonable implementation of pre/post-increment
effects is to write them back at the next sequence point, so you'd get
t1 = whichconsole + 1
t2 = whichconsole + 1
t3 = t2 % (NITE+1)
whichconsole = t3
whichconsole = t1
which after further obvious simplifications turns into
whichconsole = whichconsole + 1
aka The Wrong Thing.
Anyway, regardless of whether you're channeling Dan Pop it's all
irrelevant; code of this form should not be allowed to exist.
--
David A. Holland
dholland@netbsd.org
State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 29 Feb 2016 15:13:53 +0000
State-Changed-Why:
Christos fixed it, thanks.
>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.