From 6445fd805d366f045a29e05262ff72e283e5d5aa Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 28 Jan 2026 13:57:07 +0000 Subject: [PATCH] C++: Fix IR -> AST mapping for basic blocks. --- .../semmle/code/cpp/controlflow/IRGuards.qll | 13 ++++++++-- .../controlflow/guards-ir/tests.expected | 12 --------- .../controlflow/guards/GuardsControl.expected | 14 ---------- .../controlflow/guards/GuardsEnsure.expected | 26 ------------------- 4 files changed, 11 insertions(+), 54 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll index d5287494df9..824e5886c62 100644 --- a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll +++ b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll @@ -8,7 +8,8 @@ import semmle.code.cpp.ir.IR private import codeql.util.Void private import codeql.controlflow.Guards as SharedGuards private import semmle.code.cpp.ir.ValueNumbering -private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedExpr +private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedExpr as TE +private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedFunction as TF private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag private class BasicBlock = IRCfg::BasicBlock; @@ -1026,7 +1027,7 @@ private class GuardConditionFromIR extends GuardConditionImpl { private predicate excludeAsControlledInstruction(Instruction instr) { // Exclude the temporaries generated by a ternary expression. - exists(TranslatedConditionalExpr tce | + exists(TE::TranslatedConditionalExpr tce | instr = tce.getInstruction(ConditionValueFalseStoreTag()) or instr = tce.getInstruction(ConditionValueTrueStoreTag()) @@ -1038,6 +1039,14 @@ private predicate excludeAsControlledInstruction(Instruction instr) { or // Exclude unreached instructions, as their AST is the whole function and not a block. instr instanceof UnreachedInstruction + or + // Exclude instructions generated by a translated function as they map to the function itself + // and the function is considered the last basic block of a function body. + any(TF::TranslatedFunction tf).getInstruction(_) = instr + or + // `ChiInstruction`s generated by instructions in the above case don't come from `getInstruction` (since they are generated by AliasedSSA) + // so we need to special case them. + excludeAsControlledInstruction(instr.(ChiInstruction).getPartial()) } /** diff --git a/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected b/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected index 2e18dcd7a62..747564a745e 100644 --- a/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected +++ b/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected @@ -405,9 +405,7 @@ astGuardsControl | test.c:181:9:181:9 | x | true | 181 | 182 | | test.c:181:9:181:9 | x | true | 186 | 180 | | test.cpp:18:8:18:10 | call to get | true | 19 | 19 | -| test.cpp:31:7:31:13 | ... == ... | false | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | false | 34 | 34 | -| test.cpp:31:7:31:13 | ... == ... | true | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | true | 31 | 32 | | test.cpp:42:13:42:20 | call to getABool | true | 43 | 45 | astGuardsEnsure @@ -589,13 +587,9 @@ astGuardsEnsure | test.c:175:13:175:32 | ... == ... | test.c:175:13:175:15 | call to foo | == | test.c:175:32:175:32 | 0 | 0 | 175 | 175 | | test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | != | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 | | test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | == | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 34 | 34 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | test.cpp:31:12:31:13 | - ... | 0 | 31 | 32 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | != | test.cpp:31:7:31:7 | x | 0 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | != | test.cpp:31:7:31:7 | x | 0 | 34 | 34 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | == | test.cpp:31:7:31:7 | x | 0 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | == | test.cpp:31:7:31:7 | x | 0 | 31 | 32 | astGuardsEnsure_const | test.c:7:9:7:13 | ... > ... | test.c:7:9:7:13 | ... > ... | != | 0 | 7 | 9 | @@ -846,17 +840,11 @@ astGuardsEnsure_const | test.c:181:9:181:9 | x | test.c:181:9:181:9 | x | == | 1 | 186 | 180 | | test.cpp:18:8:18:10 | call to get | test.cpp:18:8:18:10 | call to get | != | 0 | 19 | 19 | | test.cpp:18:8:18:10 | call to get | test.cpp:18:8:18:10 | call to get | == | 1 | 19 | 19 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | 34 | 34 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | -1 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | -1 | 31 | 32 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 0 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 0 | 31 | 32 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 1 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 1 | 34 | 34 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 0 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 0 | 34 | 34 | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 1 | 30 | 30 | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 1 | 31 | 32 | | test.cpp:42:13:42:20 | call to getABool | test.cpp:42:13:42:20 | call to getABool | != | 0 | 43 | 45 | | test.cpp:42:13:42:20 | call to getABool | test.cpp:42:13:42:20 | call to getABool | == | 1 | 43 | 45 | diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected index 80a6530291d..040173a2a4b 100644 --- a/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected +++ b/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected @@ -162,17 +162,11 @@ | test.c:219:9:219:22 | call to __builtin_expect | true | test.c:219:25:221:5 | { ... } | | test.cpp:18:8:18:10 | call to get | not null | test.cpp:19:5:19:14 | ExprStmt | | test.cpp:18:8:18:10 | call to get | true | test.cpp:19:5:19:14 | ExprStmt | -| test.cpp:30:22:30:22 | x | -1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:30:22:30:22 | x | -1 | test.cpp:31:16:32:21 | { ... } | -| test.cpp:30:22:30:22 | x | not -1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:30:22:30:22 | x | not -1 | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:7 | x | -1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:7 | x | -1 | test.cpp:31:16:32:21 | { ... } | -| test.cpp:31:7:31:7 | x | not -1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:7 | x | not -1 | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:13 | ... == ... | false | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | false | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:13 | ... == ... | true | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | true | test.cpp:31:16:32:21 | { ... } | | test.cpp:42:13:42:20 | call to getABool | true | test.cpp:43:9:45:23 | { ... } | | test.cpp:60:31:60:31 | i | 0 | test.cpp:62:5:64:12 | case ...: | @@ -408,19 +402,11 @@ | test.cpp:400:11:400:25 | call to testEnumWrapper | 2 | test.cpp:404:5:406:12 | case ...: | | test.cpp:400:27:400:27 | b | false | test.cpp:404:5:406:12 | case ...: | | test.cpp:400:27:400:27 | b | true | test.cpp:401:5:403:12 | case ...: | -| test.cpp:410:26:410:26 | o | not null | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:410:26:410:26 | o | not null | test.cpp:412:1:412:1 | return ... | -| test.cpp:410:26:410:26 | o | null | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:410:26:410:26 | o | null | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:7:411:8 | ! ... | false | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | false | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:7:411:8 | ! ... | true | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | true | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:8:411:8 | o | false | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | false | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:8:411:8 | o | not null | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | not null | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:8:411:8 | o | null | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | null | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:8:411:8 | o | true | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | true | test.cpp:412:1:412:1 | return ... | diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected index 0de63fa505b..a1d3a37bea2 100644 --- a/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected +++ b/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected @@ -195,13 +195,9 @@ binary | test.c:215:6:215:18 | call to __builtin_expect | test.c:215:17:215:17 | b | < | test.c:215:13:215:13 | a | 0 | test.c:215:21:217:5 | { ... } | | test.c:219:9:219:22 | call to __builtin_expect | test.c:219:16:219:16 | a | >= | test.c:219:20:219:21 | 42 | 1 | test.c:219:25:221:5 | { ... } | | test.c:219:9:219:22 | call to __builtin_expect | test.c:219:20:219:21 | 42 | < | test.c:219:16:219:16 | a | 0 | test.c:219:25:221:5 | { ... } | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | test.cpp:31:12:31:13 | - ... | 0 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | test.cpp:31:12:31:13 | - ... | 0 | test.cpp:31:16:32:21 | { ... } | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | != | test.cpp:31:7:31:7 | x | 0 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | != | test.cpp:31:7:31:7 | x | 0 | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | == | test.cpp:31:7:31:7 | x | 0 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:12:31:13 | - ... | == | test.cpp:31:7:31:7 | x | 0 | test.cpp:31:16:32:21 | { ... } | | test.cpp:105:6:105:14 | ... != ... | test.cpp:105:6:105:6 | f | != | test.cpp:105:11:105:14 | 0.0 | 0 | test.cpp:105:17:106:7 | { ... } | | test.cpp:105:6:105:14 | ... != ... | test.cpp:105:11:105:14 | 0.0 | != | test.cpp:105:6:105:6 | f | 0 | test.cpp:105:17:106:7 | { ... } | @@ -853,17 +849,11 @@ unary | test.c:219:9:219:22 | call to __builtin_expect | test.c:219:16:219:21 | ... > ... | != | 0 | test.c:219:25:221:5 | { ... } | | test.cpp:18:8:18:10 | call to get | test.cpp:18:8:18:10 | call to get | != | 0 | test.cpp:19:5:19:14 | ExprStmt | | test.cpp:18:8:18:10 | call to get | test.cpp:18:8:18:10 | call to get | == | 1 | test.cpp:19:5:19:14 | ExprStmt | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | -1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | -1 | test.cpp:31:16:32:21 | { ... } | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 0 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 0 | test.cpp:31:16:32:21 | { ... } | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | != | 1 | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 0 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 0 | test.cpp:34:1:34:1 | return ... | -| test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 1 | test.cpp:30:6:30:16 | doSomething | | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:13 | ... == ... | == | 1 | test.cpp:31:16:32:21 | { ... } | | test.cpp:42:13:42:20 | call to getABool | test.cpp:42:13:42:20 | call to getABool | != | 0 | test.cpp:43:9:45:23 | { ... } | | test.cpp:42:13:42:20 | call to getABool | test.cpp:42:13:42:20 | call to getABool | == | 1 | test.cpp:43:9:45:23 | { ... } | @@ -1401,35 +1391,19 @@ unary | test.cpp:394:7:394:47 | ... == ... | test.cpp:394:15:394:34 | call to returnAIfNoneAreNull | == | 0 | test.cpp:394:50:395:7 | { ... } | | test.cpp:400:11:400:25 | call to testEnumWrapper | test.cpp:400:11:400:25 | call to testEnumWrapper | == | 1 | test.cpp:401:5:403:12 | case ...: | | test.cpp:400:11:400:25 | call to testEnumWrapper | test.cpp:400:11:400:25 | call to testEnumWrapper | == | 2 | test.cpp:404:5:406:12 | case ...: | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | != | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | != | 0 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | != | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | != | 1 | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | == | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | == | 0 | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | == | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:7:411:8 | ! ... | == | 1 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | != | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | != | 0 | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | != | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | != | 1 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | == | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | == | 0 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | == | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:7:411:8 | ! ... | test.cpp:411:8:411:8 | o | == | 1 | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | != | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | != | 0 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | != | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | != | 1 | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | == | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | == | 0 | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | == | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:7:411:8 | ! ... | == | 1 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | != | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | != | 0 | test.cpp:412:1:412:1 | return ... | -| test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | != | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | != | 1 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | == | 0 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | == | 0 | test.cpp:411:11:411:18 | ExprStmt | -| test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | == | 1 | test.cpp:410:6:410:18 | ensureNotNull | | test.cpp:411:8:411:8 | o | test.cpp:411:8:411:8 | o | == | 1 | test.cpp:412:1:412:1 | return ... |