[vm/ffi] regression test for 37511

I tested this test manually by reverting 48d92b3176 to confirm that it makes the test segfault.

Closes: https://github.com/dart-lang/sdk/issues/37511

Change-Id: I62cb2b83775780a2fccfd9ee4ebff793de82090a
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-debug-arm-try, app-kernel-linux-debug-x64-try, vm-kernel-linux-debug-simdbc64-try,vm-kernel-linux-debug-ia32-try,vm-dartkb-linux-debug-simarm64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-dartkb-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-dartkb-linux-release-x64-abi-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109703
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Samir Jindel <sjindel@google.com>

runtime/bin/ffi_test/ffi_test_functions.cc

@@ -499,24 +499,28 @@ DART_EXPORT float InventFloatValue() {
 // Functions for stress-testing.
 
 DART_EXPORT int64_t MinInt64() {
-  Dart_ExecuteInternalCommand("gc-on-next-allocation", nullptr);
+  Dart_ExecuteInternalCommand("gc-on-nth-allocation",
+                              reinterpret_cast<void*>(1));
   return 0x8000000000000000;
 }
 
 DART_EXPORT int64_t MinInt32() {
-  Dart_ExecuteInternalCommand("gc-on-next-allocation", nullptr);
+  Dart_ExecuteInternalCommand("gc-on-nth-allocation",
+                              reinterpret_cast<void*>(1));
   return 0x80000000;
 }
 
 DART_EXPORT double SmallDouble() {
-  Dart_ExecuteInternalCommand("gc-on-next-allocation", nullptr);
+  Dart_ExecuteInternalCommand("gc-on-nth-allocation",
+                              reinterpret_cast<void*>(1));
   return 0x80000000 * -1.0;
 }
 
 // Requires boxing on 32-bit and 64-bit systems, even if the top 32-bits are
 // truncated.
 DART_EXPORT void* LargePointer() {
-  Dart_ExecuteInternalCommand("gc-on-next-allocation", nullptr);
+  Dart_ExecuteInternalCommand("gc-on-nth-allocation",
+                              reinterpret_cast<void*>(1));
   uint64_t origin = 0x8100000082000000;
   return reinterpret_cast<void*>(origin);
 }
@@ -525,6 +529,11 @@ DART_EXPORT void TriggerGC(uint64_t count) {
   Dart_ExecuteInternalCommand("gc-now", nullptr);
 }
 
+DART_EXPORT void CollectOnNthAllocation(intptr_t num_allocations) {
+  Dart_ExecuteInternalCommand("gc-on-nth-allocation",
+                              reinterpret_cast<void*>(num_allocations));
+}
+
 // Triggers GC. Has 11 dummy arguments as unboxed odd integers which should be
 // ignored by GC.
 DART_EXPORT void Regress37069(uint64_t a,

runtime/lib/ffi_dynamic_library_patch.dart

@@ -29,7 +29,7 @@ class DynamicLibrary {
   Pointer<T> lookup<T extends NativeType>(String symbolName)
       native "Ffi_dl_lookup";
 
-  // The real implementation of this function lives in FfiUseSitesTransformer
+  // The real implementation of this function lives in FfiUseSiteTransformer
   // for interface calls. Only dynamic calls (which are illegal) reach this
   // implementation.
   @patch

runtime/vm/heap/heap.cc

@@ -44,7 +44,7 @@ Heap::Heap(Isolate* isolate,
       read_only_(false),
       gc_new_space_in_progress_(false),
       gc_old_space_in_progress_(false),
-      gc_on_next_allocation_(false) {
+      gc_on_nth_allocation_(kNoForcedGarbageCollection) {
   UpdateGlobalMaxUsed();
   for (int sel = 0; sel < kNumWeakSelectors; sel++) {
     new_weak_tables_[sel] = new WeakTable();
@@ -678,15 +678,22 @@ void Heap::AddRegionsToObjectSet(ObjectSet* set) const {
   old_space_.AddRegionsToObjectSet(set);
 }
 
-void Heap::CollectOnNextAllocation() {
+void Heap::CollectOnNthAllocation(intptr_t num_allocations) {
+  // Prevent generated code from using the TLAB fast path on next allocation.
   AbandonRemainingTLAB(Thread::Current());
-  gc_on_next_allocation_ = true;
+  gc_on_nth_allocation_ = num_allocations;
 }
 
 void Heap::CollectForDebugging() {
-  if (!gc_on_next_allocation_) return;
-  CollectAllGarbage(kDebugging);
-  gc_on_next_allocation_ = false;
+  if (gc_on_nth_allocation_ == kNoForcedGarbageCollection) return;
+  gc_on_nth_allocation_--;
+  if (gc_on_nth_allocation_ == 0) {
+    CollectAllGarbage(kDebugging);
+    gc_on_nth_allocation_ = kNoForcedGarbageCollection;
+  } else {
+    // Prevent generated code from using the TLAB fast path on next allocation.
+    AbandonRemainingTLAB(Thread::Current());
+  }
 }
 
 ObjectSet* Heap::CreateAllocatedObjectSet(

runtime/vm/heap/heap.h

@@ -311,7 +311,7 @@ class Heap {
   void AbandonRemainingTLAB(Thread* thread);
   Space SpaceForExternal(intptr_t size) const;
 
-  void CollectOnNextAllocation();
+  void CollectOnNthAllocation(intptr_t num_allocations);
 
  private:
   class GCStats : public ValueObject {
@@ -384,7 +384,7 @@ class Heap {
 
   void AddRegionsToObjectSet(ObjectSet* set) const;
 
-  // Trigger major GC if 'gc_on_next_allocation_' is set.
+  // Trigger major GC if 'gc_on_nth_allocation_' is set.
   void CollectForDebugging();
 
   Isolate* isolate_;
@@ -410,10 +410,12 @@ class Heap {
   bool gc_new_space_in_progress_;
   bool gc_old_space_in_progress_;
 
+  static const intptr_t kNoForcedGarbageCollection = -1;
+
   // Whether the next heap allocation (new or old) should trigger
   // CollectAllGarbage. Used within unit tests for testing GC on certain
   // sensitive codepaths.
-  bool gc_on_next_allocation_;
+  intptr_t gc_on_nth_allocation_;
 
   friend class Become;       // VisitObjectPointers
   friend class GCCompactor;  // VisitObjectPointers

runtime/vm/native_api_impl.cc

@@ -251,12 +251,15 @@ struct RunInSafepointAndRWCodeArgs {
 DART_EXPORT void* Dart_ExecuteInternalCommand(const char* command, void* arg) {
   if (!FLAG_enable_testing_pragmas) return nullptr;
 
-  if (!strcmp(command, "gc-on-next-allocation")) {
+  if (!strcmp(command, "gc-on-nth-allocation")) {
     TransitionNativeToVM _(Thread::Current());
-    Isolate::Current()->heap()->CollectOnNextAllocation();
+    intptr_t argument = reinterpret_cast<intptr_t>(arg);
+    ASSERT(argument > 0);
+    Isolate::Current()->heap()->CollectOnNthAllocation(argument);
     return nullptr;
 
   } else if (!strcmp(command, "gc-now")) {
+    ASSERT(arg == nullptr);  // Don't pass an argument to this command.
     TransitionNativeToVM _(Thread::Current());
     Isolate::Current()->heap()->CollectAllGarbage();
     return nullptr;

tests/ffi/analysis_options.yaml

@@ -0,0 +1,3 @@
+analyzer:
+  exclude:
+    # Do analyze this subfolder in the tests/ even if tests/ is fully excluded.

tests/ffi/ffi.status

@@ -20,6 +20,7 @@ function_callbacks_test/03: Skip
 
 [ $runtime == dart_precompiled ]
 function_callbacks_test: Skip  # Issue dartbug.com/37295
+regress_37511_callbacks_test: Skip  # Issue dartbug.com/37295
 
 [ $arch == arm && $system != android ]
 *: Skip # "hardfp" calling convention is not yet supported (iOS is also supported but not tested): dartbug.com/36309

tests/ffi/ffi_test_helpers.dart

@@ -5,12 +5,20 @@
 // Helpers for tests which trigger GC in delicate places.
 
 import 'dart:ffi' as ffi;
+
 import 'dylib_utils.dart';
 
 typedef NativeNullaryOp = ffi.Void Function();
 typedef NullaryOpVoid = void Function();
 
+typedef NativeUnaryOp = ffi.Void Function(ffi.IntPtr);
+typedef UnaryOpVoid = void Function(int);
+
 final ffi.DynamicLibrary ffiTestFunctions =
     dlopenPlatformSpecific("ffi_test_functions");
+
 final triggerGc = ffiTestFunctions
     .lookupFunction<NativeNullaryOp, NullaryOpVoid>("TriggerGC");
+
+final collectOnNthAllocation = ffiTestFunctions
+    .lookupFunction<NativeUnaryOp, UnaryOpVoid>("CollectOnNthAllocation");

tests/ffi/gc_helpers.dart

@@ -1,16 +0,0 @@
-// 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.
-//
-// Helpers for tests which trigger GC in delicate places.
-
-import 'dart:ffi' as ffi;
-import 'dylib_utils.dart';
-
-typedef NativeNullaryOp = ffi.Void Function();
-typedef NullaryOpVoid = void Function();
-
-final ffi.DynamicLibrary ffiTestFunctions =
-    dlopenPlatformSpecific("ffi_test_functions");
-final triggerGc = ffiTestFunctions
-    .lookupFunction<NativeNullaryOp, NullaryOpVoid>("TriggerGC");

tests/ffi/regress_37511_callbacks_test.dart

@@ -0,0 +1,54 @@
+// 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 test program for testing dart:ffi struct pointers.
+//
+// VMOptions=--deterministic --enable-testing-pragmas
+//
+// SharedObjects=ffi_test_functions
+//
+// TODO(37295): Merge this file with regress_37511_test.dart when callback
+// support lands.
+
+library FfiTest;
+
+import 'dart:ffi';
+
+import 'ffi_test_helpers.dart';
+
+/// Estimate of how many allocations functions in `functionsToTest` do at most.
+const gcAfterNAllocationsMax = 10;
+
+void main() {
+  for (Function() f in functionsToTest) {
+    f(); // Ensure code is compiled.
+
+    for (int n = 1; n <= gcAfterNAllocationsMax; n++) {
+      collectOnNthAllocation(n);
+      f();
+    }
+  }
+}
+
+final List<Function()> functionsToTest = [
+  // Callback trampolines.
+  doFromFunction,
+  () => callbackSmallDouble(dartFunctionPointer),
+];
+
+// Callback trampoline helpers.
+typedef NativeCallbackTest = Int32 Function(Pointer);
+typedef NativeCallbackTestFn = int Function(Pointer);
+
+final callbackSmallDouble =
+    ffiTestFunctions.lookupFunction<NativeCallbackTest, NativeCallbackTestFn>(
+        "TestSimpleMultiply");
+
+typedef SimpleMultiplyType = Double Function(Double);
+double simpleMultiply(double x) => x * 1.337;
+
+final doFromFunction =
+    () => Pointer.fromFunction<SimpleMultiplyType>(simpleMultiply, 0.0);
+
+final dartFunctionPointer = doFromFunction();

tests/ffi/regress_37511_test.dart

@@ -0,0 +1,108 @@
+// 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 test program for testing dart:ffi struct pointers.
+//
+// VMOptions=--deterministic --enable-testing-pragmas
+//
+// SharedObjects=ffi_test_functions
+
+library FfiTest;
+
+import 'dart:ffi';
+
+import 'dylib_utils.dart';
+import 'ffi_test_helpers.dart';
+
+/// Estimate of how many allocations functions in `functionsToTest` do at most.
+const gcAfterNAllocationsMax = 10;
+
+void main() {
+  for (Function() f in functionsToTest) {
+    f(); // Ensure code is compiled.
+
+    for (int n = 1; n <= gcAfterNAllocationsMax; n++) {
+      collectOnNthAllocation(n);
+      f();
+    }
+  }
+}
+
+final List<Function()> functionsToTest = [
+  // Pointer operations.
+  () => highAddressPointer.cast<Double>(),
+  () => Pointer.fromAddress(highAddressPointer.address),
+  () => highAddressPointer.address,
+  () => highAddressPointer.elementAt(1),
+  () => highAddressPointer.offsetBy(1),
+  () => highAddressPointer.asExternalTypedData(),
+
+  // DynamicLibrary operations.
+  doDlopen,
+  doDlsym, // Includes `asFunction`.
+  () => ffiTestFunctions.handle,
+
+  // Trampolines.
+  () => sumManyIntsOdd(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, mint64bit),
+  () => sumManyDoubles(1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0),
+  minInt64,
+  minInt32,
+  smallDouble,
+  largePointer,
+
+  // Callback trampolines.
+  //
+  // In regress_37511_callbacks_test.dart because callbacks are not supported
+  // in AOT yet.
+];
+
+// Pointer operation helpers.
+const mint32bit = 0xFFFFFFF0;
+const mint64bit = 0x7FFFFFFFFFFFFFF0;
+
+final int highAddress = sizeOf<IntPtr>() == 4 ? mint32bit : mint64bit;
+
+final Pointer<Int64> highAddressPointer = Pointer.fromAddress(highAddress);
+
+// Dynamic library operation helpers.
+final doDlopen = () => dlopenPlatformSpecific("ffi_test_functions");
+
+final doDlsym = () => ffiTestFunctions
+    .lookupFunction<NativeNullaryOp, NullaryOpVoid>("TriggerGC");
+
+// Trampoline helpers.
+typedef NativeUndenaryOp = IntPtr Function(IntPtr, IntPtr, IntPtr, IntPtr,
+    IntPtr, IntPtr, IntPtr, IntPtr, IntPtr, IntPtr, IntPtr);
+typedef UndenaryOp = int Function(
+    int, int, int, int, int, int, int, int, int, int, int);
+
+final UndenaryOp sumManyIntsOdd = ffiTestFunctions
+    .lookupFunction<NativeUndenaryOp, UndenaryOp>("SumManyIntsOdd");
+
+typedef NativeDoubleDecenaryOp = Double Function(Double, Double, Double, Double,
+    Double, Double, Double, Double, Double, Double);
+typedef DoubleDecenaryOp = double Function(double, double, double, double,
+    double, double, double, double, double, double);
+
+final DoubleDecenaryOp sumManyDoubles = ffiTestFunctions
+    .lookupFunction<NativeDoubleDecenaryOp, DoubleDecenaryOp>("SumManyDoubles");
+
+typedef NativeNullaryOp64 = Int64 Function();
+typedef NativeNullaryOp32 = Int32 Function();
+typedef NativeNullaryOpDouble = Double Function();
+typedef NullaryOpPtr = Pointer<Void> Function();
+typedef NullaryOp = int Function();
+typedef NullaryOpDbl = double Function();
+
+final minInt64 =
+    ffiTestFunctions.lookupFunction<NativeNullaryOp64, NullaryOp>("MinInt64");
+
+final minInt32 =
+    ffiTestFunctions.lookupFunction<NativeNullaryOp32, NullaryOp>("MinInt32");
+
+final smallDouble = ffiTestFunctions
+    .lookupFunction<NativeNullaryOpDouble, NullaryOpDbl>("SmallDouble");
+
+final largePointer =
+    ffiTestFunctions.lookupFunction<NullaryOpPtr, NullaryOpPtr>("LargePointer");