Skip to content

[llvm-cov][MC/DC][Qualification] Constant folding yields too high MCDC coverage (Rust) #109944

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

Open
escherle-validas opened this issue Sep 25, 2024 · 4 comments

Comments

@escherle-validas
Copy link

Constant folding yields too high MCDC coverage (Rust)

Criticality: MEDIUM

During qualification of MCDC coverage at Validas we found that
constant folding produces wrong (too high) coverage results in case of incomplete coverage.
In the following example where the term "(true || true) && v2" contains constants we would
expect that 1 / 3 atoms are covered (= 33.3 % MCDC), but LLVM Cov measures 50 % MCDC.
Since coverage is less than 100% a manual analysis and explanation is required, therefore the safety cirticality is not high.
Note: For the same example in C++ 100 % MCDC is measured by llvm-cov (see #109940)

constant_folding_rust

Example Source Code:
Test_000005.zip

@escherle-validas
Copy link
Author

Since we have no reply from you yet, I would like to ask for a status update for this issue. Are there any plans how this will be handled?

@evodius96
Copy link
Contributor

Ping @Lambdaris
But I think this will be addressed.

@Lambdaris
Copy link

Lambdaris commented Oct 22, 2024

The second paragraph at the comment under #109940 have answered why rust shows 50% coverage for your case.

Briefly, rustc tells llvm 3 mappings, only 1 of which is taken as constant by llvm (as that comment said, only the second constant true has two counters encoded to Zero). Then because llvm does not count constant conditions now, you get 1/2 (1 for v2 and 2 for v2, the first constant true).

This is also going to be solved by #94137

@Lambdaris
Copy link

Noticed #112694 has fixed it. Hence once rust updated its in-tree llvm to 20 we should get same result as clang for such cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants