[Vm-dev] [Pharo-dev] eventual crashes pharo vm

[Eliot Miranda]

Hi Henrik,


> On Aug 18, 2017, at 5:30 AM, Henrik Sperre Johansen <henrik.s.johansen at veloxit.no> wrote:
> 
> 
> Even with the bug fixed, if every send is lazily recorded for later
> evaluation, how does the framework deal with non-local branched returns
> though?
> 
> Seems to me, branching selectors need to be blocking points; otherwise,
> (like in this code) if the PromiseERef records it should evaluate ifTrue:
> with the block argument later as the needed value is as yet missing, it will
> also record the following adaptToInteger: which should only occur if value
> was false.

Wouldn't it be the case that the continuation gets created but never executed?  There is nothing to force the adaptToInteger path to be executed unless the wrong answer to isInteger is computed.  So I think this is more of a performance issue than a correctness issue.

> 
> A possible fix *:
> nextBytecodeOffsetAfterJump
>    "check if we are in ifTrue:ifFalse: / ifFalse:ifTrue: or in ifTrue: /
> ifFalse: and answers the next byte code offset"
>    ^destination last isJump 
>        ifTrue: [ destination last destination first bytecodeOffset ]
>        ifFalse: [ 
>            (destination sequence at: (destination sequence size min: 2))
> bytecodeOffset ]
> 
> new test, crashes with old version of nextBytecodeOffsetAfterJump , passes
> with new (it and the others): 
> testIfTrueWithCallReturnAfterTrueBlock
>    | myBooleanObject |
>    myBooleanObject := MyFalseBooleanObject new.
>    self should: [10 *  myBooleanObject] raise: MessageNotUnderstood 
> 
> (With MyFalseBooleanObject subclass of MyBooleanObject, but ifTrue:
> implementation doing nothing)

Cool!

> 
> Cheers,
> Henry
> 
> *Don't ask me how /if I'm sure it works in all cases, I just checked that
> tests pass... (...but the idea is we  want to skip the pushTemp (if present,
> which seems the case other than when followed by block, in which case
> sequence size is 1), since the deoptimized node has been called as a method,
> and its return will already be on stack.?.?.). A better implementation would
> probably be asserting the instruction we are skipping is in fact the temp
> push.

Well I would assert that the stack depth is correct.  In VMMaker.oscog you can find StackDepthFinder, a subclass of InstructionStream that computes the stack depth at each bytecode.  This can be used to check that the stack pointer of the sender context of mustBeBoolean agrees with the  pc that mustBeBooleanMagic: chooses to resume at.

The vm simulator uses StackDepthFinder to check stack depth at every send and non-local return.  It has found lots of bugs.  HTH

> --
> View this message in context: http://forum.world.st/Re-Pharo-dev-eventual-crashes-pharo-vm-tp4961763p4962031.html
> Sent from the Squeak VM mailing list archive at Nabble.com.

Best, Eliot (phone)