From 65c4f34385cd316a59befe766f1f5fcec051d0dd Mon Sep 17 00:00:00 2001
From: Max Kellermann <max.kellermann@gmail.com>
Date: Wed, 19 Feb 2020 12:11:48 +0100
Subject: [PATCH] Expr: fix memory leaks in CondExpr::Eval()

No code path had any cleanup code, leaking all the local references.

More weird was however the result building code: it took elements from
one of the existing vectors without referencing them, and passed them
to VectorVal::Assign() which assumes that the caller-owned reference
is now owned by that VectorVal.

Even in the successful code path, no references were freed.

Everything was wrong with this method!
---
 src/Expr.cc | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/Expr.cc b/src/Expr.cc
index 94e7076564..235fa15593 100644
--- a/src/Expr.cc
+++ b/src/Expr.cc
@@ -1995,11 +1995,18 @@ Val* CondExpr::Eval(Frame* f) const
 
 	Val* v2 = op2->Eval(f);
 	if ( ! v2 )
+		{
+		Unref(v1);
 		return 0;
+		}
 
 	Val* v3 = op3->Eval(f);
 	if ( ! v3 )
+		{
+		Unref(v1);
+		Unref(v2);
 		return 0;
+		}
 
 	VectorVal* cond = v1->AsVectorVal();
 	VectorVal* a = v2->AsVectorVal();
@@ -2007,6 +2014,9 @@ Val* CondExpr::Eval(Frame* f) const
 
 	if ( cond->Size() != a->Size() || a->Size() != b->Size() )
 		{
+		Unref(v1);
+		Unref(v2);
+		Unref(v3);
 		RuntimeError("vectors in conditional expression have different sizes");
 		return 0;
 		}
@@ -2018,13 +2028,19 @@ Val* CondExpr::Eval(Frame* f) const
 		{
 		Val* local_cond = cond->Lookup(i);
 		if ( local_cond )
-			result->Assign(i,
-				       local_cond->IsZero() ?
-					       b->Lookup(i) : a->Lookup(i));
+			{
+			Val *v = local_cond->IsZero() ? b->Lookup(i) : a->Lookup(i);
+			::Ref(v);
+			result->Assign(i, v);
+			}
 		else
 			result->Assign(i, 0);
 		}
 
+	Unref(v1);
+	Unref(v2);
+	Unref(v3);
+
 	return result;
 	}