Skip to content

Commit b52a9a8

Browse files
aschackmullaschackmull
committed
C#: Instantiate shared Guards.
1 parent c01ac30 commit b52a9a8

File tree

4 files changed

+369
-0
lines changed

4 files changed

+369
-0
lines changed

‎csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowElement.qll‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
4040
*/
4141
Nodes::ElementNode getAControlFlowNode() { result.getAstNode() = this }
4242

43+
ControlFlow::Node getControlFlowNode() { result.getAstNode() = this }
44+
45+
BasicBlock getBasicBlock() { result = this.getAControlFlowNode().getBasicBlock() }
46+
4347
/**
4448
* Gets a first control flow node executed within this element.
4549
*/

‎csharp/ql/lib/semmle/code/csharp/controlflow/ControlFlowGraph.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,8 @@ module ControlFlow {
251251
}
252252
}
253253

254+
class NormalExitNode extends AnnotatedExitNode instanceof Impl::NormalExitNode { }
255+
254256
/** A node for a callable exit point. */
255257
class ExitNode extends Node instanceof Impl::ExitNode {
256258
/** Gets the callable that this exit applies to. */

‎csharp/ql/lib/semmle/code/csharp/controlflow/Guards.qll‎

Lines changed: 359 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,365 @@ private import semmle.code.csharp.frameworks.System
1313
private import semmle.code.csharp.frameworks.system.Linq
1414
private import semmle.code.csharp.frameworks.system.Collections
1515
private import semmle.code.csharp.frameworks.system.collections.Generic
16+
private import codeql.controlflow.Guards as SharedGuards
17+
18+
private module GuardsInput implements
19+
SharedGuards::InputSig<Location, ControlFlow::Node, ControlFlow::BasicBlock>
20+
{
21+
private import csharp as CS
22+
23+
class NormalExitNode = ControlFlow::Nodes::NormalExitNode;
24+
25+
class AstNode = ControlFlowElement;
26+
27+
class Expr = CS::Expr;
28+
29+
private newtype TConstantValue =
30+
TStringValue(string value) { any(StringLiteral s).getValue() = value }
31+
32+
class ConstantValue extends TConstantValue {
33+
string toString() { this = TStringValue(result) }
34+
}
35+
36+
abstract class ConstantExpr extends Expr {
37+
predicate isNull() { none() }
38+
39+
boolean asBooleanValue() { none() }
40+
41+
int asIntegerValue() { none() }
42+
43+
ConstantValue asConstantValue() { none() }
44+
}
45+
46+
private class NullConstant extends ConstantExpr {
47+
NullConstant() { nullValueImplied(this) }
48+
49+
override predicate isNull() { any() }
50+
}
51+
52+
private class BooleanConstant extends ConstantExpr instanceof BoolLiteral {
53+
override boolean asBooleanValue() { result = super.getBoolValue() }
54+
}
55+
56+
private predicate intConst(Expr e, int i) {
57+
e.getValue().toInt() = i and
58+
(
59+
e.getType() instanceof Enum
60+
or
61+
e.getType() instanceof IntegralType
62+
)
63+
}
64+
65+
private class IntegerConstant extends ConstantExpr {
66+
IntegerConstant() { intConst(this, _) }
67+
68+
override int asIntegerValue() { intConst(this, result) }
69+
}
70+
71+
private class EnumConst extends ConstantExpr {
72+
EnumConst() { this.getType() instanceof Enum and this.hasValue() }
73+
74+
override int asIntegerValue() { result = this.getValue().toInt() }
75+
}
76+
77+
private class StringConstant extends ConstantExpr instanceof StringLiteral {
78+
override ConstantValue asConstantValue() { result = TStringValue(this.getValue()) }
79+
}
80+
81+
class NonNullExpr extends Expr {
82+
NonNullExpr() { nonNullValueImplied(this) }
83+
}
84+
85+
class Case extends AstNode instanceof CS::Case {
86+
Expr getSwitchExpr() { super.getExpr() = result }
87+
88+
predicate isDefaultCase() { this instanceof DefaultCase }
89+
90+
ConstantExpr asConstantCase() { super.getPattern() = result }
91+
92+
private predicate hasEdge(BasicBlock bb1, BasicBlock bb2, MatchingCompletion c) {
93+
exists(PatternExpr pe |
94+
super.getPattern() = pe and
95+
c.isValidFor(pe) and
96+
bb1.getLastNode() = pe.getAControlFlowNode() and
97+
bb1.getASuccessor(c.getAMatchingSuccessorType()) = bb2
98+
)
99+
}
100+
101+
predicate matchEdge(BasicBlock bb1, BasicBlock bb2) {
102+
exists(MatchingCompletion c | this.hasEdge(bb1, bb2, c) and c.isMatch())
103+
}
104+
105+
predicate nonMatchEdge(BasicBlock bb1, BasicBlock bb2) {
106+
exists(MatchingCompletion c | this.hasEdge(bb1, bb2, c) and c.isNonMatch())
107+
}
108+
}
109+
110+
abstract private class BinExpr extends BinaryOperation { }
111+
112+
class AndExpr extends BinExpr {
113+
AndExpr() {
114+
this instanceof LogicalAndExpr or
115+
this instanceof BitwiseAndExpr
116+
}
117+
}
118+
119+
class OrExpr extends BinExpr {
120+
OrExpr() {
121+
this instanceof LogicalOrExpr or
122+
this instanceof BitwiseOrExpr
123+
}
124+
}
125+
126+
class NotExpr = LogicalNotExpr;
127+
128+
class IdExpr extends Expr {
129+
IdExpr() { this instanceof AssignExpr or this instanceof CastExpr }
130+
131+
Expr getEqualChildExpr() {
132+
result = this.(AssignExpr).getRValue()
133+
or
134+
result = this.(CastExpr).getExpr()
135+
}
136+
}
137+
138+
predicate equalityTest(Expr eqtest, Expr left, Expr right, boolean polarity) {
139+
exists(ComparisonTest ct |
140+
ct.getExpr() = eqtest and
141+
ct.getFirstArgument() = left and
142+
ct.getSecondArgument() = right
143+
|
144+
ct.getComparisonKind().isEquality() and polarity = true
145+
or
146+
ct.getComparisonKind().isInequality() and polarity = false
147+
)
148+
or
149+
exists(IsExpr ie, PatternExpr pat |
150+
ie = eqtest and
151+
ie.getExpr() = left and
152+
ie.getPattern() = pat
153+
|
154+
right = pat.(ConstantPatternExpr) and
155+
polarity = true
156+
or
157+
right = pat.(NotPatternExpr).getPattern().(ConstantPatternExpr) and
158+
polarity = false
159+
)
160+
}
161+
162+
class ConditionalExpr = CS::ConditionalExpr;
163+
164+
class Parameter = CS::Parameter;
165+
166+
private int parameterPosition() { result in [-1, any(Parameter p).getPosition()] }
167+
168+
class ParameterPosition extends int {
169+
ParameterPosition() { this = parameterPosition() }
170+
}
171+
172+
class ArgumentPosition extends int {
173+
ArgumentPosition() { this = parameterPosition() }
174+
}
175+
176+
pragma[inline]
177+
predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos = apos }
178+
179+
final private class FinalCallable = Callable;
180+
181+
class NonOverridableMethod extends FinalCallable {
182+
NonOverridableMethod() { not this.(Overridable).isOverridableOrImplementable() }
183+
184+
Parameter getParameter(ParameterPosition ppos) {
185+
super.getParameter(ppos) = result and
186+
not result.isParams()
187+
}
188+
189+
Expr getAReturnExpr() { this.canReturn(result) }
190+
}
191+
192+
class NonOverridableMethodCall extends Expr instanceof Call {
193+
NonOverridableMethod getMethod() { super.getTarget().getUnboundDeclaration() = result }
194+
195+
Expr getArgument(ArgumentPosition apos) {
196+
result = super.getArgumentForParameter(any(Parameter p | p.getPosition() = apos))
197+
}
198+
}
199+
}
200+
201+
private module GuardsImpl = SharedGuards::Make<Location, Cfg, GuardsInput>;
202+
203+
class GuardValue = GuardsImpl::GuardValue;
204+
205+
private module LogicInput implements GuardsImpl::LogicInputSig {
206+
class SsaDefinition extends Ssa::Definition {
207+
Expr getARead() { super.getARead() = result }
208+
}
209+
210+
class SsaWriteDefinition extends SsaDefinition instanceof Ssa::ExplicitDefinition {
211+
Expr getDefinition() { result = super.getADefinition().getSource() }
212+
}
213+
214+
class SsaPhiNode extends SsaDefinition instanceof Ssa::PhiNode {
215+
predicate hasInputFromBlock(SsaDefinition inp, BasicBlock bb) {
216+
super.hasInputFromBlock(inp, bb)
217+
}
218+
}
219+
220+
predicate parameterDefinition(Parameter p, SsaDefinition def) {
221+
def.(Ssa::ImplicitParameterDefinition).getParameter() = p
222+
}
223+
224+
predicate additionalNullCheck(GuardsImpl::PreGuard guard, GuardValue val, Expr e, boolean isNull) {
225+
// Comparison with a non-`null` value, for example `x?.Length > 0`
226+
exists(ComparisonTest ct, ComparisonKind ck, Expr arg | ct.getExpr() = guard |
227+
e instanceof DereferenceableExpr and
228+
ct.getAnArgument() = e and
229+
ct.getAnArgument() = arg and
230+
arg = any(NullValue nv | nv.isNonNull()).getAnExpr() and
231+
ck = ct.getComparisonKind() and
232+
e != arg and
233+
isNull = false and
234+
not ck.isEquality() and
235+
not ck.isInequality() and
236+
val.asBooleanValue() = true
237+
)
238+
or
239+
// Call to `string.IsNullOrEmpty()` or `string.IsNullOrWhiteSpace()`
240+
exists(MethodCall mc, string name | guard = mc |
241+
mc.getTarget() = any(SystemStringClass c).getAMethod(name) and
242+
name.regexpMatch("IsNullOr(Empty|WhiteSpace)") and
243+
mc.getArgument(0) = e and
244+
val.asBooleanValue() = false and
245+
isNull = false
246+
)
247+
or
248+
guard =
249+
any(PatternMatch pm |
250+
e instanceof DereferenceableExpr and
251+
e = pm.getExpr() and
252+
(
253+
val.asBooleanValue().booleanNot() = patternMatchesNull(pm.getPattern()) and
254+
isNull = false
255+
or
256+
exists(TypePatternExpr tpe |
257+
// E.g. `x is string` where `x` has type `string`
258+
typePattern(guard, tpe, tpe.getCheckedType()) and
259+
val.asBooleanValue() = false and
260+
isNull = true
261+
)
262+
)
263+
)
264+
or
265+
e.(DereferenceableExpr).hasNullableType() and
266+
guard =
267+
any(PropertyAccess pa |
268+
pa.getQualifier() = e and
269+
pa.getTarget().hasName("HasValue") and
270+
val.asBooleanValue().booleanNot() = isNull
271+
)
272+
}
273+
274+
predicate additionalImpliesStep(
275+
GuardsImpl::PreGuard g1, GuardValue v1, GuardsImpl::PreGuard g2, GuardValue v2
276+
) {
277+
g1 instanceof DereferenceableExpr and
278+
g1 = getNullEquivParent(g2) and
279+
v1.isNullness(_) and
280+
v2 = v1
281+
or
282+
g1 instanceof DereferenceableExpr and
283+
g2 = getANullImplyingChild(g1) and
284+
v1.isNonNullValue() and
285+
v2 = v1
286+
or
287+
g2 = g1.(NullCoalescingExpr).getAnOperand() and
288+
v1.isNullValue() and
289+
v2 = v1
290+
}
291+
}
292+
293+
module Guards = GuardsImpl::Logic<LogicInput>;
294+
295+
/*
296+
* Temporary debug predicates:
297+
*/
298+
299+
predicate debug_newcontrols(Guards::Guard g, BasicBlock bb, GuardValue v) { g.valueControls(bb, v) }
300+
301+
predicate debug_oldconvert(Guards::Guard g, BasicBlock bb, GuardValue v) {
302+
exists(AbstractValue av |
303+
g.(Guard).controlsBasicBlock(bb, av) and
304+
debug_convVals(av, v)
305+
)
306+
or
307+
debug_oldconvertCase(_, _, g, bb, v)
308+
}
309+
310+
predicate debug_oldconvertCase(Guard g1, MatchValue av, Guards::Guard g2, BasicBlock bb, GuardValue v) {
311+
g1.controlsBasicBlock(bb, av) and
312+
av.getCase() = g2 and
313+
if av.isMatch() then v.asBooleanValue() = true else v.asBooleanValue() = false
314+
}
315+
316+
predicate debug_caseconverted(Guard g1, Guards::Guard g, BasicBlock bb, GuardValue v) {
317+
debug_oldconvertCase(g1, _, g, bb, v) and
318+
2 <= strictcount(Guard g0 | debug_oldconvertCase(g0, _, g, bb, v))
319+
}
320+
321+
predicate debug_useless(Guards::Guard g, BasicBlock bb, GuardValue v) {
322+
debug_oldconvert(g, bb, v) and
323+
Guards::InternalUtil::exprHasValue(g, v)
324+
}
325+
326+
predicate debug_compare(int eq, int oldconv, int oldnonconv, int added, int new) {
327+
eq =
328+
count(Guards::Guard g, BasicBlock bb, GuardValue v |
329+
debug_newcontrols(g, bb, v) and debug_oldconvert(g, bb, v)
330+
) and
331+
oldconv =
332+
count(Guards::Guard g, BasicBlock bb, GuardValue v |
333+
debug_oldconvert(g, bb, v) and
334+
not debug_newcontrols(g, bb, v) and
335+
not debug_useless(g, bb, v)
336+
) and
337+
oldnonconv =
338+
count(Guard g, BasicBlock bb, AbstractValue av |
339+
g.controlsBasicBlock(bb, av) and
340+
not debug_convVals(av, _) and
341+
not debug_oldconvertCase(g, av, _, bb, _)
342+
// Remaining that are not converted:
343+
// av instanceof EmptyCollectionValue
344+
) and
345+
added =
346+
count(Guards::Guard g, BasicBlock bb, GuardValue v |
347+
debug_newcontrols(g, bb, v) and
348+
not debug_oldconvert(g, bb, v) and
349+
not debug_newGv(v)
350+
) and
351+
new =
352+
count(Guards::Guard g, BasicBlock bb, GuardValue v |
353+
debug_newcontrols(g, bb, v) and
354+
not debug_oldconvert(g, bb, v) and
355+
debug_newGv(v)
356+
)
357+
}
358+
359+
predicate debug_newGv(GuardValue v) {
360+
v.isThrowsException() or
361+
v.getDualValue().isThrowsException() or
362+
exists(v.getDualValue().asIntValue()) or
363+
v.isIntRange(_, _)
364+
}
365+
366+
predicate debug_convVals(AbstractValue av, GuardValue gv) {
367+
av.(AbstractValues::BooleanValue).getValue() = gv.asBooleanValue()
368+
or
369+
av.(AbstractValues::IntegerValue).getValue() = gv.asIntValue()
370+
or
371+
av.(AbstractValues::NullValue).isNull() and gv.isNullValue()
372+
or
373+
av.(AbstractValues::NullValue).isNonNull() and gv.isNonNullValue()
374+
}
16375

17376
/** An expression whose value may control the execution of another element. */
18377
class Guard extends Expr {

‎shared/controlflow/codeql/controlflow/Cfg.qll‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,6 +1185,10 @@ module MakeWithSplitting<
11851185

11861186
final class AnnotatedExitNode = AnnotatedExitNodeImpl;
11871187

1188+
final class NormalExitNode extends AnnotatedExitNodeImpl {
1189+
NormalExitNode() { this = TAnnotatedExitNode(_, true) }
1190+
}
1191+
11881192
/** An exit node for a given scope. */
11891193
private class ExitNodeImpl extends NodeImpl, TExitNode {
11901194
private CfgScope scope;

0 commit comments

Comments
 (0)