[squeak-dev] Interesting Decompiler bug

Levente Uzonyi leves at elte.hu
Thu Jun 20 17:30:05 UTC 2013


On Thu, 20 Jun 2013, Frank Shearar wrote:

> On 20 June 2013 15:09, Levente Uzonyi <leves at elte.hu> wrote:
>> On Thu, 20 Jun 2013, Nicolas Cellier wrote:
>>
>>> The bug is that (true ifTrue:[^true] ifFalse:[^false]) won't push any
>>> value on the stack (same as emitEffect: rather than emitValue:)
>>> The decompiler wants to find the argument of self value: ... on the stack
>>> and it's not there.
>>>
>>> So either we should forbid such construct at compile time
>>> (emit an 'un-reachable code' error when emitValue: does not emit any
>>> value)
>>
>>
>> I think this is the right thing to do, but the problem is a bit more
>> complex.
>>
>> The following doesn't break the decompier, but it won't be decompiled
>> correctly, because the decompiler avoids creating branches:
>>
>> decompilerBug2
>>
>>         true ifTrue: [ ^1 ] ifFalse: [ ^1 ].
>>         ^2
>
> This is what I was talking about. The Compiler inlines
> #ifTrue:ifFalse: and produces bytecodes that, when decompiled, don't
> match the original source.

That's because for both

 	true ifTrue: [ ^1 ] ifFalse: [ ^1 ]

and

 	true ifTrue: [ ^1 ].
 	^1

the compiler will generate the same bytecode sequence:

17 <71> pushConstant: true
18 <99> jumpFalse: 21
19 <76> pushConstant: 1
20 <7C> returnTop
21 <76> pushConstant: 1
22 <7C> returnTop

The only way this could be "fixed" is to add some extra markers 
(e.g. bytecodes or some trailer data) to help the decompiler decide which 
construct was used in the original source code. But that's just a waste of 
resources most of the time.


Levente

>
> frank
>
>> The decompiler will produce this:
>>
>> decompilerBug2
>>         true
>>                 ifTrue: [^ 1].
>>         ^ 1.
>>         ^ 2
>>
>> Which can't be compiled anymore.
>>
>>
>> The original bug also appears with #caseOf:otherwise: :
>>
>> decompilerBug3
>>
>>         self foo: (
>>                 true
>>                         caseOf: { [ false ] -> [ ^1 ] }
>>                         otherwise: [ ^1 ])
>>
>> #caseOf: :
>>
>> decompilerBug4
>>
>>         self foo: (true caseOf: { [ false ] -> [  ^1 ] })
>>
>> and #repeat :
>>
>> decompilerBug5
>>
>>         self foo: [ ^self ] repeat
>>
>>
>> Levente
>>
>>
>>>
>>> Or patch Decompiler like mad to work around...
>>> (but Decompiler's methods are yet well below usual quality in term of
>>> complexity - long methods, many instance variables, incredibly complex
>>> state, ...)
>>>
>>> I agree, this is going to be a rare problem only for newbies, because an
>>> experimented Smalltalker will avoid writing unreachable code.
>>> So, the third solution is DO NOTHING, or more exactly add a test and an
>>> expected failure.
>>> Like that we'll say next time, hey bad luck, this is a known limitation.
>>>
>>> Nicolas
>>>
>>>
>>>
>>> 2013/6/20 Frank Shearar <frank.shearar at gmail.com>
>>>       On 20 June 2013 02:12, David T. Lewis <lewis at mail.msen.com> wrote:
>>>      > On Thu, Jun 20, 2013 at 01:26:14AM +0200, Nicolas Cellier wrote:
>>>      >> See
>>>      >>
>>> http://stackoverflow.com/questions/17129583/smalltalk-collection-is-empty-error-when-saving/17202938#17202938
>>>      >>
>>>      >> This bug was found by a newbie on pharo 1.1, but it's still there
>>> in 4.5
>>>      >> trunk image
>>>      >>
>>>      >> Try to decompile this (no matter in which class)
>>>      >>
>>>      >> < aNumberWithUnits
>>>      >>     (self compareUnits: aNumberWithUnits)
>>>      >>         ifTrue: [self value: ((aNumberWithUnits value) < (self
>>> value)
>>>      >> ifTrue: [^true] ifFalse: [^false]).]
>>>      >>         ifFalse: [^Error new signal: 'Incompatible unit types.'].
>>>      >>
>>>      >> Thanks to the newbie!
>>>      >> I would just have hated that!
>>>      >> Learning from one's own error is great, but from a dysfunctional
>>> system not
>>>      >> so :(
>>>      >
>>>      > Good bug catch! Here is a reduced version of the method that
>>> produces the
>>>      > same error when decompiled:
>>>      >
>>>      >
>>>      >   Foo>>decompilerBug
>>>      >       self value: (true
>>>      >           ifTrue: [^ true]
>>>      >           ifFalse: [^ false])
>>>
>>> I'm guessing that this is because
>>>
>>> true
>>>   ifTrue: [^ true]
>>>   ifFalse: [^ false]
>>>
>>> decompiles as
>>>
>>> true ifTrue: [^ true]
>>> ^ false
>>>
>>> ?
>>>
>>> I'm not terribly worried about claims of a "dysfunctional system".
>>> This bug is so severe that noone has noticed it before! I agree that
>>> we need to fix the bug, but the original code makes no sense in the
>>> first place. #value: is unreachable.
>>>
>>> frank
>>>
>>>> Dave
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
>
>


More information about the Squeak-dev mailing list