From 7b6bcffe8ba7208ee4c26a21a8f852876ffe5828 Mon Sep 17 00:00:00 2001
From: Raito Bezarius <raito@lix.systems>
Date: Mon, 6 Oct 2025 22:29:12 +0200
Subject: [PATCH] libexpr/genericClosure: de-ptr-ize UnsafeValueList

UnsafeValueList held pointers from Value which were not necessarily GC
allocated, causing mayhem when evaluating something with genericClosure
(texlive environments).

We get rid of storing pointers and we let comparison take places on
const references.

We keep `gc_allocator<Value>` so that GC can scan things it allocates
inside the `res` list.

Co-authored-by: eldritch horrors <pennae@lix.systems>
Reported-by: qbit
Change-Id: I4fed3a3d9e18a2ef2d751f32d81801540e196f92
Signed-off-by: Raito Bezarius <raito@lix.systems>
---
 lix/libexpr/primops.cc | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc
index 91612bb4e..7dd27f5d3 100644
--- a/lix/libexpr/primops.cc
+++ b/lix/libexpr/primops.cc
@@ -523,9 +523,9 @@ struct CompareValues : NeverAsync
 
 /// NOTE: this type must NEVER be outside of GC-scanned memory.
 #if HAVE_BOEHMGC
-using UnsafeValueList = std::list<Value *, gc_allocator<Value *>>;
+using UnsafeValueList = std::list<Value, gc_allocator<Value>>;
 #else
-using UnsafeValueList = std::list<Value *>;
+using UnsafeValueList = std::list<Value>;
 #endif
 
 static const Attr *
@@ -558,7 +558,7 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
 
     UnsafeValueList workSet;
     for (auto & elem : startSet->value.listItems()) {
-        workSet.push_back(&elem);
+        workSet.push_back(elem);
     }
 
     if (startSet->value.listSize() == 0) {
@@ -586,36 +586,41 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
     // `doneKeys' doesn't need to be a GC root, because its values are
     // reachable from res.
     auto cmp = CompareValues(state, "while comparing the `key` attributes of two genericClosure elements");
-    std::set<Value *, decltype(cmp)> doneKeys(cmp);
+    std::set<Value, decltype(cmp)> doneKeys(cmp);
     while (!workSet.empty()) {
-        Value * e = *(workSet.begin());
+        Value e = *(workSet.begin());
         workSet.pop_front();
 
-        state.forceAttrs(*e, noPos, "while evaluating one of the elements generated by (or initially passed to) builtins.genericClosure");
+        state.forceAttrs(
+            e,
+            noPos,
+            "while evaluating one of the elements generated by (or initially passed to) "
+            "builtins.genericClosure"
+        );
 
         auto key = getAttr(
             state,
             state.ctx.s.key,
-            e->attrs(),
+            e.attrs(),
             "in one of the attrsets generated by (or initially passed to) builtins.genericClosure"
         );
         state.forceValue(key->value, noPos);
 
-        if (!doneKeys.insert(&key->value).second) {
+        if (!doneKeys.insert(key->value).second) {
             continue;
         }
         res.push_back(e);
 
         /* Call the `operator' function with `e' as argument. */
         Value newElements;
-        state.callFunction(op->value, {e, 1}, newElements, noPos);
+        state.callFunction(op->value, {&e, 1}, newElements, noPos);
         state.forceList(newElements, noPos, "while evaluating the return value of the `operator` passed to builtins.genericClosure");
 
         /* Add the values returned by the operator to the work set. */
         for (auto & elem : newElements.listItems()) {
             state.forceValue(elem, noPos); // "while evaluating one one of the elements returned by
                                            // the `operator` passed to builtins.genericClosure");
-            workSet.push_back(&elem);
+            workSet.push_back(elem);
         }
     }
 
@@ -624,7 +629,7 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
     v = {NewValueAs::list, result};
     unsigned int n = 0;
     for (auto & i : res)
-        result->elems[n++] = *i;
+        result->elems[n++] = i;
 }