On Wed, Apr 29, 2020 at 5:25 AM Marc Simpson marc@0branch.com wrote:
On Tue, Apr 28, 2020 at 12:20 PM Chris Angelico rosuav@gmail.com wrote:
On Wed, Apr 29, 2020 at 5:13 AM Marc Simpson marc@0branch.com wrote:
[...]
It seems that I can reliably segfault recent Pike 8.1 installs when using sprintf-style SQLite queries: [...]
Reproduced on a very recent build of Pike. Can be done in a more self-contained way using an in-memory database:
Thanks Chris.
Okay, it's been a bit of digging, because behaviour changed at one point - sprintf-style parameters simply weren't accepted. But I've found the commit that introduced the segfault:
commit 6a37db6d0b7ff17c96ffa967c4f4c9064195fc70 Sql.Connection: always attempt to fetch any rows when converting a result to array.
Checking for fields is not sufficient, as some engines do not execute the query until the first row is fetched (SQLite for example). On queries that use sprintf bindings but return no rows (such as inserts), this resulted in the query never executing (and likely leaking the statement).
I'm not sure that that's actually the bug though - might just be where it's exposed. The crash results, I think, from calling fetch_row_array() for all rows, and then afterwards calling fetch_fields(); inside sqlite.cmod fetch_fields(), THIS->stmt has become a NULL pointer, resulting in the crash. (I believe that's happening in fetch_row(), which zeroes out the pointer once it gets SQLITE_DONE.)
That's as much as I can find.
ChrisA