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

Improve precision for % #1156

Open
michael-schwarz opened this issue Sep 11, 2023 · 6 comments
Open

Improve precision for % #1156

michael-schwarz opened this issue Sep 11, 2023 · 6 comments
Labels
bug precision sv-comp SV-COMP (analyses, results), witnesses

Comments

@michael-schwarz
Copy link
Member

michael-schwarz commented Sep 11, 2023

  int top;

  int c = -5;
  if(top) { c= -7;}
  __goblint_check(c % 2 == 1); // Doesn't hold at runtime

Goblint claims that this would hold.

For constants it works correctly:

  __goblint_check(-5 % 2 == -1);

Update: This has now been fixed to be sound in #1161, but remains more imprecise than possible, so we should carefully attempt to make it more precise while remaining sound.

@michael-schwarz
Copy link
Member Author

After some discussion with Helmut, it seems that just our implementation of [[x%y]]# is wrong, as it would need to return both the positive and the negative value if we don't know the sign of x.

@michael-schwarz
Copy link
Member Author

michael-schwarz commented Sep 12, 2023

I would propose dividing this issue into two issues, one to be sound again, which I'll work on and one to restore precision which I'll leave for someone else.

@michael-schwarz michael-schwarz changed the title Goblint unsound for x % 2 for x possibly negative but not constant Improve precision for % Sep 13, 2023
@sim642
Copy link
Member

sim642 commented Nov 21, 2023

Something related to this shows up in our own witness validation in SV-COMP.
This is illustrated with the following test, where the assertion is unknown:

#include <assert.h>

int main() {
  int x;
  if (x % 2 == 1) {
    assert(x % 2 == 1);
    return 1;
  }
  return 0;
}

According to tracing %%% congruence: rem : 1+2ℤ 2 -> 1+2ℤ , although it should be 1.

@sim642 sim642 added the sv-comp SV-COMP (analyses, results), witnesses label Nov 21, 2023
@michael-schwarz
Copy link
Member Author

%%% congruence: rem : 1+2ℤ 2 -> 1+2ℤ unless we know the sign of the the argument to %2, this is the best we can do!

@michael-schwarz
Copy link
Member Author

michael-schwarz commented Nov 21, 2023

One way to fix this is to have the guard Pos(x % 2 == 1) refine x to be positive.

@michael-schwarz
Copy link
Member Author

@hseidl and I constructed another example of this issue.

// PARAM: --enable ana.int.congruence --enable ana.int.interval --set sem.int.signed_overflow assume_none
#include <goblint.h>

int main()
{
	int x,y;

	if( y > 0) {
	if(x % 3 == 0){
		if(x + y == 5) {
                       // We have  x:  2+3ℤ
                       // But do not make use of it
                        
			if(y % 3 == 2) {
                                 // Neither by showing this to be the case in the __goblint_check
				__goblint_check(y % 3 == 2);
			} else {
				// Nor by finding this to be unreachable
			}

		}
	}
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug precision sv-comp SV-COMP (analyses, results), witnesses
Projects
None yet
Development

No branches or pull requests

2 participants