mirror of
https://github.com/dart-lang/sdk
synced 2024-11-02 10:49:00 +00:00
[VM/nnbd] Eliminate two VM crashes when running strong mode tests.
Using strong mode with a mix of opted in and opted out libraries may result in a static call with bad arguments (missing required named argument) that may crash the VM with an assert fault. Ideally, we should emit code to throw a NoSuchMethodError when the code is executed. For now, we report the error as an informative exception during compilation instead of aborting. The name of the enclosing function needs to be canonical when creating an instance of _FallThroughError when compiling a switch statement. Change-Id: I50349fcc5b5036fbdec4697238b1e840de15ec07 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144498 Commit-Queue: Régis Crelier <regis@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
parent
a36ec667e6
commit
157d567358
2 changed files with 20 additions and 13 deletions
|
@ -1714,16 +1714,23 @@ Fragment StreamingFlowGraphBuilder::CheckNull(
|
|||
clear_the_temp);
|
||||
}
|
||||
|
||||
static void BadArity() {
|
||||
static void BadArity(const Script& script,
|
||||
TokenPosition position,
|
||||
const String& error_message) {
|
||||
#ifndef PRODUCT
|
||||
// TODO(https://github.com/dart-lang/sdk/issues/37517): Should emit code to
|
||||
// throw a NoSuchMethodError.
|
||||
ASSERT(Isolate::Current()->HasAttemptedReload());
|
||||
Report::LongJump(LanguageError::Handle(LanguageError::New(String::Handle(
|
||||
String::New("Unimplemented handling of static target arity change")))));
|
||||
#else
|
||||
UNREACHABLE();
|
||||
// Correct arity is checked at compile time by CFE. However, an isolate reload
|
||||
// after an arity change may lead here.
|
||||
// Using strong mode with a mix of opted in and opted out libraries may
|
||||
// also result in this error undetected by CFE.
|
||||
Isolate* isolate = Isolate::Current();
|
||||
ASSERT(isolate->HasAttemptedReload() || isolate->null_safety());
|
||||
Report::MessageF(Report::kError, script, position, Report::AtLocation,
|
||||
"Static call with invalid arguments: %s",
|
||||
error_message.ToCString());
|
||||
#endif
|
||||
UNREACHABLE();
|
||||
}
|
||||
|
||||
Fragment StreamingFlowGraphBuilder::StaticCall(TokenPosition position,
|
||||
|
@ -1732,7 +1739,7 @@ Fragment StreamingFlowGraphBuilder::StaticCall(TokenPosition position,
|
|||
ICData::RebindRule rebind_rule) {
|
||||
String& error_message = String::Handle();
|
||||
if (!target.AreValidArgumentCounts(0, argument_count, 0, &error_message)) {
|
||||
BadArity();
|
||||
BadArity(script_, position, error_message);
|
||||
}
|
||||
return flow_graph_builder_->StaticCall(position, target, argument_count,
|
||||
rebind_rule);
|
||||
|
@ -1750,7 +1757,7 @@ Fragment StreamingFlowGraphBuilder::StaticCall(
|
|||
String& error_message = String::Handle();
|
||||
if (!target.AreValidArguments(type_args_count, argument_count, argument_names,
|
||||
&error_message)) {
|
||||
BadArity();
|
||||
BadArity(script_, position, error_message);
|
||||
}
|
||||
return flow_graph_builder_->StaticCall(
|
||||
position, target, argument_count, argument_names, rebind_rule,
|
||||
|
@ -3365,7 +3372,8 @@ Fragment StreamingFlowGraphBuilder::BuildStaticInvocation(TokenPosition* p) {
|
|||
instructions +=
|
||||
BuildArguments(&argument_names, NULL /* arg count */,
|
||||
NULL /* positional arg count */); // read arguments.
|
||||
ASSERT(target.AreValidArguments(type_args_len, argument_count, argument_names,
|
||||
ASSERT(!special_case ||
|
||||
target.AreValidArguments(type_args_len, argument_count, argument_names,
|
||||
NULL));
|
||||
|
||||
// Special case identical(x, y) call.
|
||||
|
@ -4497,9 +4505,8 @@ Fragment StreamingFlowGraphBuilder::BuildSwitchStatement() {
|
|||
Z, klass.LookupConstructorAllowPrivate(String::ZoneHandle(
|
||||
Z, Symbols::FromConcatAll(H.thread(), pieces))));
|
||||
ASSERT(!constructor.IsNull());
|
||||
const String& url = H.DartString(
|
||||
parsed_function()->function().ToLibNamePrefixedQualifiedCString(),
|
||||
Heap::kOld);
|
||||
const String& url = H.DartSymbolPlain(
|
||||
parsed_function()->function().ToLibNamePrefixedQualifiedCString());
|
||||
|
||||
// Create instance of _FallThroughError
|
||||
body_fragment += AllocateObject(TokenPosition::kNoSource, klass, 0);
|
||||
|
|
|
@ -4516,7 +4516,7 @@ TEST_CASE(IsolateReload_StaticTargetArityChange) {
|
|||
lib = TestCase::ReloadTestScript(kReloadScript);
|
||||
EXPECT_VALID(lib);
|
||||
EXPECT_ERROR(SimpleInvokeError(lib, "main"),
|
||||
"Unimplemented handling of static target arity change");
|
||||
"Static call with invalid arguments");
|
||||
}
|
||||
|
||||
#endif // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
|
||||
|
|
Loading…
Reference in a new issue