Bug 1100623 - Fix expression decompiler reporting the wrong local, get rid of FillBindingVector. r=luke
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug1100623.js
@@ -0,0 +1,12 @@
+// |jit-test| error: baz is null
+var document = {getElementById: () => null};
+
+(function() {
+ const one = 1;
+
+ function foo() { return one; }
+ function bar() { return foo(); }
+
+ var baz = document.getElementById("baz");
+ baz.value;
+})();
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -784,23 +784,17 @@ FormatFrame(JSContext *cx, const ScriptF
buf = JS_sprintf_append(buf, "%d anonymous(", num);
} else {
buf = JS_sprintf_append(buf, "%d <TOP LEVEL>", num);
}
if (!buf)
return buf;
if (showArgs && iter.hasArgs()) {
- BindingVector bindings(cx);
- if (fun && fun->isInterpreted()) {
- if (!FillBindingVector(script, &bindings))
- return buf;
- }
-
-
+ BindingIter bi(script);
bool first = true;
for (unsigned i = 0; i < iter.numActualArgs(); i++) {
RootedValue arg(cx);
if (i < iter.numFormalArgs() && script->formalIsAliased(i)) {
for (AliasedFormalIter fi(script); ; fi++) {
if (fi.frameIndex() == i) {
arg = iter.callObj().aliasedVar(fi);
break;
@@ -813,20 +807,22 @@ FormatFrame(JSContext *cx, const ScriptF
}
JSAutoByteString valueBytes;
const char *value = FormatValue(cx, arg, valueBytes);
JSAutoByteString nameBytes;
const char *name = nullptr;
- if (i < bindings.length()) {
- name = nameBytes.encodeLatin1(cx, bindings[i].name());
+ if (i < iter.numFormalArgs()) {
+ MOZ_ASSERT(i == bi.argIndex());
+ name = nameBytes.encodeLatin1(cx, bi->name());
if (!buf)
return nullptr;
+ bi++;
}
if (value) {
buf = JS_sprintf_append(buf, "%s%s%s%s%s%s",
!first ? ", " : "",
name ? name :"",
name ? " = " : "",
arg.isString() ? "\"" : "",
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1064,26 +1064,27 @@ js::FunctionToString(JSContext *cx, Hand
// This function was created with the Function constructor. We don't
// have source for the arguments, so we have to generate that. Part
// of bug 755821 should be cobbling the arguments passed into the
// Function constructor into the source string.
if (!out.append("("))
return nullptr;
// Fish out the argument names.
- BindingVector *localNames = cx->new_<BindingVector>(cx);
- ScopedJSDeletePtr<BindingVector> freeNames(localNames);
- if (!FillBindingVector(script, localNames))
- return nullptr;
- for (unsigned i = 0; i < fun->nargs(); i++) {
- if ((i && !out.append(", ")) ||
- (i == unsigned(fun->nargs() - 1) && fun->hasRest() && !out.append("...")) ||
- !out.append((*localNames)[i].name())) {
+ MOZ_ASSERT(script->bindings.numArgs() == fun->nargs());
+
+ BindingIter bi(script);
+ for (unsigned i = 0; i < fun->nargs(); i++, bi++) {
+ MOZ_ASSERT(bi.argIndex() == i);
+ if (i && !out.append(", "))
return nullptr;
- }
+ if (i == unsigned(fun->nargs() - 1) && fun->hasRest() && !out.append("..."))
+ return nullptr;
+ if (!out.append(bi->name()))
+ return nullptr;
}
if (!out.append(") {\n"))
return nullptr;
}
if ((bodyOnly && !funCon) || addUseStrict) {
// We need to get at the body either because we're only supposed to
// return the body or we need to insert "use strict" into the body.
size_t bodyStart = 0, bodyEnd;
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -1461,32 +1461,28 @@ namespace {
* 5". If we need to decompile a subexpression, we call decompilePC (step 2)
* recursively on the operands' pcs. The result is a depth-first traversal of
* the expression tree.
*
*/
struct ExpressionDecompiler
{
JSContext *cx;
- InterpreterFrame *fp;
RootedScript script;
RootedFunction fun;
- BindingVector *localNames;
BytecodeParser parser;
Sprinter sprinter;
ExpressionDecompiler(JSContext *cx, JSScript *script, JSFunction *fun)
: cx(cx),
script(cx, script),
fun(cx, fun),
- localNames(nullptr),
parser(cx, script),
sprinter(cx)
{}
- ~ExpressionDecompiler();
bool init();
bool decompilePCForStackOperand(jsbytecode *pc, int i);
bool decompilePC(jsbytecode *pc);
JSAtom *getLocal(uint32_t local, jsbytecode *pc);
JSAtom *getArg(unsigned slot);
JSAtom *loadAtom(jsbytecode *pc);
bool quote(JSString *s, uint32_t quote);
bool write(const char *s);
@@ -1626,36 +1622,24 @@ ExpressionDecompiler::decompilePC(jsbyte
return write(str);
}
default:
break;
}
return write("(intermediate value)");
}
-ExpressionDecompiler::~ExpressionDecompiler()
-{
- js_delete<BindingVector>(localNames);
-}
-
bool
ExpressionDecompiler::init()
{
assertSameCompartment(cx, script);
if (!sprinter.init())
return false;
- localNames = cx->new_<BindingVector>(cx);
- if (!localNames)
- return false;
- RootedScript script_(cx, script);
- if (!FillBindingVector(script_, localNames))
- return false;
-
if (!parser.parse())
return false;
return true;
}
bool
ExpressionDecompiler::write(const char *s)
@@ -1680,33 +1664,43 @@ ExpressionDecompiler::loadAtom(jsbytecod
{
return script->getAtom(GET_UINT32_INDEX(pc));
}
JSAtom *
ExpressionDecompiler::getArg(unsigned slot)
{
MOZ_ASSERT(fun);
- MOZ_ASSERT(slot < script->bindings.count());
- return (*localNames)[slot].name();
+ MOZ_ASSERT(slot < script->bindings.numArgs());
+
+ for (BindingIter bi(script); bi; bi++) {
+ MOZ_ASSERT(bi->kind() == Binding::ARGUMENT);
+ if (bi.argIndex() == slot)
+ return bi->name();
+ }
+
+ MOZ_CRASH("No binding");
}
JSAtom *
ExpressionDecompiler::getLocal(uint32_t local, jsbytecode *pc)
{
MOZ_ASSERT(local < script->nfixed());
if (local < script->nbodyfixed()) {
- MOZ_ASSERT(fun);
- uint32_t slot = local + fun->nargs();
- MOZ_ASSERT(slot < script->bindings.count());
- return (*localNames)[slot].name();
+ for (BindingIter bi(script); bi; bi++) {
+ if (bi->kind() != Binding::ARGUMENT && !bi->aliased() && bi.frameIndex() == local)
+ return bi->name();
+ }
+
+ MOZ_CRASH("No binding");
}
for (NestedScopeObject *chain = script->getStaticScope(pc);
chain;
- chain = chain->enclosingNestedScope()) {
+ chain = chain->enclosingNestedScope())
+ {
if (!chain->is<StaticBlockObject>())
continue;
StaticBlockObject &block = chain->as<StaticBlockObject>();
if (local < block.localOffset())
continue;
local -= block.localOffset();
if (local >= block.numVariables())
return nullptr;
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -329,27 +329,16 @@ Bindings::trace(JSTracer *trc)
return;
for (Binding *b = bindingArray(), *end = b + count(); b != end; b++) {
PropertyName *name = b->name();
MarkStringUnbarriered(trc, &name, "bindingArray");
}
}
-bool
-js::FillBindingVector(HandleScript fromScript, BindingVector *vec)
-{
- for (BindingIter bi(fromScript); bi; bi++) {
- if (!vec->append(*bi))
- return false;
- }
-
- return true;
-}
-
template<XDRMode mode>
bool
js::XDRScriptConst(XDRState<mode> *xdr, MutableHandleValue vp)
{
JSContext *cx = xdr->cx();
/*
* A script constant can be an arbitrary primitive value as they are used
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -1749,26 +1749,31 @@ class BindingIter
void operator++(int) {
MOZ_ASSERT(!done());
const Binding &binding = **this;
if (binding.kind() != Binding::ARGUMENT && !binding.aliased())
unaliasedLocal_++;
i_++;
}
- // Stack slots are assigned to arguments and unaliased locals. frameIndex()
- // returns the slot index. It's invalid to call this method when the
- // iterator is stopped on an aliased local, as it has no stack slot.
+ // Stack slots are assigned to arguments (aliased and unaliased) and
+ // unaliased locals. frameIndex() returns the slot index. It's invalid to
+ // call this method when the iterator is stopped on an aliased local, as it
+ // has no stack slot.
uint32_t frameIndex() const {
MOZ_ASSERT(!done());
if (i_ < bindings_->numArgs())
return i_;
MOZ_ASSERT(!(*this)->aliased());
return unaliasedLocal_;
}
+
+ // If the current binding is an argument, argIndex() returns its index.
+ // It returns the same value as frameIndex(), as slots are allocated for
+ // both unaliased and aliased arguments.
uint32_t argIndex() const {
MOZ_ASSERT(!done());
MOZ_ASSERT(i_ < bindings_->numArgs());
return i_;
}
uint32_t argOrLocalIndex() const {
MOZ_ASSERT(!done());
return i_ < bindings_->numArgs() ? i_ : i_ - bindings_->numArgs();
@@ -1784,26 +1789,16 @@ class BindingIter
return binding.kind() != Binding::ARGUMENT;
}
const Binding &operator*() const { MOZ_ASSERT(!done()); return bindings_->bindingArray()[i_]; }
const Binding *operator->() const { MOZ_ASSERT(!done()); return &bindings_->bindingArray()[i_]; }
};
/*
- * This helper function fills the given BindingVector with the sequential
- * values of BindingIter.
- */
-
-typedef Vector<Binding, 32> BindingVector;
-
-extern bool
-FillBindingVector(HandleScript fromScript, BindingVector *vec);
-
-/*
* Iterator over the aliased formal bindings in ascending index order. This can
* be veiwed as a filtering of BindingIter with predicate
* bi->aliased() && bi->kind() == Binding::ARGUMENT
*/
class AliasedFormalIter
{
const Binding *begin_, *p_, *end_;
unsigned slot_;
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -6030,25 +6030,24 @@ DebuggerObject_getParameterNames(JSConte
if (fun->isInterpreted()) {
RootedScript script(cx, GetOrCreateFunctionScript(cx, fun));
if (!script)
return false;
MOZ_ASSERT(fun->nargs() == script->bindings.numArgs());
if (fun->nargs() > 0) {
- BindingVector bindings(cx);
- if (!FillBindingVector(script, &bindings))
- return false;
- for (size_t i = 0; i < fun->nargs(); i++) {
+ BindingIter bi(script);
+ for (size_t i = 0; i < fun->nargs(); i++, bi++) {
+ MOZ_ASSERT(bi.argIndex() == i);
Value v;
- if (bindings[i].name()->length() == 0)
+ if (bi->name()->length() == 0)
v = UndefinedValue();
else
- v = StringValue(bindings[i].name());
+ v = StringValue(bi->name());
result->setDenseElement(i, v);
}
}
} else {
for (size_t i = 0; i < fun->nargs(); i++)
result->setDenseElement(i, UndefinedValue());
}
--- a/js/src/vm/ForkJoin.cpp
+++ b/js/src/vm/ForkJoin.cpp
@@ -1102,49 +1102,45 @@ ForkJoinOperation::reportBailoutWarnings
sp.printf("<anonymous>");
}
} else {
sp.printf("<global>");
}
sp.printf(" (%s:%u)", script->filename(), PCToLineNumber(script, frame->pc()));
// Format bindings.
- BindingVector bindings(cx_);
- if (!FillBindingVector(script, &bindings))
- return false;
-
unsigned scopeSlot = 0;
- for (unsigned i = 0; i < bindings.length(); i++) {
+ for (BindingIter bi(script); bi; bi++) {
JSAutoByteString nameBytes;
const char *nameChars = nullptr;
- RootedPropertyName bindingName(cx_, bindings[i].name());
+ RootedPropertyName bindingName(cx_, bi->name());
nameChars = nameBytes.encodeUtf8(cx_, bindingName);
if (!nameChars)
return false;
RootedValue arg(cx_);
- if (bindings[i].aliased()) {
+ if (bi->aliased()) {
arg = frame->callObj().getSlot(scopeSlot);
scopeSlot++;
- } else if (i < frame->numFormalArgs()) {
+ } else if (bi->kind() == Binding::ARGUMENT) {
if (script->argsObjAliasesFormals() && frame->hasArgsObj())
- arg = frame->argsObj().arg(i);
+ arg = frame->argsObj().arg(bi.argIndex());
else
- arg = frame->unaliasedActual(i, DONT_CHECK_ALIASING);
+ arg = frame->unaliasedActual(bi.argIndex(), DONT_CHECK_ALIASING);
} else {
- arg = frame->unaliasedLocal(i - frame->numFormalArgs());
+ arg = frame->unaliasedLocal(bi.frameIndex());
}
JSAutoByteString valueBytes;
const char *valueChars = ValueToChar(cx_, arg, valueBytes);
if (!valueChars)
return false;
sp.printf("\n %s %s = %s",
- bindings[i].kind() == Binding::ARGUMENT ? "arg" : "var",
+ bi->kind() == Binding::ARGUMENT ? "arg" : "var",
nameChars, valueChars);
}
}
}
}
if (SpewEnabled(SpewBailouts))
JS_ReportWarning(cx_, "%s", sp.string());