avoid hardcoded addresses in test cases, add pre-order iterator

[katrinafyi]

in preparation for #288, we need to avoid the use of hardcoded addresses in test cases. these addresses can easily be invalidated.

also adds all the unit tests to github actions

todo:

• tids in ssa variables in dsa
• everything that's not dsa / indir call
• exclude systemtests from unit test action
• taint analysis broken with preorder change?

[katrinafyi]

fyi @sadrabt for dsa test changes. but it's not fully done yet :3

[l-kent]

We definitely do not want to include all of these in the GitHub action and running all tests simultaneously like this breaks things at present, a bunch of tests (the various SystemTest variations) get in each other's way.

None of the following are expected to pass at present (some are not ever expected to pass) and should not be included:

• MemoryRegionTestsDSA
• MemoryRegionTestsNoRegion
• UnimplementedTests
• ExtraSpecTests
• IndirectCallTests

PointsToTest probably needs to be removed completely, it's very out of date and doesn't work.

IRTest has been bugged for months (#244) and this isn't running it - the tests that are in a particular package need to have the package prefixed like ./mill test.testOnly ir.IRTest

BitVectorAnalysisTests and IntrusiveListPublicInterfaceTest are already in the CompileAndTest job so it's redundant to run them multiple times like this.

[katrinafyi]

Thank you for the advice, I'll make the changes when I come back :)

[katrinafyi]

The changes have been made in 5253053 and 6badd36.

I also noticed that previously, you excluded the AnalysisSystemTests* from github actions but I've added them back here. Should these still be omitted?

[l-kent]

The addresses should not change as long as the binaries we are lifting from do not change (which the deterministic pipeline should guarantee?), so it should merely be a matter of updating the addresses once for the new test pipeline, rather than needing to try to abstract them like this (though it is also not actually a problem to look up the address of procedures in the program, that's trivial)?

The labels seem like the real issue for #288 and I'm not really sure how it's going to be possible to solve that.

[katrinafyi]

... the deterministic pipeline should guarantee ...

I don't think that's a very good reason against this PR. Previously, I'd been working through some of the address changes on top of #288 and it was extremely tedious to look up each decimal address in the old relf and match it back to change it. Surely, using names in the test cases is more maintainable and descriptive.

More importantly, the addresses in the deterministic pipeline are only truly guaranteed if you never change the compiler or source file. I'd like to avoid such restrictions. I agree that the labels are more troublesome since they depend on the lifters. I haven't looked at labels yet, but I did plan to as part of this PR.

Anyway, if there are other reasons against this change, then we can talk and see how to address those. Maybe having addresses within the tests is helpful when looking at the relf? Let me know.

Happy holidays!

(I'll be away from the computer for at least 3 weeks)

[l-kent]

More importantly, the addresses in the deterministic pipeline are only truly guaranteed if you never change the compiler or source file. I'd like to avoid such restrictions. I agree that the labels are more troublesome since they depend on the lifters.

If we change the compiler or C source file then we are changing the test to such a degree that it will need rewriting anyway. I don't think there is any reasonable way t

Surely, using names in the test cases is more maintainable and descriptive.

I don't really have an issue with that, though it's much trickier for the block addresses and I'm not sure what the solution there is. It's just far from really achieving the abstraction you're aiming for which I'm not really sure is possible.

[katrinafyi]

I think this can be considered ready for review, thanks to everyone's help. The two still-failing tests are ParamAnalysisTests:initialisation and IRTest, both of which are faliing in main.

There is one remaining instance of hard-coded block addresses in DataStructureAnalysisTest. I think this should be easy enough to manually fix when the tests are updated.

Edit: the DSA tests have been updated with a heuristic in 967c5de. This is a pretty complicated expression, so I am happy for it to be vetoed.

After 967c5de, there are 24 failing tests when applied on top of #288. To see this, you can very carefully use:

# from this PR's branch...
rm -rf src/test && git checkout origin/nix-docker-build -- src/test && git checkout HEAD -- src/test/scala && make -C src/test extract
./mill test
# to undo the docker changes...
git rm -rf src/test && git checkout HEAD -- src/test

[ailrst]

The failing timing out paramanalysis test is due to SSA infinite loop which is fixed by #307