Bug #29658 for DBD-mysql: Incorrect return value and segfault with large number of bound parameters

Fri Sep 28 23:07:28 2007 kolbe [...] mysql.com - Ticket created

Subject:	Incorrect return value and segfault with large number of bound parameters
Date:	Fri, 28 Sep 2007 20:07:18 -0700
To:	bug-DBD-mysql [...] rt.cpan.org
From:	Kolbe Kegel <kolbe [...] mysql.com>

Hi, Here is a testcase: use DBI; my $count = shift or die "Must provide a count"; my $dbh = DBI->connect( 'dbi:mysql:test:localhost:;mysql_socket=/home/kolbe/MySQL/inst/5.0.44sp1/mysql.sock', 'root', '', { RaiseError => 1, AutoCommit => 1, PrintError => 0}); my $query = 'SELECT 1 FROM dual WHERE "x" IN ('.join(',',('?') x $count).')'; my @bind = ('dummy') x $count; my $recs = $dbh->selectall_arrayref($query, {Slice=> {}}, @bind); print $recs, "\n"; $ perl test.pl 5000 DBI::db=HASH(0x853520) Segmentation fault $ perl test.pl 4000 ARRAY(0x933240) This is perl, v5.8.8 built for x86_64-linux-thread-multi DBD::mysql 4.005 DBI 1.58 Please let me know if you need further information or have any questions related to this report. Regards, -- Kolbe Kegel, Support Engineer MySQL Inc, Seattle, USA, www.mysql.com

Fri Oct 19 14:39:29 2007 CAPTTOFU [...] cpan.org - Correspondence added

From:

CAPTTOFU [...] cpan.org

Kolbe, Thanks for the report, I'll test and trace this and hopefully get a fix for it soon. regards, Patrick On Fri Sep 28 23:07:28 2007, kolbe@mysql.com wrote: Show quoted text

> Hi, > > Here is a testcase: > > use DBI; > my $count = shift or die "Must provide a count"; > my $dbh = DBI->connect( >

'dbi:mysql:test:localhost:;mysql_socket=/home/kolbe/MySQL/inst/5.0.44sp1/mysql.sock', Show quoted text

> 'root', '', { RaiseError => 1, AutoCommit => 1, PrintError => 0}); > my $query = 'SELECT 1 FROM dual WHERE "x" IN ('.join(',',('?') x > $count).')'; > my @bind = ('dummy') x $count; > my $recs = $dbh->selectall_arrayref($query, {Slice=> {}}, @bind); > print $recs, "\n"; > > > $ perl test.pl 5000 > DBI::db=HASH(0x853520) > Segmentation fault > > $ perl test.pl 4000 > ARRAY(0x933240) > > This is perl, v5.8.8 built for x86_64-linux-thread-multi > DBD::mysql 4.005 > DBI 1.58 > > Please let me know if you need further information or have any > questions > related to this report. > > Regards,

Fri Oct 19 14:39:30 2007 The RT System itself - Status changed from 'new' to 'open'

Fri Nov 02 20:47:25 2007 JGMYERS [...] cpan.org - Correspondence added

From:

JGMYERS [...] cpan.org

This is a bug in the DBI package. It isn't calling SPAGAIN after calling into perl. I don't have permission to move this bug into the DBI queue, please do that. Attached a proposed fix. The XS code should be audited for more missing SPAGAIN calls. I believe there is a missing one after the call_sv() in DBI::var::FETCH()

diff -ru ./Driver.xst ../DBI-1.53-spagain/Driver.xst --- ./Driver.xst 2006-02-07 14:24:45.000000000 -0800 +++ ../DBI-1.53-spagain/Driver.xst 2007-11-02 16:20:14.000000000 -0700 @@ -111,7 +111,9 @@ (DBD_ATTRIB_TRUE(attr,"Slice",5,tmp_svp) || DBD_ATTRIB_TRUE(attr,"Columns",7,tmp_svp)) ) { /* fallback to perl implementation */ - ST(0) = dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::selectall_arrayref", items); + SV *tmp = dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::selectall_arrayref", items); + SPAGAIN; + ST(0) = tmp; XSRETURN(1); } } @@ -121,6 +123,7 @@ } else { sth = dbixst_bounce_method("prepare", 3); + SPAGAIN; if (!SvROK(sth)) XSRETURN_UNDEF; } @@ -157,6 +160,7 @@ else { /* --- prepare --- */ sth = dbixst_bounce_method("prepare", 3); + SPAGAIN; if (!SvROK(sth)) { if (is_selectrow_array) { XSRETURN_EMPTY; } else { XSRETURN_UNDEF; } } @@ -383,9 +387,13 @@ * would be &sv_undef for error or an SV holding the imp_data. */ SV *sv = dbd_take_imp_data(h, imp_xxh, NULL); - ST(0) = (SvOK(sv) && !SvTRUE(sv)) - ? dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::take_imp_data", items) - : sv_2mortal(sv); + if (SvOK(sv) && !SvTRUE(sv)) { + SV *tmp = dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::take_imp_data", items); + SPAGAIN; + ST(0) = tmp; + } else { + ST(0) = sv_2mortal(sv); + } #endif @@ -633,7 +641,9 @@ SV * batch_row_count CODE: if (SvOK(slice)) { /* fallback to perl implementation */ - ST(0) = dbixst_bounce_method("DBD::~DRIVER~::st::SUPER::fetchall_arrayref", 3); + SV *tmp = dbixst_bounce_method("DBD::~DRIVER~::st::SUPER::fetchall_arrayref", 3); + SPAGAIN; + ST(0) = tmp; } else { ST(0) = dbdxst_fetchall_arrayref(sth, slice, batch_row_count); Only in ../DBI-1.53-spagain/: Driver.xst~

Sat Nov 03 08:35:38 2007 CAPTTOFU [...] cpan.org - Correspondence added

From:

CAPTTOFU [...] cpan.org

On Fri Nov 02 20:47:25 2007, JGMYERS wrote: Show quoted text

> This is a bug in the DBI package. It isn't calling SPAGAIN after > calling into perl. > > I don't have permission to move this bug into the DBI queue, please do

that. Show quoted text

> > Attached a proposed fix. > > The XS code should be audited for more missing SPAGAIN calls. I believe > there is a missing one after the call_sv() in DBI::var::FETCH()

Thanks! I was going to investigate this over the weekend, now that I have a little time, and this will save me time. I'll contact the DBI maintainers and get this into them.

Mon Nov 05 14:36:55 2007 JGMYERS [...] cpan.org - Cc JGMYERS added

Tue Nov 13 12:49:15 2007 JGMYERS [...] cpan.org - Correspondence added

From:

JGMYERS [...] cpan.org

Attached is an updated fix. The previous fix broke selectrow_arrayref/selectrow_array as it omitted adjusting the stack pointer for the difference between CODE and PPCODE.

diff -ru ./Driver.xst ../DBI-1.53-spagain/Driver.xst --- ./Driver.xst 2006-02-07 14:24:45.000000000 -0800 +++ ../DBI-1.53-spagain/Driver.xst 2007-11-05 14:28:12.000000000 -0800 @@ -111,7 +111,9 @@ (DBD_ATTRIB_TRUE(attr,"Slice",5,tmp_svp) || DBD_ATTRIB_TRUE(attr,"Columns",7,tmp_svp)) ) { /* fallback to perl implementation */ - ST(0) = dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::selectall_arrayref", items); + SV *tmp = dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::selectall_arrayref", items); + SPAGAIN; + ST(0) = tmp; XSRETURN(1); } } @@ -121,6 +123,7 @@ } else { sth = dbixst_bounce_method("prepare", 3); + SPAGAIN; if (!SvROK(sth)) XSRETURN_UNDEF; } @@ -157,6 +160,8 @@ else { /* --- prepare --- */ sth = dbixst_bounce_method("prepare", 3); + SPAGAIN; + sp -= items; if (!SvROK(sth)) { if (is_selectrow_array) { XSRETURN_EMPTY; } else { XSRETURN_UNDEF; } } @@ -383,9 +388,13 @@ * would be &sv_undef for error or an SV holding the imp_data. */ SV *sv = dbd_take_imp_data(h, imp_xxh, NULL); - ST(0) = (SvOK(sv) && !SvTRUE(sv)) - ? dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::take_imp_data", items) - : sv_2mortal(sv); + if (SvOK(sv) && !SvTRUE(sv)) { + SV *tmp = dbixst_bounce_method("DBD::~DRIVER~::db::SUPER::take_imp_data", items); + SPAGAIN; + ST(0) = tmp; + } else { + ST(0) = sv_2mortal(sv); + } #endif @@ -633,7 +642,9 @@ SV * batch_row_count CODE: if (SvOK(slice)) { /* fallback to perl implementation */ - ST(0) = dbixst_bounce_method("DBD::~DRIVER~::st::SUPER::fetchall_arrayref", 3); + SV *tmp = dbixst_bounce_method("DBD::~DRIVER~::st::SUPER::fetchall_arrayref", 3); + SPAGAIN; + ST(0) = tmp; } else { ST(0) = dbdxst_fetchall_arrayref(sth, slice, batch_row_count); Only in ../DBI-1.53-spagain: Driver.xst~

Fri Oct 28 04:10:42 2016 MICHIELB [...] cpan.org - Correspondence added

Fixed in DBI 1.602 (svn rev 10706) 8th February 2008 https://github.com/perl5-dbi/dbi/commit/5707fa36982eb24785f8cc989109168c85b32187

Fri Oct 28 04:10:44 2016 MICHIELB [...] cpan.org - Status changed from 'open' to 'resolved'