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.

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: enable leak sanitizer in Go #67833

Open
znkr opened this issue Jun 5, 2024 · 3 comments
Open

runtime: enable leak sanitizer in Go #67833

znkr opened this issue Jun 5, 2024 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest Issues asking for a new feature that does not need a proposal. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@znkr
Copy link
Contributor

znkr commented Jun 5, 2024

Proposal Details

This is follow-up to #44853 which proposed to enable ASAN for Go.

One type of error in Go / C interop are memory leaks. These can be detected by ASAN using ASAN_OPTIONS=detect_leaks=1. Unfortunately, this doesn't fully work with Go's ASAN integration. For example, allocating a C object and storing it in a global variable in Go is detected as a leak.

I believe to make LSAN work, all we need to do is to tell it about memory regions that Go manages to consider as roots using __lsan_register_root_region.

@znkr znkr added the Proposal label Jun 5, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 5, 2024
@ianlancetaylor
Copy link
Member

I don't think this has to be a proposal, taking it out of the proposal process. We should just do this as part of our ordinary sanitizer support.

It sounds like this will be pretty loose, unless there is some way to tell the sanitizer which pointers are live. But a conservative leak detector is still better than no leak detector at all.

@mknyszek mknyszek added this to the Unplanned milestone Jun 5, 2024
@mknyszek mknyszek added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FeatureRequest Issues asking for a new feature that does not need a proposal. labels Jun 5, 2024
@znkr
Copy link
Contributor Author

znkr commented Jun 6, 2024

I took another look into LSAN, to understand how conservative it would be. https://maskray.me/blog/2023-02-12-all-about-leak-sanitizer was quite helpful. What that article doesn't document is the use_poisoned flag. IIUC, LSAN can use ASAN poisoning information when scanning for pointers and will ignore pointers in poisoned memory. I think that Go already poisons freed memory, so it might be possible to make this pretty accurate.

@ruyi789
Copy link

ruyi789 commented Jun 6, 2024

It's a completely redundant feature, the example code doesn't detect the array size at all, and of course it's also your freedom, so why should you be reminded of it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FeatureRequest Issues asking for a new feature that does not need a proposal. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

5 participants