Reland "[sandbox] Sandboxify WasmExportedFunctionData::sig"

This is a reland of commit 6ec7be21

The issues that caused the CL to be reverted appear to be unrelated
to this change as they still occurred after the revert.

Original change's description:
> [sandbox] Sandboxify WasmExportedFunctionData::sig
>
> This CL changes the WasmExportedFunctionData class to store a direct
> ExternalPointer to the wasm::FunctionSig instead of referencing it
> through a Foreign. This in turn makes it possible to use a unique
> pointer tag for that external pointer when the sandbox is enabled.
>
> Drive-by: move WasmInternalFunction::call_target external pointer to the
> end of the object, in line with other external pointer fields.
>
> Bug: v8:10391, v8:12949
> Change-Id: Ic3ff622a075c9eaa2f8d8835803437466290c928
> Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3829086
> Commit-Queue: Samuel Groß <saelo@chromium.org>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82523}

Bug: v8:10391, v8:12949
Change-Id: I108810ce86b95289dfb6d6377535813deac79a9f
Cq-Include-Trybots: luci.v8.try:v8_linux64_heap_sandbox_dbg_ng,v8_linux_arm64_sim_heap_sandbox_dbg_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3838109Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82565}

include/v8-internal.h

@@ -390,7 +390,8 @@ constexpr uint64_t kAllExternalPointerTypeTags[] = {
   V(kAccessorInfoSetterTag,                 sandboxed, TAG(19)) \
   V(kWasmInternalFunctionCallTargetTag,     sandboxed, TAG(20)) \
   V(kWasmTypeInfoNativeTypeTag,             sandboxed, TAG(21)) \
-  V(kWasmContinuationJmpbufTag,             sandboxed, TAG(22))
+  V(kWasmExportedFunctionDataSignatureTag,  sandboxed, TAG(22)) \
+  V(kWasmContinuationJmpbufTag,             sandboxed, TAG(23))
 
 // All external pointer tags.
 #define ALL_EXTERNAL_POINTER_TAGS(V) \

src/builtins/x64/builtins-x64.cc

@@ -2986,17 +2986,11 @@ void LoadFunctionDataAndWasmInstance(MacroAssembler* masm,
 void LoadValueTypesArray(MacroAssembler* masm, Register function_data,
                          Register valuetypes_array_ptr, Register return_count,
                          Register param_count) {
-  Register foreign_signature = valuetypes_array_ptr;
-  __ LoadAnyTaggedField(
-      foreign_signature,
-      MemOperand(function_data,
-                 WasmExportedFunctionData::kSignatureOffset - kHeapObjectTag));
-  Register signature = foreign_signature;
+  Register signature = valuetypes_array_ptr;
   __ LoadExternalPointerField(
       signature,
-      FieldOperand(foreign_signature, Foreign::kForeignAddressOffset),
-      kForeignForeignAddressTag, kScratchRegister);
-  foreign_signature = no_reg;
+      FieldOperand(function_data, WasmExportedFunctionData::kSigOffset),
+      kWasmExportedFunctionDataSignatureTag, kScratchRegister);
   __ movq(return_count,
           MemOperand(signature, wasm::FunctionSig::kReturnCountOffset));
   __ movq(param_count,

src/diagnostics/objects-printer.cc

@@ -2053,9 +2053,10 @@ void WasmFunctionData::WasmFunctionDataPrint(std::ostream& os) {
 void WasmExportedFunctionData::WasmExportedFunctionDataPrint(std::ostream& os) {
   PrintHeader(os, "WasmExportedFunctionData");
   WasmFunctionDataPrint(os);
+  Isolate* isolate = GetIsolateForSandbox(*this);
   os << "\n - instance: " << Brief(instance());
   os << "\n - function_index: " << function_index();
-  os << "\n - signature: " << Brief(signature());
+  os << "\n - signature: " << reinterpret_cast<void*>(sig(isolate));
   os << "\n - wrapper_budget: " << wrapper_budget();
   os << "\n";
 }

src/heap/factory.cc

@@ -1676,9 +1676,8 @@ Handle<WasmResumeData> Factory::NewWasmResumeData(
 Handle<WasmExportedFunctionData> Factory::NewWasmExportedFunctionData(
     Handle<CodeT> export_wrapper, Handle<WasmInstanceObject> instance,
     Address call_target, Handle<Object> ref, int func_index,
-    Address sig_address, int wrapper_budget, Handle<Map> rtt,
+    const wasm::FunctionSig* sig, int wrapper_budget, Handle<Map> rtt,
     wasm::Promise promise) {
-  Handle<Foreign> sig_foreign = NewForeign(sig_address);
   Handle<WasmInternalFunction> internal =
       NewWasmInternalFunction(call_target, Handle<HeapObject>::cast(ref), rtt);
   Map map = *wasm_exported_function_data_map();
@@ -1691,7 +1690,7 @@ Handle<WasmExportedFunctionData> Factory::NewWasmExportedFunctionData(
   result.set_wrapper_code(*export_wrapper);
   result.set_instance(*instance);
   result.set_function_index(func_index);
-  result.set_signature(*sig_foreign);
+  result.init_sig(isolate(), sig);
   result.set_wrapper_budget(wrapper_budget);
   // We can't skip the write barrier when V8_EXTERNAL_CODE_SPACE is enabled
   // because in this case the CodeT (CodeDataContainer) objects are not

src/heap/factory.h

@@ -634,7 +634,7 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
   Handle<WasmExportedFunctionData> NewWasmExportedFunctionData(
       Handle<CodeT> export_wrapper, Handle<WasmInstanceObject> instance,
       Address call_target, Handle<Object> ref, int func_index,
-      Address sig_address, int wrapper_budget, Handle<Map> rtt,
+      const wasm::FunctionSig* sig, int wrapper_budget, Handle<Map> rtt,
       wasm::Promise promise);
   Handle<WasmApiFunctionRef> NewWasmApiFunctionRef(
       Handle<JSReceiver> callable, wasm::Suspend suspend,

src/objects/object-macros.h

@@ -398,11 +398,11 @@
                 kRelaxedStore);                                      \
   }
 
-#define DECL_EXTERNAL_POINTER_ACCESSORS(name, type)                 \
-  inline type name() const;                                         \
-  inline type name(i::Isolate* isolate_for_sandbox) const;          \
-  inline void init_##name(i::Isolate* isolate, type initial_value); \
-  inline void set_##name(i::Isolate* isolate, type value);
+#define DECL_EXTERNAL_POINTER_ACCESSORS(name, type)                       \
+  inline type name() const;                                               \
+  inline type name(i::Isolate* isolate_for_sandbox) const;                \
+  inline void init_##name(i::Isolate* isolate, const type initial_value); \
+  inline void set_##name(i::Isolate* isolate, const type value);
 
 #define EXTERNAL_POINTER_ACCESSORS(holder, name, type, offset, tag)         \
   type holder::name() const {                                               \
@@ -417,20 +417,20 @@
         Object::ReadExternalPointerField<tag>(offset, isolate_for_sandbox); \
     return reinterpret_cast<type>(reinterpret_cast<C2440*>(result));        \
   }                                                                         \
-  void holder::init_##name(i::Isolate* isolate, type initial_value) {       \
+  void holder::init_##name(i::Isolate* isolate, const type initial_value) { \
     /* This is a workaround for MSVC error C2440 not allowing  */           \
     /* reinterpret casts to the same type. */                               \
     struct C2440 {};                                                        \
-    Address the_value =                                                     \
-        reinterpret_cast<Address>(reinterpret_cast<C2440*>(initial_value)); \
+    Address the_value = reinterpret_cast<Address>(                          \
+        reinterpret_cast<const C2440*>(initial_value));                     \
     Object::InitExternalPointerField<tag>(offset, isolate, the_value);      \
   }                                                                         \
-  void holder::set_##name(i::Isolate* isolate, type value) {                \
+  void holder::set_##name(i::Isolate* isolate, const type value) {          \
     /* This is a workaround for MSVC error C2440 not allowing  */           \
     /* reinterpret casts to the same type. */                               \
     struct C2440 {};                                                        \
     Address the_value =                                                     \
-        reinterpret_cast<Address>(reinterpret_cast<C2440*>(value));         \
+        reinterpret_cast<Address>(reinterpret_cast<const C2440*>(value));   \
     Object::WriteExternalPointerField<tag>(offset, isolate, the_value);     \
   }
 

src/objects/objects-body-descriptors-inl.h

@@ -742,6 +742,27 @@ class WasmApiFunctionRef::BodyDescriptor final : public BodyDescriptorBase {
   static inline int SizeOf(Map map, HeapObject object) { return kSize; }
 };
 
+class WasmExportedFunctionData::BodyDescriptor final
+    : public BodyDescriptorBase {
+ public:
+  static bool IsValidSlot(Map map, HeapObject obj, int offset) {
+    UNREACHABLE();
+  }
+
+  template <typename ObjectVisitor>
+  static inline void IterateBody(Map map, HeapObject obj, int object_size,
+                                 ObjectVisitor* v) {
+    WasmFunctionData::BodyDescriptor::IterateBody<ObjectVisitor>(
+        map, obj, object_size, v);
+    IteratePointers(obj, kStartOfStrongFieldsOffset, kEndOfStrongFieldsOffset,
+                    v);
+    v->VisitExternalPointer(obj, obj.RawExternalPointerField(kSigOffset),
+                            kWasmExportedFunctionDataSignatureTag);
+  }
+
+  static inline int SizeOf(Map map, HeapObject object) { return kSize; }
+};
+
 class WasmInternalFunction::BodyDescriptor final : public BodyDescriptorBase {
  public:
   static bool IsValidSlot(Map map, HeapObject obj, int offset) {
@@ -751,10 +772,10 @@ class WasmInternalFunction::BodyDescriptor final : public BodyDescriptorBase {
   template <typename ObjectVisitor>
   static inline void IterateBody(Map map, HeapObject obj, int object_size,
                                  ObjectVisitor* v) {
-    v->VisitExternalPointer(obj, obj.RawExternalPointerField(kCallTargetOffset),
-                            kWasmInternalFunctionCallTargetTag);
     IteratePointers(obj, kStartOfStrongFieldsOffset, kEndOfStrongFieldsOffset,
                     v);
+    v->VisitExternalPointer(obj, obj.RawExternalPointerField(kCallTargetOffset),
+                            kWasmInternalFunctionCallTargetTag);
   }
 
   static inline int SizeOf(Map map, HeapObject object) { return kSize; }

src/wasm/wasm-objects-inl.h

@@ -296,9 +296,8 @@ EXTERNAL_POINTER_ACCESSORS(WasmInternalFunction, call_target, Address,
 // WasmFunctionData
 ACCESSORS(WasmFunctionData, internal, WasmInternalFunction, kInternalOffset)
 
-wasm::FunctionSig* WasmExportedFunctionData::sig() const {
-  return reinterpret_cast<wasm::FunctionSig*>(signature().foreign_address());
-}
+EXTERNAL_POINTER_ACCESSORS(WasmExportedFunctionData, sig, wasm::FunctionSig*,
+                           kSigOffset, kWasmExportedFunctionDataSignatureTag);
 
 // WasmJSFunction
 WasmJSFunction::WasmJSFunction(Address ptr) : JSFunction(ptr) {

src/wasm/wasm-objects.cc

@@ -1999,9 +1999,8 @@ Handle<WasmExportedFunction> WasmExportedFunction::New(
           : wasm::kNoPromise;
   Handle<WasmExportedFunctionData> function_data =
       factory->NewWasmExportedFunctionData(
-          export_wrapper, instance, call_target, ref, func_index,
-          reinterpret_cast<Address>(sig), wasm::kGenericWrapperBudget, rtt,
-          promise);
+          export_wrapper, instance, call_target, ref, func_index, sig,
+          wasm::kGenericWrapperBudget, rtt, promise);
 
   MaybeHandle<String> maybe_name;
   bool is_asm_js_module = instance->module_object().is_asm_js();

src/wasm/wasm-objects.h

@@ -16,6 +16,7 @@
 #include "src/objects/foreign.h"
 #include "src/objects/js-function.h"
 #include "src/objects/js-objects.h"
+#include "src/objects/objects-body-descriptors.h"
 #include "src/objects/objects.h"
 #include "src/objects/struct.h"
 #include "src/wasm/module-instantiate.h"
@@ -710,7 +711,8 @@ class WasmFunctionData
 
   DECL_PRINTER(WasmFunctionData)
 
-  using BodyDescriptor = FlexibleBodyDescriptor<kStartOfStrongFieldsOffset>;
+  using BodyDescriptor = FixedBodyDescriptor<kStartOfStrongFieldsOffset,
+                                             kEndOfStrongFieldsOffset, kSize>;
 
   using SuspendField = base::BitField<wasm::Suspend, 0, 1>;
   using PromiseField = base::BitField<wasm::Promise, 1, 1>;
@@ -725,14 +727,13 @@ class WasmExportedFunctionData
     : public TorqueGeneratedWasmExportedFunctionData<WasmExportedFunctionData,
                                                      WasmFunctionData> {
  public:
-  inline wasm::FunctionSig* sig() const;
+  DECL_EXTERNAL_POINTER_ACCESSORS(sig, wasm::FunctionSig*);
 
   // Dispatched behavior.
   DECL_PRINTER(WasmExportedFunctionData)
   DECL_VERIFIER(WasmExportedFunctionData)
 
-  using BodyDescriptor =
-      FlexibleBodyDescriptor<WasmFunctionData::kStartOfStrongFieldsOffset>;
+  class BodyDescriptor;
 
   TQ_OBJECT_CONSTRUCTORS(WasmExportedFunctionData)
 };

src/wasm/wasm-objects.tq

@@ -30,8 +30,6 @@ extern class WasmApiFunctionRef extends HeapObject {
 // This is the representation that is used internally by wasm to represent
 // function references.
 extern class WasmInternalFunction extends HeapObject {
-  // The call target. Tagged with the kWasmInternalFunctionCallTargetTag
-  call_target: ExternalPointer;
   // This is the "reference" value that must be passed along in the "instance"
   // register when calling the given function. It is either the target instance
   // (for wasm functions), or a WasmApiFunctionRef object (for functions defined
@@ -44,6 +42,8 @@ extern class WasmInternalFunction extends HeapObject {
   // This field is used when the call target is null.
   @if(V8_EXTERNAL_CODE_SPACE) code: CodeDataContainer;
   @ifnot(V8_EXTERNAL_CODE_SPACE) code: Code;
+  // The call target. Tagged with the kWasmInternalFunctionCallTargetTag
+  call_target: ExternalPointer;
 }
 
 extern operator '.call_target_ptr' macro LoadWasmInternalFunctionCallTargetPtr(
@@ -65,13 +65,13 @@ extern class WasmExportedFunctionData extends WasmFunctionData {
   // where the function is defined -- for the latter see WasmFunctionData::ref).
   instance: WasmInstanceObject;
   function_index: Smi;
-  signature: Foreign;
   wrapper_budget: Smi;
-  // The remaining fields are for fast calling from C++. The contract is
+  // The next two fields are for fast calling from C++. The contract is
   // that they are lazily populated, and either all will be present or none.
   @if(V8_EXTERNAL_CODE_SPACE) c_wrapper_code: CodeDataContainer;
   @ifnot(V8_EXTERNAL_CODE_SPACE) c_wrapper_code: Code;
   packed_args_size: Smi;
+  sig: ExternalPointer;  // wasm::FunctionSig*
 }
 
 extern class WasmJSFunctionData extends WasmFunctionData {