diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp index 916016601a932..4a44d113ddec9 100644 --- a/clang/lib/CodeGen/CoverageMappingGen.cpp +++ b/clang/lib/CodeGen/CoverageMappingGen.cpp @@ -573,6 +573,11 @@ struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder { /// creation. struct MCDCCoverageBuilder { + struct DecisionIDPair { + MCDCConditionID TrueID = 0; + MCDCConditionID FalseID = 0; + }; + /// The AST walk recursively visits nested logical-AND or logical-OR binary /// operator nodes and then visits their LHS and RHS children nodes. As this /// happens, the algorithm will assign IDs to each operator's LHS and RHS side @@ -616,14 +621,14 @@ struct MCDCCoverageBuilder { /// /// A node ID of '0' always means MC/DC isn't being tracked. /// - /// As the AST walk proceeds recursively, the algorithm will also use stacks + /// As the AST walk proceeds recursively, the algorithm will also use a stack /// to track the IDs of logical-AND and logical-OR operations on the RHS so /// that it can be determined which nodes are executed next, depending on how /// a LHS or RHS of a logical-AND or logical-OR is evaluated. This /// information relies on the assigned IDs and are embedded within the /// coverage region IDs of each branch region associated with a leaf-level /// condition. This information helps the visualization tool reconstruct all - /// possible test vectors for the purposes of MC/DC analysis. if a "next" node + /// possible test vectors for the purposes of MC/DC analysis. If a "next" node /// ID is '0', it means it's the end of the test vector. The following rules /// are used: /// @@ -663,54 +668,40 @@ struct MCDCCoverageBuilder { private: CodeGenModule &CGM; - llvm::SmallVector AndRHS; - llvm::SmallVector OrRHS; - llvm::SmallVector NestLevel; + llvm::SmallVector DecisionStack; llvm::DenseMap &CondIDs; llvm::DenseMap &MCDCBitmapMap; MCDCConditionID NextID = 1; bool NotMapped = false; + /// Represent a sentinel value of [0,0] for the bottom of DecisionStack. + static constexpr DecisionIDPair DecisionStackSentinel{0, 0}; + /// Is this a logical-AND operation? bool isLAnd(const BinaryOperator *E) const { return E->getOpcode() == BO_LAnd; } - /// Push an ID onto the corresponding RHS stack. - void pushRHS(const BinaryOperator *E) { - llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS; - rhs.push_back(CondIDs[CodeGenFunction::stripCond(E->getRHS())]); - } - - /// Pop an ID from the corresponding RHS stack. - void popRHS(const BinaryOperator *E) { - llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS; - if (!rhs.empty()) - rhs.pop_back(); - } - - /// If the expected ID is on top, pop it off the corresponding RHS stack. - void popRHSifTop(const BinaryOperator *E) { - if (!OrRHS.empty() && CondIDs[E] == OrRHS.back()) - OrRHS.pop_back(); - else if (!AndRHS.empty() && CondIDs[E] == AndRHS.back()) - AndRHS.pop_back(); - } - public: MCDCCoverageBuilder(CodeGenModule &CGM, llvm::DenseMap &CondIDMap, llvm::DenseMap &MCDCBitmapMap) - : CGM(CGM), CondIDs(CondIDMap), MCDCBitmapMap(MCDCBitmapMap) {} + : CGM(CGM), DecisionStack(1, DecisionStackSentinel), CondIDs(CondIDMap), + MCDCBitmapMap(MCDCBitmapMap) {} - /// Return the ID of the RHS of the next, upper nest-level logical-OR. - MCDCConditionID getNextLOrCondID() const { - return OrRHS.empty() ? 0 : OrRHS.back(); - } + /// Return whether the build of the control flow map is at the top-level + /// (root) of a logical operator nest in a boolean expression prior to the + /// assignment of condition IDs. + bool isIdle() const { return (NextID == 1 && !NotMapped); } + + /// Return whether any IDs have been assigned in the build of the control + /// flow map, indicating that the map is being generated for this boolean + /// expression. + bool isBuilding() const { return (NextID > 1); } - /// Return the ID of the RHS of the next, upper nest-level logical-AND. - MCDCConditionID getNextLAndCondID() const { - return AndRHS.empty() ? 0 : AndRHS.back(); + /// Set the given condition's ID. + void setCondID(const Expr *Cond, MCDCConditionID ID) { + CondIDs[CodeGenFunction::stripCond(Cond)] = ID; } /// Return the ID of a given condition. @@ -722,6 +713,9 @@ struct MCDCCoverageBuilder { return I->second; } + /// Return the LHS Decision ([0,0] if not set). + const DecisionIDPair &back() const { return DecisionStack.back(); } + /// Push the binary operator statement to track the nest level and assign IDs /// to the operator's LHS and RHS. The RHS may be a larger subtree that is /// broken up on successive levels. @@ -730,68 +724,67 @@ struct MCDCCoverageBuilder { return; // If binary expression is disqualified, don't do mapping. - if (NestLevel.empty() && - !MCDCBitmapMap.contains(CodeGenFunction::stripCond(E))) + if (!isBuilding() && !MCDCBitmapMap.contains(CodeGenFunction::stripCond(E))) NotMapped = true; - // Push Stmt on 'NestLevel' stack to keep track of nest location. - NestLevel.push_back(E); - // Don't go any further if we don't need to map condition IDs. if (NotMapped) return; + const DecisionIDPair &ParentDecision = DecisionStack.back(); + // If the operator itself has an assigned ID, this means it represents a - // larger subtree. In this case, pop its ID out of the RHS stack and - // assign that ID to its LHS node. Its RHS will receive a new ID. - if (CondIDs.contains(CodeGenFunction::stripCond(E))) { - // If Stmt has an ID, assign its ID to LHS - CondIDs[CodeGenFunction::stripCond(E->getLHS())] = CondIDs[E]; - - // Since the operator's LHS assumes the operator's same ID, pop the - // operator from the RHS stack so that if LHS short-circuits, it won't be - // incorrectly re-used as the node executed next. - popRHSifTop(E); - } else { - // Otherwise, assign ID+1 to LHS. - CondIDs[CodeGenFunction::stripCond(E->getLHS())] = NextID++; - } + // larger subtree. In this case, assign that ID to its LHS node. Its RHS + // will receive a new ID below. Otherwise, assign ID+1 to LHS. + if (CondIDs.contains(CodeGenFunction::stripCond(E))) + setCondID(E->getLHS(), getCondID(E)); + else + setCondID(E->getLHS(), NextID++); - // Assign ID+1 to RHS. - CondIDs[CodeGenFunction::stripCond(E->getRHS())] = NextID++; + // Assign a ID+1 for the RHS. + MCDCConditionID RHSid = NextID++; + setCondID(E->getRHS(), RHSid); - // Push ID of Stmt's RHS so that LHS nodes know about it - pushRHS(E); + // Push the LHS decision IDs onto the DecisionStack. + if (isLAnd(E)) + DecisionStack.push_back({RHSid, ParentDecision.FalseID}); + else + DecisionStack.push_back({ParentDecision.TrueID, RHSid}); + } + + /// Pop and return the LHS Decision ([0,0] if not set). + DecisionIDPair pop() { + if (!CGM.getCodeGenOpts().MCDCCoverage || NotMapped) + return DecisionStack.front(); + + assert(DecisionStack.size() > 1); + DecisionIDPair D = DecisionStack.back(); + DecisionStack.pop_back(); + return D; } - /// Pop the binary operator from the next level. If the walk is at the top of - /// the next, assign the total number of conditions. - unsigned popAndReturnCondCount(const BinaryOperator *E) { + /// Return the total number of conditions and reset the state. The number of + /// conditions is zero if the expression isn't mapped. + unsigned getTotalConditionsAndReset(const BinaryOperator *E) { if (!CGM.getCodeGenOpts().MCDCCoverage) return 0; - unsigned TotalConds = 0; - - // Pop Stmt from 'NestLevel' stack. - assert(NestLevel.back() == E); - NestLevel.pop_back(); + assert(!isIdle()); + assert(DecisionStack.size() == 1); // Reset state if not doing mapping. - if (NestLevel.empty() && NotMapped) { + if (NotMapped) { NotMapped = false; + assert(NextID == 1); return 0; } - // Pop RHS ID. - popRHS(E); + // Set number of conditions and reset. + unsigned TotalConds = NextID - 1; - // If at the parent (NestLevel=0), set conds and reset. - if (NestLevel.empty()) { - TotalConds = NextID - 1; + // Reset ID back to beginning. + NextID = 1; - // Reset ID back to beginning. - NextID = 1; - } return TotalConds; } }; @@ -1018,13 +1011,15 @@ struct CounterCoverageMappingBuilder return (Cond->EvaluateAsInt(Result, CVM.getCodeGenModule().getContext())); } + using MCDCDecisionIDPair = MCDCCoverageBuilder::DecisionIDPair; + /// Create a Branch Region around an instrumentable condition for coverage /// and add it to the function's SourceRegions. A branch region tracks a /// "True" counter and a "False" counter for boolean expressions that /// result in the generation of a branch. - void createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt, - MCDCConditionID ID = 0, MCDCConditionID TrueID = 0, - MCDCConditionID FalseID = 0) { + void + createBranchRegion(const Expr *C, Counter TrueCnt, Counter FalseCnt, + const MCDCDecisionIDPair &IDPair = MCDCDecisionIDPair()) { // Check for NULL conditions. if (!C) return; @@ -1034,6 +1029,10 @@ struct CounterCoverageMappingBuilder // function's SourceRegions) because it doesn't apply to any other source // code other than the Condition. if (CodeGenFunction::isInstrumentedCondition(C)) { + MCDCConditionID ID = MCDCBuilder.getCondID(C); + MCDCConditionID TrueID = IDPair.TrueID; + MCDCConditionID FalseID = IDPair.FalseID; + // If a condition can fold to true or false, the corresponding branch // will be removed. Create a region with both counters hard-coded to // zero. This allows us to visualize them in a special way. @@ -1822,20 +1821,28 @@ struct CounterCoverageMappingBuilder } void VisitBinLAnd(const BinaryOperator *E) { - // Keep track of Binary Operator and assign MCDC condition IDs + bool IsRootNode = MCDCBuilder.isIdle(); + + // Keep track of Binary Operator and assign MCDC condition IDs. MCDCBuilder.pushAndAssignIDs(E); extendRegion(E->getLHS()); propagateCounts(getRegion().getCounter(), E->getLHS()); handleFileExit(getEnd(E->getLHS())); + // Track LHS True/False Decision. + const auto DecisionLHS = MCDCBuilder.pop(); + // Counter tracks the right hand side of a logical and operator. extendRegion(E->getRHS()); propagateCounts(getRegionCounter(E), E->getRHS()); - // Process Binary Operator and create MCDC Decision Region if top-level + // Track RHS True/False Decision. + const auto DecisionRHS = MCDCBuilder.back(); + + // Create MCDC Decision Region if at top-level (root). unsigned NumConds = 0; - if ((NumConds = MCDCBuilder.popAndReturnCondCount(E))) + if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E))) createDecisionRegion(E, getRegionBitmap(E), NumConds); // Extract the RHS's Execution Counter. @@ -1847,30 +1854,13 @@ struct CounterCoverageMappingBuilder // Extract the Parent Region Counter. Counter ParentCnt = getRegion().getCounter(); - // Extract the MCDC condition IDs (returns 0 if not needed). - MCDCConditionID NextOrID = MCDCBuilder.getNextLOrCondID(); - MCDCConditionID NextAndID = MCDCBuilder.getNextLAndCondID(); - MCDCConditionID LHSid = MCDCBuilder.getCondID(E->getLHS()); - MCDCConditionID RHSid = MCDCBuilder.getCondID(E->getRHS()); - // Create Branch Region around LHS condition. - // MC/DC: For "LHS && RHS" - // - If LHS is TRUE, execution goes to the RHS. - // - If LHS is FALSE, execution goes to the LHS of the next logical-OR. - // If that does not exist, execution exits (ID == 0). createBranchRegion(E->getLHS(), RHSExecCnt, - subtractCounters(ParentCnt, RHSExecCnt), LHSid, RHSid, - NextOrID); + subtractCounters(ParentCnt, RHSExecCnt), DecisionLHS); // Create Branch Region around RHS condition. - // MC/DC: For "LHS && RHS" - // - If RHS is TRUE, execution goes to LHS of the next logical-AND. - // If that does not exist, execution exits (ID == 0). - // - If RHS is FALSE, execution goes to the LHS of the next logical-OR. - // If that does not exist, execution exits (ID == 0). createBranchRegion(E->getRHS(), RHSTrueCnt, - subtractCounters(RHSExecCnt, RHSTrueCnt), RHSid, - NextAndID, NextOrID); + subtractCounters(RHSExecCnt, RHSTrueCnt), DecisionRHS); } // Determine whether the right side of OR operation need to be visited. @@ -1884,20 +1874,28 @@ struct CounterCoverageMappingBuilder } void VisitBinLOr(const BinaryOperator *E) { - // Keep track of Binary Operator and assign MCDC condition IDs + bool IsRootNode = MCDCBuilder.isIdle(); + + // Keep track of Binary Operator and assign MCDC condition IDs. MCDCBuilder.pushAndAssignIDs(E); extendRegion(E->getLHS()); Counter OutCount = propagateCounts(getRegion().getCounter(), E->getLHS()); handleFileExit(getEnd(E->getLHS())); + // Track LHS True/False Decision. + const auto DecisionLHS = MCDCBuilder.pop(); + // Counter tracks the right hand side of a logical or operator. extendRegion(E->getRHS()); propagateCounts(getRegionCounter(E), E->getRHS()); - // Process Binary Operator and create MCDC Decision Region if top-level + // Track RHS True/False Decision. + const auto DecisionRHS = MCDCBuilder.back(); + + // Create MCDC Decision Region if at top-level (root). unsigned NumConds = 0; - if ((NumConds = MCDCBuilder.popAndReturnCondCount(E))) + if (IsRootNode && (NumConds = MCDCBuilder.getTotalConditionsAndReset(E))) createDecisionRegion(E, getRegionBitmap(E), NumConds); // Extract the RHS's Execution Counter. @@ -1913,28 +1911,13 @@ struct CounterCoverageMappingBuilder // Extract the Parent Region Counter. Counter ParentCnt = getRegion().getCounter(); - // Extract the MCDC condition IDs (returns 0 if not needed). - MCDCConditionID NextOrID = MCDCBuilder.getNextLOrCondID(); - MCDCConditionID NextAndID = MCDCBuilder.getNextLAndCondID(); - MCDCConditionID LHSid = MCDCBuilder.getCondID(E->getLHS()); - MCDCConditionID RHSid = MCDCBuilder.getCondID(E->getRHS()); - // Create Branch Region around LHS condition. - // MC/DC: For "LHS || RHS" - // - If LHS is TRUE, execution goes to the LHS of the next logical-AND. - // If that does not exist, execution exits (ID == 0). - // - If LHS is FALSE, execution goes to the RHS. createBranchRegion(E->getLHS(), subtractCounters(ParentCnt, RHSExecCnt), - RHSExecCnt, LHSid, NextAndID, RHSid); + RHSExecCnt, DecisionLHS); // Create Branch Region around RHS condition. - // MC/DC: For "LHS || RHS" - // - If RHS is TRUE, execution goes to LHS of the next logical-AND. - // If that does not exist, execution exits (ID == 0). - // - If RHS is FALSE, execution goes to the LHS of the next logical-OR. - // If that does not exist, execution exits (ID == 0). createBranchRegion(E->getRHS(), subtractCounters(RHSExecCnt, RHSFalseCnt), - RHSFalseCnt, RHSid, NextAndID, NextOrID); + RHSFalseCnt, DecisionRHS); } void VisitLambdaExpr(const LambdaExpr *LE) { diff --git a/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp b/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp index 5b13cc3507e96..6f47a4b901a8a 100644 --- a/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp +++ b/clang/test/CoverageMapping/mcdc-logical-stmt-ids-all.cpp @@ -129,3 +129,80 @@ bool func_ternary_or(bool a, bool b, bool c, bool d, bool e, bool f) { // CHECK: Branch,File 0, 122:26 -> 122:27 = (#6 - #7), #7 [4,0,3] // CHECK: Branch,File 0, 122:31 -> 122:32 = (#4 - #5), #5 [3,0,2] // CHECK: Branch,File 0, 122:36 -> 122:37 = (#2 - #3), #3 [2,0,0] + +bool func_if_nested_if(bool a, bool b, bool c, bool d, bool e) { + if (a || (b && c) || d || e) + return true; + else + return false; +} + +// CHECK-LABEL: Decision,File 0, 134:7 -> 134:30 = M:0, C:5 +// CHECK-NEXT: Branch,File 0, 134:7 -> 134:8 = (#0 - #6), #6 [1,0,4] +// CHECK: Branch,File 0, 134:13 -> 134:14 = #7, (#6 - #7) [4,5,3] +// CHECK: Branch,File 0, 134:18 -> 134:19 = #8, (#7 - #8) [5,0,3] +// CHECK: Branch,File 0, 134:24 -> 134:25 = (#4 - #5), #5 [3,0,2] +// CHECK: Branch,File 0, 134:29 -> 134:30 = (#2 - #3), #3 [2,0,0] + +bool func_ternary_nested_if(bool a, bool b, bool c, bool d, bool e) { + return (a || (b && c) || d || e) ? true : false; +} + +// CHECK-LABEL: Decision,File 0, 148:11 -> 148:34 = M:0, C:5 +// CHECK-NEXT: Branch,File 0, 148:11 -> 148:12 = (#0 - #6), #6 [1,0,4] +// CHECK: Branch,File 0, 148:17 -> 148:18 = #7, (#6 - #7) [4,5,3] +// CHECK: Branch,File 0, 148:22 -> 148:23 = #8, (#7 - #8) [5,0,3] +// CHECK: Branch,File 0, 148:28 -> 148:29 = (#4 - #5), #5 [3,0,2] +// CHECK: Branch,File 0, 148:33 -> 148:34 = (#2 - #3), #3 [2,0,0] + +bool func_if_nested_if_2(bool a, bool b, bool c, bool d, bool e) { + if (a || ((b && c) || d) && e) + return true; + else + return false; +} + +// CHECK-LABEL: Decision,File 0, 159:7 -> 159:32 = M:0, C:5 +// CHECK-NEXT: Branch,File 0, 159:7 -> 159:8 = (#0 - #2), #2 [1,0,2] +// CHECK: Branch,File 0, 159:14 -> 159:15 = #7, (#2 - #7) [2,5,4] +// CHECK: Branch,File 0, 159:19 -> 159:20 = #8, (#7 - #8) [5,3,4] +// CHECK: Branch,File 0, 159:25 -> 159:26 = (#5 - #6), #6 [4,3,0] +// CHECK: Branch,File 0, 159:31 -> 159:32 = #4, (#3 - #4) [3,0,0] + +bool func_ternary_nested_if_2(bool a, bool b, bool c, bool d, bool e) { + return (a || ((b && c) || d) && e) ? true : false; +} + +// CHECK-LABEL: Decision,File 0, 173:11 -> 173:36 = M:0, C:5 +// CHECK-NEXT: Branch,File 0, 173:11 -> 173:12 = (#0 - #2), #2 [1,0,2] +// CHECK: Branch,File 0, 173:18 -> 173:19 = #7, (#2 - #7) [2,5,4] +// CHECK: Branch,File 0, 173:23 -> 173:24 = #8, (#7 - #8) [5,3,4] +// CHECK: Branch,File 0, 173:29 -> 173:30 = (#5 - #6), #6 [4,3,0] +// CHECK: Branch,File 0, 173:35 -> 173:36 = #4, (#3 - #4) [3,0,0] + +bool func_if_nested_if_3(bool a, bool b, bool c, bool d, bool e, bool f) { + if ((a && (b || c) || (d && e)) && f) + return true; + else + return false; +} + +// CHECK-LABEL: Decision,File 0, 184:7 -> 184:39 = M:0, C:6 +// CHECK: Branch,File 0, 184:8 -> 184:9 = #5, (#0 - #5) [1,4,3] +// CHECK: Branch,File 0, 184:14 -> 184:15 = (#5 - #6), #6 [4,2,5] +// CHECK: Branch,File 0, 184:19 -> 184:20 = (#6 - #7), #7 [5,2,3] +// CHECK: Branch,File 0, 184:26 -> 184:27 = #8, (#4 - #8) [3,6,0] +// CHECK: Branch,File 0, 184:31 -> 184:32 = #9, (#8 - #9) [6,2,0] +// CHECK: Branch,File 0, 184:38 -> 184:39 = #3, (#2 - #3) [2,0,0] + +bool func_ternary_nested_if_3(bool a, bool b, bool c, bool d, bool e, bool f) { + return ((a && (b || c) || (d && e)) && f) ? true : false; +} + +// CHECK-LABEL: Decision,File 0, 199:11 -> 199:43 = M:0, C:6 +// CHECK: Branch,File 0, 199:12 -> 199:13 = #5, (#0 - #5) [1,4,3] +// CHECK: Branch,File 0, 199:18 -> 199:19 = (#5 - #6), #6 [4,2,5] +// CHECK: Branch,File 0, 199:23 -> 199:24 = (#6 - #7), #7 [5,2,3] +// CHECK: Branch,File 0, 199:30 -> 199:31 = #8, (#4 - #8) [3,6,0] +// CHECK: Branch,File 0, 199:35 -> 199:36 = #9, (#8 - #9) [6,2,0] +// CHECK: Branch,File 0, 199:42 -> 199:43 = #3, (#2 - #3) [2,0,0] diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h index 2757b8cd54a69..2a857917136a8 100644 --- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h +++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h @@ -295,12 +295,11 @@ struct CounterMappingRegion { Kind(Kind) {} CounterMappingRegion(MCDCParameters MCDCParams, unsigned FileID, - unsigned ExpandedFileID, unsigned LineStart, - unsigned ColumnStart, unsigned LineEnd, - unsigned ColumnEnd, RegionKind Kind) - : MCDCParams(MCDCParams), ExpandedFileID(ExpandedFileID), - LineStart(LineStart), ColumnStart(ColumnStart), LineEnd(LineEnd), - ColumnEnd(ColumnEnd), Kind(Kind) {} + unsigned LineStart, unsigned ColumnStart, + unsigned LineEnd, unsigned ColumnEnd, RegionKind Kind) + : MCDCParams(MCDCParams), FileID(FileID), LineStart(LineStart), + ColumnStart(ColumnStart), LineEnd(LineEnd), ColumnEnd(ColumnEnd), + Kind(Kind) {} static CounterMappingRegion makeRegion(Counter Count, unsigned FileID, unsigned LineStart, @@ -354,7 +353,7 @@ struct CounterMappingRegion { makeDecisionRegion(MCDCParameters MCDCParams, unsigned FileID, unsigned LineStart, unsigned ColumnStart, unsigned LineEnd, unsigned ColumnEnd) { - return CounterMappingRegion(MCDCParams, FileID, 0, LineStart, ColumnStart, + return CounterMappingRegion(MCDCParams, FileID, LineStart, ColumnStart, LineEnd, ColumnEnd, MCDCDecisionRegion); }