Skip to content

Commit 1fb9d82

Browse files
committed
Fix false positives in RULE-0-0-1 from disconnected non-Stmt CFG nodes
The codeql/cpp-all library (particularly version 5.0.0) can produce orphan BasicBlock nodes in the CFG that are completely disconnected (no predecessors and no successors). These nodes represent expressions, variable accesses, or function name nodes rather than actual statements. When they appear in functions with virtual calls returning complex types (like expected<void, E>), the isReachable predicate cannot reach them and they get incorrectly flagged as unreachable statements. Add isDisconnectedNonStmtBlock predicate to filter out these CFG artifacts. The predicate specifically targets blocks that: - Have no predecessors AND no successors (completely isolated) - Are not function entry points - Are not Stmt instances (the rule is about unreachable *statements*) This preserves detection of legitimate unreachable statements (e.g., code after infinite loops or noreturn calls) which are Stmt nodes with zero predecessors but may have successors. Verified: eliminates ~1182 false positives on eclipse-score/communication codebase while all existing test cases continue to pass. Fixes #1143
1 parent 883a46f commit 1fb9d82

3 files changed

Lines changed: 55 additions & 1 deletion

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- `RULE-0-0-1` - `UnreachableStatement.ql`:
2+
- Fixed false positives caused by orphan BasicBlock nodes (disconnected from the CFG) that represent expressions rather than statements. These artifacts are generated by the GCC extractor for functions involving virtual calls returning `std::variant`-based types with non-trivially-destructible alternatives.

cpp/misra/src/rules/RULE-0-0-1/UnreachableStatement.ql

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,28 @@ predicate isReachable(BasicBlock bb) {
6161
)
6262
}
6363

64+
/**
65+
* Holds if `bb` is an orphan basic block that is disconnected from the CFG and
66+
* does not represent a statement. These are artifacts of the extractor/CFG
67+
* construction (e.g., variable access expressions, function name nodes, or
68+
* temporary object reuse nodes) and not genuine unreachable statements.
69+
* A block with no predecessors that is a Stmt is legitimately unreachable code
70+
* (e.g., code after an infinite loop or noreturn call).
71+
*/
72+
predicate isDisconnectedNonStmtBlock(BasicBlock bb) {
73+
not exists(bb.getAPredecessor()) and
74+
not exists(bb.getASuccessor()) and
75+
not bb = any(Function f).getEntryPoint() and
76+
not bb instanceof Stmt
77+
}
78+
6479
from BasicBlock bb
6580
where
6681
not isExcluded(bb, DeadCode3Package::unreachableStatementQuery()) and
6782
not isReachable(bb) and
6883
not isCompilerGenerated(bb) and
69-
not affectedByMacro(bb)
84+
not affectedByMacro(bb) and
85+
not isDisconnectedNonStmtBlock(bb)
7086
// Note that the location of a BasicBlock will in some cases have an incorrect end location, often
7187
// preceding the end and including live code. We cast the block to an `Element` to get locations
7288
// that are not broken.

cpp/misra/test/rules/RULE-0-0-1/test.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,4 +247,40 @@ void f12() {
247247
int l1 = 0; // NON-COMPLIANT
248248
}
249249
}
250+
}
251+
252+
// Regression test: virtual call returning variant-based expected type should not
253+
// produce false positives from disconnected CFG nodes. With real GCC headers and
254+
// codeql/cpp-all <= 5.0.0, the extractor creates orphan BasicBlock nodes (0
255+
// predecessors, 0 successors) for expressions inside functions that use virtual
256+
// calls returning std::variant-based types with non-trivially-destructible
257+
// alternatives (e.g. containing std::string). The isDisconnectedNonStmtBlock
258+
// predicate filters these artifacts. This test validates the pattern compiles
259+
// and is not flagged; the actual orphan block generation requires real stdlib.
260+
#include <variant>
261+
262+
struct TestError {
263+
int code;
264+
};
265+
266+
template <typename E> class test_expected_void {
267+
std::variant<TestError, E> storage_;
268+
269+
public:
270+
test_expected_void() {}
271+
bool has_value() const noexcept;
272+
};
273+
274+
class ITestService {
275+
public:
276+
virtual test_expected_void<TestError> DoWork(int x) const noexcept = 0;
277+
virtual ~ITestService() = default;
278+
};
279+
280+
bool f13(ITestService &svc) { // COMPLIANT
281+
const auto result = svc.DoWork(42);
282+
if (!result.has_value()) {
283+
return false; // COMPLIANT
284+
}
285+
return true; // COMPLIANT
250286
}

0 commit comments

Comments
 (0)