Sequence point violation?

[Tomás Ó hÉilidhe]

I'm writing a program currently that was working perfectly until I
decided to compile it with "-O3" in gcc (-O3 specifies optimisation of
the third level).

Anyway, I found the problem. I had the following function:

void StrToLower(char *p)
{
while ( *p++ = tolower( (char unsigned)*p ) );
}

I changed it to the following and now it works properly:

void StrToLower(char *p)
{
for ( ; *p = tolower((char unsigned)*p); ++p);
}

I'm not entirely convinced however that there's a sequence point
violation in the following:

*p++ = tolower( (char unsigned)*p );

Any thoughts? There should be a sequence point at tolower's evaluation of
its arguments... right?

--
Tomás Ó hÉilidhe

[Ben Pfaff]

"Tomás Ó hÉilidhe" <to*@lavabit.comwrites:

Anyway, I found the problem. I had the following function:

void StrToLower(char *p)
{
while ( *p++ = tolower( (char unsigned)*p ) );
}

[...]

Any thoughts? There should be a sequence point at tolower's evaluation of
its arguments... right?

Yes. However, the compiler is not obligated to evaluate the
right side of the assignment before the left side. It can
evaluate *p++ and (char unsigned)*p together. Hence, the
problem.
--
"...Almost makes you wonder why Heisenberg didn't include postinc/dec operators
in the uncertainty principle. Which of course makes the above equivalent to
Schrodinger's pointer..."
--Anthony McDonald

[Chris Torek]

In article <Xn***************************@194.125.133.14>,
Tomás Ó hÉilidhe <to*@lavabit.comwrote:

while ( *p++ = tolower( (char unsigned)*p ) );

[vs]

for ( ; *p = tolower((char unsigned)*p); ++p);

The former is "wrong" (invalid C code) and the latter is right,
because:

>I'm not entirely convinced however that there's a sequence point
violation in the following:

*p++ = tolower( (char unsigned)*p );

Any thoughts? There should be a sequence point at tolower's evaluation of
its arguments... right?

There is indeed a sequence point before a call. (I forget whether
C99 fixed the "loophole" in C89, where functions described in the
standard could be macros, and thus sidestep the sequence-point
requirements for function calls, but for the moment we may as well
assume tolower() really is a function-call.)

The problem is, the left-hand side (LHS) of the assignment operator
has no sequencing constraints with respect to the right-hand side
(RHS). So, although there is a sequence point before the call to
tolower(), the "for-its-address" ("lvalue") evaluation of *p++ on
the LHS could be not-at-all, partly, or completely evaluated at
the time the RHS evaluation begins. The use of *p on the right
then conflicts with the update of p in *p++ on the left. This
renders the behavior undefined.

If the behavior were not undefined already, then we might have to
start worrying about whether tolower() is a macro instead of a
function, and whether C99 has fixed the loophole. But the
corrected version is safe -- although I would probably write
it as:

while ((*p = tolower((unsigned char)*p)) != '\0')
p++;

myself (because I dislike empty loops, as they often need extra
checking to make sure they are not simply typographic errors in
the code).
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) [phone]
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.

[Flash Gordon]

Ben Pfaff wrote, On 02/01/08 17:48:

"Tomás Ó hÉilidhe" <to*@lavabit.comwrites:

>Anyway, I found the problem. I had the following function:

void StrToLower(char *p)
{
while ( *p++ = tolower( (char unsigned)*p ) );
}

[...]

>Any thoughts? There should be a sequence point at tolower's evaluation of
its arguments... right?

Yes. However, the compiler is not obligated to evaluate the
right side of the assignment before the left side. It can
evaluate *p++ and (char unsigned)*p together. Hence, the
problem.

I.e. there is no sequence point between evaluating the p on the right
and the p++ on the left. So p is modified (by the p++) and read for a
purpose other than calculating the new value of p (as opposed to *p) and
that invokes undefined behaviour.

Tomás should think himself lucky that the problem showed up at -O3
rather than lurking hidden until demonstrating it to a big customer.
--
Flash Gordon

[=?UTF-8?q?Harald_van_D=C4=B3k?=]

On Wed, 02 Jan 2008 19:24:29 +0000, Chris Torek wrote:

There is indeed a sequence point before a call. (I forget whether C99
fixed the "loophole" in C89, where functions described in the standard
could be macros, and thus sidestep the sequence-point requirements for
function calls, but for the moment we may as well assume tolower()
really is a function-call.)

I wouldn't consider it a loophole. C99 mentions the fact that macro
definitions for standard library functions don't have the same sequence
point requirements as the function in a footnote.