Bug 1566057: Convert Detecting function to use bytecode iterator. r=tcampbell
authorWill Hawkins <whawkins@mozilla.com>
Sat, 14 Sep 2019 17:05:23 +0000
changeset 493331 3afa57585d5cdcc76519e308279d90ee239887ed
parent 493330 7d8e262ca766d4cd7b6706863cbaea65169f7e4b
child 493332 2422a9d771bddef28d977b0013ab344935aa831f
push id95419
push usercsabou@mozilla.com
push dateMon, 16 Sep 2019 05:10:37 +0000
treeherderautoland@3afa57585d5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1566057
milestone71.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 1566057: Convert Detecting function to use bytecode iterator. r=tcampbell Differential Revision: https://phabricator.services.mozilla.com/D42611
js/src/vm/BytecodeLocation-inl.h
js/src/vm/BytecodeLocation.h
js/src/vm/BytecodeUtil.h
js/src/vm/NativeObject.cpp
--- a/js/src/vm/BytecodeLocation-inl.h
+++ b/js/src/vm/BytecodeLocation-inl.h
@@ -21,11 +21,17 @@ inline bool BytecodeLocation::isValid(co
 
 inline bool BytecodeLocation::isInBounds(const JSScript* script) const {
   return script->contains(*this);
 }
 inline uint32_t BytecodeLocation::bytecodeToOffset(JSScript* script) {
   return script->pcToOffset(this->rawBytecode_);
 }
 
+inline PropertyName* BytecodeLocation::getPropertyName(
+    const JSScript* script) const {
+  MOZ_ASSERT(this->isValid());
+  return script->getName(this->rawBytecode_);
+}
+
 }  // namespace js
 
 #endif
--- a/js/src/vm/BytecodeLocation.h
+++ b/js/src/vm/BytecodeLocation.h
@@ -9,16 +9,18 @@
 
 #include "js/TypeDecls.h"
 #include "vm/BytecodeUtil.h"
 
 namespace js {
 
 typedef uint32_t RawBytecodeLocationOffset;
 
+class PropertyName;
+
 class BytecodeLocationOffset {
   RawBytecodeLocationOffset rawOffset_;
 
  public:
   explicit BytecodeLocationOffset(RawBytecodeLocationOffset offset)
       : rawOffset_(offset) {}
 
   RawBytecodeLocationOffset rawOffset() const { return rawOffset_; }
@@ -64,16 +66,18 @@ class BytecodeLocation {
   bool isValid(const JSScript* script) const;
 
   // Return true if this bytecode location is within the bounds of the
   // bytecode for a given script.
   bool isInBounds(const JSScript* script) const;
 
   uint32_t bytecodeToOffset(JSScript* script);
 
+  PropertyName* getPropertyName(const JSScript* script) const;
+
   bool operator==(const BytecodeLocation& other) const {
     MOZ_ASSERT(this->debugOnlyScript_ == other.debugOnlyScript_);
     return rawBytecode_ == other.rawBytecode_;
   }
 
   bool operator!=(const BytecodeLocation& other) const {
     return !(other == *this);
   }
@@ -115,16 +119,26 @@ class BytecodeLocation {
   bool isJumpTarget() const { return BytecodeIsJumpTarget(getOp()); }
 
   bool isJump() const { return IsJumpOpcode(getOp()); }
 
   bool fallsThrough() const { return BytecodeFallsThrough(getOp()); }
 
   uint32_t icIndex() const { return GET_ICINDEX(rawBytecode_); }
 
+  bool isEqualityOp() const { return IsEqualityOp(getOp()); }
+
+  bool isStrictEqualityOp() const {
+    return is(JSOP_STRICTEQ) || is(JSOP_STRICTNE);
+  }
+
+  bool isDetectingOp() const { return IsDetecting(getOp()); }
+
+  bool isNameOp() const { return IsNameOp(getOp()); }
+
   // Accessors:
   JSOp getOp() const { return JSOp(*rawBytecode_); }
 
   BytecodeLocation getJumpTarget() const {
     // The default target of a JSOP_TABLESWITCH also follows this format.
     MOZ_ASSERT(isJump() || is(JSOP_TABLESWITCH));
     return BytecodeLocation(*this,
                             rawBytecode_ + GET_JUMP_OFFSET(rawBytecode_));
--- a/js/src/vm/BytecodeUtil.h
+++ b/js/src/vm/BytecodeUtil.h
@@ -530,16 +530,20 @@ inline bool IsEqualityOp(JSOp op) {
 inline bool IsRelationalOp(JSOp op) {
   return op == JSOP_LT || op == JSOP_LE || op == JSOP_GT || op == JSOP_GE;
 }
 
 inline bool IsCheckStrictOp(JSOp op) {
   return CodeSpec[op].format & JOF_CHECKSTRICT;
 }
 
+inline bool IsDetecting(JSOp op) { return CodeSpec[op].format & JOF_DETECTING; }
+
+inline bool IsNameOp(JSOp op) { return CodeSpec[op].format & JOF_NAME; }
+
 #ifdef DEBUG
 inline bool IsCheckSloppyOp(JSOp op) {
   return CodeSpec[op].format & JOF_CHECKSLOPPY;
 }
 #endif
 
 inline bool IsAtomOp(JSOp op) { return JOF_OPTYPE(op) == JOF_ATOM; }
 
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -16,16 +16,17 @@
 #include "jit/BaselineIC.h"
 #include "js/CharacterEncoding.h"
 #include "js/Value.h"
 #include "vm/EqualityOperations.h"  // js::SameValue
 #include "vm/TypedArrayObject.h"
 
 #include "gc/Nursery-inl.h"
 #include "vm/ArrayObject-inl.h"
+#include "vm/BytecodeLocation-inl.h"
 #include "vm/EnvironmentObject-inl.h"
 #include "vm/JSObject-inl.h"
 #include "vm/JSScript-inl.h"
 #include "vm/Shape-inl.h"
 #include "vm/TypeInference-inl.h"
 
 using namespace js;
 
@@ -2328,66 +2329,77 @@ bool js::NativeGetExistingProperty(JSCon
                                    HandleNativeObject obj, HandleShape shape,
                                    MutableHandleValue vp) {
   RootedValue receiverValue(cx, ObjectValue(*receiver));
   return GetExistingProperty<CanGC>(cx, receiverValue, obj, shape, vp);
 }
 
 /*
  * Given pc pointing after a property accessing bytecode, return true if the
- * access is "property-detecting" -- that is, if we shouldn't warn about it
- * even if no such property is found and strict warnings are enabled.
+ * access is "property-detecting". Caller will use this value to determine
+ * whether or not to warn that an undefined object property *may* be used in
+ * a way that the programmer does not expect. In other words, we do not want
+ * to warn the programmer if he/she/they are doing something like
+ *
+ * if (obj.property) { }
+ * or
+ * if (obj.property == undefined) {}
+ * or
+ * if (obj.property == null) {}
  */
 static bool Detecting(JSContext* cx, JSScript* script, jsbytecode* pc) {
   MOZ_ASSERT(script->containsPC(pc));
 
-  // Skip jump target and dup opcodes.
-  while (pc < script->codeEnd() &&
-         (BytecodeIsJumpTarget(JSOp(*pc)) || JSOp(*pc) == JSOP_DUP))
-    pc = GetNextPc(pc);
-
-  MOZ_ASSERT(script->containsPC(pc));
-  if (pc >= script->codeEnd()) {
-    return false;
+  BytecodeIterator scriptIterator =
+      BytecodeIterator(BytecodeLocation(script, pc));
+  BytecodeIterator endIter = BytecodeIterator(script->endLocation());
+
+  // Skip over jump targets and duplication operations.
+  while (scriptIterator->isJumpTarget() || scriptIterator->is(JSOP_DUP)) {
+    if (++scriptIterator == endIter) {
+      // If we are at the end of the script, we cannot be detecting
+      // the property.
+      return false;
+    }
   }
 
-  // General case: a branch or equality op follows the access.
-  JSOp op = JSOp(*pc);
-  if (CodeSpec[op].format & JOF_DETECTING) {
+  // General case: Do not warn if the operation branches on or tests
+  // the equality of the property.
+  if (scriptIterator->isDetectingOp()) {
     return true;
   }
 
-  jsbytecode* endpc = script->codeEnd();
-
-  if (op == JSOP_NULL) {
-    // Special case #1: don't warn about (obj.prop == null).
-    if (++pc < endpc) {
-      op = JSOp(*pc);
-      return op == JSOP_EQ || op == JSOP_NE;
+  // Special case: Do not warn if we are checking whether the property is null.
+  if (scriptIterator->is(JSOP_NULL)) {
+    if (++scriptIterator == endIter) {
+      return false;
     }
-    return false;
+    return scriptIterator->isEqualityOp() &&
+           !scriptIterator->isStrictEqualityOp();
   }
 
-  // Special case #2: don't warn about (obj.prop == undefined).
-  if (op == JSOP_GETGNAME || op == JSOP_GETNAME) {
-    JSAtom* atom = script->getAtom(GET_UINT32_INDEX(pc));
-    if (atom == cx->names().undefined && (pc += CodeSpec[op].length) < endpc) {
-      op = JSOp(*pc);
-      return op == JSOP_EQ || op == JSOP_NE || op == JSOP_STRICTEQ ||
-             op == JSOP_STRICTNE;
+  // Special case #2: Do not warn if we are checking whether the property is
+  // undefined.
+  if (scriptIterator->is(JSOP_GETGNAME) || scriptIterator->is(JSOP_GETNAME) ||
+      scriptIterator->is(JSOP_UNDEFINED)) {
+    // If we using the result of a variable lookup to use in the comparison
+    // against the property and that lookup does not result in 'undefined',
+    // the type of subsequent operations do not matter -- we always warn.
+    if (scriptIterator->isNameOp() &&
+        scriptIterator->getPropertyName(script) != cx->names().undefined) {
+      return false;
     }
+    // Because we know that the top of the stack is 'undefined', if the next
+    // operation exists and it is a comparison operation (of any kind) we
+    // supress a warning.
+    if (++scriptIterator == endIter) {
+      return false;
+    }
+    return scriptIterator->isEqualityOp();
   }
-  if (op == JSOP_UNDEFINED) {
-    if ((pc += CodeSpec[op].length) < endpc) {
-      op = JSOp(*pc);
-      return op == JSOP_EQ || op == JSOP_NE || op == JSOP_STRICTEQ ||
-             op == JSOP_STRICTNE;
-    }
-  }
-
   return false;
 }
 
 enum IsNameLookup { NotNameLookup = false, NameLookup = true };
 
 /*
  * Finish getting the property `receiver[id]` after looking at every object on
  * the prototype chain and not finding any such property.