2010/3/1 Nicolas Cellier nicolas.cellier.aka.nice@gmail.com:
2010/3/1 Levente Uzonyi leves@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