Segmentation fault on PG 8.4 CVS head

"Rushabh Lathia" <rushabh.lathia@gmail.com> writes:

While running test with bind varible getting segmentation fault. ( CVS Head
8.4)

Fixed, thanks.

Another fix would be to add check for parseTree into
analyze_requires_snapshot().

This one seems the more bulletproof approach.

regards, tom lane

Tom fixed this upstream, and I just merged with PostgreSQL CVS HEAD
again. The buildfarm crash should now be fixed.

Rushabh Lathia wrote:

Hi All,

While running test with bind varible getting segmentation fault. ( CVS Head
8.4)

For testcase, please find the crash.c (C test) and test.java ( JDBC test)
attached with the mail.

Had a quick look at the core dump and found the call stack for the
segmentation fault.

(gdb) bt
#0 0x0813768d in analyze_requires_snapshot (parseTree=0x0) at analyze.c:270

#1 0x082e77a8 in exec_bind_message (input_message=0xbfd7d73c) at
postgres.c:1698
#2 0x082ec524 in PostgresMain (argc=4, argv=0x916fc70, username=0x916fb7c
"rushabh") at postgres.c:4882
#3 0x082ac10a in BackendRun (port=0x9191b18) at postmaster.c:3309
#4 0x082ab4d4 in BackendStartup (port=0x9191b18) at postmaster.c:2881
#5 0x082a8ae1 in ServerLoop () at postmaster.c:1291

Had a look at the previous version and found that because of following
condition added with the new PG merge into exec_bind_message(); we end up
with the segmentation fault.

exec_bind_message{
...
/*
* Set a snapshot if we have parameters to fetch (since the input
* functions might need it) or the query isn't a utility command (and
* hence could require redoing parse analysis and planning).
*/
if (numParams > 0 || analyze_requires_snapshot(psrc->raw_parse_tree))
{
PushActiveSnapshot(GetTransactionSnapshot());
snapshot_set = true;
}
...
}

Condition added with "Fix failure to ensure that a snapshot is available to
datatype input functions" commit. (
http://git.postgresql.org/?p=postgresql.git;a=commitdiff;h=d5e7e5dd7c81440bb46f52872906633ee2b388c1
)

Not very much sure but for the quick check I just modifiled condition by
added check for raw_parse_tree and test worked file.

Modified condition:
/*
* Set a snapshot if we have parameters to fetch (since the input
* functions might need it) or the query isn't a utility command (and
* hence could require redoing parse analysis and planning).
*/
if (numParams > 0 ||
(psrc->raw_parse_tree &&
analyze_requires_snapshot(psrc->raw_parse_tree)))
{
PushActiveSnapshot(GetTransactionSnapshot());
snapshot_set = true;
}

Another fix would be to add check for parseTree into
analyze_requires_snapshot().

Thanks ,
Rushabh Lathia
www.EnterpriseDB.com

------------------------------------------------------------------------

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Sorry for the noise, I didn't mean to sent this to the mailing list..

(the context is that this bug was found by a test case in EnterpriseDB
JDBC buildfarm)

Heikki Linnakangas wrote:

Tom fixed this upstream, and I just merged with PostgreSQL CVS HEAD
again. The buildfarm crash should now be fixed.

Rushabh Lathia wrote:

Hi All,

While running test with bind varible getting segmentation fault. ( CVS
Head
8.4)

For testcase, please find the crash.c (C test) and test.java ( JDBC test)
attached with the mail.

Had a quick look at the core dump and found the call stack for the
segmentation fault.

(gdb) bt
#0 0x0813768d in analyze_requires_snapshot (parseTree=0x0) at
analyze.c:270

#1 0x082e77a8 in exec_bind_message (input_message=0xbfd7d73c) at
postgres.c:1698
#2 0x082ec524 in PostgresMain (argc=4, argv=0x916fc70,
username=0x916fb7c
"rushabh") at postgres.c:4882
#3 0x082ac10a in BackendRun (port=0x9191b18) at postmaster.c:3309
#4 0x082ab4d4 in BackendStartup (port=0x9191b18) at postmaster.c:2881
#5 0x082a8ae1 in ServerLoop () at postmaster.c:1291

Had a look at the previous version and found that because of following
condition added with the new PG merge into exec_bind_message(); we end up
with the segmentation fault.

exec_bind_message{
...
/*
* Set a snapshot if we have parameters to fetch (since the input
* functions might need it) or the query isn't a utility command (and
* hence could require redoing parse analysis and planning).
*/
if (numParams > 0 || analyze_requires_snapshot(psrc->raw_parse_tree))
{
PushActiveSnapshot(GetTransactionSnapshot());
snapshot_set = true;
}
...
}

Condition added with "Fix failure to ensure that a snapshot is
available to
datatype input functions" commit. (
http://git.postgresql.org/?p=postgresql.git;a=commitdiff;h=d5e7e5dd7c81440bb46f52872906633ee2b388c1

)

Not very much sure but for the quick check I just modifiled condition by
added check for raw_parse_tree and test worked file.

Modified condition:
/*
* Set a snapshot if we have parameters to fetch (since the input
* functions might need it) or the query isn't a utility command (and
* hence could require redoing parse analysis and planning).
*/
if (numParams > 0 ||
(psrc->raw_parse_tree &&
analyze_requires_snapshot(psrc->raw_parse_tree)))
{
PushActiveSnapshot(GetTransactionSnapshot());
snapshot_set = true;
}

Another fix would be to add check for parseTree into
analyze_requires_snapshot().

Thanks ,
Rushabh Lathia
www.EnterpriseDB.com

------------------------------------------------------------------------

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com