about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEelco Dolstra <eelco.dolstra@logicblox.com>2013-10-28T17·50+0100
committerEelco Dolstra <eelco.dolstra@logicblox.com>2013-10-28T17·52+0100
commitdec2f195022bcc14f217aa20c1e05e4b7fe9e917 (patch)
treed7e0ec658c5df8e5a2cc4b6fe653f9606eb7fc18
parent61231449332154170eafc2b80c10328ba736f31e (diff)
Fix a segfault in genericClosure
It kept temporary data in STL containers that were not scanned by
Boehm GC, so Nix programs using genericClosure could randomly crash if
the garbage collector kicked in at a bad time.

Also make it a bit more efficient by copying points to values rather
than values.
-rw-r--r--src/libexpr/primops.cc41
1 files changed, 25 insertions, 16 deletions
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 214bf8b999aa..6e1f86c2a09d 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -158,24 +158,31 @@ static void prim_isBool(EvalState & state, Value * * args, Value & v)
 
 struct CompareValues
 {
-    bool operator () (const Value & v1, const Value & v2) const
+    bool operator () (const Value * v1, const Value * v2) const
     {
-        if (v1.type != v2.type)
+        if (v1->type != v2->type)
             throw EvalError("cannot compare values of different types");
-        switch (v1.type) {
+        switch (v1->type) {
             case tInt:
-                return v1.integer < v2.integer;
+                return v1->integer < v2->integer;
             case tString:
-                return strcmp(v1.string.s, v2.string.s) < 0;
+                return strcmp(v1->string.s, v2->string.s) < 0;
             case tPath:
-                return strcmp(v1.path, v2.path) < 0;
+                return strcmp(v1->path, v2->path) < 0;
             default:
-                throw EvalError(format("cannot compare %1% with %2%") % showType(v1) % showType(v2));
+                throw EvalError(format("cannot compare %1% with %2%") % showType(*v1) % showType(*v2));
         }
     }
 };
 
 
+#if HAVE_BOEHMGC
+typedef list<Value *, gc_allocator<Value *> > ValueVector;
+#else
+typedef vector<Value *> ValueVector;
+#endif
+
+
 static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
 {
     startNest(nest, lvlDebug, "finding dependencies");
@@ -189,7 +196,7 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
         throw EvalError("attribute `startSet' required");
     state.forceList(*startSet->value);
 
-    list<Value *> workSet;
+    ValueVector workSet;
     for (unsigned int n = 0; n < startSet->value->list.length; ++n)
         workSet.push_back(startSet->value->list.elems[n]);
 
@@ -203,8 +210,10 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
     /* Construct the closure by applying the operator to element of
        `workSet', adding the result to `workSet', continuing until
        no new elements are found. */
-    list<Value> res;
-    set<Value, CompareValues> doneKeys; // !!! use Value *?
+    ValueVector res;
+    // `doneKeys' doesn't need to be a GC root, because its values are
+    // reachable from res.
+    set<Value *, CompareValues> doneKeys;
     while (!workSet.empty()) {
         Value * e = *(workSet.begin());
         workSet.pop_front();
@@ -217,9 +226,9 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
             throw EvalError("attribute `key' required");
         state.forceValue(*key->value);
 
-        if (doneKeys.find(*key->value) != doneKeys.end()) continue;
-        doneKeys.insert(*key->value);
-        res.push_back(*e);
+        if (doneKeys.find(key->value) != doneKeys.end()) continue;
+        doneKeys.insert(key->value);
+        res.push_back(e);
 
         /* Call the `operator' function with `e' as argument. */
         Value call;
@@ -236,8 +245,8 @@ static void prim_genericClosure(EvalState & state, Value * * args, Value & v)
     /* Create the result list. */
     state.mkList(v, res.size());
     unsigned int n = 0;
-    foreach (list<Value>::iterator, i, res)
-        *(v.list.elems[n++] = state.allocValue()) = *i;
+    foreach (ValueVector::iterator, i, res)
+        v.list.elems[n++] = *i;
 }
 
 
@@ -1056,7 +1065,7 @@ static void prim_lessThan(EvalState & state, Value * * args, Value & v)
     state.forceValue(*args[0]);
     state.forceValue(*args[1]);
     CompareValues comp;
-    mkBool(v, comp(*args[0], *args[1]));
+    mkBool(v, comp(args[0], args[1]));
 }