[Vm-dev] Bug in #instVarAt:put: for read-only objects

[Clément Bera]

On Sat, Jan 6, 2018 at 1:32 PM, Norbert Hartl <norbert at hartl.name> wrote:

>
> Btw. why is the name NoModificationError? Isn’t the purpose of an Error
> that it is not resumable? And is NoModificationError very likely to be
> resumed? Like in #retryModification?
>

Normally the NoModificationError should be able to retry the modification,
and then resumes execution just after the primitive call / the inst var
store.

In the case of primitive failure I guess either the error should resume
with the value to return or the primitive fall-back code should return the
correct value. In the case of inst var store, we need to resume execution
using #jump (See #attemptToAssign: value withIndex: index)

For at:put: we could have:

<primitive: 174 error: ec>
self isReadOnlyObject
ifTrue: [*^ *(ModificationForbidden for: self atInstVar: index with:
anObject) signal]
ifFalse: [ self primitiveFailed ]

OR

<primitive: 174 error: ec>
self isReadOnlyObject
ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject)
signal. *^ anObject*]
ifFalse: [ self primitiveFailed ]

+ the index bound check mentioned by Eliot and asInteger trick (based on
at: primitive failure code)

So that when the exception resumes the correct behavior happens.


> Norbert
>
> Am 05.01.2018 um 14:34 schrieb Eliot Miranda <eliot.miranda at gmail.com>:
>
> Hi Clément,
>
>     is it too late to take a look at the VisualWorks code and use the same
> class names and selectors they use?  IIRC it is NoMidificationError.  It
> may make e.g. Gemstone's job easier if there is some consistency.
>
> _,,,^..^,,,_ (phone)
>
> On Jan 5, 2018, at 5:22 AM, Clément Bera <bera.clement at gmail.com> wrote:
>
> Hi,
>
> No this is not a bug.
>
> This needs to be handled in the primitive failure code in the image, the
> method should be something like that :
>
> Object>>instVarAt: index put: anObject
> <primitive: 174 error: ec>
>         self isReadOnlyObject ifTrue: [(ModificationForbidden for: self
> atInstVar: index with: anObject) signal]
> self primitiveFailed
>
> All primitive fall-back code triggering object mutation should be
> rewritten this way, especially primitives such as #at:put:,
> #instVarAt:put:, etc.
>
> Cheers
>
> On Fri, Jan 5, 2018 at 1:42 PM, Norbert Hartl <norbert at hartl.name> wrote:
>
>>
>> If I do
>>
>> (#foo -> #bar)
>> beReadOnlyObject;
>> value: #baz
>>
>> it throws
>>
>> *ModificationForbidden:  #foo->#bar is read-only, hence its field 2
>> cannot be modified with #baz*
>>
>>  which is as expected. But if I do
>>
>> (#foo -> #bar)
>> beReadOnlyObject;
>> instVarNamed: #value put: #baz
>>
>> I get
>>
>> *PrimitiveFailed: primitive #instVarAt:put: in Association failed*
>>
>> I think this a bug, no?
>>
>> Norbert
>>
>>
>
>
> --
> Clément Béra
> Pharo consortium engineer
> https://clementbera.wordpress.com/
> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
>
>
>
>


-- 
Clément Béra
Pharo consortium engineer
https://clementbera.wordpress.com/
Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20180106/7ed0a4a1/attachment.html>