[comp.lang.c++] subtle inlining/sequence points bug in CFront 2.0

scott@goofy.apple.com (scott douglass) (05/04/90)

I've found, actually I have been burned by, a subtle bug involving
inlining and sequence points in CFront 2.0.  In some cases CFront
accidentally eliminates the sequence point before the call of an
inline function.

Example:

    class A
	{
    public:
	A* next;
	int i;
	A* get_next();
	int f();
	};

    A* A::get_next() { return next; }

    void g(A* a)
	{
	int b = ( a->get_next()->i == a->f() );
	}

Assume that A::f() may change A::next which CFront must since A::f()
if not a const member.  Since order of evaluation is not defined for
expressions either a->get_next() or a->f() will happen first, but
the "->i" will definetly refer to one of two possible As.

Now the problem.  If I make A::get_next() inline CFront generates
code something like the following:

	b = ( a->next->i == f(a) );

The semantics of this expression are not defined.  Your C compiler
may, and mine did, evaluate a->next into a temporary, evaluate a->f()
which may change a->next, and finally evaluate temporary->i which
may no longer be valid.

One unsavory way that CFront could avoid this would be to introduce
it's own temporary like this:

	int temp;
	b = ( (temp = a->next->i, temp) == f(a) );

Always introducing such temporaries could be a serious performance
hit.  A similar temporary is already introduced already when inlining
a function such as:

	inline void A::set_next(A* a) { next = a; }

Any comments, especially from people in a position to change CFront?

ark@alice.UUCP (Andrew Koenig) (05/05/90)

In article <8066@goofy.Apple.COM>, scott@goofy.apple.com (scott douglass) writes:

>     void g(A* a)
> 	{
> 	int b = ( a->get_next()->i == a->f() );
> 	}

> Assume that A::f() may change A::next which CFront must since A::f()
> if not a const member.  Since order of evaluation is not defined for
> expressions either a->get_next() or a->f() will happen first, but
> the "->i" will definetly refer to one of two possible As.

My personal inclination is to treat any expression in which one part
changes something that another part fetches as an error.

Ultimately, though, this is the kind of thing that is going to be
decided by the ANSI C++ committee.
-- 
				--Andrew Koenig
				  ark@europa.att.com

rfg@ics.uci.edu (Ronald Guilmette) (05/05/90)

In article <8066@goofy.Apple.COM> scott@goofy.apple.com (scott douglass) writes:
>I've found, actually I have been burned by, a subtle bug involving
>inlining and sequence points in CFront 2.0.  In some cases CFront
>accidentally eliminates the sequence point before the call of an
>inline function.

I believe that you have misunderstood the exact definition of the term
"sequence point" (at least as it is proscribed by the ANSI C standard).

Reading sections A.2 and 3.6 of the ANSI C standard may help.

>
>Example:
>
>    class A
>	{
>    public:
>	A* next;
>	int i;
>	A* get_next();
>	int f();
>	};
>
>    A* A::get_next() { return next; }
>
>    void g(A* a)
>	{
>	int b = ( a->get_next()->i == a->f() );
>	}
>
>Assume that A::f() may change A::next which CFront must since A::f()
>if not a const member.  Since order of evaluation is not defined for
>expressions either a->get_next() or a->f() will happen first, but
>the "->i" will definetly refer to one of two possible As.
>
>Now the problem.  If I make A::get_next() inline CFront generates
>code something like the following:
>
>	b = ( a->next->i == f(a) );
>
>The semantics of this expression are not defined.  Your C compiler
>may, and mine did, evaluate a->next into a temporary, evaluate a->f()
>which may change a->next, and finally evaluate temporary->i which
>may no longer be valid.

It seems that you may have desired that the entire left hand operand of ==
be evaluated before *any* of the right hand operand of the same == was
evaluated.  While this may indeed have been desirable in this case, I
don't see that this is required by either the ANSI C rules or the (virtually
identical?) C++ rules relating to sequence points.

I'd be perfectly happy to be proven wrong however.


// Ron Guilmette (rfg@ics.uci.edu)
// C++ Entomologist
// Motto:  If it sticks, force it.  If it breaks, it needed replacing anyway.