Bug 1304638 - Improve error message for JSMSG_READ_ONLY when we're not in C++ code. r?nbp
MozReview-Commit-ID: 581CP726TzA
--- a/js/src/jit-test/tests/basic/expression-autopsy.js
+++ b/js/src/jit-test/tests/basic/expression-autopsy.js
@@ -95,17 +95,17 @@ check("o[- (o)]");
// A few one off tests
check_one("6", (function () { 6() }), " is not a function");
check_one("0", (function () { Array.prototype.reverse.call('123'); }), " is read-only");
check_one("(intermediate value)[Symbol.iterator](...).next(...).value",
function () { ieval("{ let x; var [a, b, [c0, c1]] = [x, x, x]; }") }, " is undefined");
check_one("void 1", function() { (void 1)(); }, " is not a function");
check_one("void o[1]", function() { var o = []; (void o[1])() }, " is not a function");
-check_one("0", function() {
+check_one("o[0]", function() {
"use strict";
var o = [0];
Object.freeze(o);
o[0] = "foo";
}, " is read-only");
// Manual testing for this case: the only way to trigger an error is *not* on
// an attempted property access during destructuring, and the error message
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -143,31 +143,46 @@ static bool
ErrorTakesObjectArgument(unsigned msg)
{
MOZ_ASSERT(msg < JSErr_Limit);
unsigned argCount = js_ErrorFormatString[msg].argCount;
MOZ_ASSERT(argCount <= 2);
return argCount == 2;
}
+static bool
+LastFrameIsFunctionCall(JSContext* cx)
+{
+ FrameIter iter(cx);
+ jsbytecode* pc = iter.pc();
+ return (JSOp)*pc == JSOP_FUNCALL;
+}
+
JS_PUBLIC_API(bool)
JS::ObjectOpResult::reportStrictErrorOrWarning(JSContext* cx, HandleObject obj, HandleId id,
bool strict)
{
static_assert(unsigned(OkCode) == unsigned(JSMSG_NOT_AN_ERROR),
"unsigned value of OkCode must not be an error code");
MOZ_ASSERT(code_ != Uninitialized);
MOZ_ASSERT(!ok());
unsigned flags = strict ? JSREPORT_ERROR : (JSREPORT_WARNING | JSREPORT_STRICT);
- if (code_ == JSMSG_OBJECT_NOT_EXTENSIBLE || code_ == JSMSG_SET_NON_OBJECT_RECEIVER) {
+
+ // Avoid walking the stack if the error is a read-only error and the last frame is a function
+ // call (that means that the error happened in a C++ function like js::array_reverse), since it
+ // produces unintuitive error messages. We fall back to only formatting the property name in
+ // that case.
+ if (code_ == JSMSG_OBJECT_NOT_EXTENSIBLE || code_ == JSMSG_SET_NON_OBJECT_RECEIVER ||
+ (code_ == JSMSG_READ_ONLY && !LastFrameIsFunctionCall(cx))) {
RootedValue val(cx, ObjectValue(*obj));
return ReportValueErrorFlags(cx, flags, code_, JSDVG_IGNORE_STACK, val,
nullptr, nullptr, nullptr);
}
+
if (ErrorTakesArguments(code_)) {
RootedValue idv(cx, IdToValue(id));
RootedString str(cx, ValueToSource(cx, idv));
if (!str)
return false;
JSAutoByteString propName;
if (!propName.encodeUtf8(cx, str))
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -1242,19 +1242,21 @@ ExpressionDecompiler::decompilePC(jsbyte
}
case JSOP_GETALIASEDVAR: {
JSAtom* atom = EnvironmentCoordinateName(cx->caches.envCoordinateNameCache, script, pc);
MOZ_ASSERT(atom);
return write(atom);
}
case JSOP_LENGTH:
case JSOP_GETPROP:
+ case JSOP_SETPROP:
case JSOP_CALLPROP: {
RootedAtom prop(cx, (op == JSOP_LENGTH) ? cx->names().length : loadAtom(pc));
- if (!decompilePCForStackOperand(pc, -1))
+ int objIndex = (op == JSOP_SETPROP) ? -2 : -1;
+ if (!decompilePCForStackOperand(pc, objIndex))
return false;
if (IsIdentifier(prop)) {
return write(".") &&
quote(prop, '\0');
}
return write("[") &&
quote(prop, '\'') &&
write("]");
--- a/js/src/tests/ecma_3/Object/8.6.1-01.js
+++ b/js/src/tests/ecma_3/Object/8.6.1-01.js
@@ -10,17 +10,17 @@ var summary = 'In strict mode, setting a
printBugNumber(BUGNUMBER);
printStatus (summary);
enterFunc (String (BUGNUMBER));
// should throw an error in strict mode
var actual = '';
-var expect = '"length" is read-only';
+var expect = 's.length is read-only';
var status = summary + ': Throw if STRICT and WERROR is enabled';
if (!options().match(/strict/))
{
options('strict');
}
if (!options().match(/werror/))
{