Bug 1517158 - add hasGlobal for AbstractFramePtr, DebugFrame and JSScript. r=jonco
authorYoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Thu, 03 Jan 2019 13:19:30 +0100
changeset 509839 501ffb16f40b1912877c2448117fdefd76e0b836
parent 509838 af76ddce64a0736c217ccd76a762e1d09ca2c93c
child 509840 e01254abd76e30e9c55f12b15415b5b20c129bba
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1517158
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1517158 - add hasGlobal for AbstractFramePtr, DebugFrame and JSScript. r=jonco To prevent triggering read barrier while accessing GlobalObject.
js/src/jit-test/tests/gc/bug-1517158.js
js/src/vm/Debugger.cpp
js/src/vm/JSScript-inl.h
js/src/vm/JSScript.h
js/src/vm/Stack-inl.h
js/src/vm/Stack.h
js/src/wasm/WasmTypes.cpp
js/src/wasm/WasmTypes.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1517158.js
@@ -0,0 +1,21 @@
+gczeal(0);
+
+(function() {
+    var g = newGlobal();
+    g.debuggeeGlobal = this;
+    g.eval("(" + function() {
+        dbg = new Debugger(debuggeeGlobal);
+        dbg.onExceptionUnwind = function(frame, exc) {
+            var s = '!';
+            for (var f = frame; f; f = f.older)
+                if (f.type === "call")
+                    s += f.callee.name;
+        };
+    } + ")();");
+    try {
+        h();
+    } catch (e) {}
+    g.dbg.enabled = false;
+})();
+// jsfunfuzz-generated
+startgc(114496726);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -4238,17 +4238,17 @@ void Debugger::removeDebuggeeGlobal(Free
   // Debugger objects that are no longer debugging the relevant global might
   // have live Frame objects. So we take the easy way out and kill them here.
   // This is a bug, since it's observable and contrary to the spec. One
   // possible fix would be to put such objects into a compartment-wide bag
   // which slowPathOnLeaveFrame would have to examine.
   for (FrameMap::Enum e(frames); !e.empty(); e.popFront()) {
     AbstractFramePtr frame = e.front().key();
     DebuggerFrame* frameobj = e.front().value();
-    if (frame.global() == global) {
+    if (frame.hasGlobal(global)) {
       frameobj->freeFrameIterData(fop);
       DebuggerFrame_maybeDecrementFrameScriptStepModeCount(fop, frame,
                                                            frameobj);
       e.removeFront();
     }
   }
 
   // Clear this global's generators from generatorFrames as well.
--- a/js/src/vm/JSScript-inl.h
+++ b/js/src/vm/JSScript-inl.h
@@ -114,16 +114,20 @@ inline js::RegExpObject* JSScript::getRe
 inline js::GlobalObject& JSScript::global() const {
   /*
    * A JSScript always marks its realm's global (via bindings) so we can
    * assert that maybeGlobal is non-null here.
    */
   return *realm()->maybeGlobal();
 }
 
+inline bool JSScript::hasGlobal(const js::GlobalObject *global) const {
+  return global == realm()->unsafeUnbarrieredMaybeGlobal();
+}
+
 inline js::LexicalScope* JSScript::maybeNamedLambdaScope() const {
   // Dynamically created Functions via the 'new Function' are considered
   // named lambdas but they do not have the named lambda scope of
   // textually-created named lambdas.
   js::Scope* scope = outermostScope();
   if (scope->kind() == js::ScopeKind::NamedLambda ||
       scope->kind() == js::ScopeKind::StrictNamedLambda) {
     MOZ_ASSERT_IF(!strict(), scope->kind() == js::ScopeKind::NamedLambda);
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -2379,16 +2379,17 @@ class JSScript : public js::gc::TenuredC
 
   inline js::TypeScript* types(const js::AutoSweepTypeScript& sweep);
   inline bool typesNeedsSweep() const;
 
   void maybeReleaseTypes();
   void sweepTypes(const js::AutoSweepTypeScript& sweep);
 
   inline js::GlobalObject& global() const;
+  inline bool hasGlobal(const js::GlobalObject* global) const;
   js::GlobalObject& uninlinedGlobal() const;
 
   uint32_t bodyScopeIndex() const { return bodyScopeIndex_; }
 
   js::Scope* bodyScope() const { return getScope(bodyScopeIndex_); }
 
   js::Scope* outermostScope() const {
     // The body scope may not be the outermost scope in the script when
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -652,16 +652,23 @@ inline wasm::Instance* AbstractFramePtr:
 
 inline GlobalObject* AbstractFramePtr::global() const {
   if (isWasmDebugFrame()) {
     return asWasmDebugFrame()->global();
   }
   return &script()->global();
 }
 
+inline bool AbstractFramePtr::hasGlobal(const GlobalObject* global) const {
+  if (isWasmDebugFrame()) {
+    return asWasmDebugFrame()->hasGlobal(global);
+  }
+  return script()->hasGlobal(global);
+}
+
 inline JSFunction* AbstractFramePtr::callee() const {
   if (isInterpreterFrame()) {
     return &asInterpreterFrame()->callee();
   }
   if (isBaselineFrame()) {
     return asBaselineFrame()->callee();
   }
   return asRematerializedFrame()->callee();
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -227,16 +227,17 @@ class AbstractFramePtr {
   inline bool isModuleFrame() const;
   inline bool isEvalFrame() const;
   inline bool isDebuggerEvalFrame() const;
 
   inline bool hasScript() const;
   inline JSScript* script() const;
   inline wasm::Instance* wasmInstance() const;
   inline GlobalObject* global() const;
+  inline bool hasGlobal(const GlobalObject* global) const;
   inline JSFunction* callee() const;
   inline Value calleev() const;
   inline Value& thisArgument() const;
 
   inline bool isConstructing() const;
   inline Value newTarget() const;
 
   inline bool debuggerNeedsCheckPrimitiveReturn() const;
--- a/js/src/wasm/WasmTypes.cpp
+++ b/js/src/wasm/WasmTypes.cpp
@@ -644,16 +644,20 @@ void DebugFrame::alignmentStaticAsserts(
   static_assert(sizeof(DebugFrame) % 16 == 0, "ARM64 SP alignment");
 #endif
 }
 
 GlobalObject* DebugFrame::global() const {
   return &instance()->object()->global();
 }
 
+bool DebugFrame::hasGlobal(const GlobalObject* global) const {
+  return global == &instance()->objectUnbarriered()->global();
+}
+
 JSObject* DebugFrame::environmentChain() const {
   return &global()->lexicalEnvironment();
 }
 
 bool DebugFrame::getLocal(uint32_t localIndex, MutableHandleValue vp) {
   ValTypeVector locals;
   size_t argsLength;
   if (!instance()->debug().debugGetLocalTypes(funcIndex(), &locals,
--- a/js/src/wasm/WasmTypes.h
+++ b/js/src/wasm/WasmTypes.h
@@ -2540,16 +2540,17 @@ class DebugFrame {
   Frame frame_;
 
  public:
   static DebugFrame* from(Frame* fp);
   Frame& frame() { return frame_; }
   uint32_t funcIndex() const { return funcIndex_; }
   Instance* instance() const { return frame_.instance(); }
   GlobalObject* global() const;
+  bool hasGlobal(const GlobalObject* global) const;
   JSObject* environmentChain() const;
   bool getLocal(uint32_t localIndex, MutableHandleValue vp);
 
   // The return value must be written from the unboxed representation in the
   // results union into cachedReturnJSValue_ by updateReturnJSValue() before
   // returnValue() can return a Handle to it.
 
   bool hasCachedReturnJSValue() const { return hasCachedReturnJSValue_; }