Repository navigation
C++: reduce scanf false positives after fallback initialization - #22804
Open
SkyCipherr wants to merge 1 commit into
Open
SkyCipherr wants to merge 1 commit into
SkyCipherr wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
Summary
Related to #12412.
The
cpp/missing-check-scanfquery can report an output variable after a failedscanf-like call even when the failure branch assigns a default value. The successful flow still reaches the merged read, where the existing guard check does not recognize that both branches have initialized the variable.This change adds an indirect IR guard barrier for branches that guarantee the corresponding output was written. It reuses the existing minimum conversion count, including the handling of suppressed conversions and
%n, and retains the existing guard check at the read. Unsafe reads on the failure path remain tracked.The barrier is restricted to non-reference local output variables whose other lvalue uses are assignments, increments, or
scanfoutput arguments. Other address uses and reference outputs are excluded because alias reads are not fully modeled by the current flow configuration.Tests
scan_and_writeand regenerate the expected results withcodeql test acceptafter reviewing the differences.SPURIOUSassertions. Existing alias-read modeling gaps remain marked asMISSING, with assertions preserving the downstream warnings.minorAnalysischange note.Validation used CodeQL CLI 2.27.2 against main at
b5b81654b6e16786e6c5669f696d423d8764e336:Both query tests and all three library tests passed. The query ID check passed with Python 3.12.14. The added fixtures also passed GCC C11 and G++ C++11 syntax-only checks.
A comparison of the unchanged main query and this candidate on the same expanded test database produced 52 and 35 results respectively: 17 removed and none added. Of the removed results, 14 are safe-case false positives and 3 are redundant later uses after an unsafe read that is still reported. The original
test.cppresults changed only from 21 to 19 by removing its two known false positives.A separate minimal C/C++ database, extracted through real GCC/G++ compilation, was analyzed with
--rerun: its unsafe read remains reported, while its safe fallback read is no longer reported (two alerts become one).The complete original
gethostid.cfile linked in the issue was also compiled and extracted separately, with its contents verified against Git blobfcef8798c615c55e62c5b272b36c2cc241d4e134. On the same database, the unchanged query reports thereturn hostidat line 55; this candidate does not. An unchecked-read control remains reported, a safe fallback control becomes unreported, and a zero-initialized static-local control is unreported by both queries. This was a Windows/MinGW extraction with a local platform-compatibility header forsys/systeminfo.h, not a full OpenZFS build. The target function's source was unchanged. None of this third-party source is included in the proposed changes.Scope and Limitations
This is not a general pointer or reference aliasing fix. Safe fallback cases with other address uses may still be reported, and pre-existing alias-read modeling gaps remain. The results above describe the covered fixtures, not a proof of absence of false negatives in other programs.
The historical static-initialization concern mentioned in the issue is already unreported in the tested main baseline; this change does not claim credit for that behavior. Other historical code-scanning alerts linked in the discussion have not been individually reproduced from their complete projects.
The full repository test suite and a full OpenZFS build were not run. Upstream CI and the change-note release workflow have not been run locally.