[squeak-dev] Advice from Plugin gurus

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[squeak-dev] Advice from Plugin gurus

Gerardo Richarte
Hi all, I'm working on a plugin (for SqueakNOS), and I have a primitive
for which I had coded a first version, and I wanted to hear comments
from people with deeper Squeak VM and plugin knowledge. I think it's
ugly, but I'm more worried on whether it's dangerous or not.

some context:

    There's a native function I need to call (I called it 'aPrintf' in
the code below) with a specific calling convention:

    It takes only one argument: an array
    the first entry in the array is apointer to a C string
    the other elements are determined by the first element.

    What I want is to provide a generic interface to this aPrintf.

    The code below will walk the array, converting every element in the
array to either an integer (integerValueOf) or a direct pointer
(firstIndexableField).

And I want to be able to read the results left in the array.

I understand that I won't be able to read non-integer elements from the
array after coming back from this primitive, but that's fine. I plan to
keep a copy of all non-integer elements in Smalltalk variables,
including strings and other buffers.

What I'd like to know is what experts think of this... like, will it
break anything, like the garbage collector? is there any other simple
way of doing this?
 
The primitive is:
primitiveaPrintf: anArray
    | answer length element |
    self primitive: 'primitiveOFWCallout' parameters: #(Array).

    length := interpreterProxy slotSizeOf: anArray cPtrAsOop.

    answer := 0.

    "convert everything to native format"
    0 to: length-1 do: [:i |
        element := anArray at: i.
        (interpreterProxy isIntegerObject: element)
            ifTrue: [anArray at: i put: (interpreterProxy
integerValueOf: element)]
            ifFalse: [anArray at: i put: (interpreterProxy
firstIndexableField: element)]
    ].

    self cCode: 'printf(anArray)'.

    "convert everything to integers. oops will be lost, but that's fine.
The client should mantain a reference"
    0 to: length-1 do: [:i |
        element := anArray at: i.
        anArray at: i put: (interpreterProxy positive32BitIntegerFor:
element)
    ].

    answer = 0
        ifTrue: [^ self]
        ifFalse: [interpreterProxy primitiveFail].


    thanks a lot!
    richie

Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Advice from Plugin gurus

Igor Stasenko
2008/9/22 Gerardo Richarte <[hidden email]>:

> Hi all, I'm working on a plugin (for SqueakNOS), and I have a primitive
> for which I had coded a first version, and I wanted to hear comments
> from people with deeper Squeak VM and plugin knowledge. I think it's
> ugly, but I'm more worried on whether it's dangerous or not.
>
> some context:
>
>    There's a native function I need to call (I called it 'aPrintf' in
> the code below) with a specific calling convention:
>
>    It takes only one argument: an array
>    the first entry in the array is apointer to a C string
>    the other elements are determined by the first element.
>
>    What I want is to provide a generic interface to this aPrintf.
>
>    The code below will walk the array, converting every element in the
> array to either an integer (integerValueOf) or a direct pointer
> (firstIndexableField).
>
> And I want to be able to read the results left in the array.
>
> I understand that I won't be able to read non-integer elements from the
> array after coming back from this primitive, but that's fine. I plan to
> keep a copy of all non-integer elements in Smalltalk variables,
> including strings and other buffers.
>
> What I'd like to know is what experts think of this... like, will it
> break anything, like the garbage collector? is there any other simple
> way of doing this?
>
> The primitive is:
> primitiveaPrintf: anArray
>    | answer length element |
>    self primitive: 'primitiveOFWCallout' parameters: #(Array).
>
>    length := interpreterProxy slotSizeOf: anArray cPtrAsOop.
>
>    answer := 0.
>
>    "convert everything to native format"
>    0 to: length-1 do: [:i |
>        element := anArray at: i.
>        (interpreterProxy isIntegerObject: element)
>            ifTrue: [anArray at: i put: (interpreterProxy
> integerValueOf: element)]
>            ifFalse: [anArray at: i put: (interpreterProxy
> firstIndexableField: element)]
>    ].
>
>    self cCode: 'printf(anArray)'.
>
>    "convert everything to integers. oops will be lost, but that's fine.
> The client should mantain a reference"
>    0 to: length-1 do: [:i |
>        element := anArray at: i.
>        anArray at: i put: (interpreterProxy positive32BitIntegerFor:
> element)
>    ].
>
>    answer = 0
>        ifTrue: [^ self]
>        ifFalse: [interpreterProxy primitiveFail].
>
>
>    thanks a lot!
>    richie
>
>

Hello Gera!

I wondering , why you prefer using string formatting in C, while in
smalltalk you have own? You can easily format strings in smalltalk,
and print only the resulting string w/o need in inventing safe schemes
or conversion.

Here is my primitive, which simply writes byte array to stdout:

writeToStdout
        "Primitive.
        Writes given bytearray to stdout"

        | bufoop bufptr bufsize |
        self var: 'bufptr' type: 'void *'.
        self export: true.

        bufoop := self stackValue: 0.
        (self isBytes: bufoop)
                ifFalse: [^ self primitiveFail].
        bufptr := self firstIndexableField: bufoop.
        bufsize := self byteSizeOf: bufoop.

        self cPerform: (self cIdentifier: #fwrite) with: bufptr with: bufsize
with: 1 with: (self cIdentifier: #stdout).
        self pop:1

---
note, #cPerform: , #cIdentifier is existing only in HydraVM, but it
can be replaced with cCode:  'fwrite(..blabla..)'

I'm using this primitive for debugging, and eventually it will be removed.


--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Advice from Plugin gurus

Gerardo Richarte
Igor Stasenko wrote:

> 2008/9/22 Gerardo Richarte <[hidden email]>:
>  
>>    There's a native function I need to call (I called it 'aPrintf' in
>> the code below) with a specific calling convention:
>>    
> I wondering , why you prefer using string formatting in C, while in
> smalltalk you have own? You can easily format strings in smalltalk,
> and print only the resulting string w/o need in inventing safe schemes
> or conversion.
>  
Hi Igor, nice to see you again!

I knew this was going to happen :)

It was just an example, I'm not using string formatting. The actual
function I'm calling is the callout interface for OpenFirmware, through
which OpenFirmware provides services to the booted operating
system. Pretty much like BIOS' INT mechanism, but much more
powerful.

   richie

Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Advice from Plugin gurus

Igor Stasenko
2008/9/22 Gerardo Richarte <[hidden email]>:

> Igor Stasenko wrote:
>> 2008/9/22 Gerardo Richarte <[hidden email]>:
>>
>>>    There's a native function I need to call (I called it 'aPrintf' in
>>> the code below) with a specific calling convention:
>>>
>> I wondering , why you prefer using string formatting in C, while in
>> smalltalk you have own? You can easily format strings in smalltalk,
>> and print only the resulting string w/o need in inventing safe schemes
>> or conversion.
>>
> Hi Igor, nice to see you again!
>
> I knew this was going to happen :)
>
> It was just an example, I'm not using string formatting. The actual
> function I'm calling is the callout interface for OpenFirmware, through
> which OpenFirmware provides services to the booted operating
> system. Pretty much like BIOS' INT mechanism, but much more
> powerful.
>

Okay. Then i don't see any problems with this primitive, except maybe one thing:

a primitive should never change the contents of any argument(s). As in
general smalltalk methods, a primitive should be allowed to change the
contents only of receiver object, otherwise its breaking a smalltalk
encapsulation rules.
I would advice you to allocate separate byte array and fill it with
values, then pass to C routine. Since its a byte array you don't need
to care about it contents, and don't need a cleanup (GC will collect
it as garbage eventually).

>   richie
>
>



--
Best regards,
Igor Stasenko AKA sig.

Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Advice from Plugin gurus

Eliot Miranda-2


On Mon, Sep 22, 2008 at 6:19 AM, Igor Stasenko <[hidden email]> wrote:
2008/9/22 Gerardo Richarte <[hidden email]>:
> Igor Stasenko wrote:
>> 2008/9/22 Gerardo Richarte <[hidden email]>:
>>
>>>    There's a native function I need to call (I called it 'aPrintf' in
>>> the code below) with a specific calling convention:
>>>
>> I wondering , why you prefer using string formatting in C, while in
>> smalltalk you have own? You can easily format strings in smalltalk,
>> and print only the resulting string w/o need in inventing safe schemes
>> or conversion.
>>
> Hi Igor, nice to see you again!
>
> I knew this was going to happen :)
>
> It was just an example, I'm not using string formatting. The actual
> function I'm calling is the callout interface for OpenFirmware, through
> which OpenFirmware provides services to the booted operating
> system. Pretty much like BIOS' INT mechanism, but much more
> powerful.
>

Okay. Then i don't see any problems with this primitive, except maybe one thing:

a primitive should never change the contents of any argument(s). As in
general smalltalk methods, a primitive should be allowed to change the
contents only of receiver object, otherwise its breaking a smalltalk
encapsulation rules.
I would advice you to allocate separate byte array and fill it with
values, then pass to C routine. Since its a byte array you don't need
to care about it contents, and don't need a cleanup (GC will collect
it as garbage eventually).

No need to use a Smalltalk object to hold the copy.  Just use alloca (stack allocating version of malloc).

But this is the kind of thing an FFI does during marshalling anyway.  Gerardo, why can't you use the FFI?


Reply | Threaded
Open this post in threaded view
|

[squeak-dev] Re: Advice from Plugin gurus

Nicolas Cellier-3
Eliot Miranda a écrit :

>
>
> On Mon, Sep 22, 2008 at 6:19 AM, Igor Stasenko <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     2008/9/22 Gerardo Richarte <[hidden email] <mailto:[hidden email]>>:
>      > Igor Stasenko wrote:
>      >> 2008/9/22 Gerardo Richarte <[hidden email]
>     <mailto:[hidden email]>>:
>      >>
>      >>>    There's a native function I need to call (I called it
>     'aPrintf' in
>      >>> the code below) with a specific calling convention:
>      >>>
>      >> I wondering , why you prefer using string formatting in C, while in
>      >> smalltalk you have own? You can easily format strings in smalltalk,
>      >> and print only the resulting string w/o need in inventing safe
>     schemes
>      >> or conversion.
>      >>
>      > Hi Igor, nice to see you again!
>      >
>      > I knew this was going to happen :)
>      >
>      > It was just an example, I'm not using string formatting. The actual
>      > function I'm calling is the callout interface for OpenFirmware,
>     through
>      > which OpenFirmware provides services to the booted operating
>      > system. Pretty much like BIOS' INT mechanism, but much more
>      > powerful.
>      >
>
>     Okay. Then i don't see any problems with this primitive, except
>     maybe one thing:
>
>     a primitive should never change the contents of any argument(s). As in
>     general smalltalk methods, a primitive should be allowed to change the
>     contents only of receiver object, otherwise its breaking a smalltalk
>     encapsulation rules.
>     I would advice you to allocate separate byte array and fill it with
>     values, then pass to C routine. Since its a byte array you don't need
>     to care about it contents, and don't need a cleanup (GC will collect
>     it as garbage eventually).
>
>
> No need to use a Smalltalk object to hold the copy.  Just use alloca
> (stack allocating version of malloc).
>
> But this is the kind of thing an FFI does during marshalling anyway.
>  Gerardo, why can't you use the FFI?
>

He said arguments type was not fixed, but would depend on service
requested in first argument as I understand it.

So the same function has different interfaces.
Would you suggest the solution is as simple as declaring the same
function with several interfaces?

like:

withFormat: aString printInteger: anInteger
        <cdecl: void 'myprintf'( char * long )>
        ^self externalCallFailed

withFormat: aString printDouble: aDouble
        <cdecl: void 'myprintf'( char * double )>
        ^self externalCallFailed

printInteger: anInteger
        ^self withFormat: '%d' print: anInteger

printDouble: aDouble
        ^self withFormat: '%f' print: anInteger

Nicolas


Reply | Threaded
Open this post in threaded view
|

[squeak-dev] Re: Advice from Plugin gurus

Andreas.Raab
In reply to this post by Gerardo Richarte
 > What I'd like to know is what experts think of this... like, will it
 > break anything, like the garbage collector?

Yes, it will. The reason is here:

   anArray at: i put: (interpreterProxy positive32BitIntegerFor: element)

This can cause an allocation and things go downhill from there. You
can't just store "wild values" in the array - if it gets processed by
the GC it will crash (GC trying to trace the references). Even if you
fix that, you'll also have to remap the array to protect from being
moved by the allocation (pushRemappableOop/popRemappableOop). Finally
you'll have to mark the array as root if it's old (i.e., using
storePointer:ofObject:withValue:).

As you can see, doing this kind of munging with existing structures is a
pain. I would recommend you do what Eliot suggested - alloca() a copy of
the array and operate on that.

Cheers,
   - Andreas

Gerardo Richarte wrote:

> Hi all, I'm working on a plugin (for SqueakNOS), and I have a primitive
> for which I had coded a first version, and I wanted to hear comments
> from people with deeper Squeak VM and plugin knowledge. I think it's
> ugly, but I'm more worried on whether it's dangerous or not.
>
> some context:
>
>     There's a native function I need to call (I called it 'aPrintf' in
> the code below) with a specific calling convention:
>
>     It takes only one argument: an array
>     the first entry in the array is apointer to a C string
>     the other elements are determined by the first element.
>
>     What I want is to provide a generic interface to this aPrintf.
>
>     The code below will walk the array, converting every element in the
> array to either an integer (integerValueOf) or a direct pointer
> (firstIndexableField).
>
> And I want to be able to read the results left in the array.
>
> I understand that I won't be able to read non-integer elements from the
> array after coming back from this primitive, but that's fine. I plan to
> keep a copy of all non-integer elements in Smalltalk variables,
> including strings and other buffers.
>
> What I'd like to know is what experts think of this... like, will it
> break anything, like the garbage collector? is there any other simple
> way of doing this?
>  
> The primitive is:
> primitiveaPrintf: anArray
>     | answer length element |
>     self primitive: 'primitiveOFWCallout' parameters: #(Array).
>
>     length := interpreterProxy slotSizeOf: anArray cPtrAsOop.
>
>     answer := 0.
>
>     "convert everything to native format"
>     0 to: length-1 do: [:i |
>         element := anArray at: i.
>         (interpreterProxy isIntegerObject: element)
>             ifTrue: [anArray at: i put: (interpreterProxy
> integerValueOf: element)]
>             ifFalse: [anArray at: i put: (interpreterProxy
> firstIndexableField: element)]
>     ].
>
>     self cCode: 'printf(anArray)'.
>
>     "convert everything to integers. oops will be lost, but that's fine.
> The client should mantain a reference"
>     0 to: length-1 do: [:i |
>         element := anArray at: i.
>         anArray at: i put: (interpreterProxy positive32BitIntegerFor:
> element)
>     ].
>
>     answer = 0
>         ifTrue: [^ self]
>         ifFalse: [interpreterProxy primitiveFail].
>
>
>     thanks a lot!
>     richie
>
>


Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Re: Advice from Plugin gurus

Gerardo Richarte
In reply to this post by Nicolas Cellier-3
Thanks Eliot, Adreas, Igor and Nicolas!

this is what I didn't want to accept, and your answers where very clear
(specially Andreas).

nicolas cellier wrote:
> He said arguments type was not fixed, but would depend on service
requested
> in first argument as I understand it.

right, that's the issue.

> So the same function has different interfaces.
> Would you suggest the solution is as simple as declaring the same function
> with several interfaces?

I think I'll try both things. I liked a lot Nicolas interpretation of
Eliot's suggestion. It will definitely work (calling the same function
from different places with different prototypes).

Also the alloca() is very easy, and straightforward to convert my code
to it, I'll
give it a try first.

    again, thanks a lot!
    richie



Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Advice from Plugin gurus

David T. Lewis
In reply to this post by Gerardo Richarte
On Sun, Sep 21, 2008 at 09:47:40PM -0300, Gerardo Richarte wrote:

>         (interpreterProxy isIntegerObject: element)
>             ifTrue: [anArray at: i put: (interpreterProxy integerValueOf: element)]
>             ifFalse: [anArray at: i put: (interpreterProxy firstIndexableField: element)]
>     ].

Watch out for storing addresses into integers. The #firstIndexableField: is a pointer.
If you are using Squeak 32-bit words (any normal image), you don't want to stuff
addresses into the slots unless you are certain that nobody will ever want to run
your plugin on a 64-bit pointer machine.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Re: Advice from Plugin gurus

Gerardo Richarte
In reply to this post by Andreas.Raab
Andreas Raab wrote:
> As you can see, doing this kind of munging with existing structures is
> a pain. I would recommend you do what Eliot suggested - alloca() a
> copy of the array and operate on that.
I had not noticed something: I need also to get the results, as the
answers (may be many) from this function are stored in the same array
(where space is already allocated).

When doing the alloca I will need to copy things from the tempArray to
anArray, or in the worst case, create a new array where I store the
answers, but I guess that then, if I create a new array, we are back to
the original place, where I have a Smalltalk array and I want to store
native things on it (after convertion)... So I think I'll have to use
something like an ExternalBuffer all the way from Smalltalk and back?

    thanks,
    richie

Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Re: Advice from Plugin gurus

Bert Freudenberg
In reply to this post by Gerardo Richarte

Am 22.09.2008 um 11:36 schrieb Gerardo Richarte:

> nicolas cellier wrote:
>> He said arguments type was not fixed, but would depend on service
> requested
>> in first argument as I understand it.
>
> right, that's the issue.


If that is indeed the issue then just construct the FFI call on the  
fly. The <cdecl> declaration is just syntactic sugar. You can as well  
create an FFI function object, add parameters, and call it.

Others: the actual function Gera tries to wrap is the OpenFirmware  
client interface:

http://www.firmworks.com/www/clntintf.htm

for some ia32 details see

http://kerneltrap.org/mailarchive/linux-kernel/2008/4/21/1522284

Example code calling back into OFW is here:

http://dev.laptop.org/git?p=olpc-2.6;a=blob;f=arch/x86/kernel/ofw.c

So this is the primitive we want to have ...

- Bert -



Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Re: Advice from Plugin gurus

Gerardo Richarte

> Others: the actual function Gera tries to wrap is the OpenFirmware
> client interface:
>
> http://www.firmworks.com/www/clntintf.htm
    yeah, I should have said it to start with,
    thanks for clarifying and helping. I think that, for anything but
performance, creating the function on the fly may not be a bad idea at
all! I give it a try too. Will it also work for return values in arguments?

oh, nono... I just realized that every function takes a single argument:
the array... I don't think the FFI interface will suit our needs, oh
well... I better stop rambling and try somthing.

    richie/gera

PD: yeah, I have a multiple personalities problem :)






Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Re: Advice from Plugin gurus

Igor Stasenko
2008/9/23 Gerardo Richarte <[hidden email]>:

>
>> Others: the actual function Gera tries to wrap is the OpenFirmware
>> client interface:
>>
>> http://www.firmworks.com/www/clntintf.htm
>    yeah, I should have said it to start with,
>    thanks for clarifying and helping. I think that, for anything but
> performance, creating the function on the fly may not be a bad idea at
> all! I give it a try too. Will it also work for return values in arguments?
>
> oh, nono... I just realized that every function takes a single argument:
> the array... I don't think the FFI interface will suit our needs, oh
> well... I better stop rambling and try somthing.
>

I think you will figure out what callout form is most useful for you
when you start using it.
Sometimes it not worth putting too much flexibility in things :)

>    richie/gera
>
> PD: yeah, I have a multiple personalities problem :)
>



--
Best regards,
Igor Stasenko AKA sig.