segfault using ParamValues from $sth

[myrrhlin]

DBD::mysql version

4.050

MySQL client version

8.0.39

Server version

8.0.39-0ubuntu0.20.04.1

Operating system version

Ubuntu 20.04.6 LTS

What happened?

Started work in an older codebase. I tried using DBI::Log module in a test script, and got a segfault.

$ perl test.pl
-- Wed Oct 15 13:36:21 2024
--  /home/pyrrhlin/repo/path/DB.pm 160
SET NAMES 'utf8'

Segmentation fault (core dumped)

As you can see, the first query, using $dbh->do() is fine. The second query is an $sth->execute(42) on a SELECT statement with one placeholder, an integer id field, that causes the segfault. (Note: nowhere did we use $sth->bind_param.)

I used perl debugger to find the offending statement, and it's this one:

%values = %{$sth->{ParamValues}};

So this module uses the statement handle attribute ParamValues to find out what the bound parameter values were. This is a sometimes supported interface from DBI. It's expected to return a hashref, where

keys of the hash are the 'names' of the placeholders, typically integers starting at 1. Returns undef if not supported by the driver

However, it should be noted that:

Some drivers cannot provide valid values for some or all of these attributes until after $sth->execute has been successfully called. Typically the attribute will be undef in these situations

So, it's clear to me that DBI::Log should not be trying to read this data before execution, because for some drivers (including obviously mysql), the data won't be there yet. Regardless, if the attribute value was undef, as DBI docs suggest, the line causing the segfault would not have been executed. To have entered the if block, the attribute must have had some true value.

In the debugger again, I find what it is:

  DB<2> c
-- Wed Oct 16 15:17:50 2024
--  /home/pyrrhlin/repo/path/DB.pm 160
SET NAMES 'utf8'

DBI::Log::pre_query(/home/pyrrhlin/repo/path/DBI/Log.pm:197):
197:	        if ($sth && $sth->{ParamValues}) {
  DB<2> print 7 if $sth && $sth->{ParamValues}
7
  DB<3> print ref $sth->{ParamValues}
HASH
  DB<4> x $sth->{ParamValues}
0  HASH(0x559dc82a05f8)
   0 => undef
  DB<5> q

So the ParamValues attribute had the true value { 0 => undef }.

However, that data structure was not a "pure-perl" clean hashref, as dereferencing it in list context caused the segfault. Alternative code to copy the hash such as this did not trigger a segfault:

my $h = $sth->{ParamValues}; $values{$_} = $h->{$_} for keys %$h`

Now, reading statement handle docs for DBD::mysql there is no mention of ParamValues attribute, which raises some question whether it is supported at all. The docs do confirm however that:

most attributes are valid only after a successful execute. An undef value will returned otherwise

Testing shows that after execution, the attribute is populated with e.g., { 0 => 42 }, which is almost useful, but non-compliant with the DBI docs (which required the first argument to be under key 1).

So we have these problems:

• under no circumstance should accessing the parameter value, regardless of context, cause a segfault.

• before execution, if we can't provide the values yet, the attribute value should be undef.

• after execution, it should probably be using integer keys starting with 1, e.g. { 1 => 42 }

Other information

Testing performed with system perl on Ubuntu 20.04 LTS :

This is perl 5, version 30, subversion 0 (v5.30.0) built for x86_64-linux-gnu-thread-multi
(with 60 registered patches, see perl -V for more detail)

Apologies I did not have time to try to replicate the failure with a newer version. If I get more time I will try to do that.

Issue #353 also suggests trouble with the xs building the data structures tied behind the statement handle attributes. Perhaps related.

[myrrhlin]

Only a single test references ParamValues at all: rt83494-quotes-comments.t.

If you confirm that we intend to support this attribute, I might try to add some tests, perhaps in some of 4?bindparam.t, or any test file you suggest.

[dveeden]

Did you mean to include the test.pl file?

keys of the hash are the 'names' of the placeholders, typically integers starting at 1.

I'm not sure if these have to start at 1. I'm not even sure if these have to be integers. Some other drivers (e.g. DBD::Pg) support named parameters, not sure how those are handled.

However I'm all for making DBD::mysql follow what DBI::Log does where possible. Do you want to create a PR for that?

[myrrhlin]

I did not include my test.pl because it was not slimmed down to a "minimized" test. I would rather just add a test to the test suite to cover the behavior I found, if that's acceptable. Can you suggest where I should put it?

However, actually solving the problem is probably beyond what I can do. I have no experience as an XS developer, and no time currently to become one.

[dveeden]

I did not include my test.pl because it was not slimmed down to a "minimized" test. I would rather just add a test to the test suite to cover the behavior I found, if that's acceptable. Can you suggest where I should put it?

I think a new file based on the GitHub issue number like t/gh447.t would be best. But if you find an existing test that is related that's also fine with me.

However, actually solving the problem is probably beyond what I can do. I have no experience as an XS developer, and no time currently to become one.

That's fine.

Thanks for the detailed report.

[dveeden]

@myrrhlin Please check #449 and let me know what you think.

I used this to test:

#!/bin/perl
use v5.40;
use Data::Dumper;
use DBI;

my $dbh = DBI->connect('DBI:mysql:database=test', 'root', '');

say "query: SELECT ? (arg: 1)";
my $sth = $dbh->prepare('SELECT ?');
say "before execute:\n" . Dumper($sth->{ParamValues});
$sth->execute(1);
say "after execute:\n" . Dumper($sth->{ParamValues});
while (my @row = $sth->fetchrow_array()) {
	say "result: " . $row[0];
}
$sth->finish;

say "\nquery: SELECT ?, ? (arg: 1,2)";
$sth = $dbh->prepare('SELECT ?, ?');
say "before execute:\n" . Dumper($sth->{ParamValues});
$sth->execute(1, 2);
say "after execute:\n" . Dumper($sth->{ParamValues});
while (my @row = $sth->fetchrow_array()) {
	say "result: " . $row[0] . "," . $row[1];
}
$sth->finish;

output:

query: SELECT ? (arg: 1)
before execute:
$VAR1 = {
          '1' => undef
        };

after execute:
$VAR1 = {
          '1' => 1
        };

result: 1

query: SELECT ?, ? (arg: 1,2)
before execute:
$VAR1 = {
          '2' => undef,
          '1' => undef
        };

after execute:
$VAR1 = {
          '1' => 1,
          '2' => 2
        };

result: 1,2

This will need a testcase before it is merged.

[dveeden]

And this code:

#!/bin/perl
use v5.40;
use Data::Dumper;
use DBI;

my $dbh = DBI->connect('DBI:mysql:database=test', 'root', '', {
    ShowErrorStatement => 1,
});

my $sth = $dbh->prepare('select now(?)');
$sth->bind_param(1, "abc");
$sth->execute;

Results in this output:

DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''abc')' at line 1 [for Statement "select now(?)" with ParamValues: 1='abc'] at ./issue447.pl line 12.

So the ParamValues here look ok to me.

[myrrhlin]

Thanks for your continued attention to this issue!

I was thinking something like this, to really flesh out the expected behavior of the ParamValues attribute. In particular, I want to make sure dereferencing in list context doesn't segfault.
gh447-test
(Note I didn't run these yet, and there's a lot of behavior I'm not sure about -- these tests just represent what I wish would happen 😁!)

[myrrhlin]

Okay. The tests I wrote are passing under the versions I reported, when placeholder indexes in ParamValues all start with 0. Then I changed a single variable to 1, and that should demonstrate correct keys (indexing starting with 1) for all tests. The file still needs some cleanup before i submit a PR but you can try them against your fix if you want.

In this test I was not able to reproduce the segfault in my test. A bit mystified, because it is quite repeatable in its original context. Perhaps later I will find a way to minimize that.