hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-01-29 14:23:03 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #218 from max/field-refs

Browse Source 
Fix handling of references to fields and methods
This commit is contained in:
Sauyon Lee
2020-01-16 14:26:55 -08:00
committed by GitHub Enterprise
parent 7040b76cf6 b7a830593d
commit aa9489ea28
23 changed files with 171 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
ql/src/InconsistentCode/InconsistentLoopOrientation.ql
View File

@@ -23,9 +23,9 @@ import go
* downward.
*/
predicate bounds(RelationalComparisonExpr test, Variable v, string direction) {
test.getLesserOperand() = v.getAUse() and direction = "upward"
test.getLesserOperand() = v.getAReference() and direction = "upward"
or
test.getGreaterOperand() = v.getAUse() and direction = "downward"
test.getGreaterOperand() = v.getAReference() and direction = "downward"
}
/**
@@ -36,7 +36,7 @@ predicate bounds(RelationalComparisonExpr test, Variable v, string direction) {
* downward.
*/
predicate updates(IncDecStmt upd, Variable v, string direction) {
upd.getExpr() = v.getAUse() and
upd.getExpr() = v.getAReference() and
(
upd instanceof IncStmt and direction = "upward"
or

2
ql/src/RedundantCode/DeadStoreOfLocal.ql
View File

@@ -36,7 +36,7 @@ where
// exclude assignments with default values or simple expressions
not isSimple(rhs) and
// exclude variables that are not used at all
exists(target.getAUse()) and
exists(target.getAReference()) and
// exclude variables with indirect references
not target.mayHaveIndirectReferences()
select def, "This definition of " + target + " is never used."

3
ql/src/semmle/go/Scopes.qll
View File

@@ -116,9 +116,6 @@ class Entity extends @object {
/** Gets the declaring identifier for this entity. */
Ident getDeclaration() { result.declares(this) }
/** Gets an identifier in rvalue position that refers to this entity. */
Ident getAUse() { result.uses(this) }
/** Gets a reference to this entity. */
Name getAReference() { result.getTarget() = this }

11
ql/src/semmle/go/controlflow/ControlFlowGraphImpl.qll
View File

@@ -114,9 +114,14 @@ newtype TControlFlowNode =
)
or
// in a return statement `return f()` where `f` has multiple return values
exists(ReturnStmt ret, CallExpr call | s = ret |
call = ret.getExpr().stripParens() and
exists(call.getType().(TupleType).getComponentType(i))
exists(ReturnStmt ret, SignatureType rettp |
s = ret and
// the return statement has a single expression
exists(ret.getExpr()) and
// but the enclosing function has multiple results
rettp = ret.getEnclosingFunction().getType() and
rettp.getNumResult() > 1 and
exists(rettp.getResultType(i))
)
or
// in a call `f(g())` where `g` has multiple return values

30
ql/src/semmle/go/controlflow/IR.qll
View File

@@ -50,16 +50,16 @@ module IR {
}
/** Holds if this instruction reads the value of variable or constant `v`. */
predicate reads(ValueEntity v) { none() }
predicate reads(ValueEntity v) { readsField(_, v) or readsMethod(_, v) }
/** Holds if this instruction updates variable or constant `v` to the value of `rhs`. */
predicate writes(ValueEntity v, Instruction rhs) { none() }
predicate writes(ValueEntity v, Instruction rhs) { writesField(_, v, rhs) }
/** Holds if this instruction reads the value of field `f` on the value of `base`. */
predicate readsField(Instruction base, Field f) { none() }
/** Holds if this instruction updates the value of field `f` on the value of `base`. */
predicate writesField(Instruction base, Field f) { none() }
predicate writesField(Instruction base, Field f, Instruction rhs) { none() }
/** Holds if this instruction looks up method `m` on the value of `receiver`. */
predicate readsMethod(Instruction receiver, Method m) { none() }
@@ -156,7 +156,7 @@ module IR {
/** Gets the expression underlying this instruction. */
Expr getExpr() { result = e }
override predicate reads(ValueEntity v) { e = v.getAUse() }
override predicate reads(ValueEntity v) { e = v.getAReference() }
override Type getResultType() { result = e.getType() }
@@ -286,7 +286,7 @@ module IR {
Instruction getRhs() { none() }
override predicate writes(ValueEntity v, Instruction rhs) {
getLhs().(VarOrConstTarget).refersTo(v) and
getLhs().refersTo(v) and
rhs = getRhs()
}
}
@@ -398,9 +398,10 @@ module IR {
/** Gets the field being written. */
Field getField() { result = lhs.getField() }
override predicate writesField(Instruction base, Field f) {
override predicate writesField(Instruction base, Field f, Instruction rhs) {
getBase() = base and
getField() = f
getField() = f and
getRhs() = rhs
}
}
@@ -1143,7 +1144,7 @@ module IR {
}
}
/** A representation of the target of of a write instruction. */
/** A representation of the target of a write instruction. */
class WriteTarget extends TWriteTarget {
ControlFlow::Node w;
@@ -1160,6 +1161,9 @@ module IR {
getWrite() = result.getDefinition().(SsaExplicitDefinition).getInstruction()
}
/** Holds if `e` is the variable or field being written to. */
predicate refersTo(ValueEntity e) { none() }
/** Gets a textual representation of this target. */
string toString() { result = "write target" }
@@ -1196,8 +1200,7 @@ module IR {
)
}
/** Holds if this is a reference to variable or constant `e`. */
predicate refersTo(ValueEntity e) {
override predicate refersTo(ValueEntity e) {
this instanceof MkLhs and
loc = e.getAReference()
or
@@ -1254,11 +1257,10 @@ module IR {
/** Get the type of the base of this field access, that is, the type that contains the field. */
Type getBaseType() { result = this.getBase().(EvalInstruction).getExpr().getType() }
/** Holds if this is a reference to variable or constant `e`. */
predicate refersTo(Field f) {
exists(SelectorExpr sel | this = MkLhs(_, sel) | sel.getSelector() = f.getAReference())
override predicate refersTo(ValueEntity e) {
exists(SelectorExpr sel | this = MkLhs(_, sel) | sel.uses(e))
or
f = w.(InitLiteralStructFieldInstruction).getField()
e = w.(InitLiteralStructFieldInstruction).getField()
}
override string getName() { exists(Field f | this.refersTo(f) | result = f.getName()) }

28
ql/src/semmle/go/dataflow/GlobalValueNumbering.qll
View File

@@ -203,9 +203,7 @@ private newtype GVNBase =
// guaranteed to have the same value.
MkOtherVariable(ValueEntity x, ControlFlow::Node dominator) { mkOtherVariable(_, x, dominator) } or
MkMethodAccess(GVN base, Function m) { mkMethodAccess(_, base, m) } or
MkFieldRead(GVN base, Variable f, ControlFlow::Node dominator) {
mkFieldRead(_, base, f, dominator)
} or
MkFieldRead(GVN base, Field f, ControlFlow::Node dominator) { mkFieldRead(_, base, f, dominator) } or
MkPureCall(Function f, GVN callee, GVNList args) { mkPureCall(_, f, callee, args) } or
MkIndex(GVN base, GVN index, ControlFlow::Node dominator) { mkIndex(_, base, index, dominator) } or
// Dereference a pointer. The value might have changed since the last
@@ -340,7 +338,10 @@ private predicate mkBoolConst(DataFlow::Node nd, boolean val) {
nd.isPlatformIndependentConstant()
}
private predicate mkFunc(DataFlow::Node nd, Function f) { nd = f.getARead() }
private predicate mkFunc(DataFlow::Node nd, Function f) {
nd = f.getARead() and
not f instanceof Method
}
private predicate analyzableConst(DataFlow::Node e) {
mkNumericConst(e, _) or mkStringConst(e, _) or mkBoolConst(e, _) or mkFunc(e, _)
@@ -351,7 +352,7 @@ private predicate analyzableMethodAccess(Read access, DataFlow::Node receiver, M
not access.isConst()
}
private predicate mkMethodAccess(DataFlow::Node access, GVN qualifier, Function m) {
private predicate mkMethodAccess(DataFlow::Node access, GVN qualifier, Method m) {
exists(DataFlow::Node base |
analyzableMethodAccess(access, base, m) and
qualifier = globalValueNumber(base)
@@ -365,7 +366,7 @@ private predicate analyzableFieldRead(Read fread, DataFlow::Node base, Field f)
}
private predicate mkFieldRead(
DataFlow::Node fread, GVN qualifier, Variable v, ControlFlow::Node dominator
DataFlow::Node fread, GVN qualifier, Field v, ControlFlow::Node dominator
) {
exists(DataFlow::Node base |
analyzableFieldRead(fread, base, v) and
@@ -393,12 +394,15 @@ private predicate mkPureCall(DataFlow::CallNode ce, Function f, GVN callee, GVNL
* variables of non-primitive type (for which deep mutations are not captured by SSA).
*/
private predicate incompleteSsa(ValueEntity v) {
not v instanceof SsaSourceVariable
or
v.(SsaSourceVariable).mayHaveIndirectReferences()
or
exists(Type tp | tp = v.(DeclaredVariable).getType().getUnderlyingType() |
not tp instanceof BasicType
not v instanceof Field and
(
not v instanceof SsaSourceVariable
or
v.(SsaSourceVariable).mayHaveIndirectReferences()
or
exists(Type tp | tp = v.(DeclaredVariable).getType().getUnderlyingType() |
not tp instanceof BasicType
)
)
}

2
ql/src/semmle/go/dataflow/SSA.qll
View File

@@ -22,7 +22,7 @@ class SsaSourceVariable extends LocalVariable {
*/
predicate mayHaveIndirectReferences() {
// variables that have their address taken
exists(AddressExpr addr | addr.getOperand().stripParens() = getAUse())
exists(AddressExpr addr | addr.getOperand().stripParens() = getAReference())
or
exists(DataFlow::MethodReadNode mrn |
mrn.getReceiver() = getARead() and

1
ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll
View File

@@ -72,6 +72,7 @@ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) {
predicate jumpStep(Node n1, Node n2) {
exists(ValueEntity v, Write w |
not v instanceof SsaSourceVariable and
not v instanceof Field and
w.writes(v, n1) and
n2 = v.getARead()
)

2
ql/test/library-tests/semmle/go/Scopes/DeclaredEntity.expected
View File

@@ -1,4 +1,5 @@
| a | types.go:24:22:24:22 | a |
| bump | main.go:23:16:23:19 | bump |
| foo | main.go:17:6:17:8 | foo |
| iHaveAMethod | types.go:3:6:3:17 | iHaveAMethod |
| main | main.go:9:6:9:9 | main |
@@ -12,6 +13,7 @@
| meth2 | types.go:28:16:28:20 | meth2 |
| notImpl | types.go:22:6:22:12 | notImpl |
| recv | main.go:13:7:13:10 | recv |
| recv | main.go:23:7:23:10 | recv |
| starImpl | types.go:12:6:12:13 | starImpl |
| t | main.go:5:6:5:6 | t |
| twoMethods | types.go:7:6:7:15 | twoMethods |

13
ql/test/library-tests/semmle/go/Scopes/EntityRead.expected Normal file
View File

@@ -0,0 +1,13 @@
| Println | main.go:10:2:10:12 | selection of Println |
| a | types.go:25:9:25:9 | a |
| false | types.go:15:9:15:13 | false |
| meth | main.go:18:2:18:7 | selection of meth |
| meth | main.go:19:2:19:7 | selection of meth |
| meth | main.go:20:2:20:10 | selection of meth |
| recv | main.go:14:9:14:12 | recv |
| recv | main.go:24:2:24:5 | recv |
| x | main.go:14:9:14:14 | selection of x |
| x | main.go:18:2:18:2 | x |
| x | main.go:24:2:24:7 | selection of x |
| y | main.go:19:2:19:2 | y |
| y | main.go:20:12:20:12 | y |

4
ql/test/library-tests/semmle/go/Scopes/EntityRead.ql Normal file
View File

@@ -0,0 +1,4 @@
import go
from ValueEntity e
select e, e.getARead()

61
ql/test/library-tests/semmle/go/Scopes/EntityReference.expected Normal file
View File

@@ -0,0 +1,61 @@
| Println | <library> | main.go:10:2:10:12 | selection of Println |
| Println | <library> | main.go:10:6:10:12 | Println |
| a | types.go@24:22-24:22 | types.go:24:22:24:22 | a |
| a | types.go@24:22-24:22 | types.go:25:9:25:9 | a |
| bool | <library> | types.go:8:10:8:13 | bool |
| bool | <library> | types.go:14:26:14:29 | bool |
| bool | <library> | types.go:24:29:24:32 | bool |
| bump | main.go@23:16-23:19 | main.go:23:16:23:19 | bump |
| false | <library> | types.go:15:9:15:13 | false |
| fmt | <library> | main.go:10:2:10:4 | fmt |
| foo | main.go@17:6-17:8 | main.go:17:6:17:8 | foo |
| iHaveAMethod | types.go@3:6-3:17 | main.go:17:12:17:23 | iHaveAMethod |
| iHaveAMethod | types.go@3:6-3:17 | types.go:3:6:3:17 | iHaveAMethod |
| int | <library> | main.go:6:4:6:6 | int |
| int | <library> | main.go:13:23:13:25 | int |
| int | <library> | types.go:4:9:4:11 | int |
| int | <library> | types.go:9:10:9:12 | int |
| int | <library> | types.go:18:25:18:27 | int |
| int | <library> | types.go:24:24:24:26 | int |
| int | <library> | types.go:28:24:28:26 | int |
| main | main.go@9:6-9:9 | main.go:9:6:9:9 | main |
| meth | main.go@13:16-13:19 | main.go:13:16:13:19 | meth |
| meth | main.go@13:16-13:19 | main.go:19:2:19:7 | selection of meth |
| meth | main.go@13:16-13:19 | main.go:19:4:19:7 | meth |
| meth | main.go@13:16-13:19 | main.go:20:2:20:10 | selection of meth |
| meth | main.go@13:16-13:19 | main.go:20:7:20:10 | meth |
| meth | types.go@4:2-4:5 | main.go:18:2:18:7 | selection of meth |
| meth | types.go@4:2-4:5 | main.go:18:4:18:7 | meth |
| meth | types.go@4:2-4:5 | types.go:4:2:4:5 | meth |
| meth1 | types.go@8:2-8:6 | types.go:8:2:8:6 | meth1 |
| meth1 | types.go@14:18-14:22 | types.go:14:18:14:22 | meth1 |
| meth1 | types.go@24:16-24:20 | types.go:24:16:24:20 | meth1 |
| meth2 | types.go@9:2-9:6 | types.go:9:2:9:6 | meth2 |
| meth2 | types.go@18:17-18:21 | types.go:18:17:18:21 | meth2 |
| meth2 | types.go@28:16-28:20 | types.go:28:16:28:20 | meth2 |
| notImpl | types.go@22:6-22:12 | types.go:22:6:22:12 | notImpl |
| notImpl | types.go@22:6-22:12 | types.go:24:7:24:13 | notImpl |
| notImpl | types.go@22:6-22:12 | types.go:28:7:28:13 | notImpl |
| recv | main.go@13:7-13:10 | main.go:13:7:13:10 | recv |
| recv | main.go@13:7-13:10 | main.go:14:9:14:12 | recv |
| recv | main.go@23:7-23:10 | main.go:23:7:23:10 | recv |
| recv | main.go@23:7-23:10 | main.go:24:2:24:5 | recv |
| starImpl | types.go@12:6-12:13 | types.go:12:6:12:13 | starImpl |
| starImpl | types.go@12:6-12:13 | types.go:14:8:14:15 | starImpl |
| starImpl | types.go@12:6-12:13 | types.go:18:7:18:14 | starImpl |
| t | main.go@5:6-5:6 | main.go:5:6:5:6 | t |
| t | main.go@5:6-5:6 | main.go:13:13:13:13 | t |
| t | main.go@5:6-5:6 | main.go:17:29:17:29 | t |
| t | main.go@5:6-5:6 | main.go:20:4:20:4 | t |
| t | main.go@5:6-5:6 | main.go:23:13:23:13 | t |
| twoMethods | types.go@7:6-7:15 | types.go:7:6:7:15 | twoMethods |
| x | main.go@6:2-6:2 | main.go:6:2:6:2 | x |
| x | main.go@6:2-6:2 | main.go:14:9:14:14 | selection of x |
| x | main.go@6:2-6:2 | main.go:14:14:14:14 | x |
| x | main.go@6:2-6:2 | main.go:24:2:24:7 | selection of x |
| x | main.go@6:2-6:2 | main.go:24:7:24:7 | x |
| x | main.go@17:10-17:10 | main.go:17:10:17:10 | x |
| x | main.go@17:10-17:10 | main.go:18:2:18:2 | x |
| y | main.go@17:26-17:26 | main.go:17:26:17:26 | y |
| y | main.go@17:26-17:26 | main.go:19:2:19:2 | y |
| y | main.go@17:26-17:26 | main.go:20:12:20:12 | y |

2
ql/test/library-tests/semmle/go/Scopes/EntityUse.ql → ql/test/library-tests/semmle/go/Scopes/EntityReference.ql
View File

@@ -6,4 +6,4 @@ where
or
not exists(e.getDeclaration()) and
declloc = "<library>"
select e, declloc, e.getAUse()
select e, declloc, e.getAReference()

2
ql/test/library-tests/semmle/go/Scopes/EntityType.expected
View File

@@ -1,4 +1,5 @@
| a | int |
| bump | func() |
| foo | func(iHaveAMethod, * t) |
| iHaveAMethod | iHaveAMethod |
| main | func() |
@@ -12,6 +13,7 @@
| meth2 | func() int |
| notImpl | notImpl |
| recv | * t |
| recv | * t |
| starImpl | starImpl |
| t | t |
| twoMethods | twoMethods |

30
ql/test/library-tests/semmle/go/Scopes/EntityUse.expected
View File

@@ -1,30 +0,0 @@
| Println | <library> | main.go:10:6:10:12 | Println |
| a | types.go@24:22-24:22 | types.go:25:9:25:9 | a |
| bool | <library> | types.go:8:10:8:13 | bool |
| bool | <library> | types.go:14:26:14:29 | bool |
| bool | <library> | types.go:24:29:24:32 | bool |
| false | <library> | types.go:15:9:15:13 | false |
| fmt | <library> | main.go:10:2:10:4 | fmt |
| iHaveAMethod | types.go@3:6-3:17 | main.go:17:12:17:23 | iHaveAMethod |
| int | <library> | main.go:6:4:6:6 | int |
| int | <library> | main.go:13:23:13:25 | int |
| int | <library> | types.go:4:9:4:11 | int |
| int | <library> | types.go:9:10:9:12 | int |
| int | <library> | types.go:18:25:18:27 | int |
| int | <library> | types.go:24:24:24:26 | int |
| int | <library> | types.go:28:24:28:26 | int |
| meth | main.go@13:16-13:19 | main.go:19:4:19:7 | meth |
| meth | main.go@13:16-13:19 | main.go:20:7:20:10 | meth |
| meth | types.go@4:2-4:5 | main.go:18:4:18:7 | meth |
| notImpl | types.go@22:6-22:12 | types.go:24:7:24:13 | notImpl |
| notImpl | types.go@22:6-22:12 | types.go:28:7:28:13 | notImpl |
| recv | main.go@13:7-13:10 | main.go:14:9:14:12 | recv |
| starImpl | types.go@12:6-12:13 | types.go:14:8:14:15 | starImpl |
| starImpl | types.go@12:6-12:13 | types.go:18:7:18:14 | starImpl |
| t | main.go@5:6-5:6 | main.go:13:13:13:13 | t |
| t | main.go@5:6-5:6 | main.go:17:29:17:29 | t |
| t | main.go@5:6-5:6 | main.go:20:4:20:4 | t |
| x | main.go@6:2-6:2 | main.go:14:14:14:14 | x |
| x | main.go@17:10-17:10 | main.go:18:2:18:2 | x |
| y | main.go@17:26-17:26 | main.go:19:2:19:2 | y |
| y | main.go@17:26-17:26 | main.go:20:12:20:12 | y |

6
ql/test/library-tests/semmle/go/Scopes/EntityWrite.expected Normal file
View File

@@ -0,0 +1,6 @@
| a | types.go:24:22:24:22 | initialization of a |
| recv | main.go:13:7:13:10 | initialization of recv |
| recv | main.go:23:7:23:10 | initialization of recv |
| x | main.go:17:10:17:10 | initialization of x |
| x | main.go:24:2:24:9 | increment statement |
| y | main.go:17:26:17:26 | initialization of y |

4
ql/test/library-tests/semmle/go/Scopes/EntityWrite.ql Normal file
View File

@@ -0,0 +1,4 @@
import go
from ValueEntity e
select e, e.getAWrite()

1
ql/test/library-tests/semmle/go/Scopes/Methods.expected
View File

@@ -1,3 +1,4 @@
| bump | github.com/Semmle/go/ql/test/library-tests/semmle/go/Scopes.t.bump | recv | * t |
| meth | github.com/Semmle/go/ql/test/library-tests/semmle/go/Scopes.iHaveAMethod.meth | | iHaveAMethod |
| meth | github.com/Semmle/go/ql/test/library-tests/semmle/go/Scopes.t.meth | recv | * t |
| meth1 | github.com/Semmle/go/ql/test/library-tests/semmle/go/Scopes.notImpl.meth1 | | notImpl |

4
ql/test/library-tests/semmle/go/Scopes/main.go
View File

@@ -19,3 +19,7 @@ func foo(x iHaveAMethod, y *t) {
y.meth()
(*t).meth(y)
}
func (recv *t) bump() {
recv.x++
}

2
ql/test/library-tests/semmle/go/dataflow/GlobalValueNumbering/GlobalValueNumber.expected
View File

@@ -40,3 +40,5 @@
| regressions.go:7:11:7:15 | false | regressions.go:7:11:7:15 | false |
| regressions.go:9:11:9:12 | !... | regressions.go:11:11:11:14 | true |
| regressions.go:11:11:11:14 | true | regressions.go:11:11:11:14 | true |
| regressions.go:30:9:30:22 | call to getPayload | regressions.go:30:9:30:22 | call to getPayload |
| regressions.go:30:26:30:39 | call to getPayload | regressions.go:30:26:30:39 | call to getPayload |

20
ql/test/library-tests/semmle/go/dataflow/GlobalValueNumbering/regressions.go
View File

@@ -9,3 +9,23 @@ const d = false
const e = !d
const f = true
type cell struct {
payload int
next *cell
}
func test4(x, y cell) int {
return x.payload +
y.payload +
x.next.payload +
y.next.payload
}
func (c *cell) getPayload() int {
return c.payload
}
func test5(x, y cell) int {
return x.getPayload() + y.getPayload()
}

1
ql/test/library-tests/semmle/go/dataflow/SSA/VarUses.expected
View File

@@ -27,4 +27,5 @@
| main.go:84:9:84:9 | x | x |
| main.go:84:15:84:15 | x | x |
| main.go:94:2:94:8 | wrapper | wrapper |
| main.go:94:2:94:10 | selection of s | s |
| main.go:97:9:97:9 | x | x |

4
ql/test/query-tests/RedundantCode/DeadStoreOfLocal/main.go
View File

@@ -31,3 +31,7 @@ func test2(x int) (int, int) {
z := x % (1)
return z, y % 13
}
func test3() (x int, y int) {
return unknownFunction()
}