Currently, there isn't a reliable way to build #20
Conversation
|
Tested on gceworker, However, I also needed to inject |
srosenberg
force-pushed
the
sr/support_libfuzzer_instrumentation
branch
from
8f31dca
to
e104af2
Compare
Go test binaries with libfuzzer instrumentation [1]. An attempt such as `bazel run --@io_bazel_rules_go//go/config:gc_goopts=-d=libfuzzer ...` fails to compile owing to unresolved symbols, e.g., runtime.libfuzzerTraceConstCmp8 [2]. This patch adds `libfuzzer_shim.go` (identical to `trace.go` [2]) to the `bzltestutil` package, which ensures the shim is linked with a Go test binary. Furthermore, we exclude `-d=libfuzzer`, when compiling any of the _external_ dependencies, or Go's stdlib. [1] bazel-contrib#3088 (comment) [2] golang/go@74f49f3
srosenberg
force-pushed
the
sr/support_libfuzzer_instrumentation
branch
from
e104af2
to
893191b
Compare
| @@ -343,6 +344,17 @@ func compileArchive( | |||
| cgoSrcs[i-len(goSrcs)] = coverSrc | |||
| } | |||
| } | |||
| if strings.Contains(outLinkObj, "external/") && slices.Contains(gcFlags, "-d=libfuzzer") { | |||
There was a problem hiding this comment.
@rickystewart I couldn't find a cleaner way to determine whether we're compiling an external dependency, but this seems to work. Maybe you have a better idea?
There was a problem hiding this comment.
This is probably how I would do it.
@rickystewart Is there an automated way to inject the shim to all the possible internal tools/generators? Otherwise, we may end up with a linker error (for a dependent generator) when building some other test binary. |
There was a problem hiding this comment.
However, I also needed to inject libfuzzer_shim.go into some of the internal tools/generators, which were require build dependencies [1].
[1] srosenberg/cockroach@24b0adb
@rickystewart Is there an automated way to inject the shim to all the possible internal tools/generators? Otherwise, we may end up with a linker error (for a dependent generator) when building some other test binary.
Can you go into a little more detail? I don't really know what "all possible internal tools/generators" means. Maybe an example?
| } | ||
| // Log instrumented objs for ease of tracking/debugging. | ||
| if slices.Contains(gcFlags, "-d=libfuzzer") { | ||
| fmt.Printf("%s -- gcFlags=%s\n", outLinkObj, gcFlags) |
There was a problem hiding this comment.
I don't think we want this checked in.
There was a problem hiding this comment.
Yep, I'll remove; couldn't find a standard way of logging debug messages, which would get suppressed by default.
| @@ -343,6 +344,17 @@ func compileArchive( | |||
| cgoSrcs[i-len(goSrcs)] = coverSrc | |||
| } | |||
| } | |||
| if strings.Contains(outLinkObj, "external/") && slices.Contains(gcFlags, "-d=libfuzzer") { | |||
There was a problem hiding this comment.
This is probably how I would do it.
I meant all targets which contain where |
I guess I still don't really get your point. But I think the point of any patch you're making here is that the build should "just work" either way. Does this not work unless we specifically drop
This doesn't really make any sense as the set of code that is linked into these helper binaries is not disjoint with the set of code linked into |
It does, but feels hacky. The minute someone creates a new package with a binary,
Yep, whatever you deem as the best path forward, I'll oblige :) |
OK, so what I expect then is that |
Go test binaries with libfuzzer instrumentation [1]. An attempt such as
bazel run --@io_bazel_rules_go//go/config:gc_goopts=-d=libfuzzer ...fails to compile owing to unresolved symbols, e.g., runtime.libfuzzerTraceConstCmp8 [2].This patch adds
libfuzzer_shim.go(identical totrace.go[2]) to thebzltestutilpackage, which ensures the shim is linked with a Go test binary. Furthermore, we exclude-d=libfuzzer, when compiling any of the external dependencies,or Go's stdlib.
[1] bazel-contrib#3088 (comment)
[2] golang/go@74f49f3