search

bazel-contrib / rules_go

Go rules for Bazel
Apache License 2.0
1.38k stars 656 forks source link

Expose an option or flag to disable stripping of symbols on `go_test` targets without `--@io_bazel_rules_go//go/config:debug` #4103

Open rickystewart opened 1 month ago

rickystewart commented 1 month ago

As of #2573, builds of go_test targets will always result in a binary with its symbols. In #2691, it's reiterated this is working as intended.

This behavior can be disabled with --@io_bazel_rules_go//go/config:debug, but that has the additional implication of disabling inlining. So, if you are trying to diagnose a problem or especially debug anything related to performance, this is not an option you realistically have as any investigation you will do will be on a generated binary that is probably substantively different from your production binary.

In the GitHub issues discussing this, it's claimed that this behavior was adopted for a performance benefit. Maybe that is true (I haven't profiled anything using recent versions of go, although it's possible the situation has changed in 4 years), but there should be some option that one can apply to disable this stripping where relevant without enabling any other command-line flags that would impact the performance or behavior of the generated binary.

I can see two obvious options:

  1. Provide a new flag (e.g. --@io_bazel-rules_go//go/config:no_strip_tests). Update the logic here to not add -s -w if either debug or no_strip_tests are set, instead of only checking debug.
  2. Honor --strip, i.e. do not strip these binaries if --strip=never is set.

Alternatively this stripping behavior could be updated to be opt-in instead of mandatory. It could also be re-evaluated whether this optimization is needed on the latest versions of the Go SDK.

If it's agreed option 1 ("Provide a new flag...") is best, I can submit a PR to that effect.

fmeum commented 1 month ago

I would very much like to do what go does by default. Do you happen to know what it does nowadays?

I like the idea of honoring --strip=never more than adding a new flag.

rickystewart commented 1 month ago

By default, with no other flags set, go test -c builds a binary that does not have symbols stripped, i.e., go tool objdump on the binary does not return any sort of error. (With the symbols stripped with a Bazel-built test binary, we see that go tool objdump returns the error objdump: disassemble ...: no symbol section)

Interestingly, the previous comments on the topic seem to suggest that rules_go's current behavior was, at least at the time, consistent with what go does by default. Maybe that was the case at the time and go updated its default behavior. Today, the behavior is not consistent with what go does by default (and unfortunately outside of patching rules_go there is no way to get that behavior).

fmeum commented 1 month ago

@jayconrod Do you happen to know whether this changed?

rickystewart commented 1 month ago

By default, with no other flags set, go test -c builds a binary that does not have symbols stripped, i.e., go tool objdump on the binary does not return any sort of error. (With the symbols stripped with a Bazel-built test binary, we see that go tool objdump returns the error objdump: disassemble ...: no symbol section)

(I'm talking specifically about Linux, by the way. Mac seems to behave slightly differently.)