From 72ee25b4025257fdaab7b8e8d5d1ccc83858fdab Mon Sep 17 00:00:00 2001
From: Qyriad <qyriad@qyriad.me>
Date: Tue, 16 Jul 2024 09:32:03 -0600
Subject: [PATCH] libexpr: add a strongly typed version of gcAllocBytes()

This commit adds a new helper template function to gc-alloc.hh (which is
probably where you want to look at first, O great reviewer [custom file
ordering in review diffs when]), which uses a type argument to determine
the size to allocate, rather than making the caller use sizeof().

Change-Id: Ib5d138d91a28bdda304a80db24ea9fb08669ad22
---
 src/libexpr/eval.cc     |  5 ++---
 src/libexpr/gc-alloc.hh | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 06b1f27f5..c0e7a9a2e 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -777,8 +777,7 @@ static void copyContextToValue(Value & v, const NixStringContext & context)
 {
     if (!context.empty()) {
         size_t n = 0;
-        v.string.context = (const char * *)
-            gcAllocBytes((context.size() + 1) * sizeof(char *));
+        v.string.context = gcAllocType<char const *>(context.size() + 1);
         for (auto & i : context)
             v.string.context[n++] = gcCopyStringIfNeeded(i.to_string());
         v.string.context[n] = 0;
@@ -834,7 +833,7 @@ void EvalState::mkList(Value & v, size_t size)
 {
     v.mkList(size);
     if (size > 2)
-        v.bigList.elems = (Value * *) gcAllocBytes(size * sizeof(Value *));
+        v.bigList.elems = gcAllocType<Value *>(size);
     nrListElems += size;
 }
 
diff --git a/src/libexpr/gc-alloc.hh b/src/libexpr/gc-alloc.hh
index f691bfc4b..04ac28ea8 100644
--- a/src/libexpr/gc-alloc.hh
+++ b/src/libexpr/gc-alloc.hh
@@ -18,6 +18,8 @@
 #include <gc/gc_allocator.h>
 #include <gc/gc_cpp.h>
 
+#include "checked-arithmetic.hh"
+
 /// calloc, transparently GC-enabled.
 #define LIX_GC_CALLOC(size) GC_MALLOC(size)
 
@@ -104,6 +106,30 @@ inline void * gcAllocBytes(size_t n)
     return ptr;
 }
 
+/// Typed, safe wrapper around calloc() (transparently GC-enabled). Allocates
+/// enough for the requested count of the specified type. Also checks for
+/// nullptr (and throws @ref std::bad_alloc), and casts the void pointer to
+/// a pointer of the specified type, for type-convenient goodness.
+template<typename T>
+[[gnu::always_inline]]
+inline T * gcAllocType(size_t howMany = 1)
+{
+    // NOTE: size_t * size_t, which can definitely overflow.
+    // Unsigned integer overflow is definitely a bug, but isn't undefined
+    // behavior, so we can just check if we overflowed after the fact.
+    // However, people can and do request zero sized allocations, so we need
+    // to check that neither of our multiplicands were zero before complaining
+    // about it.
+    auto checkedSz = checked::Checked<size_t>(howMany) * sizeof(T);
+    size_t sz = checkedSz.valueWrapping();
+    if (checkedSz.overflowed()) {
+        // Congrats, you done did an overflow.
+        throw std::bad_alloc();
+    }
+
+    return static_cast<T *>(gcAllocBytes(sz));
+}
+
 /// GC-transparently allocates a buffer for a C-string of @ref size *bytes*,
 /// meaning you should include the size needed by the NUL terminator in the
 /// passed size. Memory allocated with this function must never contain other