From 599d00aa68cf444c7ddc5c10fe55a07b6b79e1f0 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 26 Jan 2015 12:18:25 -0500 Subject: [PATCH] Fix volatile-safety issue in pltcl_SPI_execute_plan(). The "callargs" variable is modified within PG_TRY and then referenced within PG_CATCH, which is exactly the coding pattern we've now found to be unsafe. Marking "callargs" volatile would be problematic because it is passed by reference to some Tcl functions, so fix the problem by not modifying it within PG_TRY. We can just postpone the free() till we exit the PG_TRY construct, as is already done elsewhere in this same file. Also, fix failure to free(callargs) when exiting on too-many-arguments error. This is only a minor memory leak, but a leak nonetheless. In passing, remove some unnecessary "volatile" markings in the same function. Those doubtless are there because gcc 2.95.3 whinged about them, but we now know that its algorithm for complaining is many bricks shy of a load. This is certainly a live bug with compilers that optimize similarly to current gcc, so back-patch to all active branches. --- src/pl/tcl/pltcl.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c index 8f9804696e5..d6b72f7d686 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -2233,9 +2233,9 @@ pltcl_SPI_execute_plan(ClientData cdata, Tcl_Interp *interp, int j; Tcl_HashEntry *hashent; pltcl_query_desc *qdesc; - const char *volatile nulls = NULL; - CONST84 char *volatile arrayname = NULL; - CONST84 char *volatile loop_body = NULL; + const char *nulls = NULL; + CONST84 char *arrayname = NULL; + CONST84 char *loop_body = NULL; int count = 0; int callnargs; CONST84 char **callargs = NULL; @@ -2365,6 +2365,8 @@ pltcl_SPI_execute_plan(ClientData cdata, Tcl_Interp *interp, if (i != argc) { Tcl_SetResult(interp, usage, TCL_STATIC); + if (callargs) + ckfree((char *) callargs); return TCL_ERROR; } @@ -2403,10 +2405,6 @@ pltcl_SPI_execute_plan(ClientData cdata, Tcl_Interp *interp, } } - if (callargs) - ckfree((char *) callargs); - callargs = NULL; - /************************************************************ * Execute the plan ************************************************************/ @@ -2433,6 +2431,9 @@ pltcl_SPI_execute_plan(ClientData cdata, Tcl_Interp *interp, } PG_END_TRY(); + if (callargs) + ckfree((char *) callargs); + return my_rc; }