Fix stack overflow in mixin application from separate library

Make sure that calls to super class methods from mixin application
stubs are always generated using `super` and not `this`.

Replacing `super` by `this` should not be used as non-virtual field access optimization in forwarding stubs that intend to redirect to the super class members (using `this` redirects to itself and causes infinite recursion).

Closes: #50119
Change-Id: Ib9c9aebdc88555518998db64aefa1fd0905c5b11
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/267822
Reviewed-by: Nicholas Shahan <[email protected]>
Commit-Queue: Anna Gringauze <[email protected]>

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

@@ -184,6 +184,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   final JSTypeRep _typeRep;
 
   bool _superAllowed = true;
+  bool _optimizeNonVirtualFieldAccess = true;
 
   final _superHelpers = <String, js_ast.Method>{};
 
@@ -2008,7 +2009,8 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       }
       fn = _emitNativeFunctionBody(member);
     } else {
-      fn = _emitFunction(member.function, member.name.text);
+      fn = _withMethodDeclarationContext(
+          member, () => _emitFunction(member.function, member.name.text));
     }
 
     var method = js_ast.Method(_declareMemberName(member), fn,
@@ -3660,6 +3662,22 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     return result;
   }
 
+  /// Executes [action] in context of the current [member].
+  ///
+  /// Saves and restores important context information about the member
+  /// that can be used to generate code inside the body of the member.
+  T _withMethodDeclarationContext<T>(Procedure member, T Function() action) {
+    // Mixin applications require using 'super' in calls to members of
+    // the super class. Store this information to disable non-virtual
+    // super field access optimization when compiling the member body.
+    var savedOptimizeNonVirtualFieldAccess = _optimizeNonVirtualFieldAccess;
+    _optimizeNonVirtualFieldAccess =
+        member.stubKind != ProcedureStubKind.ConcreteMixinStub;
+    var result = action();
+    _optimizeNonVirtualFieldAccess = savedOptimizeNonVirtualFieldAccess;
+    return result;
+  }
+
   /// Returns true if the underlying type does not accept a null value.
   bool _mustBeNonNullable(DartType type) =>
       type.nullability == Nullability.nonNullable;
@@ -5485,7 +5503,10 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   /// [jsTarget].[jsName], replacing `super` if it is not allowed in scope.
   js_ast.PropertyAccess _emitSuperTarget(Member member, {bool setter = false}) {
     var jsName = _emitMemberName(member.name.text, member: member);
-    if (member is Field && !_virtualFields.isVirtual(member)) {
+    // Optimize access to non-virtual fields, if allowed in the current context.
+    if (_optimizeNonVirtualFieldAccess &&
+        member is Field &&
+        !_virtualFields.isVirtual(member)) {
       return js_ast.PropertyAccess(js_ast.This(), jsName);
     }
     if (_superAllowed) return js_ast.PropertyAccess(js_ast.Super(), jsName);

tests/dartdevc/mixin/mixin_application_test.dart

@@ -0,0 +1,40 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'mixin_declaration.dart';
+
+void main() {
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C0().foo);
+  Expect.equals('B._foo', D0().foo);
+
+  // When mixin application is in a separate library from the declaration,
+  // private symbol from the current library is used to access `_foo`.
+  Expect.equals('A._foo', C()._foo);
+  Expect.equals('A._foo', D()._foo);
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C().foo);
+  Expect.equals('B._foo', D().foo);
+
+  // E overrides `_foo`.
+  Expect.equals('E._foo', E()._foo);
+}
+
+class C0 = A0 with B;
+class C = A with B;
+
+class D0 extends A0 with B {}
+
+class D extends A with B {}
+
+class E extends A with B {
+  String? _foo = 'E._foo';
+}
+
+class A0 {}
+
+class A {
+  String? _foo = 'A._foo';
+}

tests/dartdevc/mixin/mixin_declaration.dart

@@ -0,0 +1,8 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+class B {
+  String? _foo = 'B._foo';
+  String get foo => _foo!;
+}

tests/dartdevc/mixin/mixin_declaration_application_test.dart

@@ -0,0 +1,44 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+
+void main() {
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C0().foo);
+  Expect.equals('B._foo', D0().foo);
+
+  // When mixin application is in the same library as the declaration,
+  // private symbol from the mixin delaration `B._foo` overrides `A._foo`.
+  Expect.equals('B._foo', C()._foo);
+  Expect.equals('B._foo', D()._foo);
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C().foo);
+  Expect.equals('B._foo', D().foo);
+
+  // E overrides `_foo`.
+  Expect.equals('E._foo', E()._foo);
+}
+
+class C0 = A0 with B;
+class C = A with B;
+
+class D0 extends A0 with B {}
+
+class D extends A with B {}
+
+class E extends A with B {
+  String? _foo = 'E._foo';
+}
+
+class A0 {}
+
+class A {
+  String? _foo = 'A._foo';
+}
+
+class B {
+  String? _foo = 'B._foo';
+  String get foo => _foo!;
+}

tests/dartdevc_2/mixin/mixin_application_test.dart

@@ -0,0 +1,42 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// @dart=2.6
+
+import 'package:expect/expect.dart';
+import 'mixin_declaration.dart';
+
+void main() {
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C0().foo);
+  Expect.equals('B._foo', D0().foo);
+
+  // When mixin application is in a separate library from the declaration,
+  // private symbol from the current library is used to access `_foo`.
+  Expect.equals('A._foo', C()._foo);
+  Expect.equals('A._foo', D()._foo);
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C().foo);
+  Expect.equals('B._foo', D().foo);
+
+  // E overrides `_foo`.
+  Expect.equals('E._foo', E()._foo);
+}
+
+class C0 = A0 with B;
+class C = A with B;
+
+class D0 extends A0 with B {}
+
+class D extends A with B {}
+
+class E extends A with B {
+  String _foo = 'E._foo';
+}
+
+class A0 {}
+
+class A {
+  String _foo = 'A._foo';
+}

tests/dartdevc_2/mixin/mixin_declaration.dart

@@ -0,0 +1,10 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// @dart = 2.6
+
+class B {
+  String _foo = 'B._foo';
+  String get foo => _foo;
+}

tests/dartdevc_2/mixin/mixin_declaration_application_test.dart

@@ -0,0 +1,46 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// @dart=2.6
+
+import 'package:expect/expect.dart';
+
+void main() {
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C0().foo);
+  Expect.equals('B._foo', D0().foo);
+
+  // When mixin application is in the same library as the declaration,
+  // private symbol from the mixin delaration `B._foo` overrides `A._foo`.
+  Expect.equals('B._foo', C()._foo);
+  Expect.equals('B._foo', D()._foo);
+  // Getter `foo` returns `B._foo` that is coming from the mixin declaration.
+  Expect.equals('B._foo', C().foo);
+  Expect.equals('B._foo', D().foo);
+
+  // E overrides `_foo`.
+  Expect.equals('E._foo', E()._foo);
+}
+
+class C0 = A0 with B;
+class C = A with B;
+
+class D0 extends A0 with B {}
+
+class D extends A with B {}
+
+class E extends A with B {
+  String _foo = 'E._foo';
+}
+
+class A0 {}
+
+class A {
+  String _foo = 'A._foo';
+}
+
+class B {
+  String _foo = 'B._foo';
+  String get foo => _foo;
+}