Fix MSan canary census and unavailable-instrumentation reporting - #570
Conversation
…rumentation as skipped
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aec29db6aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Tests/TestRunnerSemanticsReal.cpp", | ||
| # Verifies the sanitizer harness and instrumented C++ runtime, not a | ||
| # test-local copy of engine behavior. Non-MSan builds explicitly skip it. | ||
| "Tests/TestMSanCanary.cpp", |
There was a problem hiding this comment.
Classifying this file as production-source makes its two tests contribute to the blocking minimumProductionSourceTests census, even though non-MSan builds now skip them and MSan builds exercise compiler instrumentation and libc++, not SparkEngine production source or a packaged binary. This overstates shipped-code coverage and can mask the removal of real production tests near the ratchet floor; classify infrastructure canaries separately rather than counting them as production-source tests.
Useful? React with 👍 / 👎.
⏳ CodeQL Report PendingThe pull-request head changed. Results from earlier commits do not apply while the new head is being scanned. Current PR head: |
1 participant
The instrumented-libc++ PR (#568) adds sanitizer harness canaries that the source census currently rejects as new mirror tests. On non-MSan builds they also report fabricated passes.
Classify this exact canary file as a harness test and explicitly skip both tests when MSan instrumentation is unavailable. The instrumented branches and their shadow-memory assertions remain unchanged.
Validation: census check passes (7,263 registered tests); actual non-MSan canary harness reports 2 registered, 2 skipped, 0 passed assertions (before: 0 skipped and 2 passed assertions); documentation generators and whitespace checks pass. Independent source review found no issues. An instrumented MSan run is still required; this does not certify #568 or the release.
This companion PR targets #568 so its runtime changes can retain their own validation and review.