[SkyCipherr]

Summary

Related to #12412.

The cpp/missing-check-scanf query can report an output variable after a failed scanf-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 scanf output arguments. Other direct address uses and reference outputs are excluded because alias reads are not fully modeled by the current flow configuration. Parenthesized and conditional reference aliases remain a known gap, now characterized by tests.

Tests

• Initially add 19 C and 5 C++ independently written regression functions covering fallback initialization, saved and overwritten return values, multiple outputs, partial initialization, conversion counts, conditional reads, unsafe updates, repeated calls, and pointer/reference aliases.
• Remove the two existing known false positives in scan_and_write and regenerate the expected results with codeql test accept after reviewing the differences.
• Retain two known safe-alias false positives as SPURIOUS assertions. Existing alias-read modeling gaps remain marked as MISSING, with assertions preserving the downstream warnings.
• Add a minorAnalysis change note.

Initial validation used CodeQL CLI 2.27.2 against main at b5b81654b6e16786e6c5669f696d423d8764e336:

codeql query format --check-only cpp/ql/src/Critical/MissingCheckScanf.ql
codeql test run cpp/ql/test/query-tests/Critical/MissingCheckScanf --additional-packs=. --threads=2 --ram=4096
codeql test run cpp/ql/test/library-tests/scanf --additional-packs=. --threads=2 --ram=4096
python misc/scripts/check-query-ids.py
git diff --check

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.

For the initial 24-function fixture set, 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.cpp results 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.c file linked in the issue was also compiled and extracted separately, with its contents verified against Git blob fcef8798c615c55e62c5b272b36c2cc241d4e134. On the same database, the unchanged query reports the return hostid at 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 for sys/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.

Review follow-up

Commit 2606c679330193d1af9a0be2f66b1b9f5f844e91 follows the review suggestions while keeping the agreed scope:

• Replace the unused existential index with _.
• Add seven characterization functions (31 total: 24 C and 7 C++). Three cover positional arguments with partial fallback, complete fallback, and unchecked outputs; two cover parenthesized/conditional reference aliases; two cover potentially failing literals or suppressed conversions before %n.
• Explicitly retain MISSING and SPURIOUS assertions for current precision gaps rather than claiming they are fixed. No blanket positional-format exclusion, general enclosing-lvalue alias fix, or %n reachability fix is included.

After syncing the maintainer's merge of main, local validation with CLI 2.27.4 passed both query tests, all three scanf library tests, QL formatting, the official query-ID check (Python 3.12.14), GCC/G++ syntax-only checks, and git diff --check.

On the same expanded test database, the pre-follow-up and simplified queries both produce 38 results with identical source/sink locations and messages. The original fixture retains its 19 results. Reanalysis of the original Issue file still removes its false positive while preserving the unchecked-read control. Upstream CI for this new commit is separate from these local results.

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.

[SkyCipherr]

Hi, just following up on this PR. The workflow runs are still marked action_required. When you have a chance, could a maintainer help approve the runs if appropriate and review the proposed fix? I'm happy to address any feedback. Thanks for your time!

[geoffw0]

Code changes look good to me, certainly gets the job done. Very thorough tests (justified I think as there are a lot of corner cases here).

I'll start CI...

[Copilot]

🟡 Changes recommended

The new barrier can suppress valid uninitialized-read warnings involving %n, reference aliases, and positional scanf arguments, leaving three critical correctness issues unresolved.

3 open issues

• Account for reference aliases when applying initialization barriers · New
• Exclude positional formats from initialization barriers · New
• Do not treat return count as proof that %n was reached · New

What changed in this PR

Updates cpp/missing-check-scanf to reduce false positives when a failure branch initializes an output variable with a fallback value, addressing #12412.

Changes:

• Adds branch-sensitive flow barriers for eligible local outputs while retaining read-site guards.
• Adds 24 C/C++ regression functions and regenerates expected results.
• Adds a minorAnalysis change note describing the precision improvement.

File	Description
cpp/​ql/​test/​query-tests/​Critical/​MissingCheckScanf/​test.cpp	Removes expectations for two known fallback-initialization false positives.
cpp/​ql/​test/​query-tests/​Critical/​MissingCheckScanf/​overwrites.cpp	Adds reference and pointer-alias regression cases.
cpp/​ql/​test/​query-tests/​Critical/​MissingCheckScanf/​overwrites.c	Adds fallback, conversion-count, and unsafe-read regression cases.
cpp/​ql/​test/​query-tests/​Critical/​MissingCheckScanf/​MissingCheckScanf.expected	Updates generated query results and flow paths.
cpp/​ql/​src/​Critical/​MissingCheckScanf.ql	Adds guarded-flow barriers and local-output eligibility checks.
cpp/​ql/​src/​change-notes/​2026-10-10-missing-check-scanf-overwrite.md	Documents the reduction in false positives.

_{Review effort: Max (auto)}

[geoffw0]

Thanks for addressing Copilot’s concerns and adding tests. 👍

DCA looks great - a good number of false positive results gone. 🚀

Thank you for your contribution to CodeQL analysis! 🎉