From c0c7c1ef4991d5bab38e728578a74c6b1eadf3f4 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Thu, 1 Feb 2024 18:28:53 +0000 Subject: [PATCH] [vm/ffi] Update inner pointer accesses in IL FFI loads and stores via structs can have a TypedData as receiver, so this CL updates those loads to `kMayBeInnerPointer`. This CL adds an IL test to verify that for `Pointer` loads the untagged value is treated correctly as `kCannotBeInnerPointer`. (And adds some prefer-inline pragmas to make some common operations be inlined to avoid allocating `Pointer` objects.) This CL updates the load in the FFI closures to use a load-field. This can also potentially enable not allocating a pointer object when this closure is inlined. TEST=tests/ffi/unwrap_typeddata_generated_test.dart TEST=tests/ffi CoreLibraryReviewExempt: Only adding some pragmas. Change-Id: If687e54c676f275cc849b3fed526a13766ab331a Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349241 Reviewed-by: Alexander Markov --- runtime/tools/ffi/sdk_lib_ffi_generator.dart | 6 + .../frontend/kernel_binary_flowgraph.cc | 11 +- .../frontend/kernel_binary_flowgraph.h | 4 +- runtime/vm/compiler/frontend/kernel_to_il.cc | 16 +- sdk/lib/_internal/vm/lib/ffi_patch.dart | 44 ++++ sdk/lib/ffi/ffi.dart | 22 ++ tests/ffi/ffi.status | 9 + .../ffi/vmspecific_pointer_load_il_test.dart | 222 ++++++++++++++++++ 8 files changed, 323 insertions(+), 11 deletions(-) create mode 100644 tests/ffi/vmspecific_pointer_load_il_test.dart diff --git a/runtime/tools/ffi/sdk_lib_ffi_generator.dart b/runtime/tools/ffi/sdk_lib_ffi_generator.dart index 66ab886912b..ce6cd46e19b 100644 --- a/runtime/tools/ffi/sdk_lib_ffi_generator.dart +++ b/runtime/tools/ffi/sdk_lib_ffi_generator.dart @@ -237,6 +237,7 @@ $platform$truncate$alignment external void operator []=(int index, $dartType va /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer<$nativeType> operator +(int offset) => Pointer.fromAddress(address + sizeOf<$nativeType>() * offset); /// A pointer to the [offset]th [$nativeType] before this one. @@ -250,6 +251,7 @@ $platform$truncate$alignment external void operator []=(int index, $dartType va /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer<$nativeType> operator -(int offset) => Pointer.fromAddress(address - sizeOf<$nativeType>() * offset); $asTypedList @@ -308,15 +310,19 @@ void generatePatchExtension( @patch extension ${nativeType}Pointer on Pointer<$nativeType> { @patch + @pragma("vm:prefer-inline") $dartType get value => _load$nativeType(this, 0); @patch + @pragma("vm:prefer-inline") set value($dartType value) => _store$nativeType(this, 0, value); @patch + @pragma("vm:prefer-inline") $dartType operator [](int index) => _load$nativeType(this, ${sizeTimes}index); @patch + @pragma("vm:prefer-inline") operator []=(int index, $dartType value) => _store$nativeType(this, ${sizeTimes}index, value); $asTypedList diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc index 30a05c3f521..d218f63d31d 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc @@ -1648,8 +1648,10 @@ Fragment StreamingFlowGraphBuilder::AllocateContext( return flow_graph_builder_->AllocateContext(context_slots); } -Fragment StreamingFlowGraphBuilder::LoadNativeField(const Slot& field) { - return flow_graph_builder_->LoadNativeField(field); +Fragment StreamingFlowGraphBuilder::LoadNativeField( + const Slot& field, + InnerPointerAccess loads_inner_pointer) { + return flow_graph_builder_->LoadNativeField(field, loads_inner_pointer); } Fragment StreamingFlowGraphBuilder::StoreLocal(TokenPosition position, @@ -6241,8 +6243,9 @@ Fragment StreamingFlowGraphBuilder::BuildFfiCall() { Fragment code; // Push the target function pointer passed as Pointer object. code += BuildExpression(); - // This can only be Pointer, so it is always safe to LoadUntagged. - code += B->LoadUntagged(compiler::target::PointerBase::data_offset()); + // This can only be Pointer, so the data field points to unmanaged memory. + code += LoadNativeField(Slot::PointerBase_data(), + InnerPointerAccess::kCannotBeInnerPointer); code += B->ConvertUntaggedToUnboxed(kUnboxedFfiIntPtr); // Skip (empty) named arguments list. diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h index cd69b6a0457..5785b089230 100644 --- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h +++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.h @@ -215,7 +215,9 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper { const Class& klass, intptr_t argument_count); Fragment AllocateContext(const ZoneGrowableArray& context_slots); - Fragment LoadNativeField(const Slot& field); + Fragment LoadNativeField(const Slot& field, + InnerPointerAccess loads_inner_pointer = + InnerPointerAccess::kNotUntagged); Fragment StoreLocal(TokenPosition position, LocalVariable* variable); Fragment StoreStaticField(TokenPosition position, const Field& field); Fragment StringInterpolate(TokenPosition position); diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc index a0d7e5cba26..aa02738e8c8 100644 --- a/runtime/vm/compiler/frontend/kernel_to_il.cc +++ b/runtime/vm/compiler/frontend/kernel_to_il.cc @@ -1453,18 +1453,20 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( compiler::ffi::ElementTypedDataCid(ffi_type_arg_cid); ASSERT_EQUAL(function.NumParameters(), 2); - LocalVariable* arg_pointer = parsed_function_->RawParameterVariable(0); + // Argument can be a TypedData for loads on struct fields. + LocalVariable* arg_typed_data_base = + parsed_function_->RawParameterVariable(0); LocalVariable* arg_offset = parsed_function_->RawParameterVariable(1); body += LoadLocal(arg_offset); body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); LocalVariable* arg_offset_not_null = MakeTemporary(); - body += LoadLocal(arg_pointer); + body += LoadLocal(arg_typed_data_base); body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); // No GC from here til LoadIndexed. body += LoadNativeField(Slot::PointerBase_data(), - InnerPointerAccess::kCannotBeInnerPointer); + InnerPointerAccess::kMayBeInnerPointer); body += LoadLocal(arg_offset_not_null); body += UnboxTruncate(kUnboxedFfiIntPtr); body += LoadIndexed(typed_data_cid, /*index_scale=*/1, @@ -1520,7 +1522,9 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( const classid_t typed_data_cid = compiler::ffi::ElementTypedDataCid(ffi_type_arg_cid); - LocalVariable* arg_pointer = parsed_function_->RawParameterVariable(0); + // Argument can be a TypedData for stores on struct fields. + LocalVariable* arg_typed_data_base = + parsed_function_->RawParameterVariable(0); LocalVariable* arg_offset = parsed_function_->RawParameterVariable(1); LocalVariable* arg_value = parsed_function_->RawParameterVariable(2); @@ -1532,11 +1536,11 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod( body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); LocalVariable* arg_value_not_null = MakeTemporary(); - body += LoadLocal(arg_pointer); // Pointer. + body += LoadLocal(arg_typed_data_base); // Pointer. body += CheckNullOptimized(String::ZoneHandle(Z, function.name())); // No GC from here til StoreIndexed. body += LoadNativeField(Slot::PointerBase_data(), - InnerPointerAccess::kCannotBeInnerPointer); + InnerPointerAccess::kMayBeInnerPointer); body += LoadLocal(arg_offset_not_null); body += UnboxTruncate(kUnboxedFfiIntPtr); body += LoadLocal(arg_value_not_null); diff --git a/sdk/lib/_internal/vm/lib/ffi_patch.dart b/sdk/lib/_internal/vm/lib/ffi_patch.dart index e22aa88240f..6f48328a400 100644 --- a/sdk/lib/_internal/vm/lib/ffi_patch.dart +++ b/sdk/lib/_internal/vm/lib/ffi_patch.dart @@ -572,15 +572,19 @@ extension NativeFunctionPointer @patch extension Int8Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadInt8(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeInt8(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadInt8(this, index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeInt8(this, index, value); @patch @@ -605,15 +609,19 @@ extension Int8Pointer on Pointer { @patch extension Int16Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadInt16(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeInt16(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadInt16(this, 2 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeInt16(this, 2 * index, value); @patch @@ -638,15 +646,19 @@ extension Int16Pointer on Pointer { @patch extension Int32Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadInt32(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeInt32(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadInt32(this, 4 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeInt32(this, 4 * index, value); @patch @@ -671,15 +683,19 @@ extension Int32Pointer on Pointer { @patch extension Int64Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadInt64(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeInt64(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadInt64(this, 8 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeInt64(this, 8 * index, value); @patch @@ -704,15 +720,19 @@ extension Int64Pointer on Pointer { @patch extension Uint8Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadUint8(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeUint8(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadUint8(this, index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeUint8(this, index, value); @patch @@ -737,15 +757,19 @@ extension Uint8Pointer on Pointer { @patch extension Uint16Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadUint16(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeUint16(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadUint16(this, 2 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeUint16(this, 2 * index, value); @patch @@ -770,15 +794,19 @@ extension Uint16Pointer on Pointer { @patch extension Uint32Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadUint32(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeUint32(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadUint32(this, 4 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeUint32(this, 4 * index, value); @patch @@ -803,15 +831,19 @@ extension Uint32Pointer on Pointer { @patch extension Uint64Pointer on Pointer { @patch + @pragma("vm:prefer-inline") int get value => _loadUint64(this, 0); @patch + @pragma("vm:prefer-inline") set value(int value) => _storeUint64(this, 0, value); @patch + @pragma("vm:prefer-inline") int operator [](int index) => _loadUint64(this, 8 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, int value) => _storeUint64(this, 8 * index, value); @patch @@ -836,15 +868,19 @@ extension Uint64Pointer on Pointer { @patch extension FloatPointer on Pointer { @patch + @pragma("vm:prefer-inline") double get value => _loadFloat(this, 0); @patch + @pragma("vm:prefer-inline") set value(double value) => _storeFloat(this, 0, value); @patch + @pragma("vm:prefer-inline") double operator [](int index) => _loadFloat(this, 4 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, double value) => _storeFloat(this, 4 * index, value); @patch @@ -869,15 +905,19 @@ extension FloatPointer on Pointer { @patch extension DoublePointer on Pointer { @patch + @pragma("vm:prefer-inline") double get value => _loadDouble(this, 0); @patch + @pragma("vm:prefer-inline") set value(double value) => _storeDouble(this, 0, value); @patch + @pragma("vm:prefer-inline") double operator [](int index) => _loadDouble(this, 8 * index); @patch + @pragma("vm:prefer-inline") operator []=(int index, double value) => _storeDouble(this, 8 * index, value); @patch @@ -902,15 +942,19 @@ extension DoublePointer on Pointer { @patch extension BoolPointer on Pointer { @patch + @pragma("vm:prefer-inline") bool get value => _loadBool(this, 0); @patch + @pragma("vm:prefer-inline") set value(bool value) => _storeBool(this, 0, value); @patch + @pragma("vm:prefer-inline") bool operator [](int index) => _loadBool(this, index); @patch + @pragma("vm:prefer-inline") operator []=(int index, bool value) => _storeBool(this, index, value); } diff --git a/sdk/lib/ffi/ffi.dart b/sdk/lib/ffi/ffi.dart index 866318825bb..2782538476c 100644 --- a/sdk/lib/ffi/ffi.dart +++ b/sdk/lib/ffi/ffi.dart @@ -335,6 +335,7 @@ extension Int8Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -349,6 +350,7 @@ extension Int8Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -412,6 +414,7 @@ extension Int16Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -426,6 +429,7 @@ extension Int16Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -491,6 +495,7 @@ extension Int32Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -505,6 +510,7 @@ extension Int32Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -561,6 +567,7 @@ extension Int64Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -575,6 +582,7 @@ extension Int64Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -634,6 +642,7 @@ extension Uint8Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -648,6 +657,7 @@ extension Uint8Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -711,6 +721,7 @@ extension Uint16Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -725,6 +736,7 @@ extension Uint16Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -790,6 +802,7 @@ extension Uint32Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -804,6 +817,7 @@ extension Uint32Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -860,6 +874,7 @@ extension Uint64Pointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -874,6 +889,7 @@ extension Uint64Pointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -939,6 +955,7 @@ extension FloatPointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -953,6 +970,7 @@ extension FloatPointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -1009,6 +1027,7 @@ extension DoublePointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -1023,6 +1042,7 @@ extension DoublePointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); @@ -1074,6 +1094,7 @@ extension BoolPointer on Pointer { /// Also `(this + offset).value` is equivalent to `this[offset]`, /// and similarly for setting. @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator +(int offset) => Pointer.fromAddress(address + sizeOf() * offset); @@ -1088,6 +1109,7 @@ extension BoolPointer on Pointer { /// Also, `(this - offset).value` is equivalent to `this[-offset]`, /// and similarly for setting, @Since('3.3') + @pragma("vm:prefer-inline") Pointer operator -(int offset) => Pointer.fromAddress(address - sizeOf() * offset); } diff --git a/tests/ffi/ffi.status b/tests/ffi/ffi.status index 56ff2d398f9..018c564d4ce 100644 --- a/tests/ffi/ffi.status +++ b/tests/ffi/ffi.status @@ -42,11 +42,17 @@ function_callbacks_structs_by_value_generated_test: Pass, Slow [ $mode == product ] regress_47594_test: Skip # Profiler is not available in Product. +[ $nnbd == weak ] +vmspecific_pointer_load_il_test: SkipByDesign # Unsound NNBD boxes things because of nulls. + [ $system == android ] *: Pass, Slow # https://github.com/dart-lang/sdk/issues/38489 regress_47594_test: Skip # DartDev is not available on Android. vmspecific_native_finalizer_isolate_groups_test: Skip # SpawnUri not available on Android tester. +[ $system == fuchsia ] +vmspecific_pointer_load_il_test: SkipByDesign # Not bloating the Fuchsia test package with package:vm/testing/il_matchers.dart + [ $system == windows ] regress_47594_test: Skip # DynamicLibrary.process() is not available on Windows. @@ -71,6 +77,9 @@ vmspecific_regress_37511_test: SkipByDesign # Symbols are not exposed on purpose vmspecific_regress_37780_test: SkipByDesign # Symbols are not exposed on purpose and are not linked in Windows Precompiled. dartbug.com/40579 vmspecific_regress_51794_test: SkipByDesign # Symbols are not exposed on purpose and are not linked in Windows Precompiled. dartbug.com/40579 +[ $arch == arm || $arch == arm_x64 || $arch == ia32 || $arch == riscv32 || $arch == simarm || $arch == simriscv32 ] +vmspecific_pointer_load_il_test: SkipByDesign # 32 bit archs use uint32 for pointers and have more int conversions. + # These tests trigger and catch an abort (intentionally) and terminate the VM. # They're incompatible with ASAN because not all memory is freed when aborting and # with AppJit because the abort the VM before it can generate a snapshot. diff --git a/tests/ffi/vmspecific_pointer_load_il_test.dart b/tests/ffi/vmspecific_pointer_load_il_test.dart new file mode 100644 index 00000000000..55fe33e8ca5 --- /dev/null +++ b/tests/ffi/vmspecific_pointer_load_il_test.dart @@ -0,0 +1,222 @@ +// Copyright (c) 2024, 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. + +// Check that `Pointer` are not allocated before being passed into a load. + +import 'dart:ffi'; + +import 'package:expect/expect.dart'; +import 'package:ffi/ffi.dart'; +import 'package:vm/testing/il_matchers.dart'; + +void main() async { + using((arena) { + const length = 100; + final pointer = arena(100); + for (int i = 0; i < length; i++) { + pointer[i] = i; + } + Expect.equals( + 10, + testOffset(pointer), + ); + Expect.equals( + 10, + testAllocate(pointer), + ); + Expect.equals( + 45, + testHoist(pointer), + ); + print(globalVar); + }); +} + +@pragma('vm:never-inline') +@pragma('vm:testing:print-flow-graph') +int testOffset(Pointer pointer) { + // `pointer2` is not allocated. + final pointer2 = pointer + 10; + return pointer2.value; +} + +void matchIL$testOffset(FlowGraph graph) { + graph.dump(); + graph.match([ + match.block('Graph', [ + 'int 10' << match.UnboxedConstant(value: 10), + 'int 0' << match.UnboxedConstant(value: 0), + ]), + match.block('Function', [ + 'pointer' << + match.Parameter( + index: 0, + ), + 'pointer.address untagged' << + match.LoadField( + 'pointer', + slot: 'PointerBase.data', + ), + 'pointer.address int64' << + match.IntConverter( + 'pointer.address untagged', + from: 'untagged', + to: 'int64', + ), + 'pointer2.address int64' << + match.BinaryInt64Op( + 'pointer.address int64', + 'int 10', + ), + // `pointer2` is not allocated. + 'pointer2.address untagged' << + match.IntConverter( + 'pointer2.address int64', + from: 'int64', + to: 'untagged', + ), + 'pointer2.value' << + match.LoadIndexed( + 'pointer2.address untagged', + 'int 0', + ), + match.Return('pointer2.value'), + ]), + ]); +} + +class A { + final int i; + + A(this.i); +} + +A? globalVar; + +@pragma('vm:never-inline') +@pragma('vm:testing:print-flow-graph') +int testAllocate(Pointer pointer) { + final pointer2 = pointer + 10; + globalVar = A(10); + return pointer2.value; +} + +void matchIL$testAllocate(FlowGraph graph) { + graph.dump(); + graph.match([ + match.block('Graph', [ + 'int 10' << match.UnboxedConstant(value: 10), + 'int 0' << match.UnboxedConstant(value: 0), + ]), + match.block('Function', [ + 'pointer' << + match.Parameter( + index: 0, + ), + 'pointer.address untagged' << + match.LoadField( + 'pointer', + slot: 'PointerBase.data', + ), + 'pointer.address int64' << + match.IntConverter( + 'pointer.address untagged', + from: 'untagged', + to: 'int64', + ), + 'pointer2.address int64' << + match.BinaryInt64Op( + 'pointer.address int64', + 'int 10', + ), + 'pointer2.address untagged' << + match.IntConverter( + 'pointer2.address int64', + from: 'int64', + to: 'untagged', + ), + // The untagged pointer2.address can live through an allocation + // even though it is marked `InnerPointerAccess::kMayBeInnerPointer` + // because its cid is a Pointer cid. + match.AllocateObject(), + match.StoreStaticField(match.any), + 'pointer2.value' << + match.LoadIndexed( + 'pointer2.address untagged', + 'int 0', + ), + match.Return('pointer2.value'), + ]), + ]); +} + +@pragma('vm:never-inline') +@pragma('vm:testing:print-flow-graph') +int testHoist(Pointer pointer) { + int result = 0; + for (int i = 0; i < 10; i++) { + globalVar = A(10); + // The address load is hoisted out of the loop. + // The indexed load is _not_ hoisted out of the loop. + result += pointer[i]; + } + return result; +} + +void matchIL$testHoist(FlowGraph graph) { + graph.dump(); + graph.match([ + match.block('Graph', [ + 'int 0' << match.UnboxedConstant(value: 0), + 'int 10' << match.UnboxedConstant(value: 10), + 'int 1' << match.UnboxedConstant(value: 1), + ]), + match.block('Function', [ + 'pointer' << + match.Parameter( + index: 0, + ), + 'pointer.address' << + match.LoadField( + 'pointer', + slot: 'PointerBase.data', + ), + match.Goto('B1'), + ]), + 'B1' << + match.block('Join', [ + match.CheckStackOverflow(), + match.Branch(match.RelationalOp(match.any, match.any, kind: '<'), + ifTrue: 'B2', ifFalse: 'B3'), + ]), + 'B2' << + match.block('Target', [ + // Do some allocation. + match.AllocateObject(), + match.StoreStaticField(match.any), + // Do a load indexed with the untagged pointer.address that is + // hoisted out of the loop. + 'pointer[i]' << + match.LoadIndexed( + 'pointer.address', + match.any, // i + ), + 'result' << + match.BinaryInt64Op( + match.any, + 'pointer[i]', + ), + 'i' << + match.BinaryInt64Op( + match.any, + match.any, + ), + match.Goto('B1'), + ]), + 'B3' << + match.block('Target', [ + match.Return(match.any), + ]), + ]); +}