From d6e3ae48ef4822259b00fe5fabd9791a30c19387 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 8 Feb 2007 18:37:55 +0000
Subject: [PATCH] Fix an ancient logic error in plpgsql's exec_stmt_block: it
thought it could get away with not (re)initializing a local variable if the
variable is marked "isconst" and not "isnull". Unfortunately it makes this
decision after having already freed the old value, meaning that something
like
for i in 1..10 loop
declare c constant text := 'hi there';
leads to subsequent accesses to freed memory, and hence probably crashes.
(In particular, this is why Asif Ali Rehman's bug leads to crash and not
just an unexpectedly-NULL value for SQLERRM: SQLERRM is marked CONSTANT
and so triggers this error.)
The whole thing seems wrong on its face anyway: CONSTANT means that you can't
change the variable inside the block, not that the initializer expression is
guaranteed not to change value across successive block entries. Hence,
remove the "optimization" instead of trying to fix it.
---
src/pl/plpgsql/src/pl_exec.c | 37 ++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index f537175414a..6bc97eaa5c4 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3,7 +3,7 @@
* procedural language
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.154.2.5 2007/01/30 18:02:34 tgl Exp $
+ * $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.154.2.6 2007/02/08 18:37:55 tgl Exp $
*
* This software is copyrighted by Jan Wieck - Hamburg.
*
@@ -822,24 +822,25 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
{
PLpgSQL_var *var = (PLpgSQL_var *) (estate->datums[n]);
+ /* free any old value, in case re-entering block */
free_var(var);
- if (!var->isconst || var->isnull)
+
+ /* Initially it contains a NULL */
+ var->value = (Datum) 0;
+ var->isnull = true;
+
+ if (var->default_val == NULL)
{
- if (var->default_val == NULL)
- {
- var->value = (Datum) 0;
- var->isnull = true;
- if (var->notnull)
- ereport(ERROR,
+ if (var->notnull)
+ ereport(ERROR,
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
errmsg("variable \"%s\" declared NOT NULL cannot default to NULL",
var->refname)));
- }
- else
- {
- exec_assign_expr(estate, (PLpgSQL_datum *) var,
- var->default_val);
- }
+ }
+ else
+ {
+ exec_assign_expr(estate, (PLpgSQL_datum *) var,
+ var->default_val);
}
}
break;
@@ -951,7 +952,9 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
rc = exec_stmts(estate, exception->action);
free_var(state_var);
+ state_var->value = (Datum) 0;
free_var(errm_var);
+ errm_var->value = (Datum) 0;
break;
}
}
@@ -4530,6 +4533,12 @@ plpgsql_xact_cb(XactEvent event, void *arg)
simple_eval_estate = NULL;
}
+/*
+ * free_var --- pfree any pass-by-reference value of the variable.
+ *
+ * This should always be followed by some assignment to var->value,
+ * as it leaves a dangling pointer.
+ */
static void
free_var(PLpgSQL_var *var)
{