Closed LeszekSwirski closed 4 months ago
Attention: Patch coverage is 77.08333%
with 11 lines
in your changes missing coverage. Please review.
Project coverage is 67.00%. Comparing base (
0ed6a68
) to head (9c35335
). Report is 31 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
internal/binutils/addr2liner_llvm.go | 77.08% | 8 Missing and 3 partials :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
With this merged, I think #823 can be easily solved? The same JSON frame data from llvm-symbolizer
that is being read can provide StartLine
for Function.start_line
.
If this change is welcome, I can create a PR.
In some edge cases (e.g. injected JIT symbols), function names can have new lines. This breaks the llvm-symbolizer output parsing, and makes pprof hang.
Conveniently, as of LLVM 13, llvm-symbolizer has a JSON output mode, which is robust against all kinds of weirdness like new lines. We can use this instead of the line-based parsing, and as a bonus we get much simpler handling of multiple frames in a stack, as the JSON output already returns these as an array.
This also requires splitting the CODE and DATA processing into separate functions, since their JSON output is incompatible. For now, we keep the DATA output as before, a slightly hacky but functional concatenation of start + size, but this could be improved.