[ddc] Stop modifying the native JavaScript Object prototype

With this change the Dart Core Object members (`.hashCode`, 
`.runtimeType`, `.noSuchMethod()`, `.toString()`, and 
`operator ==`) are no longer installed onto the native JavaScript
Object prototype. This is done because the Object prototype will be 
sealed as a security precaution in some environments to avoid 
prototype pollution exploits.

This means that dispatching to these APIs will change when the 
compiler cannot know if the receiver may be null or a value from 
JavaScript interop. In those cases a call to a helper method is 
inserted instead. The helpers will probe for the API on the value, 
call it if available or execute a default version.

NOTE: Many other native JavaScript prototypes are still modified. This
change is only for the Object prototype.

Issue: https://github.com/dart-lang/sdk/issues/49670

Change-Id: Iddb3a48e790dd414aa3254d729535c4408e99b3d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310971
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>

pkg/dev_compiler/lib/src/kernel/compiler.dart

@@ -966,10 +966,16 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     body = [classDef];
     _emitStaticFieldsAndAccessors(c, body);
     if (finishGenericTypeTest != null) body.add(finishGenericTypeTest);
-    for (var peer in jsPeerNames) {
-      _registerExtensionType(c, peer, body);
+    if (c == _coreTypes.objectClass) {
+      // Avoid polluting the native JavaScript Object prototype with the members
+      // of the Dart Core Object class.
+      // Instead, just assign the identity equals method.
+      body.add(runtimeStatement('_installIdentityEquals()'));
+    } else {
+      for (var peer in jsPeerNames) {
+        _registerExtensionType(c, peer, body);
+      }
     }
-
     _classProperties = savedClassProperties;
     return js_ast.Statement.from(body);
   }
@@ -5269,6 +5275,54 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
         node.receiver, node.interfaceTarget, node.value, node.name.text);
   }
 
+  /// True when the result of evaluating [e] is not known to have the Object
+  /// members installed so a helper method should be called instead of a direct
+  /// instance invocation.
+  ///
+  /// This is a best effort approach determined by the static type information
+  /// and may return `true` when the evaluation result does in fact have the
+  /// members at runtime.
+  bool _shouldCallObjectMemberHelper(Expression e) {
+    if (isNullable(e)) return true;
+    var type = e.getStaticType(_staticTypeContext);
+    if (type is RecordType || type is FunctionType) return false;
+    if (type is InterfaceType) {
+      // TODO(nshahan): This could be expanded to any classes where we know all
+      // implementations at compile time and none of them are JS interop.
+      var cls = type.classNode;
+      // NOTE: This is not guaranteed to always be true. Currently in the SDK
+      // none of the final classes or their subtypes use JavaScript interop.
+      // If that was to ever change, this check will need to be updated.
+      // For now, this is a shortcut since all subclasses of a class are not
+      // immediately accessible.
+      if (cls.isFinal && cls.enclosingLibrary.importUri.isScheme('dart')) {
+        return false;
+      }
+    }
+    // Constants have a static type known at compile time that will not be a
+    // subtype at runtime.
+    return !_triviallyConstNoInterop(e);
+  }
+
+  /// True when [e] is known to evaluate to a constant that has an interface
+  /// type that is not a JavaScript interop type.
+  ///
+  /// This is a simple approach and not an exhaustive search.
+  bool _triviallyConstNoInterop(Expression? e) {
+    if (e is ConstantExpression) {
+      var type = e.constant.getType(_staticTypeContext);
+      if (type is InterfaceType) return !usesJSInterop(type.classNode);
+    } else if (e is StaticGet && e.target.isConst) {
+      var target = e.target;
+      if (target is Field) {
+        return _triviallyConstNoInterop(target.initializer);
+      }
+    } else if (e is VariableGet && e.variable.isConst) {
+      return _triviallyConstNoInterop(e.variable.initializer);
+    }
+    return false;
+  }
+
   js_ast.Expression _emitPropertyGet(
       Expression receiver, Member? member, String memberName) {
     // TODO(jmesserly): should tearoff of `.call` on a function type be
@@ -5286,13 +5340,18 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     // they can be hovered. Unfortunately this is not possible as Kernel does
     // not store this data.
     if (_isObjectMember(memberName)) {
-      if (isNullable(receiver)) {
-        // If the receiver is nullable, use a helper so calls like
-        // `null.hashCode` and `null.runtimeType` will work.
-        // Also method tearoffs like `null.toString`.
+      if (_shouldCallObjectMemberHelper(receiver)) {
         if (_isObjectMethodTearoff(memberName)) {
-          return runtimeCall('bind(#, #)', [jsReceiver, jsName]);
+          if (memberName == 'toString') {
+            return runtimeCall('toStringTearoff(#)', [jsReceiver]);
+          }
+          if (memberName == 'noSuchMethod') {
+            return runtimeCall('noSuchMethodTearoff(#)', [jsReceiver]);
+          }
+          assert(false, 'Unexpected Object method tearoff: $memberName');
         }
+        // The names of the static helper methods in the runtime must match the
+        // names of the Object instance members.
         return runtimeCall('#(#)', [memberName, jsReceiver]);
       }
       // Otherwise generate this as a normal typed property get.
@@ -5541,11 +5600,12 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
 
     var jsName = _emitMemberName(name, member: target);
 
-    // Handle Object methods that are supported by `null`.
+    // Handle Object methods that are supported by `null` and potentially
+    // JavaScript interop values.
     if (_isObjectMethodCall(name, arguments)) {
-      if (isNullable(receiver)) {
-        // If the receiver is nullable, use a helper so calls like
-        // `null.toString()` will work.
+      if (_shouldCallObjectMemberHelper(receiver)) {
+        // The names of the static helper methods in the runtime must match the
+        // names of the Object instance members.
         return runtimeCall('#(#, #)', [name, jsReceiver, args]);
       }
       // Otherwise generate this as a normal typed method call.
@@ -5945,18 +6005,15 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       return _emitCoreIdenticalCall([left, right], negated: negated);
     }
 
-    // If the left side is nullable, we need to use a runtime helper to check
-    // for null. We could inline the null check, but it did not seem to have
-    // a measurable performance effect (possibly the helper is simple enough to
-    // be inlined).
-    if (isNullable(left)) {
+    if (_shouldCallObjectMemberHelper(left)) {
+      // The LHS isn't guaranteed to have an equals method we need to use a
+      // runtime helper.
       return js.call(negated ? '!#' : '#', [
         runtimeCall(
             'equals(#, #)', [_visitExpression(left), _visitExpression(right)])
       ]);
     }
-
-    // Otherwise we emit a call to the == method.
+    // Otherwise it is safe to call the equals method on the LHS directly.
     return js.call(negated ? '!#[#](#)' : '#[#](#)', [
       _visitExpression(left),
       _emitMemberName('==', memberClass: targetClass),
@@ -6883,11 +6940,17 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
         continue;
       }
       var type = e.getStaticType(_staticTypeContext);
-      parts.add(DartTypeEquivalence(_coreTypes, ignoreTopLevelNullability: true)
-                  .areEqual(type, _coreTypes.stringNonNullableRawType) &&
-              !isNullable(e)
-          ? jsExpr
-          : runtimeCall('str(#)', [jsExpr]));
+      if (DartTypeEquivalence(_coreTypes, ignoreTopLevelNullability: true)
+              .areEqual(type, _coreTypes.stringNonNullableRawType) &&
+          !isNullable(e)) {
+        parts.add(jsExpr);
+      } else if (_shouldCallObjectMemberHelper(e)) {
+        parts.add(runtimeCall('str(#)', [jsExpr]));
+      } else {
+        // It is safe to call a version of `str()` that does not probe for the
+        // toString method before calling it.
+        parts.add(runtimeCall('strSafe(#)', [jsExpr]));
+      }
     }
     if (parts.isEmpty) return js.string('');
     return js_ast.Expression.binary(parts, '+');

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/classes.dart

@@ -469,11 +469,12 @@ void _installPropertiesForObject(jsProto) {
   }
 }
 
-void _installPropertiesForGlobalObject(jsProto) {
-  _installPropertiesForObject(jsProto);
-  // Use JS toString for JS objects, rather than the Dart one.
-  JS('', '#[dartx.toString] = function() { return this.toString(); }', jsProto);
-  identityEquals ??= JS('', '#[dartx._equals]', jsProto);
+/// Sets the [identityEquals] method to the equality operator from the Core
+/// Object class.
+///
+/// Only called once by generated code after the Core Object class definition.
+void _installIdentityEquals() {
+  identityEquals ??= JS('', '#.prototype[dartx._equals]', JS_CLASS_REF(Object));
 }
 
 final _extensionMap = JS('', 'new Map()');
@@ -486,10 +487,7 @@ void _applyExtension(jsType, dartExtType) {
   var jsProto = JS<Object?>('', '#.prototype', jsType);
   if (jsProto == null) return;
 
-  if (JS('!', '# === #', dartExtType, JS_CLASS_REF(Object))) {
-    _installPropertiesForGlobalObject(jsProto);
-    return;
-  }
+  if (JS('!', '# === #', dartExtType, JS_CLASS_REF(Object))) return;
 
   if (JS('!', '# === #.Object', jsType, global_)) {
     var extName = JS<String>('!', '#.name', dartExtType);

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart

@@ -1014,61 +1014,170 @@ constFn(x) => JS('', '() => x');
 /// This is inlined by the compiler when used with a literal string.
 extensionSymbol(String name) => JS('', 'dartx[#]', name);
 
-// The following are helpers for Object methods when the receiver
-// may be null. These should only be generated by the compiler.
+/// Helper method for `operator ==` used when the receiver isn't statically
+/// known to have one attached to its prototype (null or a JavaScript interop
+/// value).
+@notNull
 bool equals(x, y) {
   // We handle `y == null` inside our generated operator methods, to keep this
   // function minimal.
   // This pattern resulted from performance testing; it found that dispatching
   // was the fastest solution, even for primitive types.
-  return JS('!', '# == null ? # == null : #[#](#)', x, y, x,
-      extensionSymbol('_equals'), y);
+  if (JS<bool>('!', '# == null', x)) return JS<bool>('!', '# == null', y);
+  var probe = JS('', '#[#]', x, extensionSymbol('_equals'));
+  if (JS<bool>('!', '# !== void 0', probe)) {
+    return JS('!', '#.call(#, #)', probe, x, y);
+  }
+  return JS<bool>('!', '# === #', x, y);
 }
 
+/// Helper method for `.hashCode` used when the receiver isn't statically known
+/// to have one attached to its prototype (null or a JavaScript interop value).
+@notNull
 int hashCode(obj) {
-  return obj == null ? 0 : JS('!', '#[#]', obj, extensionSymbol('hashCode'));
+  if (obj == null) return 0;
+  var probe = JS('', '#[#]', obj, extensionSymbol('hashCode'));
+  if (JS<bool>('!', '# !== void 0', probe)) return JS<int>('!', '#', probe);
+  return identityHashCode(obj);
 }
 
+/// Helper method for `.toString` used when the receiver isn't statically known
+/// to have one attached to its prototype (null or a JavaScript interop value).
 @JSExportName('toString')
+@notNull
 String _toString(obj) {
-  if (obj == null) return "null";
   if (obj is String) return obj;
-  return JS('!', '#[#]()', obj, extensionSymbol('toString'));
+  if (obj == null) return 'null';
+  // If this object has a Dart toString method, call it.
+  var probe = JS('', '#[#]', obj, extensionSymbol('toString'));
+  if (JS<bool>('!', '# !== void 0', probe)) {
+    return JS('', '#.call(#)', probe, obj);
+  }
+  // Otherwise call the native JavaScript toString method.
+  // This differs from dart2js to provide a more useful toString at development
+  // time if one is available.
+  // If obj does not have a native toString method this will throw but that
+  // matches the behavior of dart2js and it would be misleading to make this
+  // work at development time but allow it to fail in production.
+  return JS('', '#.toString()', obj);
+}
+
+/// Helper method to provide a `.toString` tearoff used when the receiver isn't
+/// statically known to have one attached to its prototype (null or a JavaScript
+/// interop value).
+@notNull
+String Function() toStringTearoff(obj) {
+  if (obj == null ||
+      JS<bool>('', '#[#] !== void 0', obj, extensionSymbol('toString'))) {
+    // The bind helper can handle finding the toString method for null or Dart
+    // Objects.
+    return bind(obj, extensionSymbol('toString'), null);
+  }
+  // Otherwise bind the native JavaScript toString method.
+  // This differs from dart2js to provide a more useful toString at development
+  // time if one is available.
+  // If obj does not have a native toString method this will throw but that
+  // matches the behavior of dart2js and it would be misleading to make this
+  // work at development time but allow it to fail in production.
+  return bind(obj, 'toString', null);
 }
 
 /// Converts to a non-null [String], equivalent to
 /// `dart.notNull(dart.toString(obj))`.
 ///
-/// This is commonly used in string interpolation.
+/// Only called from generated code for string interpolation.
 @notNull
 String str(obj) {
-  if (obj == null) return "null";
   if (obj is String) return obj;
-  final result = JS('', '#[#]()', obj, extensionSymbol('toString'));
+  if (obj == null) return "null";
+  var probe = JS('', '#[#]', obj, extensionSymbol('toString'));
   // TODO(40614): Declare `result` as String once non-nullability is sound.
+  final result = JS<bool>('!', '# !== void 0', probe)
+      // If this object has a Dart toString method, call it.
+      ? JS('', '#.call(#)', probe, obj)
+      // Otherwise call the native JavaScript toString method.
+      // This differs from dart2js to provide a more useful toString at
+      // development time if one is available.
+      // If obj does not have a native toString method this will throw but that
+      // matches the behavior of dart2js and it would be misleading to make this
+      // work at development time but allow it to fail in production.
+      : JS('', '#.toString()', obj);
   if (result is String) return result;
   // Since Dart 2.0, `null` is the only other option.
   throw ArgumentError.value(obj, 'object', "toString method returned 'null'");
 }
 
+/// An version of [str] that is optimized for values that we know have the Dart
+/// Core Object members on their prototype chain somewhere so it is safe to
+/// immediately call `.toString()` directly.
+///
+/// Converts to a non-null [String], equivalent to
+/// `dart.notNull(dart.toString(obj))`.
+///
+/// Only called from generated code for string interpolation.
+@notNull
+String strSafe(obj) {
+  // TODO(40614): Declare `result` as String once non-nullability is sound.
+  final result = JS('', '#[#]()', obj, extensionSymbol('toString'));
+  if (result is String) return result;
+  // Since Dart 2.0, `null` is the only other option.
+  throw ArgumentError.value(obj, 'object', "toString method returned 'null'");
+}
+
+/// Helper method for `.noSuchMethod` used when the receiver isn't statically
+/// known to have one attached to its prototype (null or a JavaScript interop
+/// value).
 // TODO(jmesserly): is the argument type verified statically?
+@notNull
 noSuchMethod(obj, Invocation invocation) {
-  if (obj == null) defaultNoSuchMethod(obj, invocation);
+  if (obj == null ||
+      JS<bool>('!', '#[#] == null', obj, extensionSymbol('noSuchMethod'))) {
+    defaultNoSuchMethod(obj, invocation);
+  }
   return JS('', '#[#](#)', obj, extensionSymbol('noSuchMethod'), invocation);
 }
 
+/// Helper method to provide a `.noSuchMethod` tearoff used when the receiver
+/// isn't statically known to have one attached to its prototype (null or a
+/// JavaScript interop value).
+@notNull
+dynamic Function(Invocation) noSuchMethodTearoff(obj) {
+  if (obj == null ||
+      JS<bool>('', '#[#] !== void 0', obj, extensionSymbol('noSuchMethod'))) {
+    // The bind helper can handle finding the toString method for null or Dart
+    // Objects.
+    return bind(obj, extensionSymbol('noSuchMethod'), null);
+  }
+  // Otherwise, manually pass the Dart Core Object noSuchMethod to the bind
+  // helper.
+  return bind(
+      obj,
+      extensionSymbol('noSuchMethod'),
+      JS('!', '#.prototype[#]', JS_CLASS_REF(Object),
+          extensionSymbol('noSuchMethod')));
+}
+
 /// The default implementation of `noSuchMethod` to match `Object.noSuchMethod`.
-defaultNoSuchMethod(obj, Invocation i) {
+Never defaultNoSuchMethod(obj, Invocation i) {
   throw NoSuchMethodError.withInvocation(obj, i);
 }
 
+/// Helper method to provide a `.toString` tearoff used when the receiver isn't
+/// statically known to have one attached to its prototype (null or a JavaScript
+/// interop value).
 // TODO(nshahan) Replace with rti.getRuntimeType() when classes representing
 // native types don't have to "pretend" to be Dart classes. Ex:
 // JSNumber -> int or double
 // JSArray<E> -> List<E>
 // NativeFloat32List -> Float32List
-runtimeType(obj) {
-  return obj == null ? Null : JS('', '#[dartx.runtimeType]', obj);
+@notNull
+Type runtimeType(obj) {
+  if (obj == null) return Null;
+  var probe = JS<Type?>('', '#[#]', obj, extensionSymbol('runtimeType'));
+  if (JS<bool>('!', '# !== void 0', probe)) return JS<Type>('!', '#', probe);
+  return JS_GET_FLAG('NEW_RUNTIME_TYPES')
+      ? rti.createRuntimeType(JS<rti.Rti>('!', '#', getReifiedType(obj)))
+      : wrapType(getReifiedType(obj));
 }
 
 final identityHashCode_ = JS<Object>('!', 'Symbol("_identityHashCode")');

tests/lib/js/object_apis_interop_values_test.dart

@@ -47,6 +47,23 @@ self.JavaScriptClass = class JavaScriptClass {
   testToStringTearoff(emptyObject);
   testEquals(emptyObject, other);
 
+  var objectWithNoProto = eval('Object.create(null)');
+  testHashCode(objectWithNoProto);
+  testRuntimeType(objectWithNoProto);
+  testNoSuchMethod(objectWithNoProto);
+  testNoSuchMethodTearoff(objectWithNoProto);
+  // These operations throwing is only testing for consistency, and does not
+  // imply a choice for the desired behavior. This is simply the state of
+  // JavaScript interop at the time this test was written.
+  Expect.throws(() => objectWithNoProto.toString());
+  Expect.throws(() {
+    // DDC will fail at the point of the tearoff.
+    var toStringTearoff = objectWithNoProto.toString;
+    // Dart2js fails if you call the tearoff.
+    toStringTearoff();
+  });
+  testEquals(objectWithNoProto, other);
+
   var jsNull = eval('null');
   testHashCode(jsNull);
   testRuntimeType(jsNull);

tests/lib/js/static_interop_test/object_apis_static_interop_values_test.dart

@@ -51,6 +51,23 @@ self.JavaScriptClass = class JavaScriptClass {
   testToStringTearoff(emptyObject);
   testEquals(emptyObject, other);
 
+  var objectWithNoProto = eval('Object.create(null)');
+  testHashCode(objectWithNoProto);
+  testRuntimeType(objectWithNoProto);
+  testNoSuchMethod(objectWithNoProto);
+  testNoSuchMethodTearoff(objectWithNoProto);
+  // These operations throwing is only testing for consistency, and does not
+  // imply a choice for the desired behavior. This is simply the state of
+  // JavaScript interop at the time this test was written.
+  Expect.throws(() => objectWithNoProto.toString());
+  Expect.throws(() {
+    // DDC will fail at the point of the tearoff.
+    var toStringTearoff = objectWithNoProto.toString;
+    // Dart2js fails if you call the tearoff.
+    toStringTearoff();
+  });
+  testEquals(objectWithNoProto, other);
+
   var jsNull = eval('null');
   testHashCode(jsNull);
   testRuntimeType(jsNull);