Path: utzoo!attcan!uunet!lll-winken!ames!coherent!dplatt
From: dplatt@coherent.com (Dave Platt)
Newsgroups: comp.sys.mac.programmer
Subject: Re: serious code generation bug in Lightspeed C
Summary: Not really.
Message-ID: <17782@coherent.com>
Date: 12 Jan 89 22:03:18 GMT
References: <6275@hoptoad.uucp>
Reply-To: dplatt@coherent.com (Dave Platt)
Distribution: comp
Organization: Coherent Thought Inc., Palo Alto CA
Lines: 81

In article <6275@hoptoad.uucp> tim@hoptoad.uucp (Tim Maroney) writes:
> There is a serious bug in how Lightspeed C handles the assignment
> operation.  The lvalue is evaluated before the value is computed.  This
> means that assigments to relocatable structure members do not work
> correctly.
> 
> For example, suppose you have a relocatable data structure and a handle
> to that structure in "h".  Further suppose that one member of the
> structure referred to by "h" is of type Ptr, called "p".  Then this
> straightforward statement gives an incorrect result:
> 
> 	(*h)->p = NewPtr(size);
> 
> The address of "(*h)->p", the lvalue, is computed before the right-hand
> side is evaluated.  The address of "(*h)->p" may change as a result of
> the NewPtr trap, therefore it may be wrong on return from the trap and
> the result of NewPtr may be assigned into invalid memory.
> 
> This is pretty amazing, even for a compiler like Think's that
> deliberately produces low-budget, non-optimized code.  There is a
> work-around, assigning the result of NewPtr into an intermediate
> variable and then assigning it to the lvalue, but it is unwieldy, and
> it is difficult to spot all places where this compiler bug could bite.

I sympathize with you... I've been bitten by this characteristic of LsC
at least five times during the past year.  However, I must disagree with
your assessment of this as a "bug".  It isn't... at least, not according
to the Wholly White Book as written by Kernighan and Ritchie.

To quote from page 50 of the 1978 edition of K&R:

   Function calls, nested assignment statements, and increment and
   decrement operators cause "side effects" -- some variable is changed
   as a byproduct of the evaluation of an expression.  In any expression
   involving side effects, there can be subtle dependencies on the order
   in which variables taking part in the expression are stored.  One
   unhappy situation is typified by the statement

	a[i] = i++;

   The question is whether the subscript is the old value of i or the
   new.  The compiler can do this in different ways, and generate
   different answers depending on its interpretation.  When side effects
   (assignment to actual variables) takes place is left to the
   discretion of the compiler, since the best order depends strongly on
   machine architecture.

   The moral of this discussion is that writing code which depends on
   order of evaluation is a bad programming practice in any language.
   Naturally, it is necessary to know what things to avoid, but it you
   don't know _how_ they are done on various machines, that innocence
   may halp protect you.  (The C verifier _lint_ will detect most
   dependencies on order of evaluation).

The statemement that you've specified above can be rewritten (in the
abstract) as:

	OFFSET_TO_P[0[h]] = NewPtr(size);

Because the NewPtr() function can scramble or compact the heap, the
subexpression 0[h] (or *h, if you prefer) can be changed as a side
effect.  Hence, if the compiler has chosen to evaluate 0[h] prior to
calling NewPtr (as it is _permitted_ to do by the K&R definition!), then
the value it evaluated and cached away will have been invalidated, and
the result of NewPtr() will be stored into the wrong place.

As I said... I sympathize with your dismay and annoyance, but it's not
due to a bug in LsC;  it's due to a well-documented instance in which
different compilers may choose different orders of evaluation.

Running your program with TMON's heap-scramble option turned on will
catch most of these problems quite quickly.  Failing to perform such a
stress-test will tend to leave such bugs undiscovered until two days
after your product goes into general commercial release, at which point
machines will begin crashing with utmost predictability ( 1/2 ;-).

-- 
Dave Platt    FIDONET:  Dave Platt on 1:204/444        VOICE: (415) 493-8805
  UUCP: ...!{ames,sun,uunet}!coherent!dplatt     DOMAIN: dplatt@coherent.com
  INTERNET:   coherent!dplatt@ames.arpa,    ...@sun.com,    ...@uunet.uu.net 
  USNAIL: Coherent Thought Inc.  3350 West Bayshore #205  Palo Alto CA 94303