[BUG]Compiler screws up bigtime with same global on both sides of an assign

Tim Rowledge tim at sumeru.stanford.edu
Sun Feb 2 05:29:10 UTC 2003


I was attempting to test some code that happened to have
	Smalltalk := SmalltalkEnvironment newFrom: Smalltalk.
in there. It failed. Not, I must point out because of the rather
annoying readonlybinding stuff that seems to have crept into the system.
No, this is a fullscale failure of the Compiler. Sound the alarms! Call
the Fire Brigade!

After a lot of cussing and scratching I feel reasonably sure it is
caused by the (ab)use of LiteralVariableNodes. When translating the
sourcecode
'Smalltalk := Smalltalk'
(the simplest expression that illustrates the problem) we build a
LiteralVariableNode and make it the variable side of an AssignemtnNode.
Then we find 'Smalltalk' again and lookup the global to find a binding
and set the value side of the AssignmentNode to be the very same
LiteralVariableNode. This is how we share literals within a method.

Unfortunately, during the code sizing part of the compilation we send
#sizeForValue: to said LiteralVariableNode, which cheerfully sideeffects
the crucial instvar 'code'. Later, we send #sizeForStorePop: which also
frigs with it. The next phase of compiling is to actually emit the
opcodes into the CompiledMethod. Sadly because the LiteralVariableNode
is now messed up the code output is 
((Compiler new  compile:'junkTest3
Smalltalk _ Smalltalk' in: Object notifying: nil ifFail:[])  generate: #(0 0 0 0)) symbolic '
9 <40> pushLit: Smalltalk
10 <40> pushLit: Smalltalk
11 <CA> send: value:
12 <87> pop
13 <78> returnSelf
'
.. which as any fool can see is f'ing wrong. You end up sending #value:
to the object Smalltalk instead of the global literal association which
has the String 'Smalltalk' as its key and the object Smalltalk as its
value. Kaboom, say goodnight Gracie.

What we _should_ get is
13 <21> pushConstant: ROB(#Smalltalk)
14 <40> pushLit: Smalltalk
15 <CA> send: value:
16 <87> pop
17 <78> returnSelf

Sigh.

Changing Encode>global:name: to return a fresh LiteralVariableNode each
time fixes the code problem but results in CompiledMethods with multiple
literals for each global variable referred. Not the best result we
could hope for.

I'm too tired right now to do any more with this but I'd say there are
several options available to improve this:-

i) change Encoder>global:name: as above and fix up the handling of
literals elsewhere in the code

ii) leave the variable node sharing and change the sizeForStorePop: etc
code to not screw things up.

iii) Y'know we really don't need to do this presizing crap in
MethodNode>generate: It would be entirely plausible to emit into a
stream on a ByteArray and the copy those bytes into a CompiledMethod
which size we would work out trivially from the contents of the byte
array. And it gets even simpler in VI4 since the CompiledMethod isn't
this funky screwed up format anyway. Heck, it might even make compiling
faster.

If you want to play with this, try a few expressions like

((Compiler new  compile:'junkTest3
Smalltalk _ Smalltalk' in: Object notifying: nil ifFail:[]) generate: #(0 0 0 0)) symbolic

and put a halt before the #generate: . Look at the difference between
Smalltalk := Smalltalk
and
Smalltalk := 1
and so on.

tired tim 

-- 
+================================================================+
|Though a nation watched her falling, yet a world could only cry |
|As they passed from us to Glory , riding fire in the sky.       |
|(Jordin Kare, Fire In The Sky)                                  |
|Farewell Columbia.                                              |
+================================================================+
Tim Rowledge, tim at sumeru.stanford.edu, http://sumeru.stanford.edu/tim



More information about the Squeak-dev mailing list