Conversation
The compiler rejects every statically detectable undefined operation, but undefined behavior that only appears at runtime in the generated C++ (a shift count >= 64, a zero divisor, signed overflow, misaligned access, ...) is silently compiled into whatever the CPU happens to do. tpc already supports --sanitize, but nothing in CI used it. run-tests.php gains --sanitize <list>: every test binary is compiled with the given sanitizers, and UBSAN_OPTIONS=halt_on_error=1 turns a report into a failing test instead of a line in its output. The option travels to the parallel workers with the other globals. The Linux PHPT job gets one more matrix entry, PHP 8.5 with sanitize=undefined. The current suite is clean under UBSan (1247 passed, 27 skipped locally), so the job starts green and guards against new undefined behavior in generated code. Release packaging still comes only from the unsanitized entries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The compiler rejects every statically detectable undefined operation, but undefined behavior that only shows up at runtime in the generated C++ is compiled into whatever the CPU happens to do.
tpcalready supports--sanitize, but nothing in CI used it, so a regression of this kind is invisible to the test suite: the binary prints some value, and if the PHPT expectation was written from that binary, the test passes.The change
run-tests.php --sanitize <list>(undefined,address, or both): every test binary is compiled with--sanitize <list>, andUBSAN_OPTIONS=halt_on_error=1turns a report into a failing test instead of a line in its output. The option reaches the parallel workers together with the other globals. An unknown sanitizer is rejected up front.sanitize: undefined.sanitize: [""]in the base matrix makes GitHub add it as a separate job rather than merging the key into the existing PHP 8.5 entry. Its artifacts get a-undefinedsuffix, and release packaging still comes only from the unsanitized entries.It catches what it should
A PHPT whose expectation was written from the binary (
shiftLeft(1, 64)prints1) passes without the option and fails with it, with the location in the generated C++:The same trap fails when it runs among other tests with
-j4, so the option does reach the workers and is not bypassed by the build cache (sanitizeis part of the native command options).The current suite is clean
tests/compilerwith--sanitize undefinedPHPT - PHP 8.5 ZTS (undefined sanitizer))So the job starts green; from here on, new undefined behavior in generated code fails CI instead of silently changing results. All 9 jobs of that run passed, and
actionlintreports no issues for the workflow.