[MC/DC] Enable nested expressions (#125413)
A warning "contains an operation with a nested boolean expression." is no longer emitted. At the moment, split expressions are treated as individual Decisions.
This commit is contained in:
parent
a34e89fcb5
commit
faf3e06dc0
@ -210,6 +210,7 @@ Improvements to Clang's time-trace
|
||||
Improvements to Coverage Mapping
|
||||
--------------------------------
|
||||
|
||||
- [MC/DC] Nested expressions are handled as individual MC/DC expressions.
|
||||
- "Single byte coverage" now supports branch coverage and can be used
|
||||
together with ``-fcoverage-mcdc``.
|
||||
|
||||
|
||||
@ -515,13 +515,6 @@ requires 8 test vectors.
|
||||
Expressions such as ``((a0 && b0) || (a1 && b1) || ...)`` can cause the
|
||||
number of test vectors to increase exponentially.
|
||||
|
||||
Also, if a boolean expression is embedded in the nest of another boolean
|
||||
expression but separated by a non-logical operator, this is also not supported.
|
||||
For example, in ``x = (a && b && c && func(d && f))``, the ``d && f`` case
|
||||
starts a new boolean expression that is separated from the other conditions by
|
||||
the operator ``func()``. When this is encountered, a warning will be generated
|
||||
and the boolean expression will not be instrumented.
|
||||
|
||||
Switch statements
|
||||
-----------------
|
||||
|
||||
|
||||
@ -449,11 +449,6 @@ def warn_simdlen_must_fit_lanes
|
||||
"2048-bit, by steps of 128-bit)">,
|
||||
InGroup<SIMDLen>;
|
||||
|
||||
def warn_pgo_nested_boolean_expr
|
||||
: Warning<
|
||||
"unsupported MC/DC boolean expression; contains an operation with a "
|
||||
"nested boolean expression. Expression will not be covered">,
|
||||
InGroup<PGOCoverage>;
|
||||
def warn_pgo_condition_limit
|
||||
: Warning<"unsupported MC/DC boolean expression; number of conditions (%0) "
|
||||
"exceeds max (%1). Expression will not be covered">,
|
||||
|
||||
@ -233,10 +233,17 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
|
||||
/// The stacks are also used to find error cases and notify the user. A
|
||||
/// standard logical operator nest for a boolean expression could be in a form
|
||||
/// similar to this: "x = a && b && c && (d || f)"
|
||||
unsigned NumCond = 0;
|
||||
bool SplitNestedLogicalOp = false;
|
||||
SmallVector<const Stmt *, 16> NonLogOpStack;
|
||||
SmallVector<const BinaryOperator *, 16> LogOpStack;
|
||||
struct DecisionState {
|
||||
llvm::DenseSet<const Stmt *> Leaves; // Not BinOp
|
||||
const Expr *DecisionExpr; // Root
|
||||
bool Split; // In splitting with Leaves.
|
||||
|
||||
DecisionState() = delete;
|
||||
DecisionState(const Expr *E, bool Split = false)
|
||||
: DecisionExpr(E), Split(Split) {}
|
||||
};
|
||||
|
||||
SmallVector<DecisionState, 1> DecisionStack;
|
||||
|
||||
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
|
||||
bool dataTraverseStmtPre(Stmt *S) {
|
||||
@ -244,35 +251,28 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
|
||||
if (MCDCMaxCond == 0)
|
||||
return true;
|
||||
|
||||
/// At the top of the logical operator nest, reset the number of conditions,
|
||||
/// also forget previously seen split nesting cases.
|
||||
if (LogOpStack.empty()) {
|
||||
NumCond = 0;
|
||||
SplitNestedLogicalOp = false;
|
||||
}
|
||||
|
||||
if (const Expr *E = dyn_cast<Expr>(S)) {
|
||||
if (const auto *BinOp =
|
||||
dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
|
||||
BinOp && BinOp->isLogicalOp()) {
|
||||
/// Check for "split-nested" logical operators. This happens when a new
|
||||
/// boolean expression logical-op nest is encountered within an existing
|
||||
/// boolean expression, separated by a non-logical operator. For
|
||||
/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
|
||||
/// starts a new boolean expression that is separated from the other
|
||||
/// conditions by the operator foo(). Split-nested cases are not
|
||||
/// supported by MC/DC.
|
||||
SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
|
||||
|
||||
LogOpStack.push_back(BinOp);
|
||||
/// Mark "in splitting" when a leaf is met.
|
||||
if (!DecisionStack.empty()) {
|
||||
auto &StackTop = DecisionStack.back();
|
||||
if (!StackTop.Split) {
|
||||
if (StackTop.Leaves.contains(S)) {
|
||||
assert(!StackTop.Split);
|
||||
StackTop.Split = true;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
// Split
|
||||
assert(StackTop.Split);
|
||||
assert(!StackTop.Leaves.contains(S));
|
||||
}
|
||||
|
||||
/// Keep track of non-logical operators. These are OK as long as we don't
|
||||
/// encounter a new logical operator after seeing one.
|
||||
if (!LogOpStack.empty())
|
||||
NonLogOpStack.push_back(S);
|
||||
if (const auto *E = dyn_cast<Expr>(S)) {
|
||||
if (const auto *BinOp =
|
||||
dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
|
||||
BinOp && BinOp->isLogicalOp())
|
||||
DecisionStack.emplace_back(E);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
@ -281,41 +281,41 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
|
||||
// an AST Stmt node. MC/DC will use it to to signal when the top of a
|
||||
// logical operation (boolean expression) nest is encountered.
|
||||
bool dataTraverseStmtPost(Stmt *S) {
|
||||
/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
|
||||
if (MCDCMaxCond == 0)
|
||||
if (DecisionStack.empty())
|
||||
return true;
|
||||
|
||||
if (const Expr *E = dyn_cast<Expr>(S)) {
|
||||
const BinaryOperator *BinOp =
|
||||
dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
|
||||
if (BinOp && BinOp->isLogicalOp()) {
|
||||
assert(LogOpStack.back() == BinOp);
|
||||
LogOpStack.pop_back();
|
||||
/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
|
||||
assert(MCDCMaxCond > 0);
|
||||
|
||||
/// At the top of logical operator nest:
|
||||
if (LogOpStack.empty()) {
|
||||
/// Was the "split-nested" logical operator case encountered?
|
||||
if (SplitNestedLogicalOp) {
|
||||
Diag.Report(S->getBeginLoc(), diag::warn_pgo_nested_boolean_expr);
|
||||
return true;
|
||||
}
|
||||
auto &StackTop = DecisionStack.back();
|
||||
|
||||
/// Was the maximum number of conditions encountered?
|
||||
if (NumCond > MCDCMaxCond) {
|
||||
Diag.Report(S->getBeginLoc(), diag::warn_pgo_condition_limit)
|
||||
<< NumCond << MCDCMaxCond;
|
||||
return true;
|
||||
}
|
||||
|
||||
// Otherwise, allocate the Decision.
|
||||
MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size();
|
||||
}
|
||||
return true;
|
||||
if (StackTop.DecisionExpr != S) {
|
||||
if (StackTop.Leaves.contains(S)) {
|
||||
assert(StackTop.Split);
|
||||
StackTop.Split = false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!LogOpStack.empty())
|
||||
NonLogOpStack.pop_back();
|
||||
/// Allocate the entry (with Valid=false)
|
||||
auto &DecisionEntry =
|
||||
MCDCState
|
||||
.DecisionByStmt[CodeGenFunction::stripCond(StackTop.DecisionExpr)];
|
||||
|
||||
/// Was the maximum number of conditions encountered?
|
||||
auto NumCond = StackTop.Leaves.size();
|
||||
if (NumCond > MCDCMaxCond) {
|
||||
Diag.Report(S->getBeginLoc(), diag::warn_pgo_condition_limit)
|
||||
<< NumCond << MCDCMaxCond;
|
||||
DecisionStack.pop_back();
|
||||
return true;
|
||||
}
|
||||
|
||||
// The Decision is validated.
|
||||
DecisionEntry.ID = MCDCState.DecisionByStmt.size() - 1;
|
||||
|
||||
DecisionStack.pop_back();
|
||||
|
||||
return true;
|
||||
}
|
||||
@ -327,14 +327,17 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
|
||||
/// order to use MC/DC, count the number of total LHS and RHS conditions.
|
||||
bool VisitBinaryOperator(BinaryOperator *S) {
|
||||
if (S->isLogicalOp()) {
|
||||
if (CodeGenFunction::isInstrumentedCondition(S->getLHS()))
|
||||
NumCond++;
|
||||
if (CodeGenFunction::isInstrumentedCondition(S->getLHS())) {
|
||||
if (!DecisionStack.empty())
|
||||
DecisionStack.back().Leaves.insert(S->getLHS());
|
||||
}
|
||||
|
||||
if (CodeGenFunction::isInstrumentedCondition(S->getRHS())) {
|
||||
if (ProfileVersion >= llvm::IndexedInstrProf::Version7)
|
||||
CounterMap[S->getRHS()] = NextCounter++;
|
||||
|
||||
NumCond++;
|
||||
if (!DecisionStack.empty())
|
||||
DecisionStack.back().Leaves.insert(S->getRHS());
|
||||
}
|
||||
}
|
||||
return Base::VisitBinaryOperator(S);
|
||||
|
||||
@ -1,20 +1,42 @@
|
||||
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2> %t.stderr.txt | FileCheck %s
|
||||
// RUN: FileCheck %s --check-prefix=WARN < %t.stderr.txt
|
||||
// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -fcoverage-mcdc -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only %s 2>&1 | FileCheck %s
|
||||
|
||||
// CHECK-NOT: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
|
||||
|
||||
// "Split-nest" -- boolean expressions within boolean expressions.
|
||||
extern bool bar(bool);
|
||||
// CHECK: func_split_nest{{.*}}:
|
||||
bool func_split_nest(bool a, bool b, bool c, bool d, bool e, bool f, bool g) {
|
||||
// WARN: :[[@LINE+1]]:14: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
|
||||
bool res = a && b && c && bar(d && e) && f && g;
|
||||
// CHECK: Decision,File 0, [[@LINE-1]]:14 -> [[#L:@LINE-1]]:50 = M:10, C:6
|
||||
// CHECK: Branch,File 0, [[#L]]:14 -> [[#L]]:15 = #9, (#0 - #9) [1,6,0]
|
||||
// CHECK: Branch,File 0, [[#L]]:19 -> [[#L]]:20 = #10, (#9 - #10) [6,5,0]
|
||||
// CHECK: Branch,File 0, [[#L]]:24 -> [[#L]]:25 = #8, (#7 - #8) [5,4,0]
|
||||
// CHECK: Branch,File 0, [[#L]]:29 -> [[#L]]:40 = #6, (#5 - #6) [4,3,0]
|
||||
|
||||
// The inner expr -- "d && e" (w/o parentheses)
|
||||
// CHECK: Decision,File 0, [[#L]]:33 -> [[#L]]:39 = M:3, C:2
|
||||
// CHECK: Branch,File 0, [[#L]]:33 -> [[#L]]:34 = #11, (#5 - #11) [1,2,0]
|
||||
// CHECK: Branch,File 0, [[#L]]:38 -> [[#L]]:39 = #12, (#11 - #12) [2,0,0]
|
||||
|
||||
// CHECK: Branch,File 0, [[#L]]:44 -> [[#L]]:45 = #4, (#3 - #4) [3,2,0]
|
||||
// CHECK: Branch,File 0, [[#L]]:49 -> [[#L]]:50 = #2, (#1 - #2) [2,0,0]
|
||||
return bar(res);
|
||||
}
|
||||
|
||||
// The inner expr begins with the same Loc as the outer expr
|
||||
// CHECK: func_condop{{.*}}:
|
||||
bool func_condop(bool a, bool b, bool c) {
|
||||
// WARN: :[[@LINE+1]]:10: warning: unsupported MC/DC boolean expression; contains an operation with a nested boolean expression.
|
||||
return (a && b ? true : false) && c;
|
||||
// CHECK: Decision,File 0, [[@LINE-1]]:10 -> [[#L:@LINE-1]]:38 = M:6, C:2
|
||||
// This covers outer parenthses.
|
||||
// CHECK: Branch,File 0, [[#L]]:10 -> [[#L]]:33 = #1, (#0 - #1) [1,2,0]
|
||||
|
||||
// The inner expr "a && b" (w/o parenthses)
|
||||
// CHECK: Decision,File 0, [[#L]]:11 -> [[#L]]:17 = M:3, C:2
|
||||
// CHECK: Branch,File 0, [[#L]]:11 -> [[#L]]:12 = #4, (#0 - #4) [1,2,0]
|
||||
// CHECK: Branch,File 0, [[#L]]:16 -> [[#L]]:17 = #5, (#4 - #5) [2,0,0]
|
||||
|
||||
// CHECK: Branch,File 0, [[#L]]:37 -> [[#L]]:38 = #2, (#1 - #2) [2,0,0]
|
||||
}
|
||||
|
||||
// __builtin_expect
|
||||
|
||||
@ -1,9 +1,9 @@
|
||||
// RUN: %clang_cc1 -emit-llvm-only -fprofile-instrument=clang -fcoverage-mcdc -Werror -Wno-error=pgo-coverage -Wno-unused-value %s -verify
|
||||
// RUN: %clang_cc1 -emit-llvm-only -fprofile-instrument=clang -fcoverage-mcdc -Werror -Wno-error=pgo-coverage -Wno-unused-value %s -verify -fmcdc-max-conditions=2
|
||||
|
||||
int foo(int x);
|
||||
|
||||
int main(void) {
|
||||
int a, b, c;
|
||||
a && foo( b && c ); // expected-warning{{unsupported MC/DC boolean expression; contains an operation with a nested boolean expression. Expression will not be covered}}
|
||||
a && foo( a && b && c ); // expected-warning{{unsupported MC/DC boolean expression; number of conditions (3) exceeds max (2). Expression will not be covered}}
|
||||
return 0;
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user