Bug in selectrow_arrayref (C version)

I have tracked down a subtle bug in selectrow_arrayref. It's in
DBI-1.54/Driver.xst, so it's probably in various drivers - I found it
in DBD-Oracle-1.19.

What happens is selectrow_arrayref calls dbixst_bounce_method which uses
EXPAND to grow the stack. This may cause the stack to be realloc'd
with the result that the stack has moved to a new location when
dbixst_bounce_method returns. selectrow_arrayref doesn't take this into
account and carries on using the old location in its copy of sp.
Just before returning it uses PUTBACK to put the incorrect value into
stack_sp. In my case, this made the DBI dispatch code think that a
negative number of items had been returned to it:

dbd_st_execute SELECT returned (SUCCESS, rpc0, fn4, out0)
dbd_st_fetch 1 fields...
OCIStmtFetch(a67040,a60c78,1,2,0)=SUCCESS
dbih_setup_fbav alloc for 1 fields
dbih_setup_fbav now 1 fields
dbd_st_fetch 1 fields SUCCESS
0 (rc=0): '709'
dbd_st_finish
OCIStmtFetch(a67040,a60c78,0,2,0)=SUCCESS
<- selectrow_array= ( ) [-210625 items] ...

Suprisingly, perl manages to recover from this situation quite well. The
end result was that a call to selectrow_array returned no data instead of
the expected value.

The patch below is my attempt at fixing the problem. Someone who actually
understands perl and XS may well be able to come up with something better.
The patch is for Driver.xst in DBI version 1.54. I tested the fix in
DBD::Oracle and it does provide a cure.

I've checked all the other places where dbixst_bounce_method is used, and
as far as I can tell this is the only one that causes a problem.
selectall_arrayref may be affected, but I don't think this is the case.
All of the other functions that use it appear to not use sp before they
return.

Rob Davies
The Sanger Institute http://www.sanger.ac.uk/Users/rmd/
Hinxton, Cambs., Tel. +44 (1223) 834244
CB10 1SA, U.K. Fax. +44 (1223) 494919

diff -Naur DBI-1.54/Driver.xst DBI-1.54.patched/Driver.xst
--- DBI-1.54/Driver.xst 2007-02-22 01:36:15.000000000 +0000
+++ DBI-1.54.patched/Driver.xst 2007-04-25 21:24:00.843000000 +0000
@@ -157,6 +157,12 @@
else {
/* --- prepare --- */
sth = dbixst_bounce_method("prepare", 3);
+
+ /* The following two lines are needed in case dbixst_bounce_method
+ caused the stack to be reallocated */
+ SPAGAIN;
+ sp -= items;
+
if (!SvROK(sth)) {
if (is_selectrow_array) { XSRETURN_EMPTY; } else { XSRETURN_UNDEF; }
}

Re: Bug in selectrow_arrayref (C version)

On Wed, Apr 25, 2007 at 11:30:52PM +0100, Rob Davies wrote:
>
> I have tracked down a subtle bug in selectrow_arrayref. It's in
> DBI-1.54/Driver.xst, so it's probably in various drivers - I found it
> in DBD-Oracle-1.19.
>
> What happens is selectrow_arrayref calls dbixst_bounce_method which uses
> EXPAND to grow the stack. This may cause the stack to be realloc'd
> with the result that the stack has moved to a new location when
> dbixst_bounce_method returns. selectrow_arrayref doesn't take this into
> account and carries on using the old location in its copy of sp.

Good catch.

> The patch below is my attempt at fixing the problem. Someone who actually
> understands perl and XS may well be able to come up with something better.
> The patch is for Driver.xst in DBI version 1.54. I tested the fix in
> DBD::Oracle and it does provide a cure.
>
> I've checked all the other places where dbixst_bounce_method is used, and
> as far as I can tell this is the only one that causes a problem.
> selectall_arrayref may be affected, but I don't think this is the case.
> All of the other functions that use it appear to not use sp before they
> return.

> + /* The following two lines are needed in case dbixst_bounce_method
> + caused the stack to be reallocated */
> + SPAGAIN;
> + sp -= items;

Thanks for this Rob. I've applied the fix to both selectall_arrayref
and selectrow_arrayref.

Tim.

p.s. I expect to release 1.55 next week.