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

[SMTChecker] Add option divModWithSlacks #11738

Merged
merged 1 commit into from
Aug 6, 2021
Merged

Conversation

leonardoalt
Copy link
Member

Currently we encode division and modulo as the constraint a = b * d + m where d = a / b and m = a % b because Spacer doesn't like syntactic / and mod inside Horn rules. However, other solvers (like Eldarica) might prefer the actual precise operations. This PR adds an option for that.

@leonardoalt
Copy link
Member Author

The effect this has is that if divModWithSlacks=false, CHC will return Error on division and modulo. BMC doesn't because it uses z3's standard SMT solver and not Spacer.

@leonardoalt
Copy link
Member Author

@hrkrshnn tiny one 😬

@leonardoalt
Copy link
Member Author

Actually missing command line JSON tests

hrkrshnn
hrkrshnn previously approved these changes Aug 5, 2021
docs/smtchecker.rst Outdated Show resolved Hide resolved
docs/using-the-compiler.rst Show resolved Hide resolved
libsolidity/formal/ModelCheckerSettings.h Outdated Show resolved Hide resolved
solc/CommandLineParser.cpp Outdated Show resolved Hide resolved
test/solc/CommandLineParser.cpp Outdated Show resolved Hide resolved
@leonardoalt
Copy link
Member Author

@cameel see the t_ubu_cli failing test

@cameel
Copy link
Member

cameel commented Aug 5, 2021

Oh, so it looks like instead of simply cutting out all messages we should be inserting Compiler run successful, no output requested. into the output when it's empty.

I'll try to fix that and submit a PR.

@cameel
Copy link
Member

cameel commented Aug 5, 2021

Here's a PR that fixes the warning stripping + adds pragmas and SPDX comments to tests that were missing them: #11751.

@leonardoalt leonardoalt force-pushed the smt_div_mod_slacks branch 2 times, most recently from b9d0bcd to b1a4d62 Compare August 5, 2021 20:42
@leonardoalt
Copy link
Member Author

Updated the comments, added JSON tests and put the license lines back after rebasing over @cameel 's PR that fixed the script.

solc/CommandLineParser.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1 @@

Copy link
Member

Choose a reason for hiding this comment

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

You don't need these files when they are empty. I've just removed a bunch of empty ones in the PR that fixed the checks for empty output :P

We should really modify cmdlineTests.sh to just not create them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea.. I didn't create them, I just ran the script and committed whatever files I had for convenience

Copy link
Member

Choose a reason for hiding this comment

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

Right. I think it just accepts the test without the file but if you ask it to update, it always creates it. I'll create an issue to clean this up eventually.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I just removed this and other empty files from this PR.

@@ -0,0 +1,49 @@
{"auxiliaryInputRequested":{"smtlib2queries":{"0x10763dfdad96614fe8fcaf54161ed2be500bbaa57bda8e2d706b675f1290f13b":"(set-option :produce-models true)
Copy link
Member

Choose a reason for hiding this comment

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

A few remarks not completely related to this PR:

  • auxiliaryInputRequested
    • The field is not documented as a part of Standard JSON output in Using the compiler.
    • CLI does not produce this output. Why?
    • I see that CHC does not produce this output either. Why?
  • solvers option added in [SMTChecker] Solver option #11421 is not listed in Standard JSON input either. It's only on the page about SMT.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, if any of this needs fixing, it does not necessarily have to be done in this PR (unless it's this PR that broke it).

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, this auxiliaryInputRequested is a bit of legacy that I'm not sure was ever used. Back when we didn't have emscripten z3 embedded into the compiler, we used this double run where you run the compiler once, get the queries like this, run a local solver, and input the results back into another compiler run via auxiliaryInput, using the same query hashes.
CLI doesn't produce it because the feature is only available via JSON.
CHC does produce it as well, when it can't prove a property.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok, solver option should be there, so will open another PR fixing that later.

Copy link
Member

Choose a reason for hiding this comment

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

OK, that explains it. What are your plans for auxiliaryInputRequested? Are you going to eventually remove it or can it be still useful in some rare cases?

Copy link
Member Author

@leonardoalt leonardoalt Aug 6, 2021

Choose a reason for hiding this comment

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

We talked about removing it in a call, but didn't reach consensus. Maybe we should just remove it

@leonardoalt leonardoalt enabled auto-merge August 6, 2021 14:00
@leonardoalt leonardoalt merged commit c69c08a into develop Aug 6, 2021
@leonardoalt leonardoalt deleted the smt_div_mod_slacks branch August 6, 2021 14:34
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.

3 participants