From 3fedb0508ca2a07550d310b073469b5a30aabef8 Mon Sep 17 00:00:00 2001 From: Will Hawkins Date: Sat, 14 Sep 2019 17:05:23 +0000 Subject: [PATCH] Bug 1566057: Convert Detecting function to use bytecode iterator. r=tcampbell Differential Revision: https://phabricator.services.mozilla.com/D42611 --HG-- extra : moz-landing-system : lando --- js/src/vm/BytecodeLocation-inl.h | 6 +++ js/src/vm/BytecodeLocation.h | 14 ++++++ js/src/vm/BytecodeUtil.h | 4 ++ js/src/vm/NativeObject.cpp | 80 ++++++++++++++++++-------------- 4 files changed, 70 insertions(+), 34 deletions(-) diff --git a/js/src/vm/BytecodeLocation-inl.h b/js/src/vm/BytecodeLocation-inl.h index b4c3f51912a7..d01ed4dc2c5f 100644 --- a/js/src/vm/BytecodeLocation-inl.h +++ b/js/src/vm/BytecodeLocation-inl.h @@ -26,6 +26,12 @@ 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 diff --git a/js/src/vm/BytecodeLocation.h b/js/src/vm/BytecodeLocation.h index f472d1a976e6..25aef0abe995 100644 --- a/js/src/vm/BytecodeLocation.h +++ b/js/src/vm/BytecodeLocation.h @@ -14,6 +14,8 @@ namespace js { typedef uint32_t RawBytecodeLocationOffset; +class PropertyName; + class BytecodeLocationOffset { RawBytecodeLocationOffset rawOffset_; @@ -69,6 +71,8 @@ class BytecodeLocation { 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_; @@ -120,6 +124,16 @@ class BytecodeLocation { 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_); } diff --git a/js/src/vm/BytecodeUtil.h b/js/src/vm/BytecodeUtil.h index 1296b98e951a..6dfa0261a8d8 100644 --- a/js/src/vm/BytecodeUtil.h +++ b/js/src/vm/BytecodeUtil.h @@ -535,6 +535,10 @@ 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; diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 44761fbce21e..6f51f2482778 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -21,6 +21,7 @@ #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" @@ -2333,56 +2334,67 @@ bool js::NativeGetExistingProperty(JSContext* cx, HandleObject receiver, /* * 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); + BytecodeIterator scriptIterator = + BytecodeIterator(BytecodeLocation(script, pc)); + BytecodeIterator endIter = BytecodeIterator(script->endLocation()); - MOZ_ASSERT(script->containsPC(pc)); - if (pc >= script->codeEnd()) { - return false; + // 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; } - } - 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; + // 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(); } - return false; }