Skip to content

Fix cli mistakes #198

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

Merged
merged 4 commits into from
Feb 24, 2023
Merged

Fix cli mistakes #198

merged 4 commits into from
Feb 24, 2023

Conversation

d-xo
Copy link
Collaborator

@d-xo d-xo commented Feb 17, 2023

Description

Fixes two small mistakes in the cli interface:

  1. We were ignoring the solver chosen by the user and always executing using Z3
  2. The equivalence command had it's success condition backwards, and would report success for failure and vice versa...

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

Copy link
Collaborator

@msooseth msooseth left a comment

Choose a reason for hiding this comment

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

isQed -> isCex otherwise great stuff :)

@asymmetric
Copy link
Contributor

Any idea why this is failing on Linux and not on macOS?

@msooseth
Copy link
Collaborator

The PR pipeline tests broke (see logs here) because this randomly generation expression, unrated to the PR itself:

EVM.Expr.simplify WriteWord (SDiv (Sub (LT (Lit 0x1a) (Lit 0x2b)) (Lit 0x59)) (Lit 0x57)) (Lit 0xba0f7483eaf0000033e5f50af97000007b670a8fe8e000017b866ff8ee9) (WriteWord (Lit 0x10) (Lit 0x194f1c1b0a5600000ccdec88b707000004743426102c000005e06b915fdc) (AbstractBuf "esc_NFIIGOFlwfrBCZWnIWOEvFBT"))

Overflows Expr.simplify, in particular, the offset (SDiv (Sub (LT (Lit 0x1a) (Lit 0x2b)) (Lit 0x59)) (Lit 0x57)) happens to be:

EVM.Expr.simplify (SDiv (Sub (LT (Lit 0x1a) (Lit 0x2b)) (Lit 0x59)) (Lit 0x57))
Lit 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff

i.e. very large. This is related to the CopySlice simplifier overflow, where a similar issue happened and has been fixed by @zoep .

@msooseth
Copy link
Collaborator

I am re-running the PR pipeline, and created the issue #207 to track the problem itself so we can merge this unrelated fix and fix the other issue separately.

@d-xo d-xo requested a review from msooseth February 24, 2023 18:25
@d-xo d-xo merged commit fc9d95c into main Feb 24, 2023
@d-xo d-xo deleted the fix-cli-mistakes branch February 24, 2023 21:51
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.

4 participants