Bug 1533890: Minor cleanups r=mgaudet
authorIain Ireland <iireland@mozilla.com>
Wed, 01 May 2019 20:35:16 +0000
changeset 472558 cbf988d7f823f40af06d3124273626c6b63ee1f2
parent 472557 5d03ed4eeeb06b1bd28306e47ec0b0327690de25
child 472559 914bcc5d82716ce7ae91b90202b754ead0db06aa
push id84721
push useriireland@mozilla.com
push dateFri, 03 May 2019 18:10:56 +0000
treeherderautoland@cbf988d7f823 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmgaudet
bugs1533890, 1545278
milestone68.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 1533890: Minor cleanups r=mgaudet 1. In bug 1545278, Ted is making our story about guarding on object pointer identity more robust. Since I am adding a new guardSpecificObject call in the ConstStringSplit patch, it seemed reasonable to add guardSpecificFunction now instead of later. 2. It's not directly relevant in the current patch, but in a previous version of the StringSplit patch (before I realized we could delete the whole thing) it turned out that calling isSelfHostedFunctionWithName on an arbitrary function can trigger assertions, because GetSelfHostedFunctionName assumes isExtended, but isSelfHostedBuiltin does not necessarily imply isExtended (in the case of nested anonymous functions). 3. Fixing the format string in a JitSpew message I added in a previous stack. Depends on D29535 Differential Revision: https://phabricator.services.mozilla.com/D29536
js/src/jit/BaselineCacheIRCompiler.cpp
js/src/jit/CacheIR.cpp
js/src/jit/CacheIR.h
js/src/vm/SelfHosting.cpp
--- a/js/src/jit/BaselineCacheIRCompiler.cpp
+++ b/js/src/jit/BaselineCacheIRCompiler.cpp
@@ -2298,17 +2298,17 @@ ICStub* js::jit::AttachBaselineCacheIRSt
 
     // We found a stub that's exactly the same as the stub we're about to
     // attach. Just return nullptr, the caller should do nothing in this
     // case.
     if (updated) {
       *attached = true;
     } else {
       JitSpew(JitSpew_BaselineICFallback,
-              "Tried attaching identical stub for (%s:%u%u)",
+              "Tried attaching identical stub for (%s:%u:%u)",
               outerScript->filename(), outerScript->lineno(),
               outerScript->column());
     }
     return nullptr;
   }
 
   // Time to allocate and attach a new stub.
 
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -5070,17 +5070,17 @@ AttachDecision CallIRGenerator::tryAttac
   // Load the callee and ensure it is an object
   ValOperandId calleeValId =
       writer.loadArgumentDynamicSlot(ArgumentKind::Callee, argcId, flags);
   ObjOperandId calleeObjId = writer.guardIsObject(calleeValId);
 
   FieldOffset calleeOffset = 0;
   if (isSpecialized) {
     // Ensure callee matches this stub's callee
-    calleeOffset = writer.guardSpecificObject(calleeObjId, calleeFunc);
+    calleeOffset = writer.guardSpecificFunction(calleeObjId, calleeFunc);
     // Guard against relazification
     writer.guardFunctionHasJitEntry(calleeObjId, isConstructing);
   } else {
     // Guard that object is a scripted function
     writer.guardClass(calleeObjId, GuardClassKind::JSFunction);
     writer.guardFunctionHasJitEntry(calleeObjId, isConstructing);
 
     if (isConstructing) {
@@ -5240,17 +5240,17 @@ AttachDecision CallIRGenerator::tryAttac
   // Load the callee and ensure it is an object
   ValOperandId calleeValId =
       writer.loadArgumentDynamicSlot(ArgumentKind::Callee, argcId, flags);
   ObjOperandId calleeObjId = writer.guardIsObject(calleeValId);
 
   FieldOffset calleeOffset = 0;
   if (isSpecialized) {
     // Ensure callee matches this stub's callee
-    calleeOffset = writer.guardSpecificObject(calleeObjId, calleeFunc);
+    calleeOffset = writer.guardSpecificFunction(calleeObjId, calleeFunc);
     writer.callNativeFunction(calleeObjId, argcId, op_, calleeFunc, flags);
   } else {
     // Guard that object is a native function
     writer.guardClass(calleeObjId, GuardClassKind::JSFunction);
     writer.guardFunctionIsNative(calleeObjId);
 
     if (isConstructing) {
       // If callee is not a constructor, we have to throw.
@@ -5494,17 +5494,17 @@ AttachDecision CallIRGenerator::tryAttac
   }
 
   Int32OperandId argcId(writer.setInputOperandId(0));
 
   // Guard that callee is the self-hosted String_split function
   ValOperandId calleeValId =
       writer.loadArgumentFixedSlot(ArgumentKind::Callee, argc_);
   ObjOperandId calleeObjId = writer.guardIsObject(calleeValId);
-  writer.guardSpecificObject(calleeObjId, calleeFunc);
+  writer.guardSpecificFunction(calleeObjId, calleeFunc);
 
   // Guard that |this| is the expected string
   ValOperandId strValId =
       writer.loadArgumentFixedSlot(ArgumentKind::This, argc_);
   StringOperandId strStringId = writer.guardIsString(strValId);
   FieldOffset strOffset = writer.guardSpecificAtom(strStringId, &str->asAtom());
 
   // Guard that the argument is the expected separator
--- a/js/src/jit/CacheIR.h
+++ b/js/src/jit/CacheIR.h
@@ -964,16 +964,21 @@ class MOZ_RAII CacheIRWriter : public JS
   void guardNotDOMProxy(ObjOperandId obj) {
     writeOpWithOperandId(CacheOp::GuardNotDOMProxy, obj);
   }
   FieldOffset guardSpecificObject(ObjOperandId obj, JSObject* expected) {
     assertSameCompartment(expected);
     writeOpWithOperandId(CacheOp::GuardSpecificObject, obj);
     return addStubField(uintptr_t(expected), StubField::Type::JSObject);
   }
+  FieldOffset guardSpecificFunction(ObjOperandId obj, JSFunction* expected) {
+    // Guard object is a specific function. This implies immutable fields on
+    // the JSFunction struct itself are unchanged.
+    return guardSpecificObject(obj, expected);
+  }
   FieldOffset guardSpecificAtom(StringOperandId str, JSAtom* expected) {
     writeOpWithOperandId(CacheOp::GuardSpecificAtom, str);
     return addStubField(uintptr_t(expected), StubField::Type::String);
   }
   void guardSpecificSymbol(SymbolOperandId sym, JS::Symbol* expected) {
     writeOpWithOperandId(CacheOp::GuardSpecificSymbol, sym);
     addStubField(uintptr_t(expected), StubField::Type::Symbol);
   }
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -3506,17 +3506,18 @@ void JSRuntime::assertSelfHostedFunction
   JSFunction* selfHostedFun = getUnclonedSelfHostedFunction(cx, name);
   MOZ_ASSERT(selfHostedFun);
   MOZ_ASSERT(selfHostedFun->getExtendedSlot(HAS_SELFHOSTED_CANONICAL_NAME_SLOT)
                  .toBoolean());
 #endif
 }
 
 bool js::IsSelfHostedFunctionWithName(JSFunction* fun, JSAtom* name) {
-  return fun->isSelfHostedBuiltin() && GetSelfHostedFunctionName(fun) == name;
+  return fun->isSelfHostedBuiltin() && fun->isExtended() &&
+         GetSelfHostedFunctionName(fun) == name;
 }
 
 JSAtom* js::GetSelfHostedFunctionName(JSFunction* fun) {
   Value name = fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT);
   if (!name.isString()) {
     return nullptr;
   }
   return &name.toString()->asAtom();