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.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index efce8004b935a2bce69a5e78ec94cb5d38df1e2d..3a381e16b6fae20f2ea58fbf9bc26cc7b7cd4f20 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.180.2.3 2007/02/01 19:23:00 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.180.2.4 2007/02/08 18:37:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -856,43 +856,43 @@ 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)
+                                               /*
+                                                * If needed, give the datatype a chance to reject
+                                                * NULLs, by assigning a NULL to the variable.
+                                                * We claim the value is of type UNKNOWN, not the
+                                                * var's datatype, else coercion will be skipped.
+                                                * (Do this before the notnull check to be
+                                                * consistent with exec_assign_value.)
+                                                */
+                                               if (!var->datatype->typinput.fn_strict)
                                                {
-                                                       /* Initially it contains a NULL */
-                                                       var->value = (Datum) 0;
-                                                       var->isnull = true;
-                                                       /*
-                                                        * If needed, give the datatype a chance to reject
-                                                        * NULLs, by assigning a NULL to the variable.
-                                                        * We claim the value is of type UNKNOWN, not the
-                                                        * var's datatype, else coercion will be skipped.
-                                                        * (Do this before the notnull check to be
-                                                        * consistent with exec_assign_value.)
-                                                        */
-                                                       if (!var->datatype->typinput.fn_strict)
-                                                       {
-                                                               bool    valIsNull = true;
-
-                                                               exec_assign_value(estate,
-                                                                                                 (PLpgSQL_datum *) var,
-                                                                                                 (Datum) 0,
-                                                                                                 UNKNOWNOID,
-                                                                                                 &valIsNull);
-                                                       }
-                                                       if (var->notnull)
-                                                               ereport(ERROR,
+                                                       bool    valIsNull = true;
+
+                                                       exec_assign_value(estate,
+                                                                                         (PLpgSQL_datum *) var,
+                                                                                         (Datum) 0,
+                                                                                         UNKNOWNOID,
+                                                                                         &valIsNull);
+                                               }
+                                               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;
@@ -1026,7 +1026,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;
                                }
                        }
@@ -4812,6 +4814,12 @@ plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid,
        }
 }
 
+/*
+ * 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)
 {