[ddc] Add nullability assertions for native values
Bug: https://github.com/dart-lang/sdk/issues/42535
Adds a runtime flag to ddc to enable runtime checks on native APIs.
In the case where an API is typed non-nullable but returns a null
value, throws a null assertion error.
Change-Id: I4d5d7529ba28d9308687dad5d51f1b9c71274455
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/162461
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart
index d6ee556..f4d1611 100644
--- a/pkg/dev_compiler/lib/src/kernel/compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart
@@ -4200,8 +4200,14 @@
@override
js_ast.Expression visitPropertyGet(PropertyGet node) {
- return _emitPropertyGet(
- node.receiver, node.interfaceTarget, node.name.text);
+ var propertyGet =
+ _emitPropertyGet(node.receiver, node.interfaceTarget, node.name.text);
+ if (_isCheckableNative(node.interfaceTarget)) {
+ // If target is a native getter with a non-nullable type, add a null check
+ // for soundness.
+ return runtimeCall('checkNativeNonNull(#)', [propertyGet]);
+ }
+ return propertyGet;
}
@override
@@ -4264,6 +4270,16 @@
}
}
+ /// Return whether [member] returns a native object whose type needs to be
+ /// checked.
+ bool _isCheckableNative(Member member) =>
+ member != null &&
+ member.isExternal &&
+ _extensionTypes.isNativeClass(member.enclosingClass) &&
+ member is Procedure &&
+ member.function != null &&
+ member.function.returnType.isPotentiallyNonNullable;
+
// TODO(jmesserly): can we encapsulate REPL name lookups and remove this?
// _emitMemberName would be a nice place to handle it, but we don't have
// access to the target expression there (needed for `dart.replNameLookup`).
@@ -4334,8 +4350,14 @@
@override
js_ast.Expression visitMethodInvocation(MethodInvocation node) {
- return _emitMethodCall(
+ var methodCall = _emitMethodCall(
node.receiver, node.interfaceTarget, node.arguments, node);
+ if (_isCheckableNative(node.interfaceTarget)) {
+ // If target is a native method with a non-nullable type, add a null check
+ // for soundness.
+ return runtimeCall('checkNativeNonNull(#)', [methodCall]);
+ }
+ return methodCall;
}
@override
@@ -5066,11 +5088,30 @@
var result = js.parseForeignJS(source).instantiate(jsArgs);
+ // Add a check to make sure any JS() values from a native type are typed
+ // properly.
+ if (_isWebLibrary(_currentLibrary.importUri)) {
+ var type = node.getStaticType(_staticTypeContext);
+ if (type.isPotentiallyNonNullable) {
+ result = runtimeCall('checkNativeNonNull(#)', [result]);
+ }
+ }
+
assert(result is js_ast.Expression ||
result is js_ast.Statement && node.parent is ExpressionStatement);
return result;
}
+ bool _isWebLibrary(Uri importUri) =>
+ importUri.scheme == 'dart' &&
+ (importUri.path == 'html' ||
+ importUri.path == 'svg' ||
+ importUri.path == 'indexed_db' ||
+ importUri.path == 'web_audio' ||
+ importUri.path == 'web_gl' ||
+ importUri.path == 'web_sql' ||
+ importUri.path == 'html_common');
+
bool _isNull(Expression expr) =>
expr is NullLiteral ||
expr.getStaticType(_staticTypeContext) == _coreTypes.nullType;
diff --git a/pkg/dev_compiler/tool/ddb b/pkg/dev_compiler/tool/ddb
index b2cd5ce..126090a 100755
--- a/pkg/dev_compiler/tool/ddb
+++ b/pkg/dev_compiler/tool/ddb
@@ -54,6 +54,11 @@
'parameters are not null.',
defaultsTo: false,
negatable: true)
+ ..addFlag('native-null-assertions',
+ help: 'Run with assertions on non-nullable values returned from native '
+ 'APIs.',
+ defaultsTo: true,
+ negatable: true)
..addFlag('observe',
help:
'Run the compiler in the Dart VM with --observe. Implies --debug.',
@@ -109,6 +114,7 @@
var run = mode == 'run' || mode == 'all';
var verbose = options['verbose'] as bool;
var nonNullAsserts = options['null-assertions'] as bool;
+ var nativeNonNullAsserts = options['null-assertions'] as bool;
var soundNullSafety = options['sound-null-safety'] as bool;
// Enable null safety either by passing the `non-nullable` experiment flag or
@@ -280,6 +286,7 @@
if ($nnbd) {
sdk.dart.weakNullSafetyWarnings(!$soundNullSafety);
sdk.dart.nonNullAsserts($nonNullAsserts);
+ sdk.dart.nativeNonNullAsserts($nativeNonNullAsserts);
}
sdk._debugger.registerDevtoolsFormatter();
app.$libname.main([]);
@@ -315,6 +322,7 @@
if ($nnbd) {
sdk.dart.weakNullSafetyWarnings(!$soundNullSafety);
sdk.dart.nonNullAsserts($nonNullAsserts);
+ sdk.dart.nativeNonNullAsserts($nativeNonNullAsserts);
}
sdk._isolate_helper.startRootIsolate(main, []);
} catch(e) {
@@ -351,6 +359,7 @@
if ($nnbd) {
dart.weakNullSafetyWarnings(!$soundNullSafety);
dart.nonNullAsserts($nonNullAsserts);
+ dart.nativeNonNullAsserts($nativeNonNullAsserts);
}
_isolate_helper.startRootIsolate(() => {}, []);
main([]);
diff --git a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart
index d931293..6fcd3d1 100644
--- a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart
+++ b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart
@@ -870,3 +870,10 @@
}
return ${defineProperty(to, name, desc)};
})()''');
+
+checkNativeNonNull(dynamic variable) {
+ if (_nativeNonNullAsserts) {
+ return nullCheck(variable);
+ }
+ return variable;
+}
diff --git a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
index 8dd643c..acb1615 100644
--- a/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
+++ b/sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart
@@ -44,6 +44,15 @@
_nonNullAsserts = enable;
}
+@notNull
+bool _nativeNonNullAsserts = false;
+
+/// Enables null assertions on native APIs to make sure value returned from the
+/// browser is sound. These apply to dart:html and similar web libraries.
+void nativeNonNullAsserts(bool enable) {
+ _nativeNonNullAsserts = enable;
+}
+
final metadata = JS('', 'Symbol("metadata")');
/// Types in dart are represented internally at runtime as follows.
