run-tests: capture Windows-specific error exit codes #6722
Conversation
the lack of such a check leads to false-passes of tests on Windows which expect no output, but produce a segfault or similar issue. I discovered this a while ago due to bad tests in an extension I maintain.
|
There are two test failures on Appveyor, with Termsig -1 and -2. |
|
I had a feeling that it might show up some issues in php-src own tests. My intent was only to capture The broken tests are these: |
this is sufficient to capture segfaults and such, but won't capture signals produced by TerminateProcess and similar.
|
Some heap corruption errors are showing up on the second run. I won't take blame for those. 😅 |
| @@ -1188,6 +1188,9 @@ function system_with_timeout($commandline, $env = null, $stdin = null, $captureS | |||
| } | |||
| if ($stat["exitcode"] > 128 && $stat["exitcode"] < 160) { | |||
| $data .= "\nTermsig=" . ($stat["exitcode"] - 128) . "\n"; | |||
| } else if (defined('PHP_WINDOWS_VERSION_MAJOR') && (($stat["exitcode"] >> 28) & 0b1111) === 0b1100) { | |||
There was a problem hiding this comment.
It's not available prior to 8.0.
There was a problem hiding this comment.
ctrl+f here: https://github.com/php/php-src/blob/992960e38eca2fd46601a6f286ef1908ca802e84/run-tests.php
This PR targets 7.3, IS_WINDOWS is not defined here. It's only defined in 8.0+'s version of run-tests.php: https://github.com/php/php-src/blob/PHP-8.0/run-tests.php#L155
|
Maybe @cmb69 can take a look at the failing This change should probably target PHP-7.4. |
|
I have been able to get a segfault (not the heap corruption reported by AppVeyor) when running ext\dba\tests\bug65708.php, but only on rare occassions, and only with PHP-7.3. That looks like a UAF (backtrace). I'm going to rebuild (for reference the fail happend with https://ci.appveyor.com/project/php/php-src/builds/37917434).
ACK. While I very much appreciate to catch segfaults in tests on Windows, I think this is more suitable for 7.4+. Since the patch applies cleanly there, there is no need to rebase/change the target branch, though. Thanks for the PR, Dylan! |
|
This time AppVeyor passed, so I think it's best to merge this PR, and to see whether the issue reproduces. |
Just for the record, it just reproduced for https://ci.appveyor.com/project/php/php-src/builds/38000695 (this time in the 2nd job). There appears to be something wrong, but it is not related to this PR. |
the lack of such a check leads to false-passes of tests on Windows, because if a segfault occurs, there's no indication from run-tests. This means that a test which crashed after emitting all of its expected output (or not producing any output) would still be seen as passed, even though it actually crashed.
I discovered this problem because of bad tests in an extension I maintain which had empty --EXPECT-- sections, which led to segfaulting tests passing.
This PR makes run-tests capture ERROR severity statuses, as described here: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/87fba13e-bf06-450e-83b1-9241dc81e781