Re: [Pharo-dev] OSSubprocess 0.2.5 fails in Pharo 6.1 64 bits (macOS)

Hi Eliot,

On 21 February 2018 at 18:56, Eliot Miranda <[hidden email]> wrote:
>
> Hi Alistair,
>
> On Wed, Feb 21, 2018 at 9:31 AM, Alistair Grant <[hidden email]> wrote:
>>
>> Yep, OSSubprocess doesn't work on 64 bit VMs at the moment.
>>
>> Eliot, Mariano has indicated he's waiting on
>> https://github.com/pharo-project/pharo-vm/pull/142
>> Do you know of a reason why it shouldn't be merged?
>
>
> Well, the changes to FilePlugin.class belong in VMMaker.oscog.

Good point.  I'm embarrassed to say I didn't look at the code, just
the comments.


>  And cfileRecordSize doesn't make sense , since all pointers are of the same size in C:
>
> cfileRecordSize
> + "Return the size of a stdio FILE* handle"
> + <option: #PharoVM>
> + <static: false>
> + ^self sizeof: #'FILE*'
>
> I would simple use (self sizeof: #'void *') or (self sizeof: #'FILE *').

Yep - although this could use <inline: #always>.


> Other than that I can't see any problems.

I've sent Mariano a message asking him to clarify what he needs from
this PR.  I'll follow up again after he replies (I'm keen to see
OSSubprocess on the 64 bit VM).

Thanks!
Alistair

>>
>> Thanks,
>> Alistair
>>
>>
>>
>> On 21 February 2018 at 18:22, phideaux <[hidden email]> wrote:
>> > When trying the simple example (ls -la /Users) in Pharo 6.1 64bit you get a
>> > talkback because
>> > ExternalAddress integerAt:put:size:signed: fails using
>> > 'primitiveFFIIntegerAtPut' in module 'SqueakFFIPrims'
>> >
>> > Something to do with OSSubprocess not accommodating 64 bit pointers perhaps?
>> >
>> > Same code works fine on 32bit version.
>> >
>> > VM is CoInterpreter VMMaker.oscog-eem.2254 Jul 20 2017 for macOS downloaded
>> > from pharo.org/downloaded page
>> >
>> > Regards,
>> > Jay+
>> >
>> >
>> >
>> > --
>> > Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html
>> >
>>
>
>
>

> --
> _,,,^..^,,,_
> best, Eliot
>

Hmm, wait, I think these are two different issues :)

I managed to successfully use OSSubprocess in 64bits with some tweaks. The problem is that OSSubprocess does some manual type coertions like assumming 32 bits:

collectArgumentPointersInto: aPointer

    [...]

    aPointer nbUInt32AtOffset: (self argVArguments size - 1) * self systemAccessor sizeOfPointer put: 0

That should probably be replaced by:

    aPointer platformUnsignedLongAt: (self argVArguments size - 1) * self systemAccessor sizeOfPointer put: 0.

Now, those methods depend on UFFI, having UFFI is a must for OSSubprocess.

On Thu, Feb 22, 2018 at 7:07 PM, Alistair Grant <[hidden email]> wrote:

Hi Eliot,

On 21 February 2018 at 18:56, Eliot Miranda <[hidden email]> wrote:
>
> Hi Alistair,
>
> On Wed, Feb 21, 2018 at 9:31 AM, Alistair Grant <[hidden email]> wrote:
>>
>> Yep, OSSubprocess doesn't work on 64 bit VMs at the moment.
>>
>> Eliot, Mariano has indicated he's waiting on
>> https://github.com/pharo-project/pharo-vm/pull/142
>> Do you know of a reason why it shouldn't be merged?
>
>
> Well, the changes to FilePlugin.class belong in VMMaker.oscog.

Good point.  I'm embarrassed to say I didn't look at the code, just
the comments.


>  And cfileRecordSize doesn't make sense , since all pointers are of the same size in C:
>
> cfileRecordSize
> + "Return the size of a stdio FILE* handle"
> + <option: #PharoVM>
> + <static: false>
> + ^self sizeof: #'FILE*'
>
> I would simple use (self sizeof: #'void *') or (self sizeof: #'FILE *').

Yep - although this could use <inline: #always>.


> Other than that I can't see any problems.

I've sent Mariano a message asking him to clarify what he needs from
this PR.  I'll follow up again after he replies (I'm keen to see
OSSubprocess on the 64 bit VM).

Thanks!
Alistair

>>
>> Thanks,
>> Alistair
>>
>>
>>
>> On 21 February 2018 at 18:22, phideaux <[hidden email]> wrote:
>> > When trying the simple example (ls -la /Users) in Pharo 6.1 64bit you get a
>> > talkback because
>> > ExternalAddress integerAt:put:size:signed: fails using
>> > 'primitiveFFIIntegerAtPut' in module 'SqueakFFIPrims'
>> >
>> > Something to do with OSSubprocess not accommodating 64 bit pointers perhaps?
>> >
>> > Same code works fine on 32bit version.
>> >
>> > VM is CoInterpreter VMMaker.oscog-eem.2254 Jul 20 2017 for macOS downloaded
>> > from pharo.org/downloaded page
>> >
>> > Regards,
>> > Jay+
>> >
>> >
>> >
>> > --
>> > Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html
>> >
>>
>
>
>

> --
> _,,,^..^,,,_
> best, Eliot
>

--

Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr Web: http://guillep.github.io Phone: <a href="tel:+33%206%2052%2070%2066%2013" value="+33652706613" target="_blank">+33 06 52 70 66 13

I'll try making a PR now to fix that.

On Mon, Feb 26, 2018 at 10:19 AM, Guillermo Polito <[hidden email]> wrote:

Hmm, wait, I think these are two different issues :)

I managed to successfully use OSSubprocess in 64bits with some tweaks. The problem is that OSSubprocess does some manual type coertions like assumming 32 bits:

collectArgumentPointersInto: aPointer

    [...]

    aPointer nbUInt32AtOffset: (self argVArguments size - 1) * self systemAccessor sizeOfPointer put: 0

That should probably be replaced by:

    aPointer platformUnsignedLongAt: (self argVArguments size - 1) * self systemAccessor sizeOfPointer put: 0.

Now, those methods depend on UFFI, having UFFI is a must for OSSubprocess.

On Thu, Feb 22, 2018 at 7:07 PM, Alistair Grant <[hidden email]> wrote:

Hi Eliot,

On 21 February 2018 at 18:56, Eliot Miranda <[hidden email]> wrote:
>
> Hi Alistair,
>
> On Wed, Feb 21, 2018 at 9:31 AM, Alistair Grant <[hidden email]> wrote:
>>
>> Yep, OSSubprocess doesn't work on 64 bit VMs at the moment.
>>
>> Eliot, Mariano has indicated he's waiting on
>> https://github.com/pharo-project/pharo-vm/pull/142
>> Do you know of a reason why it shouldn't be merged?
>
>
> Well, the changes to FilePlugin.class belong in VMMaker.oscog.

Good point.  I'm embarrassed to say I didn't look at the code, just
the comments.


>  And cfileRecordSize doesn't make sense , since all pointers are of the same size in C:
>
> cfileRecordSize
> + "Return the size of a stdio FILE* handle"
> + <option: #PharoVM>
> + <static: false>
> + ^self sizeof: #'FILE*'
>
> I would simple use (self sizeof: #'void *') or (self sizeof: #'FILE *').

Yep - although this could use <inline: #always>.


> Other than that I can't see any problems.

I've sent Mariano a message asking him to clarify what he needs from
this PR.  I'll follow up again after he replies (I'm keen to see
OSSubprocess on the 64 bit VM).

Thanks!
Alistair

>>
>> Thanks,
>> Alistair
>>
>>
>>
>> On 21 February 2018 at 18:22, phideaux <[hidden email]> wrote:
>> > When trying the simple example (ls -la /Users) in Pharo 6.1 64bit you get a
>> > talkback because
>> > ExternalAddress integerAt:put:size:signed: fails using
>> > 'primitiveFFIIntegerAtPut' in module 'SqueakFFIPrims'
>> >
>> > Something to do with OSSubprocess not accommodating 64 bit pointers perhaps?
>> >
>> > Same code works fine on 32bit version.
>> >
>> > VM is CoInterpreter VMMaker.oscog-eem.2254 Jul 20 2017 for macOS downloaded
>> > from pharo.org/downloaded page
>> >
>> > Regards,
>> > Jay+
>> >
>> >
>> >
>> > --
>> > Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html
>> >
>>
>
>
>

> --
> _,,,^..^,,,_
> best, Eliot
>

--

Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr Web: http://guillep.github.io Phone: <a href="tel:+33%206%2052%2070%2066%2013" value="+33652706613" target="_blank">+33 06 52 70 66 13

--

Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr Web: http://guillep.github.io Phone: <a href="tel:+33%206%2052%2070%2066%2013" value="+33652706613" target="_blank">+33 06 52 70 66 13

 

Could you check this version?

https://github.com/marianopeck/OSSubprocess/pull/30

On Mon, Feb 26, 2018 at 10:19 AM, Guillermo Polito <[hidden email]> wrote:

I'll try making a PR now to fix that.

On Mon, Feb 26, 2018 at 10:19 AM, Guillermo Polito <[hidden email]> wrote:

Hmm, wait, I think these are two different issues :)

I managed to successfully use OSSubprocess in 64bits with some tweaks. The problem is that OSSubprocess does some manual type coertions like assumming 32 bits:

collectArgumentPointersInto: aPointer

    [...]

    aPointer nbUInt32AtOffset: (self argVArguments size - 1) * self systemAccessor sizeOfPointer put: 0

That should probably be replaced by:

    aPointer platformUnsignedLongAt: (self argVArguments size - 1) * self systemAccessor sizeOfPointer put: 0.

Now, those methods depend on UFFI, having UFFI is a must for OSSubprocess.

On Thu, Feb 22, 2018 at 7:07 PM, Alistair Grant <[hidden email]> wrote:

Hi Eliot,

On 21 February 2018 at 18:56, Eliot Miranda <[hidden email]> wrote:
>
> Hi Alistair,
>
> On Wed, Feb 21, 2018 at 9:31 AM, Alistair Grant <[hidden email]> wrote:
>>
>> Yep, OSSubprocess doesn't work on 64 bit VMs at the moment.
>>
>> Eliot, Mariano has indicated he's waiting on
>> https://github.com/pharo-project/pharo-vm/pull/142
>> Do you know of a reason why it shouldn't be merged?
>
>
> Well, the changes to FilePlugin.class belong in VMMaker.oscog.

Good point.  I'm embarrassed to say I didn't look at the code, just
the comments.


>  And cfileRecordSize doesn't make sense , since all pointers are of the same size in C:
>
> cfileRecordSize
> + "Return the size of a stdio FILE* handle"
> + <option: #PharoVM>
> + <static: false>
> + ^self sizeof: #'FILE*'
>
> I would simple use (self sizeof: #'void *') or (self sizeof: #'FILE *').

Yep - although this could use <inline: #always>.


> Other than that I can't see any problems.

I've sent Mariano a message asking him to clarify what he needs from
this PR.  I'll follow up again after he replies (I'm keen to see
OSSubprocess on the 64 bit VM).

Thanks!
Alistair

>>
>> Thanks,
>> Alistair
>>
>>
>>
>> On 21 February 2018 at 18:22, phideaux <[hidden email]> wrote:
>> > When trying the simple example (ls -la /Users) in Pharo 6.1 64bit you get a
>> > talkback because
>> > ExternalAddress integerAt:put:size:signed: fails using
>> > 'primitiveFFIIntegerAtPut' in module 'SqueakFFIPrims'
>> >
>> > Something to do with OSSubprocess not accommodating 64 bit pointers perhaps?
>> >
>> > Same code works fine on 32bit version.
>> >
>> > VM is CoInterpreter VMMaker.oscog-eem.2254 Jul 20 2017 for macOS downloaded
>> > from pharo.org/downloaded page
>> >
>> > Regards,
>> > Jay+
>> >
>> >
>> >
>> > --
>> > Sent from: http://forum.world.st/Pharo-Smalltalk-Developers-f1294837.html
>> >
>>
>
>
>

> --
> _,,,^..^,,,_
> best, Eliot
>

--

Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr Web: http://guillep.github.io Phone: <a href="tel:+33%206%2052%2070%2066%2013" value="+33652706613" target="_blank">+33 06 52 70 66 13

--

Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr Web: http://guillep.github.io Phone: <a href="tel:+33%206%2052%2070%2066%2013" value="+33652706613" target="_blank">+33 06 52 70 66 13

--

Guille Polito Research Engineer Centre de Recherche en Informatique, Signal et Automatique de Lille CRIStAL - UMR 9189 French National Center for Scientific Research - http://www.cnrs.fr Web: http://guillep.github.io Phone: <a href="tel:06%2052%2070%2066%2013" value="+33652706613" target="_blank">+33 06 52 70 66 13