Couple of POSSIBLE Cog fixes [WAS] Re: [Vm-dev] [COG] primitiveClosureValue assert is failing with StackVM (possible BUG in bytecodes!!)

>  
> On Wed, Dec 22, 2010 at 10:34 PM, Eliot Miranda <[hidden email]>wrote:
>
> >
> > Hi Mariano,
> >
> >     indeed it's a bug.  Your fix (objectMemory is: rcvr instanceOf: (self
> > splObj: ClassBlockClosure) compactClassIndex:
> > ClassBlockClosureCompactIndex) is the correct one.  There are a few other
> > places where this needs to be done also.  I'll roll out new code soon.
> >
> >
> Hi Eliot. Before you roll out new code, I would like to share with you some
> "fixes" I have to do to Cog, mostly to improve the support of SmallInteger
> as methods. I attach a .cs but still, some need some explanations. Don't
> expect them to be the better solution/fix, but hopefully their are useful to
> show you a bug/problem and you may come with a better solution.
>
> So, some notes about the changes:
>
> - #primitiveIndexOf:   now answers 0 if it is not a ooCompiledMethod.
> Otherwise, it crash. This method is called by, for example,
> #primitiveFlushCacheByMethod. If I want to use SmallInteger as methods, I
> need to implement #flushCache and call the primitive. But this crashes if we
> don't apply this change.
>
> - Igor have changed CoInterpreter >> readImageFromFile: f HeapSize:
> desiredHeapSize StartingAt: imageOffset
> and added a <var: #desiredHeapSize type: 'usqInt'>. With this we can compile
> the MacOS platform code.
> However, the same change need to be applied to StackInterpreter.

>
> - In #activateInterpreterMethodFromMachineCode  the assert "self assert:
> (self primitiveIndexOf: newMethod) ~= 0"  fails when using Objects as
> methods, since "primitiveFunctionPointer" is different than zero, but
> #primitiveIndexOf:  is zero. Check #addNewMethodToCache:  to see where the
> #primitiveInvokeObjectAsMethod  is set
> FOr this problem, there is probably a better way to solve my ugly hack. But
> at least, the assert doesn't fail ;)
>
> - #methodHasCogMethod:   I am not sure about this one. What do you think?
> Because I have some crashes but I cannot garuantee 100% it was because of
> that.
>
> - #primitiveFlushCacheByMethod  needs to check if the method is really a
> method, because it can be a SmallINteger or any other object
>
> That's all.
>
> Cheers
>
> Mariano
>
>
>
> > best
> > Eliot
> >
> > On Wed, Dec 22, 2010 at 12:03 PM, Mariano Martinez Peck <
> > [hidden email]> wrote:
> >
> >>
> >> Hi Eliot. If I compile a StackVM, and I just press the left arrow in a
> >> code pane of a OB windows, in a pharo image, I get an asser that fails:
> >> (oop & 1) 19202
> >>
> >> Sorry I bother with these asserts, but I am trying to solve the crashes I
> >> have and I think starting for the asserts is a good idea.
> >>
> >> The assert that fails is in primitiveClosureValue()  the part:
> >>
> >> blockClosure = longAt(GIV(stackPointer) + (GIV(argumentCount) *
> >> BytesPerWord));
> >>     /* begin quickFetchInteger:ofObject: */
> >>     oop = longAt((blockClosure + BaseHeaderSize) + (ClosureNumArgsIndex <<
> >> ShiftForWord));
> >>     assert((oop & 1));
> >>     numArgs = (oop >> 1);
> >>     if (!(GIV(argumentCount) == numArgs)) {
> >>         /* begin primitiveFail */
> >>         if (GIV(primFailCode) == 0) {
> >>             GIV(primFailCode) = 1;
> >>         }
> >>         return;
> >>     }
> >>
> >>
> >> numArgs results to be a large number (using #printNum:   was 718613205),
> >> and if I try to do a #printOop:  I got a " 0x55aa55aa0x55aa55aa is not on
> >> the heap"
> >>
> >> Now, of course, the "if (!(GIV(argumentCount) == numArgs)) " fails.
> >>
> >> What it is funny is that I did a #printOop: of blockClosure instVar, and
> >> this is what I get:  0x1f59b578: a(n) Association
> >>
> >> Association? WTF ?   so...I guess something weird is happening. I continue
> >> debugging it..and I noticed that:
> >>
> >> - Several classes were called in addition to Association,like Character.
> >> These seems to be compact classes and implement #value and #value:   ;)
> >> - Both, #bytecodePrimValue and #bytecodePrimValueWithArg  were not calling
> >> #primitiveClosureValue. So, "isBlock" was false...
> >>
> >> I tried modifying #bytecodePrimValueWithArg and #bytecodePrimValue  this
> >> line:
> >>
> >> isBlock := objectMemory is: rcvr instanceOf: ClassBlockClosurecompactClassIndex: ClassBlockClosureCompactIndex.
> >>
> >> to this one:
> >>
> >> isBlock := objectMemory is: rcvr instanceOf: (self splObj:
> >> ClassBlockClosure) compactClassIndex: ClassBlockClosureCompactIndex.
> >>
> >> WIth this change, at least, the assert doesn't fail anymore. I made
> >> progress (maybe). But still, for normal closures, "isBlock" seems to be
> >> false. So.....all block values are being managed as a normal send ????
> >>
> >> Now, I don't understand why ClassBlockClosureCompactIndex  is 0. I think
> >> this may be the problem. Why it is not 12? (in Pharo  Smalltalk
> >> compactClassesArray at: 12 ->>> BlockClosure.)
> >> I notice that ObjectMemory >> initializeCompactClassIndices  does:
> >>
> >>     ClassBlockClosureCompactIndex := 0 "12". "Prospective.  Currently
> >> TranslatedMethod class"
> >>
> >>
> >> Notice that this ZERO changes the sematic of #is: oop instanceOf: classOop
> >> compactClassIndex: compactClassIndex
> >> called from the #bytecodePrimValue and bytecodePrimValueWithArg
> >>
> >> Finally, I changed the mentioned line to this:
> >>
> >> isBlock := objectMemory is: rcvr instanceOf: (self splObj:
> >> ClassBlockClosure) compactClassIndex: 12.
> >>
> >> and finally, with block closures, it is calling the primitive, and for
> >> Association it doesn't fail the assert.  ahh and the image doesn't crash ;)
> >>
> >> I don't know if this is a bug, if my solution was ok, nor it impact.
> >> Please let me know.
> >>
> >> Cheers
> >>
> >> Mariano
> >>
> >>
> >>
> >>
> >>
> >>
> >
> >

>
>>
>
> i added
> <var: #desiredHeapSize type: 'usqInt'>
> to oscog VMMaker (but only to CoInterpreter, not to StackInterpreter)
>
> Name: VMMaker-oscog-Igor.Stasenko.38
> Author: Igor.Stasenko
> Time: 6 December 2010, 2:46:52.417 pm
> UUID: 6cd6eeee-5691-40d3-9dca-46aaed01d49a
> Ancestors: VMMaker-Igor.Stasenko.37
>
> - added a type definition for readImageFromFile....
>
>
>> Dave
>>
>>>
>>
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.

>  
> >
>
> i added
> <var: #desiredHeapSize type: 'usqInt'>
> to oscog VMMaker (but only to CoInterpreter, not to StackInterpreter)
>
> Name: VMMaker-oscog-Igor.Stasenko.38
> Author: Igor.Stasenko
> Time: 6 December 2010, 2:46:52.417 pm
> UUID: 6cd6eeee-5691-40d3-9dca-46aaed01d49a
> Ancestors: VMMaker-Igor.Stasenko.37
>
> - added a type definition for readImageFromFile....
>
>
> > Dave
> >
> >>
> >
>
>
>
> --
> Best regards,
> Igor Stasenko AKA sig.