[squeak-dev] Decompiler issuesNicolas Cellier nicolas.cellier.aka.nice at gmail.comMon Mar 1 21:57:59 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 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? > I suggest replacing: temporaries remove: aTempVariableNode ifAbsent: [']. with: self actualScope temporaries remove: aTempVariableNode ifAbsent: [']. Nicolas > Nicolas > >> Nicolas >> >>> Eliot might be more efficient than me... >>> >>> Nicolas >>> >>>> >>>> Cheers, >>>> Levente >>>> >>>> >>> >> >
More information about the Squeak-dev mailing list |