fix: Deduce textstart from runtime.text symbol or moduledata for Go 1.26#1146
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21d6647152
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const textStartOff = 22 * 8 | ||
| if textStartOff+8 > len(moduleData) { | ||
| return 0, fmt.Errorf("invalid text start offset: %v", textStartOff) | ||
| } | ||
| textStart = *(*uintptr)(unsafe.Pointer(&moduleData[textStartOff])) | ||
| return textStart, nil |
There was a problem hiding this comment.
Apply relocations when reading moduledata.text
For stripped Go 1.26 PIE/external-linked binaries (no runtime.text symbol), this falls back to reading moduledata.text directly from the .go.module section bytes. Those pointer fields are relocated at load time for ET_DYN/PIE, and Section.Data returns the raw on-disk contents without applying relocations, so the value is often 0 or otherwise unrelocated. That means textStart can remain wrong specifically for stripped PIE binaries, regressing unwinding compared to the old .text-section fallback. Consider resolving relocations for .go.module (or falling back to .text if the unrelocated value is 0) before using it.
Useful? React with 👍 / 👎.
…the whole .go.module section into memory.
36e5dc9 to
f59dc48
Compare
| return 0, fmt.Errorf("could not read .go.module section at offset %v: %w", textOffset, err) | ||
| } | ||
|
|
||
| return *(*uintptr)(unsafe.Pointer(&textBytes[0])), nil |
There was a problem hiding this comment.
For easier maintainability we could use the following instead.
| return *(*uintptr)(unsafe.Pointer(&textBytes[0])), nil | |
| return uintptr(binary.LittleEndian.Uint64(textBytes[:])), nil |
There was a problem hiding this comment.
I was hesitating to use binary since the rest of the file was using unsafe casts.
open-telemetry#1146 added new executables in testdata. Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Fixes #1145
Starting from Go 1.26, textStart address in pclntab is always set to 0, currently address of
.textsection is used when textStart is 0, but that is not always correct (for example with cgo binaries or when built with -linkmode=external).The proposed change is to deduce textStart from either
runtime.textsymbol ormoduledata.textif there are not symbols.