From 2963dc1cb13b5da51f577fd7577383440fcec223 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 2 Mar 2023 22:38:43 +0000 Subject: [PATCH] C++: Include phi read nodes in SSA. There's a small fix to the mapping from 'global def -> use'. Finally, this commit also accepts a test failure related to new missing types for phi nodes. The fix for that is in the next commit. --- .../cpp/ir/dataflow/internal/SsaInternals.qll | 58 +++++++++++-------- .../dataflow/internal/ssa0/SsaInternals.qll | 13 +++-- .../dataflow/dataflow-tests/test.cpp | 2 +- .../dataflow-tests/type-bugs.expected | 5 ++ 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index 243b18e9ac1..5db883362b4 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -530,15 +530,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 +550,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 +673,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 +709,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 +718,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 +803,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 +813,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 +825,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 +974,14 @@ class Def extends DefOrUse { private module SsaImpl = SsaImplCommon::Make; -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; diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll index 7997bf7dee4..aa6a43a2580 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll @@ -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; -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; diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index edaaa3ade77..351fc386cdf 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -626,5 +626,5 @@ void test_def_via_phi_read(bool b) use(buffer); } intPointerSource(buffer); - sink(buffer); // $ ast MISSING: ir + sink(buffer); // $ ast,ir } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected index a77f3044647..dae4b7895eb 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected @@ -1,3 +1,8 @@ failures astTypeBugs irTypeBugs +| test.cpp:15:3:15:6 | test.cpp:15:3:15:6 | test.cpp:15:3:15:6 | Phi | +| test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | Phi | +| test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | Phi | +| test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | Phi | +| test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | Phi |