Skip to content

Commit d1c6022

Browse files
rdmarsh2rdmarsh2
committed
C++: fix inconsistencies from IR global vars
1 parent 5b80fd1 commit d1c6022

File tree

6 files changed

+69
-29
lines changed

6 files changed

+69
-29
lines changed

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class Node extends TIRDataFlowNode {
100100
Declaration getEnclosingCallable() { none() } // overridden in subclasses
101101

102102
/** Gets the function to which this node belongs, if any. */
103-
Function getFunction() { none() } // overridden in subclasses
103+
Declaration getFunction() { none() } // overridden in subclasses
104104

105105
/** Gets the type of this node. */
106106
IRType getType() { none() } // overridden in subclasses
@@ -196,7 +196,7 @@ class InstructionNode extends Node, TInstructionNode {
196196

197197
override Declaration getEnclosingCallable() { result = this.getFunction() }
198198

199-
override Function getFunction() { result = instr.getEnclosingFunction() }
199+
override Declaration getFunction() { result = instr.getEnclosingFunction() }
200200

201201
override IRType getType() { result = instr.getResultIRType() }
202202

@@ -222,7 +222,7 @@ class OperandNode extends Node, TOperandNode {
222222

223223
override Declaration getEnclosingCallable() { result = this.getFunction() }
224224

225-
override Function getFunction() { result = op.getUse().getEnclosingFunction() }
225+
override Declaration getFunction() { result = op.getUse().getEnclosingFunction() }
226226

227227
override IRType getType() { result = op.getIRType() }
228228

@@ -274,7 +274,7 @@ class StoreNodeInstr extends StoreNode, TStoreNodeInstr {
274274
/** Gets the underlying instruction. */
275275
Instruction getInstruction() { result = instr }
276276

277-
override Function getFunction() { result = this.getInstruction().getEnclosingFunction() }
277+
override Declaration getFunction() { result = this.getInstruction().getEnclosingFunction() }
278278

279279
override IRType getType() { result = this.getInstruction().getResultIRType() }
280280

@@ -328,7 +328,7 @@ class StoreNodeOperand extends StoreNode, TStoreNodeOperand {
328328
/** Gets the underlying operand. */
329329
Operand getOperand() { result = operand }
330330

331-
override Function getFunction() { result = operand.getDef().getEnclosingFunction() }
331+
override Declaration getFunction() { result = operand.getDef().getEnclosingFunction() }
332332

333333
override IRType getType() { result = operand.getIRType() }
334334

@@ -384,7 +384,7 @@ class ReadNode extends Node, TReadNode {
384384

385385
override Declaration getEnclosingCallable() { result = this.getFunction() }
386386

387-
override Function getFunction() { result = this.getInstruction().getEnclosingFunction() }
387+
override Declaration getFunction() { result = this.getInstruction().getEnclosingFunction() }
388388

389389
override IRType getType() { result = this.getInstruction().getResultIRType() }
390390

@@ -436,7 +436,7 @@ class SsaPhiNode extends Node, TSsaPhiNode {
436436

437437
override Declaration getEnclosingCallable() { result = this.getFunction() }
438438

439-
override Function getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
439+
override Declaration getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
440440

441441
override IRType getType() { result instanceof IRVoidType }
442442

@@ -673,7 +673,7 @@ class VariableNode extends Node, TVariableNode {
673673
/** Gets the variable corresponding to this node. */
674674
Variable getVariable() { result = v }
675675

676-
override Function getFunction() { none() }
676+
override Declaration getFunction() { none() }
677677

678678
override Declaration getEnclosingCallable() {
679679
// When flow crosses from one _enclosing callable_ to another, the

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/IRConsistency.qll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,4 +524,23 @@ module InstructionConsistency {
524524
"' has a `this` argument operand that is not an address, in function '$@'." and
525525
irFunc = getInstructionIRFunction(instr, irFuncText)
526526
}
527+
528+
query predicate nonUniqueIRVariable(
529+
Instruction instr, string message, OptionalIRFunction irFunc, string irFuncText
530+
) {
531+
exists(VariableInstruction vi, IRVariable v1, IRVariable v2 |
532+
instr = vi and vi.getIRVariable() = v1 and vi.getIRVariable() = v2 and v1 != v2
533+
) and
534+
message =
535+
"Variable instruction '" + instr.toString() +
536+
"' has multiple associated variables, in function '$@'." and
537+
irFunc = getInstructionIRFunction(instr, irFuncText)
538+
or
539+
instr.getOpcode() instanceof Opcode::VariableAddress and
540+
not instr instanceof VariableInstruction and
541+
message =
542+
"Variable address instruction '" + instr.toString() +
543+
"' has no associated variable, in function '$@'." and
544+
irFunc = getInstructionIRFunction(instr, irFuncText)
545+
}
527546
}

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ private import TranslatedElement
1313
private import TranslatedExpr
1414
private import TranslatedStmt
1515
private import TranslatedFunction
16+
private import TranslatedGlobalVar
1617

1718
TranslatedElement getInstructionTranslatedElement(Instruction instruction) {
1819
instruction = TRawInstruction(result, _)
@@ -44,8 +45,10 @@ module Raw {
4445
}
4546

4647
cached
47-
predicate hasUserVariable(Function func, Variable var, CppType type) {
48-
getTranslatedFunction(func).hasUserVariable(var, type)
48+
predicate hasUserVariable(Declaration decl, Variable var, CppType type) {
49+
getTranslatedFunction(decl).hasUserVariable(var, type)
50+
or
51+
getTranslatedVarInit(decl).hasUserVariable(var, type)
4952
}
5053

5154
cached

‎cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedGlobalVar.qll

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ private import semmle.code.cpp.ir.implementation.internal.OperandTag
66
private import semmle.code.cpp.ir.internal.CppType
77
private import TranslatedInitialization
88
private import InstructionTag
9+
private import semmle.code.cpp.ir.internal.IRUtilities
910

1011
class TranslatedGlobalOrNamespaceVarInit extends TranslatedRootElement,
1112
TTranslatedGlobalOrNamespaceVarInit, InitializationContext {
@@ -98,6 +99,33 @@ class TranslatedGlobalOrNamespaceVarInit extends TranslatedRootElement,
9899
}
99100

100101
override Type getTargetType() { result = var.getUnspecifiedType() }
102+
103+
/**
104+
* Holds if this variable defines or accesses variable `var` with type `type`. This includes all
105+
* parameters and local variables, plus any global variables or static data members that are
106+
* directly accessed by the function.
107+
*/
108+
final predicate hasUserVariable(Variable varUsed, CppType type) {
109+
(
110+
(
111+
varUsed instanceof GlobalOrNamespaceVariable
112+
or
113+
varUsed instanceof MemberVariable and not varUsed instanceof Field
114+
) and
115+
exists(VariableAccess access |
116+
access.getTarget() = varUsed and
117+
access.getEnclosingVariable() = var
118+
)
119+
or
120+
var = varUsed
121+
or
122+
varUsed.(LocalScopeVariable).getEnclosingElement*() = var
123+
or
124+
varUsed.(Parameter).getCatchBlock().getEnclosingElement*() = var
125+
) and
126+
type = getTypeForPRValue(getVariableType(varUsed))
127+
}
128+
101129
}
102130

103131
TranslatedGlobalOrNamespaceVarInit getTranslatedVarInit(GlobalOrNamespaceVariable var) {

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,4 @@
11
uniqueEnclosingCallable
2-
| globals.cpp:9:5:9:19 | Address | Node should have one enclosing callable but has 0. |
3-
| globals.cpp:9:5:9:19 | AliasedDefinition | Node should have one enclosing callable but has 0. |
4-
| globals.cpp:9:5:9:19 | VariableAddress | Node should have one enclosing callable but has 0. |
5-
| globals.cpp:9:5:9:19 | VariableAddress [post update] | Node should have one enclosing callable but has 0. |
6-
| globals.cpp:9:23:9:23 | 0 | Node should have one enclosing callable but has 0. |
7-
| globals.cpp:9:23:9:23 | ChiPartial | Node should have one enclosing callable but has 0. |
8-
| globals.cpp:9:23:9:23 | ChiTotal | Node should have one enclosing callable but has 0. |
9-
| globals.cpp:9:23:9:23 | Store | Node should have one enclosing callable but has 0. |
10-
| globals.cpp:9:23:9:23 | StoreValue | Node should have one enclosing callable but has 0. |
11-
| globals.cpp:16:12:16:26 | Address | Node should have one enclosing callable but has 0. |
12-
| globals.cpp:16:12:16:26 | AliasedDefinition | Node should have one enclosing callable but has 0. |
13-
| globals.cpp:16:12:16:26 | VariableAddress | Node should have one enclosing callable but has 0. |
14-
| globals.cpp:16:12:16:26 | VariableAddress [post update] | Node should have one enclosing callable but has 0. |
15-
| globals.cpp:16:30:16:30 | 0 | Node should have one enclosing callable but has 0. |
16-
| globals.cpp:16:30:16:30 | ChiPartial | Node should have one enclosing callable but has 0. |
17-
| globals.cpp:16:30:16:30 | ChiTotal | Node should have one enclosing callable but has 0. |
18-
| globals.cpp:16:30:16:30 | Store | Node should have one enclosing callable but has 0. |
19-
| globals.cpp:16:30:16:30 | StoreValue | Node should have one enclosing callable but has 0. |
202
uniqueType
213
uniqueNodeLocation
224
| BarrierGuard.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 6. |
@@ -238,10 +220,10 @@ postWithInFlow
238220
| lambdas.cpp:20:11:20:11 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |
239221
| lambdas.cpp:20:11:20:11 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |
240222
| lambdas.cpp:20:11:20:11 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |
223+
| lambdas.cpp:23:3:23:3 | (reference dereference) [post update] | PostUpdateNode should not be the target of local flow. |
241224
| lambdas.cpp:23:3:23:14 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |
242225
| lambdas.cpp:23:3:23:14 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
243226
| lambdas.cpp:23:3:23:14 | v [post update] | PostUpdateNode should not be the target of local flow. |
244-
| lambdas.cpp:23:15:23:15 | (reference dereference) [post update] | PostUpdateNode should not be the target of local flow. |
245227
| lambdas.cpp:28:7:28:7 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
246228
| lambdas.cpp:28:10:31:2 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |
247229
| lambdas.cpp:28:10:31:2 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |

‎cpp/ql/test/library-tests/ir/ir/raw_consistency.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ invalidOverlap
2727
nonUniqueEnclosingIRFunction
2828
fieldAddressOnNonPointer
2929
thisArgumentIsNonPointer
30+
nonUniqueIRVariable
31+
| ir.cpp:1038:6:1038:8 | VariableAddress: lam | Variable address instruction 'VariableAddress: lam' has no associated variable, in function '$@'. | ir.cpp:1038:6:1038:8 | (lambda [] type at line 1038, col. 12) lam | (lambda [] type at line 1038, col. 12) lam |
32+
| ir.cpp:1759:5:1759:12 | VariableAddress: global_2 | Variable address instruction 'VariableAddress: global_2' has no associated variable, in function '$@'. | ir.cpp:1759:5:1759:12 | int global_2 | int global_2 |
33+
| ir.cpp:1761:11:1761:18 | VariableAddress: global_3 | Variable address instruction 'VariableAddress: global_3' has no associated variable, in function '$@'. | ir.cpp:1761:11:1761:18 | int const global_3 | int const global_3 |
34+
| ir.cpp:1763:18:1763:25 | VariableAddress: global_4 | Variable address instruction 'VariableAddress: global_4' has no associated variable, in function '$@'. | ir.cpp:1763:18:1763:25 | constructor_only global_4 | constructor_only global_4 |
35+
| ir.cpp:1765:18:1765:25 | VariableAddress: global_5 | Variable address instruction 'VariableAddress: global_5' has no associated variable, in function '$@'. | ir.cpp:1765:18:1765:25 | constructor_only global_5 | constructor_only global_5 |
36+
| ir.cpp:1767:7:1767:19 | VariableAddress: global_string | Variable address instruction 'VariableAddress: global_string' has no associated variable, in function '$@'. | ir.cpp:1767:7:1767:19 | char* global_string | char* global_string |
37+
| struct_init.cpp:9:13:9:25 | VariableAddress: infos_in_file | Variable address instruction 'VariableAddress: infos_in_file' has no associated variable, in function '$@'. | struct_init.cpp:9:13:9:25 | Info infos_in_file[] | Info infos_in_file[] |
3038
missingCanonicalLanguageType
3139
multipleCanonicalLanguageTypes
3240
missingIRType

0 commit comments

Comments
 (0)