Parse and propagate the name of the kernel relocation symbol (#675)
* Extend ObjTool to propagate the original mapping name.
For kernel mappings, the original mapping file name (not the name under
which we found the actual symbols file) is necessary to correctly
compute the kernel relocation offset. Preserve that name through
ObjTool.
Tested: unit.
* Preserve DSO name at decode time.
* Use DSO consistently instead of baseName
* Handle empty File correctly.
* Clarify merging / mapping semantics for DSO field.
* Switch extra field from DSO to a more narrow KernelRelocationOffset.
* Wording.
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 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 @@
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 @@
}
// 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 @@
// 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 @@
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/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 @@
} {
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 @@
} {
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 @@
} {
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 @@
// 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 @@
// 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 @@
} {
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/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 @@
// 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/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 @@
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.go b/internal/driver/fetch.go
index b8a69e8..0b36165 100644
--- a/internal/driver/fetch.go
+++ b/internal/driver/fetch.go
@@ -420,12 +420,14 @@
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 @@
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/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 @@
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 @@
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/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 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 @@
}
}
- 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 @@
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 @@
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/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 @@
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/encode.go b/profile/encode.go
index ab7f03a..96aa271 100644
--- a/profile/encode.go
+++ b/profile/encode.go
@@ -17,6 +17,7 @@
import (
"errors"
"sort"
+ "strings"
)
func (p *Profile) decoder() []decoder {
@@ -252,6 +253,14 @@
} 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 @@
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 @@
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
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 @@
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 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
