libexpr: refactor gc-agnostic helpers into one place

Change-Id: Icc4b367e4f670d47256f62a3a002cd248a5c2d3b

src/libexpr/attr-set.cc

@@ -1,5 +1,6 @@
 #include "attr-set.hh"
-#include "eval-inline.hh"
+#include "eval.hh"
+#include "gc-alloc.hh"
 
 #include <algorithm>
 
@@ -19,7 +20,7 @@ Bindings * EvalState::allocBindings(size_t capacity)
         throw Error("attribute set of size %d is too big", capacity);
     nrAttrsets++;
     nrAttrsInAttrsets += capacity;
-    return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity);
+    return new (gcAllocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity);
 }
 
 

src/libexpr/eval-inline.hh

@@ -4,26 +4,10 @@
 #include "print.hh"
 #include "eval.hh"
 #include "eval-error.hh"
+#include "gc-alloc.hh"
 
 namespace nix {
 
-/**
- * Note: Various places expect the allocated memory to be zeroed.
- */
-[[gnu::always_inline]]
-inline void * allocBytes(size_t n)
-{
-    void * p;
-#if HAVE_BOEHMGC
-    p = GC_MALLOC(n);
-#else
-    p = calloc(n, 1);
-#endif
-    if (!p) throw std::bad_alloc();
-    return p;
-}
-
-
 [[gnu::always_inline]]
 Value * EvalState::allocValue()
 {
@@ -43,7 +27,7 @@ Value * EvalState::allocValue()
     *valueAllocCache = GC_NEXT(p);
     GC_NEXT(p) = nullptr;
 #else
-    void * p = allocBytes(sizeof(Value));
+    void * p = gcAllocBytes(sizeof(Value));
 #endif
 
     nrValues++;
@@ -73,7 +57,7 @@ Env & EvalState::allocEnv(size_t size)
         env = (Env *) p;
     } else
 #endif
-        env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *));
+        env = (Env *) gcAllocBytes(sizeof(Env) + size * sizeof(Value *));
 
     /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */
 

src/libexpr/eval.cc

@@ -9,6 +9,7 @@
 #include "store-api.hh"
 #include "derivations.hh"
 #include "downstream-placeholder.hh"
+#include "gc-alloc.hh"
 #include "globals.hh"
 #include "eval-inline.hh"
 #include "filetransfer.hh"
@@ -48,19 +49,6 @@ using json = nlohmann::json;
 
 namespace nix {
 
-static char * allocString(size_t size)
-{
-    char * t;
-#if HAVE_BOEHMGC
-    t = (char *) GC_MALLOC_ATOMIC(size);
-#else
-    t = (char *) malloc(size);
-#endif
-    if (!t) throw std::bad_alloc();
-    return t;
-}
-
-
 // When there's no need to write to the string, we can optimize away empty
 // string allocations.
 // This function handles makeImmutableString(std::string_view()) by returning
@@ -70,13 +58,12 @@ static const char * makeImmutableString(std::string_view s)
     const size_t size = s.size();
     if (size == 0)
         return "";
-    auto t = allocString(size + 1);
+    auto t = gcAllocString(size + 1);
     memcpy(t, s.data(), size);
     t[size] = '\0';
     return t;
 }
 
-
 RootValue allocRootValue(Value * v)
 {
 #if HAVE_BOEHMGC
@@ -806,7 +793,7 @@ static void copyContextToValue(Value & v, const NixStringContext & context)
     if (!context.empty()) {
         size_t n = 0;
         v.string.context = (const char * *)
-            allocBytes((context.size() + 1) * sizeof(char *));
+            gcAllocBytes((context.size() + 1) * sizeof(char *));
         for (auto & i : context)
             v.string.context[n++] = makeImmutableString(i.to_string());
         v.string.context[n] = 0;
@@ -862,7 +849,7 @@ void EvalState::mkList(Value & v, size_t size)
 {
     v.mkList(size);
     if (size > 2)
-        v.bigList.elems = (Value * *) allocBytes(size * sizeof(Value *));
+        v.bigList.elems = (Value * *) gcAllocBytes(size * sizeof(Value *));
     nrListElems += size;
 }
 
@@ -2076,7 +2063,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
        Value. allocating a GC'd string directly and moving it into a
        Value lets us avoid an allocation and copy. */
     const auto c_str = [&] {
-        char * result = allocString(sSize + 1);
+        char * result = gcAllocString(sSize + 1);
         char * tmp = result;
         for (const auto & part : s) {
             memcpy(tmp, part->data(), part->size());

src/libexpr/eval.hh

@@ -3,6 +3,7 @@
 
 #include "attr-set.hh"
 #include "eval-error.hh"
+#include "gc-alloc.hh"
 #include "types.hh"
 #include "value.hh"
 #include "nixexpr.hh"
@@ -37,22 +38,6 @@ namespace eval_cache {
     class EvalCache;
 }
 
-/** Alias for std::map which uses boehmgc's allocator conditional on us actually
- * using boehmgc in this build.
- */
-#if HAVE_BOEHMGC
-    template<typename KeyT, typename ValueT>
-    using GcMap = std::map<
-        KeyT,
-        ValueT,
-        std::less<KeyT>,
-        traceable_allocator<std::pair<KeyT const, ValueT>>
-    >;
-#else
-    using GcMap = std::map<KeyT, ValueT>
-#endif
-
-
 /**
  * Function that implements a primop.
  */

src/libexpr/gc-alloc.hh

@@ -0,0 +1,119 @@
+#pragma once
+/// @file Aliases and wrapper functions that are transparently GC-enabled
+/// if Lix is compiled with BoehmGC enabled.
+
+#include <cstddef>
+#include <cstring>
+#include <list>
+#include <map>
+#include <new>
+#include <vector>
+
+#if HAVE_BOEHMGC
+#include <functional> // std::less
+#include <utility> // std::pair
+#define GC_INCLUDE_NEW
+#include <gc/gc.h>
+#include <gc/gc_allocator.h>
+#include <gc/gc_cpp.h>
+
+/// calloc, transparently GC-enabled.
+#define LIX_GC_CALLOC(size) GC_MALLOC(size)
+
+/// strdup, transaprently GC-enabled.
+#define LIX_GC_STRDUP(str) GC_STRDUP(str)
+
+/// Atomic GC malloc() with GC enabled, or regular malloc() otherwise.
+#define LIX_GC_MALLOC_ATOMIC(size) GC_MALLOC_ATOMIC(size)
+
+namespace nix
+{
+
+/// Alias for std::map which uses BoehmGC's allocator conditional on this Lix
+/// build having GC enabled.
+template<typename KeyT, typename ValueT>
+using GcMap = std::map<
+    KeyT,
+    ValueT,
+    std::less<KeyT>,
+    traceable_allocator<std::pair<KeyT const, ValueT>>
+>;
+
+/// Alias for std::vector which uses BoehmGC's allocator conditional on this Lix
+/// build having GC enabled.
+template<typename ItemT>
+using GcVector = std::vector<ItemT, traceable_allocator<ItemT>>;
+
+/// Alias for std::list which uses BoehmGC's allocator conditional on this Lix
+/// build having GC enabled.
+template<typename ItemT>
+using GcList = std::list<ItemT, traceable_allocator<ItemT>>;
+
+}
+
+#else
+
+#include <cstdlib>
+
+/// calloc, transparently GC-enabled.
+#define LIX_GC_CALLOC(size) calloc(size, 1)
+
+/// strdup, transparently GC-enabled.
+#define LIX_GC_STRDUP(str) strdup(str)
+
+/// Atomic GC malloc() with GC enabled, or regular malloc() otherwise.
+/// The returned memory must never contain pointers.
+#define LIX_GC_MALLOC_ATOMIC(size) malloc(size)
+
+namespace nix
+{
+
+/// Alias for std::map which uses BoehmGC's allocator conditional on this Lix
+/// build having GC enabled.
+template<typename KeyT, typename ValueT>
+using GcMap = std::map<KeyT, ValueT>;
+
+/// Alias for std::vector which uses BoehmGC's allocator conditional on this Lix
+/// build having GC enabled.
+template<typename ItemT>
+using GcVector = std::vector<ItemT>;
+
+/// Alias for std::list which uses BoehmGC's allocator conditional on this Lix
+/// build having GC enabled.
+template<typename ItemT>
+using GcList = std::list<ItemT>;
+
+}
+
+#endif
+
+namespace nix
+{
+
+[[gnu::always_inline]]
+inline void * gcAllocBytes(size_t n)
+{
+    // Note: various places expect the allocated memory to be zero.
+    // Hence: calloc().
+    void * ptr = LIX_GC_CALLOC(n);
+    if (ptr == nullptr) {
+        throw std::bad_alloc();
+    }
+
+    return ptr;
+}
+
+/// 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
+/// pointers.
+inline char * gcAllocString(size_t size)
+{
+    char * cstr = static_cast<char *>(LIX_GC_MALLOC_ATOMIC(size));
+    if (cstr == nullptr) {
+        throw std::bad_alloc();
+    }
+    return cstr;
+}
+
+}

src/libexpr/get-drvs.hh

@@ -80,13 +80,7 @@ public:
     bool hasFailed() { return failed; };
 };
 
-
-#if HAVE_BOEHMGC
-typedef std::list<DrvInfo, traceable_allocator<DrvInfo>> DrvInfos;
-#else
-typedef std::list<DrvInfo> DrvInfos;
-#endif
-
+using DrvInfos = GcList<DrvInfo>;
 
 /**
  * If value `v` denotes a derivation, return a DrvInfo object

src/libexpr/meson.build

@@ -58,6 +58,7 @@ libexpr_headers = files(
   'function-trace.hh',
   'gc-small-vector.hh',
   'get-drvs.hh',
+  'gc-alloc.hh',
   'json-to-value.hh',
   'nixexpr.hh',
   'parser/change_head.hh',

src/libexpr/primops.cc

@@ -622,14 +622,13 @@ struct CompareValues
     }
 };
 
-
+/// NOTE: this type must NEVER be outside of GC-scanned memory.
 #if HAVE_BOEHMGC
-typedef std::list<Value *, gc_allocator<Value *>> ValueList;
+using UnsafeValueList = std::list<Value *, gc_allocator<Value *>>;
 #else
-typedef std::list<Value *> ValueList;
+using UnsafeValueList = std::list<Value *>;
 #endif
 
-
 static Bindings::iterator getAttr(
     EvalState & state,
     Symbol attrSym,
@@ -652,7 +651,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a
 
     state.forceList(*startSet->value, noPos, "while evaluating the 'startSet' attribute passed as argument to builtins.genericClosure");
 
-    ValueList workSet;
+    UnsafeValueList workSet;
     for (auto elem : startSet->value->listItems())
         workSet.push_back(elem);
 
@@ -668,7 +667,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a
     /* Construct the closure by applying the operator to elements of
        `workSet', adding the result to `workSet', continuing until
        no new elements are found. */
-    ValueList res;
+    UnsafeValueList res;
     // `doneKeys' doesn't need to be a GC root, because its values are
     // reachable from res.
     auto cmp = CompareValues(state, noPos, "while comparing the `key` attributes of two genericClosure elements");

src/libexpr/value.hh

@@ -4,6 +4,7 @@
 #include <cassert>
 #include <climits>
 
+#include "gc-alloc.hh"
 #include "symbol-table.hh"
 #include "value/context.hh"
 #include "input-accessor.hh"
@@ -11,9 +12,6 @@
 #include "print-options.hh"
 #include "checked-arithmetic.hh"
 
-#if HAVE_BOEHMGC
-#include <gc/gc_allocator.h>
-#endif
 #include <nlohmann/json_fwd.hpp>
 
 namespace nix {
@@ -464,17 +462,9 @@ void Value::mkBlackhole()
     thunk.expr = (Expr*) &eBlackHole;
 }
 
-
-#if HAVE_BOEHMGC
-typedef std::vector<Value *, traceable_allocator<Value *>> ValueVector;
-typedef std::map<Symbol, Value *, std::less<Symbol>, traceable_allocator<std::pair<const Symbol, Value *>>> ValueMap;
-typedef std::map<Symbol, ValueVector, std::less<Symbol>, traceable_allocator<std::pair<const Symbol, ValueVector>>> ValueVectorMap;
-#else
-typedef std::vector<Value *> ValueVector;
-typedef std::map<Symbol, Value *> ValueMap;
-typedef std::map<Symbol, ValueVector> ValueVectorMap;
-#endif
-
+using ValueVector = GcVector<Value *>;
+using ValueMap = GcMap<Symbol, Value *>;
+using ValueVectorMap = std::map<Symbol, ValueVector>;
 
 /**
  * A value allocated in traceable memory.