put_by_id does will incorrectly cache writes where a specific value exists, where at the point of caching the same value is being written.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Gavin Barraclough]

When performing a put_by_id that is replacing a property already present on the object, there are three interesting cases regarding the state of the specific value:

(1) No specific value set – nothing to do, leave the structure in it's current state, can cache.
(2) A specific value was set, the new put is not of a specified value (i.e. function), or is of a different specific value – in these cases we need to perform a despecifying transition to clear the specific value in the structure, but having done so this is a normal property so as such we can again cache normally.
(3) A specific value was set, and we are overwriting with the same value – in these cases, leave the structure unchanged, but since a specific value is set we cannot cache this put (we would need the JIT to dynamically check the value being written matched).

Unfortunately, the current behaviour does not match this.  the checks for a specific value being present & the value matching are combined in such a way that in case (2), above we will unnecessarily prevent the transition being cached, but in case (3) we will incorrectly fail to prevent caching.

The bug exposes itself if multiple puts of the same specific value are performed to a property, and erroneously the put is allowed to be cached by the JIT.  Method checks may be generated caching calls of this structure.  Subsequent puts performed from JIT code may write different values without triggering a despecify transition, and as such cached method checks will continue to pass, despite the value having changed.

[Gavin Barraclough]

Created attachment 49737 [details]
The patch

No performance impact.

[Oliver Hunt]

Comment on attachment 49737 [details]
The patch

> +        //  (1) There is an existing specific value set, and we're overwriting with *the same value*.
> +        //       * Do nothing â no need to despecify, but that means we can't cache (a cached
r+ but get rid of yer funny foreign characters :p

--Oliver

[Gavin Barraclough]

fixed in r55379.

[Shinichiro Hamaji]

Committed r55521: <http://trac.webkit.org/changeset/55521>