debug/buildinfo: may use lots of memory if data segment is large #68592
Comments
prattmic
added
the
NeedsFix
Sorry, |
|
Change https://go.dev/cl/601460 mentions this issue: |
|
Change https://go.dev/cl/601457 mentions this issue: |
|
Change https://go.dev/cl/601458 mentions this issue: |
|
Change https://go.dev/cl/601456 mentions this issue: |
|
Change https://go.dev/cl/601459 mentions this issue: |
Existing documentation is a bit sparse, and more importantly focuses almost entirely on the old pre-1.18 format, with the new format as an afterthought. Since the new format is the primary format, make it more prominent. Updates #68592. Change-Id: I108ecde1b33650b4812fa5d278b08cb9197f6329 Reviewed-on: https://go-review.googlesource.com/c/go/+/601456 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Michael Pratt <mpratt@google.com>
There is currently no coverage for the pre-1.18 buildinfo format, or for parsing non-Go binaries. Add basic tests for each of these. Updates #68592. Change-Id: Iec14d29ffc1392e46f592c0c7bebf2eb75f7d0d3 Reviewed-on: https://go-review.googlesource.com/c/go/+/601457 Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Michael Pratt <mpratt@google.com>
Updates #68592. Change-Id: I00c6c740ca0bdd19af24e08a219ec3c90196097e Reviewed-on: https://go-review.googlesource.com/c/go/+/601458 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Michael Pratt <mpratt@google.com>
Allocating a new buffer for each chunk in searchMagic is very inefficient. Refactor reading to allow us to reuse the same buffer for each iteration. This reduces the runtime of `go version` on a 2.5GB non-Go binary from ~1s and ~25MB RSS to ~250ms and ~15MB RSS. For #68592. Change-Id: Idae5c2c9b3b8a7158d5cc7f2f008998be75fd7af Reviewed-on: https://go-review.googlesource.com/c/go/+/601460 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
|
Change https://go.dev/cl/602435 mentions this issue: |
If the length does not fit in int, saferio.ReadDataAt returns io.ErrUnexpectedEOF. Treat is as an invalid format. Fixes #68692. For #68592. Cq-Include-Trybots: luci.golang.try:gotip-linux-386-longtest Change-Id: Ie856f29c907fd10e6d9b7dfbb6f0d8008a75a1c6 Reviewed-on: https://go-review.googlesource.com/c/go/+/602435 Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Since https://go.dev/cl/514075,
debug/buildinforeads the entire "data segment/section" containing the buildinfo in one go, regardless of size.Normally,
DataStartreturns the.go.buildinfosection, which should be a modest size (and we'll need all of it anyway). But if the binary is stripped, or simply not a Go binary, then as a fallback it returns the first loadable data segment, which could be arbitrarily large.This can be painful for tools scanning binaries in low-memory environments (e.g., CI), as the data segment could be quite large.
This is conceptually similar to #68454, but more straightforward to fix as this is all internal and could do chunked reading.
cc @mknyszek @zpavlinovic
The text was updated successfully, but these errors were encountered: