On Wed, Dec 22, 2010 at 10:34 PM, Eliot Miranda eliot.miranda@gmail.comwrote:
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 < marianopeck@gmail.com> 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
On Thu, Dec 23, 2010 at 01:52:15PM +0100, Mariano Martinez Peck wrote:
On Wed, Dec 22, 2010 at 10:34 PM, Eliot Miranda eliot.miranda@gmail.comwrote:
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.
The #readImageFromFile:HeapSize:StartingAt: patch is correct. John originally provided the fix, and it should go into oscog also. But I would expect this to cause problems only when compiling for 64 bit pointers, so it should not really affect Cog right now (?). From the MC change history:
Name: VMMaker-dtl.165 Author: dtl Time: 28 March 2010, 6:54:54 pm UUID: e7d7d22f-035f-46f2-ad5b-cda5b704a5aa Ancestors: VMMaker-dtl.164
Fix declaration of readImageFromFile:HeapSize:StartingAt: as per John's note at http://lists.squeakfoundation.org/pipermail/vm-dev/2010-January/003614.html
Dave
- 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 < marianopeck@gmail.com> 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
yep... I added that too
El 23/12/2010, a las 12:02p.m., Igor Stasenko escribió:
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.
Yes, I am just confirming that your change in CoInterpreter is correct and consistent with the one for Interpreter in VMMaker trunk. So this change should also be applied to Interpreter and StackInterpreter in oscog. But note this is not a concern for the upcoming VM builds, because the 32 bit Cog version is OK.
Thanks, Dave
On Thu, Dec 23, 2010 at 04:02:12PM +0100, Igor Stasenko wrote:
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.
vm-dev@lists.squeakfoundation.org