Commit ac73e1d5 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

Reland "[test] Check for illegal uses of mjsunit methods"

This is a reland of a9e93572

Original change's description:
> [test] Check for illegal uses of mjsunit methods
> 
> The assertThrows and assertDoesNotThrow methods expect either a
> function to execute, or a string to eval. In several tests however we
> accidentally passed the *result* of the statement to be tested instead
> of the code.
> This CL adds check to catch such error early, and removes wrong uses.
> In most places, we do not need to use assertDoesNotThrow anyway,
> because exceptions are handled as test failures.
> 
> Drive-by: Unify catch syntax in mjsunit.js and make sure to propagate
> MjsUnitAssertionErrors correctly.
> 
> R=mathias@chromium.org
> 
> Bug: v8:8562
> Change-Id: I88894a667cbe0570774f748a9a23e8a527887a49
> Reviewed-on: https://chromium-review.googlesource.com/c/1439238
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#59277}

Bug: v8:8562
Change-Id: I3b26935f7b35302d499266155273ea271bf8151d
Reviewed-on: https://chromium-review.googlesource.com/c/1449792Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59328}
parent 5fc71d7b
......@@ -213,7 +213,7 @@ var prettyPrinted;
// TODO(neis): Remove try-catch once BigInts are enabled by default.
try {
BigIntPrototypeValueOf = BigInt.prototype.valueOf;
} catch(e) {}
} catch (e) {}
function classOf(object) {
// Argument must not be null or undefined.
......@@ -480,14 +480,17 @@ var prettyPrinted;
}
};
function executeCode(code) {
if (typeof code === 'function') return code();
if (typeof code === 'string') return eval(code);
failWithMessage(
'Given code is neither function nor string, but ' + (typeof code) +
': <' + prettyPrinted(code) + '>');
}
assertThrows = function assertThrows(code, type_opt, cause_opt) {
try {
if (typeof code === 'function') {
code();
} else {
eval(code);
}
executeCode(code);
} catch (e) {
if (typeof type_opt === 'function') {
assertInstanceof(e, type_opt);
......@@ -508,11 +511,10 @@ var prettyPrinted;
failWithMessage("Did not throw exception");
};
assertThrowsEquals = function assertThrowsEquals(fun, val) {
try {
fun();
} catch(e) {
} catch (e) {
assertSame(val, e);
return;
}
......@@ -533,15 +535,11 @@ var prettyPrinted;
}
};
assertDoesNotThrow = function assertDoesNotThrow(code, name_opt) {
assertDoesNotThrow = function assertDoesNotThrow(code, name_opt) {
try {
if (typeof code === 'function') {
return code();
} else {
return eval(code);
}
executeCode(code);
} catch (e) {
if (e instanceof MjsUnitAssertionError) throw e;
failWithMessage("threw an exception: " + (e.message || e));
}
};
......@@ -669,7 +667,9 @@ var prettyPrinted;
// option is provided. Such tests must add --opt to flags comment.
assertFalse((opt_status & V8OptimizationStatus.kNeverOptimize) !== 0,
"test does not make sense with --no-opt");
assertTrue((opt_status & V8OptimizationStatus.kIsFunction) !== 0, name_opt);
assertTrue(
(opt_status & V8OptimizationStatus.kIsFunction) !== 0,
'should be a function: ' + name_opt);
if (skip_if_maybe_deopted &&
(opt_status & V8OptimizationStatus.kMaybeDeopted) !== 0) {
// When --deopt-every-n-times flag is specified it's no longer guaranteed
......@@ -677,7 +677,9 @@ var prettyPrinted;
// to stress test the deoptimizer.
return;
}
assertTrue((opt_status & V8OptimizationStatus.kOptimized) !== 0, name_opt);
assertTrue(
(opt_status & V8OptimizationStatus.kOptimized) !== 0,
'should be optimized: ' + name_opt);
}
isNeverOptimizeLiteMode = function isNeverOptimizeLiteMode() {
......@@ -774,7 +776,7 @@ var prettyPrinted;
return frame;
});
return "" + error.message + "\n" + ArrayPrototypeJoin.call(stack, "\n");
} catch(e) {};
} catch (e) {};
return error.stack;
}
})();
......@@ -33,7 +33,7 @@ var p = "floor";
function test() {
var bignumber = 31363200000;
assertDoesNotThrow(assertEquals(m[p](Math.round(bignumber/864E5)/7)+1, 52));
assertEquals(m[p](Math.round(bignumber/864E5)/7)+1, 52);
}
test();
......@@ -29,9 +29,8 @@
// update a accessor property to a data property using Object.defineProperty.
var obj = { get value() {}, set value (v) { throw "Error";} };
assertDoesNotThrow(
Object.defineProperty(obj, "value",
{ value: 5, writable:true, configurable: true }));
Object.defineProperty(obj, "value",
{ value: 5, writable:true, configurable: true });
var desc = Object.getOwnPropertyDescriptor(obj, "value");
assertEquals(obj.value, 5);
assertTrue(desc.configurable);
......@@ -49,7 +48,7 @@ var proto = {
var create = Object.create(proto);
assertEquals(create.value, undefined);
assertDoesNotThrow(create.value = 4);
create.value = 4;
assertEquals(create.value, 4);
// These tests where provided in bug 959, but are all related to the this issue.
......
......@@ -20,6 +20,6 @@ for (var i = 0; i < test_set.length; ++i) {
src = src.replace(/MODULE/g, "Module" + i);
src = src.replace(/LIMIT/g, test_set[i]);
var module = eval("(" + src + ")");
assertDoesNotThrow(module(this).f());
module(this).f();
assertFalse(%IsAsmWasmCode(module));
}
......@@ -24,8 +24,7 @@ function instance(bytes, imports = {}) {
// instantiate should succeed but run should fail.
function instantiateAndFailAtRuntime(bytes, imports = {}) {
var instance =
assertDoesNotThrow(new WebAssembly.Instance(module(bytes), imports));
var instance = new WebAssembly.Instance(module(bytes), imports);
instance.exports.run();
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment