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

Fortunac/bap 2.0 upgrade #65

Merged
merged 8 commits into from
Sep 5, 2019
Merged

Fortunac/bap 2.0 upgrade #65

merged 8 commits into from
Sep 5, 2019

Conversation

fortunac
Copy link

Ports VSA and wp to BAP 2.0. Major changes include:

WP:

  • no longer saving a BAP project in favor of a BAP program for binary diffing
  • using the dummy binary as a way to determine the architecture
  • using the last block of a sub to determine how much to increment SP rather than looking for a block with Ret since Ret has been removed

VSA:

  • removing bounded_lcm in favor of Word.lcm_exn

@fortunac fortunac requested a review from codyroux August 28, 2019 21:43
@ivg
Copy link

ivg commented Aug 29, 2019

There's no guarantee that the last block is the exit one. Neither that there's only one exit block. I would suggest implementing a simple dataflow analysis to compute the SP increments or use SSA to find it out.

@codyroux
Copy link
Contributor

@ivg how about taking the minimal elements in the reverse topological order? Obviously it's still kind of a hack, but I'm not sure I've seen SP incremented across several blocks.

Also: I'm afraid I don't see how SSA helps here, can you explain?

@ccasin
Copy link
Collaborator

ccasin commented Aug 29, 2019

How is ret indicated in BAP 2.0? An explicit read from the stack and a jump?

@codyroux
Copy link
Contributor

@ccasin I think it's a call with noreturn to the return address, if it's statically known.

@ivg
Copy link

ivg commented Aug 29, 2019

@ivg how about taking the minimal elements in the reverse topological order? Obviously it's still kind of a hack, but I'm not sure I've seen SP incremented across several blocks.

Also: I'm afraid I don't see how SSA helps here, can you explain?

RTO won't work, because we can have many exit blocks. The best way to classify a block as an exit/return blocks is to look into it's out degree — if it is zero, i.e., no outcoming edges, then it is an exit block. After all exit blocks are identifed, you can fold over them to identify the maximum stack increment.

Concerning SSA, sorry it was a long train of thought, so let me elaborate :) A right and general solution for computing the frame size of a subroutine is to write an AI and compute the approximation of it. A simple dataflow analysis will already work, especially if we can ignore recursive functions. However, SSA already provides enough dataflow facts, so it can be used instead.

@codyroux
Copy link
Contributor

@ivg thanks for the quick and detailed reply! My gut feeling is that we will:

  1. Merge the current version of the code
  2. Quickly implement a fix where we find return blocks using the heuristic you outlined
  3. Longer term, implement the AI to determine the stack size of a call as a separate pass, maybe tagging the call site with the stack size, but we'll only do this when the simple heuristic starts to suffer.

How does this sound?

@codyroux
Copy link
Contributor

All right! As far as I'm concerned, we can rebase whenever everyone's ready! Great job @fortunac !

@fortunac
Copy link
Author

Sounds good! I'll work on the heuristic for finding the return blocks on a separate branch that we can merge in after this one.

@philzook58
Copy link
Contributor

Everything looks good to me. I compiled and ran successfully ran make test on a bap:latest container. Good work!

@codyroux
Copy link
Contributor

codyroux commented Sep 5, 2019

All right I'm pulling the trigger on this one, we'll roll back if there are issues.

@codyroux codyroux merged commit f327bb7 into master Sep 5, 2019
@jtpaasch
Copy link
Collaborator

jtpaasch commented Sep 6, 2019

Didn't get a chance to run this yesterday, on account of being in 105 Broadway most of the day, but for Cody's sanity, Chloe's code does build on my machine, and tests pass, and conc2 builds too, using your fancy new BAP 2.0-friendly bap_wp.

@codyroux
Copy link
Contributor

codyroux commented Sep 7, 2019 via email

@fortunac fortunac deleted the fortunac/bap-2.0-upgrade branch September 27, 2019 19:38
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.

None yet

6 participants