[Vm-dev] 64-bit plugin support

[Nicolas Cellier]

2015-04-02 3:56 GMT+02:00 David T. Lewis <lewis at mail.msen.com>:

>
> On Wed, Apr 01, 2015 at 10:16:37PM +0200, Nicolas Cellier wrote:
> >
> > Hello,
> > I noticed a few #positive32BitValueOf: usage in plugins for the purpose
> of
> > retrieving a pointer stored in a #sqInt or #usqInt.
>
> That would definitely be wrong. Any plugin that stores machine pointers in
> sqInt or usqInt is broken.
>
> There are number of plugins that are still not updated for 64-bit VMs. I've
> tried to track some of the status of this on bugs.squeak.org although the
> information is not complete.
>
> Of the top of my head, I think that the SSL plugin, Camera plugin, anything
> dependent on SurfacePlugin, and probably a few others are broken. FFIPlugin
> was notably broken (with the fixes are on Mantis but not integrated),
> though
> that my no longer be relevant with newer FFI plugins.
>
> >
> > Yes, a #sqInt is large enough to hold a pointer, but shouldn't it be
> filled
> > with positiveMachineIntegerValueOf:?
> > At least this is necessary for spur64.
>
> A sqInt is *not* large enough to hold a pointer, specifically in the most
> common case of a 32-bit object memory running on a 64-bit interpreter VM.
>
>

>
> > Here are 3 patches attached.
> >
> > Nicolas
>
> Thanks! I know your patches are focused on Spur but I'll try running them
> also on
> the interpreter VM as soon as I am able. Note that
> positiveMachineIntegerValueOf:
> is a Spur idiom, although I would not expect that it would necessary to
> rely on that,
> since usually just declaring the variables correctly is sufficient. So for
> example in
>
> QuicktimePlugin>>primitiveSetGWorldPtrOntoSurface:width:height:rowBytes:depth:movie:
> there is a variable #buffer that looks like it needs to be a (char *), but
> is undeclared
> and takes the default #sqInt. Casting it to (char *) will always fail in
> the case of
> a 32-bit sqInt on a 64-bit host platform, regardless of which idiom you
> use in the
> cCoerce cast. It is better to declare both #buffer and #bitMapPtr
> explicitly, and
> get rid of the cast. That usually requires also fixing up the declarations
> in the
> support code and header files in ./Cross, but that's probably the only way
> to get
> it right.
>
> Dave
>
>
Let me reformulate.

My patch is for the case when we stored an external pointer (a handle or
something) into an Integer at image side (possibly a LargeInteger).

When the image is willing to pass the handle back to another primitive of
the plugin, the conversion from smalltalk Small/LargeInteger -> pointer has
to occur one way or another.

As I see it, something as positiveMachineIntegerValueOf: that would use
either 32 or 64 bits variant depending on wordSize is the way to go.

What you say, is that the variable that holds the pointer should not be
declared #sqInt, because even if it works with Spur64, it's not large
enough in classical Interpreter.

The classical Interpreter version of positiveMachineIntegerValueOf: could
answer an unsigned long for example (asserting that
(sizeof(long)>=wordSize)), so we can continue using that message for the
compatibility layer (having single plugin variant for several VM brands).

But you are right, the plugin itself should not use a #sqInt declaration
for holding the handle/pointer - even if it works with Spur64, I'll try to
make another pass with that in mind.

Nicolas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20150402/1b566491/attachment.htm