Skip to content

[clang][CoverageMapping] Refactor setting MC/DC True/False Condition IDs #78202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 18, 2024
Merged

[clang][CoverageMapping] Refactor setting MC/DC True/False Condition IDs #78202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8751417
[clang][CoverageMapping] Refactor when setting MC/DC True/False Condi…
evodius96 Jan 15, 2024
768a040
Update formatting changes
evodius96 Jan 15, 2024
cd516da
Cleanup Decision push/pop and Nesting Level Tracking
evodius96 Jan 16, 2024
642d1b1
Cleanup latest commit based on review comments
evodius96 Jan 18, 2024
637d98c
Minor revision
evodius96 Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 102 additions & 119 deletions clang/lib/CodeGen/CoverageMappingGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
///
Expand Down Expand Up @@ -663,54 +668,40 @@ struct MCDCCoverageBuilder {
private:
CodeGenModule &CGM;

llvm::SmallVector<MCDCConditionID> AndRHS;
llvm::SmallVector<MCDCConditionID> OrRHS;
llvm::SmallVector<const BinaryOperator *> NestLevel;
llvm::SmallVector<DecisionIDPair> DecisionStack;
llvm::DenseMap<const Stmt *, MCDCConditionID> &CondIDs;
llvm::DenseMap<const Stmt *, unsigned> &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<MCDCConditionID> &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<MCDCConditionID> &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<const Stmt *, MCDCConditionID> &CondIDMap,
llvm::DenseMap<const Stmt *, unsigned> &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.
Expand All @@ -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.
Expand All @@ -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;
}
};
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the APIs are all the same and are encapculated within MCDCBuilder, we don't need all of this here. We can just move it down into createBranchRegion().

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.
Expand All @@ -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.
Expand All @@ -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) {
Expand Down
Loading