Author: NAKAMURA Takumi Date: 2026-01-31T13:31:02+09:00 New Revision: faf3e06dc0314b9d02c9fdfbc47b7c1a45a1d5b9
URL: https://github.com/llvm/llvm-project/commit/faf3e06dc0314b9d02c9fdfbc47b7c1a45a1d5b9 DIFF: https://github.com/llvm/llvm-project/commit/faf3e06dc0314b9d02c9fdfbc47b7c1a45a1d5b9.diff LOG: [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. Added: Modified: clang/docs/ReleaseNotes.rst clang/docs/SourceBasedCodeCoverage.rst clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/lib/CodeGen/CodeGenPGO.cpp clang/test/CoverageMapping/mcdc-nested-expr.cpp clang/test/Frontend/custom-diag-werror-interaction.c Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index de155256999d0..2e7a9fff3161b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -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``. diff --git a/clang/docs/SourceBasedCodeCoverage.rst b/clang/docs/SourceBasedCodeCoverage.rst index 2f114070a8fb2..395f2bcc85ef0 100644 --- a/clang/docs/SourceBasedCodeCoverage.rst +++ b/clang/docs/SourceBasedCodeCoverage.rst @@ -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 ----------------- diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index e2b257ceae80d..5c62bb70ebd0f 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -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">, diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 41bdf4f81cd94..4921eba7934a2 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -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,36 +251,29 @@ 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; + /// 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)); } - if (const Expr *E = dyn_cast<Expr>(S)) { + if (const auto *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); - return true; - } + BinOp && BinOp->isLogicalOp()) + DecisionStack.emplace_back(E); } - /// 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); - 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(); - - /// 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; - } - - /// 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 MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing. + assert(MCDCMaxCond > 0); + + auto &StackTop = DecisionStack.back(); + + 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); diff --git a/clang/test/CoverageMapping/mcdc-nested-expr.cpp b/clang/test/CoverageMapping/mcdc-nested-expr.cpp index f393919a91892..5cf4654c0b215 100644 --- a/clang/test/CoverageMapping/mcdc-nested-expr.cpp +++ b/clang/test/CoverageMapping/mcdc-nested-expr.cpp @@ -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 diff --git a/clang/test/Frontend/custom-diag-werror-interaction.c b/clang/test/Frontend/custom-diag-werror-interaction.c index c1480ae4c6681..9a9214379b567 100644 --- a/clang/test/Frontend/custom-diag-werror-interaction.c +++ b/clang/test/Frontend/custom-diag-werror-interaction.c @@ -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; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
