link/uprobe: fix offsets for statically linked binaries #385
Conversation
| return &s, nil | ||
| func (ex *Executable) offset(symbol string) (uint64, error) { | ||
| if off, ok := ex.offsets[symbol]; ok { | ||
| // Symbols with location 0 from section undef are shared library calls and |
There was a problem hiding this comment.
Can you break this change into a separate commit and explain why moving it makes sense?
There was a problem hiding this comment.
My reasoning is: since we have to do operations on the offsets, we either store a fake elf.Symbol with the modified offset value or store the offset values directly. I decided to go for the latter and move this check where we get the value.
Since I don't have an elf.Symbol to check anymore, I am now assuming that if the offset value is 0, we have an external symbol. (I am not 100% sure about this assumption so the error text may be misleading here)
Let me know if you have a better idea
mmat11
force-pushed
the
matt/fix-uprobe-offsets
branch
2 times, most recently
from
b2e7af2 to
27067b1
Compare
mmat11
force-pushed
the
matt/fix-uprobe-offsets
branch
2 times, most recently
from
db1e24a to
5aa637b
Compare
|
Putting back as draft, the test is failing on 4.9 (tracefs) but I can't reproduce locally even if I force that path, I'll investigate |
mmat11
force-pushed
the
matt/fix-uprobe-offsets
branch
from
5aa637b to
a928b86
Compare
What do you mean by that? You can't repro locally on 4.9, or you can't reproduce if you force tracefs usage on a newer kernel? |
The latter. If I modify the code and force tracefs, the test passes on all kernels except 4.9. I wrote "can't reproduce locally" because I assumed the kernel version was not a problem, but it is. |
|
Maybe @brycekahle or @Gui774ume have an idea what this could be? |
|
I think it's worth getting this merged if it fixes stuff on newer kernels. How about we skip the failing test on 4.9 and create an issue that contains a reproducer? An unrelated idea: could we instrument the |
Makes sense, my plan was to manually test the same binary on a 4.9 kernel VM. I can do this later and eventually add some comments (or a fix, depending on what's causing it to fail).
Good idea, I'll explore this option later today or tomorrow |
mmat11
force-pushed
the
matt/fix-uprobe-offsets
branch
from
a928b86 to
c2c4534
Compare
Use the following function to calculate the start address of a function:
fn symbol offset = fn symbol VA - .text VA + .text offset
Store raw offsets instead of elf.Symbol in the Executable struct.
Signed-off-by: Mattia Meleleo <[email protected]>
Signed-off-by: Mattia Meleleo <[email protected]>
mmat11
force-pushed
the
matt/fix-uprobe-offsets
branch
from
c2c4534 to
c488609
Compare
Context: https://cilium.slack.com/archives/CKC55JL8L/p1629493947056900
I could reproduce the problem, after applying these changes (https://github.com/iovisor/bcc/blob/master/libbpf-tools/uprobe_helpers.c#L268, iovisor/bcc@af3da23), it works for me locally.