[squeak-dev] Interesting Decompiler bug

Frank Shearar frank.shearar at gmail.com
Thu Jun 20 14:14:26 UTC 2013


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.

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