-
Notifications
You must be signed in to change notification settings - Fork 64
Early multi-solutions system #643
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
Conversation
src/EVM.hs
Outdated
fallback :: a -> EVM t s () | ||
fallback _ = do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this function needs an argument that you are then ignoring anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me make it a BIT more sane, but it's basically that we use this function here, above:
delegateCall this gas xTo' xTo' (Lit 0) xInOffset xInSize xOutOffset xOutSize xs fallback $
where delegateCall
-s last argument is a function of the type: (Expr EAddr -> EVM t s ())
. But we don't need the first argument. I wanted to be able to pass this function to delegateCall
and also use it here:
False -> fallback undefined
and here:
Nothing -> fallback undefined
But yeah, it's annoying. I'll make a helper function strip1st :: (x) ->(a->x)
that makes a Monadic function that takes no arguments into a Monadic function that takes 1 argument, and ignores it. Then it will be maybe cleaner!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aaaah, there is a built-in function for this! https://hackage.haskell.org/package/base-4.21.0.0/docs/Prelude.html#v:const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha nice, OK, it's fixed now, using const
. Thanks, this was useful. I tried figuring out this function, I also didn't like it, but I couldn't find it. I now used LLMs because, why not. It told me about const
. Can you check if it's better now?
src/EVM.hs
Outdated
case length vals of | ||
-- if 2, we run both, otherwise, we run 1st and run ourselves with the rest | ||
2 -> if leftOrRight then runOne $ head vals | ||
else runOne (head $ tail vals) | ||
_ -> if leftOrRight then runOne $ head vals | ||
else choose . PleaseChoosePath ewordExpr $ runMore (tail vals) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this a bit? I just probably don't understand choose
and PleaseChoosePath
, but what does the parameter leftOrRight
represents and where is it actually passed in?
For the case of 2
, if leftOrRight
is false
, isn't this just ignoring the first value? I don't see where the "we run both" from the comment actually happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sorry. It's actually not CHOOSING anything. It's running both. It USED to be choosing things, because there was a debugger! But no longer. Let me rename that thing and then it'll be lot easier to understand :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay, OK, it should be a lot cleaner now, renamed all relevant functions and datatypes. Can you re-review?
BTW, I also added a test: + , test "symbolic-to-concrete-multi" $ do
+ Just c <- solcRuntime "MyContract"
+ [i|
+ interface Vm {
+ function deal(address,uint256) external;
+ }
+ contract MyContract {
+ function fun(uint160 a) external {
+ Vm vm = Vm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D);
+ uint160 c = 10 + (a % 2);
+ address b = address(c);
+ vm.deal(b, 10);
+ }
+ }
+ |]
+ let sig =Just (Sig "fun(uint160)" [AbiUIntType 160])
+ (e, [Qed _]) <- withDefaultSolver $ \s -> checkAssert s defaultPanicCodes c sig [] defaultVeriOpts
+ assertBoolM "The expression is not partial" $ Prelude.not (Expr.containsNode isPartial e) Which failed before. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better, but I still don't understand the leftOrRight
argument of runMore
.
Can you explain what it represents? What is its type? Where is it provided?
Co-authored-by: Martin Blicha <[email protected]>
Yes. So this is a If there are more than 2 things to run, we run the 1st on the 1st thread, and then the rest on the 2nd thread, by calling ourselves in the 2nd thread. Then, there'll be 1 less things to run... and so on. So it does this:
Do you have some idea how to express this better in code? I recognize this is non-trivial, but this kind of recursive self-calling with elements is kinda normal for Haskell. Maybe it could be done with some kind of foldr or something.... |
(sorry, had to update above description to be more precise) |
Hmm, I can give it another look in the evening. But it's fine if you want to merge this and move on right away. |
There is no rush actually :) Take your time! |
Ahhh, I see you approved... OK, let me merge. You can ask me about issues if have some. I feel like it's a non-trivial change, mostly because |
Description
Making progress towards symbolic jumpdestinations via SMT solving. This PR allows us to branch out to all potential jumpdests.
The system can restrict the returned value's number of relevant bytes to a fixed value. So if we are looking for a JUMP, it ignores high bytes, because it's impossible to have a PC that's more than 2^16 (more than 65K instructions). Similarly for addresses. This matters, because it can happen that there's some junk that's not restricted and we could get thousands of solutions, but they are actually all the same address if we ignore the bits other than the 160b that an address has.
Currently, it launches a new query for each new solution, which is very slow. This needs to be improved in a new PR. That will require a slight re-engineering of Solvers.hs, so we can query the system for more than one solution at a time.
Number of potential concrete is set to 10 by default. This is actually kinda low. However, we need to be a bit cautious, as things can blow up quickly, so I set 10. It can be changed via
--max-branch K
.Currently, it does NOT use incremental solving to get multiple solutions. This is not optimal. Hence, it's called "early", as a next PR will fix that.
Checklist