codeql: fix argv-unchecked-index scope and its same-line guard

[colourbill-ctrl]

Closes the two open iccdev/argv-unchecked-index code-scanning alerts on
iccApplyProfiles.cpp:279, and fixes the reason the query produced them.

• alert 2366
• alert 2367

The alerts

Line 279 is the help-screen guard:

if (argc == 2 && (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))) {

argv[1] is read only because argc == 2 already held. Two alerts rather
than one because the line reads argv[1] twice — they are not duplicates of
each other, they are columns 29 and 55.

Two defects, which have to be fixed together

1. hasArgcGuard() required the guard on a strictly earlier line.
cmp.getStartLine() < access.getStartLine() cannot see a guard that
short-circuits on the same line, which is how every Tools/CmdLine tool
spells its help screen.

Same line is not on its own enough, though. A plain <= also swallows the
case the ordering test exists for — if (!strcmp(argv[1], "-x") && argc > 2),
where argv[1] is read before argc is compared and is out of bounds at
argc == 1. On a shared line the fix therefore compares columns: the
comparison has to start left of the access.

line test	helpGuard (must not fire)	readBeforeGuard (must fire)
< (today)	fires — the two alerts	fires
<=	silent	missed
column-ordering	silent	fires

Still an approximation: a one-line guard inverted relative to its access
(if (argc > 5) { … } else { … argv[3] … }) reads left to right and is
missed. GuardCondition.controls() is the sound replacement for the whole
predicate and is much more than these alerts need.

2. isArgv() required the parameter's unspecified type to be a
PointerType. A parameter written char* argv[] keeps its declared type in
the extractor, so it is an ArrayType and never matched — even though the
predicate's own comment has always said "char** or char*[]".

Fixing (2) alone makes things worse: four of the five iccApply* tools use
the array spelling and the same-line guard, so widening isArgv() without
relaxing the line test takes the tree from 2 false positives to 10. The
arrayHelpGuardSameLine case pins that.

Measured, on a database built from this tree

	before	after
argv[N>0] reads under Tools/ in query scope	13 / 229 (5.7%)	229 / 229
findings	2 (both false)	0

The 0 is a real result, not a silent one: every one of the 229 reads now in
scope carries an argc comparison ordered before it. hasArgcGuard() remains
an ordering approximation rather than control-flow dominance, so that means
"an argc comparison precedes this read", not "this read is provably guarded".

Tests

Adds the red/green test the fix was developed against. It discriminates three
ways — it fails on the unfixed query (the two same-line false positives
present, the array-spelled true positives missing) and on a plain <=
relaxation, which drops readBeforeGuardSameLine. Without that last case the
suite could not tell a correct same-line rule from one that merely stops
looking at the line.

The cases live in Tools/CmdLine/case.h rather than beside the .qlref
because the query filters on a Tools/ path prefix and the qltest cpp
extractor does not descend into subdirectories: a case compiled at the test
root is out of the query's scope and the test passes having asserted nothing.
That is not hypothetical — the fixture did exactly that twice while it was
being written, once printing found nothing to extract and still PASSED.

Nothing was running these tests

ci-codeql-security.yml builds a database and its analyze job is gated
behind the codeql-ready label, so the existing unbounded-profile-loop test
had never run in CI either. ci-codeql-query-tests.yml runs them on any PR
touching .github/codeql-queries/**; it needs no database and no build
(~3 min, dominated by query compilation). It is a separate workflow rather
than a job in ci-codeql-security.yml because that workflow's pull_request
trigger is types:[labeled]; adding the tests there would have meant widening
its triggers for every PR.

Two deliberate differences from the sibling workflows:

• it asserts every test directory has a source file beside its .qlref, so
  the extract-nothing failure cannot pass silently again;
• it always installs the SHA-pinned bundle instead of accepting a codeql
  already on the runner, because .expected asserts extractor line/column
  locations exactly and an unpinned CLI could flip these tests for reasons
  unrelated to the diff.

docs/codeql.md named two bundle pin sites; this makes three, so the doc and
the new workflow's header now name all three.

Also gitignores *.testproj/, the databases codeql test run extracts beside
each test directory. They were never ignored, so running the tests left
untracked directories behind.

Verification

• Red/green confirmed three ways (unfixed / <= / shipped).
• Full suite passes on CodeQL 2.25.6 and on the CI-pinned bundle 2.26.4, so
  the .expected locations are version-stable.
• Pack resolution checked from an emptied package cache, not a warm one.
• preflight-safety-checks.sh: 0 failures. yamllint on the new workflow
  reports the same two warnings ci-codeql-security.yml reports
  (document-start, truthy) and nothing else.

[xsscx]

2026-09-06 11:04:35 UTC