[squeak-dev] Decompiler issues
Nicolas Cellier
nicolas.cellier.aka.nice at gmail.com
Mon Mar 1 21:29:29 UTC 2010
2010/3/1 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
> 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...
>
Debugging this:
Compiler evaluate: '''1 0'' readStream in: [ :input |
| i |
[
input skipSeparators.
i := Integer readFrom: input.
i = 0 ] whileFalse: [
| k count digits |
k := 1.
count := 0.
digits := Array new: 10 withAll: 0.
[ count < i ] whileTrue: [
| index1 ps |
ps := k printString.
((index1 := ps indexOf: $1) = 0 or: [ (ps indexOf: $3
startingAt: index1) = 0 ]) ifTrue: [
count := count + 1.
ps do: [ :each | digits at: each asciiValue - 47 put:
(digits at: each asciiValue - 47) + 1 ] ].
k := k + 1 ].
self halt ] ] '
Visibly, the digits temp has two slots reserved, one for itself, one
for copiedValues <2-8>:
encoder blockExtentsToLocal
-> a Dictionary(
(0 to: 10)->#()
(2 to: 8)->{{input} . {i} . {index1} . {ps} . {k} . {count} .
{digits} . {<2-8>}}
(4 to: 6)->{{each} . {<2-8>}} )
Nicolas
> Eliot might be more efficient than me...
>
> Nicolas
>
>>
>> Cheers,
>> Levente
>>
>>
>
More information about the Squeak-dev
mailing list
|