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

make cil_exp_of_linexpr1 work with fractional expressions #1493

Merged
merged 11 commits into from
Jul 2, 2024

Conversation

DrMichaelPetter
Copy link
Collaborator

Fixes #1328 , use Q instead of Z to extract coefficients, and then scale the coefficient with the lcm of their denominators.

@DrMichaelPetter DrMichaelPetter added bug in progress relational Relational analyses (Apron, affeq, lin2var) labels May 28, 2024
@DrMichaelPetter DrMichaelPetter self-assigned this May 28, 2024
@DrMichaelPetter DrMichaelPetter marked this pull request as ready for review June 17, 2024 11:53
Copy link
Member

@michael-schwarz michael-schwarz left a comment

Choose a reason for hiding this comment

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

It would be good to add a (cram) test where we test that the information now actually appears in the witness. Maybe @sim642 can help setting that up?

src/cdomains/apron/sharedFunctions.apron.ml Outdated Show resolved Hide resolved
src/cdomains/apron/sharedFunctions.apron.ml Outdated Show resolved Hide resolved
if Scalar.is_infty scalar <> 0 then (* infinity means unbounded *)
None
else match scalar with
| Float f -> if Stdlib.Float.is_integer f then Some (Q.of_float f) else None
Copy link
Member

Choose a reason for hiding this comment

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

According to Zarith documentation, Q.of_float is exact, so shouldn't this also work for non-integers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could, but in general, I would expect this case to happen, if we come from a float-based apron analysis. Correct me if I am wrong in the following argumentation:

These floats would likely stem from a floating point based analysis in apron, and thus would probably lead to very large denominators due to some +/-1 rounding in the low end of the mantissa. This might not hurt soundness so bad, since the result is only used to perform a scaling on the whole equality, but still the outcome might be very scary looking. In this implementation, I opted for a more conservative approach, just ignoring these floats.

src/cdomains/apron/sharedFunctions.apron.ml Outdated Show resolved Hide resolved
src/cdomains/apron/sharedFunctions.apron.ml Outdated Show resolved Hide resolved
@sim642 sim642 modified the milestones: v2.5.0, SV-COMP 2025 Jul 2, 2024
@sim642
Copy link
Member

sim642 commented Jul 2, 2024

I should try this on our relational witnesses for Freiburg to see if this produces any new ones we couldn't before.

@DrMichaelPetter DrMichaelPetter merged commit d7833ec into master Jul 2, 2024
23 checks passed
@DrMichaelPetter DrMichaelPetter deleted the issue_1328 branch July 2, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug relational Relational analyses (Apron, affeq, lin2var)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invariant for relationAnalysis: Lincons with fractional coefficients not outputted
3 participants