hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-23 20:26:32 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #12356 from MathiasVP/use-phi-reads

Browse Source 
C++: Include "phi reads" in `DataFlow::Node`
This commit is contained in:
Mathias Vorreiter Pedersen
2023-03-03 11:14:08 +00:00
committed by GitHub
parent 7f9b856a21 959237e8d2
commit 16e817c814
5 changed files with 84 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
View File

@@ -43,7 +43,7 @@ class Node0Impl extends TIRDataFlowNode0 {
/**
* Gets the type of this node.
*
* If `asInstruction().isGLValue()` holds, then the type of this node
* If `isGLValue()` holds, then the type of this node
* should be thought of as "pointer to `getType()`".
*/
DataFlowType getType() { none() } // overridden in subclasses

9
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
View File

@@ -498,7 +498,14 @@ class SsaPhiNode extends Node, TSsaPhiNode {
override Declaration getFunction() { result = phi.getBasicBlock().getEnclosingFunction() }
override DataFlowType getType() { result = this.getAnInput().getType().getUnspecifiedType() }
override DataFlowType getType() {
exists(Ssa::SourceVariable sv |
this.getPhiNode().definesAt(sv, _, _, _) and
result = sv.getType()
)
}
override predicate isGLValue() { phi.getSourceVariable().isGLValue() }
final override Location getLocationImpl() { result = phi.getBasicBlock().getLocation() }

76
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
View File

@@ -48,6 +48,16 @@ private module SourceVariables {
* indirections) of this source variable.
*/
abstract BaseSourceVariable getBaseVariable();
/** Holds if this variable is a glvalue. */
predicate isGLValue() { none() }
/**
* Gets the type of this source variable. If `isGLValue()` holds, then
* the type of this source variable should be thought of as "pointer
* to `getType()`".
*/
abstract DataFlowType getType();
}
class SourceIRVariable extends SourceVariable, TSourceIRVariable {
@@ -66,6 +76,12 @@ private module SourceVariables {
ind > 0 and
result = this.getIRVariable().toString() + " indirection"
}
override predicate isGLValue() { ind = 0 }
override DataFlowType getType() {
if ind = 0 then result = var.getType() else result = getTypeImpl(var.getType(), ind - 1)
}
}
class CallVariable extends SourceVariable, TCallVariable {
@@ -84,6 +100,8 @@ private module SourceVariables {
ind > 0 and
result = "Call indirection"
}
override DataFlowType getType() { result = getTypeImpl(call.getResultType(), ind) }
}
}
@@ -530,15 +548,15 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) {
exists(IRBlock bb1, int i1, SourceVariable v |
defOrUse1.asDefOrUse().hasIndexInBlock(bb1, i1, v)
|
exists(IRBlock bb2, int i2, Definition def |
adjacentDefRead(pragma[only_bind_into](def), pragma[only_bind_into](bb1),
exists(IRBlock bb2, int i2, DefinitionExt def |
adjacentDefReadExt(pragma[only_bind_into](def), pragma[only_bind_into](bb1),
pragma[only_bind_into](i1), pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
def.getSourceVariable() = v and
use.asDefOrUse().(UseImpl).hasIndexInBlock(bb2, i2, v)
)
or
exists(PhiNode phi |
lastRefRedef(_, bb1, i1, phi) and
lastRefRedefExt(_, bb1, i1, phi) and
use.asPhi() = phi and
phi.getSourceVariable() = pragma[only_bind_into](v)
)
@@ -550,11 +568,18 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) {
* flows to `useOrPhi`.
*/
private predicate globalDefToUse(GlobalDef globalDef, UseOrPhi useOrPhi) {
exists(IRBlock bb1, int i1, IRBlock bb2, int i2, SourceVariable v |
globalDef.hasIndexInBlock(bb1, i1, v) and
adjacentDefRead(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1),
pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
useOrPhi.asDefOrUse().hasIndexInBlock(bb2, i2, v)
exists(IRBlock bb1, int i1, SourceVariable v | globalDef.hasIndexInBlock(bb1, i1, v) |
exists(IRBlock bb2, int i2 |
adjacentDefReadExt(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1),
pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
useOrPhi.asDefOrUse().hasIndexInBlock(bb2, i2, v)
)
or
exists(PhiNode phi |
lastRefRedefExt(_, bb1, i1, phi) and
useOrPhi.asPhi() = phi and
phi.getSourceVariable() = pragma[only_bind_into](v)
)
)
}
@@ -666,17 +691,17 @@ private predicate ssaFlowImpl(SsaDefOrUse defOrUse, Node nodeFrom, Node nodeTo,
* Holds if `def` is the corresponding definition of
* the SSA library's `definition`.
*/
private Definition ssaDefinition(Def def) {
private DefinitionExt ssaDefinition(Def def) {
exists(IRBlock block, int i, SourceVariable sv |
def.hasIndexInBlock(block, i, sv) and
result.definesAt(sv, block, i)
result.definesAt(sv, block, i, _)
)
}
/** Gets a node that represents the prior definition of `node`. */
private Node getAPriorDefinition(SsaDefOrUse defOrUse) {
exists(IRBlock bb, int i, SourceVariable sv, Definition def, DefOrUse defOrUse0 |
SsaCached::lastRefRedef(pragma[only_bind_into](def), pragma[only_bind_into](bb),
exists(IRBlock bb, int i, SourceVariable sv, DefinitionExt def, DefOrUse defOrUse0 |
lastRefRedefExt(pragma[only_bind_into](def), pragma[only_bind_into](bb),
pragma[only_bind_into](i), ssaDefinition(defOrUse)) and
def.getSourceVariable() = sv and
defOrUse0.hasIndexInBlock(bb, i, sv) and
@@ -702,7 +727,7 @@ pragma[nomagic]
private predicate fromPhiNodeToUse(PhiNode phi, SourceVariable sv, IRBlock bb1, int i1, UseOrPhi use) {
exists(IRBlock bb2, int i2 |
use.asDefOrUse().hasIndexInBlock(bb2, i2, sv) and
adjacentDefRead(pragma[only_bind_into](phi), pragma[only_bind_into](bb1),
adjacentDefReadExt(pragma[only_bind_into](phi), pragma[only_bind_into](bb1),
pragma[only_bind_into](i1), pragma[only_bind_into](bb2), pragma[only_bind_into](i2))
)
}
@@ -711,13 +736,13 @@ private predicate fromPhiNodeToUse(PhiNode phi, SourceVariable sv, IRBlock bb1,
predicate fromPhiNode(SsaPhiNode nodeFrom, Node nodeTo) {
exists(PhiNode phi, SourceVariable sv, IRBlock bb1, int i1, UseOrPhi use |
phi = nodeFrom.getPhiNode() and
phi.definesAt(sv, bb1, i1) and
phi.definesAt(sv, bb1, i1, _) and
useToNode(use, nodeTo)
|
fromPhiNodeToUse(phi, sv, bb1, i1, use)
or
exists(PhiNode phiTo |
lastRefRedef(phi, _, _, phiTo) and
lastRefRedefExt(phi, _, _, phiTo) and
nodeTo.(SsaPhiNode).getPhiNode() = phiTo
)
)
@@ -796,8 +821,8 @@ module SsaCached {
* path between them without any read of `def`.
*/
cached
predicate adjacentDefRead(Definition def, IRBlock bb1, int i1, IRBlock bb2, int i2) {
SsaImpl::adjacentDefRead(def, bb1, i1, bb2, i2)
predicate adjacentDefReadExt(DefinitionExt def, IRBlock bb1, int i1, IRBlock bb2, int i2) {
SsaImpl::adjacentDefReadExt(def, _, bb1, i1, bb2, i2)
}
/**
@@ -806,8 +831,8 @@ module SsaCached {
* without passing through another read or write.
*/
cached
predicate lastRefRedef(Definition def, IRBlock bb, int i, Definition next) {
SsaImpl::lastRefRedef(def, bb, i, next)
predicate lastRefRedefExt(DefinitionExt def, IRBlock bb, int i, DefinitionExt next) {
SsaImpl::lastRefRedefExt(def, _, bb, i, next)
}
}
@@ -818,8 +843,8 @@ private newtype TSsaDefOrUse =
or
// Like in the pruning stage, we only include definition that's live after the
// write as the final definitions computed by SSA.
exists(Definition def, SourceVariable sv, IRBlock bb, int i |
def.definesAt(sv, bb, i) and
exists(DefinitionExt def, SourceVariable sv, IRBlock bb, int i |
def.definesAt(sv, bb, i, _) and
defOrUse.(DefImpl).hasIndexInBlock(bb, i, sv)
)
} or
@@ -967,9 +992,14 @@ class Def extends DefOrUse {
private module SsaImpl = SsaImplCommon::Make<SsaInput>;
class PhiNode = SsaImpl::PhiNode;
class PhiNode extends SsaImpl::DefinitionExt {
PhiNode() {
this instanceof SsaImpl::PhiNode or
this instanceof SsaImpl::PhiReadNode
}
}
class Definition = SsaImpl::Definition;
class DefinitionExt = SsaImpl::DefinitionExt;
class UncertainWriteDefinition = SsaImpl::UncertainWriteDefinition;

13
cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll
View File

@@ -225,8 +225,8 @@ private newtype TSsaDefOrUse =
or
// If `defOrUse` is a definition we only include it if the
// SSA library concludes that it's live after the write.
exists(Definition def, SourceVariable sv, IRBlock bb, int i |
def.definesAt(sv, bb, i) and
exists(DefinitionExt def, SourceVariable sv, IRBlock bb, int i |
def.definesAt(sv, bb, i, _) and
defOrUse.(DefImpl).hasIndexInBlock(bb, i, sv)
)
} or
@@ -301,6 +301,11 @@ class Def extends DefOrUse {
private module SsaImpl = SsaImplCommon::Make<SsaInput>;
class PhiNode = SsaImpl::PhiNode;
class PhiNode extends SsaImpl::DefinitionExt {
PhiNode() {
this instanceof SsaImpl::PhiNode or
this instanceof SsaImpl::PhiReadNode
}
}
class Definition = SsaImpl::Definition;
class DefinitionExt = SsaImpl::DefinitionExt;

13
cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
View File

@@ -615,3 +615,16 @@ void test_flow_through_void_double_pointer(int *p) // $ ast-def=p
void* q = (void*)&p;
sink(**(int**)q); // $ ir MISSING: ast
}
void use(int *);
void test_def_via_phi_read(bool b)
{
static int buffer[10]; // This is missing an initialisation in IR dataflow
if (b)
{
use(buffer);
}
intPointerSource(buffer);
sink(buffer); // $ ast,ir
}