-
Notifications
You must be signed in to change notification settings - Fork 103
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
sailcov fails upon goodness within badness #502
Comments
Do you have this commit: 28ef9e9? |
Yes, I tested this with a sailcov binary I built from source as of cb0a396 |
And the sail binary used was also from that commit? Just making sure I have the same environment before I try to reproduce it myself. I think the asserts may be trying to check slightly different invariants, so even if changing the asserts produces outputs it might be worth thinking a bit harder about the invariants to ensure there isn't a deeper underlying problem. |
I have just now been able to reproduce this issue with both sail and sailcov binaries generated from cb0a396. I had to do some hackery to work around the coverage filename override issue, but I managed to do it outside of the sail repository this time. For the avoidance of doubt, here are the commands I used to generate the binaries:
|
Ok, after looking at the generated C from the CHERIoT model, I think the problem is how the separate function clauses interact with the coverage points. |
With some more investigation, that is not the case... Could you send me the complete set of coverage points and all the complete output from running the test? The generated code looks like:
the first call to Maybe I should change these asserts to warnings though, as they are reliant on location information for the coverage points being fully accurate. |
2024_04_26-cheriot-sailcov.zip Are these the files you're after? |
I found today that I'd forgotten to rebuild libsail_coverage.a as part of my cb0a396 rebuild. Here are similar files again after another go with the rebuilt lib, just in case it matters for you. I've also tried adding a .txt suffix this time so I could upload them without zipping. |
Hmmm. I notice there are odd lines in the sail_coverage file like:
|
What do you think we should do next on this @Alasdair? |
I haven't been able to reproduce the issue, so I'm still not sure what exactly is going wrong. |
I followed these instructions: https://github.com/lowRISC/TestRIG/tree/cheriot?tab=readme-ov-file#getting-started When I ran
Tip: I limited the run to just the arithmetic tests using the suggested arguments so that it would finish quickly. |
I've rebuilt with 1edb36b and am also now getting these warnings instead of a crash when I run sailcov. HTML generation now seems to work for all files, except for the warnings. |
Please feel free to close this issue since it the warning now no longer causes a failure. |
I have reproduced the "goodness within badness" in a small example:
The problem is an apparently spurious entry in the |
The C backend tries not to emit branch coverage points for matches containing a single case but was still emitting a line in all_branches. This confused sailcov and resulting in warnings and weird output. Hopefully fixes #502. Also adds a test case containing a tuple deconstruction (which apparently gets compiled to a single case match).
Should be fixed by #625 |
This is a big improvement but we still get some warnings from The assertion at Here are the relevant lines from
and
Looks like multiple branches are emitted for the same span in |
Here is a reduced example:
|
It appears that sailcov fatally crashes when it encounters a region of "goodness" within a region of "badness" when writing out HTML files.
Here is a minimal test case I have put together.
all_branches_edit:
sail_coverage_edit:
cheri_insts.sail (relevant section lines 292-297):
https://github.com/microsoft/cheriot-sail/blob/d641fb1e65378596b79306938ad70a2b58b91cb7/src/cheri_insts.sail#L292-L297
Command:
$SAILCOV -a all_branches_edit -t sail_coverage_edit cheri_insts.sail
...where
$SAILCOV
is a path to a sailcov binary.Terminal output:
Tail of the output "cheri_insts.html" file:
I have an idea for a fix, but am uncertain whether it would be correct or not due to my limited understanding of sailcov.
Here is the assert that is triggered:
sail/sailcov/main.ml
Lines 540 to 542 in a3f4047
Purely going on the pattern of the surrounding code, it looks like it should instead be:
If written this way, it would match the pattern set by the if-else case above it:
sail/sailcov/main.ml
Lines 532 to 534 in a3f4047
I'm happy to create a PR for this fix if requested.
The text was updated successfully, but these errors were encountered: