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

Add theoretical motivation section #474

Merged
merged 3 commits into from
Jul 18, 2023

Conversation

smoelius
Copy link
Collaborator

@smoelius smoelius commented Jun 5, 2023

No description provided.

README.md Outdated

Unless `x.foo` is ignored via a [configuration file], Necessist will remove `x.foo();`. It does so despite the fact the `x.foo();` does not change the context, and `Q ^ x.y == x.z ^ x.y > 2` implies `Q`.

However, it's not entirely clear whether Necessist _should_ remove this statement. The condition `x.y == x.z ^ x.y > 2` could be the result of side effects that were unintentional. In that case, knowing whether the enclosing test passes without `x.foo();` could be useful.
Copy link

@2over12 2over12 Jun 15, 2023

Choose a reason for hiding this comment

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

This is a bit imprecise to me imo. My major concern is in an SP calculus we could have the information:

{ Q ^ x.y == x.z ^ x.y > 2}
x.foo();
{ Q /\ x.y' == x.y +1 }

We are imagining here that foo checks some state on y and z then increments. But say there is no assert related to x.y' and x.y in the rest of the test then we still get a WP result:

{ Q ^ x.y == x.z ^ x.y > 2}
x.foo();
{ Q }

Since the post condition is essentially trivially satisfied wrt to Q. ie. my more major concern is that (*) drops unasserted effects, which is often the source of a buggy/under-constrained test. ie. in a very rough sense I think verify_untrusted_callback_override_ok is an example of a test that doesnt really uphold (*) since the post condition on the callback is essentially True

On the other hand, state being tested by asserts isnt a bad signal that that portion of the state is interesting. other statements that touch related/the same state are worth attempting to remove

This is a bit pedantic tho, the point remains that there are probably actions that should be/are removed that may not reflect (*)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you please give this another glance when you have a moment?

Copy link

@2over12 2over12 left a comment

Choose a reason for hiding this comment

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

This makes sense to me! also checked overflows were a nice relevant example

@smoelius
Copy link
Collaborator Author

Thanks a million, @2over12. 🙏

@smoelius smoelius force-pushed the theoretical-motivation branch from bde6b2f to f23026b Compare July 18, 2023 15:51
@smoelius smoelius merged commit 610dbb5 into trailofbits:master Jul 18, 2023
@smoelius smoelius deleted the theoretical-motivation branch July 18, 2023 17:02
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