Add some optimization options to our asm #12
Conversation
Since we don't actually emit code to speak of, we can let rustc/LLVM know that we don't clobber anything, letting them avoid saving/restoring registers or other state around the probe. We can't mark the probe as `pure`, even though our _code_ is `pure`, since that would enable LLVM to delete it entirely (since we have no outputs).
|
There's some tension here between what the assembly does (nothing) and what a debugger or other tool may do at that probe point (anything), and we need to strike a balance somehow. It's probably too extreme to try to deal with arbitrary writes in that respect, but reading memory from a probe seems reasonable -- so |
src/platform/systemtap.rs
Outdated
| @@ -181,7 +181,7 @@ macro_rules! sdt_asm( | |||
| #[doc(hidden)] | |||
| #[macro_export] | |||
| macro_rules! _sdt_asm( | |||
| ($addr:tt, options $opt:tt, $provider:ident, $name:ident, $argstr:tt, $($arg:expr),*) => ( | |||
| ($addr:tt, options ($opt:tt), $provider:ident, $name:ident, $argstr:tt, $($arg:expr),*) => ( | |||
There was a problem hiding this comment.
I think this change won't work for non-x86 targets that are passing just options() here. Maybe we should use a complete repetition pattern, options ( $( $opt:ident ),* ).
There was a problem hiding this comment.
You're right -- tested this on a non-x86 arch and it fails as you predict. Looking closer, though, do we actually need the att_syntax? If I remove it, it still builds for me on both 32- and 64-bit Linux x86, and as best I can tell everything we emit is either directives, which don't very with gas syntax, or a nop, which is perfectly valid Intel syntax. Removing that switch would certainly simplify things a bit.
There was a problem hiding this comment.
It matters for how the register is written in the probe note. When I first tried in the default (intel) syntax, GDB wasn't able to parse the probe argument correctly. Maybe GDB (and other tools!) could be taught otherwise, but then we'd only work with newer versions.
Make it hopefully easier for debuggers stopped at a probe to get a consistent view of the world.
|
Fixed both comments. Tested building on:
which I think should give good coverage of all the variants. Thanks for the prompt response and feedback! |
|
Great, thanks! I should probably add CI with target coverage like that... |
|
BTW, is your use of this crate publicly visible? |
|
Sort of. I was playing with it for https://github.com/nelhage/ultimattt -- I've been working on solving https://en.wikipedia.org/wiki/Ultimate_tic-tac-toe as a quarantine side project. Unfortunately I backed off from using rust-libprobe for the moment because of the dependency on a bleeding-edge nightly compiler and did something ad-hoc based on injecting ELF symbols with special names: https://github.com/nelhage/ultimattt/blob/863fdfe2d47cf5faac5b1a785fd80fa2d85ed659/src/lib/prove/pn_dfpn.rs#L242-L257 I'll probably switch back to this crate once it's possible to get a nightly build that supports both new-style ASM and has |
That should be fixed on the next nightly: rust-lang/rust#72671 (comment) And hopefully the new |
Since we don't actually emit code to speak of, we can let rustc/LLVM
know that we don't clobber anything, letting them avoid
saving/restoring registers or other state around the probe.
We can't mark the probe as
pure, even though our code ispure,since that would enable LLVM to delete it entirely (since we have no
outputs).