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

Simple CLI for macaw-symbolic #390

Closed
wants to merge 6 commits into from
Closed

Simple CLI for macaw-symbolic #390

wants to merge 6 commits into from

Conversation

langston-barrett
Copy link
Contributor

Fixes #389.

@langston-barrett langston-barrett self-assigned this Jul 3, 2024
@langston-barrett langston-barrett force-pushed the lb/cli branch 3 times, most recently from f5652b2 to ed41178 Compare July 3, 2024 20:25
macaw-cli/src/Data/Macaw/CLI.hs Outdated Show resolved Hide resolved
import Data.Macaw.Discovery qualified as MD
import Data.Macaw.Memory.ElfLoader.PLTStubs qualified as MPLT
import Data.Macaw.Symbolic qualified as MS
import Data.Macaw.Symbolic.Testing qualified as MST
Copy link
Contributor

Choose a reason for hiding this comment

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

We are now using Data.Macaw.Symbolic.Testing for reasons beyond just simple testing. I wonder if we should consider migrating the name to something more appropriate. (Data.Macaw.Symbolic.Driver, perhaps?)

Copy link
Contributor

Choose a reason for hiding this comment

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

I've also observed that Data.Macaw.Symbolic.Testing has some peculiar conventions that we may not want to use in macaw-cli. For instance, Data.Macaw.Symbolic.Testing won't take assumptions into account when checking an entrypoint function unless it begins with the prefix "test_and_verify_", so most functions won't check if they use the memory model in valid ways. This seems undesirable for macaw-cli's needs.

Perhaps we could factor out the code used in common between Data.Macaw.Symbolic.Testing and the new module we use to power macaw-cli (be it named Data.Macaw.Symbolic.Driver or something else).

@RyanGlScott
Copy link
Contributor

@langston-barrett, what are your plans for this PR now that #423 has landed? #423 only adds support for S-expression programs, whereas this PR is about binaries. Do you want to extend macaw-x86-cli to support binaries as well?

@langston-barrett
Copy link
Contributor Author

@langston-barrett, what are your plans for this PR now that #423 has landed?

I don't have further plans for this PR; as we've discussed in other forums, we should likely work towards a CLI that ingests binaries, but the testing harness code I'ver reused here is actually not reusable enough to suit this purpose. Instead, we should do a larger refactor of Macaw's APIs to make it easier to create frontends, and spend more effort creating one that has all of the conventions and functionality we actually think it should.

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.

Simple CLI frontend for macaw-*-symbolic
2 participants