giaccone@uhura.cc.rochester.edu (Tony Giaccone) (06/29/89)
I've spent a little time recently trying to track down a problem with
my Lightspeed C code. After a little investigation, I've come upon the
following problem. I think it worth noting. In general it seems that
if your asigning values returned from functions to fields of data
structures pointed to handles you should make sure you lock the block
before hand.
In my case I was trying to allocate memory for a bitmap image, and
I stored that Pointer in a structure pointed to by a handle. The C
source code looks like this:
(**myImage).rasI_image = (SIPtr) NewPtr(size);
if ((**myImage).rasI_image == NULL) {
Where myImage is a handle to a image data structure, and rasI_image is a
pointer to unsigned char.
Lightspeed C compiles this C code to the following code fragment:
1 MOVEA.L $FE8E(A5), A0 ; $FE8E(A5) is the displacment off the
; stack where the local variable
; myImage is found.
2 CLR.L -(A7)
3 MOVE.L $FFFC(A6),-(A7) ; $FFFC(A6) is the local variable size
4 MOVE.L (A0),$FFD6(A6) ; $FFD6(A6) is not one of my local
; variables.
5 JSR ^$079528 ; Do the NewPtr
6 MOVEA.L (A7)+,A0 ; Pop the Return Code
7 MOVEA.L $FFD6(A6),A1 ; DeRefernce the Pointer.
8 MOVE.L A0,$0008(A1) ; Store the pointer in the myImage
; Data Structure
9 MOVEA.L $FE8E(A5),A0 ; Get the handle
10 MOVEA.L (A0),A0 ; Dereference the Handle
11 TST.L $0008(A0) ;
Now, this code works fine the first time I execute it, I because the
heap doesn't get shuffled. However, the next time I try to execute it, a
serious problem occurs. On line 1 the value of the handle for myImage is
placed in register A0. Then on line 4 that value is dereferenced, and stored
as a displacement $FFD6 off register A6. That dereference occurs before the
call to NewPtr. During NewPtr the handle for the block of memory that
$FE8E(A5) contains changes. Now, when the pointer value returned by NewPtr
is stored, the address used in $FFD6(A6) is used (which is no longer correct
becuase of the heap shuffle caused by NewPtr).
Now obviously the simple work around is to lock the block pointed to by
myImage before I make the call to NewPtr (and in fact that is what I do).
Still it seems odd that Lightspeed C should do part of the dereference
before the call, and the other part after the call. What do you think?
Tony Giaccone
tonyg@cvs.rochester.edurpd@apple.com (Rick Daley) (06/29/89)
In article <2394@ur-cc.UUCP> giaccone@uhura.cc.rochester.edu (Tony Giaccone) writes: > (**myImage).rasI_image = (SIPtr) NewPtr(size); > if ((**myImage).rasI_image == NULL) { ... > it seems odd that Lightspeed C should do part of the dereference > before the call, and the other part after the call. What do you think? This is a nasty problem that bites people every once in a while. However, C does not define the order of evaluation in an assignment. Your code might work with some compilers, but it's not safe. The best thing to do is to use a temporary variable: tmp = (SIPtr) NewPtr(size); if (((**myImage).rasI_image = tmp)== NULL) { This is better than HLocking the block because it avoids fragmenting the heap. The reason you saw the bug is that the Memory Manager had to compact the heap to make room for the new block. If you lock the relocatable block, you might cause the NewPtr to fail. Rick Daley rpd@apple.com
dplatt@coherent.com (Dave Platt) (06/29/89)
In article <2394@ur-cc.UUCP> giaccone@uhura.cc.rochester.edu writes: > > In my case I was trying to allocate memory for a bitmap image, and > I stored that Pointer in a structure pointed to by a handle. The C > source code looks like this: > > (**myImage).rasI_image = (SIPtr) NewPtr(size); (....) > > Now, this code works fine the first time I execute it, I because the > heap doesn't get shuffled. However, the next time I try to execute it, a > serious problem occurs.... > > Now obviously the simple work around is to lock the block pointed to by > myImage before I make the call to NewPtr (and in fact that is what I do). > Still it seems odd that Lightspeed C should do part of the dereference > before the call, and the other part after the call. What do you think? This is a classic "problem"... or, if you wish, a wll-known ambiguity in C. The C language as defined by K&R does not specify the order in which intermediate results are to be computed in many cases. If, for example, a statement contains the expression: (foo(a) + bar) * (foo(c) + d) then the compiler may issue the two calls to "foo" in either order. Similarly, in the case of the NewPtr() call you've issued above, the compiler must evaluate two intermediate results before performing the final assignment. One intermediate result is the value of "NewPtr(size)"; the other is the value of "*myImage". This may be a bit clearer if we rewrite your assigment in the entirely-equivalent form: (*myImage)->rasI_image = (SIPtr) NewPtr(size); Because C does not specify the order in which intermediate values are to be calculated, one must be very cautious about "doing things" that can cause side-effects. In your case, the call to NewPtr() can have a side-effect... memory gets shuffed. If this happens, and if the newPtr() call occurs after the calculation of *myImage, then the side-effect of the memory shuffling invalidates the value of *myImage, and (as you've noticed) the whole world collapses messily. There are two solutions: one acceptable, and one better. The acceptable solution is the one you've proposed: lock the block to which myImage is a handle; this will prevent the objectionable side-effect from occurring. The better solution is to use two statements: tempPtr = (SIPtr) NewPtr(size); (**myImage).rasI_image = tempPtr; This approach is cleaner, because it explicitly specifies the order-of-evaluation and thus avoids the ambituity in C entirely. It's more CPU-efficient, because it's not necessary to spend cycles calling HLock and HUnlock (you weren't even _thinking_ of diddling the lock bit in the handle directly, now were you? ;-). It's also more likely to work if your application is running low on memory; the Memory Manager will be free to move the block referred to by myImage if it needs to make room for the new nonrelocatable block. I'd be somewhat chagrined to admit the number of times I've forgotten about this ambiguity in C and have written "allocate me a foo" code that broke when heap-compaction occurred. Using TMON's heap scrambling code is a sure cure for this sort of bug, though. -- 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, ...@uunet.uu.net USNAIL: Coherent Thought Inc. 3350 West Bayshore #205 Palo Alto CA 94303
duggie@Jessica.stanford.edu (Doug Felt) (06/29/89)
Others have pointed out that C does not specify the order of evaluation in an assignment; I just thought I'd mention the related gotcha that calls of functions in other segments can cause memory compaction as well. Especially since one may change the segmentation scheme in the process of tuning a program, it's a good idea to never use dereferenced handles in function calls, either as parameters or to get the function result, without locking them (or using temporaries). (This problem shows up often in Object Pascal as well, where the notational convenience of being able to use 'object.foo' instead of 'object^^.foo' leads one to forget that dereferencing is taking place. Of course, with Pascal you aren't supposed to know...) Doug Felt Courseware Authoring Tools Project duggie@jessica.stanford.edu
lsr@Apple.COM (Larry Rosenstein) (06/30/89)
In article <3232@portia.Stanford.EDU> duggie@Jessica.stanford.edu (Doug Felt) writes: > (This problem shows up often in Object Pascal as well, where the > notational convenience of being able to use 'object.foo' instead of > 'object^^.foo' leads one to forget that dereferencing is taking place. But the Pascal compiler recognizes these cases and generates safe code (or gives you a compile-time error). Specifically, object.foo := NewHandle(...) dereferences the object after the call to NewHandle. Larry Rosenstein, Apple Computer, Inc. Object Specialist Internet: lsr@Apple.com UUCP: {nsc, sun}!apple!lsr AppleLink: Rosenstein1