VM Maker: VMMaker.oscog- nice.2366.mcz

 
Nicolas Cellier uploaded a new version of VMMaker to project VM Maker:
http://source.squeak.org/VMMaker/VMMaker.oscog- nice.2366.mcz

==================== Summary ====================

Name: VMMaker.oscog- nice.2366
Author:  nice
Time: 17 April 2018, 3:37:48.684079 pm
UUID: 6d9bd2fa-c381-5d4b-b31e-c6d21225c848
Ancestors: VMMaker.oscog-VB.2365

Correct a 32bit-hardcoded pointer size in FFI
Correct two copy/paste typos in num32BitUnitsOf:

Note: I don't like the FFI code that I just corrected. IMO, it does the wrong thing.

if I have an argument spec is
    MyLib>>foo: aFoo
        <cdecl: void foo(Foo *)>
where Foo is some ExternalStructure subclass (Foo class>>fields ^#((x 'ushort') (y 'ushort')))

and that I try to pass (MyLib new foo: Foo new), it seems to me that the Foo new getHandle will be (ByteArray new: 4).
What I understand form the code that I just corrected is that we are trying to pass the contents of the ByteArray re-interpreted as a void pointer. Scary and wrong...

If I instead pass (MyLib new foo: Foo externalNew), it seems that we don't even bother to check if the (argSpec anyMask: FFIFlagPointer) and just force passing the structure by value (thru a memcpy on stack). Scary and wrong...

In general, every one use <cdecl: void foo(void *)> to work around this ill-behavior, and thus bypass type checks...

Also note that we can't even pass an ExternalData (think an Array of Foo), because ffiArgument:Spec:Class:in: insists on having actualArg class inheritsFrom: argType referentClass. ExternalData does not inherit from Foo, event if its type matches (ExternalType structTypeNamed: #Foo). That's crazy...
Another reason while people use <cdecl: void foo(void *)>

It's high time to consider a rewrite IMO.

=============== Diff against VMMaker.oscog-VB.2365 ===============

Item was changed:
  ----- Method: ObjectMemory>>num32BitUnitsOf: (in category 'object access') -----
  num32BitUnitsOf: objOop
+ "Answer the number of 32-bit units in the given non-immediate object.
- "Answer the number of 16-bit units in the given non-immediate object.
  N..B. Rounds down 8-bit units, so a 7 byte object has 1 32-bit unit.
  Does not adjust the size of contexts by stackPointer."
  ^(self numBytesOf: objOop) >> 2!

Item was changed:
  ----- Method: SpurMemoryManager>>num32BitUnitsOf: (in category 'object access') -----
  num32BitUnitsOf: objOop
+ "Answer the number of 32-bit units in the given non-immediate object.
- "Answer the number of 16-bit units in the given non-immediate object.
  N..B. Rounds down 8-bit units, so a 7 byte object has 1 32-bit unit.
  Does not adjust the size of contexts by stackPointer."
  ^(self numBytesOf: objOop) >> 2!

Item was changed:
  ----- Method: ThreadedFFIPlugin>>ffiPushStructureContentsOf:in: (in category 'callout support') -----
  ffiPushStructureContentsOf: oop in: calloutState
  <var: #calloutState type: #'CalloutState *'>
  "Push the contents of the given external structure"
  | ptrClass ptrAddress |
  <inline: true>
  ptrClass := interpreterProxy fetchClassOf: oop.
  ptrClass = interpreterProxy classExternalAddress ifTrue: "ExternalAddress is bytes"
  [ptrAddress := (interpreterProxy fetchPointer: 0 ofObject: oop) asVoidPointer.
  "There is no way we can make sure the structure is valid.
  But we can at least check for attempts to pass pointers to ST memory."
  (interpreterProxy isInMemory: ptrAddress) ifTrue:
  [^FFIErrorInvalidPointer].
  ^self ffiPushStructure: ptrAddress
  ofSize: (calloutState ffiArgHeader bitAnd: FFIStructSizeMask)
  typeSpec: calloutState ffiArgSpec
  ofLength: calloutState ffiArgSpecSize
  in: calloutState].
  ptrClass = interpreterProxy classByteArray ifTrue:
  ["The following is a somewhat pessimistic test but I like being sure..."
  (interpreterProxy byteSizeOf: oop) = (calloutState ffiArgHeader bitAnd: FFIStructSizeMask)
  ifFalse:[^FFIErrorStructSize].
  ptrAddress := interpreterProxy firstIndexableField: oop.
  (calloutState ffiArgHeader anyMask: FFIFlagPointer) ifFalse:
  "Since this involves passing the address of the first indexable field we need to fail
   the call if it is threaded and the object is young, since it may move during the call."
  [self cppIf: COGMTVM ifTrue:
  [((calloutState callFlags anyMask: FFICallFlagThreaded)
  and: [interpreterProxy isYoung: oop]) ifTrue:
  [^PrimErrObjectMayMove negated]].
  ^self ffiPushStructure: ptrAddress
  ofSize: (calloutState ffiArgHeader bitAnd: FFIStructSizeMask)
  typeSpec: calloutState ffiArgSpec
  ofLength: calloutState ffiArgSpecSize
  in: calloutState].
  "If FFIFlagPointer + FFIFlagStructure is set use ffiPushPointer on the contents"
+ (calloutState ffiArgHeader bitAnd: FFIStructSizeMask) = BytesPerWord ifFalse:
- (calloutState ffiArgHeader bitAnd: FFIStructSizeMask) = 4 ifFalse:
  [^FFIErrorStructSize].
  ptrAddress := (interpreterProxy fetchPointer: 0 ofObject: oop) asVoidPointer.
  (interpreterProxy isInMemory: ptrAddress) ifTrue:
  [^FFIErrorInvalidPointer].
  ^self ffiPushPointer: ptrAddress in: calloutState].
  ^FFIErrorBadArg!