[vm/ffi] Fix garbage collection of ffi.Pointer subtypes.

Fixes dart-lang/sdk#36125.

Change-Id: Ib9069d1e15684c785b86954ed8c14e5fdde35fec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/95652
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>

Author: sjindel-google

runtime/lib/ffi.cc

@@ -231,14 +231,15 @@ DEFINE_NATIVE_ENTRY(Ffi_allocate, 1, 1) {
   CheckRange(argCount, 1, max_count, "count");
 
   size_t size = ffi::ElementSizeInBytes(type_cid) * count;
-  uint8_t* memory = reinterpret_cast<uint8_t*>(malloc(size));
-  if (memory == NULL) {
+  intptr_t memory = reinterpret_cast<intptr_t>(malloc(size));
+  if (memory == 0) {
     const String& error = String::Handle(String::NewFormatted(
         "allocating (%" Pd ") bytes of memory failed", size));
     Exceptions::ThrowArgumentError(error);
   }
 
-  RawPointer* result = Pointer::New(type_arg, memory);
+  RawPointer* result =
+      Pointer::New(type_arg, Integer::Handle(zone, Integer::New(memory)));
   return result;
 }
 
@@ -250,27 +251,27 @@ DEFINE_NATIVE_ENTRY(Ffi_fromAddress, 1, 1) {
   CheckIsConcreteNativeType(native_type);
   GET_NON_NULL_NATIVE_ARGUMENT(Integer, arg_ptr, arguments->NativeArgAt(0));
 
-  uint8_t* address = reinterpret_cast<uint8_t*>(arg_ptr.AsInt64Value());
-  // TODO(dacoharkes): should this return NULL if addres is 0?
+  // TODO(dacoharkes): should this return NULL if address is 0?
   // https://github.com/dart-lang/sdk/issues/35756
 
   RawPointer* result =
-      Pointer::New(native_type, address, type_arg.type_class_id());
+      Pointer::New(native_type, arg_ptr, type_arg.type_class_id());
   return result;
 }
 
 DEFINE_NATIVE_ENTRY(Ffi_elementAt, 0, 2) {
   GET_NON_NULL_NATIVE_ARGUMENT(Pointer, pointer, arguments->NativeArgAt(0));
   GET_NON_NULL_NATIVE_ARGUMENT(Integer, index, arguments->NativeArgAt(1));
   AbstractType& pointer_type_arg =
-      AbstractType::Handle(pointer.type_argument());
+      AbstractType::Handle(zone, pointer.type_argument());
   CheckSized(pointer_type_arg);
 
   classid_t class_id = pointer_type_arg.type_class_id();
-  uint8_t* address = pointer.GetCMemoryAddress();
-  uint8_t* address_new =
-      address + index.AsInt64Value() * ffi::ElementSizeInBytes(class_id);
-  RawPointer* result = Pointer::New(pointer_type_arg, address_new);
+  Integer& address = Integer::Handle(zone, pointer.GetCMemoryAddress());
+  address =
+      Integer::New(address.AsInt64Value() +
+                   index.AsInt64Value() * ffi::ElementSizeInBytes(class_id));
+  RawPointer* result = Pointer::New(pointer_type_arg, address);
   return result;
 }
 
@@ -280,9 +281,11 @@ DEFINE_NATIVE_ENTRY(Ffi_offsetBy, 0, 2) {
   AbstractType& pointer_type_arg =
       AbstractType::Handle(pointer.type_argument());
 
-  uint8_t* address = pointer.GetCMemoryAddress();
-  uint8_t* address_new = address + offset.AsInt64Value();
-  RawPointer* result = Pointer::New(pointer_type_arg, address_new);
+  intptr_t address =
+      Integer::Handle(zone, pointer.GetCMemoryAddress()).AsInt64Value() +
+      offset.AsInt64Value();
+  RawPointer* result = Pointer::New(
+      pointer_type_arg, Integer::Handle(zone, Integer::New(address)));
   return result;
 }
 
@@ -294,7 +297,7 @@ DEFINE_NATIVE_ENTRY(Ffi_cast, 1, 1) {
       type_args.TypeAtNullSafe(Pointer::kNativeTypeArgPos));
   CheckIsConcreteNativeType(native_type);
 
-  uint8_t* address = pointer.GetCMemoryAddress();
+  const Integer& address = Integer::Handle(zone, pointer.GetCMemoryAddress());
   RawPointer* result =
       Pointer::New(native_type, address, type_arg.type_class_id());
   return result;
@@ -303,35 +306,37 @@ DEFINE_NATIVE_ENTRY(Ffi_cast, 1, 1) {
 DEFINE_NATIVE_ENTRY(Ffi_free, 0, 1) {
   GET_NON_NULL_NATIVE_ARGUMENT(Pointer, pointer, arguments->NativeArgAt(0));
 
-  uint8_t* address = pointer.GetCMemoryAddress();
-  free(address);
-  pointer.SetCMemoryAddress(0);
+  const Integer& address = Integer::Handle(zone, pointer.GetCMemoryAddress());
+  free(reinterpret_cast<void*>(address.AsInt64Value()));
+  pointer.SetCMemoryAddress(Integer::Handle(zone, Integer::New(0)));
   return Object::null();
 }
 
 DEFINE_NATIVE_ENTRY(Ffi_address, 0, 1) {
   GET_NON_NULL_NATIVE_ARGUMENT(Pointer, pointer, arguments->NativeArgAt(0));
-
-  uint8_t* address = pointer.GetCMemoryAddress();
-  intptr_t int_ptr = reinterpret_cast<intptr_t>(address);
-  return Integer::NewFromUint64(int_ptr);
+  return pointer.GetCMemoryAddress();
 }
 
-static RawInstance* BoxLoadPointer(uint8_t* address,
+static RawInstance* BoxLoadPointer(Zone* zone,
+                                   uint8_t* address,
                                    const AbstractType& instance_type_arg,
                                    intptr_t type_cid) {
   // TODO(dacoharkes): should this return NULL if addres is 0?
   // https://github.com/dart-lang/sdk/issues/35756
-  if (address == 0) {  // 0 is the c++ null pointer
+  if (address == nullptr) {
     return Instance::null();
   }
   AbstractType& type_arg =
       AbstractType::Handle(TypeArguments::Handle(instance_type_arg.arguments())
                                .TypeAt(Pointer::kNativeTypeArgPos));
-  return Pointer::New(type_arg, address, type_cid);
+  return Pointer::New(
+      type_arg,
+      Integer::Handle(zone, Integer::New(reinterpret_cast<intptr_t>(address))),
+      type_cid);
 }
 
-static RawInstance* LoadValue(uint8_t* address,
+static RawInstance* LoadValue(Zone* zone,
+                              uint8_t* address,
                               const AbstractType& instance_type_arg) {
   classid_t type_cid = instance_type_arg.type_class_id();
   switch (type_cid) {
@@ -360,7 +365,7 @@ static RawInstance* LoadValue(uint8_t* address,
     case kFfiPointerCid:
     default:
       ASSERT(IsPointerType(instance_type_arg));
-      return BoxLoadPointer(*reinterpret_cast<uint8_t**>(address),
+      return BoxLoadPointer(zone, *reinterpret_cast<uint8_t**>(address),
                             instance_type_arg, type_cid);
   }
 }
@@ -373,14 +378,17 @@ DEFINE_NATIVE_ENTRY(Ffi_load, 1, 1) {
   CheckSized(pointer_type_arg);
   ASSERT(DartAndCTypeCorrespond(pointer_type_arg, type_arg));
 
-  uint8_t* address = pointer.GetCMemoryAddress();
-  return LoadValue(address, pointer_type_arg);
+  uint8_t* address = reinterpret_cast<uint8_t*>(
+      Integer::Handle(pointer.GetCMemoryAddress()).AsInt64Value());
+  return LoadValue(zone, address, pointer_type_arg);
 }
 
-static void StoreValue(const Pointer& pointer,
+static void StoreValue(Zone* zone,
+                       const Pointer& pointer,
                        classid_t type_cid,
                        const Instance& new_value) {
-  uint8_t* address = pointer.GetCMemoryAddress();
+  uint8_t* address = reinterpret_cast<uint8_t*>(
+      Integer::Handle(pointer.GetCMemoryAddress()).AsInt64Value());
   AbstractType& pointer_type_arg =
       AbstractType::Handle(pointer.type_argument());
   switch (type_cid) {
@@ -428,14 +436,16 @@ static void StoreValue(const Pointer& pointer,
     case kFfiPointerCid:
     default: {
       ASSERT(IsPointerType(pointer_type_arg));
-      uint8_t* new_value_unwrapped = nullptr;
+      intptr_t new_value_unwrapped = 0;
       if (!new_value.IsNull()) {
         ASSERT(new_value.IsPointer());
-        new_value_unwrapped = AsPointer(new_value).GetCMemoryAddress();
+        new_value_unwrapped =
+            Integer::Handle(AsPointer(new_value).GetCMemoryAddress())
+                .AsInt64Value();
         // TODO(dacoharkes): should this return NULL if addres is 0?
         // https://github.com/dart-lang/sdk/issues/35756
       }
-      *reinterpret_cast<uint8_t**>(address) = new_value_unwrapped;
+      *reinterpret_cast<intptr_t*>(address) = new_value_unwrapped;
     } break;
   }
 }
@@ -450,7 +460,7 @@ DEFINE_NATIVE_ENTRY(Ffi_store, 0, 2) {
   ASSERT(DartAndCTypeCorrespond(pointer_type_arg, arg_type));
 
   classid_t type_cid = pointer_type_arg.type_class_id();
-  StoreValue(pointer, type_cid, new_value);
+  StoreValue(zone, pointer, type_cid, new_value);
   return Object::null();
 }
 
@@ -561,9 +571,7 @@ DEFINE_NATIVE_ENTRY(Ffi_asFunction, 1, 1) {
   // the function so that we can reuse the function for each c function with
   // the same signature.
   Context& context = Context::Handle(Context::New(1));
-  context.SetAt(0,
-                Object::Handle(Integer::NewFromUint64(
-                    reinterpret_cast<intptr_t>(pointer.GetCMemoryAddress()))));
+  context.SetAt(0, Integer::Handle(zone, pointer.GetCMemoryAddress()));
 
   RawClosure* raw_closure =
       Closure::New(Object::null_type_arguments(), Object::null_type_arguments(),
@@ -623,7 +631,8 @@ DEFINE_NATIVE_ENTRY(Ffi_fromFunction, 1, 1) {
   THR_Print("Ffi_fromFunction: %p\n", entryPoint);
   THR_Print("Ffi_fromFunction: %" Pd "\n", code.Size());
 
-  void* address = GenerateFfiInverseTrampoline(c_signature, entryPoint);
+  intptr_t address = reinterpret_cast<intptr_t>(
+      GenerateFfiInverseTrampoline(c_signature, entryPoint));
 
   TypeArguments& type_args = TypeArguments::Handle(zone);
   type_args = TypeArguments::New(1);
@@ -642,8 +651,8 @@ DEFINE_NATIVE_ENTRY(Ffi_fromFunction, 1, 1) {
 
   address = 0;  // https://github.com/dart-lang/sdk/issues/35761
 
-  Pointer& result = Pointer::Handle(
-      Pointer::New(native_function_type, reinterpret_cast<uint8_t*>(address)));
+  Pointer& result = Pointer::Handle(Pointer::New(
+      native_function_type, Integer::Handle(zone, Integer::New(address))));
 
   return result.raw();
 }

runtime/lib/ffi_dynamic_library.cc

@@ -51,8 +51,8 @@ DEFINE_NATIVE_ENTRY(Ffi_dl_lookup, 1, 2) {
   void* handle = dlib.GetHandle();
 
   dlerror();  // Clear any errors.
-  uint8_t* pointer =
-      reinterpret_cast<uint8_t*>(dlsym(handle, argSymbolName.ToCString()));
+  intptr_t pointer =
+      reinterpret_cast<intptr_t>(dlsym(handle, argSymbolName.ToCString()));
   char* error;
   if ((error = dlerror()) != NULL) {
     const String& msg = String::Handle(
@@ -62,7 +62,8 @@ DEFINE_NATIVE_ENTRY(Ffi_dl_lookup, 1, 2) {
 
   // TODO(dacoharkes): should this return NULL if addres is 0?
   // https://github.com/dart-lang/sdk/issues/35756
-  RawPointer* result = Pointer::New(type_arg, pointer);
+  RawPointer* result =
+      Pointer::New(type_arg, Integer::Handle(zone, Integer::New(pointer)));
   return result;
 #endif
 }

runtime/vm/ffi_trampoline_stubs_x64.cc

@@ -231,7 +231,8 @@ static void GenerateMarshalPointer(Assembler* assembler, Register reg) {
 
   // If not null but a Pointer, load the address.
   __ Bind(&not_null);
-  __ movq(reg, FieldAddress(reg, Pointer::address_offset()));
+  __ movq(reg, FieldAddress(reg, Pointer::c_memory_address_offset()));
+  GenerateMarshalInt64(assembler, reg);
   __ Bind(&done);
 }
 
@@ -256,9 +257,9 @@ static void GenerateUnmarshalPointer(Assembler* assembler,
   __ LoadObject(RAX, Object::null_object());
   __ jmp(&done);
 
-  // Backup result value (to avoid GC).
   __ Bind(&not_null);
-  GenerateSaveInt64GCSafe(assembler, RAX);
+  GenerateUnmarshalInt64(assembler);
+  __ pushq(RAX);
 
   // Allocate object (can call into runtime).
   __ movq(TMP, closure_dart);
@@ -269,8 +270,8 @@ static void GenerateUnmarshalPointer(Assembler* assembler,
   __ popq(TMP);  // Pop type arguments.
 
   // Store the result value.
-  GenerateLoadInt64GCSafe(assembler, RDX);
-  __ movq(FieldAddress(RAX, Pointer::address_offset()), RDX);
+  __ popq(RDX);
+  __ movq(FieldAddress(RAX, Pointer::c_memory_address_offset()), RDX);
   __ Bind(&done);
 }
 

runtime/vm/object.cc

@@ -7785,8 +7785,9 @@ RawString* Function::GetSource() const {
       uint16_t end_char = src.CharAt(end_token_pos().value());
       if ((end_char == ',') ||  // Case 1.
           (end_char == ')') ||  // Case 2.
-          (end_char == ';' && String::Handle(zone, name())
-                                  .Equals("<anonymous closure>"))) {  // Case 3.
+          (end_char == ';' &&
+           String::Handle(zone, name())
+               .Equals("<anonymous closure>"))) {  // Case 3.
         to_length = 0;
       }
     }
@@ -12738,28 +12739,25 @@ static int PrintVarInfo(char* buffer,
   const RawLocalVarDescriptors::VarInfoKind kind = info.kind();
   const int32_t index = info.index();
   if (kind == RawLocalVarDescriptors::kContextLevel) {
-    return Utils::SNPrint(buffer, len,
-                          "%2" Pd
-                          " %-13s level=%-3d"
-                          " begin=%-3d end=%d\n",
+    return Utils::SNPrint(buffer, len, "%2" Pd
+                                       " %-13s level=%-3d"
+                                       " begin=%-3d end=%d\n",
                           i, LocalVarDescriptors::KindToCString(kind), index,
                           static_cast<int>(info.begin_pos.value()),
                           static_cast<int>(info.end_pos.value()));
   } else if (kind == RawLocalVarDescriptors::kContextVar) {
     return Utils::SNPrint(
-        buffer, len,
-        "%2" Pd
-        " %-13s level=%-3d index=%-3d"
-        " begin=%-3d end=%-3d name=%s\n",
+        buffer, len, "%2" Pd
+                     " %-13s level=%-3d index=%-3d"
+                     " begin=%-3d end=%-3d name=%s\n",
         i, LocalVarDescriptors::KindToCString(kind), info.scope_id, index,
         static_cast<int>(info.begin_pos.Pos()),
         static_cast<int>(info.end_pos.Pos()), var_name.ToCString());
   } else {
     return Utils::SNPrint(
-        buffer, len,
-        "%2" Pd
-        " %-13s scope=%-3d index=%-3d"
-        " begin=%-3d end=%-3d name=%s\n",
+        buffer, len, "%2" Pd
+                     " %-13s scope=%-3d index=%-3d"
+                     " begin=%-3d end=%-3d name=%s\n",
         i, LocalVarDescriptors::KindToCString(kind), info.scope_id, index,
         static_cast<int>(info.begin_pos.Pos()),
         static_cast<int>(info.end_pos.Pos()), var_name.ToCString());
@@ -20914,7 +20912,7 @@ const char* ExternalTypedData::ToCString() const {
 }
 
 RawPointer* Pointer::New(const AbstractType& type_arg,
-                         uint8_t* c_memory_address,
+                         const Integer& c_memory_address,
                          intptr_t cid,
                          Heap::Space space) {
   Thread* thread = Thread::Current();
@@ -20941,7 +20939,8 @@ const char* Pointer::ToCString() const {
   String& type_args_name = String::Handle(type_args.UserVisibleName());
   return OS::SCreate(Thread::Current()->zone(), "Pointer%s: address=0x%" Px,
                      type_args_name.ToCString(),
-                     reinterpret_cast<uintptr_t>(GetCMemoryAddress()));
+                     static_cast<intptr_t>(
+                         Integer::Handle(GetCMemoryAddress()).AsInt64Value()));
 }
 
 RawDynamicLibrary* DynamicLibrary::New(void* handle, Heap::Space space) {

runtime/vm/object.h

@@ -8499,7 +8499,7 @@ class ByteBuffer : public AllStatic {
 class Pointer : public Instance {
  public:
   static RawPointer* New(const AbstractType& type_arg,
-                         uint8_t* c_memory_address,
+                         const Integer& c_memory_address,
                          intptr_t class_id = kFfiPointerCid,
                          Heap::Space space = Heap::kNew);
 
@@ -8509,20 +8509,17 @@ class Pointer : public Instance {
 
   static bool IsPointer(const Instance& obj);
 
-  uint8_t* GetCMemoryAddress() const {
-    ASSERT(!IsNull());
-    return raw_ptr()->c_memory_address_;
-  }
+  RawInteger* GetCMemoryAddress() const { return raw_ptr()->c_memory_address_; }
 
-  void SetCMemoryAddress(uint8_t* value) const {
-    StoreNonPointer(&raw_ptr()->c_memory_address_, value);
+  void SetCMemoryAddress(const Integer& value) const {
+    StorePointer(&raw_ptr()->c_memory_address_, value.raw());
   }
 
   static intptr_t type_arguments_offset() {
     return OFFSET_OF(RawPointer, type_arguments_);
   }
 
-  static intptr_t address_offset() {
+  static intptr_t c_memory_address_offset() {
     return OFFSET_OF(RawPointer, c_memory_address_);
   }
 

runtime/vm/raw_object.h

@@ -2263,8 +2263,8 @@ class RawPointer : public RawInstance {
   RAW_HEAP_OBJECT_IMPLEMENTATION(Pointer);
   VISIT_FROM(RawCompressed, type_arguments_)
   RawTypeArguments* type_arguments_;
-  VISIT_TO(RawCompressed, type_arguments_)
-  uint8_t* c_memory_address_;
+  RawInteger* c_memory_address_;
+  VISIT_TO(RawCompressed, c_memory_address_)
 
   friend class Pointer;
 };