-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
[Coverage][MC/DC] Constant folding might make conditions never covered #93560
Comments
As discussion on rust, @RenjiSann shows the expected output of llvm:
I have a premature thought as this comment suggests:
|
I am not sure I understand this correctly.
From what I understand, with the following example, void foo(bool a, bool b) {
if (a || (b && true)) {
print("success");
}
}
int main() {
foo(true, true);
foo(false, true);
} Here, if I look at |
Still Practical test vectors are whose bit of |
I see, so practical test vectors is the subset of all "actually possible" test vectors, which excludes hypothetical test vectors with a Still, I'm a bit confused. Doesn't this approach need us to eagerly generate all test vectors to check if MCDC is achievable for a given condition ? I feel like it would be easier to use the Binary decision diagram representation to propagate the folding to conditions and deduce which one are indeed folded. |
LLVM has already generated all test vectors by looking up the binary decision diagram, see buildTestVector. Indeed folded conditions can be easily detected upon traversing the decision diagram. Nevertheless I feel a bit troublesome on detecting uncoverable conditions, so I went over to test vectors. But now thanks to your remind, I think we could find folded conditions as building test vectors and distinguish uncoverable conditions later. Edit: It seems that the most cost is brought by detecting uncoverable conditions, so maybe it's still better to detect folded conditions with test vectors for clean (thus we can do them all in one function)? Or is there any good algorithm to find uncoverable conditions? |
Ok I have found it can be cleaner if we just partition practical test vectors by value of the decision. Then we get
Considering all constants have been marked as folded in advance, the rest conditions are normal. |
#94137 is drafted to solve this issue. |
Right, a folded constant condition does impact whether other conditions are reachable or coverable. However, I am of two minds about whether they ought to be excluded from MC/DC, although I could be persuaded. What do other coverage tool vendors, like LDRA or vectorcast, do for these cases? When I originally implemented MC/DC, my (perhaps erroneous) conclusion was that, while it didn't make sense to include actual constant conditions in MC/DC, an uncoverable or unreachable condition should be included and a lack of coverage would indicate a case where perhaps the code should be refactored to be testable, or in the case of templates, a more complete test written that provided different constants in different instances. It seems to me this is where MC/DC is useful since, like branch coverage and other coverage criteria, it's ultimately a measure of your source code coverage. |
That's a quite convincing argument. Upon implementing this feature I'm also indecisive about how we should treat these results. Though it seems to be responsibility of some linters to warn users (not sure about clang, but rust linters can easily do such stuff), coverage itself could do some thing too. I thought at least unreachable conditions are something coder should avoid. However we can not distinguish constants from unreachable conditions only by counters (Rust marks all unreachable counters as Could we add an additional index like "Uncoverable" after |
Thanks! I would be concerned that an additional index or category would be missed by those (mainly in embedded) who just want to see a code coverage metric for MC/DC at the end of the day, though maybe it's ok. I would be ok with adding an option to control whether uncoverable conditions are included or excluded, but I'd like the default setting to correspond to what other vendors have done, if possible. My recollection is that uncoverable/unreachable conditions are included by default with tools like LDRA, but I would need to do some additional research to be sure. Not saying that's a hard requirement, but I want to keep expectations aligned with what users may already be used to. I'm traveling this week but can look into this more hopefully soon. Thanks for looking into this! |
Sorry for the delay here -- I plan to run some experiments next week that hopefully will inform the decision. Thanks! |
I've added an option |
Let's consider such code:
Because the decision cannot be evaluated to
true
, no condition could be shown to be covered by MCDC.The constant may be a template parameter or a platform-dependent variable. As a result, one of the two branches might be optimized out and there is no branch in practice.
So could we eliminate such conditions and decisions from mcdc? For example,
a && CONST
, ifCONST==false
, conditiona
is treated asfolded
too.a || CONST
, ifCONST==true
,a
will be taken as folded. (Just in mcdc, normal branch coverage could still count it)a && (b || CONST)
, ifCONST==true
,b
is taken as folded.Also if a decision can not be evaluated to the other value, we mark it as folded.
We can check value of
CONST
in mcdc as long as the front end preserves the non-zero counter. For now clang assignZero
to bothCounter
andFalseCounter
if the condition was constant. By only setting the never reached counter toZero
, we can ensure the constant istrue
if itsFalseCounter
isZero
and vice versa. Then by analyzing the decision tree, conditions that would be never covered in meaning of mcdc can be found out and marked as folded too.The text was updated successfully, but these errors were encountered: