dslab-epfl / klint

Repository for the "Automated Verification of Network Function Binaries" paper (NSDI'22).
MIT License
8 stars 5 forks source link

add CI #5

Closed tharvik closed 1 year ago

tharvik commented 1 year ago

add GitHub actions support to automatically check that PRs lint/compile/build/...

questions

SolalPirelli commented 1 year ago

https://github.com/angr/claripy/issues/271 is the shift issue

SolalPirelli commented 1 year ago

f402ee7dfe7db28adb885f3ac1ce845a8f684c12 should fix the verif issue... at the cost of verifying the iptables NF, but well, omelette, eggs, ...

tharvik commented 1 year ago

In general all of the bpf stuff isn't for general use, it was for an example in the paper

as discussed, removing bpf & experiments in a next PR

f402ee7 should fix the verif issue... at the cost of verifying the iptables NF, but well, omelette, eggs, ...

hoo nice, thanks for fix, it works (waaay faster) now :)

SolalPirelli commented 1 year ago

need to revert the "CI on push only" change so it triggers for this PR as well, I guess? e436377f405772dfd14b02447de09b818e91949b

tharvik commented 1 year ago

need to revert the "CI on push only" change so it triggers for this PR as well, I guess? e436377

ho right, forgot to drop the "TMP" commits, good catch

tharvik commented 1 year ago

sadly, the more I run the verify-nf, the more issues arises: dropping router because of the shift issue.

SolalPirelli commented 1 year ago

maybe if you open an issue or a PR for angr/claripy#271 they'll be more willing to take it since it affects >1 person 😇

(or maybe it's already fixed and the dependency versions need an update?)

tharvik commented 1 year ago

(or maybe it's already fixed and the dependency versions need an update?)

yep, that's what I'm trying currently :crossed_fingers:

maybe if you open an issue or a PR for angr/claripy#271 they'll be more willing to take it since it affects >1

and if need be, fallback on that, maybe with a PR to push it even more!

tharvik commented 1 year ago

(or maybe it's already fixed and the dependency versions need an update?)

yep, that's what I'm trying currently crossed_fingers

sadly, it isn't fixed w/ latest release :disappointed:

tharvik commented 1 year ago

maybe if you open an issue or a PR for angr/claripy#271 they'll be more willing to take it since it affects >1

and if need be, fallback on that, maybe with a PR to push it even more!

PR opened at https://github.com/angr/claripy/pull/297 :) waiting for merge and release, then bumping theses deps and then most of the NF can be verified :tada:

tharvik commented 1 year ago

alright, upstream released, deps bumped, one last review and we can merge it IMO :)

SolalPirelli commented 1 year ago

Does the tmp commit still need a revert? Or is there something else preventing CI from working on this PR?

tharvik commented 1 year ago

Does the tmp commit still need a revert?

a rebase & force push got rid of it

Or is there something else preventing CI from working on this PR?

hum, as an external contributor is adding the CI, maybe you need to enable the checks somehow? or you can see the action ran on the c4dt fork

SolalPirelli commented 1 year ago

Weird, GitHub's docs say I should see something about this, oh well.