Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ddc] Patching the native Object prorotype causes confusing internal representation issues #49670

Open
nshahan opened this issue Aug 15, 2022 · 0 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P3 A lower priority bug or feature request web-dev-compiler web-triage-1 priority assigned

Comments

@nshahan
Copy link
Contributor

nshahan commented Aug 15, 2022

DDC patches the native JS Object prototype to have some additional properties for the dart equals, hashCode, and runtimeType.

void _installPropertiesForObject(jsProto) {
// core.Object members need to be copied from the non-symbol name to the
// symbol name.
var coreObjProto = JS<Object>('!', '#.prototype', Object);
var names = getOwnPropertyNames(coreObjProto);
for (int i = 0, n = JS('!', '#.length', names); i < n; ++i) {
var name = JS<String>('!', '#[#]', names, i);
if (name == 'constructor') continue;
var desc = getOwnPropertyDescriptor(coreObjProto, name);
defineProperty(jsProto, JS('', '#.#', dartx, name), desc);
}
}

This means that any native JS object created on the page has those properties, including any object created for internal representation purposes. This causes some confusing conditions when debugging internal runtime logic. One such example is that we store method and setter signatures in a native object:

setSignature = runtimeStatement('set${name}Signature(#, () => #)', [
className,
js_ast.ObjectInitializer(elements, multiline: elements.length > 1)
]);

Later when we read the properties from this native object we find some that were not intended. This means we advance into code paths that are not expected, for example when calling a setter dynamically:

if (setterType != null) {
return JS('', '#[#] = #.as(#)', obj, f, setterType, value);
}

We could avoid this confusion in our internal state by avoiding the patches on the native prototypes or by avoiding the use of native objects in our representation (use native maps instead).

@nshahan nshahan added P3 A lower priority bug or feature request web-dev-compiler area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-triage-1 priority assigned labels Aug 15, 2022
copybara-service bot pushed a commit that referenced this issue Aug 10, 2023
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: #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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P3 A lower priority bug or feature request web-dev-compiler web-triage-1 priority assigned
Projects
None yet
Development

No branches or pull requests

1 participant