[squeak-dev] Interesting Decompiler bug

Levente Uzonyi leves at elte.hu
Thu Jun 20 14:09:29 UTC 2013


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

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