is this valid c or a compiler bug?

[Keith Vetter]

Hi,

After a long debugging session I tracked down a
bug to the following line of code:

np = &nl[alloc_memory()]; /* nl & np are NODE * */

The problem is that nl is volatile--alloc_memory() may
change it (via realloc). The AIX compiler fails--it grabs
the value of nl before the call to alloc_memory(). The
Windows VC 6.0 compiler handles it correctly.

So is this a compiler bug, programmer error or ambiguous c?

Thanks,
Keith

 

[Joona I Palaste]

Hi,

After a long debugging session I tracked down a
bug to the following line of code:

np = &nl[alloc_memory()]; /* nl & np are NODE * */

The problem is that nl is volatile--alloc_memory() may
change it (via realloc). The AIX compiler fails--it grabs
the value of nl before the call to alloc_memory(). The
Windows VC 6.0 compiler handles it correctly.

So is this a compiler bug, programmer error or ambiguous c?

Ambiguous C. This is the same as np = &(*(nl + alloc_memory())), and
the order of evaluation of the + operator's operands is unspecified.

--
/-- Joona Palaste ([email protected]) ------------- Finland --------\
\-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
"The day Microsoft makes something that doesn't suck is probably the day they
start making vacuum cleaners."
- Ernst Jan Plugge

 

[Leor Zolman]

Hi,

After a long debugging session I tracked down a
bug to the following line of code:

np = &nl[alloc_memory()]; /* nl & np are NODE * */

That would be rewritten by any compiler to:

np = & * (nl + alloc_memory());

There would be no obligation on the part of any compiler to evaluate nl
before alloc_memory() or vice-versa. Looks like UB if alloc_memory futzes
with nl.
-leor

The problem is that nl is volatile--alloc_memory() may
change it (via realloc). The AIX compiler fails--it grabs
the value of nl before the call to alloc_memory(). The
Windows VC 6.0 compiler handles it correctly.

So is this a compiler bug, programmer error or ambiguous c?

Thanks,
Keith

Leor Zolman
BD Software
(e-mail address removed)
www.bdsoft.com -- On-Site Training in C/C++, Java, Perl & Unix
C++ users: Download BD Software's free STL Error Message
Decryptor at www.bdsoft.com/tools/stlfilt.html

 

[Richard Bos]

After a long debugging session I tracked down a
bug to the following line of code:

np = &nl[alloc_memory()]; /* nl & np are NODE * */

That would be rewritten by any compiler to:

np = & * (nl + alloc_memory());

There would be no obligation on the part of any compiler to evaluate nl
before alloc_memory() or vice-versa. Looks like UB if alloc_memory futzes
with nl.

No, though I understand why you think so. But there's a sequence point
just before a function is called, and another just after any statement,
including alloc_memory()'s return statement. So regardless of whether nl
is read before or after alloc_memory() is called, there's always a
sequence point in between.
This makes the behaviour not-undefined; but because of the unspecified
order of the operands of + (or most other operators), it isn't _well_
defined, either; in fact, AFAICT the entire behaviour is unspecified, as
well. IOW, it must _either_ read nl and then update it inside
alloc_memory(), _or_ first update nl in the function and then read it;
it may not scribble over nl while it's still reading it, which would be
allowed with undefined behaviour.
All of this, of course, assumes that alloc_memory() _is_ a function, and
not a macro; if it's a macro, all bets are off, and you could easily
have undefined behaviour.

Richard

 

[Dan Pop]

After a long debugging session I tracked down a
bug to the following line of code:

np = &nl[alloc_memory()]; /* nl & np are NODE * */

The problem is that nl is volatile--alloc_memory() may
change it (via realloc). The AIX compiler fails--it grabs
the value of nl before the call to alloc_memory(). The
Windows VC 6.0 compiler handles it correctly.

So is this a compiler bug, programmer error or ambiguous c?

Programmer error, if the function call was supposed to be evaluated
before nl. When the language doesn't specify an evaluation order
(and it seldom does), it is programmer's responsibility to enforce the
desired evaluation order, by decomposing the expression into
subexpressions:

tmp = alloc_memory();
np = nl + tmp;

Now, you are guaranteed that the alloc_memory function call will be
evaluated before nl, because the end of the first expression acts as a
sequence point.

The volatility of nl just doesn't make any difference to this discussion:
it means that nl *must* be evaluated (instead of reusing an old copy of
it that might still be available in a register), but the exact moment nl
is evaluated is still unspecified (both before and after the alloc_memory
function calls are valid options for the compiler).

Dan

 

[Leor Zolman]

After a long debugging session I tracked down a
bug to the following line of code:

np = &nl[alloc_memory()]; /* nl & np are NODE * */

That would be rewritten by any compiler to:

np = & * (nl + alloc_memory());

There would be no obligation on the part of any compiler to evaluate nl
before alloc_memory() or vice-versa. Looks like UB if alloc_memory futzes
with nl.

No, though I understand why you think so. But there's a sequence point
just before a function is called, and another just after any statement,
including alloc_memory()'s return statement. So regardless of whether nl
is read before or after alloc_memory() is called, there's always a
sequence point in between.
This makes the behaviour not-undefined; but because of the unspecified
order of the operands of + (or most other operators), it isn't _well_
defined, either; in fact, AFAICT the entire behaviour is unspecified, as
well. IOW, it must _either_ read nl and then update it inside
alloc_memory(), _or_ first update nl in the function and then read it;
it may not scribble over nl while it's still reading it, which would be
allowed with undefined behaviour.

Okay, I've finally gotten clear on what I was trying to say in the first
place. Turns out I /think/ I actually had "a right idea", but did an awful
job articulating it.

First of all, I agree 100% with everything you say about the sequence
points and order of evaluation of the operands to '+' contributing to
unspecified (rather than undefined) behavior in the (rewritten)
subexpression:

(nl + alloc_memory())

However, I maintain that there is still the /potential/ for undefined
behavior as soon as the + operator is applied, since the result of the
addition might be an invalid pointer (and most certainly /would/ be if nl
represents a "simple" dynamically allocated array, whose old memory was
just deleted during the alloc_memory() call, in the case where the compiler
evaluates nl before the call to the function.)

I'll admit I was originally thinking that the UB wouldn't kick in until the
application of the indirection operator, but now it seems that we've got
potential UB immediately upon conclusion of the addition, making the last
thing I said up above ("Looks like UB...") technically correct. Do I have
a leg to stand on?
-leor

All of this, of course, assumes that alloc_memory() _is_ a function, and
not a macro; if it's a macro, all bets are off, and you could easily
have undefined behaviour.

Richard

Leor Zolman
BD Software
(e-mail address removed)
www.bdsoft.com -- On-Site Training in C/C++, Java, Perl & Unix
C++ users: Download BD Software's free STL Error Message
Decryptor at www.bdsoft.com/tools/stlfilt.html

 

[Richard Bos]

First of all, I agree 100% with everything you say about the sequence
points and order of evaluation of the operands to '+' contributing to
unspecified (rather than undefined) behavior in the (rewritten)
subexpression:

(nl + alloc_memory())

However, I maintain that there is still the /potential/ for undefined
behavior as soon as the + operator is applied, since the result of the
addition might be an invalid pointer

Ah, yes, of course! Hadn't thought of that. Yes, in that case, you would
certainly get UB.

Richard