latest vmmaker wont run with pharo 1.1.1 (dev)

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

Re: latest vmmaker wont run with pharo 1.1.1 (dev)

David T. Lewis
 
On Mon, Nov 08, 2010 at 06:41:57PM -0800, Andreas Raab wrote:

>  
> On 11/8/2010 5:08 PM, Levente Uzonyi wrote:
> >The problem is that the method expects ByteStrings as arguments, but it
> >won't fail if one of them is a WideString. Instead it returns wrong
> >results. Here's an example (in Squeak):
> >
> >ByteString new
> >findSubstring: 'b'
> >in: 'abc' asWideString
> >startingAt: 1
> >matchTable: ((0 to: 255) as: ByteArray). "===> 0"
> >
> >The result is 0, while it should be 2. The problem is worked around in
> >Squeak by making sure that the primitive is not used in this case.
> >
> >'abc' asWideString
> >findString: 'b'
> >startingAt: 1
> >caseSensitive: true. "===> 2"
> >
> >Though a naive user can still use the method as shown here.
>
> Thanks. Looks like a problem in extracting string arguments in
> translated primitives. With Squeaksource being down again (should we
> move VMMaker to source.squeak.org?) I've attached a change set with a
> fix. This adds an additional check for char* arguments in translated
> prims to ensure that they're actually byte objects.

Andreas,

Your fix does indeed resolve the underlying problem. I added it
to VMMaker (with a note to Eliot to do likewise with oscog), and
also added unit tests based on Levente's example. This should
allow the various WideString workarounds to be removed once the
updated VMs are in circulation.

Thanks,
Dave

Reply | Threaded
Open this post in threaded view
|

Re: latest vmmaker wont run with pharo 1.1.1 (dev)

Eliot Miranda-2
 


On Tue, Nov 9, 2010 at 7:30 PM, David T. Lewis <[hidden email]> wrote:

On Mon, Nov 08, 2010 at 06:41:57PM -0800, Andreas Raab wrote:
>
> On 11/8/2010 5:08 PM, Levente Uzonyi wrote:
> >The problem is that the method expects ByteStrings as arguments, but it
> >won't fail if one of them is a WideString. Instead it returns wrong
> >results. Here's an example (in Squeak):
> >
> >ByteString new
> >findSubstring: 'b'
> >in: 'abc' asWideString
> >startingAt: 1
> >matchTable: ((0 to: 255) as: ByteArray). "===> 0"
> >
> >The result is 0, while it should be 2. The problem is worked around in
> >Squeak by making sure that the primitive is not used in this case.
> >
> >'abc' asWideString
> >findString: 'b'
> >startingAt: 1
> >caseSensitive: true. "===> 2"
> >
> >Though a naive user can still use the method as shown here.
>
> Thanks. Looks like a problem in extracting string arguments in
> translated primitives. With Squeaksource being down again (should we
> move VMMaker to source.squeak.org?) I've attached a change set with a
> fix. This adds an additional check for char* arguments in translated
> prims to ensure that they're actually byte objects.

Andreas,

Your fix does indeed resolve the underlying problem. I added it
to VMMaker (with a note to Eliot to do likewise with oscog), and
also added unit tests based on Levente's example. This should
allow the various WideString workarounds to be removed once the
updated VMs are in circulation.

OK, it's in my working version.  It'll percolate out to oscog soon.

Thanks!
 

Thanks,
Dave


Reply | Threaded
Open this post in threaded view
|

Re: latest vmmaker wont run with pharo 1.1.1 (dev)

Henrik Sperre Johansen
In reply to this post by Andreas.Raab
 


Den 09.11.2010 03:41, skrev Andreas Raab:
>  Thanks. Looks like a problem in extracting string arguments in
> translated primitives. I've attached a change set with a
> fix. This adds an additional check for char* arguments in translated
> prims to ensure that they're actually byte objects.
>
>  
>
> Cheers,
>   - Andreas
Sorry for sort of dropping of the map and not providing any feedback
sooner, work got real hectic this week.
Nice solution though! Looking forward to seeing it included, I'll make
sure all the safeguards in Pharo are removed and the method reverted
once the new VMs are in distribution :D

The part I wrote about matchTable being in need of changes was more of a
feature-request when I think about it, in that it would be nice to have
a comparable function whose matchtable can also include substitutions
outside the latin1-range.
Then again, you'd probably also need normalization and other stuff in
such a method...

Cheers,
Henry
Reply | Threaded
Open this post in threaded view
|

Re: latest vmmaker wont run with pharo 1.1.1 (dev)

David T. Lewis
 
On Thu, Nov 11, 2010 at 12:43:57PM +0100, Henrik Johansen wrote:

>
> Den 09.11.2010 03:41, skrev Andreas Raab:
> >  Thanks. Looks like a problem in extracting string arguments in
> > translated primitives. I've attached a change set with a
> > fix. This adds an additional check for char* arguments in translated
> > prims to ensure that they're actually byte objects.
> >
> >  
> >
> > Cheers,
> >   - Andreas
> Sorry for sort of dropping of the map and not providing any feedback
> sooner, work got real hectic this week.
> Nice solution though! Looking forward to seeing it included, I'll make
> sure all the safeguards in Pharo are removed and the method reverted
> once the new VMs are in distribution :D

Andreas' fix is now in VMMaker on SqueakSource, and Eliot is adding it
to the cog branch, so this will be in all of the future VM builds. If
you can follow up on removing the Pharo safeguards, that would be
great. You would probably want to do that early next year after the
updated VMs are in general circulation.

Thanks,
Dave

Reply | Threaded
Open this post in threaded view
|

Re: latest vmmaker wont run with pharo 1.1.1 (dev)

stephane ducasse-2
 
Yes we will pay attention to do that
Tx all!

Stef

On Nov 11, 2010, at 2:11 PM, David T. Lewis wrote:

>
> On Thu, Nov 11, 2010 at 12:43:57PM +0100, Henrik Johansen wrote:
>>
>> Den 09.11.2010 03:41, skrev Andreas Raab:
>>> Thanks. Looks like a problem in extracting string arguments in
>>> translated primitives. I've attached a change set with a
>>> fix. This adds an additional check for char* arguments in translated
>>> prims to ensure that they're actually byte objects.
>>>
>>>
>>>
>>> Cheers,
>>>  - Andreas
>> Sorry for sort of dropping of the map and not providing any feedback
>> sooner, work got real hectic this week.
>> Nice solution though! Looking forward to seeing it included, I'll make
>> sure all the safeguards in Pharo are removed and the method reverted
>> once the new VMs are in distribution :D
>
> Andreas' fix is now in VMMaker on SqueakSource, and Eliot is adding it
> to the cog branch, so this will be in all of the future VM builds. If
> you can follow up on removing the Pharo safeguards, that would be
> great. You would probably want to do that early next year after the
> updated VMs are in general circulation.
>
> Thanks,
> Dave
>

12