gopclntab: Use address of .text section for Go 1.26

[florianl]

Starting with Go 1.26 the field textStart in the gopclntab header is set to 0. Use the address of the .text section instead.

[christos68k]

I looked at the upstream diff quickly and it now documents:

entryOff uint32 // start pc, as offset from moduledata.text

So is it guaranteed that moduledata.text will always match ELF .text?

[florianl]

Yes - moduledata.text does always match .text. If this changes in the future, other changes need to happen first in the Go upstream project.

[christos68k]

I grepped for textStart (to see how it's used in codebase), and saw this which implies that moduledata.text need not always match .text.

The textStart field does not necessarily point to the first byte of the
.text section and might be preceded by libc functions. It points to the first
Go generated function.

[florianl]

As explained in the linked change (golang/go@0e1bd8b but also https://go-review.googlesource.com/c/go/+/717240) the behavior of the linker changed. Prior to this change, it was possible, that .text might be preceded by libc functions.

The textStart field requires a relocation, the only relocation in pclntab.
And nothing uses it. So remove it. Replace it with a zero [..]

[christos68k]

The change that you're referring to only documents that textStart has been effectively removed to get rid of the previously required relocation. It says nothing about moduledata.text semantics (which AFAICT remain exactly the same as before).

Since moduledata.text previously stored the same value as textStart, the comment I posted above would also apply to it meaning "moduledata.text does not necessarily point to the first byte of the .text section ...". So either our documentation for this is wrong, or we can't always assume that moduledata.text will match .text.

I just want to ensure we're not missing something here. If we don't want to make this assumption, we need to abide by what the Go code documents and use moduledata.text.

@fabled WDYT?

[fabled]

Looking at the code, the equivalent was symbol runtime.text. But that can be stripped away. Looking at a built Go executable, the .text section address seems same, but I am not sure yet how it behaves with CGO or other non-Go code.

[florianl]

If one sets GOTOOLCHAIN=go1.26rc1 in the testdata Makefile we can show already, that the proposed change works fine.

[fabled]

Yes - moduledata.text does always match .text. If this changes in the future, other changes need to happen first in the Go upstream project.

Do you have a Go source code reference for this?

The textStart field requires a relocation, the only relocation in pclntab. And nothing uses it. So remove it. Replace it with a zero [..]

This is because Go runtime internally uses the moduledata.text for pclntab parsing. And when it parses pclntab from external executable, it uses the ELF symbol runtime.text. Neither of them use ELF .text section address.

If one sets GOTOOLCHAIN=go1.26rc1 in the testdata Makefile we can show already, that the proposed change works fine.

Yes, it works as long as the assumption is true. Did you try all possible variations of linking different CGO, external code, and all different PIC/PIE etc build flags?

I'm not saying the change here is wrong. I think both me and @christos68k just want some documentation that it is always true regardless of the build flags and other variables.

I read the Go linker code, and I think this is correct code. The code paths for generation of runtime.text seem to be based on the text section.

As side note, if we want to support inlining, we do have to find runtime.moduledata and if this is done, we should probably get the .text start from that data instead for consistency.

[christos68k]

If one sets GOTOOLCHAIN=go1.26rc1 in the testdata Makefile we can show already, that the proposed change works fine.

The question here is, should we abide by what the Go documentation explicitly describes, and use moduledata.text or should we assume that moduledata.text will always match .text ?

The latter assumption conflicts with our documentation here which explicitly mentions a case where that's not true.

Making that assumption is fine with me as long as we document it and add a caveat that it might not always hold (also to avoid conflict/confusion with the bit of documentation I linked and make our lives easier in case of future breakage).

[florianl]

[..] just want some documentation that it is always true regardless of the build flags and other variables.

To my knowledge there exists no such documentation.

[..] conflicts with our documentation here which explicitly mentions a case where that's not true

The linked documentation does not mention the case where this is true. Maybe this part of the documentation is not correct?

if we want to support inlining, we do have to find runtime.moduledata and if this is done [..]

This should not be part of this proposed change I think. As Go 1.26 RC1 was released, I just want to make sure ebpf-profiler works with this new release. Supporting inlining should be a dedicated task.

[bobrik]

Should this have enabled Go 1.26 binaries? I recompiled ebpf-profiler itself with go1.26rc2 and it couldn't unwind itself anymore. Rolling back to go1.25 fixed the issue.