Hi Denis,

On Fri, Feb 26, 2016 at 6:15 AM, Denis Kudriashov <dionisiydk@gmail.com> wrote:
 
Hi.

In Pharo we have special logic to debug failure code of primitives. When debugger detects named prim it tries to create same prim method but without failure code. Then if it fails debugger step into primitive failure code.

There is a better way of doing this which I will show below, but first...
 

To create such cleaned primitive method Pharo uses template method like this:

tryNamedPrimitiveTemplate
"This method is a template that the Smalltalk simulator uses to 
execute primitives. See Object documentation whatIsAPrimitive."
<primitive:' module:' error: errorCode>
^ Context primitiveFailTokenFor: errorCode

Then it changes arguments count to be like given method:

theMethod := self class tryNamedPrimitiveTemplateMethod.
theMethod prepareForSimulationWith: arguments size.
self setNamedPrimitiveInformationFrom: aCompiledMethod toMethod: theMethod.

CompiledMethod>>prepareForSimulationWith: numArgs
"This method changes the argument count of a CompiledMethod header to numArgs, its temporary count to numArgs + 1 and change the code handling primitive error to store the error code in the unique temporary of the method"
| newHeader xpc |
newHeader := (((self header bitAnd: 2r01110000000000111111111111111111)
bitOr: (numArgs bitShift: 24))
bitOr: (numArgs + 1 bitShift: 18)).
newHeader := newHeader + (self class headerFlagForEncoder: self encoderClass).
self objectAt: 1 put: newHeader.
xpc := self initialPC.
"long store temp"
(self at: xpc) = 129
ifTrue: [
self at: xpc + 1 put: (16r40 + numArgs).
self at: xpc + 3 put: (16r10 + numArgs)]


Then method executed by:

theMethod flushCache
theMethod valueWithReceiver: aReceiver arguments: arguments

This approach works correctly on Cog and not working in Spur.
In Spur result of theMethod execution become wrong when given method has arguments.
Which means that changing method arguments of CompiledMethod is incorrect for Spur.

Yes; in Spur CompiledMethod's header format has changed.  It looks like this (from Squeak 5's CompiledMethod class comment)

The header is a SmallInteger (which in the 32-bit system has 31 bits, and in the 64-bit system, 61 bits) in the following format:

(index 0) 15 bits: number of literals (#numLiterals)
(index 15)  1 bit: is optimized - reserved for methods that have been optimized by Sista
(index 16)  1 bit: has primitive
(index 17)  1 bit: whether a large frame size is needed (#frameSize => either SmallFrame or LargeFrame)
(index 18)  6 bits: number of temporary variables (#numTemps)
(index 24)  4 bits: number of arguments to the method (#numArgs)
(index 28)  2 bits: reserved for an access modifier (00-unused, 01-private, 10-protected, 11-public), although accessors for bit 29 exist (see #flag).
sign bit:  1 bit: selects the instruction set, >= 0 Primary, < 0 Secondary (#signFlag)

If the method has a primitive then the first bytecode of the method must be a callPrimitive: bytecode that encodes the primitive index.

So the code to change the number of arguments is fine.  What's wrong is the code to store the error code.  That comes /after/ the three byte call primitive bytecode at the start of the method.  So it should read

xpc := self initialPC + 3.
"long store temp"
(self at: xpc) = 129 ifTrue:
[self at: xpc + 1 put: (16r40 + numArgs).
self at: xpc + 3 put: (16r10 + numArgs)]

Is there any solution to make it working?

The above should work.  But there's a much easier way.  Cog provides a call named primitive primitive.  Here's the Pharo version, which should be in the Pharo image as it was included in the Spur bootstrap (but I've attached anyway):

Context methods for private
tryNamedPrimitiveIn: aCompiledMethod for: aReceiver withArgs: arguments
"Invoke the named primitive for aCompiledMethod, answering its result, or,
if the primiitve fails, answering the error code."
<primitive: 218 error: ec>
ec ifNotNil:
["If ec is an integer other than -1 there was a problem with primitive 218,
 not with the external primitive itself.  -1 indicates a generic failure (where
 ec should be nil) but ec = nil means primitive 218 is not implemented.  So
 interpret -1 to mean the external primitive failed with a nil error code."
ec isInteger ifTrue:
[ec = -1
ifTrue: [ec := nil]
ifFalse: [self primitiveFailed]]].
^self class primitiveFailTokenFor: ec

this is then called from Context>>#doPrimitive:method:receiver:args: as 

primitiveIndex = 118 ifTrue: "tryPrimitive:withArgs:; avoid recursing in the VM"
[(arguments size = 2
and: [arguments first isInteger
and: [(self objectClass: arguments last) == Array]]) ifFalse:
[^ContextPart primitiveFailTokenFor: nil].
^self doPrimitive: arguments first method: meth receiver: receiver args: arguments last].

value := primitiveIndex = 120 "FFI method"
ifTrue: [(meth literalAt: 1) tryInvokeWithArguments: arguments]
ifFalse:
[primitiveIndex = 117 "named primitives"
ifTrue: [self tryNamedPrimitiveIn: meth for: receiver withArgs: arguments]
ifFalse: [receiver tryPrimitive: primitiveIndex withArgs: arguments]].

Then there's no need to edit a method just to invoke a named primitive in the debugger, and you can delete the tryNamedPrimitiveTemplateMethod.

HTH
_,,,^..^,,,_
best, Eliot