[vm] Avoid field guard on Compound._typedDataBase

`KernelLoader::FinishClassLoading` was leaving guarded state of this
field in an inconsistent state: it was setting guarded cid to
`kDynamicCid`, but not setting guarded length to `kNoFixedLength`.

As a result in JIT mode we would generate stale length guards
for these fields and these guards would inhibit allocation
sinking.

This CL fixes this.

Issue #56705

TEST=vm/cc/Ffi_StructSinking
[email protected]

Change-Id: I2eae6447555a7c1dfa25591e51815936aa3dbaf3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/384842
Commit-Queue: Slava Egorov <[email protected]>
Reviewed-by: Daco Harkes <[email protected]>

runtime/vm/compiler/backend/il_test_helper.h

@@ -448,6 +448,21 @@ class FlowGraphBuilderHelper {
   GrowableArray<PendingPhiInput> pending_phis_;
 };
 
+template <typename... Args>
+ObjectPtr Invoke(const Library& lib, const char* name, const Args&... args) {
+  Thread* thread = Thread::Current();
+  Dart_Handle api_lib = Api::NewHandle(thread, lib.ptr());
+  Dart_Handle argv[] = {Api::NewHandle(thread, args.ptr())...};
+  Dart_Handle result;
+  {
+    TransitionVMToNative transition(thread);
+    result =
+        Dart_Invoke(api_lib, NewString(name), /*argc=*/sizeof...(Args), argv);
+    EXPECT_VALID(result);
+  }
+  return Api::UnwrapHandle(result);
+}
+
 }  // namespace dart
 
 #endif  // RUNTIME_VM_COMPILER_BACKEND_IL_TEST_HELPER_H_

runtime/vm/compiler/backend/redundancy_elimination_test.cc

@@ -1957,6 +1957,50 @@ ISOLATE_UNIT_TEST_CASE(AllocationSinking_NoViewDataMaterialization) {
 
 #endif  // !defined(TARGET_ARCH_IA32)
 
+// Verify that temporary |Pointer| and |Struct| allocations are sunk away
+// in JIT mode.
+ISOLATE_UNIT_TEST_CASE(Ffi_StructSinking) {
+  const char* kScript =
+      R"(
+      import 'dart:ffi';
+
+      final class S extends Struct {
+        @Int8()
+        external int a;
+      }
+
+      int test(int addr) =>
+        Pointer<S>.fromAddress(addr)[0].a;
+      )";
+
+  const auto& lib = Library::Handle(LoadTestScript(kScript, NoopNativeLookup));
+  EXPECT(!lib.IsNull());
+  const auto& function = Function::ZoneHandle(GetFunction(lib, "test"));
+  EXPECT(!function.IsNull());
+
+  // Run the unoptimized code.
+  uint8_t buffer[10] = {42};
+  auto& result = Object::Handle(Invoke(
+      lib, "test",
+      Integer::Handle(Integer::New(reinterpret_cast<int64_t>(&buffer)))));
+  EXPECT_EQ(42, Integer::Cast(result).Value());
+
+  // Optimize 'test' function.
+  TestPipeline pipeline(function, CompilerPass::kJIT);
+  FlowGraph* flow_graph = pipeline.RunPasses({});
+
+  FlowGraphPrinter::PrintGraph("aaa", flow_graph);
+
+  // Verify that it does not contain any allocations or calls.
+  for (auto block : flow_graph->reverse_postorder()) {
+    for (auto instr : block->instructions()) {
+      EXPECT_PROPERTY(instr, !it.IsAllocateObject());
+      EXPECT_PROPERTY(instr, !it.IsStaticCall() && !it.IsInstanceCall() &&
+                                 !it.IsPolymorphicInstanceCall());
+    }
+  }
+}
+
 // Regression test for https://github.com/dart-lang/sdk/issues/51220.
 // Verifies that deoptimization at the hoisted BinarySmiOp
 // doesn't result in the infinite re-optimization loop.

runtime/vm/kernel_loader.cc

@@ -1540,14 +1540,18 @@ void KernelLoader::FinishClassLoading(const Class& klass,
     // TypedData in them, without using guards because they are force
     // optimized. We immediately set the guarded_cid_ to kDynamicCid, which
     // is effectively the same as calling this method first with Pointer and
-    // subsequently with TypedData with field guards.
+    // subsequently with TypedData with field guards. We also set
+    // guarded_list_length_ to kNoFixedLength for similar reasons.
     if (klass.UserVisibleName() == Symbols::Compound().ptr() &&
         Library::Handle(Z, klass.library()).url() == Symbols::DartFfi().ptr()) {
       ASSERT_EQUAL(fields_.length(), 2);
       ASSERT(String::Handle(Z, fields_[0]->name())
                  .StartsWith(Symbols::_typedDataBase()));
       fields_[0]->set_guarded_cid(kDynamicCid);
       fields_[0]->set_is_nullable(true);
+      fields_[0]->set_guarded_list_length(Field::kNoFixedLength);
+      fields_[0]->set_guarded_list_length_in_object_offset(
+          Field::kUnknownLengthOffset);
     }
 
     // Check that subclasses of AbiSpecificInteger have a mapping for the