Sequence point violation?

  • Thread starter Tomás Ó hÉilidhe
  • Start date
T

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?
 
B

Ben Pfaff

Tomás Ó hÉilidhe said:
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.
 
C

Chris Torek

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).
 
K

Keith Thompson

Tomás Ó hÉilidhe said:
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?

Not necessarily. tolower() can be, and often is, implemented as a
macro. And even if that weren't the case, the evaluation order is
unspecified, as Ben Pfaff pointed out.
 
F

Flash Gordon

Ben Pfaff wrote, On 02/01/08 17:48:
Tomás Ó hÉilidhe said:
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.
 
H

Harald van Dijk

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.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top