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

containerized test automation #49

Merged
merged 22 commits into from
Sep 11, 2023
Merged

containerized test automation #49

merged 22 commits into from
Sep 11, 2023

Conversation

ailrst
Copy link
Contributor

@ailrst ailrst commented Aug 7, 2023

Adds container config to compile and test bap, aslp, the bap aslp plugin, and basil, and a github action to execute tests automatically.

This means we automatically check if any PR compiles and passes the bitvec tests.

There are also some changes to the test scripts so they work with ubuntu's cross compile toolchain in the docker file.

Todo:

  • Add boogie for unit tests
  • Disable invalid unit tests

@ailrst ailrst marked this pull request as draft August 7, 2023 05:32
@l-kent
Copy link
Contributor

l-kent commented Aug 23, 2023

We want to use the latest build of the UQ-PAC fork of BAP, not the upstream release. This fork comes with the ASLi plugin, so you don't need to go to the hassle of compiling that separately either. liftasli.sh (and all the other scripts there) are a bit out of date. As I said before, the scripts in src/test/correct and src/test/incorrect are a more up to date reference point for compiling and lifting, and running a single example in src/test can be done through testOnly *SystemTests -- -z [example name].

We also don't really need github actions for testing lifting, I'm just not sure what the point of that is?

A bit to the side, I've been thinking it would probably be good to set up a new system of lifting, with a makefile per example that defines which compiler and compiler flags we want to use for each example, rather than the current system which just uses every combination of compiler and flags for every example which gives us more coverage than we need - the differences aren't interesting for every example.

@ailrst
Copy link
Contributor Author

ailrst commented Aug 25, 2023

Hi Liam I've updated it to use the bap fork.

The lifting test is mostly pointless it was just there as a basic test of the container so that the whole system works. Also note the new scripts eg examples/liftnew.sh don't work in the container for the ones that use -fpic because it fails to link against the stack protector in glibc for whatever reason (-fno-stack-protector fixes this).

I don't include the systemtests in the action since they take a very long time to run and we have limited actions minutes, do you think its worth including maybe a single one?

I definitely agree on moving towards makefiles rather than scripts.

8828da8 adds another dockerfile which bundles a godbolt instance with all the neccessary tools, so you can complile a c file in the web interface, add the basil tool and see the boogie output.

image

As an aside, did you refactor the basil arg parsing somewhere? I'd also like to know if I can find your WIP atomics example somewhere.

@l-kent
Copy link
Contributor

l-kent commented Aug 28, 2023

If we aren't including the system tests because they take too much time to run then there's not really much point to the github actions at all at this stage? Maybe just the bitvector tests and testing sbt compile to make sure it compiles? (sbt assembly creates a separate .jar which can be useful, and is necessary for running via the scripts but is not necessary in general if you are working through sbt) The system tests are the most relevant for any automation like this, but we would need to narrow it down to a smaller subset because a lot of the cases are fairly redundant which do contribute to it taking a long time.

Do you mean the ones with -fno-pic in liftnew.sh? That's known and accounted for - some examples just do not compile with certain flags and that's expected. liftnew.sh is just an initial version of what we've moved towards with the tests in the test folder.

For reasons I'm unsure of, -fno-pic was being used as the default (I think it simplifies some cases and just stuck around from early on), but the stack protector does not link with that enabled, so examples where that was an issue were separately compiled with either -fPIC or -fno-stack-protector. There are scripts for both of these - liftnostackprotector.sh, liftpic,sh and liftclangfpic.sh. The stack protector is an interesting enough case that we wanted to keep it and so the -fPIC scripts were used for those examples most recently (however -fPIC is not actually the default behaviour either which is why one of the compilation variations in the up-to-date tests folder is no compiler flags). The whole examples folder is out of date though as I've said, and we are trying to move away from that haphazard approach. There's no real point to modifying those lifting scripts to add -fno-stack-protector as the default, because that's undesirable, and trying to clean up the examples folder is a waste of time since we're already moving past it. We don't need or want to relift any of those examples.

I wouldn't recommend using run_c.sh for anything either, as it is compiling and lifting before running and that's unnecessary for an example that's already been lifted.

I haven't refactored the argument parsing, the only difference from the documentation is that there's also the -interpret option now.

I'm hoping to have the chase-lev-deque example with atomics in the repo either later today or tomorrow.

@ailrst ailrst marked this pull request as ready for review August 30, 2023 00:41
@l-kent
Copy link
Contributor

l-kent commented Aug 31, 2023

Is it a good idea to use the tests you've chosen to automate if we're only going to automate a small number? Some of those don't pass as-is, so it'll just inevitably tell people the build is broken, and some of them are a bit redundant.

@ailrst
Copy link
Contributor Author

ailrst commented Sep 5, 2023

Yes we should use tests that pass

@l-kent l-kent merged commit 9c90a8b into main Sep 11, 2023
1 check passed
@ailrst ailrst deleted the test-automation branch October 17, 2023 01:07
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.

2 participants