Fix some shell rooting hazards and false positives

RESOLVED FIXED in mozilla23

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 747512 [details] [diff] [review]
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.
Attachment #747512 - Flags: review?(terrence)
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 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.
Attachment #747512 - Flags: review?(terrence) → review+
(Assignee)

Comment 3

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

> UndefinedValue()

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

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/2aa65478e490
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.