Fix an ancient logic error in plpgsql's exec_stmt_block: it thought it could

tglsfdc · tglsfdc · commit 675a313b545d · 2007-02-08T18:38:31.000Z
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.
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
@@ -3,7 +3,7 @@
  *			  procedural language
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.65.2.4 2005/06/20 20:45:06 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.65.2.5 2007/02/08 18:38:31 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -767,26 +767,26 @@ 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 */
 					if (var->freeval)
 					{
 						pfree((void *) (var->value));
 						var->freeval = false;
 					}
 
-					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)
-								elog(ERROR, "variable '%s' declared NOT NULL cannot default to NULL", var->refname);
-						}
-						else
-						{
-							exec_assign_expr(estate, (PLpgSQL_datum *) var,
-											 var->default_val);
-						}
+						if (var->notnull)
+							elog(ERROR, "variable '%s' declared NOT NULL cannot default to NULL", var->refname);
+					}
+					else
+					{
+						exec_assign_expr(estate, (PLpgSQL_datum *) var,
+										 var->default_val);
 					}
 				}
 				break;

Original file line number	Diff line number	Diff line change
`@@ -3,7 +3,7 @@`
`3`	`3`	`* procedural language`
`4`	`4`	`*`
`5`	`5`	`* IDENTIFICATION`
`6`		`- * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.65.2.4 2005/06/20 20:45:06 tgl Exp $`
	`6`	`+ * $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.65.2.5 2007/02/08 18:38:31 tgl Exp $`
`7`	`7`	`*`
`8`	`8`	`* This software is copyrighted by Jan Wieck - Hamburg.`
`9`	`9`	`*`
`@@ -767,26 +767,26 @@ exec_stmt_block(PLpgSQL_execstate * estate, PLpgSQL_stmt_block * block)`
`767`	`767`	`{`
`768`	`768`	`PLpgSQL_var var = (PLpgSQL_var ) (estate->datums[n]);`
`769`	`769`
	`770`	`+ /* free any old value, in case re-entering block */`
`770`	`771`	`if (var->freeval)`
`771`	`772`	`{`
`772`	`773`	`pfree((void *) (var->value));`
`773`	`774`	`var->freeval = false;`
`774`	`775`	`}`
`775`	`776`
`776`		`- if (!var->isconst \|\| var->isnull)`
	`777`	`+ /* Initially it contains a NULL */`
	`778`	`+ var->value = (Datum) 0;`
	`779`	`+ var->isnull = true;`
	`780`	`+`
	`781`	`+ if (var->default_val == NULL)`
`777`	`782`	`{`
`778`		`- if (var->default_val == NULL)`
`779`		`- {`
`780`		`- var->value = (Datum) 0;`
`781`		`- var->isnull = true;`
`782`		`- if (var->notnull)`
`783`		`- elog(ERROR, "variable '%s' declared NOT NULL cannot default to NULL", var->refname);`
`784`		`- }`
`785`		`- else`
`786`		`- {`
`787`		`- exec_assign_expr(estate, (PLpgSQL_datum *) var,`
`788`		`- var->default_val);`
`789`		`- }`
	`783`	`+ if (var->notnull)`
	`784`	`+ elog(ERROR, "variable '%s' declared NOT NULL cannot default to NULL", var->refname);`
	`785`	`+ }`
	`786`	`+ else`
	`787`	`+ {`
	`788`	`+ exec_assign_expr(estate, (PLpgSQL_datum *) var,`
	`789`	`+ var->default_val);`
`790`	`790`	`}`
`791`	`791`	`}`
`792`	`792`	`break;`