- Pharo-dev - lists.pharo.org

Re: [Pharo-dev] [Vm-dev] Re: mustBeBooleanMagicIn can crash the vm

by Nicolai Hess

Hi Eliot, I am pretty sure this is not a vm issue :) The mustBeBooleanInMagic: creates a method on the fly and executes it with the a context as argument. The "ThisContext" is not the "thisContext", but the argument of the generated method. This is the bytecode of the generated method: 17 <00> pushRcvr: 0 18 <8F 00 00 02> closureNumCopied: 0 numArgs: 0 bytes 22 to 23 22 <76> pushConstant: 1 23 <7D> blockReturn 24 <8F 00 00 02> closureNumCopied: 0 numArgs: 0 bytes 28 to 29 28 <75> pushConstant: 0 29 <7D> blockReturn 30 <F0> send: ifTrue:ifFalse: 31 <7C> returnTop The original method is foo ^ notABool1 ifTrue:[1] ifFalse:[0] The generated method compiles this part notABool1 ifTrue:[1] ifFalse:[0] to bytecode without the jumpFalse optimization. The problem is that the receiver for the ifTrue:ifFalse send is accessed through pushRcvr:0, whereas the thisContext is not the context of the original method anymore. Nicolai 2014-08-10 17:34 GMT+02:00 Eliot Miranda <eliot.miranda(a)gmail.com>: > > Hi Nicolai, > > please try and remember to invlude vm-dev when you see a VM issue... > > > On Sun, Aug 10, 2014 at 7:34 AM, Nicolai Hess <nicolaihess(a)web.de> wrote: > >> There is a certain risk, that the mustBeBooleanMagicIn method rewrite can >> crash the >> vm >> Issue 13805 <https://pharo.fogbugz.com/default.asp?13805> >> > > Since you're using the Opal compiler ca you add the description of the > bytecode (aCompiledMethod symbolic) to the issue? I'll try and take a look > soon but want to be sure I'm debugging the relevant code. > > The receiver of the rewritten method is the "nonboolean" value receiving >> the >> ifTrue:ifFalse: message. But the rewritten method can include >> instvar accessor and self sends of the original method. >> >> We can include more rewrite rules in mustBeBooleanMagicIn like >> "rewrite instvar accessing" >> context receiver class instVarNamesAndOffsetsDo: [ :n :o | >> RBParseTreeRewriter new >> replace: n with: ('ThisContext receiver instVarAt: ', o asString); >> executeTree: methodNode. >> ]. >> "rewrite self sends" >> RBParseTreeRewriter new >> replace: 'self' with: 'ThisContext receiver'; >> executeTree: methodNode. >> >> But I don't know if this works for all possible situations. Or if there >> are >> better ways to do the rewriting. >> > > Remember the mirror methods: > thisContext object: o instVarAt: > which doesn't send to o, reaching directly into it. > > But what's the point of replacing "self" with "thisContext receiver"? The > latter is potentially an extremely slow way of doing the former. The > former won't create a context object in a method activation that doesn't > need one (doesn't include a block), whereas the latter always will. But > they will always yield the same result. > -- > curious, > Eliot > >

Aug. 10, 2014

Re: [Pharo-dev] mustBeBooleanMagicIn can crash the vm

by Camille Teruel

Hi Nicolai and Eliot, This is not a bug in the VM but in mustBeBooleanMagic:. The last line is "^ self withArgs: {context} executeMethod: method" whereas it should be "^ context receiver withArgs: {context} executeMethod: method". Thank you for reporting this, I committed the fix. Slice in inbox. Cheers, Camille On 10 aoÃ»t 2014, at 17:34, Eliot Miranda <eliot.miranda(a)gmail.com> wrote: > Hi Nicolai, > > please try and remember to invlude vm-dev when you see a VM issue... > > > On Sun, Aug 10, 2014 at 7:34 AM, Nicolai Hess <nicolaihess(a)web.de> wrote: > There is a certain risk, that the mustBeBooleanMagicIn method rewrite can crash the > vm > Issue 13805 > > Since you're using the Opal compiler ca you add the description of the bytecode (aCompiledMethod symbolic) to the issue? I'll try and take a look soon but want to be sure I'm debugging the relevant code. > > The receiver of the rewritten method is the "nonboolean" value receiving the > ifTrue:ifFalse: message. But the rewritten method can include > instvar accessor and self sends of the original method. > > We can include more rewrite rules in mustBeBooleanMagicIn like > "rewrite instvar accessing" > context receiver class instVarNamesAndOffsetsDo: [ :n :o | > RBParseTreeRewriter new > replace: n with: ('ThisContext receiver instVarAt: ', o asString); > executeTree: methodNode. > ]. > "rewrite self sends" > RBParseTreeRewriter new > replace: 'self' with: 'ThisContext receiver'; > executeTree: methodNode. > > But I don't know if this works for all possible situations. Or if there are > better ways to do the rewriting. > > Remember the mirror methods: > thisContext object: o instVarAt: > which doesn't send to o, reaching directly into it. > > But what's the point of replacing "self" with "thisContext receiver"? The latter is potentially an extremely slow way of doing the former. The former won't create a context object in a method activation that doesn't need one (doesn't include a block), whereas the latter always will. But they will always yield the same result. > -- > curious, > Eliot

Aug. 10, 2014

Re: [Pharo-dev] mustBeBooleanMagicIn can crash the vm

by Eliot Miranda

Hi Nicolai, please try and remember to invlude vm-dev when you see a VM issue... On Sun, Aug 10, 2014 at 7:34 AM, Nicolai Hess <nicolaihess(a)web.de> wrote: > There is a certain risk, that the mustBeBooleanMagicIn method rewrite can > crash the > vm > Issue 13805 <https://pharo.fogbugz.com/default.asp?13805> > Since you're using the Opal compiler ca you add the description of the bytecode (aCompiledMethod symbolic) to the issue? I'll try and take a look soon but want to be sure I'm debugging the relevant code. The receiver of the rewritten method is the "nonboolean" value receiving the > ifTrue:ifFalse: message. But the rewritten method can include > instvar accessor and self sends of the original method. > > We can include more rewrite rules in mustBeBooleanMagicIn like > "rewrite instvar accessing" > context receiver class instVarNamesAndOffsetsDo: [ :n :o | > RBParseTreeRewriter new > replace: n with: ('ThisContext receiver instVarAt: ', o asString); > executeTree: methodNode. > ]. > "rewrite self sends" > RBParseTreeRewriter new > replace: 'self' with: 'ThisContext receiver'; > executeTree: methodNode. > > But I don't know if this works for all possible situations. Or if there are > better ways to do the rewriting. > Remember the mirror methods: thisContext object: o instVarAt: which doesn't send to o, reaching directly into it. But what's the point of replacing "self" with "thisContext receiver"? The latter is potentially an extremely slow way of doing the former. The former won't create a context object in a method activation that doesn't need one (doesn't include a block), whereas the latter always will. But they will always yield the same result. -- curious, Eliot

Aug. 10, 2014

Re: [Pharo-dev] [Vm-dev] VM crash from triple 'Smalltalk garbageCollect' then forked Transcript stress test

by Eliot Miranda

Hi Ben, I'll try and take a look at this, but it'll really make things easier to do that if you create an image that runs the snipped from startup. e.g. evaluate the snipped preceded by Smalltalk snapshot: true andQuit: true Launch and if the VM doesn't crash rinse and repeat changing the snippet until the VM does crash on startup without user intervention. It's much easier to debug then. Then put that image & changes somewhere and email me the URL. On Sun, Aug 10, 2014 at 7:31 AM, Ben Coman <btc(a)openinworld.com> wrote: > > For Pharo I found the following snippet crashes the VM with fresh images. > > > Smalltalk garbageCollect. Smalltalk garbageCollect. Smalltalk > garbageCollect. > Transcript open. > [ Transcript crShow: ( > [ | string | > string := '-'. > 1 to: 2000 do: > [ :n | > string := string , '-', n printString. > Transcript show: string. > ]. > (Delay forMilliseconds: 10) wait. > ]) timeToRun. > ] fork. > > Over a dozen runs, this usually occurred on the second evaluation of that > snippet, and once each on the third and fourth evaluation of that snippet. > > > If there is only a single 'Smalltalk garbageCollect' rather than the > three, then the VM doesn't crash. > (Aside: I don't really know what use running it three times is, I just see > it down elsewhere.) > > I recorded four runs. > run1, run2, run3 were with... > $ curl get.pharo.org/alpha+vmLatest | bash > and run4 with... > $ curl get.pharo.org/30+vmLatest | bash > > Console logs and crash.dmp attached. > > --------- > All four runs show the same crash info at top... > --------- > Segmentation fault Sun Aug 10 21:17:09 2014 > C stack backtrace: > 0 Pharo 0x0004c0ef reportStackState + 159 > --------- > and bottom... > --------- > (Segmentation fault) > ./pharo-ui: line 11: 21593 Abort trap: 6 > "$DIR"/"pharo-vm/Pharo.app/Contents/MacOS/Pharo" "$@" > --------- > but different stack dumps... > ---------run1 > Smalltalk stack dump: > 0xbffb68e4 M SmallInteger(Integer)>timesRepeat: 0x1=0 > 0xbffb6900 M CompositionScanner(CharacterScanner)>handleIndentation > 0x2300ced4: a(n) CompositionScanner > 0xbffb6920 M > CompositionScanner>composeFrom:inRectangle:firstLine:leftSide:rightSide: > 0x2300ced4: a(n) CompositionScanner > 0xbffb695c M TextComposer>composeEachRectangleIn: 0x21f2a340: a(n) > TextComposer > ---------run2 > Smalltalk stack dump: > 0xbffb7868 M BlockClosure>on:do: 0x21ff5a64: a(n) BlockClosure > 0xbffb7888 M BlockClosure>on:fork: 0x21ff5a64: a(n) BlockClosure > 0xbffb78a8 M WeakAnnouncementSubscription>deliver: 0x20e20260: a(n) > WeakAnnouncementSubscription > 0xbffb78cc M [] in SubscriptionRegistry>deliver:to:startingAt: 0x20e1e688: > a(n) SubscriptionRegistry > 0xbffb78ec M BlockClosure>ifCurtailed: 0x21ff59d4: a(n) BlockClosure > ---------run3 > 0xbfff204c M SmalltalkImage>garbageCollect 0x1fd577c4: a(n) SmalltalkImage > 0xbfff2064 M UndefinedObject>(nil) 0x1f500004: a(n) UndefinedObject > 0xbfff2094 I OpalCompiler>evaluate 0x220c1184: a(n) OpalCompiler > 0xbfff20c0 I SmalltalkEditor>evaluateSelectionAndDo: 0x20b2e930: a(n) > SmalltalkEditor > 0xbfff20e4 I SmalltalkEditor>evaluateSelection 0x20b2e930: a(n) > SmalltalkEditor > 0xbfff2104 I [] in PluggableTextMorph>doIt 0x20b18014: a(n) > PluggableTextMorph > ---------run4 > Smalltalk stack dump: > 0xbffb501c I SmalltalkImage>garbageCollect 0x1f969748: a(n) SmalltalkImage > 0xbffb5034 M UndefinedObject>(nil) 0x1f500004: a(n) UndefinedObject > 0xbffb5064 I OpalCompiler>evaluate 0x21eebda0: a(n) OpalCompiler > 0xbffb5090 I SmalltalkEditor>evaluateSelectionAndDo: 0x20970488: a(n) > SmalltalkEditor > 0xbffb50b4 I SmalltalkEditor>evaluateSelection 0x20970488: a(n) > SmalltalkEditor > 0xbffb50d4 I [] in PluggableTextMorph>doIt 0x209587f0: a(n) > PluggableTextMorph > --------- > > Maybe its Image code doing bad things rather than a VM bug, but I'm not > qualified to tell. Also I'm working on removing > ThreadSafeTranscriptPluggableTextMorph which wraps #update: and #drawOn: > with a mutex to avoid the same snippet causing a Red-Morph-Of-Death, so > maybe those changes will bypass the bug, but thought this report might be > generally useful if it flushed out some corner case. > > I'll copy any mail list discussion to > https://pharo.fogbugz.com/default.asp?13822. > cheers -ben > > -- best, Eliot

Aug. 10, 2014

New address for the Spec jenkins

by Benjamin

If people are interested having the latest Spec bleeding image, it can now be retrieved at: http://benjamin.is-a-geek.com:8080/job/Spec/ :) Ben

Aug. 10, 2014

mustBeBooleanMagicIn can crash the vm

by Nicolai Hess

There is a certain risk, that the mustBeBooleanMagicIn method rewrite can crash the vm Issue 13805 <https://pharo.fogbugz.com/default.asp?13805> The receiver of the rewritten method is the "nonboolean" value receiving the ifTrue:ifFalse: message. But the rewritten method can include instvar accessor and self sends of the original method. We can include more rewrite rules in mustBeBooleanMagicIn like "rewrite instvar accessing" context receiver class instVarNamesAndOffsetsDo: [ :n :o | RBParseTreeRewriter new replace: n with: ('ThisContext receiver instVarAt: ', o asString); executeTree: methodNode. ]. "rewrite self sends" RBParseTreeRewriter new replace: 'self' with: 'ThisContext receiver'; executeTree: methodNode. But I don't know if this works for all possible situations. Or if there are better ways to do the rewriting.

Aug. 10, 2014

VM crash from triple 'Smalltalk garbageCollect' then forked Transcript stress test

by Ben Coman

Aug. 10, 2014

pillar parser with tokens?

by Tudor Girba

Hi, The Pillar parser does not record the token from the original text. Are there any plans to change this part :)? Doru -- www.tudorgirba.com "Every thing has its own flow"

Aug. 10, 2014

Re: [Pharo-dev] allUnimplementedCalls removed

by stepharo

I know that marcus is cleaning systemNavigation and moving some queries opening UI to AbstractTools. Now someone should have a look at what he is doing and give him feedback. Stef On 10/8/14 08:42, Pavel Krivanek wrote: > Hi Marcus, > > was it appropriate to remove allUnimplementedCalls from the > SystemNavigation? We lost an important message to check the image > health. Is there any replacement? > > Cheers, > -- Pavel > > >

Aug. 10, 2014

Re: [Pharo-dev] Duplicate repositories in fresh Image

by stepharo

Aug. 10, 2014