870442 - Fix some shell rooting hazards and false positives

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

Attachment: Proposed changes

Here's a patch to fix some of the remaining rooting hazards in the shell.

I think what is happening in ctypes::ExtractStructField() DebuggerObject_defineProperties() is that the compiler is hoisting a temporary that is not modified in the loop, hence making it live over GC even when it looks like it is only created after the GCing function.

ParallelDo had a HeapPtrObject member converted to HandleObject - I /think/ this is what was originally meant but maybe there was some other reason for this.

ImplicitConvert() in ctypes got some more rooting and IonCache::attachStub() had a parameter rooted, even though this is probably a false positive.

Comment 1

[Tom S [:evilpie]]

Comment on attachment 747512 [details] [diff] [review]
Proposed changes

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

::: js/src/vm/Debugger.cpp
@@ +4504,5 @@
>          Maybe<AutoCompartment> ac;
>          ac.construct(cx, obj);
>          RootedId id(cx);
>          for (size_t i = 0; i < n; i++) {
> +            if (!rewrappedIds.append(JSID_VOID) || !rewrappedDescs.append())

UndefinedValue()

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 747512 [details] [diff] [review]
Proposed changes

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

r=me

::: js/src/vm/ForkJoin.cpp
@@ -164,5 @@
>      ExecutionStatus apply();
>  
>    private:
>      JSContext *cx_;
> -    HeapPtrObject fun_;

Yikes! I wonder if this is what is responsible for the crashes I'm getting in parallel code under GGC right now.

Comment 3

[Jon Coppeard (:jonco)]

(In reply to Tom Schuster [:evilpie] from comment #1)

> UndefinedValue()

This needs a jsid not a Value, so that doesn't work here.

Comment 4

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2aa65478e490

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/mozilla-central/rev/2aa65478e490