bz2: Fix truncation of total output size causing erroneous errors

[ndossche]

Also switch to uint64_t as that's used on master and makes the code simpler to fix.

[arnaud-lb]

I get that this fixes the scaling of total_out_hi32, which was broken because the operation has unsigned int result type. This also backports #20807.

This looks right to me.

It seems worth it to add a test:

$compressed_file = 'test.bz2';
$sum = 'd4b5e52ed34a774fa645f94369b0c61375436d30';

/*
    $fd = popen("bzip2 > $compressed_file", "w");
    for ($i = 0; $i < 8; $i++) {
        fwrite($fd, str_repeat((string)$i, 2**30));
    }
    fclose($fd);
*/

var_dump(sha1(bzdecompress(file_get_contents($compressed_file))) === $sum);

test.bz2 is 8KiB, so we can embed it to save some time.

[ndossche]

Thanks. The test will consume a lot of memory though so perhaps we should postpone merging this until #20762 & co are resolved

[arnaud-lb]

Yes this seems reasonable

[iluuu1994]

The new test timed out in quite a few jobs:

• https://github.com/php/php-src/actions/runs/22510575543/job/65218841502
• https://github.com/php/php-src/actions/runs/22510575543/job/65218841561
• https://github.com/php/php-src/actions/runs/22510575543/job/65218841561
• https://github.com/php/php-src/actions/runs/22510575543/job/65218841536
• https://github.com/php/php-src/actions/runs/22510575543/job/65218841415
• https://github.com/php/php-src/actions/runs/22510575543/job/65218841402

ASAN could simply be skipped, it's not too surprising it performs worse. Though it also failed for a macOS release job. I presume the test can't be meaningfully made cheaper?

[ndossche]

ASAN could simply be skipped, it's not too surprising it performs worse.

Agreed

Though it also failed for a macOS release job. I presume the test can't be meaningfully made cheaper?

Not really, we need to cross the 4GiB boundary to reproduce the issue. I don't know why macOS fails.

[iluuu1994]

How quickly does the test run on your machine? Given this was limited to macOS apart from ASAN, maybe it's ok to just skip it there as well, assuming the fix is not platform-dependent.

[ndossche]

Fix isn't platform dependent. I pushed some extra changes to the SKIPIF.
Takes around 41s on my i7-4790, on a release build

[iluuu1994]

@ndossche Thanks! The timeout doesn't seem too surprising then. The run-tests.php timeout can be controlled through the TEST_TIMEOUT env var, but only for all tests. Another option might be to allow increasing this on a test-by-test basis. I'm happy with the SKIPIF though.

[ndossche]

We'll see if the issue reoccurs and can take action then