[coverage] MCDC reports unrechable and uncoverable conditions

Author: Lambdaris

clang/docs/SourceBasedCodeCoverage.rst

@@ -517,6 +517,18 @@ 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.
 
+Similar as branch coverage, MCDC also is not tracked for constant folded conditions.
+Moreover it's even not tracked for conditions that can not effect outcomes of decisions
+due to other constants. These conditions will be shown as ``uncoverable`` or 
+``unreachable``, determined by whether they can be visited. For instance, in 
+``a || true || b``, value of the decision is always ``true`` by reason of the constant
+condition. Thus ``a`` can not lead the decision to ``false`` even though it could branch, 
+while ``b`` is always short-circuited. Hence ``a`` is shown as ``uncoverable``
+and ``b`` is marked as ``unreachable``. Statistics of MCDC does not count constant 
+conditions which do not vary or such conditions which make no difference on value of 
+decisions. If a decision is proved to no branch theoretically, it shows mark ``Folded``
+rather than coverage percent.
+
 Switch statements
 -----------------
 

llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h

@@ -384,6 +384,13 @@ struct MCDCRecord {
   /// are effectively ignored.
   enum CondState { MCDC_DontCare = -1, MCDC_False = 0, MCDC_True = 1 };
 
+  enum CondResult {
+    MCDC_Normal,
+    MCDC_Constant,
+    MCDC_Uncoverable,
+    MCDC_Unreachable
+  };
+
   /// Emulate SmallVector<CondState> with a pair of BitVector.
   ///
   ///          True  False DontCare (Impossible)
@@ -442,30 +449,39 @@ struct MCDCRecord {
   using TVPairMap = llvm::DenseMap<unsigned, TVRowPair>;
   using CondIDMap = llvm::DenseMap<unsigned, unsigned>;
   using LineColPairMap = llvm::DenseMap<unsigned, LineColPair>;
+  using ResultVector = llvm::SmallVector<CondResult>;
 
 private:
   CounterMappingRegion Region;
   TestVectors TV;
   TVPairMap IndependencePairs;
-  BoolVector Folded;
+  ResultVector CondResults;
   CondIDMap PosToID;
   LineColPairMap CondLoc;
 
 public:
   MCDCRecord(const CounterMappingRegion &Region, TestVectors &&TV,
-             TVPairMap &&IndependencePairs, BoolVector &&Folded,
+             TVPairMap &&IndependencePairs, ResultVector &&CondResults,
              CondIDMap &&PosToID, LineColPairMap &&CondLoc)
       : Region(Region), TV(std::move(TV)),
         IndependencePairs(std::move(IndependencePairs)),
-        Folded(std::move(Folded)), PosToID(std::move(PosToID)),
+        CondResults(std::move(CondResults)), PosToID(std::move(PosToID)),
         CondLoc(std::move(CondLoc)){};
 
   CounterMappingRegion getDecisionRegion() const { return Region; }
   unsigned getNumConditions() const {
     return Region.getDecisionParams().NumConditions;
   }
   unsigned getNumTestVectors() const { return TV.size(); }
-  bool isCondFolded(unsigned Condition) const { return Folded[Condition]; }
+  bool isCondCoverable(unsigned Condition) const {
+    return getCondResult(Condition) == CondResult::MCDC_Normal;
+  }
+  bool isCondConstant(unsigned Condition) const {
+    return getCondResult(Condition) == CondResult::MCDC_Constant;
+  }
+  CondResult getCondResult(unsigned Condition) const {
+    return CondResults[Condition];
+  }
 
   /// Return the evaluation of a condition (indicated by Condition) in an
   /// executed test vector (indicated by TestVectorIndex), which will be True,
@@ -505,20 +521,25 @@ struct MCDCRecord {
     return IndependencePairs[PosToID[Condition]];
   }
 
-  float getPercentCovered() const {
-    unsigned Folded = 0;
+  /// Return if the decision is coverable and percent of covered conditions.
+  /// Only coverable conditions are counted as denominator.
+  std::pair<bool, float> getPercentCovered() const {
+    unsigned Excluded = 0;
     unsigned Covered = 0;
     for (unsigned C = 0; C < getNumConditions(); C++) {
-      if (isCondFolded(C))
-        Folded++;
+      if (isCondConstant(C))
+        Excluded++;
       else if (isConditionIndependencePairCovered(C))
         Covered++;
     }
 
-    unsigned Total = getNumConditions() - Folded;
+    unsigned Total = getNumConditions() - Excluded;
     if (Total == 0)
-      return 0.0;
-    return (static_cast<double>(Covered) / static_cast<double>(Total)) * 100.0;
+      return {false, 0.0};
+    return {
+        true,
+        (static_cast<double>(Covered) / static_cast<double>(Total)) * 100.0,
+    };
   }
 
   std::string getConditionHeaderString(unsigned Condition) {
@@ -553,7 +574,7 @@ struct MCDCRecord {
     // Add individual condition values to the string.
     OS << "  " << TestVectorIndex + 1 << " { ";
     for (unsigned Condition = 0; Condition < NumConditions; Condition++) {
-      if (isCondFolded(Condition))
+      if (isCondConstant(Condition))
         OS << "C";
       else {
         switch (getTVCondition(TestVectorIndex, Condition)) {
@@ -589,14 +610,25 @@ struct MCDCRecord {
     std::ostringstream OS;
 
     OS << "  C" << Condition + 1 << "-Pair: ";
-    if (isCondFolded(Condition)) {
+    switch (getCondResult(Condition)) {
+    case CondResult::MCDC_Normal:
+      if (isConditionIndependencePairCovered(Condition)) {
+        TVRowPair rows = getConditionIndependencePair(Condition);
+        OS << "covered: (" << rows.first << ",";
+        OS << rows.second << ")\n";
+      } else
+        OS << "not covered\n";
+      break;
+    case CondResult::MCDC_Constant:
       OS << "constant folded\n";
-    } else if (isConditionIndependencePairCovered(Condition)) {
-      TVRowPair rows = getConditionIndependencePair(Condition);
-      OS << "covered: (" << rows.first << ",";
-      OS << rows.second << ")\n";
-    } else
-      OS << "not covered\n";
+      break;
+    case CondResult::MCDC_Uncoverable:
+      OS << "uncoverable\n";
+      break;
+    case CondResult::MCDC_Unreachable:
+      OS << "unreachable\n";
+      break;
+    }
 
     return OS.str();
   }

llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

@@ -365,11 +365,15 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
   unsigned NumConditions;
 
   /// Vector used to track whether a condition is constant folded.
-  MCDCRecord::BoolVector Folded;
+  MCDCRecord::ResultVector CondResults;
 
   /// Mapping of calculated MC/DC Independence Pairs for each condition.
   MCDCRecord::TVPairMap IndependencePairs;
 
+  /// All possible Test Vectors for the boolean expression derived from
+  /// binary decision diagran of the expression.
+  MCDCRecord::TestVectors TestVectors;
+
   /// Storage for ExecVectors
   /// ExecVectors is the alias of its 0th element.
   std::array<MCDCRecord::TestVectors, 2> ExecVectorsByCond;
@@ -395,8 +399,9 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
       : NextIDsBuilder(Branches), TVIdxBuilder(this->NextIDs), Bitmap(Bitmap),
         Region(Region), DecisionParams(Region.getDecisionParams()),
         Branches(Branches), NumConditions(DecisionParams.NumConditions),
-        Folded(NumConditions, false), IndependencePairs(NumConditions),
-        ExecVectors(ExecVectorsByCond[false]), IsVersion11(IsVersion11) {}
+        CondResults(NumConditions, MCDCRecord::CondResult::MCDC_Normal),
+        IndependencePairs(NumConditions), ExecVectors(ExecVectorsByCond[false]),
+        IsVersion11(IsVersion11) {}
 
 private:
   // Walk the binary decision diagram and try assigning both false and true to
@@ -418,6 +423,7 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
 
       assert(TVIdx < SavedNodes[ID].Width);
       assert(TVIdxs.insert(NextTVIdx).second && "Duplicate TVIdx");
+      TestVectors.push_back({TV, MCDCCond});
 
       if (!Bitmap[IsVersion11
                       ? DecisionParams.BitmapIdx * CHAR_BIT + TV.getIndex()
@@ -445,7 +451,6 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     buildTestVector(TV, 0, 0);
     assert(TVIdxs.size() == unsigned(NumTestVectors) &&
            "TVIdxs wasn't fulfilled");
-
     // Fill ExecVectors order by False items and True items.
     // ExecVectors is the alias of ExecVectorsByCond[false], so
     // Append ExecVectorsByCond[true] on it.
@@ -477,48 +482,130 @@ class MCDCRecordProcessor : NextIDsBuilder, mcdc::TVIdxBuilder {
     }
   }
 
+  void findCoverablePairs(const MCDCRecord::CondIDMap &PosToID) {
+    llvm::SmallVector<unsigned> FoldedCondPos;
+    for (unsigned I = 0; I < CondResults.size(); ++I) {
+      if (CondResults[I] == MCDCRecord::MCDC_Constant ||
+          CondResults[I] == MCDCRecord::MCDC_Unreachable) {
+        FoldedCondPos.push_back(I);
+      }
+    }
+    if (FoldedCondPos.empty()) {
+      return;
+    }
+    std::array<MCDCRecord::TestVectors, 2> PracticalTestVectorsByCond;
+    for (const auto &TVWithCond : TestVectors) {
+      const bool Practical =
+          llvm::all_of(FoldedCondPos, [&](const unsigned &Pos) {
+            const auto &[TV, Cond] = TVWithCond;
+            const auto ID = PosToID.at(Pos);
+            if (TV[ID] == MCDCRecord::MCDC_DontCare) {
+              return true;
+            }
+            if (CondResults[Pos] == MCDCRecord::MCDC_Constant) {
+              const auto ConstantValue = Branches[Pos]->Count.isZero()
+                                             ? MCDCRecord::MCDC_False
+                                             : MCDCRecord::MCDC_True;
+              if (TV[ID] == ConstantValue) {
+                return true;
+              }
+            }
+            return false;
+          });
+
+      if (Practical) {
+        PracticalTestVectorsByCond[TVWithCond.second].push_back(TVWithCond);
+      }
+    }
+
+    // If a condition:
+    // - is uncoverable, all test vectors in exact one element of
+    // `PracticalTestVectorsByCond` show it is `DontCare`;
+    // - is unreachable, all test vectors in both elements of
+    // `PracticalTestVectorsByCond` show it is `DontCare`;
+    //
+    // Otherwise, the condition is coverable as long as it has not been marked
+    // as constant or unreachable before.
+    for (unsigned Pos = 0; Pos < Branches.size(); ++Pos) {
+      if (CondResults[Pos] != MCDCRecord::MCDC_Normal) {
+        continue;
+      }
+      const auto ID = PosToID.at(Pos);
+      unsigned InaccessibleCondCount =
+          llvm::count_if(PracticalTestVectorsByCond,
+                         [=](const MCDCRecord::TestVectors &TestVectors) {
+                           for (const auto &[TV, Cond] : TestVectors) {
+                             if (TV[ID] != MCDCRecord::MCDC_DontCare) {
+                               return false;
+                             }
+                           }
+                           return true;
+                         });
+      switch (InaccessibleCondCount) {
+      case 1:
+        CondResults[Pos] = MCDCRecord::CondResult::MCDC_Uncoverable;
+        break;
+      case 2:
+        CondResults[Pos] = MCDCRecord::CondResult::MCDC_Unreachable;
+        break;
+      default:
+        break;
+      }
+    }
+  }
+
 public:
   /// Process the MC/DC Record in order to produce a result for a boolean
   /// expression. This process includes tracking the conditions that comprise
   /// the decision region, calculating the list of all possible test vectors,
   /// marking the executed test vectors, and then finding an Independence Pair
   /// out of the executed test vectors for each condition in the boolean
-  /// expression. A condition is tracked to ensure that its ID can be mapped to
-  /// its ordinal position in the boolean expression. The condition's source
-  /// location is also tracked, as well as whether it is constant folded (in
-  /// which case it is excuded from the metric).
+  /// expression. A condition is tracked to ensure that its ID can be mapped
+  /// to its ordinal position in the boolean expression. The condition's
+  /// source location is also tracked, as well as whether it is constant
+  /// folded (in which case it is excuded from the metric).
   MCDCRecord processMCDCRecord() {
     unsigned I = 0;
     MCDCRecord::CondIDMap PosToID;
     MCDCRecord::LineColPairMap CondLoc;
 
     // Walk the Record's BranchRegions (representing Conditions) in order to:
-    // - Hash the condition based on its corresponding ID. This will be used to
+    // - Hash the condition based on its corresponding ID. This will be used
+    // to
     //   calculate the test vectors.
     // - Keep a map of the condition's ordinal position (1, 2, 3, 4) to its
     //   actual ID.  This will be used to visualize the conditions in the
     //   correct order.
     // - Keep track of the condition source location. This will be used to
     //   visualize where the condition is.
-    // - Record whether the condition is constant folded so that we exclude it
+    // - Record whether the condition is folded so that we exclude it
     //   from being measured.
     for (const auto *B : Branches) {
       const auto &BranchParams = B->getBranchParams();
       PosToID[I] = BranchParams.ID;
       CondLoc[I] = B->startLoc();
-      Folded[I++] = (B->Count.isZero() || B->FalseCount.isZero());
+      if (B->Count.isZero() && B->FalseCount.isZero()) {
+        CondResults[I] = MCDCRecord::CondResult::MCDC_Unreachable;
+      } else if (B->Count.isZero() || B->FalseCount.isZero()) {
+        CondResults[I] = MCDCRecord::CondResult::MCDC_Constant;
+      }
+      ++I;
     }
 
     // Using Profile Bitmap from runtime, mark the executed test vectors.
     findExecutedTestVectors();
 
-    // Compare executed test vectors against each other to find an independence
-    // pairs for each condition.  This processing takes the most time.
+    // Compare executed test vectors against each other to find an
+    // independence pairs for each condition.  This processing takes the most
+    // time.
     findIndependencePairs();
 
+    // Identify all conditions making no difference on outcome of the decision.
+    findCoverablePairs(PosToID);
+
     // Record Test vectors, executed vectors, and independence pairs.
     return MCDCRecord(Region, std::move(ExecVectors),
-                      std::move(IndependencePairs), std::move(Folded),
+                      std::move(IndependencePairs), std::move(CondResults),
                       std::move(PosToID), std::move(CondLoc));
   }
 };
@@ -910,8 +997,8 @@ Error CoverageMapping::loadFunctionRecord(
   }
 
   // Don't create records for (filenames, function) pairs we've already seen.
-  auto FilenamesHash = hash_combine_range(Record.Filenames.begin(),
-                                          Record.Filenames.end());
+  auto FilenamesHash =
+      hash_combine_range(Record.Filenames.begin(), Record.Filenames.end());
   if (!RecordProvenance[FilenamesHash].insert(hash_value(OrigFuncName)).second)
     return Error::success();
 
@@ -961,12 +1048,11 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
 
 // If E is a no_data_found error, returns success. Otherwise returns E.
 static Error handleMaybeNoDataFoundError(Error E) {
-  return handleErrors(
-      std::move(E), [](const CoverageMapError &CME) {
-        if (CME.get() == coveragemap_error::no_data_found)
-          return static_cast<Error>(Error::success());
-        return make_error<CoverageMapError>(CME.get(), CME.getMessage());
-      });
+  return handleErrors(std::move(E), [](const CoverageMapError &CME) {
+    if (CME.get() == coveragemap_error::no_data_found)
+      return static_cast<Error>(Error::success());
+    return make_error<CoverageMapError>(CME.get(), CME.getMessage());
+  });
 }
 
 Error CoverageMapping::loadFromFile(
@@ -1058,7 +1144,7 @@ Expected<std::unique_ptr<CoverageMapping>> CoverageMapping::load(
         std::string Path = std::move(*PathOpt);
         StringRef Arch = Arches.size() == 1 ? Arches.front() : StringRef();
         if (Error E = loadFromFile(Path, Arch, CompilationDir, *ProfileReader,
-                                  *Coverage, DataFound))
+                                   *Coverage, DataFound))
           return std::move(E);
       } else if (CheckBinaryIDs) {
         return createFileError(
@@ -1152,9 +1238,9 @@ class SegmentBuilder {
     // emit closing segments in sorted order.
     auto CompletedRegionsIt = ActiveRegions.begin() + FirstCompletedRegion;
     std::stable_sort(CompletedRegionsIt, ActiveRegions.end(),
-                      [](const CountedRegion *L, const CountedRegion *R) {
-                        return L->endLoc() < R->endLoc();
-                      });
+                     [](const CountedRegion *L, const CountedRegion *R) {
+                       return L->endLoc() < R->endLoc();
+                     });
 
     // Emit segments for all completed regions.
     for (unsigned I = FirstCompletedRegion + 1, E = ActiveRegions.size(); I < E;