[vm] Native effects

Adds a way to express native effects in Dart expressions in kernel.

This CL adds a `void _nativeEffect(Object)` to `dart:internal`.
The semantics of `_nativeEffect` are to not execute its arguments and
return `null`.

This CL uses this `_nativeEffect` to make sure that we never execute
the struct constructor invocations used to simulate the native behavior
of FFI trampolines.

Closes: #45607

TEST=tests/ffi(_2)/native_effect_test.dart

Change-Id: Ie06de145e49f8b1cae9e148c2d5d97d5cd8e6878
Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm64-try,vm-kernel-precomp-asan-linux-release-x64-try,vm-kernel-precomp-dwarf-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-obfuscate-linux-release-x64-try,analyzer-analysis-server-linux-try,analyzer-linux-release-try,dart-sdk-linux-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-reload-linux-debug-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/194421
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>

pkg/vm/lib/transformations/ffi.dart

@@ -203,6 +203,7 @@ class FfiTransformer extends Transformer {
   final Class listClass;
   final Class typeClass;
   final Procedure unsafeCastMethod;
+  final Procedure nativeEffectMethod;
   final Class typedDataClass;
   final Procedure typedDataBufferGetter;
   final Procedure typedDataOffsetInBytesGetter;
@@ -294,6 +295,8 @@ class FfiTransformer extends Transformer {
         typeClass = coreTypes.typeClass,
         unsafeCastMethod =
             index.getTopLevelMember('dart:_internal', 'unsafeCast'),
+        nativeEffectMethod =
+            index.getTopLevelMember('dart:_internal', '_nativeEffect'),
         typedDataClass = index.getClass('dart:typed_data', 'TypedData'),
         typedDataBufferGetter =
             index.getMember('dart:typed_data', 'TypedData', 'get:buffer'),

pkg/vm/lib/transformations/ffi_use_sites.dart

@@ -387,29 +387,29 @@ class _FfiUseSiteTransformer extends FfiTransformer {
   }
 
   /// Prevents the struct from being tree-shaken in TFA by invoking its
-  /// constructor in a let expression.
-  ///
-  /// TFA does not recognize this as dead code, only the VM does.
-  /// TODO(http://dartbug.com/45607): Wrap with `_nativeEffect` to make the
-  /// intent of this code clear.
+  /// constructor in a `_nativeEffect` expression.
   Expression _invokeStructConstructor(
-      Expression nestedExpression, Class structClass) {
-    final constructor = structClass.constructors
+      Expression nestedExpression, Class compositeClass) {
+    final constructor = compositeClass.constructors
         .firstWhere((c) => c.name == Name("#fromTypedDataBase"));
-    return Let(
-        VariableDeclaration.forValue(
-            ConstructorInvocation(
-                constructor,
-                Arguments([
-                  StaticInvocation(
-                      uint8ListFactory,
-                      Arguments([
-                        ConstantExpression(IntConstant(1)),
-                      ]))
-                    ..fileOffset = nestedExpression.fileOffset,
-                ]))
-              ..fileOffset = nestedExpression.fileOffset,
-            type: InterfaceType(structClass, Nullability.nonNullable)),
+    return BlockExpression(
+        Block([
+          ExpressionStatement(StaticInvocation(
+              nativeEffectMethod,
+              Arguments([
+                ConstructorInvocation(
+                    constructor,
+                    Arguments([
+                      StaticInvocation(
+                          uint8ListFactory,
+                          Arguments([
+                            ConstantExpression(IntConstant(1)),
+                          ]))
+                        ..fileOffset = nestedExpression.fileOffset,
+                    ]))
+                  ..fileOffset = nestedExpression.fileOffset
+              ])))
+        ]),
         nestedExpression)
       ..fileOffset = nestedExpression.fileOffset;
   }

pkg/vm/testcases/transformations/type_flow/transformer/ffi_struct_constructors.dart.expect

@@ -65,21 +65,27 @@ static method main() → void {
 }
 static method testLookupFunctionReturn() → void {
   final ffi::DynamicLibrary* dylib = [@vm.inferred-type.metadata=dart.ffi::DynamicLibrary?] ffi::DynamicLibrary::executable();
-  final () →* self::Struct1* function1 = let final self::Struct1 #t1 = new self::Struct1::#fromTypedDataBase([@vm.inferred-type.metadata=dart.typed_data::_Uint8List] typ::Uint8List::•(#C18)) in ffi::_asFunctionInternal<() →* self::Struct1*, () →* self::Struct1*>([@vm.direct-call.metadata=dart.ffi::DynamicLibrary.lookup??] [@vm.inferred-type.metadata=dart.ffi::Pointer? (skip check)] dylib.{ffi::DynamicLibrary::lookup}<ffi::NativeFunction<() →* self::Struct1*>*>("function1"));
+  final () →* self::Struct1* function1 = block {
+    _in::_nativeEffect(new self::Struct1::#fromTypedDataBase([@vm.inferred-type.metadata=dart.typed_data::_Uint8List] typ::Uint8List::•(#C18)));
+  } =>ffi::_asFunctionInternal<() →* self::Struct1*, () →* self::Struct1*>([@vm.direct-call.metadata=dart.ffi::DynamicLibrary.lookup??] [@vm.inferred-type.metadata=dart.ffi::Pointer? (skip check)] dylib.{ffi::DynamicLibrary::lookup}<ffi::NativeFunction<() →* self::Struct1*>*>("function1"));
   final self::Struct1* struct1 = [@vm.call-site-attributes.metadata=receiverType:#lib::Struct1* Function()*] function1.call();
   core::print(struct1);
 }
 static method testAsFunctionReturn() → void {
   final ffi::Pointer<ffi::NativeFunction<() →* self::Struct2*>*>* pointer = [@vm.inferred-type.metadata=dart.ffi::Pointer?] ffi::Pointer::fromAddress<ffi::NativeFunction<() →* self::Struct2*>*>(3735928559);
-  final () →* self::Struct2* function2 = let final self::Struct2 #t2 = new self::Struct2::#fromTypedDataBase([@vm.inferred-type.metadata=dart.typed_data::_Uint8List] typ::Uint8List::•(#C18)) in ffi::_asFunctionInternal<() →* self::Struct2*, () →* self::Struct2*>(pointer);
+  final () →* self::Struct2* function2 = block {
+    _in::_nativeEffect(new self::Struct2::#fromTypedDataBase([@vm.inferred-type.metadata=dart.typed_data::_Uint8List] typ::Uint8List::•(#C18)));
+  } =>ffi::_asFunctionInternal<() →* self::Struct2*, () →* self::Struct2*>(pointer);
   final self::Struct2* struct2 = [@vm.call-site-attributes.metadata=receiverType:#lib::Struct2* Function()*] function2.call();
   core::print(struct2);
 }
 [@vm.unboxing-info.metadata=(b)->i]static method useStruct3(self::Struct3* struct3) → core::int* {
   return 42;
 }
 static method testFromFunctionArgument() → void {
-  final ffi::Pointer<ffi::NativeFunction<(self::Struct3*) →* ffi::Int32*>*>* pointer = let final self::Struct3 #t3 = new self::Struct3::#fromTypedDataBase([@vm.inferred-type.metadata=dart.typed_data::_Uint8List] typ::Uint8List::•(#C18)) in [@vm.inferred-type.metadata=dart.ffi::Pointer?] self::_#ffiCallback0;
+  final ffi::Pointer<ffi::NativeFunction<(self::Struct3*) →* ffi::Int32*>*>* pointer = block {
+    _in::_nativeEffect(new self::Struct3::#fromTypedDataBase([@vm.inferred-type.metadata=dart.typed_data::_Uint8List] typ::Uint8List::•(#C18)));
+  } =>[@vm.inferred-type.metadata=dart.ffi::Pointer?] self::_#ffiCallback0;
   core::print(pointer);
 }
 static method testLookupFunctionArgument() → void {

runtime/lib/object.cc

@@ -279,6 +279,10 @@ DEFINE_NATIVE_ENTRY(Internal_unsafeCast, 0, 1) {
   return arguments->NativeArgAt(0);
 }
 
+DEFINE_NATIVE_ENTRY(Internal_nativeEffect, 0, 1) {
+  UNREACHABLE();
+}
+
 DEFINE_NATIVE_ENTRY(Internal_reachabilityFence, 0, 1) {
   UNREACHABLE();
 }

runtime/vm/bootstrap_natives.h

@@ -333,6 +333,7 @@ namespace dart {
   V(GrowableList_setLength, 2)                                                 \
   V(GrowableList_setData, 2)                                                   \
   V(Internal_unsafeCast, 1)                                                    \
+  V(Internal_nativeEffect, 1)                                                  \
   V(Internal_reachabilityFence, 1)                                             \
   V(Internal_collectAllGarbage, 0)                                             \
   V(Internal_makeListFixedLength, 1)                                           \

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -2643,7 +2643,7 @@ Fragment StreamingFlowGraphBuilder::BuildStaticSet(TokenPosition* p) {
 }
 
 Fragment StreamingFlowGraphBuilder::BuildMethodInvocation(TokenPosition* p) {
-  const intptr_t offset = ReaderOffset() - 1;     // Include the tag.
+  const intptr_t offset = ReaderOffset() - 1;  // Include the tag.
 
   const uint8_t flags = ReadFlags();  // read flags.
   const bool is_invariant = (flags & kMethodInvocationFlagInvariant) != 0;
@@ -3007,7 +3007,9 @@ Fragment StreamingFlowGraphBuilder::BuildStaticInvocation(TokenPosition* p) {
   }
 
   const auto recognized_kind = target.recognized_kind();
-  if (recognized_kind == MethodRecognizer::kFfiAsFunctionInternal) {
+  if (recognized_kind == MethodRecognizer::kNativeEffect) {
+    return BuildNativeEffect();
+  } else if (recognized_kind == MethodRecognizer::kFfiAsFunctionInternal) {
     return BuildFfiAsFunctionInternal();
   } else if (CompilerState::Current().is_aot() &&
              recognized_kind == MethodRecognizer::kFfiNativeCallbackFunction) {
@@ -5018,20 +5020,42 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionNode(
   return instructions;
 }
 
+Fragment StreamingFlowGraphBuilder::BuildNativeEffect() {
+  const intptr_t argc = ReadUInt();  // Read argument count.
+  ASSERT(argc == 1);                 // Native side effect to ignore.
+  const intptr_t list_length = ReadListLength();  // Read types list length.
+  ASSERT(list_length == 0);
+
+  const intptr_t positional_count =
+      ReadListLength();  // Read positional argument count.
+  ASSERT(positional_count == 1);
+
+  BuildExpression();  // Consume expression but don't save the fragment.
+  Pop();              // Restore the stack.
+
+  const intptr_t named_args_len =
+      ReadListLength();  // Skip empty named arguments.
+  ASSERT(named_args_len == 0);
+
+  Fragment code;
+  code += NullConstant();  // Return type is void.
+  return code;
+}
+
 Fragment StreamingFlowGraphBuilder::BuildFfiAsFunctionInternal() {
-  const intptr_t argc = ReadUInt();               // read argument count.
-  ASSERT(argc == 1);                              // pointer
-  const intptr_t list_length = ReadListLength();  // read types list length.
-  ASSERT(list_length == 2);  // dart signature, then native signature
+  const intptr_t argc = ReadUInt();               // Read argument count.
+  ASSERT(argc == 1);                              // Pointer.
+  const intptr_t list_length = ReadListLength();  // Read types list length.
+  ASSERT(list_length == 2);  // Dart signature, then native signature.
   const TypeArguments& type_arguments =
-      T.BuildTypeArguments(list_length);  // read types.
+      T.BuildTypeArguments(list_length);  // Read types.
   Fragment code;
   const intptr_t positional_count =
-      ReadListLength();  // read positional argument count
+      ReadListLength();  // Read positional argument count.
   ASSERT(positional_count == 1);
-  code += BuildExpression();  // build first positional argument (pointer)
+  code += BuildExpression();  // Build first positional argument (pointer).
   const intptr_t named_args_len =
-      ReadListLength();  // skip (empty) named arguments list
+      ReadListLength();  // Skip empty named arguments list.
   ASSERT(named_args_len == 0);
   code += B->BuildFfiAsFunctionInternalCall(type_arguments);
   return code;
@@ -5044,24 +5068,24 @@ Fragment StreamingFlowGraphBuilder::BuildFfiNativeCallbackFunction() {
   //
   // The FE also guarantees that all three arguments are constants.
 
-  const intptr_t argc = ReadUInt();  // read argument count
-  ASSERT(argc == 2);                 // target, exceptionalReturn
+  const intptr_t argc = ReadUInt();  // Read argument count.
+  ASSERT(argc == 2);                 // Target, exceptionalReturn.
 
-  const intptr_t list_length = ReadListLength();  // read types list length
-  ASSERT(list_length == 1);                       // native signature
+  const intptr_t list_length = ReadListLength();  // Read types list length.
+  ASSERT(list_length == 1);                       // The native signature.
   const TypeArguments& type_arguments =
-      T.BuildTypeArguments(list_length);  // read types.
+      T.BuildTypeArguments(list_length);  // Read types.
   ASSERT(type_arguments.Length() == 1 && type_arguments.IsInstantiated());
   const FunctionType& native_sig =
       FunctionType::CheckedHandle(Z, type_arguments.TypeAt(0));
 
   Fragment code;
   const intptr_t positional_count =
-      ReadListLength();  // read positional argument count
+      ReadListLength();  // Read positional argument count.
   ASSERT(positional_count == 2);
 
   // Read target expression and extract the target function.
-  code += BuildExpression();  // build first positional argument (target)
+  code += BuildExpression();  // Build first positional argument (target).
   Definition* target_def = B->Peek();
   ASSERT(target_def->IsConstant());
   const Closure& target_closure =
@@ -5081,7 +5105,7 @@ Fragment StreamingFlowGraphBuilder::BuildFfiNativeCallbackFunction() {
   code += Drop();
 
   const intptr_t named_args_len =
-      ReadListLength();  // skip (empty) named arguments list
+      ReadListLength();  // Skip (empty) named arguments list.
   ASSERT(named_args_len == 0);
 
   const Function& result =

runtime/vm/compiler/frontend/kernel_binary_flowgraph.h

@@ -344,11 +344,14 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
   Fragment BuildFunctionNode(TokenPosition parent_position,
                              StringIndex name_index);
 
-  // Build build FG for '_asFunctionInternal'. Reads an Arguments from the
+  // Build flow graph for '_nativeEffect'.
+  Fragment BuildNativeEffect();
+
+  // Build FG for '_asFunctionInternal'. Reads an Arguments from the
   // Kernel buffer and pushes the resulting closure.
   Fragment BuildFfiAsFunctionInternal();
 
-  // Build build FG for '_nativeCallbackFunction'. Reads an Arguments from the
+  // Build FG for '_nativeCallbackFunction'. Reads an Arguments from the
   // Kernel buffer and pushes the resulting Function object.
   Fragment BuildFfiNativeCallbackFunction();
 

runtime/vm/compiler/recognized_methods_list.h

@@ -166,6 +166,7 @@ namespace dart {
   V(::, _abi, FfiAbi, 0x7c4ab775)                                              \
   V(::, _asFunctionInternal, FfiAsFunctionInternal, 0xbbcb235a)                \
   V(::, _nativeCallbackFunction, FfiNativeCallbackFunction, 0x3ff5ae9c)        \
+  V(::, _nativeEffect, NativeEffect, 0x61e00b59)                               \
   V(::, _loadInt8, FfiLoadInt8, 0x0f04dfd6)                                    \
   V(::, _loadInt16, FfiLoadInt16, 0xec44312d)                                  \
   V(::, _loadInt32, FfiLoadInt32, 0xee223fc3)                                  \

sdk/lib/_internal/vm/lib/internal_patch.dart

@@ -163,11 +163,16 @@ Int32List _growRegExpStack(Int32List stack) {
 T unsafeCast<T>(Object? v) native "Internal_unsafeCast";
 
 // This function can be used to keep an object alive til that point.
-//
 @pragma("vm:recognized", "other")
 @pragma('vm:prefer-inline')
 void reachabilityFence(Object object) native "Internal_reachabilityFence";
 
+// This function can be used to encode native side effects.
+//
+// The function call and it's argument are removed in flow graph construction.
+@pragma("vm:recognized", "other")
+void _nativeEffect(Object object) native "Internal_nativeEffect";
+
 void sendAndExit(SendPort sendPort, var message)
     native "SendPortImpl_sendAndExitInternal_";
 

tests/ffi/native_effect_test.dart

@@ -0,0 +1,33 @@
+// Copyright (c) 2021, 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.
+//
+// SharedObjects=ffi_test_functions
+
+// Tests that the dart:internal _nativeEffect flow graph builder works.
+
+import 'dart:ffi';
+
+import "package:expect/expect.dart";
+
+import 'dylib_utils.dart';
+
+void main() {
+  testReturnStruct1ByteInt();
+}
+
+final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions");
+
+final returnStruct1ByteInt = ffiTestFunctions.lookupFunction<
+    Struct1ByteInt Function(Int8),
+    Struct1ByteInt Function(int)>("ReturnStruct1ByteInt");
+
+void testReturnStruct1ByteInt() {
+  final result = returnStruct1ByteInt(1);
+  Expect.equals(1, result.a0);
+}
+
+class Struct1ByteInt extends Struct {
+  @Int8()
+  external int a0;
+}

tests/ffi_2/native_effect_test.dart

@@ -0,0 +1,33 @@
+// Copyright (c) 2021, 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.
+//
+// SharedObjects=ffi_test_functions
+
+// Tests that the dart:internal _nativeEffect flow graph builder works.
+
+import 'dart:ffi';
+
+import "package:expect/expect.dart";
+
+import 'dylib_utils.dart';
+
+void main() {
+  testReturnStruct1ByteInt();
+}
+
+final ffiTestFunctions = dlopenPlatformSpecific("ffi_test_functions");
+
+final returnStruct1ByteInt = ffiTestFunctions.lookupFunction<
+    Struct1ByteInt Function(Int8),
+    Struct1ByteInt Function(int)>("ReturnStruct1ByteInt");
+
+void testReturnStruct1ByteInt() {
+  final result = returnStruct1ByteInt(1);
+  Expect.equals(1, result.a0);
+}
+
+class Struct1ByteInt extends Struct {
+  @Int8()
+  int a0;
+}