dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[dart2js] Fix for #46175

Browse Source 
Pass compile-time shadowing information through tearoff code, rather
than trying to feature-test for potential overrides of the method
accessed via a super-getter.

Bug: 46175

Change-Id: Idca8e440cf12bb6cbae020f305763575e6a37b08
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/202803
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
This commit is contained in:
Stephen Adams
2021-06-10 03:30:09 +00:00
committed by commit-bot@chromium.org
parent 70ee62e55f
commit 8ee467361b
5 changed files with 136 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/compiler/lib/src/js_emitter/class_stub_generator.dart
+25 -31
View File
@@ -228,21 +228,21 @@ List<jsAst.Statement> buildTearOffCode(
js(r'''function() { throw "Helper 'closureFromTearOff' missing." }''');
}
jsAst.Statement tearOffGetter;
jsAst.Statement instanceTearOffGetter;
if (options.useContentSecurityPolicy) {
tearOffGetter = js.statement(
instanceTearOffGetter = js.statement(
'''
function tearOffGetter(funcs, applyTrampolineIndex, reflectionInfo, name, isIntercepted) {
function instanceTearOffGetter(funcs, applyTrampolineIndex, reflectionInfo, name, isIntercepted, needsDirectAccess) {
var cache = null;
return isIntercepted
? function(receiver) {
if (cache === null) cache = #createTearOffClass(
this, funcs, applyTrampolineIndex, reflectionInfo, false, true, name);
funcs, applyTrampolineIndex, reflectionInfo, false, true, name, needsDirectAccess);
return new cache(this, funcs[0], receiver, name);
}
: function() {
if (cache === null) cache = #createTearOffClass(
this, funcs, applyTrampolineIndex, reflectionInfo, false, false, name);
funcs, applyTrampolineIndex, reflectionInfo, false, false, name, needsDirectAccess);
return new cache(this, funcs[0], null, name);
};
}''',
@@ -270,46 +270,40 @@ List<jsAst.Statement> buildTearOffCode(
// create a context with the closed-over values. The closed-over values
// include parameters, (Dart) top-level definitions, and the local `cache`
// variable all in one context (passing `null` to initialize `cache`).
tearOffGetter = js.statement(
instanceTearOffGetter = js.statement(
'''
function tearOffGetter(funcs, applyTrampolineIndex, reflectionInfo, name, isIntercepted) {
return isIntercepted
? new Function("funcs, applyTrampolineIndex, reflectionInfo, name, createTearOffClass, cache",
function instanceTearOffGetter(funcs, applyTrampolineIndex, reflectionInfo, name, isIntercepted, needsDirectAccess) {
if (isIntercepted)
return new Function("funcs, applyTrampolineIndex, reflectionInfo, name, needsDirectAccess, createTearOffClass, cache",
"return function tearOff_" + name + (functionCounter++) + "(receiver) {" +
"if (cache === null) cache = createTearOffClass(" +
"this, funcs, applyTrampolineIndex, reflectionInfo, false, true, name);" +
"funcs, applyTrampolineIndex, reflectionInfo, false, true, name, needsDirectAccess);" +
"return new cache(this, funcs[0], receiver, name);" +
"}")(funcs, applyTrampolineIndex, reflectionInfo, name, #createTearOffClass, null)
: new Function("funcs, applyTrampolineIndex, reflectionInfo, name, createTearOffClass, cache",
"}")(funcs, applyTrampolineIndex, reflectionInfo, name, needsDirectAccess, #createTearOffClass, null);
else
return new Function("funcs, applyTrampolineIndex, reflectionInfo, name, needsDirectAccess, createTearOffClass, cache",
"return function tearOff_" + name + (functionCounter++)+ "() {" +
"if (cache === null) cache = createTearOffClass(" +
"this, funcs, applyTrampolineIndex, reflectionInfo, false, false, name);" +
"funcs, applyTrampolineIndex, reflectionInfo, false, false, name, needsDirectAccess);" +
"return new cache(this, funcs[0], null, name);" +
"}")(funcs, applyTrampolineIndex, reflectionInfo, name, #createTearOffClass, null);
"}")(funcs, applyTrampolineIndex, reflectionInfo, name, needsDirectAccess, #createTearOffClass, null);
}''',
{'createTearOffClass': closureFromTearOffAccessExpression},
);
}
jsAst.Statement tearOff = js.statement(
jsAst.Statement staticTearOffGetter = js.statement(
'''
function tearOff(funcs, applyTrampolineIndex,
reflectionInfo, isStatic, name, isIntercepted) {
var cache = null;
return isStatic
? function() {
if (cache === null) cache = #createTearOffClass(
this, funcs, applyTrampolineIndex,
reflectionInfo, true, false, name).prototype;
return cache;
}
: tearOffGetter(funcs, applyTrampolineIndex,
reflectionInfo, name, isIntercepted);
}''',
function staticTearOffGetter(funcs, applyTrampolineIndex, reflectionInfo, name) {
var cache = null;
return function() {
if (cache === null) cache = #createTearOffClass(
funcs, applyTrampolineIndex, reflectionInfo, true, false, name).prototype;
return cache;
}
}''',
{'createTearOffClass': closureFromTearOffAccessExpression},
);
return [tearOffGetter, tearOff];
return [instanceTearOffGetter, staticTearOffGetter];
}
pkg/compiler/lib/src/js_emitter/model.dart
+26 -13
View File
@@ -475,6 +475,14 @@ class InstanceMethod extends DartMethod {
/// and [aliasName].
final js.Name aliasName;
/// `true` if the tear-off needs to access methods directly rather than rely
/// on JavaScript prototype lookup. This happens when a tear-off getter is
/// called via `super.method` and there is a shadowing definition of `method`
/// in some sublcass.
// TODO(sra): Consider instead having an alias per stub, creating tear-off
// trampolines that target the stubs.
final bool tearOffNeedsDirectAccess;
/// True if this is the implicit `call` instance method of an anonymous
/// closure. This predicate is false for explicit `call` methods and for
/// functions that can be torn off.
@@ -486,19 +494,24 @@ class InstanceMethod extends DartMethod {
/// Name called via the general 'catch all' path of Function.apply.
///final js.Name applyName;
InstanceMethod(FunctionEntity element, js.Name name, js.Expression code,
List<ParameterStubMethod> parameterStubs, js.Name callName,
{bool needsTearOff,
js.Name tearOffName,
this.aliasName,
bool canBeApplied,
int requiredParameterCount,
/* List | Map */ optionalParameterDefaultValues,
this.isClosureCallMethod,
this.isIntercepted,
js.Expression functionType,
int applyIndex})
: super(element, name, code, parameterStubs, callName,
InstanceMethod(
FunctionEntity element,
js.Name name,
js.Expression code,
List<ParameterStubMethod> parameterStubs,
js.Name callName, {
bool needsTearOff,
js.Name tearOffName,
this.aliasName,
this.tearOffNeedsDirectAccess,
bool canBeApplied,
int requiredParameterCount,
/* List | Map */ optionalParameterDefaultValues,
this.isClosureCallMethod,
this.isIntercepted,
js.Expression functionType,
int applyIndex,
}) : super(element, name, code, parameterStubs, callName,
needsTearOff: needsTearOff,
tearOffName: tearOffName,
canBeApplied: canBeApplied,
pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart
+6 -2
View File
@@ -800,6 +800,7 @@ class ProgramBuilder {
if (code == null) return null;
bool canTearOff = false;
bool tearOffNeedsDirectAccess = false;
js.Name tearOffName;
bool isClosureCallMethod = false;
bool isNotApplyTarget =
@@ -819,9 +820,11 @@ class ProgramBuilder {
isClosureCallMethod = true;
} else {
// Careful with operators.
canTearOff = _codegenWorld.hasInvokedGetter(element) ||
_codegenWorld.methodsNeedsSuperGetter(element);
bool needsSuperGetter = _codegenWorld.methodsNeedsSuperGetter(element);
canTearOff =
_codegenWorld.hasInvokedGetter(element) || needsSuperGetter;
tearOffName = _namer.getterForElement(element);
tearOffNeedsDirectAccess = needsSuperGetter;
}
}
@@ -865,6 +868,7 @@ class ProgramBuilder {
_generateParameterStubs(element, canTearOff, canBeApplied), callName,
needsTearOff: canTearOff,
tearOffName: tearOffName,
tearOffNeedsDirectAccess: tearOffNeedsDirectAccess,
isClosureCallMethod: isClosureCallMethod,
isIntercepted: isIntercepted,
aliasName: aliasName,
pkg/compiler/lib/src/js_emitter/startup_emitter/fragment_emitter.dart
+30 -29
View File
@@ -4,17 +4,6 @@
part of dart2js.js_emitter.startup_emitter.model_emitter;
/// The name of the property that stores the tear-off getter on a static
/// function.
///
/// This property is only used when isolates are used.
///
/// When serializing static functions we transmit the
/// name of the static function, but not the name of the function's getter. We
/// store the getter-function on the static function itself, which allows us to
/// find it easily.
const String _tearOffPropertyName = r'$tearOff';
/// The fast startup emitter's goal is to minimize the amount of work that the
/// JavaScript engine has to do before it can start running user code.
///
@@ -232,7 +221,7 @@ var functionCounter = 0;
// Each deferred hunk comes with its own types which are added to the end
// of the types-array.
// The `funTypes` passed to the `installTearOff` function below is relative to
// The `funType` passed to the `installTearOff` function below is relative to
// the hunk the function comes from. The `typesOffset` variable encodes the
// offset at which the new types will be added.
var typesOffset = 0;
@@ -253,13 +242,18 @@ var typesOffset = 0;
// different tearOffCode?
function installTearOff(
container, getterName, isStatic, isIntercepted, requiredParameterCount,
optionalParameterDefaultValues, callNames, funsOrNames, funType, applyIndex) {
optionalParameterDefaultValues, callNames, funsOrNames, funType, applyIndex,
needsDirectAccess) {
// A function can have several stubs (for example to fill in optional
// arguments). We collect these functions in the `funs` array.
var funs = [];
for (var i = 0; i < funsOrNames.length; i++) {
var fun = funsOrNames[i];
if ((typeof fun) == "string") fun = container[fun];
if ((typeof fun) == "string") {
var stubName = fun;
fun = container[fun];
fun.#stubName = stubName;
}
fun.#callName = callNames[i];
funs.push(fun);
}
@@ -271,19 +265,18 @@ function installTearOff(
fun[#defaultArgumentValues] = optionalParameterDefaultValues;
var reflectionInfo = funType;
if (typeof reflectionInfo == "number") {
// The reflectionInfo can either be a function, or a pointer into the types
// table. If it points into the types-table we need to update the index,
// in case the tear-off is part of a deferred hunk.
// The reflectionInfo can be a string type recipe or an index into the types
// table. If it points into the types-table we need to update the index, in
// case the tear-off is part of a deferred hunk.
reflectionInfo = reflectionInfo + typesOffset;
}
var name = funsOrNames[0];
fun.#stubName = name;
var getterFunction =
tearOff(funs, applyIndex || 0, reflectionInfo, isStatic, name, isIntercepted);
applyIndex = applyIndex || 0;
var getterFunction = isStatic
? staticTearOffGetter(funs, applyIndex, reflectionInfo, name)
: instanceTearOffGetter(funs, applyIndex, reflectionInfo, name,
isIntercepted, needsDirectAccess);
container[getterName] = getterFunction;
if (isStatic) {
fun.$_tearOffPropertyName = getterFunction;
}
}
function installStaticTearOff(
@@ -295,18 +288,18 @@ function installStaticTearOff(
return installTearOff(
container, getterName, true, false,
requiredParameterCount, optionalParameterDefaultValues,
callNames, funsOrNames, funType, applyIndex);
callNames, funsOrNames, funType, applyIndex, false);
}
function installInstanceTearOff(
container, getterName, isIntercepted,
requiredParameterCount, optionalParameterDefaultValues,
callNames, funsOrNames, funType, applyIndex) {
callNames, funsOrNames, funType, applyIndex, needsDirectAccess) {
// TODO(sra): Specialize installTearOff for instance methods.
return installTearOff(
container, getterName, false, isIntercepted,
requiredParameterCount, optionalParameterDefaultValues,
callNames, funsOrNames, funType, applyIndex);
callNames, funsOrNames, funType, applyIndex, !!needsDirectAccess);
}
// Instead of setting the interceptor tags directly we use this update
@@ -359,7 +352,8 @@ var #hunkHelpers = (function(){
return installInstanceTearOff(
container, getterName, isIntercepted,
requiredParameterCount, optionalParameterDefaultValues,
callNames, [name], funType, applyIndex);
callNames, [name], funType, applyIndex,
/*needsDirectAccess:*/ false);
}
},
@@ -1471,10 +1465,13 @@ class FragmentEmitter {
"applyIndex": applyIndex,
});
} else {
bool tearOffNeedsDirectAccess =
(method as InstanceMethod).tearOffNeedsDirectAccess;
if (requiredParameterCount <= 2 &&
callNames.length == 1 &&
optionalParameterDefaultValues is js.LiteralNull &&
method.applyIndex == 0) {
method.applyIndex == 0 &&
!tearOffNeedsDirectAccess) {
js.Statement finish(int arity) {
// Short form for exactly 0/1/2 arguments.
String isInterceptedTag = isIntercepted ? 'i' : 'u';
@@ -1501,7 +1498,8 @@ class FragmentEmitter {
return js.js.statement('''
#install(#container, #getterName, #isIntercepted,
#requiredParameterCount, #optionalParameterDefaultValues,
#callNames, #funsOrNames, #funType, #applyIndex)''', {
#callNames, #funsOrNames, #funType, #applyIndex,
#tearOffNeedsDirectAccess)''', {
"install": install,
"container": container,
"getterName": js.quoteName(method.tearOffName),
@@ -1513,6 +1511,9 @@ class FragmentEmitter {
"funsOrNames": funsOrNamesArray,
"funType": method.functionType,
"applyIndex": applyIndex,
// 'Truthy' values are ok for `tearOffNeedsDirectAccess`.
"tearOffNeedsDirectAccess":
js.number(tearOffNeedsDirectAccess ? 1 : 0),
});
}
}
sdk/lib/_internal/js_runtime/lib/js_helper.dart
+49 -45
View File
@@ -1831,8 +1831,7 @@ convertDartClosureToJS(closure, int arity) {
/// Superclass for Dart closures.
///
/// All static, tear-off, function declaration and function expression closures
/// extend this class, but classes that implement Function via a `call` method
/// do not.
/// extend this class.
abstract class Closure implements Function {
// TODO(ahe): These constants must be in sync with
// reflection_data_parser.dart.
@@ -1874,13 +1873,13 @@ abstract class Closure implements Function {
/// Caution: this function may be called when building constants.
/// TODO(ahe): Don't call this function when building constants.
static fromTearOff(
receiver,
List functions,
int? applyTrampolineIndex,
int applyTrampolineIndex,
var reflectionInfo,
bool isStatic,
bool isIntercepted,
String propertyName,
bool needsDirectAccess,
) {
JS_EFFECT(() {
// The functions are called here to model the calls from JS forms below.
@@ -1931,7 +1930,7 @@ abstract class Closure implements Function {
// we only use the new instance to access the constructor property and use
// Object.create to create the desired prototype.
//
// TODO(sra): Perhaps cache the prototype to avoid the allocation.
// TODO(sra): Cache the prototype to avoid the allocation.
var prototype = isStatic
? JS('StaticClosure', 'Object.create(#.constructor.prototype)',
new StaticClosure())
@@ -1963,7 +1962,7 @@ abstract class Closure implements Function {
// Create a closure and "monkey" patch it with call stubs.
var trampoline = function;
if (!isStatic) {
trampoline = forwardCallTo(receiver, function, isIntercepted);
trampoline = forwardCallTo(function, isIntercepted, needsDirectAccess);
JS('', '#.\$reflectionInfo = #', trampoline, reflectionInfo);
} else {
JS('', '#[#] = #', prototype, STATIC_FUNCTION_NAME_PROPERTY_NAME,
@@ -1982,7 +1981,9 @@ abstract class Closure implements Function {
var stubCallName = JS('String|Null', '#[#]', stub,
JS_GET_NAME(JsGetName.CALL_NAME_PROPERTY));
if (stubCallName != null) {
stub = isStatic ? stub : forwardCallTo(receiver, stub, isIntercepted);
stub = isStatic
? stub
: forwardCallTo(stub, isIntercepted, needsDirectAccess);
JS('', '#[#] = #', prototype, stubCallName, stub);
}
if (i == applyTrampolineIndex) {
@@ -2041,10 +2042,22 @@ abstract class Closure implements Function {
}
static cspForwardCall(
int arity, bool isSuperCall, String? stubName, function) {
int arity, bool needsDirectAccess, String? stubName, function) {
var getSelf = RAW_DART_FUNCTION_REF(BoundClosure.selfOf);
// Handle intercepted stub-names with the default slow case.
if (isSuperCall) arity = -1;
// We have the target method (or an arity stub for the method) in
// [function]. These fixed-arity forwarding stubs could use
// `Function.prototype.call` on the target directly, but on some browsers it
// is quite a bit faster to do a property access again to get the
// function. Accessing the property again will fail (retrieve the wrong
// function) if the desired property is shadowed. This can happen, e.g.,
// when the tear-off was created by a super-getter call `super.method` and
// `method` has an override on some subclass.
//
// To handle the shadowing-of-a-method-that-has-a-super-tearoff case, we use
// the default slow case that uses `Function.prototype.apply`.
if (needsDirectAccess) arity = -1;
switch (arity) {
case 0:
return JS(
@@ -2121,19 +2134,14 @@ abstract class Closure implements Function {
static bool get isCsp => JS_GET_FLAG('USE_CONTENT_SECURITY_POLICY');
static forwardCallTo(receiver, function, bool isIntercepted) {
if (isIntercepted) return forwardInterceptedCallTo(receiver, function);
static forwardCallTo(function, bool isIntercepted, bool needsDirectAccess) {
if (isIntercepted)
return forwardInterceptedCallTo(function, needsDirectAccess);
String? stubName = JS('String|Null', '#.\$stubName', function);
int arity = JS('int', '#.length', function);
var lookedUpFunction = JS('', '#[#]', receiver, stubName);
// The receiver[stubName] may not be equal to the function if we try to
// forward to a super-method. Especially when we create a bound closure
// of a super-call we need to make sure that we don't forward back to the
// dynamically looked up function.
bool isSuperCall = !identical(function, lookedUpFunction);
if (isCsp || isSuperCall || arity >= 27) {
return cspForwardCall(arity, isSuperCall, stubName, function);
if (isCsp || needsDirectAccess || arity >= 27) {
return cspForwardCall(arity, needsDirectAccess, stubName, function);
}
if (arity == 0) {
@@ -2160,11 +2168,11 @@ abstract class Closure implements Function {
}
static cspForwardInterceptedCall(
int arity, bool isSuperCall, String? name, function) {
int arity, bool needsDirectAccess, String? name, function) {
var getSelf = RAW_DART_FUNCTION_REF(BoundClosure.selfOf);
var getReceiver = RAW_DART_FUNCTION_REF(BoundClosure.receiverOf);
// Handle intercepted stub-names with the default slow case.
if (isSuperCall) arity = -1;
if (needsDirectAccess) arity = -1;
switch (arity) {
case 0:
// Intercepted functions always takes at least one argument (the
@@ -2252,21 +2260,16 @@ abstract class Closure implements Function {
}
}
static forwardInterceptedCallTo(receiver, function) {
static forwardInterceptedCallTo(function, bool needsDirectAccess) {
String selfField = BoundClosure.selfFieldName();
String receiverField = BoundClosure.receiverFieldName();
String? stubName = JS('String|Null', '#.\$stubName', function);
int arity = JS('int', '#.length', function);
bool isCsp = JS_GET_FLAG('USE_CONTENT_SECURITY_POLICY');
var lookedUpFunction = JS('', '#[#]', receiver, stubName);
// The receiver[stubName] may not be equal to the function if we try to
// forward to a super-method. Especially when we create a bound closure
// of a super-call we need to make sure that we don't forward back to the
// dynamically looked up function.
bool isSuperCall = !identical(function, lookedUpFunction);
if (isCsp || isSuperCall || arity >= 28) {
return cspForwardInterceptedCall(arity, isSuperCall, stubName, function);
if (isCsp || needsDirectAccess || arity >= 28) {
return cspForwardInterceptedCall(
arity, needsDirectAccess, stubName, function);
}
if (arity == 1) {
return JS(
@@ -2310,16 +2313,17 @@ abstract class Closure implements Function {
}
/// Called from implicit method getter (aka tear-off).
closureFromTearOff(receiver, functions, applyTrampolineIndex, reflectionInfo,
isStatic, isIntercepted, name) {
closureFromTearOff(functions, applyTrampolineIndex, reflectionInfo, isStatic,
isIntercepted, name, needsDirectAccess) {
return Closure.fromTearOff(
receiver,
JS('JSArray', '#', functions),
JS('int|Null', '#', applyTrampolineIndex),
reflectionInfo,
JS('bool', '!!#', isStatic),
JS('bool', '!!#', isIntercepted),
JS('String', '#', name));
JS('JSArray', '#', functions),
JS('int', '#||0', applyTrampolineIndex),
reflectionInfo,
JS('bool', '!!#', isStatic),
JS('bool', '!!#', isIntercepted),
JS('String', '#', name),
JS('bool', '!!#', needsDirectAccess),
);
}
/// Represents an implicit closure of a function.
@@ -2337,18 +2341,18 @@ class StaticClosure extends TearOffClosure {
/// Represents a 'tear-off' or property extraction closure of an instance
/// method, that is an instance method bound to a specific receiver (instance).
class BoundClosure extends TearOffClosure {
/// The receiver or interceptor.
// TODO(ahe): This could just be the interceptor, we always know if
// we need the interceptor when generating the call method.
/// The JavaScript receiver, which is the Dart receiver or the interceptor.
final _self;
/// The method.
final _target;
/// The receiver. Null if [_self] is not an interceptor.
/// The Dart receiver if [_target] is an intercepted method (in which case
/// [_self] is the interceptor), otherwise `null`.
final _receiver;
/// The name of the function. Only used by the mirror system.
/// The name of the function. Only used by `toString()`.
// TODO(sra): This should be part of the generated tear-off class.
final String _name;
BoundClosure(this._self, this._target, this._receiver, this._name);