search

cuviper / probe-rs

probe: Static probes for Rust
Apache License 2.0
92 stars 9 forks source link

Probes not firing due to missing `.probes` section #22

Closed xxuejie closed 1 year ago

xxuejie commented 1 year ago

Hi, I was recently trying out this project, and I notice that probes are not firing correctly when using bpftrace.

My first thought is that something breaks in my test machine(I was using Ubuntu 22.04), but after some debugging, I was able to dig into code here, for probes generated via this project, semaphore_offset will be 0, which then leads to code at here. So it seems that bcc (used by bpftrace underneath) is actually trying to find a .probes section, and use it to generate semaphore offsets. Without a .probes section, bpftrace will not be able to trigger SDT probes with semaphores.

Following a tutorial elsewhere(download build.sh, tick-dtrace.d and tick-main.c, then use build.sh to build the binary), I was able to build a C based binary with proper semaphore based USDT probes, that can be triggered via bpftrace. I compared the C binary with examples compiled from this project, and the only difference, is that Rust binaries lack .probes section. And it appears that dtrace generates this section when building tick-dtrace.d into tick-dtrace.o.

So my question is: is there a way we can replicate the same process, so as to provide legit .probes section here?

xxuejie commented 1 year ago

It does seem to me that this particular .probes section is used to keep all the semaphore variables. This changes the question to: can we somehow organize those semaphore variables in a particular section?

cuviper commented 1 year ago

Hmm, is that the root cause of #19? We can indeed add #[link_section = ".probes"], as I mentioned in https://github.com/cuviper/rust-libprobe/pull/17#issuecomment-1110373422 -- I just didn't know why anyone needed that. :)

xxuejie commented 1 year ago

Update: I found a solution to this problem, a proposed fix is included in #23

xxuejie commented 1 year ago

Hmm, we are replying at almost the same time :)

Yes I think this is the root cause of #19

cuviper commented 1 year ago

Great! So, I think the proposed split in #21 still has value to avoid the semaphore branch altogether, when it's not really needed, but if the section lets tools work better anyway, that's even better.

I'm going to merge my change first, so we can keep your attribution on the line in its new place. :)

xxuejie commented 1 year ago

Yeah I totally agree that #21 has its own merits, it's just that #21 is not exactly a direct fix of #19. Those are separate issues :P

cuviper commented 1 year ago

Released in 0.5.0.