Skip to content

Commit ad1379a

Browse files
NateD-MSFTCopilot
andcommitted
Irql.qll: add loop-body AST fallback to getPotentialExitIrqlAtCfn
The recursive cascade in `getPotentialExitIrqlAtCfn` returns no value for argument-expression CFNs of function calls reached via a loop back-edge in some extracted databases. When the cascade is empty, downstream queries that consume the result either drop the relevant finding (because their `irqlSource = getPotentialExitIrqlAtCfn(...)` binding fails) or fire spuriously (because they treat empty as out-of-range). This is the upstream complement to the IFSM `irqlChangesBetween` AST-loop OR-branch added in 39dd725. Fix: rename the existing predicate body to a private `getPotentialExitIrqlAtCfnRaw` helper and introduce a public wrapper that returns Raw's bindings unchanged, plus -- only when Raw yields no value at all -- a single-step AST-level fallback that returns the cascade's IRQL at the closest source-line preceding sibling Stmt. The fallback is restricted to CFNs whose enclosing Stmt sits inside a loop body, which is the empirical failure mode and avoids over-approximating on linear branching code. Layering preserves existing semantics on every input where Raw binds: the wrapper agrees with Raw setwise there, so no query that previously bound correctly can regress. The only behavioural change is to fill in silence with one conservative value derived from textually-preceding code in the loop body. Internal callers inside Irql.qll that participated in the recursive stratum (getIrqlLevel for the three save-globals classes and functionExitIrql) are switched to call Raw directly so the wrapper's negation does not cycle back through them. Validation: Local IFSM TestDB: the documented "known false negative" `driver_utility_loop_bad` at driver_snippet.c:156 (a for-loop where the restore is textually above the save) now fires as a true positive, recovered exactly as the source comment block at lines 127-145 predicted would require improvements to the IRQL analysis library. SARIF baseline updated to include the new finding. Out-of-tree corpora (clean cache, --ram tuned per DB): for the five queries that call this predicate and have non-zero baseline: * IrqlFunctionNotAnnotated, IrqlInconsistentWithRequired, IrqlTooHigh, IrqlTooLow: identical findings before and after on both small corpora. * KeSetEventIrql: one fewer finding on the larger of the two small corpora (a true false-positive suppression in a worker-thread routine where a release-spinlock immediately precedes a SetEvent inside a `for(;;)` loop body; the cascade was empty pre-fix and the KeSetEvent query treated empty as out-of-range, warning spuriously). The fallback walks back to the release sibling stmt, binds PASSIVE, and the warning is correctly silenced. Sanity-check queries that do not call the predicate (IrqlNotUsed, IrqlCancelRoutine) are unchanged, confirming the rename did not perturb unrelated paths. Performance: no measurable wall-time regression observed on either small corpus. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 39dd725 commit ad1379a

2 files changed

Lines changed: 152 additions & 16 deletions

File tree

src/drivers/general/queries/IrqlFloatStateMismatch/IrqlFloatStateMismatch.sarif

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@
112112
"extensions": [
113113
{
114114
"name": "microsoft/windows-drivers",
115-
"semanticVersion": "1.9.0+4cacaf0d92da0d47dab3a6bd6640729cf07eec99",
115+
"semanticVersion": "1.9.0+39dd725fc01992bf14a42934229a3cc1fd7bf25b",
116116
"locations": [
117117
{
118118
"uri": "file:///F:/source/repos/Windows-Driver-Developer-Supplemental-Tools/src/",
@@ -254,14 +254,14 @@
254254
"text": ""
255255
},
256256
"level": "none",
257-
"timeUtc": "2026-04-29T20:06:00.500637300Z",
257+
"timeUtc": "2026-04-29T20:46:26.236012500Z",
258258
"descriptor": {
259259
"id": "cli/file-coverage-baseline",
260260
"index": 1
261261
},
262262
"properties": {
263263
"attributes": {
264-
"durationMilliseconds": 197
264+
"durationMilliseconds": 233
265265
},
266266
"visibility": {
267267
"statusPage": false,
@@ -274,7 +274,7 @@
274274
"text": ""
275275
},
276276
"level": "none",
277-
"timeUtc": "2026-04-29T20:06:00.504647400Z",
277+
"timeUtc": "2026-04-29T20:46:26.241012400Z",
278278
"descriptor": {
279279
"id": "cli/platform",
280280
"index": 2
@@ -297,7 +297,7 @@
297297
"markdown": "Internal telemetry for the C++ extractor.\n\nNo action needed."
298298
},
299299
"level": "note",
300-
"timeUtc": "2026-04-29T20:06:43.926354300Z",
300+
"timeUtc": "2026-04-29T20:47:05.794926800Z",
301301
"descriptor": {
302302
"id": "cpp/extractor/summary",
303303
"index": 3
@@ -350,6 +350,68 @@
350350
}
351351
],
352352
"results": [
353+
{
354+
"ruleId": "cpp/drivers/irql-float-state-mismatch",
355+
"ruleIndex": 0,
356+
"rule": {
357+
"id": "cpp/drivers/irql-float-state-mismatch",
358+
"index": 0
359+
},
360+
"message": {
361+
"text": "The irql level where the floating-point state was saved (0) does not match the irql level for the restore operation (1).\nThe irql level where the floating-point state was saved (2) does not match the irql level for the restore operation (1)."
362+
},
363+
"locations": [
364+
{
365+
"physicalLocation": {
366+
"artifactLocation": {
367+
"uri": "driver/driver_snippet.c",
368+
"uriBaseId": "%SRCROOT%",
369+
"index": 0
370+
},
371+
"region": {
372+
"startLine": 156,
373+
"startColumn": 38,
374+
"endColumn": 46
375+
}
376+
}
377+
}
378+
],
379+
"partialFingerprints": {
380+
"primaryLocationLineHash": "7203877b7e98c247:1",
381+
"primaryLocationStartColumnFingerprint": "29"
382+
}
383+
},
384+
{
385+
"ruleId": "cpp/drivers/irql-float-state-mismatch",
386+
"ruleIndex": 0,
387+
"rule": {
388+
"id": "cpp/drivers/irql-float-state-mismatch",
389+
"index": 0
390+
},
391+
"message": {
392+
"text": "The irql level where the floating-point state was saved (0) does not match the irql level for the restore operation (1).\nThe irql level where the floating-point state was saved (2) does not match the irql level for the restore operation (1)."
393+
},
394+
"locations": [
395+
{
396+
"physicalLocation": {
397+
"artifactLocation": {
398+
"uri": "driver/driver_snippet.c",
399+
"uriBaseId": "%SRCROOT%",
400+
"index": 0
401+
},
402+
"region": {
403+
"startLine": 156,
404+
"startColumn": 37,
405+
"endColumn": 46
406+
}
407+
}
408+
}
409+
],
410+
"partialFingerprints": {
411+
"primaryLocationLineHash": "7203877b7e98c247:1",
412+
"primaryLocationStartColumnFingerprint": "28"
413+
}
414+
},
353415
{
354416
"ruleId": "cpp/drivers/irql-float-state-mismatch",
355417
"ruleIndex": 0,

src/drivers/libraries/Irql.qll

Lines changed: 85 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -534,13 +534,13 @@
534534
* "the IRQL before the corresponding save global call."
535535
*/
536536
int getIrqlLevel() {
537-
result = any(getPotentialExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor()))
537+
result = any(getPotentialExitIrqlAtCfnRaw(this.getMostRecentRaise().getAPredecessor()))
538538
}
539-
539+
540540
int getIrqlLevelExplicit() {
541541
result = any(getExplicitExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor()))
542542
}
543-
543+
544544
/** Returns the matching call to a function that saved the IRQL. */
545545
IrqlSaveCall getMostRecentRaise() {
546546
result =
@@ -614,7 +614,7 @@
614614
*/
615615
int getIrqlLevel() {
616616
result =
617-
any(getPotentialExitIrqlAtCfn(this.getMostRecentRaiseInterprocedural().getAPredecessor()))
617+
any(getPotentialExitIrqlAtCfnRaw(this.getMostRecentRaiseInterprocedural().getAPredecessor()))
618618
}
619619

620620
int getIrqlLevelExplicit() {
@@ -665,13 +665,13 @@
665665
* "the IRQL before the corresponding save global call."
666666
*/
667667
int getIrqlLevel() {
668-
result = any(getPotentialExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor()))
668+
result = any(getPotentialExitIrqlAtCfnRaw(this.getMostRecentRaise().getAPredecessor()))
669669
}
670-
670+
671671
int getIrqlLevelExplicit() {
672672
result = any(getExplicitExitIrqlAtCfn(this.getMostRecentRaise().getAPredecessor()))
673673
}
674-
674+
675675
/**
676676
* Returns the matching call to a function that saved the IRQL to a global state.
677677
*
@@ -735,10 +735,17 @@
735735
/**
736736
* Pre-computed summary: the potential exit IRQL of function `f`,
737737
* computed once per function rather than re-discovered per call site.
738+
*
739+
* Calls the Raw cascade directly rather than the public wrapper so
740+
* that this internal building block stays inside a single recursive
741+
* stratum with the cascade. The wrapper layers an AST-level fallback
742+
* over Raw using a negation, which would otherwise cycle back through
743+
* here. Equivalent to the historical pre-wrapper behaviour because
744+
* the wrapper agrees with Raw on every input where Raw binds.
738745
*/
739746
pragma[nomagic]
740747
private int functionExitIrql(Function f) {
741-
result = getPotentialExitIrqlAtCfn(getAnExitPointOfFunction(f))
748+
result = getPotentialExitIrqlAtCfnRaw(getAnExitPointOfFunction(f))
742749
}
743750

744751
/**
@@ -765,9 +772,38 @@
765772
* - If there are no prior CFNs and no calls to this function, then the IRQL is determined by annotations applied to this function.
766773
* - Failing all this, we set the IRQL to 0.
767774
*
775+
* Layering note: this public predicate wraps the recursive cascade
776+
* `getPotentialExitIrqlAtCfnRaw` with a single AST-level fallback
777+
* (`astLevelExitIrqlFallback`) that activates only when the cascade
778+
* yields no value at all. The fallback walks one source-line step
779+
* back to the closest preceding sibling Stmt and returns the cascade's
780+
* IRQL there. This recovers binding for argument-expression CFNs of
781+
* function calls inside loop bodies, where the cpp CFG in some
782+
* extracted databases is too sparse for the cascade's predecessor walk
783+
* to converge. The fallback is restricted to CFNs inside loop bodies
784+
* (the empirical failure mode), keeping it from over-approximating on
785+
* linear branching code where the cascade's silence may be hiding
786+
* IRQL-changing work that an AST-level scan cannot see. When the
787+
* cascade already binds, this wrapper returns exactly the cascade's
788+
* result set: the fallback never widens an existing binding.
789+
*
768790
* Not implemented: _IRQL_limited_to_
769791
*/
770792
int getPotentialExitIrqlAtCfn(ControlFlowNode cfn) {
793+
result = getPotentialExitIrqlAtCfnRaw(cfn)
794+
or
795+
not exists(getPotentialExitIrqlAtCfnRaw(cfn)) and
796+
result = astLevelExitIrqlFallback(cfn)
797+
}
798+
799+
/**
800+
* Internal recursive cascade for `getPotentialExitIrqlAtCfn`. Behaves
801+
* exactly like the historical `getPotentialExitIrqlAtCfn` did before
802+
* the AST fallback wrapper was introduced. Callers should use
803+
* `getPotentialExitIrqlAtCfn` instead, which additionally consults
804+
* `astLevelExitIrqlFallback` when this cascade yields no value.
805+
*/
806+
private int getPotentialExitIrqlAtCfnRaw(ControlFlowNode cfn) {
771807
if cfn instanceof KeRaiseIrqlCall
772808
then result = cfn.(KeRaiseIrqlCall).getIrqlLevel()
773809
else
@@ -788,18 +824,18 @@
788824
if
789825
cfn instanceof FunctionCall and
790826
cfn.(FunctionCall).getTarget() instanceof IrqlRequiresSameAnnotatedFunction
791-
then result = getPotentialExitIrqlAtCfn(cfn.getAPredecessor())
827+
then result = getPotentialExitIrqlAtCfnRaw(cfn.getAPredecessor())
792828
else
793829
if cfn instanceof FunctionCall
794830
then result = functionExitIrql(cfn.(FunctionCall).getTarget())
795831
else
796832
if exists(ControlFlowNode cfn2 | cfn2 = cfn.getAPredecessor())
797-
then result = getPotentialExitIrqlAtCfn(cfn.getAPredecessor())
833+
then result = getPotentialExitIrqlAtCfnRaw(cfn.getAPredecessor())
798834
else
799835
if exists(callerPredecessor(cfn.getControlFlowScope()))
800836
then
801837
result =
802-
getPotentialExitIrqlAtCfn(callerPredecessor(cfn.getControlFlowScope()))
838+
getPotentialExitIrqlAtCfnRaw(callerPredecessor(cfn.getControlFlowScope()))
803839
else
804840
if
805841
cfn.getControlFlowScope() instanceof IrqlRestrictsFunction and
@@ -808,6 +844,44 @@
808844
else result = 0
809845
}
810846

847+
/**
848+
* AST-level fallback for `getPotentialExitIrqlAtCfn`. Walks to the
849+
* closest source-line preceding sibling Stmt of `cfn`'s enclosing Stmt
850+
* (within the same parent Stmt) and returns the cascade's IRQL there.
851+
* Yields no value when there is no preceding sibling at the same
852+
* nesting level (e.g. `cfn` is the first stmt in its block) or when
853+
* the cascade also yields no value at that sibling.
854+
*
855+
* Restricted to CFNs whose enclosing Stmt sits inside a loop body. The
856+
* cascade's CFG-predecessor walk is reliable on linear control flow but
857+
* empirically can fail to bind on argument-expression CFNs of function
858+
* calls reached via a loop back-edge; this fallback exists to recover
859+
* binding for that specific case. Restricting to loops avoids
860+
* supplying coarse single-value approximations on linear code where the
861+
* cascade's silence (when it does occur) often reflects branching that
862+
* the AST-level scan cannot reason about (e.g. an if-stmt whose body
863+
* raises and lowers IRQL).
864+
*
865+
* Consulted by `getPotentialExitIrqlAtCfn` only when the cascade
866+
* returns no value at all, so this never widens an existing binding;
867+
* it only fills in silence with a single conservative IRQL value
868+
* derived from textually-preceding code in the loop body.
869+
*/
870+
private int astLevelExitIrqlFallback(ControlFlowNode cfn) {
871+
exists(Stmt cfnStmt, Stmt prev, Loop l |
872+
cfnStmt = cfn.getEnclosingStmt() and
873+
cfnStmt.getParent+() = l and
874+
prev.getParentStmt() = cfnStmt.getParentStmt() and
875+
prev.getLocation().getStartLine() < cfnStmt.getLocation().getStartLine() and
876+
not exists(Stmt closer |
877+
closer.getParentStmt() = cfnStmt.getParentStmt() and
878+
closer.getLocation().getStartLine() < cfnStmt.getLocation().getStartLine() and
879+
closer.getLocation().getStartLine() > prev.getLocation().getStartLine()
880+
) and
881+
result = getPotentialExitIrqlAtCfnRaw(prev)
882+
)
883+
}
884+
811885
/*
812886
* Similar to above, but only exit points where the Irql is explicit
813887
*/

0 commit comments

Comments
 (0)