fix: do not incorrectly symbolize addresses beyong gopclntab end

[rogercoll]

sort.Seach package docs:

If there is no such index, Search returns n. (Note that the "not found" return value is not -1 as in, for instance, strings.Index.) Search calls f(i) only for i in the range [0, n).

[rogercoll]

Is this subtraction needed? Coredump tests are failing because the Go symbolization does not map anymore, I am wondering if this was added because assuming "if search not found" returns 0

cc @open-telemetry/ebpf-profiler-approvers

[fabled]

As you see, this PR results tests failing and the change is incorrect.

The reason is that sort.Search lambda is designed so that the returned entry is the first entry that has address greater than pc. But we are interested in the entry that is the first below the pc. Thus the -1 is needed to adjust the result to the wanted record.

The search needs to be like due to the way sort.Search works, and how the on-disk map table is sorted.

Hope this clarifies the issue?

[rogercoll]

Thanks for the context, hope 8c266b5 fixes the search result. (Although some MacOS/arm64 tests might need to be fixed)

[fabled]

No the fix is incorrect. It breaks tests where the correct match is against the first entry. There's something more in the issue at hand.

[fabled]

Does this work:

--- a/nativeunwind/elfunwindinfo/elfgopclntab.go
+++ b/nativeunwind/elfunwindinfo/elfgopclntab.go
@@ -541,11 +541,11 @@ func (g *Gopclntab) mapPcval(offs int32, startPc, pc uint) (int32, bool) {
 
 // Symbolize returns the file, line and function information for given PC
 func (g *Gopclntab) Symbolize(pc uintptr) (sourceFile string, line uint, funcName string) {
-       index := sort.Search(g.numFuncs, func(i int) bool {
+       index := sort.Search(g.numFuncs + 1, func(i int) bool {
                funcPc, _ := g.getFuncMapEntry(i)
                return funcPc > pc
        }) - 1
-       if index < 0 {
+       if index < 0 || index >= g.numFuncs {
                return "", 0, ""
        }
 

This is valid because in .gopclntab the function table contains one extra item in the end which is the end-of-the-code entry. This should fix the

[fabled]

If it works, I can prepare a PR with proper explanation and also amend some comments on how the lookup is designed to work.

[rogercoll]

Yes, these changes work. Moving the PR ready for review, but feel free to close it and fix it in another one if preferred.

[fabled]

Approving. Perhaps you can commit the suggested additional commit (committing from web ui should also mark me as co-author).