[Vm-dev] Re: SmallInetger as methods was not ready yet....now maybe?

[Igor Stasenko]

On 3 December 2010 16:58, Mariano Martinez Peck <marianopeck at gmail.com> wrote:
>
>
>
> On Fri, Dec 3, 2010 at 3:41 PM, Mariano Martinez Peck <marianopeck at gmail.com> wrote:
>>
>> Hi folks. After fighting and fighting about crashes using SmallInteger as methods, I finally could debug the VM (thanks Esteban for the help in compiling and debugging!!) and I think I found a problem in theGC.
>> In the mark phase of the GC, it tries to mark all interpreter oops in the method:  markAndTraceInterpreterOops
>>
>> If you see that method....it does this (a part of it):
>>
>> compilerInitialized
>>         ifTrue: [self markAndTrace: receiver.
>>             self markAndTrace: method]
>>         ifFalse: [self markAndTrace: activeContext].
>>     self markAndTrace: messageSelector.
>>     self markAndTrace: newMethod.
>>
>> If you are using SmallInteger as methods.....newMethod can be a SmallIneteger, and not a method....so if we then see the method markAndTrace:
>>
>> the first lines are:
>>
>>     | header lastFieldOffset action statMarkCountLocal |
>>     header := self longAt: oop.
>>
>>
>> And of course, it crash in that #longAt:
>>
>> :)
>>
>> So, solutions:
>>
>> 1) Put an if in each place where it uses newMethod or method or newNativeMethod or suspendedMethods  or whatever
>> 2) Put an if in #markAndSweep.
>>
>> I think 2) is easier and it is just adding one line of code at the beginning:
>>
>>     | header lastFieldOffset action statMarkCountLocal |
>>     (self isIntegerObject: oop) ifTrue: [ ^ 0 ].
>>     header := self longAt: oop.
>> ....
>>
>> what do you think ?
>>
>>
>> Finally, I am afraid that there are more places where Interpreter uses any of those instVar that represent methods, and treat them as real objects.
>> So maybe there still pending future possible crashes?
>>
>
>
> For example, take a look to the methods:
>
> printUnbalancedStackFromNamedPrimitive
> primitiveMethod
> internalJustActivateNewMethod
> internalActivateNewMethod
> activateNewMethod
>
> And all the senders of
>
> literal: offset ofMethod: methodPointer
> literalCountOf: methodPointer
> primitiveIndexOf:
>
no, but these  methods usually should be fired only after you already
determined that given oop is a compiled method,
not a smallinteger or something else.

> maybe supporting SmallInteger as methods was not a good idea :(
>
it should work.

> cheers
>
> mariano
>
>
>>
>> Thanks
>>
>> Mariano
>
>
>



-- 
Best regards,
Igor Stasenko AKA sig.