Possible bug in plpgsql/src/gram.y

Confirmed. I found a second problem in the file too, very similar.
Patch applied.

In this bit of code in src/pl/plpgsql/src/gram.y in the current CVS
sources, curname_def is defined as PLpgSQL_expr * but it is is
allocated the space required for a PLpgSQL_var. This looks like a
bug.

Ian

| decl_varname K_CURSOR decl_cursor_args decl_is_from K_SELECT decl_cursor_query
{
PLpgSQL_var *new;
PLpgSQL_expr *curname_def;
char buf[1024];
char *cp1;
char *cp2;

plpgsql_ns_pop();

new = malloc(sizeof(PLpgSQL_var));
memset(new, 0, sizeof(PLpgSQL_var));

curname_def = malloc(sizeof(PLpgSQL_var));
memset(curname_def, 0, sizeof(PLpgSQL_var));

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Also, can someone tell my why we use malloc in plpgsql?

In this bit of code in src/pl/plpgsql/src/gram.y in the current CVS
sources, curname_def is defined as PLpgSQL_expr * but it is is
allocated the space required for a PLpgSQL_var. This looks like a
bug.

Ian

| decl_varname K_CURSOR decl_cursor_args decl_is_from K_SELECT decl_cursor_query
{
PLpgSQL_var *new;
PLpgSQL_expr *curname_def;
char buf[1024];
char *cp1;
char *cp2;

plpgsql_ns_pop();

new = malloc(sizeof(PLpgSQL_var));
memset(new, 0, sizeof(PLpgSQL_var));

curname_def = malloc(sizeof(PLpgSQL_var));
memset(curname_def, 0, sizeof(PLpgSQL_var));

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Also, can someone tell my why we use malloc in plpgsql?

Plain palloc() won't do because the compiled tree for the function needs
to outlive the current query. However, malloc() is not cool. Really,
these structures ought to be built in a memory context created specially
for each function --- then it'd be possible to reclaim the memory if the
function is deleted or we realize we need to invalidate its compiled
tree.

I've had this in mind to do for awhile, but haven't gotten to it.
Do you want to put it on TODO?

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Also, can someone tell my why we use malloc in plpgsql?

Plain palloc() won't do because the compiled tree for the function needs
to outlive the current query. However, malloc() is not cool. Really,
these structures ought to be built in a memory context created specially
for each function --- then it'd be possible to reclaim the memory if the
function is deleted or we realize we need to invalidate its compiled
tree.

I've had this in mind to do for awhile, but haven't gotten to it.
Do you want to put it on TODO?

Done:

* Change PL/PgSQL to use palloc() instead of malloc()

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian wrote:

Confirmed. I found a second problem in the file too, very similar.
Patch applied.

Cut'n paste error. Thanks to both of you, good catch.

Jan

In this bit of code in src/pl/plpgsql/src/gram.y in the current CVS
sources, curname_def is defined as PLpgSQL_expr * but it is is
allocated the space required for a PLpgSQL_var. This looks like a
bug.

Ian

| decl_varname K_CURSOR decl_cursor_args decl_is_from K_SELECT decl_cursor_query
{
PLpgSQL_var *new;
PLpgSQL_expr *curname_def;
char buf[1024];
char *cp1;
char *cp2;

plpgsql_ns_pop();

new = malloc(sizeof(PLpgSQL_var));
memset(new, 0, sizeof(PLpgSQL_var));

curname_def = malloc(sizeof(PLpgSQL_var));
memset(curname_def, 0, sizeof(PLpgSQL_var));

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

--
Bruce Momjian                        |  http://candle.pha.pa.us
pgman@candle.pha.pa.us               |  (610) 853-3000
+  If your life is a hard drive,     |  830 Blythe Avenue
+  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Index: src/pl/plpgsql/src/gram.y
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/pl/plpgsql/src/gram.y,v
retrieving revision 1.22
diff -c -r1.22 gram.y
*** src/pl/plpgsql/src/gram.y    2001/07/11 18:54:18 1.22
--- src/pl/plpgsql/src/gram.y    2001/07/12 01:15:05
***************
*** 332,338 ****
{
PLpgSQL_rec         *new;

! new = malloc(sizeof(PLpgSQL_var));

new->dtype          = PLPGSQL_DTYPE_REC;
new->refname   = $1.name;
--- 332,338 ----
{
PLpgSQL_rec         *new;

! new = malloc(sizeof(PLpgSQL_rec));

new->dtype = PLPGSQL_DTYPE_REC;
new->refname = $1.name;
***************
*** 374,381 ****
new = malloc(sizeof(PLpgSQL_var));
memset(new, 0, sizeof(PLpgSQL_var));

! curname_def = malloc(sizeof(PLpgSQL_var));
! memset(curname_def, 0, sizeof(PLpgSQL_var));

new->dtype          = PLPGSQL_DTYPE_VAR;
new->refname   = $1.name;
--- 374,381 ----
new = malloc(sizeof(PLpgSQL_var));
memset(new, 0, sizeof(PLpgSQL_var));

! curname_def = malloc(sizeof(PLpgSQL_expr));
! memset(curname_def, 0, sizeof(PLpgSQL_expr));

new->dtype = PLPGSQL_DTYPE_VAR;
new->refname = $1.name;

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Also, can someone tell my why we use malloc in plpgsql?

Plain palloc() won't do because the compiled tree for the function needs
to outlive the current query. However, malloc() is not cool. Really,
these structures ought to be built in a memory context created specially
for each function --- then it'd be possible to reclaim the memory if the
function is deleted or we realize we need to invalidate its compiled
tree.

I've had this in mind to do for awhile, but haven't gotten to it.
Do you want to put it on TODO?

Planned that myself, but dropped the plan again because I
think it'd be better to start more or less from scratch with
a complete new PL that supports modules, global variables and
the like. After 2-3 years we could simply remove the old
style PL/pgSQL then.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com