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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: //go:norace for ASAN and other instrumentation #64310

Closed
znkr opened this issue Nov 21, 2023 · 7 comments
Closed

cmd/compile: //go:norace for ASAN and other instrumentation #64310

znkr opened this issue Nov 21, 2023 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@znkr
Copy link
Contributor

znkr commented Nov 21, 2023

What version of Go are you using (go version)?

tip

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

Linux with GOARCH='amd64' and GOARCH='arm64'

What did you do?

Our Go toolchain contains a patch that allows handling Go runtime crashes for reporting. Code that needs to run as part of that patch is very restricted. The biggest restriction is that it doesn't allow memory allocation. It's possible to write Go code that doesn't allocate memory, but it's near impossible when the code is instrumented. For example, to make the crash handling work with the race detector, all functions have to be annotated with //go:norace.

We're now running into this problem again when enabling ASAN using -asan. The problem is that when ASAN is enabled, the existing code starts to allocate memory where it doesn't without the instrumentation. My best guess is that the ASAN instrumentation leads to different escape analysis results. (I suspected inlining decisions as well, but I don't see any additional allocations when I disable inlining completely).

It would help a lot to be able to either disable ASAN instrumentation for crash handling functions with //go:noasan or, even better, to disable all instrumentation with, say,//go:noinstrumentation.

/cc @cherrymui, @dr2chase
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 21, 2023
@mauri870
Copy link
Member

I wonder if this related to #64257

@mknyszek mknyszek added this to the Backlog milestone Nov 21, 2023
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 21, 2023
@cherrymui
Copy link
Member

I wonder if this related to #64257

Not directly related, although it can be, e.g. it is possible that in order to make all tests passing some function needs to be marked no-instrumentation.

@zhangfannie
Copy link
Contributor

The problem is that when ASAN is enabled, the existing code starts to allocate memory where it doesn't without the instrumentation. My best guess is that the ASAN instrumentation leads to different escape analysis results. (I suspected inlining decisions as well, but I don't see any additional allocations when I disable inlining completely).

@znkr The reason for allocating memory may be that when -asan is turned on, the -checkptr option is implicitly set to 2, which is implemented by CL 393315, so some stack objects with usafe.Pointer operations will escape to the heap, resulting in additional heap allocation. Does your code have stack objects with unsafe.Pointer operations?

I'm a little curious why no additional memory is allocated after disabling inlining. Does anyone know about this? Thanks.

@znkr
Copy link
Contributor Author

znkr commented Nov 22, 2023

@znkr The reason for allocating memory may be that when -asan is turned on, the -checkptr option is implicitly set to 2, which is implemented by CL 393315, so some stack objects with usafe.Pointer operations will escape to the heap, resulting in additional heap allocation. Does your code have stack objects with unsafe.Pointer operations?

Thank you, that's probably it. The code is unsafe.Pointer heavy out of necessity. I suspect there's no annotation to turn that off?

@cherrymui
Copy link
Member

Thanks. There is //go:nocheckptr. Maybe you'd like to try that?

@znkr
Copy link
Contributor Author

znkr commented Nov 23, 2023

Thank you! I don't have time to test this right away, but I'll get to it eventually and will report back.

@zhangfannie zhangfannie mentioned this issue Nov 27, 2023
Open
@znkr
Copy link
Contributor Author

znkr commented Nov 27, 2023

This worked, thank you very much!

@znkr znkr closed this as completed Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
6 participants
@mknyszek @gopherbot @mauri870 @cherrymui @zhangfannie @znkr