Treat Buffered and External StringPrimitives as variable

Summary:
BufferedStringPrimitive inherits from VariableSizedRuntimeCell and
its VTable should contain a size of 0.

Similarly, we currently have special treatment for ExternStringPrimitive
that lets us treat it like a fixed size cell in places even though it is
variable sized. Remove that special case.

Reviewed By: dulinriley

Differential Revision: D24316034

fbshipit-source-id: c2035555fbf85426cca7cd0f3c419add8770b19f

include/hermes/VM/GCCell.h

@@ -83,9 +83,7 @@ class GCCell {
     // ExternalStringPrimitive<T> is special: it extends VariableSizeRuntimeCell
     // but it's actually fixed-size.
     static_assert(
-        !std::is_convertible<C *, VariableSizeRuntimeCell *>::value ||
-            std::is_same<C, ExternalStringPrimitive<char>>::value ||
-            std::is_same<C, ExternalStringPrimitive<char16_t>>::value,
+        !std::is_convertible<C *, VariableSizeRuntimeCell *>::value,
         "must be fixed-size");
     return sizeof(C);
   }

include/hermes/VM/StringPrimitive.h

@@ -822,7 +822,7 @@ using DynamicUniquedASCIIStringPrimitive =
 template <typename T>
 const VTable ExternalStringPrimitive<T>::vt = VTable(
     ExternalStringPrimitive<T>::getCellKind(),
-    cellSize<ExternalStringPrimitive<T>>(),
+    0,
     ExternalStringPrimitive<T>::_finalizeImpl,
     nullptr, // markWeak.
     ExternalStringPrimitive<T>::_mallocSizeImpl,
@@ -842,7 +842,7 @@ using ExternalASCIIStringPrimitive = ExternalStringPrimitive<char>;
 template <typename T>
 const VTable BufferedStringPrimitive<T>::vt = VTable(
     BufferedStringPrimitive<T>::getCellKind(),
-    sizeof(BufferedStringPrimitive<T>),
+    0,
     nullptr, // finalize.
     nullptr, // markWeak.
     nullptr, // mallocSize

lib/VM/StringPrimitive.cpp

@@ -176,8 +176,8 @@ void deserializeExternalStringImpl(Deserializer &d) {
   d.readData(&contents[0], length * sizeof(T));
 
   // Construct an ExternalStringPrimitive from what we deserialized.
-  void *mem = d.getRuntime()->alloc</*fixedSize*/ true, HasFinalizer::Yes>(
-      cellSize<ExternalStringPrimitive<T>>());
+  void *mem = d.getRuntime()->alloc</*fixedSize*/ false, HasFinalizer::Yes>(
+      sizeof(ExternalStringPrimitive<T>));
   auto *cell =
       new (mem) ExternalStringPrimitive<T>(d.getRuntime(), std::move(contents));
   if (uniqued)
@@ -534,7 +534,7 @@ ExternalStringPrimitive<T>::ExternalStringPrimitive(
     : SymbolStringPrimitive(
           runtime,
           &vt,
-          cellSize<ExternalStringPrimitive<T>>(),
+          sizeof(ExternalStringPrimitive<T>),
           contents.size()),
       contents_(std::forward<BasicString>(contents)) {
   static_assert(
@@ -557,8 +557,11 @@ CallResult<HermesValue> ExternalStringPrimitive<T>::create(
       "ExternalStringPrimitive mismatched char type");
   if (LLVM_UNLIKELY(str.size() > MAX_STRING_LENGTH))
     return runtime->raiseRangeError("String length exceeds limit");
-  void *mem = runtime->alloc</*fixedSize*/ true, HasFinalizer::Yes>(
-      cellSize<ExternalStringPrimitive<T>>());
+  // We have to use a variable sized alloc here even though the size is already
+  // known, because ExternalStringPrimitive is derived from
+  // VariableSizeRuntimeCell
+  void *mem = runtime->alloc</*fixedSize*/ false, HasFinalizer::Yes>(
+      sizeof(ExternalStringPrimitive<T>));
   auto *extStr = new (mem)
       ExternalStringPrimitive<T>(runtime, std::forward<BasicString>(str));
   runtime->getHeap().creditExternalMemory(
@@ -578,10 +581,13 @@ CallResult<HermesValue> ExternalStringPrimitive<T>::createLongLived(
     return runtime->raiseRangeError(
         "Cannot allocate an external string primitive.");
   }
-  auto *extStr = runtime->makeAFixed<
+  // Use variable size alloc since ExternalStringPrimitive is derived from
+  // VariableSizeRuntimeCell.
+  auto *extStr = runtime->makeAVariable<
       ExternalStringPrimitive<T>,
       HasFinalizer::Yes,
-      LongLived::Yes>(runtime, std::move(str));
+      LongLived::Yes>(
+      sizeof(ExternalStringPrimitive<T>), runtime, std::move(str));
   runtime->getHeap().creditExternalMemory(
       extStr, extStr->calcExternalMemorySize());
   return HermesValue::encodeStringValue(extStr);
@@ -684,7 +690,10 @@ PseudoHandle<StringPrimitive> BufferedStringPrimitive<T>::create(
     Runtime *runtime,
     uint32_t length,
     Handle<ExternalStringPrimitive<T>> storage) {
-  void *mem = runtime->alloc</*fixedSize*/ true, HasFinalizer::No>(
+  // We have to use a variable sized alloc here even though the size is already
+  // known, because BufferedStringPrimitive is derived from
+  // VariableSizeRuntimeCell.
+  void *mem = runtime->alloc</*fixedSize*/ false, HasFinalizer::No>(
       sizeof(BufferedStringPrimitive<T>));
   return createPseudoHandle<StringPrimitive>(
       new (mem) BufferedStringPrimitive<T>(runtime, length, storage.get()));