-
Notifications
You must be signed in to change notification settings - Fork 247
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
scripts/pdg.sh: also run analysis #974
Conversation
"-C" \ | ||
"metadata=4095517b1921578c" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this? What's that hash for? It looks fragile.
"-C" \ | ||
"metadata=4095517b1921578c" \ | ||
"-L" "dependency=${deps_dir}" \ | ||
"--extern" "libc=${libc_dep}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need c2rust_bitfields
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this meant to run end-to-end on? lighttpd-minimal
? lighttpd-rust-amalgamated
? Or the c2rust-analyze
tests (which aren't full crates)? Anything else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to move these additions to Rust code in c2rust_pdg::tests
rather than adding to this shell script, even if that's not their eventual location once c2rust-analyze
gets cargo
-wrapped. Right now, that's the canonical invocation of c2rust-instrument
that is run in CI tests. scripts/pdg.sh
isn't used regularly anymore and I'm not sure if it's fully up-to-date (I'd prefer to remove it entirely). This way, this end-to-end static + runtime analysis will be tested in CI.
this is fairly hacky and best-effort, but until c2rust-analyze is a rustc wrapper for cargo, it's hard to do better, and this avoids duplicating the work of figuring out these hacks
4da6b95
to
e71c2b3
Compare
Superseded by #1069. |
This is fairly hacky and best-effort, but until c2rust-analyze is a rustc wrapper for cargo, it's hard to do better, and this avoids duplicating the work of figuring out these hacks.
To see it in action:
./scripts/pdg.sh analysis/tests/minimal