[squeak-dev] Decompiler issues

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Mon Mar 1 20:42:41 UTC 2010


2010/3/1 Levente Uzonyi <leves at elte.hu>:
> Hi,
>
>
> I recently ran into a new decompiler issue which doesn't allow the debugger
> to open. Details here: http://bugs.squeak.org/view.php?id=7467
>
> So I thought it's time to revisit my decision of making DecompilerTests a
> subclass of LongTestCase, because I'm pretty sure noone is running these
> tests. According to my measurements 60-70% of their runtime is spent with
> garbage collecting. DecompilerTests >> #decompileClassesSelect: forces a
> full GC for every class in the image (>2000 in a clear trunk image). Is this
> GC still necessary?
>
> There are also two new methods besides EventSensor >> #eventTickler which
> give a Syntax Error when running these tests. The new methods are
> SHMCClassDefinition >> #withAllSuperclasses and SHMCClassDefinition >>
> #allInstVarNames.
> This problem occurs when a temporary is declared in an inlined block which
> is the receiver of #whileTrue, #whileTrue:, #whileFalse or #whileFalse and
> the variable is also referenced from a normal block.
> For example:
>        [
>                | foo |
>                [ foo := false ] value ] whileTrue
> will be decompiled as: [[_r1 := false] value] whileTrue.
>
> A lot of decompiler tests are failing, because the temporaries are indexed
> in a different order during the second decompilation.
>
> I hope someone with enough knowledge will fix these issues.
>

I have no special knowledge.
The code is quite complex (measured with # of instVars, # of methods,
and # line of codes in certain methods).
But, thanks you provided a good test case.

The first problem I see right from beginning is the order of trailer temp names:
trailer tempNames -> '[input i index1 ps k count (digits)[each (digits)]]'
I guess parentheses (digits) expresses a copiedValue.

Let explore the byte codes to see if it fits:

'1 0' readStream
81 <22> pushConstant: '1 0'
82 <D1> send: readStream

[:input: | ...] block argument
83 <8F 01 00 67> closureNumCopied: 0 numArgs: 1 bytes 87 to 189
87 	<73> pushConstant: nil => i
88 	<73> pushConstant: nil => index1
89 	<73> pushConstant: nil => ps
90 	<73> pushConstant: nil => k
91 	<73> pushConstant: nil => count
92 	<73> pushConstant: nil => UNUSED SLOT ?????
93 	<73> pushConstant: nil => copiedValues #( digits )
94 	<10> pushTemp: 0               ( input )
95 	<DE> send: skipSeparators
96 	<87> pop
97 	<50> pushLit: Integer
98 	<10> pushTemp: 0              ( input )
99 	<EF> send: readFrom:
100 	<69> popIntoTemp: 1          ( i := )
101 	<11> pushTemp: 1             ( i )
102 	<75> pushConstant: 0
103 	<B6> send: =

whileFalse: [...
104 	<A8 52> jumpTrue: 188
106 	<8A 01> push: (Array new: 1)
108 	<6F> popIntoTemp: 7         ( #( digits ) ) copiedValues... Note
that this is at rank 8, not 7 as expected from trailer tempNames...
109 	<76> pushConstant: 1
110 	<6C> popIntoTemp: 4        ( k :=)
111 	<75> pushConstant: 0
112 	<6D> popIntoTemp: 5        ( count := )
113 	<44> pushLit: Array
114 	<25> pushConstant: 10
115 	<75> pushConstant: 0
116 	<F3> send: new:withAll:
117 	<8E 00 07> popIntoTemp: 0 inVectorAt: 7  ( digits := )

120 	<15> pushTemp: 5            ( count )
121 	<11> pushTemp: 1            ( i )
122 	<B2> send: <

whileTrue: [
123 	<AC 3A> jumpFalse: 183
125 	<14> pushTemp: 4            ( k )
126 	<D6> send: printString
127 	<6B> popIntoTemp: 3        ( ps := )
128 	<13> pushTemp: 3
129 	<2C> pushConstant: $1
130 	<EB> send: indexOf:
131 	<81 42> storeIntoTemp: 2  ( index1 := )
133 	<75> pushConstant: 0
134 	<B6> send: =
135 	<99> jumpFalse: 138
136 	<71> pushConstant: true
137 	<95> jumpTo: 144
138 	<13> pushTemp: 3
139 	<2A> pushConstant: $3
140 	<12> pushTemp: 2            ( index1 )
141 	<F9> send: indexOf:startingAt:
142 	<75> pushConstant: 0
143 	<B6> send: =

ifTrue: [
144 	<AC 1F> jumpFalse: 177
146 	<15> pushTemp: 5            ( count )
147 	<76> pushConstant: 1
148 	<B0> send: +
149 	<6D> popIntoTemp: 5        ( count := )
150 	<13> pushTemp: 3            ( ps )
151 	<17> pushTemp: 7            ( #( digits) )
152 	<8F 11 00 13> closureNumCopied: 1 numArgs: 1 bytes 156 to 174
156 		<8C 00 01> pushTemp: 0 inVectorAt: 1  ( digits )
159 		<10> pushTemp: 0                                ( each )
160 		<D7> send: asciiValue
161 		<28> pushConstant: 47
162 		<B1> send: -
163 		<8C 00 01> pushTemp: 0 inVectorAt: 1  ( digits )
166 		<10> pushTemp: 0                               ( each )
167 		<D7> send: asciiValue
168 		<28> pushConstant: 47
169 		<B1> send: -
170 		<C0> send: at:
171 		<76> pushConstant: 1
172 		<B0> send: +
173 		<C1> send: at:put:
174 		<7D> blockReturn
175 	<CB> send: do:
176 	<87> pop
177 	<14> pushTemp: 4       ( k )
178 	<76> pushConstant: 1
179 	<B0> send: +
180 	<6C> popIntoTemp: 4   ( k := )
181 	<A3 C1> jumpTo: 120
183 	<70> self
184 	<DD> send: halt
185 	<87> pop
186 	<A3 A2> jumpTo: 94
188 	<73> pushConstant: nil
189 	<7D> blockReturn
190 <E0> send: in:
191 <7C> returnTop

What look *** STRANGE *** is the UNUSED slot reserved in [:input | ...
] temp vector...


Now decompiling in initSymbols: >>
mapFromBlockStartsIn:toTempVarsFrom:constructor:

I got this map: there are only 3 blocks: (the other onesare inlined)
- the whole method (startpc=81)
- the [:input | ...] block (startpc=87)
- the [:each | ...] block (startpc=156)

And the temps names are (for each block indexed by start pc)
a Dictionary(
    81->#()
    87->#(
           #('input' 1)
           #('i' 2)
           #('index1' 3)
           #('ps' 4)
           #('k' 5)
           #('count' 6)
           #('digits' #(7 1)))
    156->#(
           #('each' 1)
           #('digits' #(2 1))) )
I think we got a problem here... digits should be #(8 1) to fit the
compiled code and the UNUSED slot...

the tempVector has just 7 slots:
    {{input} . {i} . {index1} . {ps} . {k} . {count} . {{digits}}}
later replaced with:
    {{input} . {i} . {index1} . {ps} . {k} . {count} . {_r7}}

Then, when decompiling:
108 	<6F> popIntoTemp: 7         ( #( digits ) ) copiedValues... Note
that this is at rank 8, not 7 as expected from trailer tempNames...
the assertion fails because we only have 7 temps vars and try to
assign temp of rank 8...

Now, just have to discover how the UNUSED slot was ever compiled...

Eliot might be more efficient than me...

Nicolas

>
> Cheers,
> Levente
>
>



More information about the Squeak-dev mailing list