960011 - Fix unsafe references in jsapi.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

The following unsafe references are reported:

Function '_Z26JS_LookupPropertyWithFlagsP9JSContextP8JSObjectPKcjN2JS13MutableHandleINS5_5ValueEEE|uint8 JS_LookupPropertyWithFlags(JSContext*, JSObject*, int8*, uint32, class JS::MutableHandle<JS::Value>)' takes unsafe address of unrooted 'obj2' at js/src/jsapi.cpp:2869

Function '_ZL24DefineSelfHostedPropertyP9JSContextN2JS6HandleIP8JSObjectEEPKcS7_S7_jji|jsapi.cpp:uint8 DefineSelfHostedProperty(JSContext*, class JS::Handle<JSObject*>, int8*, int8*, int8*, uint32, uint32, int32)' takes unsafe address of unrooted '__temp_46' at js/src/jsapi.cpp:3165

Function '_Z15JS_DefineObjectP9JSContextP8JSObjectPKcPK7JSClassS2_j|JSObject* JS_DefineObject(JSContext*, JSObject*, int8*, JSClass*, JSObject*, uint32)' takes unsafe address of unrooted '__temp_10' at js/src/jsapi.cpp:3256

Function '_Z21JS_DefineConstDoublesP9JSContextP8JSObjectPK17JSConstDoubleSpec|uint8 JS_DefineConstDoubles(JSContext*, JSObject*, JSConstDoubleSpec*)' takes unsafe address of unrooted 'value' at js/src/jsapi.cpp:3280

Function '_Z19JS_DefinePropertiesP9JSContextP8JSObjectPK14JSPropertySpec|uint8 JS_DefineProperties(JSContext*, JSObject*, JSPropertySpec*)' takes unsafe address of unrooted '__temp_5' at js/src/jsapi.cpp:3302

Function '_Z22JS_DeletePropertyById2P9JSContextP8JSObject4jsidPb|uint8 JS_DeletePropertyById2(JSContext*, JSObject*, jsid, uint8*)' takes unsafe address of unrooted '__temp_11' at js/src/jsapi.cpp:3530

Function '_Z18JS_DeleteProperty2P9JSContextP8JSObjectPKcPb|uint8 JS_DeleteProperty2(JSContext*, JSObject*, int8*, uint8*)' takes unsafe address of unrooted '__temp_5' at js/src/jsapi.cpp:3556

Function '_Z20JS_DeleteUCProperty2P9JSContextP8JSObjectPKDsmPb|uint8 JS_DeleteUCProperty2(JSContext*, JSObject*, uint16*, uint64, uint8*)' takes unsafe address of unrooted '__temp_5' at js/src/jsapi.cpp:3571

Comment 1

[Jon Coppeard (:jonco)]

Attachment: unsafe-refs-jsapi-1

Here's a patch to fix the first five issues:

- Handlify JS_LookupPropertyWithFlags() and JS_LookupPropertyWithFlagsById() APIs
- Handlify internal DefineProperty()

The others are related to JSObject::deleteByValue(), which is the cause of a bunch of the other warnings too.  It takes a reference to a Value, and doesn't root in the case that it turns out to definitely an index.  I assume this is performance sensitive.  I haven't worked out a good fix for this.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 8360438 [details] [diff] [review]
unsafe-refs-jsapi-1

Review of attachment 8360438 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! r=me

Comment 3

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6a232abb5896

Comment 4

[Jon Coppeard (:jonco)]

Ugh, and here's followup to fix the rooting hazard introduced:

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbfac99adeef

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6a232abb5896
https://hg.mozilla.org/mozilla-central/rev/cbfac99adeef

Comment 6

[Florian Bender]

Is this bug still valid?

Comment 7

[Terrence Cole [:terrence]]

We can open up more bugs if we work on this further.