[MC/DC] Introduce -fmcdc-single-conditions to include also single conditions

clang/docs/ReleaseNotes.rst

@@ -118,6 +118,9 @@ Improvements to Coverage Mapping
 
 - [MC/DC] Nested expressions are handled as individual MC/DC expressions.
 
+- [MC/DC] Non-boolean expressions on conditions can be included with
+  `-fmcdc-single-conditions`. (#GH95336)
+
 Bug Fixes in This Version
 -------------------------
 

clang/docs/SourceBasedCodeCoverage.rst

@@ -510,6 +510,10 @@ requires 8 test vectors.
 Expressions such as ``((a0 && b0) || (a1 && b1) || ...)`` can cause the
 number of test vectors to increase exponentially.
 
+Clang handles only binary logical operators as MC/DC coverage. Single
+conditions without logcal operators on `do/for/while/if/?!` can be
+included with `-Xclang -fmcdc-single-conditions`.
+
 Switch statements
 -----------------
 

clang/include/clang/Basic/CodeGenOptions.def

@@ -236,6 +236,7 @@ CODEGENOPT(DumpCoverageMapping , 1, 0) ///< Dump the generated coverage mapping
 CODEGENOPT(MCDCCoverage , 1, 0) ///< Enable MC/DC code coverage criteria.
 VALUE_CODEGENOPT(MCDCMaxConds, 16, 32767) ///< MC/DC Maximum conditions.
 VALUE_CODEGENOPT(MCDCMaxTVs, 32, 0x7FFFFFFE) ///< MC/DC Maximum test vectors.
+VALUE_CODEGENOPT(MCDCSingleCond, 1, 0) ///< Enable MC/DC single conditions.
 
   /// If -fpcc-struct-return or -freg-struct-return is specified.
 ENUM_CODEGENOPT(StructReturnConvention, StructReturnConventionKind, 2, SRCK_Default)

clang/include/clang/Driver/Options.td

@@ -1742,6 +1742,10 @@ def fmcdc_max_test_vectors_EQ : Joined<["-"], "fmcdc-max-test-vectors=">,
   Group<f_Group>, Visibility<[CC1Option]>,
   HelpText<"Maximum number of test vectors in MC/DC coverage">,
   MarshallingInfoInt<CodeGenOpts<"MCDCMaxTVs">, "0x7FFFFFFE">;
+def fmcdc_single_conditions : Flag<["-"], "fmcdc-single-conditions">,
+  Group<f_Group>, Visibility<[CC1Option]>,
+  HelpText<"Include also single conditions as MC/DC coverage">,
+  MarshallingInfoFlag<CodeGenOpts<"MCDCSingleCond">>;
 def fprofile_generate : Flag<["-"], "fprofile-generate">,
     Group<f_Group>, Visibility<[ClangOption, CLOption]>,
     HelpText<"Generate instrumented code to collect execution counts into default.profraw (overridden by LLVM_PROFILE_FILE env var)">;

clang/lib/CodeGen/CGExpr.cpp

@@ -196,20 +196,36 @@ RawAddress CodeGenFunction::CreateMemTempWithoutCast(QualType Ty,
 /// EvaluateExprAsBool - Perform the usual unary conversions on the specified
 /// expression and compare the result against zero, returning an Int1Ty value.
 llvm::Value *CodeGenFunction::EvaluateExprAsBool(const Expr *E) {
+  auto DecisionExpr = stripCond(E);
+  if (isMCDCDecisionExpr(DecisionExpr) && isInstrumentedCondition(DecisionExpr))
+    maybeResetMCDCCondBitmap(DecisionExpr);
+  else
+    DecisionExpr = nullptr;
+
   PGO.setCurrentStmt(E);
+  llvm::Value *Result;
   if (const MemberPointerType *MPT = E->getType()->getAs<MemberPointerType>()) {
     llvm::Value *MemPtr = EmitScalarExpr(E);
-    return CGM.getCXXABI().EmitMemberPointerIsNotNull(*this, MemPtr, MPT);
+    Result = CGM.getCXXABI().EmitMemberPointerIsNotNull(*this, MemPtr, MPT);
+  } else {
+    QualType BoolTy = getContext().BoolTy;
+    SourceLocation Loc = E->getExprLoc();
+    CGFPOptionsRAII FPOptsRAII(*this, E);
+    if (!E->getType()->isAnyComplexType())
+      Result =
+          EmitScalarConversion(EmitScalarExpr(E), E->getType(), BoolTy, Loc);
+    else
+      Result = EmitComplexToScalarConversion(EmitComplexExpr(E), E->getType(),
+                                             BoolTy, Loc);
   }
 
-  QualType BoolTy = getContext().BoolTy;
-  SourceLocation Loc = E->getExprLoc();
-  CGFPOptionsRAII FPOptsRAII(*this, E);
-  if (!E->getType()->isAnyComplexType())
-    return EmitScalarConversion(EmitScalarExpr(E), E->getType(), BoolTy, Loc);
+  if (DecisionExpr) {
+    if (isMCDCBranchExpr(stripCond(E)))
+      maybeUpdateMCDCCondBitmap(stripCond(E), Result);
+    maybeUpdateMCDCTestVectorBitmap(DecisionExpr);
+  }
 
-  return EmitComplexToScalarConversion(EmitComplexExpr(E), E->getType(), BoolTy,
-                                       Loc);
+  return Result;
 }
 
 /// EmitIgnoredExpr - Emit code to compute the specified expression,

clang/lib/CodeGen/CodeGenFunction.h

@@ -1684,7 +1684,7 @@ class CodeGenFunction : public CodeGenTypeCache {
 
   /// Zero-init the MCDC temp value.
   void maybeResetMCDCCondBitmap(const Expr *E) {
-    if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) {
+    if (isMCDCCoverageEnabled()) {
       PGO.emitMCDCCondBitmapReset(Builder, E);
       PGO.setCurrentStmt(E);
     }
@@ -1693,7 +1693,7 @@ class CodeGenFunction : public CodeGenTypeCache {
   /// Increment the profiler's counter for the given expression by \p StepV.
   /// If \p StepV is null, the default increment is 1.
   void maybeUpdateMCDCTestVectorBitmap(const Expr *E) {
-    if (isMCDCCoverageEnabled() && isBinaryLogicalOp(E)) {
+    if (isMCDCCoverageEnabled()) {
       PGO.emitMCDCTestVectorBitmapUpdate(Builder, E, *this);
       PGO.setCurrentStmt(E);
     }

clang/lib/CodeGen/CodeGenPGO.cpp

@@ -168,6 +168,8 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
   MCDC::State &MCDCState;
   /// Maximum number of supported MC/DC conditions in a boolean expression.
   unsigned MCDCMaxCond;
+  /// Take single conditions into account.
+  bool MCDCSingleCond;
   /// The profile version.
   uint64_t ProfileVersion;
   /// Diagnostics Engine used to report warnings.
@@ -176,10 +178,11 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
   MapRegionCounters(PGOHashVersion HashVersion, uint64_t ProfileVersion,
                     llvm::DenseMap<const Stmt *, CounterPair> &CounterMap,
                     MCDC::State &MCDCState, unsigned MCDCMaxCond,
-                    DiagnosticsEngine &Diag)
+                    bool MCDCSingleCond, DiagnosticsEngine &Diag)
       : NextCounter(0), Hash(HashVersion), CounterMap(CounterMap),
         MCDCState(MCDCState), MCDCMaxCond(MCDCMaxCond),
-        ProfileVersion(ProfileVersion), Diag(Diag) {}
+        MCDCSingleCond(MCDCSingleCond), ProfileVersion(ProfileVersion),
+        Diag(Diag) {}
 
   // Blocks and lambdas are handled as separate functions, so we need not
   // traverse them in the parent context.
@@ -240,12 +243,31 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
 
   SmallVector<DecisionState, 1> DecisionStack;
 
+  llvm::DenseSet<const Expr *> StagingDecisions;
+
+  template <class T> bool pushInstrumentedCond(Stmt *S) {
+    if (auto *St = dyn_cast<T>(S)) {
+      if (auto *Cond = St->getCond();
+          Cond && CodeGenFunction::isInstrumentedCondition(Cond)) {
+        StagingDecisions.insert(CodeGenFunction::stripCond(Cond));
+        return true;
+      }
+    }
+
+    return false;
+  }
+
   // Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
   bool dataTraverseStmtPre(Stmt *S) {
     /// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
     if (MCDCMaxCond == 0)
       return true;
 
+    const auto *E = dyn_cast<Expr>(S);
+
+    if (StagingDecisions.contains(E))
+      DecisionStack.emplace_back(E, true);
+
     /// Mark "in splitting" when a leaf is met.
     if (!DecisionStack.empty()) {
       auto &StackTop = DecisionStack.back();
@@ -262,13 +284,20 @@ struct MapRegionCounters : public RecursiveASTVisitor<MapRegionCounters> {
       assert(!StackTop.Leaves.contains(S));
     }
 
-    if (const auto *E = dyn_cast<Expr>(S)) {
+    if (E) {
       if (const auto *BinOp =
               dyn_cast<BinaryOperator>(CodeGenFunction::stripCond(E));
           BinOp && BinOp->isLogicalOp())
         DecisionStack.emplace_back(E);
     }
 
+    if (MCDCSingleCond) {
+      pushInstrumentedCond<AbstractConditionalOperator>(S) ||
+          pushInstrumentedCond<DoStmt>(S) || pushInstrumentedCond<IfStmt>(S) ||
+          pushInstrumentedCond<ForStmt>(S) ||
+          pushInstrumentedCond<WhileStmt>(S);
+    }
+
     return true;
   }
 
@@ -1098,7 +1127,8 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) {
   RegionCounterMap.reset(new llvm::DenseMap<const Stmt *, CounterPair>);
   RegionMCDCState.reset(new MCDC::State);
   MapRegionCounters Walker(HashVersion, ProfileVersion, *RegionCounterMap,
-                           *RegionMCDCState, MCDCMaxConditions, CGM.getDiags());
+                           *RegionMCDCState, MCDCMaxConditions,
+                           CGM.getCodeGenOpts().MCDCSingleCond, CGM.getDiags());
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
     Walker.TraverseDecl(const_cast<FunctionDecl *>(FD));
   else if (const ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(D))

clang/lib/CodeGen/CoverageMappingGen.cpp

@@ -1164,9 +1164,7 @@ struct CounterCoverageMappingBuilder
   /// result in the generation of a branch.
   void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt,
                           const mcdc::ConditionIDs &Conds = {}) {
-    // Check for NULL conditions.
-    if (!C)
-      return;
+    assert(C && "Condition Expr shouldn't be null!");
 
     // Ensure we are an instrumentable condition (i.e. no "&&" or "||").  Push
     // region onto RegionStack but immediately pop it (which adds it to the
@@ -1630,6 +1628,9 @@ struct CounterCoverageMappingBuilder
   }
 
   void VisitWhileStmt(const WhileStmt *S) {
+    unsigned SourceRegionsSince = SourceRegions.size();
+    MCDCBuilder.checkDecisionRootOrPush(S->getCond());
+
     extendRegion(S);
 
     Counter ParentCount = getRegion().getCounter();
@@ -1676,10 +1677,15 @@ struct CounterCoverageMappingBuilder
 
     // Create Branch Region around condition.
     if (!llvm::EnableSingleByteCoverage)
-      createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
+      createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped,
+                         MCDCBuilder.getCurCondIDs());
+    createOrCancelDecision(S->getCond(), SourceRegionsSince);
   }
 
   void VisitDoStmt(const DoStmt *S) {
+    unsigned SourceRegionsSince = SourceRegions.size();
+    MCDCBuilder.checkDecisionRootOrPush(S->getCond());
+
     extendRegion(S);
 
     Counter ParentCount = getRegion().getCounter();
@@ -1722,13 +1728,20 @@ struct CounterCoverageMappingBuilder
 
     // Create Branch Region around condition.
     if (!llvm::EnableSingleByteCoverage)
-      createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
+      createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped,
+                         MCDCBuilder.getCurCondIDs());
+    createOrCancelDecision(S->getCond(), SourceRegionsSince);
 
     if (BodyHasTerminateStmt)
       HasTerminateStmt = true;
   }
 
   void VisitForStmt(const ForStmt *S) {
+    const Expr *Cond = S->getCond();
+    unsigned SourceRegionsSince = SourceRegions.size();
+    if (Cond)
+      MCDCBuilder.checkDecisionRootOrPush(Cond);
+
     extendRegion(S);
     if (S->getInit())
       Visit(S->getInit());
@@ -1775,7 +1788,7 @@ struct CounterCoverageMappingBuilder
     assert(BranchCount.Executed.isZero() || BranchCount.Executed == BodyCount ||
            llvm::EnableSingleByteCoverage);
 
-    if (const Expr *Cond = S->getCond()) {
+    if (Cond) {
       propagateCounts(CondCount, Cond);
       adjustForOutOfOrderTraversal(getEnd(S));
     }
@@ -1797,8 +1810,11 @@ struct CounterCoverageMappingBuilder
     }
 
     // Create Branch Region around condition.
-    if (!llvm::EnableSingleByteCoverage)
-      createBranchRegion(S->getCond(), BodyCount, BranchCount.Skipped);
+    if (!llvm::EnableSingleByteCoverage && Cond) {
+      createBranchRegion(Cond, BodyCount, BranchCount.Skipped,
+                         MCDCBuilder.getCurCondIDs());
+      createOrCancelDecision(Cond, SourceRegionsSince);
+    }
   }
 
   void VisitCXXForRangeStmt(const CXXForRangeStmt *S) {
@@ -2069,6 +2085,9 @@ struct CounterCoverageMappingBuilder
     else if (S->isConstexpr())
       return coverIfConstexpr(S);
 
+    unsigned SourceRegionsSince = SourceRegions.size();
+    MCDCBuilder.checkDecisionRootOrPush(S->getCond());
+
     extendRegion(S);
     if (S->getInit())
       Visit(S->getInit());
@@ -2127,7 +2146,9 @@ struct CounterCoverageMappingBuilder
 
     if (!llvm::EnableSingleByteCoverage)
       // Create Branch Region around condition.
-      createBranchRegion(S->getCond(), ThenCount, ElseCount);
+      createBranchRegion(S->getCond(), ThenCount, ElseCount,
+                         MCDCBuilder.getCurCondIDs());
+    createOrCancelDecision(S->getCond(), SourceRegionsSince);
   }
 
   void VisitCXXTryStmt(const CXXTryStmt *S) {
@@ -2150,6 +2171,9 @@ struct CounterCoverageMappingBuilder
   }
 
   void VisitAbstractConditionalOperator(const AbstractConditionalOperator *E) {
+    unsigned SourceRegionsSince = SourceRegions.size();
+    MCDCBuilder.checkDecisionRootOrPush(E->getCond());
+
     extendRegion(E);
 
     Counter ParentCount = getRegion().getCounter();
@@ -2189,7 +2213,9 @@ struct CounterCoverageMappingBuilder
 
     // Create Branch Region around condition.
     if (!llvm::EnableSingleByteCoverage)
-      createBranchRegion(E->getCond(), TrueCount, FalseCount);
+      createBranchRegion(E->getCond(), TrueCount, FalseCount,
+                         MCDCBuilder.getCurCondIDs());
+    createOrCancelDecision(E->getCond(), SourceRegionsSince);
   }
 
   inline unsigned findMCDCBranchesInSourceRegion(