[Vm-dev] (Squeak) Compiler bug?

Thu Jan 12 00:38:28 UTC 2023

Hi Eliot,

Thank you for fixing the decompiler.

And now for an actual compiler bug :)
It is not critical, but it's a bug nonetheless: the parser "swallows"
comments for the last statement in blocks where the statement is not a
return and it ends with a period.
In these situations, the parser ends up calling addComment twice on the
same parseNode, nilling out the comment that was put there in the first
call.

As an example:

test
true ifTrue: [self yourself. "a comment"]

or, even simpler:

test
self yourself. "a comment"

All the best,
Florin

On Tue, Jan 10, 2023 at 12:19 AM Florin Mateoc <florin.mateoc at gmail.com>
wrote:

> To be more precise, I do agree with you that the bug has nothing to do
> with Compiler-eem.483, and it is rather related to the perturbations that
> the new bytecode set and the full blocks have introduced into the delicate
> machinery of the decompiler.
> I am pretty sure that, in a parallel universe, someone wrote or generated
> a method that is triggering the same bug without the compiler changes, we
> have just not stumbled upon it yet :)
>
> On Mon, Jan 9, 2023 at 8:45 PM Florin Mateoc <florin.mateoc at gmail.com>
> wrote:
>
>> It's weirder than that.
>> I don't think Compiler-eem.483 itself introduces any bug, but the
>> changes it introduces are enough to make the bug appear.
>> Compile with SistaV1 bytecode set and full blocks in the stable 6.0
>> release (which is prior to Compiler-eem.483) and decompilation works
>> there as well
>>
>>
>> On Mon, Jan 9, 2023 at 7:53 PM Eliot Miranda <eliot.miranda at gmail.com>
>> wrote:
>>
>>>
>>> BTW, it's not to do with Compiler-eem.483; it's to do with the SistaV1
>>> bytecode set and full blocks.  Compile with the old bytecode set and
>>> embedded blocks and decompilation works.
>>>
>>> On Mon, Jan 9, 2023 at 10:45 AM Eliot Miranda <eliot.miranda at gmail.com>
>>> wrote:
>>>
>>>> Hi Florin,
>>>>
>>>>     I've found the issue, which is an infinite loop in the Decompiler
>>>> for your undecompilable version.  Strangely enough it has nothing to do
>>>> with the code you modify. It has to do with the middle ifTrue: of the three
>>>> at the end of the final block:
>>>>
>>>>                    mm1 eventHandler
>>>>                         ifNotNil: [(morphs includesAllOf: mm1
>>>> eventHandler allRecipients)
>>>>                                 ifTrue: [mm1 eventHandler: nil]].
>>>>
>>>>
>>>> *mm1 isMorphicModel                        ifTrue: [mm1 model
>>>> isMorphicModel                                ifTrue: [mm1 model
>>>> breakDependents]].*
>>>>                     mm1 isTextMorph
>>>>                         ifTrue: [mm1 setContainer: nil]]].
>>>>
>>>> I can fix it for this one method, but not without breaking
>>>> decompilation of about 40 other methods.  So I need to understand it
>>>> better.  Give me a few days.  I've called on a higher power (the original
>>>> author is Dan himself). So hopefully we'll get this sorted very soon.
>>>>
>>>> On Sun, Jan 8, 2023 at 7:01 PM Florin Mateoc <florin.mateoc at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> Happy New Year everybody!
>>>>>
>>>>> I hope you will forgive my transgression, I know this is not about the
>>>>> VM, it is about the bytecode compiler, but I am only subscribed to this
>>>>> list, and the Squeak compiler is somewhere in between.
>>>>>
>>>>> I have recently stumbled upon a regression introduced by
>>>>> Compiler-eem.483
>>>>> One of my generated (transformed) methods is apparently valid
>>>>> Smalltalk - it can still be compiled - but it cannot be decompiled anymore.
>>>>> I am not sure if this is a compiler bug, or a latent decompiler bug exposed
>>>>> by the changes in Compiler-eem.483
>>>>>
>>>>> My method is below, it is a transformed variant of
>>>>> NativeImageSegment>>findRogueRootsPrep
>>>>>
>>>>> The bug is somehow caused by the literal #Marker. I have started at
>>>>> the bytecodes and they look ok to me, but the decompiler enters an
>>>>> (apparently unrelated) infinite loop. It is also not just about the literal
>>>>> itself or the immediate surrounding code, the method needs to be
>>>>> big/complex enough (over a certain number of literals?) to trigger the bug.
>>>>>
>>>>>
>>>>> findRogueRootsPrep1
>>>>> "Part of the tool to track down unwanted pointers into the segment.
>>>>> Break all owner pointers in submorphs, scripts, and viewers in flaps."
>>>>> | wld players morphs |
>>>>> wld := arrayOfRoots detect: [:obj | obj isMorph and: [obj
>>>>> isWorldMorph]] ifNone: [].
>>>>> wld == nil ifTrue: [
>>>>>   (wld := arrayOfRoots
>>>>>     detect: [:obj1 | obj1 isMorph]
>>>>>     ifNone: [#Marker]
>>>>>   ) == #Marker ifTrue: [
>>>>>     ^ self error: 'can''t find a root morph']].
>>>>> morphs := IdentitySet new: 400.
>>>>> wld allMorphsAndBookPagesInto: morphs.
>>>>> players := wld presenter allExtantPlayers. "just the cached list"
>>>>> players do: [:pp | | scriptEditors |
>>>>>   scriptEditors := pp class tileScriptNames collect: [:nn | pp
>>>>> scriptEditorFor: nn].
>>>>>   scriptEditors do: [:se | morphs addAll: se allMorphs]].
>>>>> wld submorphs do: [:mm | "non showing flaps"
>>>>>   (mm isKindOf: FlapTab) ifTrue: [
>>>>>     mm referent allMorphsAndBookPagesInto: morphs]].
>>>>> morphs do: [:mm1 | "break the back pointers"
>>>>>   mm1 isInMemory ifTrue: [
>>>>>     (mm1 respondsTo: #target) ifTrue: [
>>>>>       mm1 nearestOwnerThat: [:ow | | zzzTemp |
>>>>>         ow == mm1 target ifTrue: [
>>>>>           mm1 target: nil.
>>>>>           zzzTemp := true
>>>>>         ] ifFalse: [
>>>>>           zzzTemp := false].
>>>>>         zzzTemp]].
>>>>>     (mm1 respondsTo: #arguments) ifTrue: [
>>>>>       mm1 arguments do: [:arg |
>>>>>         arg == nil ifFalse: [
>>>>>           mm1 nearestOwnerThat: [:ow1 | | zzzTemp1 |
>>>>>             ow1 == arg ifTrue: [
>>>>>               mm1 arguments at: (mm1 arguments indexOf: arg) put: nil.
>>>>>               zzzTemp1 := true
>>>>>             ] ifFalse: [
>>>>>               zzzTemp1 := false].
>>>>>             zzzTemp1]]]].
>>>>>     mm1 eventHandler == nil ifFalse: ["recipients point back up"
>>>>>       (morphs includesAllOf: mm1 eventHandler allRecipients) ifTrue: [
>>>>>         mm1 eventHandler: nil]].
>>>>>     "temporary, until using Model for PartsBin"
>>>>>     mm1 isMorphicModel ifTrue: [
>>>>>       mm1 model isMorphicModel ifTrue: [
>>>>>         mm1 model breakDependents]].
>>>>>     mm1 isTextMorph ifTrue: [
>>>>>       mm1 setContainer: nil]]].
>>>>> (Smalltalk includesKey: #Owners) ifTrue: [
>>>>>   Smalltalk at: #Owners put: nil].
>>>>> "in case findOwnerMap: is commented out"
>>>>> "self findOwnerMap: morphs."
>>>>> morphs do: [:mm2 | "break the back pointers"
>>>>>   mm2 isInMemory ifTrue: [
>>>>>     mm2 privateOwner: nil]]
>>>>>
>>>>>
>>>>> Florin
>>>>>
>>>>
>>>>
>>>> --
>>>> _,,,^..^,,,_
>>>> best, Eliot
>>>>
>>>
>>>
>>> --
>>> _,,,^..^,,,_
>>> best, Eliot
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20230111/3bca8505/attachment.html>