Skip to content

sqlite3_bind_bug68849 fail on x86 with sqlite 3.43 #12076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andypost opened this issue Aug 29, 2023 · 16 comments
Open

sqlite3_bind_bug68849 fail on x86 with sqlite 3.43 #12076

andypost opened this issue Aug 29, 2023 · 16 comments

Comments

@andypost
Copy link
Contributor

andypost commented Aug 29, 2023

Description

The test ext/sqlite3/tests/sqlite3_bind_bug68849.phpt fail on x86 arch (32-bits)

Previous build (8.2.10RC1) passed but sqlite was 3.42.0 (current is 3.43.0)

The same test failed building PHP 8.3RC1 so very probably precision somehow affected

========DIFF========
--
       ["b"]=>
       string(5) "hello"
       ["c"]=>
[1;32m007+   float(3.1399999999999997)[0m
[1;31m007-   float(3.14)[0m
     }
     array(3) {
       ["a"]=>
--
       ["b"]=>
       string(5) "hello"
       ["c"]=>
[1;32m015+   float(3.1399999999999997)[0m
[1;31m015-   float(3.14)[0m
     }
     array(3) {
       ["a"]=>
--
       ["b"]=>
       string(2) "42"
       ["c"]=>
[1;32m023+   float(0.42000000000000004)[0m
[1;31m023-   float(0.42)[0m
     }
     array(3) {
       ["a"]=>
--
========DONE========

Refs

PHP Version

PHP 8.1.22, 8.2.9, 8.3.0RC1

Operating System

Alpinelinux

@andypost
Copy link
Contributor Author

Confirm it fails on 8.1.22 too when sqlite3 is 3.43.0

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Aug 31, 2023

I did a little research on this issue.
I have prepared 3 containers with docker.

  1. 32bit Alpine, Sqlite3.42
  2. 32bit Alpine, Sqlite3.43
  3. 64bit Alpine, Sqlite3.43

Among these, only "2" is reproduced, however, a little more research revealed something strange.
When executing only the select statement using the sqlite DB file created and inserted in the "2" environment in the other two environments, the same result as "2" was obtained.
Conversely, if the DB file created and inserted in "1, 3" is used in "2", the result will be normal.

From this it at least appears that the problem is occurring with writes rather than reads.

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Aug 31, 2023

I tried following these steps:

32bit Alpine, Sqlite3.43

Create a table from php and insert it:

# php test.php

<?php

$db = new SQLite3('sq');

$db->exec("CREATE TABLE test (id int, value REAL);" .
        "INSERT INTO test VALUES (1, 3.14);"
);

Insert directly from sqlite:

# sqlite sq
sqlite> INSERT INTO test VALUES (2, 3.14);

Check the data from sqlite:

sqlite> select * from test;
1|3.14
2|3.14
sqlite> SELECT (SELECT value FROM test WHERE id = 1) == (SELECT value FROM test WHERE id = 2);
0

Check data from php:

<?php

$db = new SQLite3('sq');

$s = $db->prepare('SELECT * FROM test WHERE id = ?;');
$s->bindValue(1, 1);
$r = $s->execute();
var_dump($r->fetchArray(SQLITE3_ASSOC));

$s = $db->prepare('SELECT * FROM test WHERE id = ?;');
$s->bindValue(1, 2);
$r = $s->execute();
var_dump($r->fetchArray(SQLITE3_ASSOC));
array(2) {
  ["id"]=>
  int(1)
  ["value"]=>
  float(3.1399999999999997)
}
array(2) {
  ["id"]=>
  int(2)
  ["value"]=>
  float(3.14)
}

I currently have no idea why this is happening.

@SakiTakamachi
Copy link
Member

Oops.....

create table test (lang text, value real);

write.rb

#!/usr/bin/ruby

require 'sqlite3'

db = SQLite3::Database.new('sq')
db.execute("INSERT INTO test VALUES ('ruby', 3.14);")
db.close

write.php

<?php

$db = new SQLite3('sq');
$db->exec("INSERT INTO test VALUES ('php', 3.14);");
$db->close();
ruby write.rb
php write.php

result:

array(2) {
  ["lang"]=>
  string(4) "ruby"
  ["value"]=>
  float(3.14)
}
array(2) {
  ["lang"]=>
  string(3) "php"
  ["value"]=>
  float(3.1399999999999997)
}

@andypost
Copy link
Contributor Author

andypost commented Sep 2, 2023

One more mysterious detail - it fails only on x86 32-bits, on armv7 and armhf arches (32-bits too) it passed(

@SakiTakamachi
Copy link
Member

Using a prepared statement and binding the value gives us 3.14.

Maybe there is something wrong with the zval string?

I haven't done any research yet, so I'm guessing...

@SakiTakamachi
Copy link
Member

Apparently ruby ​​uses prepared statements inside execute.

It was a bit premature to assume that php was the cause.

Looks like I need to investigate a little more carefully.

@SakiTakamachi
Copy link
Member

I might have gotten some hints.
Even using prepared statements, the results could sometimes be corrupted.

When passing a value as a string:

$s->bindValue(1, '3.14');

or

$s->bindValue(1, 3.14, SQLITE3_TEXT);

// result
float(3.1399999999999997)

When passing a value as a float:

$s->bindValue(1, 3.14);

or

$s->bindValue(1, '3.14', SQLITE3_FLOAT);

//result
float(3.14)

@andypost
Copy link
Contributor Author

andypost commented Sep 3, 2023

So default is SQLITE3_TEXT which means that it's sqlite issue when text casted to float?

@SakiTakamachi
Copy link
Member

I feel that way too.

However, unless you establish a reproduction procedure that excludes PHP or identify the source code that is causing the problem, it will be difficult to get the SQLite side to acknowledge the problem.

So we need to somehow establish a reproducible procedure.

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Sep 3, 2023

I currently have no idea why this is happening.....
I mention this to avoid any unnecessary worries, but all of these tests were performed in the same docker container.

php:

<?php

$db = new SQLite3('sq');

$s = $db->prepare('SELECT ? == CAST((SELECT CAST(? as TEXT)) as REAL);');
$s->bindValue(1, 3.14);
$s->bindValue(2, 3.14);
$r = $s->execute();
var_dump($r->fetchArray(SQLITE3_ASSOC));

// result
array(1) {
  ["? == CAST((SELECT CAST(? as TEXT)) as REAL)"]=>
  int(0)
}

ruby:

#!/usr/bin/ruby

require 'sqlite3'

db = SQLite3::Database.new('sq')

s = db.prepare("SELECT ? == CAST((SELECT CAST(? as TEXT)) as REAL);")
s.bind_param(1, 3.14)
s.bind_param(2, 3.14)
s.execute().each do |row|
    p row
end

// result
[1]

python:

import sqlite3

db = sqlite3.connect('sq')
cur = db.cursor()

cur.execute('SELECT ? == CAST((SELECT CAST(? as TEXT)) as REAL);', (3.14, 3.14))

print(cur.fetchall())

// result
[(1,)]

cli:

sqlite> SELECT 3.14 == CAST((SELECT CAST(3.14 as TEXT)) as REAL);
1

At least in this case, only php has broken results.
I'm starting to think about the possibility that the sqlite API being called is using an outdated one.

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Sep 3, 2023

I wrote C and tried inserting it so that the conditions are almost the same as when using php's sqlite3.
Unfortunately, it doesn't seem to break in this case.

#include <stdio.h>
#include <sqlite3.h>

int main() {
    sqlite3* db;
    sqlite3_open_v2("sq", &db, 0x00000002 | 0x00000004, NULL);

    sqlite3_exec(
        db,
        "INSERT INTO test VALUES ('c', 3.14);",
        NULL,
        NULL,
        NULL
    );

    sqlite3_close(db);

    return 0;
}

php read result:

array(2) {
  ["lang"]=>
  string(1) "c"
  ["value"]=>
  float(3.14)
}

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Sep 4, 2023

I understand the cause.
Floating point behavior on 32-bit architectures may vary depending on FPU settings.

_xpfpa_fpu_cw = (_xpfpa_fpu_oldcw & ~0x100) | 0x200; \

The Linux default should be 64bit, but it seems that 53bit is set in the environment like this in the startup of php.
When I tried building php with this set to 64bit, I got the expected results.

Changing to a 64-bit system would certainly solve the problem, but I'm worried about the impact on other parts.
(However, we don't know how many 32-bit architectures are currently in use...)

Since the default value of windows seems to be 53bit, it may be necessary to increase the number of conditional branches of the preprocessor in some cases.

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Sep 4, 2023

First of all, I reported this problem and the cause to the sqlite side.

Regarding the response on the php side, it seems better to wait for their response first and then discuss it.

https://www.sqlite.org/forum/forumpost/abbb95376ec6cd5f

@SakiTakamachi
Copy link
Member

SakiTakamachi commented Sep 4, 2023

I've been asking a lot of questions on the Sqlite forum.

Possible solutions that you can employ:

1. Do not use in-line assembly to artificially lower the precision of computations involving "long double".
2. Run "sqlite3_test_control(SQLITE_TESTCTRL_USELONGDOUBLE,0);" to prevent SQLite from using "long double" and to force it to use the slower algorithm that gives full accuracy with ordinary binary-64.

Note that #2 works in SQLite 3.43.0, but it is not a supported interface and is not recommended. You really should be doing #1.

I think it's a bit unreasonable to change the precision of double in patch versions.
I think the fundamental solution is not a patch version, but something that should be proposed in the php8.4 RFC. Until then, I'm wondering why not use "2" as a workaround.

@andypost
Copy link
Contributor Author

andypost commented Nov 8, 2023

After upgrade to sqlite 3.44.0 another test started to fail #12633

TEST 12220/16282 [ext/sqlite3/tests/sqlite3_defensive.phpt]
========DIFF========
     bool(true)
003+ int(0)
     int(1)
004- int(1)
     
     Warning: SQLite3::querySingle(): Unable to prepare statement: 1, table sqlite_master may not be modified in %s on line %d
     bool(false)

mfrischknecht added a commit to mfrischknecht/nixpkgs that referenced this issue Nov 25, 2023
PHP unit tests are broken with SQLite >= 3.43 [1].
If I understand the discussion in the SQLite forums [2]
on the issue correctly, the trigger for this should not
be a problem with SQLite itself but the test itself
(and thus using a current SQLite version shouldn't
generally be a problem for actual PHP code).

[1]: php/php-src#12076
[2]: https://www.sqlite.org/forum/forumpost/abbb95376ec6cd5f
github-actions bot pushed a commit to NixOS/nixpkgs that referenced this issue Nov 25, 2023
PHP unit tests are broken with SQLite >= 3.43 [1].
If I understand the discussion in the SQLite forums [2]
on the issue correctly, the trigger for this should not
be a problem with SQLite itself but the test itself
(and thus using a current SQLite version shouldn't
generally be a problem for actual PHP code).

[1]: php/php-src#12076
[2]: https://www.sqlite.org/forum/forumpost/abbb95376ec6cd5f

(cherry picked from commit fb1cc4a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants