[squeak-dev] Decompiler issuesNicolas Cellier nicolas.cellier.aka.nice at gmail.comMon Mar 1 21:51:18 UTC 2010
2010/3/1 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>: > 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>}} ) > BlockNode>>addRemoteTemp: aTempVariableNode rootNode: rootNode "<MethodNode>" ...snip... temporaries remove: aTempVariableNode ifAbsent: [self halt: 'debug me']. triggers the halt because temporaries isEmpty... in block node [ | k count digits | k := 1. ...] temporaries isEmpty because block is optimized... So the block does not really exist, and the temps are stored in parent block... So when block is optimized, we should better not ask it to remove: aTempVariableNode, should we? Nicolas > Nicolas > >> Eliot might be more efficient than me... >> >> Nicolas >> >>> >>> Cheers, >>> Levente >>> >>> >> >
More information about the Squeak-dev mailing list |