From 72bd9baa54bb71fd4e1bf90fa2be6976d0f37e27 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Tue, 14 Oct 2025 13:35:02 -0700 Subject: [PATCH] [vm,aot] Fix unboxing info (including calling conventions) on unreachable fields For some reason unboxing information was not generated for unreachable fields. However, unreachable field can be used as an interface target, and unboxing info contains a flag indicating if register calling convention should be used which affects how the call via this field as an interface target is going to be performed. So it is incorrect to omit such unboxing info and this change fixes this bug. TEST=vm/dart/regress_b_450678575_test Bug: b/450678575 Change-Id: Ic1f299d9c0b09005c8bbfd95d1a409c113630263 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/454681 Commit-Queue: Alexander Markov Reviewed-by: Slava Egorov --- .../type_flow/transformer.dart | 30 ++++----- .../regress_b404559785.dart.expect | 1 + .../vm/dart/regress_b_450678575_test.dart | 65 +++++++++++++++++++ 3 files changed, 80 insertions(+), 16 deletions(-) create mode 100644 runtime/tests/vm/dart/regress_b_450678575_test.dart diff --git a/pkg/vm/lib/transformations/type_flow/transformer.dart b/pkg/vm/lib/transformations/type_flow/transformer.dart index e781924ed2d..d0907745437 100644 --- a/pkg/vm/lib/transformations/type_flow/transformer.dart +++ b/pkg/vm/lib/transformations/type_flow/transformer.dart @@ -731,24 +731,22 @@ class AnnotateKernel extends RecursiveVisitor { _setUnreachable(member); } - if (member is! Field) { - final unboxingInfoMetadata = _unboxingInfo.getUnboxingInfoOfMember( - member, - ); - if (unboxingInfoMetadata != null) { - // Check for partitions that only have abstract methods should be marked as boxed. - if (unboxingInfoMetadata.returnInfo == UnboxingType.kUnknown) { - unboxingInfoMetadata.returnInfo = UnboxingType.kBoxed; - } - for (int i = 0; i < unboxingInfoMetadata.argsInfo.length; i++) { - if (unboxingInfoMetadata.argsInfo[i] == UnboxingType.kUnknown) { - unboxingInfoMetadata.argsInfo[i] = UnboxingType.kBoxed; - } - } - if (!unboxingInfoMetadata.isTrivial) { - _unboxingInfoMetadata.mapping[member] = unboxingInfoMetadata; + final unboxingInfoMetadata = _unboxingInfo.getUnboxingInfoOfMember( + member, + ); + if (unboxingInfoMetadata != null) { + // Check for partitions that only have abstract methods should be marked as boxed. + if (unboxingInfoMetadata.returnInfo == UnboxingType.kUnknown) { + unboxingInfoMetadata.returnInfo = UnboxingType.kBoxed; + } + for (int i = 0; i < unboxingInfoMetadata.argsInfo.length; i++) { + if (unboxingInfoMetadata.argsInfo[i] == UnboxingType.kUnknown) { + unboxingInfoMetadata.argsInfo[i] = UnboxingType.kBoxed; } } + if (!unboxingInfoMetadata.isTrivial) { + _unboxingInfoMetadata.mapping[member] = unboxingInfoMetadata; + } } } diff --git a/pkg/vm/testcases/transformations/type_flow/transformer/regress_b404559785.dart.expect b/pkg/vm/testcases/transformations/type_flow/transformer/regress_b404559785.dart.expect index 72bb8823880..cb00e68e846 100644 --- a/pkg/vm/testcases/transformations/type_flow/transformer/regress_b404559785.dart.expect +++ b/pkg/vm/testcases/transformations/type_flow/transformer/regress_b404559785.dart.expect @@ -6,6 +6,7 @@ abstract class Foo extends core::Object { [@vm.unreachable.metadata=] [@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasNonThisUses:false,hasTearOffUses:false,getterSelectorId:1] + [@vm.unboxing-info.metadata=[!regcc]] @#C3 late final field core::int hashCode = throw "Attempt to execute code removed by Dart AOT compiler (TFA)"; } diff --git a/runtime/tests/vm/dart/regress_b_450678575_test.dart b/runtime/tests/vm/dart/regress_b_450678575_test.dart new file mode 100644 index 00000000000..3cb8b9322e9 --- /dev/null +++ b/runtime/tests/vm/dart/regress_b_450678575_test.dart @@ -0,0 +1,65 @@ +// Copyright (c) 2025, 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. + +// Regression test for b/450678575. +// +// Verifies that compiler uses correct calling conventions (!regcc) +// when using unreachable field as an interface target. + +import 'package:expect/expect.dart'; + +bool opaqueTrue = int.parse('1') == 1; + +int listener = -1; + +abstract class ChangeNotifier { + void addListener(); +} + +class Notifier1 implements ChangeNotifier { + @pragma('vm:never-inline') + void addListener() { + listener = 1; + } +} + +class Notifier2 implements ChangeNotifier { + @pragma('vm:never-inline') + void addListener() { + listener = 2; + } +} + +abstract class M { + // This field is unreachable and only serves as + // an interface target. + @pragma("vm:entry-point") + final ChangeNotifier accessibilityFocus = Notifier2(); +} + +class B implements M { + @pragma("vm:entry-point") + final ChangeNotifier accessibilityFocus = Notifier1(); +} + +class C implements M { + @pragma("vm:entry-point") + final ChangeNotifier accessibilityFocus = Notifier2(); +} + +// Clobber values on top of the stack. +@pragma('vm:never-inline') +@pragma("vm:entry-point") +void boxed(int a, int b) { + print(a); + print(b); +} + +M instance = opaqueTrue ? B() : C(); + +void main(List args) { + boxed(1, 2); + instance.accessibilityFocus.addListener(); + Expect.equals(1, listener); +}