The default bug view has changed. See this FAQ.

Make XPCWrappedJSClass::CallMethod use public rooting APIs

RESOLVED FIXED

Status

()

Core
XPConnect
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: jorendorff, Assigned: mrbkap)

Tracking

({sec-other})

Other Branch
sec-other
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse])

(Reporter)

Description

8 years ago
There's this comment in XPCWrappedJSClass::CallMethod:

http://hg.mozilla.org/mozilla-central/file/5f349409c9d5/js/src/xpconnect/src/xpcwrappedjsclass.cpp#l1259

  // We use js_AllocStack, js_Invoke, and js_FreeStack so that the gcthings
  // we use as args will be rooted by the engine as we do conversions and
  // prepare to do the function call. This adds a fair amount of complexity,
  // but is a good optimization compared to calling JS_AddRoot for each item.

I don't think this is correct anymore.  In jsgc.cpp it seems like js_TraceContext only traces those parts of the stack that are pointed-to by JSStackFrames.  The right thing here, I think, would be a JSAutoTempValueRooter.

Incidentally, I think making that change would also fix bug 476643.
Group: core-security
Whiteboard: [sg:critical?]
(Assignee)

Comment 1

8 years ago
I think we should do this, but there's no GC hazard here. js_AllocStack adds an entry to cx->stackHeaders, which is scanned on trace.
Group: core-security
Whiteboard: [sg:critical?] → [sg:nse]
(Assignee)

Comment 2

8 years ago
By "on trace," I mean "on GC."
(Assignee)

Updated

8 years ago
Summary: Possible GC hazard in XPCWrappedJSClass::CallMethod → Make XPCWrappedJSClass::CallMethod use public rooting APIs
Keywords: sec-other
(Assignee)

Comment 3

5 years ago
This happened over a series of patches a while back.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.