Use llvm-symbolizer's JSON output to provide function start lines

[insilications]

When analyzing a perf.data profile converted automatically via perf_to_profile via pprof -raw perf.data, no function start lines (s=0) are present in any of the locations. With 813a5fb, this can be easily solved by using the same JSON frame data from llvm-symbolizer to provide StartLine for Function.start_line. This solves #823.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.91%. Comparing base (0ed6a68) to head (27c179c).
Report is 42 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #891      +/-   ##
==========================================
+ Coverage   66.86%   66.91%   +0.05%     
==========================================
  Files          44       44              
  Lines        9824     9857      +33     
==========================================
+ Hits         6569     6596      +27     
- Misses       2794     2820      +26     
+ Partials      461      441      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[aalexand]

A few CI tests failed:

++ go test github.com/google/pprof/fuzz github.com/google/pprof/internal/binutils github.com/google/pprof/internal/driver github.com/google/pprof/internal/elfexec github.com/google/pprof/internal/graph github.com/google/pprof/internal/measurement github.com/google/pprof/internal/report github.com/google/pprof/internal/symbolizer github.com/google/pprof/internal/symbolz github.com/google/pprof/profile
ok  	github.com/google/pprof/fuzz	0.003s
--- FAIL: TestObjFile (0.80s)
    --- FAIL: TestObjFile/fixed_load_address (0.67s)
        binutils_test.go:379: SourceLine for main: got [{main /tmp/hello.c 3 0 0}]; want [{main /tmp/hello.c 3 0 3}]
    --- FAIL: TestObjFile/simulated_ASLR_address (0.13s)
        binutils_test.go:379: SourceLine for main: got [{main /tmp/hello.c 3 0 0}]; want [{main /tmp/hello.c 3 0 3}]
FAIL
FAIL	github.com/google/pprof/internal/binutils	0.860s

[insilications]

Fixed TestPEFile windows tests that were failing and were obviously wrong. I have no idea why TestObjFile linux tests were failing in the CI tests. Let's try again please?

[aalexand]

Looks like the CI still failed.

[aalexand]

Maybe the CI setup needs to install llvm-symbolizer.

[insilications]

Maybe the CI setup needs to install llvm-symbolizer.

Could be... The TestPEFile windows tests were fixed because ci / test-windows is not failing anymore. I have no idea why it is failing on Linux. In my Linux system I even added a temporary debug log to double check: t.Logf("DEBUG SourceLine for main: got %v; want %v\n", gotFrames, wantFrames):

=== RUN   TestObjFile
=== RUN   TestObjFile/fixed_load_address
    binutils_test.go:378: DEBUG SourceLine for main: got [{main /tmp/hello.c 3 0 3}]; want [{main /tmp/hello.c 3 0 3}]
=== RUN   TestObjFile/simulated_ASLR_address
    binutils_test.go:378: DEBUG SourceLine for main: got [{main /tmp/hello.c 3 0 3}]; want [{main /tmp/hello.c 3 0 3}]
--- PASS: TestObjFile (0.10s)
    --- PASS: TestObjFile/fixed_load_address (0.09s)
    --- PASS: TestObjFile/simulated_ASLR_address (0.01s)

I wish we could solve this because it would close #823

[aalexand]

Could be... The TestPEFile windows tests were fixed because ci / test-windows is not failing anymore. I have no idea why it is failing on Linux. In my Linux system I even added a temporary debug log to double check: t.Logf("DEBUG SourceLine for main: got %v; want %v\n", gotFrames, wantFrames):

binutils.go falls back to nm or binutils' addr2line if llvm-symbolizer is not available. I think this is what might be happening since perhaps the CI environment doesn't have llvm-symbolizer. I suspect we need to figure out how to install llvm-symbolizer around this line:

pprof/.github/workflows/ci.yaml

Line 149 in a0b0bb1

sudo apt-get install graphviz

[insilications]

binutils.go falls back to nm or binutils' addr2line if llvm-symbolizer is not available. I think this is what might be happening since perhaps the CI environment doesn't have llvm-symbolizer. I suspect we need to figure out how to install llvm-symbolizer around this line:

pprof/.github/workflows/ci.yaml

Line 149 in a0b0bb1

sudo apt-get install graphviz

I have pushed changes to ci.yaml to attempt to fix it. Lets check if it passes now?

[insilications]

I ran the CI workflow on my fork. It is now passing all test-linux jobs on ubuntu-22.04. But it's failing on ubuntu-20.04. My added Verify llvm-symbolizer installation step shows that llvm-symbolizer --version on ubuntu-22.04 is:

llvm-symbolizer
Ubuntu LLVM version 14.0.0
  
  Optimized build.
  Default target: x86_64-pc-linux-gnu
  Host CPU: znver3

But on ubuntu-20.04 is:

LLVM (http://llvm.org/):
  LLVM version 10.0.0
  
  Optimized build.
  Default target: x86_64-pc-linux-gnu
  Host CPU: (unknown)

[insilications]

Last commit addresses the issue of an outdated llvm-symbolizer on Ubuntu 20.04. It is now passing all tests.

[aalexand]

Sorry it's taking me a while to review this. I've been staring at the code on and off, I'm just not a fan of how much this complicates the CI configuration with this code to get llvm/clang installed. But I guess there is no better way so I should just bite the bullet and LGTM.

[insilications]

Sorry it's taking me a while to review this. I've been staring at the code on and off, I'm just not a fan of how much this complicates the CI configuration with this code to get llvm/clang installed. But I guess there is no better way so I should just bite the bullet and LGTM.

Not a problem at all. Take the time you need to give it a thorough review.

Yeah, the additional complexity is sad but it is required... Ubuntu 20.04 provides LLVM 10's llvm-symbolizer, which is very old and does not produce a JSON data frame with StartLine. The alternative would be to add a minimum version check (LLVM 14) before the test that uses llvm-symbolizer and skip it:

pprof/internal/binutils/binutils_test.go

Line 336 in a0b0bb1

skipUnlessLinuxAmd64(t)