Repository: google/pprof
Pull request #675 (2021-12-01 17:19:50)

Parse and propagate the name of the kernel relocation symbol
For linux kernel mappings generated by recent version of the perf tool, the suffix of the mapping file name (not the name under which we found the actual symbols file) contains the kernel relocation symbol ("_text" or "_stext" in, for example "[kernel.kallsyms]_stext"). The symbol name is required to correctly compute the relocation offset of the kernel mapping.

We add it to Mapping as a public, but non-persisted field, so it can be preserved between profile fetching and symbolization, even if we found the kernel image locally under a different filename.
We also propagate it through the ObjTool interface.

Tested: unit.

s-kanev: Split the refactoring part from #674. @nolanmar511, could you take a look?
s-kanev: > When the binary is located by the build ID via $buildid/$name pattern such as in example in b/168662041, the binary name will be overridden - see [here](https://github.com/google/pprof/blob/2007db6d4f53c44a417ddae675d50f56b8e8c2fd/internal/driver/fetch.go#L429). I believe that happens before the symbolization is done. And I am not sure that this PR ensures that the original name will be stored anywhere, so unless I missed it seems that the proposed solution is somewhat fragile...

Oh, good catch. The overwriting happens after the successful call to `binutils.openELF()` (the only part that cares about the correct kernel relocation offset) -- which is why this approach works at all in the first place. Agree it's unnecessarily fragile.
aalexand: > > When the binary is located by the build ID via $buildid/$name pattern such as in example in b/168662041, the binary name will be overridden - see [here](https://github.com/google/pprof/blob/2007db6d4f53c44a417ddae675d50f56b8e8c2fd/internal/driver/fetch.go#L429). I believe that happens before the symbolization is done. And I am not sure that this PR ensures that the original name will be stored anywhere, so unless I missed it seems that the proposed solution is somewhat fragile...
> 
> Oh, good catch. The overwriting happens after the successful call to `binutils.openELF()` (the only part that cares about the correct kernel relocation offset) -- which is why this approach works at all in the first place. Agree it's unnecessarily fragile.

Two solutions come to mind, unfortunately both are somewhat intrusive:

1) Disallow the file search mode where the basename changes. This would affect scenario like b/168662041 - the user would need to ensure the filename matches what's in profile.
2) Or modify the Mapping struct to carry the "located filename" separately so that the original filename value is maintained as is and both first mapping override (`pprof /path/binary profile.pb.gz` mode) and the `$buildid/$name` lookup mode (and other modes actually) track the located path separately from the path stored in the profile.

I'll keep thinking of a better alternative.
codecov-commenter: # [Codecov](https://codecov.io/gh/google/pprof/pull/675?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
> Merging [#675](https://codecov.io/gh/google/pprof/pull/675?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (dbe5400) into [master](https://codecov.io/gh/google/pprof/commit/d25a53d42d00953a3db8e90122e377ff670683ae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (d25a53d) will **increase** coverage by `0.04%`.
> The diff coverage is `n/a`.

> :exclamation: Current head dbe5400 differs from pull request most recent head 01a1c08. Consider uploading reports for the commit 01a1c08 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/google/pprof/pull/675/graphs/tree.svg?width=650&height=150&src=pr&token=nHgs4Smibv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)](https://codecov.io/gh/google/pprof/pull/675?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)

```diff
@@            Coverage Diff             @@
##           master     #675      +/-   ##
==========================================
+ Coverage   68.13%   68.18%   +0.04%     
==========================================
  Files          41       41              
  Lines        7385     7393       +8     
==========================================
+ Hits         5032     5041       +9     
  Misses       1911     1911              
+ Partials      442      441       -1     
```


| [Impacted Files](https://codecov.io/gh/google/pprof/pull/675?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) | Coverage Δ | |
|---|---|---|
| [.../github.com/google/pprof/internal/report/report.go](https://codecov.io/gh/google/pprof/pull/675/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-c3JjL2dpdGh1Yi5jb20vZ29vZ2xlL3Bwcm9mL2ludGVybmFsL3JlcG9ydC9yZXBvcnQuZ28=) | `28.50% <0.00%> (-0.05%)` | :arrow_down: |
| [...github.com/google/pprof/internal/driver/webhtml.go](https://codecov.io/gh/google/pprof/pull/675/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-c3JjL2dpdGh1Yi5jb20vZ29vZ2xlL3Bwcm9mL2ludGVybmFsL2RyaXZlci93ZWJodG1sLmdv) | `100.00% <0.00%> (ø)` | |
| [.../github.com/google/pprof/internal/report/source.go](https://codecov.io/gh/google/pprof/pull/675/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-c3JjL2dpdGh1Yi5jb20vZ29vZ2xlL3Bwcm9mL2ludGVybmFsL3JlcG9ydC9zb3VyY2UuZ28=) | `80.46% <0.00%> (+0.03%)` | :arrow_up: |
| [...com/google/pprof/internal/symbolizer/symbolizer.go](https://codecov.io/gh/google/pprof/pull/675/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-c3JjL2dpdGh1Yi5jb20vZ29vZ2xlL3Bwcm9mL2ludGVybmFsL3N5bWJvbGl6ZXIvc3ltYm9saXplci5nbw==) | `36.09% <0.00%> (+0.42%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/pprof/pull/675?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/pprof/pull/675?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google). Last update [d25a53d...01a1c08](https://codecov.io/gh/google/pprof/pull/675?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).

s-kanev: > I'll keep thinking of a better alternative.

How about a simpler twist on 2.? Instead of preserving the "located filename" in Mapping, we preserve the original one. We can even just store its baseName, which has semantic meaning -- it's the DSO. This way, for every usage of Mapping.File (there's quite a few, including internally), we don't have to decide if it needs "original" or "located" -- we keep the current behavior.

I sketched out how that might look like. PTAL.
aalexand: > > I'll keep thinking of a better alternative.
> 
> How about a simpler twist on 2.? Instead of preserving the "located filename" in Mapping, we preserve the original one. We can even just store its baseName, which has semantic meaning -- it's the DSO. This way, for every usage of Mapping.File (there's quite a few, including internally), we don't have to decide if it needs "original" or "located" -- we keep the current behavior.
> 
> I sketched out how that might look like. PTAL.

What worries me a bit is that this appears to be the first time where we have a public field in profile.proto that is not a part of the persisted state. We would need to mention this in the comment to the field, as well as document semantics for operations like profile merge. Including handling outside pprof - e.g. see hasher.go in the internal profiler source code.

If we are going this route, I wonder if we should look more seriously into having something like "string type" field in the mapping proto - basically a way to disambiguate different mappings within the same file. We could define the semantics for that reasonably well (such as make sure the type is the part of the merge key) and that seems like a universally useful thing, even though there are upfront costs to pay like with any new field in profile.proto.
aalexand: I think one scenario that is problematic with the current approach is that in some cases the original name (that guides the correctness of the kernel symbolization) can still be irreversibly lost. For example, I think running `pprof -output profile.pb.gz -symbolize=none input.pb.gz` where pprof is capable to find the kernel image and rename it to, say, `vmlinux` will lose the "text" vs "stext" distinction but won't symbolize the profile and the subsequent opening that will attempt to symbolize the profile won't be able to symbolize the kernel properly.
aalexand: One possible idea (which is a hack but less invasive):

Earlier investigation says "When the perf tool generates an PERF_RECORD_MMAP, it can use either _text or _stext for the relocation symbol offset. Which one seems to just depend on the order of parsing the symbol table."

Could we apply similar logic in the local kernel symbolization and use _text or _stext depending on which one of the two is first in the symbol table?

I understand this is fragile but this is one of those hacks that if it works it can happily stay functional forever...
s-kanev: > Could we apply similar logic in the local kernel symbolization and use _text or _stext depending on which one of the two is first in the symbol table?
> 
> I understand this is fragile but this is one of those hacks that if it works it can happily stay functional forever...

I'm a little hesitant, for obvious hackery reasons. It could be a thing that stays functional forever, but we'd need to replicate it in at least two places where we parse the symbol tables -- for local and remote symbolization -- here in pprof, and the internal remote symbol injestion pipeline. So the main benefit of being an isolated hack is somewhat decreased.
s-kanev: Any way to make progress here?

I can take a stab at implementing the original "located filename" idea from [here](https://github.com/google/pprof/pull/675#issuecomment-991448362)?

That still has the downside of being a public non-persisted field in Mapping, and will be relatively invasive wrt to users of Mapping. But, at least, compared to the current proposal, it is idempotent, so something like `pprof -output profile.pb.gz -symbolize=none input.pb.gz` should continue to work.
s-kanev: @aalexand PTAL.

For the record, we discussed offline in a lot more detail and extending this many public interfaces is no-one's favorite solution. But we ran out of other options to correctly symbolize kernels that can have either "_text" or "_stext" as the relocation symbol.

We decided to name the extra Mapping field narrowly (KernelRelocationOffset) to discourage more general use of this kludge.
s-kanev: FWIW, github requires re-approval once I pushed more commits (Is that a new feature? Seems counter-intuitive...)

Patch:
diff --git a/driver/driver.go b/driver/driver.go
index fc05f91..5a8222f 100644
--- a/driver/driver.go
+++ b/driver/driver.go
@@ -137,8 +137,10 @@ type MappingSources map[string][]struct {
 type ObjTool interface {
 	// Open opens the named object file. If the object is a shared
 	// library, start/limit/offset are the addresses where it is mapped
-	// into memory in the address space being inspected.
-	Open(file string, start, limit, offset uint64) (ObjFile, error)
+	// into memory in the address space being inspected. If the object
+	// is a linux kernel, relocationSymbol is the name of the symbol
+	// corresponding to the start address.
+	Open(file string, start, limit, offset uint64, relocationSymbol string) (ObjFile, error)
 
 	// Disasm disassembles the named object file, starting at
 	// the start address and stopping at (before) the end address.
@@ -232,8 +234,8 @@ type internalObjTool struct {
 	ObjTool
 }
 
-func (o *internalObjTool) Open(file string, start, limit, offset uint64) (plugin.ObjFile, error) {
-	f, err := o.ObjTool.Open(file, start, limit, offset)
+func (o *internalObjTool) Open(file string, start, limit, offset uint64, relocationSymbol string) (plugin.ObjFile, error) {
+	f, err := o.ObjTool.Open(file, start, limit, offset, relocationSymbol)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/binutils/binutils.go b/internal/binutils/binutils.go
index e920eeb..3043ecf 100644
--- a/internal/binutils/binutils.go
+++ b/internal/binutils/binutils.go
@@ -284,7 +284,7 @@ func (bu *Binutils) Disasm(file string, start, end uint64, intelSyntax bool) ([]
 }
 
 // Open satisfies the plugin.ObjTool interface.
-func (bu *Binutils) Open(name string, start, limit, offset uint64) (plugin.ObjFile, error) {
+func (bu *Binutils) Open(name string, start, limit, offset uint64, relocationSymbol string) (plugin.ObjFile, error) {
 	b := bu.get()
 
 	// Make sure file is a supported executable.
@@ -316,7 +316,7 @@ func (bu *Binutils) Open(name string, start, limit, offset uint64) (plugin.ObjFi
 
 	// Match against supported file types.
 	if elfMagic == elf.ELFMAG {
-		f, err := b.openELF(name, start, limit, offset)
+		f, err := b.openELF(name, start, limit, offset, relocationSymbol)
 		if err != nil {
 			return nil, fmt.Errorf("error reading ELF file %s: %v", name, err)
 		}
@@ -425,7 +425,7 @@ func (b *binrep) openMachO(name string, start, limit, offset uint64) (plugin.Obj
 	return b.openMachOCommon(name, of, start, limit, offset)
 }
 
-func (b *binrep) openELF(name string, start, limit, offset uint64) (plugin.ObjFile, error) {
+func (b *binrep) openELF(name string, start, limit, offset uint64, relocationSymbol string) (plugin.ObjFile, error) {
 	ef, err := elfOpen(name)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing %s: %v", name, err)
diff --git a/internal/driver/cli.go b/internal/driver/cli.go
index 492400c..237cc33 100644
--- a/internal/driver/cli.go
+++ b/internal/driver/cli.go
@@ -98,7 +98,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) {
 	// Recognize first argument as an executable or buildid override.
 	if len(args) > 1 {
 		arg0 := args[0]
-		if file, err := o.Obj.Open(arg0, 0, ^uint64(0), 0); err == nil {
+		if file, err := o.Obj.Open(arg0, 0, ^uint64(0), 0, ""); err == nil {
 			file.Close()
 			execName = arg0
 			args = args[1:]
diff --git a/internal/driver/fetch.go b/internal/driver/fetch.go
index b8a69e8..0b36165 100644
--- a/internal/driver/fetch.go
+++ b/internal/driver/fetch.go
@@ -420,12 +420,14 @@ mapping:
 				fileNames = append(fileNames, filepath.Join(path, m.File))
 			}
 			for _, name := range fileNames {
-				if f, err := obj.Open(name, m.Start, m.Limit, m.Offset); err == nil {
+				if f, err := obj.Open(name, m.Start, m.Limit, m.Offset, m.KernelRelocationSymbol); err == nil {
 					defer f.Close()
 					fileBuildID := f.BuildID()
 					if m.BuildID != "" && m.BuildID != fileBuildID {
 						ui.PrintErr("Ignoring local file " + name + ": build-id mismatch (" + m.BuildID + " != " + fileBuildID + ")")
 					} else {
+						// Explicitly do not update KernelRelocationSymbol --
+						// the new local file name is most likely missing it.
 						m.File = name
 						continue mapping
 					}
@@ -449,6 +451,8 @@ mapping:
 	if execName, buildID := s.ExecName, s.BuildID; execName != "" || buildID != "" {
 		m := p.Mapping[0]
 		if execName != "" {
+			// Explicitly do not update KernelRelocationSymbol --
+			// the source override is most likely missing it.
 			m.File = execName
 		}
 		if buildID != "" {
diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go
index a57a0b2..98eb1dd 100644
--- a/internal/plugin/plugin.go
+++ b/internal/plugin/plugin.go
@@ -109,8 +109,10 @@ type MappingSources map[string][]struct {
 type ObjTool interface {
 	// Open opens the named object file. If the object is a shared
 	// library, start/limit/offset are the addresses where it is mapped
-	// into memory in the address space being inspected.
-	Open(file string, start, limit, offset uint64) (ObjFile, error)
+	// into memory in the address space being inspected. If the object
+	// is a linux kernel, relocationSymbol is the name of the symbol
+	// corresponding to the start address.
+	Open(file string, start, limit, offset uint64, relocationSymbol string) (ObjFile, error)
 
 	// Disasm disassembles the named object file, starting at
 	// the start address and stopping at (before) the end address.
diff --git a/internal/report/report.go b/internal/report/report.go
index e2fb003..36ddf2e 100644
--- a/internal/report/report.go
+++ b/internal/report/report.go
@@ -526,7 +526,7 @@ func symbolsFromBinaries(prof *profile.Profile, g *graph.Graph, rx *regexp.Regex
 			}
 		}
 
-		f, err := obj.Open(m.File, m.Start, m.Limit, m.Offset)
+		f, err := obj.Open(m.File, m.Start, m.Limit, m.Offset, m.KernelRelocationSymbol)
 		if err != nil {
 			fmt.Printf("%v\n", err)
 			continue
diff --git a/internal/report/source.go b/internal/report/source.go
index 33d04c5..d8b4395 100644
--- a/internal/report/source.go
+++ b/internal/report/source.go
@@ -744,7 +744,7 @@ func (sp *sourcePrinter) objectFile(m *profile.Mapping) plugin.ObjFile {
 	if object, ok := sp.objects[m.File]; ok {
 		return object // May be nil if we detected an error earlier.
 	}
-	object, err := sp.objectTool.Open(m.File, m.Start, m.Limit, m.Offset)
+	object, err := sp.objectTool.Open(m.File, m.Start, m.Limit, m.Offset, m.KernelRelocationSymbol)
 	if err != nil {
 		object = nil
 	}
diff --git a/internal/symbolizer/symbolizer.go b/internal/symbolizer/symbolizer.go
index 5ba0122..cbb0ed4 100644
--- a/internal/symbolizer/symbolizer.go
+++ b/internal/symbolizer/symbolizer.go
@@ -328,7 +328,7 @@ func newMapping(prof *profile.Profile, obj plugin.ObjTool, ui plugin.UI, force b
 		if m.BuildID != "" {
 			name += fmt.Sprintf(" (build ID %s)", m.BuildID)
 		}
-		f, err := obj.Open(m.File, m.Start, m.Limit, m.Offset)
+		f, err := obj.Open(m.File, m.Start, m.Limit, m.Offset, m.KernelRelocationSymbol)
 		if err != nil {
 			ui.PrintErr("Local symbolization failed for ", name, ": ", err)
 			missingBinaries = true
diff --git a/profile/encode.go b/profile/encode.go
index ab7f03a..96aa271 100644
--- a/profile/encode.go
+++ b/profile/encode.go
@@ -17,6 +17,7 @@ package profile
 import (
 	"errors"
 	"sort"
+	"strings"
 )
 
 func (p *Profile) decoder() []decoder {
@@ -252,6 +253,14 @@ func (p *Profile) postDecode() error {
 		} else {
 			mappings[m.ID] = m
 		}
+
+		// If this a main linux kernel mapping with a relocation symbol suffix
+		// ("[kernel.kallsyms]_text"), extract said suffix.
+		// It is fairly hacky to handle at this level, but the alternatives appear even worse.
+		if strings.HasPrefix(m.File, "[kernel.kallsyms]") {
+			m.KernelRelocationSymbol = strings.ReplaceAll(m.File, "[kernel.kallsyms]", "")
+		}
+
 	}
 
 	functions := make(map[uint64]*Function, len(p.Function))
diff --git a/profile/merge.go b/profile/merge.go
index 9978e73..6fcd11d 100644
--- a/profile/merge.go
+++ b/profile/merge.go
@@ -303,16 +303,17 @@ func (pm *profileMerger) mapMapping(src *Mapping) mapInfo {
 		return mi
 	}
 	m := &Mapping{
-		ID:              uint64(len(pm.p.Mapping) + 1),
-		Start:           src.Start,
-		Limit:           src.Limit,
-		Offset:          src.Offset,
-		File:            src.File,
-		BuildID:         src.BuildID,
-		HasFunctions:    src.HasFunctions,
-		HasFilenames:    src.HasFilenames,
-		HasLineNumbers:  src.HasLineNumbers,
-		HasInlineFrames: src.HasInlineFrames,
+		ID:                     uint64(len(pm.p.Mapping) + 1),
+		Start:                  src.Start,
+		Limit:                  src.Limit,
+		Offset:                 src.Offset,
+		File:                   src.File,
+		KernelRelocationSymbol: src.KernelRelocationSymbol,
+		BuildID:                src.BuildID,
+		HasFunctions:           src.HasFunctions,
+		HasFilenames:           src.HasFilenames,
+		HasLineNumbers:         src.HasLineNumbers,
+		HasInlineFrames:        src.HasInlineFrames,
 	}
 	pm.p.Mapping = append(pm.p.Mapping, m)
 
diff --git a/profile/profile.go b/profile/profile.go
index 2590c8d..5a3807f 100644
--- a/profile/profile.go
+++ b/profile/profile.go
@@ -106,6 +106,15 @@ type Mapping struct {
 
 	fileX    int64
 	buildIDX int64
+
+	// Name of the kernel relocation symbol ("_text" or "_stext"), extracted from File.
+	// For linux kernel mappings generated by some tools, correct symbolization depends
+	// on knowing which of the two possible relocation symbols was used for `Start`.
+	// This is given to us as a suffix in `File` (e.g. "[kernel.kallsyms]_stext").
+	//
+	// Note, this public field is not persisted in the proto. For the purposes of
+	// copying / merging / hashing profiles, it is considered subsumed by `File`.
+	KernelRelocationSymbol string
 }
 
 // Location corresponds to Profile.Location


Test patch:
diff --git a/internal/binutils/binutils_test.go b/internal/binutils/binutils_test.go
index 7d7b450..8fe67ee 100644
--- a/internal/binutils/binutils_test.go
+++ b/internal/binutils/binutils_test.go
@@ -346,7 +346,7 @@ func TestObjFile(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			bu := &Binutils{}
-			f, err := bu.Open(filepath.Join("testdata", "exe_linux_64"), tc.start, tc.limit, tc.offset)
+			f, err := bu.Open(filepath.Join("testdata", "exe_linux_64"), tc.start, tc.limit, tc.offset, "")
 			if err != nil {
 				t.Fatalf("Open: unexpected error %v", err)
 			}
@@ -416,7 +416,7 @@ func TestMachoFiles(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			bu := &Binutils{}
-			f, err := bu.Open(filepath.Join("testdata", tc.file), tc.start, tc.limit, tc.offset)
+			f, err := bu.Open(filepath.Join("testdata", tc.file), tc.start, tc.limit, tc.offset, "")
 			if err != nil {
 				t.Fatalf("Open: unexpected error %v", err)
 			}
@@ -505,7 +505,7 @@ func TestPEFile(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			bu := &Binutils{}
-			f, err := bu.Open(filepath.Join("testdata", "exe_windows_64.exe"), tc.start, tc.limit, tc.offset)
+			f, err := bu.Open(filepath.Join("testdata", "exe_windows_64.exe"), tc.start, tc.limit, tc.offset, "")
 			if err != nil {
 				t.Fatalf("Open: unexpected error %v", err)
 			}
@@ -544,7 +544,7 @@ func TestOpenMalformedELF(t *testing.T) {
 	// Test that opening a malformed ELF file will report an error containing
 	// the word "ELF".
 	bu := &Binutils{}
-	_, err := bu.Open(filepath.Join("testdata", "malformed_elf"), 0, 0, 0)
+	_, err := bu.Open(filepath.Join("testdata", "malformed_elf"), 0, 0, 0, "")
 	if err == nil {
 		t.Fatalf("Open: unexpected success")
 	}
@@ -558,7 +558,7 @@ func TestOpenMalformedMachO(t *testing.T) {
 	// Test that opening a malformed Mach-O file will report an error containing
 	// the word "Mach-O".
 	bu := &Binutils{}
-	_, err := bu.Open(filepath.Join("testdata", "malformed_macho"), 0, 0, 0)
+	_, err := bu.Open(filepath.Join("testdata", "malformed_macho"), 0, 0, 0, "")
 	if err == nil {
 		t.Fatalf("Open: unexpected success")
 	}
@@ -818,7 +818,7 @@ func TestELFObjAddr(t *testing.T) {
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			b := binrep{}
-			o, err := b.openELF(name, tc.start, tc.limit, tc.offset)
+			o, err := b.openELF(name, tc.start, tc.limit, tc.offset, "")
 			if (err != nil) != tc.wantOpenError {
 				t.Errorf("openELF got error %v, want any error=%v", err, tc.wantOpenError)
 			}
diff --git a/internal/driver/driver_test.go b/internal/driver/driver_test.go
index c6c9c93..2564eb0 100644
--- a/internal/driver/driver_test.go
+++ b/internal/driver/driver_test.go
@@ -1580,7 +1580,7 @@ func TestSymbolzAfterMerge(t *testing.T) {
 
 type mockObjTool struct{}
 
-func (*mockObjTool) Open(file string, start, limit, offset uint64) (plugin.ObjFile, error) {
+func (*mockObjTool) Open(file string, start, limit, offset uint64, relocationSymbol string) (plugin.ObjFile, error) {
 	return &mockFile{file, "abcdef", 0}, nil
 }
 
diff --git a/internal/driver/fetch_test.go b/internal/driver/fetch_test.go
index b561151..bdd1732 100644
--- a/internal/driver/fetch_test.go
+++ b/internal/driver/fetch_test.go
@@ -147,7 +147,7 @@ type testObj struct {
 	home string
 }
 
-func (o testObj) Open(file string, start, limit, offset uint64) (plugin.ObjFile, error) {
+func (o testObj) Open(file string, start, limit, offset uint64, relocationSymbol string) (plugin.ObjFile, error) {
 	switch file {
 	case "/alternate/architecture/binary":
 		return testFile{file, "abcde10001"}, nil
diff --git a/internal/driver/webui_test.go b/internal/driver/webui_test.go
index 35fe472..81a68ff 100644
--- a/internal/driver/webui_test.go
+++ b/internal/driver/webui_test.go
@@ -181,7 +181,7 @@ func (f fakeObj) Symbols(r *regexp.Regexp, addr uint64) ([]*plugin.Sym, error) {
 
 type fakeObjTool struct{}
 
-func (obj fakeObjTool) Open(file string, start, limit, offset uint64) (plugin.ObjFile, error) {
+func (obj fakeObjTool) Open(file string, start, limit, offset uint64, relocationSymbol string) (plugin.ObjFile, error) {
 	return fakeObj{}, nil
 }
 
diff --git a/internal/symbolizer/symbolizer_test.go b/internal/symbolizer/symbolizer_test.go
index 50e33a6..c94e38b 100644
--- a/internal/symbolizer/symbolizer_test.go
+++ b/internal/symbolizer/symbolizer_test.go
@@ -263,7 +263,7 @@ func frame(fname, file string, line int) plugin.Frame {
 
 type mockObjTool struct{}
 
-func (mockObjTool) Open(file string, start, limit, offset uint64) (plugin.ObjFile, error) {
+func (mockObjTool) Open(file string, start, limit, offset uint64, relocationSymbol string) (plugin.ObjFile, error) {
 	return mockObjFile{frames: mockAddresses}, nil
 }
 
diff --git a/profile/profile_test.go b/profile/profile_test.go
index 0bf51a8..ee8da7e 100644
--- a/profile/profile_test.go
+++ b/profile/profile_test.go
@@ -249,6 +249,16 @@ var cpuM = []*Mapping{
 		HasLineNumbers:  true,
 		HasInlineFrames: true,
 	},
+	{
+		ID:              5,
+		Start:           0xffff000010080000,
+		Limit:           0xffffffffffffffff,
+		File:            "[kernel.kallsyms]_text",
+		HasFunctions:    true,
+		HasFilenames:    true,
+		HasLineNumbers:  true,
+		HasInlineFrames: true,
+	},
 }
 
 var cpuF = []*Function{
@@ -1434,6 +1444,13 @@ func TestSetMain(t *testing.T) {
 	}
 }
 
+func TestParseKernelRelocation(t *testing.T) {
+	src := testProfile1.Copy()
+	if src.Mapping[len(src.Mapping)-1].KernelRelocationSymbol != "_text" {
+		t.Errorf("got %s for Mapping.KernelRelocationSymbol", src.Mapping[0].KernelRelocationSymbol)
+	}
+}
+
 // parallel runs n copies of fn in parallel.
 func parallel(n int, fn func()) {
 	var wg sync.WaitGroup