Обсуждение: Possible bug in plpgsql/src/gram.y
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));
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
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;
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,
Pennsylvania19026
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, Pennsylvania19026
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
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