Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fe796ea

Browse files
committed
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.
1 parent 7ad33ce commit fe796ea

File tree

1 file changed

+40
-32
lines changed

1 file changed

+40
-32
lines changed

src/pl/plpgsql/src/pl_exec.c

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.187 2007/02/01 19:22:07 tgl Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.188 2007/02/08 18:37:30 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -885,43 +885,43 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
885885
{
886886
PLpgSQL_var *var = (PLpgSQL_var *) (estate->datums[n]);
887887

888+
/* free any old value, in case re-entering block */
888889
free_var(var);
889-
if (!var->isconst || var->isnull)
890+
891+
/* Initially it contains a NULL */
892+
var->value = (Datum) 0;
893+
var->isnull = true;
894+
895+
if (var->default_val == NULL)
890896
{
891-
if (var->default_val == NULL)
897+
/*
898+
* If needed, give the datatype a chance to reject
899+
* NULLs, by assigning a NULL to the variable.
900+
* We claim the value is of type UNKNOWN, not the
901+
* var's datatype, else coercion will be skipped.
902+
* (Do this before the notnull check to be
903+
* consistent with exec_assign_value.)
904+
*/
905+
if (!var->datatype->typinput.fn_strict)
892906
{
893-
/* Initially it contains a NULL */
894-
var->value = (Datum) 0;
895-
var->isnull = true;
896-
/*
897-
* If needed, give the datatype a chance to reject
898-
* NULLs, by assigning a NULL to the variable.
899-
* We claim the value is of type UNKNOWN, not the
900-
* var's datatype, else coercion will be skipped.
901-
* (Do this before the notnull check to be
902-
* consistent with exec_assign_value.)
903-
*/
904-
if (!var->datatype->typinput.fn_strict)
905-
{
906-
bool valIsNull = true;
907-
908-
exec_assign_value(estate,
909-
(PLpgSQL_datum *) var,
910-
(Datum) 0,
911-
UNKNOWNOID,
912-
&valIsNull);
913-
}
914-
if (var->notnull)
915-
ereport(ERROR,
907+
bool valIsNull = true;
908+
909+
exec_assign_value(estate,
910+
(PLpgSQL_datum *) var,
911+
(Datum) 0,
912+
UNKNOWNOID,
913+
&valIsNull);
914+
}
915+
if (var->notnull)
916+
ereport(ERROR,
916917
(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
917918
errmsg("variable \"%s\" declared NOT NULL cannot default to NULL",
918919
var->refname)));
919-
}
920-
else
921-
{
922-
exec_assign_expr(estate, (PLpgSQL_datum *) var,
923-
var->default_val);
924-
}
920+
}
921+
else
922+
{
923+
exec_assign_expr(estate, (PLpgSQL_datum *) var,
924+
var->default_val);
925925
}
926926
}
927927
break;
@@ -1065,7 +1065,9 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
10651065
rc = exec_stmts(estate, exception->action);
10661066

10671067
free_var(state_var);
1068+
state_var->value = (Datum) 0;
10681069
free_var(errm_var);
1070+
errm_var->value = (Datum) 0;
10691071
break;
10701072
}
10711073
}
@@ -4867,6 +4869,12 @@ plpgsql_subxact_cb(SubXactEvent event, SubTransactionId mySubid,
48674869
}
48684870
}
48694871

4872+
/*
4873+
* free_var --- pfree any pass-by-reference value of the variable.
4874+
*
4875+
* This should always be followed by some assignment to var->value,
4876+
* as it leaves a dangling pointer.
4877+
*/
48704878
static void
48714879
free_var(PLpgSQL_var *var)
48724880
{

0 commit comments

Comments
 (0)