-
Notifications
You must be signed in to change notification settings - Fork 18k
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
runtime: stack split at bad time during fuzzing #53190
Comments
hint: ngolo-fuzzing uses libprotobufmutator |
I have no idea what it is doing but here is a one line fix :
|
inspired by https://go-review.googlesource.com/c/go/+/44390/ |
@catenacyber Thanks for reporting the issue and a potential fix. I'll have a look. |
cc @randall77 |
Change https://go.dev/cl/410034 mentions this issue: |
I've just created a pull request with a fix: https://go-review.googlesource.com/c/go/+/410034 |
Thanks Khaled |
@catenacyber The fix is now merged into master and you can give it another try. |
Thanks Khaled, google/oss-fuzz#7831 should remove the workaround. But you can see that this workaround was only applied if your patch was not applied yet, so everything looks good :-) |
I dug into this a bit. The Though adding //go:nosplit has fixed the immediate issue, I don't think runtime/internal/syscall should be getting instrumented at all (runtime is excluded, and runtime/internal/syscall is effectively part of runtime). |
Change https://go.dev/cl/413818 mentions this issue: |
@prattmic You are right. We don't need to instrument this package. However, the hooks should be marked as |
These functions can be inserted by the compiler into the code to be instrumented. This may result in these functions having callers that are nosplit. That is why they must be nosplit. This is a followup for CL 410034 in order to fix golang#53190.
Change https://go.dev/cl/413978 mentions this issue: |
These functions can be inserted by the compiler into the code to be instrumented. This may result in these functions having callers that are nosplit. That is why they must be nosplit. This is a followup for CL 410034 in order to fix #53190. Change-Id: I03746208a2a302a581a1eaad6c9d0672bb1e949a GitHub-Last-Rev: 6506d86 GitHub-Pull-Request: #53544 Reviewed-on: https://go-review.googlesource.com/c/go/+/413978 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
runtime/internal/syscall is a runtime package, so it should be built with -+. Specifically, we don't want libfuzzer instrumentation in Go functions defined in runtime/internal/syscall, which is disabled with -+. For #53190. Change-Id: I9f16f5c7c7ce10b98371e9de82fcea6da854e163 Reviewed-on: https://go-review.googlesource.com/c/go/+/413818 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
now that golang/go#53190 is closed
* infra: have timeout per fuzz target for coverage As is done for other languages * ngolo-fuzzing: remove temporary workaround now that golang/go#53190 is closed * ngolo-fuzzing: use built go toolchain in its directory without copying it to /root/.go/ in order to get coverage for std lib in the end * infra: ability to get coverage for additional golang package And uses it with ngolo-fuzzing : ngolo-fuzzing fuzz targets live in a different repository than the code being fuzzed, and we we want to get the coverage, for both the fuzz target and the package being fuzzed * fixup bash unbound * fixup ngolo-fuzzing only match at beginning for std package * stricter check for every additional go package
These functions can be inserted by the compiler into the code to be instrumented. This may result in these functions having callers that are nosplit. That is why they must be nosplit. This is a followup for CL 410034 in order to fix golang#53190. Change-Id: I03746208a2a302a581a1eaad6c9d0672bb1e949a GitHub-Last-Rev: 6506d86 GitHub-Pull-Request: golang#53544 Reviewed-on: https://go-review.googlesource.com/c/go/+/413978 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
runtime/internal/syscall is a runtime package, so it should be built with -+. Specifically, we don't want libfuzzer instrumentation in Go functions defined in runtime/internal/syscall, which is disabled with -+. For golang#53190. Change-Id: I9f16f5c7c7ce10b98371e9de82fcea6da854e163 Reviewed-on: https://go-review.googlesource.com/c/go/+/413818 Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
* infra: have timeout per fuzz target for coverage As is done for other languages * ngolo-fuzzing: remove temporary workaround now that golang/go#53190 is closed * ngolo-fuzzing: use built go toolchain in its directory without copying it to /root/.go/ in order to get coverage for std lib in the end * infra: ability to get coverage for additional golang package And uses it with ngolo-fuzzing : ngolo-fuzzing fuzz targets live in a different repository than the code being fuzzed, and we we want to get the coverage, for both the fuzz target and the package being fuzzed * fixup bash unbound * fixup ngolo-fuzzing only match at beginning for std package * stricter check for every additional go package
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Everything works fine with go 1.18 and go1.19 at commit e66f895
cc @kyakdan as this was introduced by your fix of #51318
It breaks also with latest commit 8a56c77
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I build ngolo-fuzzing project on oss-fuzz and run a fuzzer
cf google/oss-fuzz#7792
Let me know if you need more details
What did you expect to see?
Fuzzer running normally like
What did you see instead?
fatal error: runtime: stack split at bad time
Complete log is
cc @ianlancetaylor as you have been fixing bugs found by ngolo-fuzzing
cc @kyakdan as this was introduced by your fix of #51318
The text was updated successfully, but these errors were encountered: