Closed Bug 951693 Opened 6 years ago Closed 6 years ago

Fix some newly-discovered rooting hazards in jsinfer.cpp

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: jonco, Assigned: jonco)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

Attached patch fix-infer-hazards (obsolete) — Splinter Review
Fix for the following rooting hazards:

Function '_ZN2js5types15TypeCompartment24setTypeToHomogenousArrayEPNS_16ExclusiveContextEP8JSObjectNS0_4TypeE|void js::types::TypeCompartment::setTypeToHomogenousArray(js::ExclusiveContext*, JSObject*, js::types::Type)' has unrooted 'obj' of type 'JSObject*' live across GC call '_ZN2js5types15TypeCompartment13newTypeObjectEPNS_16ExclusiveContextEPKNS_5ClassEN2JS6HandleINS_11TaggedProtoEEEj|js::types::TypeObject* js::types::TypeCompartment::newTypeObject(js::ExclusiveContext*, js::Class*, class JS::Handle<js::TaggedProto>, uint32)' at js/src/jsinfer.cpp:2359

Function '_ZN2js5types45AddClearDefiniteGetterSetterForPrototypeChainEP9JSContextPNS0_10TypeObjectE4jsid|uint8 js::types::AddClearDefiniteGetterSetterForPrototypeChain(JSContext*, js::types::TypeObject*, jsid)' has unrooted 'id' of type 'jsid' live across GC call '_ZN8JSObject7getTypeEP9JSContext|js::types::TypeObject* JSObject::getType(JSContext*)' at js/src/jsinfer.cpp:3240

Function '_ZL24CheckNewScriptPropertiesP9JSContextPN2js5types10TypeObjectEP10JSFunction|jsinfer.cpp:void CheckNewScriptProperties(JSContext*, js::types::TypeObject*, JSFunction*)' has unrooted 'fun' of type 'JSFunction*' live across GC call '_ZN2js23NewBuiltinClassInstanceEPNS_16ExclusiveContextEPKNS_5ClassENS_2gc9AllocKindENS_13NewObjectKindE|JSObject* js::NewBuiltinClassInstance(js::ExclusiveContext*, js::Class*, uint32, uint32)' at js/src/jsinfer.cpp:3339
Attachment #8349454 - Flags: review?(bhackett1024)
Comment on attachment 8349454 [details] [diff] [review]
fix-infer-hazards

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

I think most of these changes are not actually needed because of the AutoEnterAnalysis is fixArrayType and fixRestArgumentsType.

::: js/src/jsinfer.cpp
@@ -2342,1 @@
>  {

All callers of setTypeToHomogenousArray are wrapped in an AutoEnterAnalysis, so this suite of changes should not be required. Could you add an assertion that cx->compartment()->activeAnalysis is true here?
Attachment #8349454 - Flags: review?(bhackett1024)
Attached patch fix-infer-hazards (obsolete) — Splinter Review
Ah yes, that's much much better.
Attachment #8349454 - Attachment is obsolete: true
Attachment #8350564 - Flags: review?(terrence)
Ugh, and the one in CheckNewScriptProperties isn't needed either.
Attachment #8350564 - Attachment is obsolete: true
Attachment #8350564 - Flags: review?(terrence)
Attachment #8350576 - Flags: review?(terrence)
Comment on attachment 8350576 [details] [diff] [review]
fix-infer-hazards-v3

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

Yup! r=me
Attachment #8350576 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/6ee810e00d3c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.