[llvm-cov][MC/DC][Qualification] Too High MCDC coverage for C++

[escherle-validas]

Too High MCDC coverage for C++

Criticality: HIGH

During qualification of MCDC coverage at Validas we found that
the computation of MCDC coverage for the term "if ((TRUE ||TRUE) && v2)" is too high.
The result is 100%, but should be 33% (or 50% or 66% depending on the interpretation of folded constants).
This is highly critical, since tester might think they have covered everything and do not optimize the code.
When we run the same example using Rust, we see 50% coverage, which is different and safe.

Rust example:

Source Code and generated Reports:
Test_000005.zip

C++ example:

Source Code and generated Reports:
Test_000005.zip

[evodius96]

This may be addressed by: #94137

In short, with Rust, there is a desire for more granular results for constant conditions, including those conditions that are uncoverable or unreachable and whether they ought to be included in the metrics. This is presently not the default for standard clang, in which constant conditions are simply ignored, which also corresponds to the behavior of other vendors. With the above pull request, this can be changed for clang.

[escherle-validas]

For me it is not clear why C++ and Rust should be handled differently here.

Furthermore, it is task of MCDC coverage to find weaknesses in the Code and Constants in the term might be a problem. In this case there should be reported a coverage value less than 100 % MCDC.

[evodius96]

HI @escherle-validas MC/DC for Rust is presently a work-in-progress with some issues outstanding. What I can say is that clang folkds the conditions for C/C++, and folded conditions are not included in the metrics because they aren't influenced by variable test inputs. In looking at this further, I don't think this is the same issue as is addressed by #94137 which deals with non-constant conditions that may be uncoverable or unreachable due to constant conditions.

But if there is a desire to include constant folded conditions in the metrics, it seems feasible that could be added as an option.

[Lambdaris]

For me it is not clear why C++ and Rust should be handled differently here.

Furthermore, it is task of MCDC coverage to find weaknesses in the Code and Constants in the term might be a problem. In this case there should be reported a coverage value less than 100 % MCDC.

The reason why C++ and Rust give different results is the way we generate Zero counters for branches. Currently llvm only treats branch mappings whose both Count and FalseCount are hard coded to Zero as constant. And clang does so for all constants discovered in AST.

Otherwise, rustc does not know which conditions are constants until mir optimization, where all counters inserted in eliminated regions by control flow analysis are replaced with Zero. Regarding your example, the first true is assigned two counters at start, one for true and one for false. After CF analysis the region for false branch is removed and the false counter is replaced by Zero. But its true counter is still unchanged (note that we generate coverage mappings before this optimization) so llvm won't treat it as constant. As for the second true, regions for both two branches are removed, so it has counters like a constant llvm expects.

#94137 fixes it so that branches with only one Zero counter can be distinguished as constant, then rust and clang can give same result as clang does now (100% coverage). If you expects constant conditions are included in coverage you can pass --mcdc-exclude=none once that patch landed (by which you get 1/3 coverage).

[Endilll]

CC @chapuni

[chapuni]

@Endilll I supposed this was rust specific issue. Thanks for the notification.