Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go uprobes attachment fails for go 1.20.4 and later #1318

Closed
ddelnano opened this issue May 9, 2023 · 1 comment
Closed

Go uprobes attachment fails for go 1.20.4 and later #1318

ddelnano opened this issue May 9, 2023 · 1 comment
Assignees
Labels
area/datacollector Issues related to Stirling (datacollector) kind/bug Something isn't working

Comments

@ddelnano
Copy link
Member

ddelnano commented May 9, 2023

@vihangm tried to upgrade our go version to 1.20.4 (from 1.20.2) in #1266. After investigating this more, I determined that our go version detection fails for these newer binaries and prevents stirling from attaching uprobes to the application. Detecting an application's go version is necessary because applications built with go 1.18 and later must be handled different with our uprobes (source).

In addition to the go 1.20.4 issues, we recently discovered the same go version detection logic also doesn't handle 32 bit binaries properly (#1300). The go cli provides a mechanism for reading the go version of an application and its implementation is very different from what we currently do. We are proceeding with a go 1.20.4 specific fix to unblock #1266, but we need to rehaul our version detection to be more similar to Go's implementation so we can fix #1300 and issues with newer binaries.

@ddelnano ddelnano added kind/bug Something isn't working area/datacollector Issues related to Stirling (datacollector) labels May 9, 2023
@ddelnano ddelnano self-assigned this May 9, 2023
vihangm added a commit that referenced this issue May 10, 2023
…lures (#1319)

Summary: This upgrades our golang SDKs to the latest patch version
to pull in bug fixes and security fixes.

Our go tls tracing tests failed with this minor version upgrade. These
failures are due to our go version detection failing, which prevents
stirling from adding uprobes to the application. For now I've applied a
temporary fix for our go version detection, but in #1300 and #1318 we
need to revamp how this detection works since it has issues with 32 bit
binaries and now binaries built for go 1.20.4 and later.

Relevant Issues: N/A

Type of change: /kind cleanup

Test Plan: Existing tests should provide enough coverage.
- [x] Verified that `go_tls_trace_bpf_tests` failing on #1266 pass
([P381](https://phab.corp.pixielabs.ai/P381))

---------

Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Co-authored-by: Vihang Mehta <vihang@pixielabs.ai>
aimichelle pushed a commit that referenced this issue Jul 11, 2023
Summary: Implement Go's varint 64 bit integer decoding in binary decoder

Our go version detection has been the source of a few bugs (#1300,
#1318). This has highlighted that our go version detection is very
different than how the `go version` cli works. In order to align our
version detection with the go cli, we need to be able to decode these
varints since parts of Go's buildinfo header use this encoding
([source](https://github.com/golang/go/blob/1dbbafc70fd3e2c284469ab3e0936c1bb56129f6/src/debug/buildinfo/buildinfo.go#L185-L190)).

Relevant Issues: #1318 #1300

Type of change: /kind feature

Test Plan: Used the following [go
program](https://go.dev/play/p/x4x48NcxBMC) to draft the new test cases

<img width="1712" alt="Screenshot 2023-06-28 at 8 51 11 AM"
src="https://github.com/pixie-io/pixie/assets/5855593/67ed02b9-b855-417d-864b-589af2906dcd">

---------

Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
aimichelle pushed a commit that referenced this issue Jul 11, 2023
…tring_view` (#1592)

Summary: Make `LEndianBytesToInt` and `BEndianBytesToInt` compatible
with `u8string_view`

In order to parse Go's buildinfo header, we need to convert bytes
contained within a
[u8string_view](https://github.com/pixie-io/pixie/blob/c092058a5172396ea9bead1e209c2ed4e2336943/src/common/base/byte_utils.h#L30-L31)
(return values from
[ElfReader](https://github.com/pixie-io/pixie/blob/c092058a5172396ea9bead1e209c2ed4e2336943/src/stirling/obj_tools/elf_reader.cc#L608)).
This PR adds template specializations to the existing
`LEndianBytesToInt` and `BEndianBytesToInt` functions so they are
compatible with the `u8string_view` data type. This will allow the code
that will eventually parse a Go buildinfo header to be cleaner by
preventing an explicit conversion via `CreateStringView` as seen in
[this
commit](ae5dd20#diff-a5cd29c11ef9bc8c2ae92c1eb15bdace92d54e4e634fe5889a64979bd272e0b8R109-R110).

Relevant Issues: #1300 #1318

Type of change: /kind feature

Test Plan: Verified that the new test cases fail to compile without this
change and that this interface is more compatible for code linked above.

Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
@ddelnano
Copy link
Member Author

This was fixed in #1605.

ddelnano added a commit that referenced this issue Aug 15, 2024
…ries (#1976)

Summary: Address PEM crash caused by parsing certain older Go
application binaries

This PR fixes a crash caused by certain older Go application binaries.
In addition, this change includes the
`//src/stirling/binaries:go_binary_parse_profiling` cli tool. This tool
was helpful for debugging the previous 32 bit issue and aided in
debugging this problem (see Background section for more details). This
change is best reviewed commit by commit.

**Background**

Our Golang binary parsing was revamped in
#1605 to support Go 1.20.4
applications and later (#1318)
in addition to fixing a PEM crash caused by 32 bit go binaries
(#1300). While this solved the
aforementioned issues, it resulted in a new crash that we weren't able
to reproduce (#1646).

I was able to work with a Pixie Community Slack user to track down where
one of these issues originate from. The overview is that Go embeds
virtual addresses within the `.go.buildinfo` ELF section. These virtual
addresses are used in certain cases to read the build settings used when
the binary was created (toolchain version, go experiments, etc). In
order to properly read these strings, these virtual addresses need to be
converted into file offsets (binary addresses).

This bug presents itself when the `LOAD` ELF segments in the binary are
not contiguous or ordered by increasing virtual memory address. Meaning
if there are LOAD segments for segments 1, 2 and 3, this bug occurs if
those segments aren't adjacent to each other or don't have increasing
virtual memory addresses (vaddr of segment 1 < vaddr of segment 2 <
vaddr of segment 3). Instead the virtual address that needs to be looked
up, should be matched against the relevant segment and that segment's
virtual address offset should be used.

Relevant Issues: Partially addresses #1646 -- there is one more known
case, which must be investigated further

Type of change: /kind bug

Test Plan: Verified this change through the following
- [x] User from the community slack
[verified](https://pixie-community.slack.com/archives/CQ63KEVFY/p1722271309767939?thread_ts=1721315312.198319&cid=CQ63KEVFY)
that the issue was fixed.
- [x] New ElfReader function is covered with a test
- [x] go 1.17 test case ("[little
endian](https://github.com/pixie-io/pixie/blob/50ddcd32eb217e1aa5e87124883ee284a36052a1/src/stirling/obj_tools/go_syms_test.cc#L51)"
case) still works despite it not triggering this bug
- I was unable to recreate a binary that had the segments in an
unordered fashion.

Changelog Message: Fixed an issue with Go uprobe attachment that
previously caused crashes for a subset of older Go applications (Go 1.17
and earlier)

---------

Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/datacollector Issues related to Stirling (datacollector) kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant