Bug 154009 – GetValueFunc/PutValueFunc should not take both slotBase and thisValue

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gavin Barraclough]

In JavaScript there are two types of properties - regular value properties, and accessor properties. One difference between these is how they are reflected by getOwnPropertyDescriptor, and another is what object they operate on in the case of a prototype access. If you access a value property of a prototype object it return a value pertinent to the prototype, but in the case of a prototype object returning an accessor, then the accessor function is applied to the base object of the access.

JSC supports special 'custom' properties implemented as a c++ callback, and these custom properties can be used to implement either value- or accessor-like behavior. getOwnPropertyDescriptor behavior is selected via the CustomAccessor attribute. Value- or accessor-like object selection is current supported by passing both the slotBase and the thisValue to the callback,and hoping it uses the right one. This is probably inefficient, bug-prone, and leads to crazy like JSBoundSlotBaseFunction.

Instead, just pass one thisValue to the callback functions, consistent with CustomAccessor.

[Gavin Barraclough]

Created attachment 270914 [details]
Fix

[WebKit Commit Bot]

Attachment 270914 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/CustomGetterSetter.h:73:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gavin Barraclough]

As a follow up to this, more cleanup around JSBoundSlotBaseFunction/CustomGetter should be possible.

JSBoundSlotBaseFunction no longer needs to bind the slotBase (it's never used!), And we can probably merge these back with CustomGetters (reify of value/accessor properties always just creates a CustomGetter; lazily create a JSFunction derivate only if requested by getOwnPropertyDescriptor).

[Gavin Barraclough]

Created attachment 270915 [details]
With style fix

[Gavin Barraclough]

Created attachment 270939 [details]
Again

[Geoffrey Garen]

Comment on attachment 270939 [details]
Again

r=me

[Gavin Barraclough]

Fixed in r196331

[Csaba Osztrogonác]

(In reply to comment #7)
> Fixed in r196331

It made ~180 JSC tests crash on ARMv7 Linux bots: bug154064

Do you think if it is a Linux specific bug or have you noticed
this regression on your internal iOS ARM JSC tester bots too?

[Csaba Osztrogonác]

(In reply to comment #8)
> (In reply to comment #7)
> > Fixed in r196331
> 
> It made ~180 JSC tests crash on ARMv7 Linux bots: bug154064
> 
> Do you think if it is a Linux specific bug or have you noticed
> this regression on your internal iOS ARM JSC tester bots too?

I found the bug and fixed it in bug154064. The problem is related to ARM EABI,
which says that 64 bits value should be aligned to even numbered register.