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

fix: Use the right bitvector comparison in ComputeLessEq #2512

Merged
merged 3 commits into from
Jul 29, 2022

Conversation

cpitclaudel
Copy link
Member

@cpitclaudel cpitclaudel commented Jul 28, 2022

Dafny adds a free invariant on each loop that states that the loop variable is
smaller than or equal to its initial value. For bitvectors, Dafny produced ≥
instead of ≤, possibly due to a typo?

Specifically, the issue was:

  • In Translator.TrStatement.cs (TrLoop):

    // include a free invariant that says that all completed iterations so far have only decreased the termination metric
    …
    Bpl.Expr decrCheck = DecreasesCheck(toks, types, types, decrs, initDecr, null, null, true, false);
    
  • In Translator.cs (DecreasesCheck):

    Bpl.Expr less, atmost, eq;
    ComputeLessEq(toks[i], types0[i], types1[i], ee0[i], ee1[i], out less, out atmost, out eq, includeLowerBound);
    
  • In Translator.cs (ComputeLessEq):

    } else if (ty0 is BitvectorType) {
      BitvectorType bv = (BitvectorType)ty0;
      eq = Bpl.Expr.Eq(e0, e1);
      less = FunctionCall(tok, "lt_bv" + bv.Width, Bpl.Type.Bool, e0, e1);
      atmost = FunctionCall(tok, "ge_bv" + bv.Width, Bpl.Type.Bool, e0, e1);
    

    "ge_bv" is wrong.

  • Source/Dafny/Verifier/Translator.cs
    (ComputeLessEq): Change ge into le.
  • Test/git-issues/git-issue-2511.dfy:
    New file, showing the issue in all places where ComputeLessEq was used.

Introduced in 3f899b9.

Closes #2511.

@cpitclaudel cpitclaudel force-pushed the cpitclaudel_fix-bitvector-unsoundness-2511 branch from c787bd5 to f5068f2 Compare July 28, 2022 18:50
Dafny adds a free invariant on each loop that states that the loop variable is
smaller than or equal to its initial value.  For bitvectors, Dafny produced ≥
instead of ≤, possibly due to a typo.

Specifically, the issue was:

- In `Translator.TrStatement.cs` (`TrLoop`):

  ```
  // include a free invariant that says that all completed iterations so far have only decreased the termination metric
  …
  Bpl.Expr decrCheck = DecreasesCheck(toks, types, types, decrs, initDecr, null, null, true, false);
  ```

- In `Translator.cs` (`DecreasesCheck`):

  ```
  Bpl.Expr less, atmost, eq;
  ComputeLessEq(toks[i], types0[i], types1[i], ee0[i], ee1[i], out less, out atmost, out eq, includeLowerBound);
  ```

- In `Translator.cs` (`ComputeLessEq`):

  ```
  } else if (ty0 is BitvectorType) {
    BitvectorType bv = (BitvectorType)ty0;
    eq = Bpl.Expr.Eq(e0, e1);
    less = FunctionCall(tok, "lt_bv" + bv.Width, Bpl.Type.Bool, e0, e1);
    atmost = FunctionCall(tok, "ge_bv" + bv.Width, Bpl.Type.Bool, e0, e1);
  ```

  `"ge_bv"` is wrong.

* `Source/Dafny/Verifier/Translator.cs`
  (`ComputeLessEq`): Change `ge` into `le`.
* `Test/git-issues/git-issue-2511.dfy`:
  New file, showing the issue in all places where `ComputeLessEq` was used.

Introduced in 3f899b9.

Closes #2511.
@cpitclaudel cpitclaudel force-pushed the cpitclaudel_fix-bitvector-unsoundness-2511 branch from f5068f2 to e4980a2 Compare July 28, 2022 18:51
MikaelMayer
MikaelMayer previously approved these changes Jul 28, 2022
Copy link
Member

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

Great job, straightforward solution that makes totally sense. I confirm this really looks like a typo.

@cpitclaudel cpitclaudel enabled auto-merge (squash) July 28, 2022 22:34
@cpitclaudel cpitclaudel merged commit c37a52c into master Jul 29, 2022
@cpitclaudel cpitclaudel deleted the cpitclaudel_fix-bitvector-unsoundness-2511 branch July 29, 2022 14:46
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.

Prove false with bitvectors
2 participants