src: use predefined AliasedBuffer types in the code base

Instead of allowing the callers to instantiate the template
with any numeric types (such as aliasing a Uint8Array to double[]),
predefine types that make sense and use those instead.

PR-URL: #27334
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: joyeecheung

src/aliased_buffer.h

@@ -3,12 +3,16 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "v8.h"
+#include <cinttypes>
 #include "util-inl.h"
+#include "v8.h"
 
 namespace node {
 
 /**
+ * Do not use this class directly when creating instances of it - use the
+ * Aliased*Array defined at the end of this file instead.
+ *
  * This class encapsulates the technique of having a native buffer mapped to
  * a JS object. Writes to the native buffer can happen efficiently without
  * going through JS, and the data is then available to user's via the exposed
@@ -22,18 +26,16 @@ namespace node {
  * The encapsulation herein provides a placeholder where such writes can be
  * observed. Any notification APIs will be left as a future exercise.
  */
-template <class NativeT, class V8T,
+template <class NativeT,
+          class V8T,
           // SFINAE NativeT to be scalar
           typename = std::enable_if_t<std::is_scalar<NativeT>::value>>
-class AliasedBuffer {
+class AliasedBufferBase {
  public:
-  AliasedBuffer(v8::Isolate* isolate, const size_t count)
-      : isolate_(isolate),
-        count_(count),
-        byte_offset_(0) {
+  AliasedBufferBase(v8::Isolate* isolate, const size_t count)
+      : isolate_(isolate), count_(count), byte_offset_(0) {
     CHECK_GT(count, 0);
     const v8::HandleScope handle_scope(isolate_);
-
     const size_t size_in_bytes =
         MultiplyWithOverflowCheck(sizeof(NativeT), count);
 
@@ -48,22 +50,20 @@ class AliasedBuffer {
   }
 
   /**
-   * Create an AliasedBuffer over a sub-region of another aliased buffer.
+   * Create an AliasedBufferBase over a sub-region of another aliased buffer.
    * The two will share a v8::ArrayBuffer instance &
    * a native buffer, but will each read/write to different sections of the
    * native buffer.
    *
    *  Note that byte_offset must by aligned by sizeof(NativeT).
    */
-  // TODO(refack): refactor into a non-owning `AliasedBufferView`
-  AliasedBuffer(v8::Isolate* isolate,
-                const size_t byte_offset,
-                const size_t count,
-                const AliasedBuffer<uint8_t,
-                v8::Uint8Array>& backing_buffer)
-      : isolate_(isolate),
-        count_(count),
-        byte_offset_(byte_offset) {
+  // TODO(refack): refactor into a non-owning `AliasedBufferBaseView`
+  AliasedBufferBase(
+      v8::Isolate* isolate,
+      const size_t byte_offset,
+      const size_t count,
+      const AliasedBufferBase<uint8_t, v8::Uint8Array>& backing_buffer)
+      : isolate_(isolate), count_(count), byte_offset_(byte_offset) {
     const v8::HandleScope handle_scope(isolate_);
 
     v8::Local<v8::ArrayBuffer> ab = backing_buffer.GetArrayBuffer();
@@ -81,16 +81,16 @@ class AliasedBuffer {
     js_array_ = v8::Global<V8T>(isolate, js_array);
   }
 
-  AliasedBuffer(const AliasedBuffer& that)
+  AliasedBufferBase(const AliasedBufferBase& that)
       : isolate_(that.isolate_),
         count_(that.count_),
         byte_offset_(that.byte_offset_),
         buffer_(that.buffer_) {
     js_array_ = v8::Global<V8T>(that.isolate_, that.GetJSArray());
   }
 
-  AliasedBuffer& operator=(AliasedBuffer&& that) noexcept {
-    this->~AliasedBuffer();
+  AliasedBufferBase& operator=(AliasedBufferBase&& that) noexcept {
+    this->~AliasedBufferBase();
     isolate_ = that.isolate_;
     count_ = that.count_;
     byte_offset_ = that.byte_offset_;
@@ -109,10 +109,8 @@ class AliasedBuffer {
    */
   class Reference {
    public:
-    Reference(AliasedBuffer<NativeT, V8T>* aliased_buffer, size_t index)
-        : aliased_buffer_(aliased_buffer),
-          index_(index) {
-    }
+    Reference(AliasedBufferBase<NativeT, V8T>* aliased_buffer, size_t index)
+        : aliased_buffer_(aliased_buffer), index_(index) {}
 
     Reference(const Reference& that)
         : aliased_buffer_(that.aliased_buffer_),
@@ -149,7 +147,7 @@ class AliasedBuffer {
     }
 
    private:
-    AliasedBuffer<NativeT, V8T>* aliased_buffer_;
+    AliasedBufferBase<NativeT, V8T>* aliased_buffer_;
     size_t index_;
   };
 
@@ -216,7 +214,7 @@ class AliasedBuffer {
 
   // Should only be used to extend the array.
   // Should only be used on an owning array, not one created as a sub array of
-  // an owning `AliasedBuffer`.
+  // an owning `AliasedBufferBase`.
   void reserve(size_t new_capacity) {
     DCHECK_GE(new_capacity, count_);
     DCHECK_EQ(byte_offset_, 0);
@@ -251,6 +249,12 @@ class AliasedBuffer {
   NativeT* buffer_;
   v8::Global<V8T> js_array_;
 };
+
+typedef AliasedBufferBase<int32_t, v8::Int32Array> AliasedInt32Array;
+typedef AliasedBufferBase<uint8_t, v8::Uint8Array> AliasedUint8Array;
+typedef AliasedBufferBase<uint32_t, v8::Uint32Array> AliasedUint32Array;
+typedef AliasedBufferBase<double, v8::Float64Array> AliasedFloat64Array;
+typedef AliasedBufferBase<uint64_t, v8::BigUint64Array> AliasedBigUint64Array;
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

src/env-inl.h

@@ -109,16 +109,15 @@ inline AsyncHooks::AsyncHooks()
   NODE_ASYNC_PROVIDER_TYPES(V)
 #undef V
 }
-
-inline AliasedBuffer<uint32_t, v8::Uint32Array>& AsyncHooks::fields() {
+inline AliasedUint32Array& AsyncHooks::fields() {
   return fields_;
 }
 
-inline AliasedBuffer<double, v8::Float64Array>& AsyncHooks::async_id_fields() {
+inline AliasedFloat64Array& AsyncHooks::async_id_fields() {
   return async_id_fields_;
 }
 
-inline AliasedBuffer<double, v8::Float64Array>& AsyncHooks::async_ids_stack() {
+inline AliasedFloat64Array& AsyncHooks::async_ids_stack() {
   return async_ids_stack_;
 }
 
@@ -243,8 +242,7 @@ inline void Environment::PopAsyncCallbackScope() {
 inline ImmediateInfo::ImmediateInfo(v8::Isolate* isolate)
     : fields_(isolate, kFieldsCount) {}
 
-inline AliasedBuffer<uint32_t, v8::Uint32Array>&
-    ImmediateInfo::fields() {
+inline AliasedUint32Array& ImmediateInfo::fields() {
   return fields_;
 }
 
@@ -279,7 +277,7 @@ inline void ImmediateInfo::ref_count_dec(uint32_t decrement) {
 inline TickInfo::TickInfo(v8::Isolate* isolate)
     : fields_(isolate, kFieldsCount) {}
 
-inline AliasedBuffer<uint8_t, v8::Uint8Array>& TickInfo::fields() {
+inline AliasedUint8Array& TickInfo::fields() {
   return fields_;
 }
 
@@ -486,13 +484,11 @@ inline void Environment::set_abort_on_uncaught_exception(bool value) {
   options_->abort_on_uncaught_exception = value;
 }
 
-inline AliasedBuffer<uint32_t, v8::Uint32Array>&
-Environment::should_abort_on_uncaught_toggle() {
+inline AliasedUint32Array& Environment::should_abort_on_uncaught_toggle() {
   return should_abort_on_uncaught_toggle_;
 }
 
-inline AliasedBuffer<int32_t, v8::Int32Array>&
-Environment::stream_base_state() {
+inline AliasedInt32Array& Environment::stream_base_state() {
   return stream_base_state_;
 }
 
@@ -622,13 +618,11 @@ void Environment::set_debug_enabled(DebugCategory category, bool enabled) {
   debug_enabled_[static_cast<int>(category)] = enabled;
 }
 
-inline AliasedBuffer<double, v8::Float64Array>*
-Environment::fs_stats_field_array() {
+inline AliasedFloat64Array* Environment::fs_stats_field_array() {
   return &fs_stats_field_array_;
 }
 
-inline AliasedBuffer<uint64_t, v8::BigUint64Array>*
-Environment::fs_stats_field_bigint_array() {
+inline AliasedBigUint64Array* Environment::fs_stats_field_bigint_array() {
   return &fs_stats_field_bigint_array_;
 }
 

src/env.h

@@ -611,9 +611,9 @@ class AsyncHooks : public MemoryRetainer {
     kUidFieldsCount,
   };
 
-  inline AliasedBuffer<uint32_t, v8::Uint32Array>& fields();
-  inline AliasedBuffer<double, v8::Float64Array>& async_id_fields();
-  inline AliasedBuffer<double, v8::Float64Array>& async_ids_stack();
+  inline AliasedUint32Array& fields();
+  inline AliasedFloat64Array& async_id_fields();
+  inline AliasedFloat64Array& async_ids_stack();
 
   inline v8::Local<v8::String> provider_string(int idx);
 
@@ -652,12 +652,12 @@ class AsyncHooks : public MemoryRetainer {
   // Keep a list of all Persistent strings used for Provider types.
   std::array<v8::Eternal<v8::String>, AsyncWrap::PROVIDERS_LENGTH> providers_;
   // Stores the ids of the current execution context stack.
-  AliasedBuffer<double, v8::Float64Array> async_ids_stack_;
+  AliasedFloat64Array async_ids_stack_;
   // Attached to a Uint32Array that tracks the number of active hooks for
   // each type.
-  AliasedBuffer<uint32_t, v8::Uint32Array> fields_;
+  AliasedUint32Array fields_;
   // Attached to a Float64Array that tracks the state of async resources.
-  AliasedBuffer<double, v8::Float64Array> async_id_fields_;
+  AliasedFloat64Array async_id_fields_;
 
   void grow_async_ids_stack();
 };
@@ -676,7 +676,7 @@ class AsyncCallbackScope {
 
 class ImmediateInfo : public MemoryRetainer {
  public:
-  inline AliasedBuffer<uint32_t, v8::Uint32Array>& fields();
+  inline AliasedUint32Array& fields();
   inline uint32_t count() const;
   inline uint32_t ref_count() const;
   inline bool has_outstanding() const;
@@ -698,12 +698,12 @@ class ImmediateInfo : public MemoryRetainer {
 
   enum Fields { kCount, kRefCount, kHasOutstanding, kFieldsCount };
 
-  AliasedBuffer<uint32_t, v8::Uint32Array> fields_;
+  AliasedUint32Array fields_;
 };
 
 class TickInfo : public MemoryRetainer {
  public:
-  inline AliasedBuffer<uint8_t, v8::Uint8Array>& fields();
+  inline AliasedUint8Array& fields();
   inline bool has_tick_scheduled() const;
   inline bool has_rejection_to_warn() const;
 
@@ -720,7 +720,7 @@ class TickInfo : public MemoryRetainer {
 
   enum Fields { kHasTickScheduled = 0, kHasRejectionToWarn, kFieldsCount };
 
-  AliasedBuffer<uint8_t, v8::Uint8Array> fields_;
+  AliasedUint8Array fields_;
 };
 
 class TrackingTraceStateObserver :
@@ -908,10 +908,9 @@ class Environment : public MemoryRetainer {
   // This is a pseudo-boolean that keeps track of whether an uncaught exception
   // should abort the process or not if --abort-on-uncaught-exception was
   // passed to Node. If the flag was not passed, it is ignored.
-  inline AliasedBuffer<uint32_t, v8::Uint32Array>&
-  should_abort_on_uncaught_toggle();
+  inline AliasedUint32Array& should_abort_on_uncaught_toggle();
 
-  inline AliasedBuffer<int32_t, v8::Int32Array>& stream_base_state();
+  inline AliasedInt32Array& stream_base_state();
 
   // The necessary API for async_hooks.
   inline double new_async_id();
@@ -957,9 +956,8 @@ class Environment : public MemoryRetainer {
   inline void set_debug_enabled(DebugCategory category, bool enabled);
   void set_debug_categories(const std::string& cats, bool enabled);
 
-  inline AliasedBuffer<double, v8::Float64Array>* fs_stats_field_array();
-  inline AliasedBuffer<uint64_t, v8::BigUint64Array>*
-      fs_stats_field_bigint_array();
+  inline AliasedFloat64Array* fs_stats_field_array();
+  inline AliasedBigUint64Array* fs_stats_field_bigint_array();
 
   inline std::vector<std::unique_ptr<fs::FileHandleReadWrap>>&
       file_handle_read_wrap_freelist();
@@ -1204,12 +1202,12 @@ class Environment : public MemoryRetainer {
   uint32_t script_id_counter_ = 0;
   uint32_t function_id_counter_ = 0;
 
-  AliasedBuffer<uint32_t, v8::Uint32Array> should_abort_on_uncaught_toggle_;
+  AliasedUint32Array should_abort_on_uncaught_toggle_;
   int should_not_abort_scope_counter_ = 0;
 
   std::unique_ptr<TrackingTraceStateObserver> trace_state_observer_;
 
-  AliasedBuffer<int32_t, v8::Int32Array> stream_base_state_;
+  AliasedInt32Array stream_base_state_;
 
   std::unique_ptr<performance::performance_state> performance_state_;
   std::unordered_map<std::string, uint64_t> performance_marks_;
@@ -1252,8 +1250,8 @@ class Environment : public MemoryRetainer {
 
   bool debug_enabled_[static_cast<int>(DebugCategory::CATEGORY_COUNT)] = {0};
 
-  AliasedBuffer<double, v8::Float64Array> fs_stats_field_array_;
-  AliasedBuffer<uint64_t, v8::BigUint64Array> fs_stats_field_bigint_array_;
+  AliasedFloat64Array fs_stats_field_array_;
+  AliasedBigUint64Array fs_stats_field_bigint_array_;
 
   std::vector<std::unique_ptr<fs::FileHandleReadWrap>>
       file_handle_read_wrap_freelist_;

src/memory_tracker-inl.h

@@ -226,7 +226,7 @@ void MemoryTracker::TrackField(const char* name,
 
 template <class NativeT, class V8T>
 void MemoryTracker::TrackField(const char* name,
-                               const AliasedBuffer<NativeT, V8T>& value,
+                               const AliasedBufferBase<NativeT, V8T>& value,
                                const char* node_name) {
   TrackField(name, value.GetJSArray(), "AliasedBuffer");
 }

src/memory_tracker.h

@@ -215,7 +215,7 @@ class MemoryTracker {
                          const char* node_name = nullptr);
   template <class NativeT, class V8T>
   inline void TrackField(const char* edge_name,
-                         const AliasedBuffer<NativeT, V8T>& value,
+                         const AliasedBufferBase<NativeT, V8T>& value,
                          const char* node_name = nullptr);
 
   // Put a memory container into the graph, create an edge from

src/node_file.cc

@@ -756,9 +756,9 @@ inline FSReqBase* GetReqWrap(Environment* env, Local<Value> value,
     return Unwrap<FSReqBase>(value.As<Object>());
   } else if (value->StrictEquals(env->fs_use_promises_symbol())) {
     if (use_bigint) {
-      return FSReqPromise<uint64_t, BigUint64Array>::New(env, use_bigint);
+      return FSReqPromise<AliasedBigUint64Array>::New(env, use_bigint);
     } else {
-      return FSReqPromise<double, Float64Array>::New(env, use_bigint);
+      return FSReqPromise<AliasedFloat64Array>::New(env, use_bigint);
     }
   }
   return nullptr;

src/node_file.h

@@ -192,8 +192,9 @@ constexpr uint64_t ToNative(uv_timespec_t ts) {
 #undef constexpr  // end N3652 bug workaround
 
 template <typename NativeT, typename V8T>
-constexpr void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields,
-                              const uv_stat_t* s, const size_t offset = 0) {
+constexpr void FillStatsArray(AliasedBufferBase<NativeT, V8T>* fields,
+                              const uv_stat_t* s,
+                              const size_t offset = 0) {
   fields->SetValue(offset + 0, static_cast<NativeT>(s->st_dev));
   fields->SetValue(offset + 1, static_cast<NativeT>(s->st_mode));
   fields->SetValue(offset + 2, static_cast<NativeT>(s->st_nlink));
@@ -227,7 +228,7 @@ inline Local<Value> FillGlobalStatsArray(Environment* env,
   }
 }
 
-template <typename NativeT = double, typename V8T = v8::Float64Array>
+template <typename AliasedBufferT>
 class FSReqPromise : public FSReqBase {
  public:
   static FSReqPromise* New(Environment* env, bool use_bigint) {
@@ -304,7 +305,7 @@ class FSReqPromise : public FSReqBase {
         stats_field_array_(env->isolate(), kFsStatsFieldsNumber) {}
 
   bool finished_ = false;
-  AliasedBuffer<NativeT, V8T> stats_field_array_;
+  AliasedBufferT stats_field_array_;
 };
 
 class FSReqAfterScope {