Skip to content

Commit ab64d9a

Browse files
MathiasVPMathiasVP
authored
Merge pull request #14713 from MathiasVP/no-gvn-as-ssa-in-range-analysis
C++: Don't use GVN as SSAVariable in new range analysis
2 parents 5442cdb + 2d43eec commit ab64d9a

File tree

3 files changed

+5
-48
lines changed

3 files changed

+5
-48
lines changed

‎cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticExprSpecific.qll

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -130,17 +130,7 @@ module SemanticExprConfig {
130130

131131
newtype TSsaVariable =
132132
TSsaInstruction(IR::Instruction instr) { instr.hasMemoryResult() } or
133-
TSsaOperand(IR::Operand op) { op.isDefinitionInexact() } or
134-
TSsaPointerArithmeticGuard(ValueNumber instr) {
135-
exists(Guard g, IR::Operand use |
136-
use = instr.getAUse() and use.getIRType() instanceof IR::IRAddressType
137-
|
138-
g.comparesLt(use, _, _, _, _) or
139-
g.comparesLt(_, use, _, _, _) or
140-
g.comparesEq(use, _, _, _, _) or
141-
g.comparesEq(_, use, _, _, _)
142-
)
143-
}
133+
TSsaOperand(IR::Operand op) { op.isDefinitionInexact() }
144134

145135
class SsaVariable extends TSsaVariable {
146136
string toString() { none() }
@@ -149,8 +139,6 @@ module SemanticExprConfig {
149139

150140
IR::Instruction asInstruction() { none() }
151141

152-
ValueNumber asPointerArithGuard() { none() }
153-
154142
IR::Operand asOperand() { none() }
155143
}
156144

@@ -166,18 +154,6 @@ module SemanticExprConfig {
166154
final override IR::Instruction asInstruction() { result = instr }
167155
}
168156

169-
class SsaPointerArithmeticGuard extends SsaVariable, TSsaPointerArithmeticGuard {
170-
ValueNumber vn;
171-
172-
SsaPointerArithmeticGuard() { this = TSsaPointerArithmeticGuard(vn) }
173-
174-
final override string toString() { result = vn.toString() }
175-
176-
final override Location getLocation() { result = vn.getLocation() }
177-
178-
final override ValueNumber asPointerArithGuard() { result = vn }
179-
}
180-
181157
class SsaOperand extends SsaVariable, TSsaOperand {
182158
IR::Operand op;
183159

@@ -210,11 +186,7 @@ module SemanticExprConfig {
210186
)
211187
}
212188

213-
Expr getAUse(SsaVariable v) {
214-
result.(IR::LoadInstruction).getSourceValue() = v.asInstruction()
215-
or
216-
result = v.asPointerArithGuard().getAnInstruction()
217-
}
189+
Expr getAUse(SsaVariable v) { result.(IR::LoadInstruction).getSourceValue() = v.asInstruction() }
218190

219191
SemType getSsaVariableType(SsaVariable v) {
220192
result = getSemanticType(v.asInstruction().getResultIRType())
@@ -253,10 +225,7 @@ module SemanticExprConfig {
253225
final override Location getLocation() { result = block.getLocation() }
254226

255227
final override predicate hasRead(SsaVariable v) {
256-
exists(IR::Operand operand |
257-
operand.getDef() = v.asInstruction() or
258-
operand.getDef() = v.asPointerArithGuard().getAnInstruction()
259-
|
228+
exists(IR::Operand operand | operand.getDef() = v.asInstruction() |
260229
not operand instanceof IR::PhiInputOperand and
261230
operand.getUse().getBlock() = block
262231
)
@@ -274,10 +243,7 @@ module SemanticExprConfig {
274243
final override Location getLocation() { result = succ.getLocation() }
275244

276245
final override predicate hasRead(SsaVariable v) {
277-
exists(IR::PhiInputOperand operand |
278-
operand.getDef() = v.asInstruction() or
279-
operand.getDef() = v.asPointerArithGuard().getAnInstruction()
280-
|
246+
exists(IR::PhiInputOperand operand | operand.getDef() = v.asInstruction() |
281247
operand.getPredecessorBlock() = pred and
282248
operand.getUse().getBlock() = succ
283249
)

‎cpp/ql/test/query-tests/Security/CWE/CWE-193/InvalidPointerDeref.expected

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ edges
2222
| test.cpp:52:19:52:37 | call to malloc | test.cpp:53:12:53:23 | ... + ... |
2323
| test.cpp:53:12:53:23 | ... + ... | test.cpp:51:33:51:35 | end |
2424
| test.cpp:60:34:60:37 | mk_array output argument | test.cpp:67:9:67:14 | ... = ... |
25-
| test.cpp:194:15:194:33 | call to malloc | test.cpp:195:17:195:23 | ... + ... |
26-
| test.cpp:195:17:195:23 | ... + ... | test.cpp:195:17:195:23 | ... + ... |
27-
| test.cpp:195:17:195:23 | ... + ... | test.cpp:201:5:201:19 | ... = ... |
28-
| test.cpp:195:17:195:23 | ... + ... | test.cpp:201:5:201:19 | ... = ... |
2925
| test.cpp:205:15:205:33 | call to malloc | test.cpp:206:17:206:23 | ... + ... |
3026
| test.cpp:206:17:206:23 | ... + ... | test.cpp:206:17:206:23 | ... + ... |
3127
| test.cpp:206:17:206:23 | ... + ... | test.cpp:213:5:213:13 | ... = ... |
@@ -125,10 +121,6 @@ nodes
125121
| test.cpp:53:12:53:23 | ... + ... | semmle.label | ... + ... |
126122
| test.cpp:60:34:60:37 | mk_array output argument | semmle.label | mk_array output argument |
127123
| test.cpp:67:9:67:14 | ... = ... | semmle.label | ... = ... |
128-
| test.cpp:194:15:194:33 | call to malloc | semmle.label | call to malloc |
129-
| test.cpp:195:17:195:23 | ... + ... | semmle.label | ... + ... |
130-
| test.cpp:195:17:195:23 | ... + ... | semmle.label | ... + ... |
131-
| test.cpp:201:5:201:19 | ... = ... | semmle.label | ... = ... |
132124
| test.cpp:205:15:205:33 | call to malloc | semmle.label | call to malloc |
133125
| test.cpp:206:17:206:23 | ... + ... | semmle.label | ... + ... |
134126
| test.cpp:206:17:206:23 | ... + ... | semmle.label | ... + ... |
@@ -214,7 +206,6 @@ subpaths
214206
| test.cpp:30:14:30:15 | * ... | test.cpp:28:15:28:37 | call to malloc | test.cpp:30:14:30:15 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:28:15:28:37 | call to malloc | call to malloc | test.cpp:29:20:29:27 | ... + ... | ... + ... |
215207
| test.cpp:32:14:32:21 | * ... | test.cpp:28:15:28:37 | call to malloc | test.cpp:32:14:32:21 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:28:15:28:37 | call to malloc | call to malloc | test.cpp:29:20:29:27 | ... + ... | ... + ... |
216208
| test.cpp:67:9:67:14 | ... = ... | test.cpp:52:19:52:37 | call to malloc | test.cpp:67:9:67:14 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:52:19:52:37 | call to malloc | call to malloc | test.cpp:53:20:53:23 | size | size |
217-
| test.cpp:201:5:201:19 | ... = ... | test.cpp:194:15:194:33 | call to malloc | test.cpp:201:5:201:19 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:194:15:194:33 | call to malloc | call to malloc | test.cpp:195:21:195:23 | len | len |
218209
| test.cpp:213:5:213:13 | ... = ... | test.cpp:205:15:205:33 | call to malloc | test.cpp:213:5:213:13 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:205:15:205:33 | call to malloc | call to malloc | test.cpp:206:21:206:23 | len | len |
219210
| test.cpp:264:13:264:14 | * ... | test.cpp:260:13:260:24 | new[] | test.cpp:264:13:264:14 | * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:260:13:260:24 | new[] | new[] | test.cpp:261:19:261:21 | len | len |
220211
| test.cpp:274:5:274:10 | ... = ... | test.cpp:270:13:270:24 | new[] | test.cpp:274:5:274:10 | ... = ... | This write might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:270:13:270:24 | new[] | new[] | test.cpp:271:19:271:21 | len | len |

‎cpp/ql/test/query-tests/Security/CWE/CWE-193/test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ void test12(unsigned len, unsigned index) {
198198
return;
199199
}
200200

201-
p[index] = '\0'; // $ deref=L195->L201 // BAD
201+
p[index] = '\0'; // $ MISSING: deref=L195->L201 // BAD [NOT DETECTED]
202202
}
203203

204204
void test13(unsigned len, unsigned index) {

0 commit comments

Comments
 (0)