From ba7cb8f4ae4b075a9a07e91a0091e3e6150df8d7 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Wed, 21 Jun 2023 17:54:52 -0400 Subject: [PATCH 1/3] C++: fix range analysis back edge detection for irreducible CFGs --- .../new/internal/semantic/SemanticSSA.qll | 15 ++++++++++++ .../library-tests/ir/range-analysis/test.cpp | 24 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll index 29580c2c507..65c0efec4aa 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll @@ -70,6 +70,21 @@ predicate semBackEdge(SemSsaPhiNode phi, SemSsaVariable inp, SemSsaReadPositionP // Conservatively assume that every edge is a back edge if we don't have dominance information. ( phi.getBasicBlock().bbDominates(edge.getOrigBlock()) or + trimmedReachable(phi.getBasicBlock(), edge.getOrigBlock()) or not edge.getOrigBlock().hasDominanceInformation() ) } + +private predicate trimmedReachable(SemBasicBlock b1, SemBasicBlock b2) { + b1 = b2 + or + exists(SemBasicBlock mid | + trimmedReachable(b1, mid) and + trimmedEdges(mid, b2) + ) +} + +private predicate trimmedEdges(SemBasicBlock pred, SemBasicBlock succ) { + pred.getASuccessor() = succ and + not succ.bbDominates(pred) +} diff --git a/cpp/ql/test/library-tests/ir/range-analysis/test.cpp b/cpp/ql/test/library-tests/ir/range-analysis/test.cpp index 95e6474124a..1e28d858b78 100644 --- a/cpp/ql/test/library-tests/ir/range-analysis/test.cpp +++ b/cpp/ql/test/library-tests/ir/range-analysis/test.cpp @@ -70,3 +70,27 @@ int f4(int x) { } } } + +// No interesting ranges to check here - this irreducible CFG caused an infinite loop due to back edge detection +void gotoLoop(bool b1, bool b2) +{ + int j; + + if (b1) + return; + + if (!b2) + { + for (j = 0; j < 10; ++j) + { + goto main_decode_loop; + } + } + else + { + for (j = 0; j < 10; ++j) + { + main_decode_loop: + } + } +} \ No newline at end of file From aff4066020cdc6782fd3a9a24b3fbc761f22449f Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Mon, 26 Jun 2023 15:39:09 -0400 Subject: [PATCH 2/3] C++: improve irreducible back edge detection --- .../new/internal/semantic/SemanticSSA.qll | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll index 65c0efec4aa..a02760a9f2a 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll @@ -70,21 +70,26 @@ predicate semBackEdge(SemSsaPhiNode phi, SemSsaVariable inp, SemSsaReadPositionP // Conservatively assume that every edge is a back edge if we don't have dominance information. ( phi.getBasicBlock().bbDominates(edge.getOrigBlock()) or - trimmedReachable(phi.getBasicBlock(), edge.getOrigBlock()) or + irreducibleSccEdge(phi.getBasicBlock(), edge.getOrigBlock()) or not edge.getOrigBlock().hasDominanceInformation() ) } -private predicate trimmedReachable(SemBasicBlock b1, SemBasicBlock b2) { - b1 = b2 - or - exists(SemBasicBlock mid | - trimmedReachable(b1, mid) and - trimmedEdges(mid, b2) - ) +/** + * Holds if the edge from b1 to b2 is part of a multiple-entry cycle in an irreducible control flow + * graph. + * + * An ireducible control flow graph is one where the usual dominance-based back edge detection does + * not work, because there is a cycle with multiple entry points, meaning there are + * mutually-reachable basic blocks where neither dominates the other. For such a graph, we first + * all detectable back-edges using the normal condition that the predecessor block is dominated by + * the successor block, then mark all edges in a cycle in the resulting graph as back edges. + */ +private predicate irreducibleSccEdge(SemBasicBlock b1, SemBasicBlock b2) { + trimmedEdge(b1, b2) and trimmedEdge+(b2, b1) } -private predicate trimmedEdges(SemBasicBlock pred, SemBasicBlock succ) { +private predicate trimmedEdge(SemBasicBlock pred, SemBasicBlock succ) { pred.getASuccessor() = succ and not succ.bbDominates(pred) } From dcb349434cf65b1d5bfe2c390ebfd24e01dab2f3 Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Mon, 26 Jun 2023 15:52:32 -0400 Subject: [PATCH 3/3] C++: fix comment formatting --- .../rangeanalysis/new/internal/semantic/SemanticSSA.qll | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll index a02760a9f2a..1a5a30d1454 100644 --- a/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll +++ b/cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticSSA.qll @@ -78,12 +78,13 @@ predicate semBackEdge(SemSsaPhiNode phi, SemSsaVariable inp, SemSsaReadPositionP /** * Holds if the edge from b1 to b2 is part of a multiple-entry cycle in an irreducible control flow * graph. - * + * * An ireducible control flow graph is one where the usual dominance-based back edge detection does * not work, because there is a cycle with multiple entry points, meaning there are * mutually-reachable basic blocks where neither dominates the other. For such a graph, we first - * all detectable back-edges using the normal condition that the predecessor block is dominated by - * the successor block, then mark all edges in a cycle in the resulting graph as back edges. + * remove all detectable back-edges using the normal condition that the predecessor block is + * dominated by the successor block, then mark all edges in a cycle in the resulting graph as back + * edges. */ private predicate irreducibleSccEdge(SemBasicBlock b1, SemBasicBlock b2) { trimmedEdge(b1, b2) and trimmedEdge+(b2, b1)