Skip to content
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

Extend formal verification #192

Merged

Conversation

ChristopherRabotin
Copy link
Member

@ChristopherRabotin ChristopherRabotin commented Dec 15, 2022

Refer to #184 for progress.

Implements #184

List of bugs fixed thanks to Kani:

  • normalize() in Duration (which is called after every operation on a Duration) could overflow near the maximum or minimum number of centuries when the nanoseconds were still within the MIN/MAX of the structure.
  • normalize() in Duration would overflow when the duration was near (but below) the MAX
  • Duration::neg() would overflow near MIN/MAX
  • PartialEq on Duration would overflow near MIN/MAX
  • try_truncated_nanoseconds would overflow on any duration where the input was larger than the MAX
  • Ops::Add would fail when the centuries was the i16::MIN but the nanoseconds were still valid
  • Ops::Add would overflow when the right hand side input was not normalized
  • Ops::Sub would underflow when subtracting a duration whose nanoseconds was greater than self and when self.centuries was near the MIN.

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Thanks kani!
In some cases, there was a risk to an overflow in either the multiplication or addition. I thought that the code handled that, but obv it didn't.

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2022

Codecov Report

Base: 0.00% // Head: 79.83% // Increases project coverage by +79.83% 🎉

Coverage data is based on head (226557e) compared to base (783af30).
Patch coverage: 72.67% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #192       +/-   ##
===========================================
+ Coverage        0   79.83%   +79.83%     
===========================================
  Files           0       10       +10     
  Lines           0     2886     +2886     
===========================================
+ Hits            0     2304     +2304     
- Misses          0      582      +582     
Impacted Files Coverage Δ
src/lib.rs 57.14% <0.00%> (ø)
src/timescale.rs 92.92% <ø> (ø)
src/duration.rs 84.01% <68.54%> (ø)
src/timeunits.rs 81.75% <79.41%> (ø)
src/epoch.rs 87.51% <87.50%> (ø)
src/parser.rs 79.77% <0.00%> (ø)
src/deprecated.rs 0.00% <0.00%> (ø)
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Currently when the test fail I can't easily debug the case that caused a failure

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Also remove an assert in src code. Formal verification means that isn't needed.

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
…cations

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
It should be centered on zero, but the current implementation does not do that (and kani catches it).
Anyway, this needs to be fixed.

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Also added much needed documentation on how negative durations work,
even I had forgotten how it was setup.
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
@@ -284,6 +284,14 @@ In order to provide full interoperability with NAIF, hifitime uses the NAIF algo

# Changelog

## 3.8.0
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a work in progress, but I'm adding this to not forget the release notes.

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
It does not complete on my machine. I will need to find another formulation for it.

Signed-off-by: Christopher Rabotin <christopher.rabotin@gmail.com>
@ChristopherRabotin
Copy link
Member Author

Woohoo! All of the formal verification passes! Will merge now.

kani-formal-verification-reports.tar.gz

@ChristopherRabotin ChristopherRabotin merged commit 09bfcf3 into master Dec 29, 2022
@ChristopherRabotin ChristopherRabotin deleted the 184-greatly-improve-formal-verification-testing branch December 29, 2022 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants