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

build: add memory/address sanitizer builder #50394

Open
mengzhuo opened this issue Dec 30, 2021 · 5 comments
Open

build: add memory/address sanitizer builder #50394

mengzhuo opened this issue Dec 30, 2021 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. new-builder
Milestone

Comments

@mengzhuo
Copy link
Contributor

mengzhuo commented Dec 30, 2021

We have builders that runs compiler and testes in race mode.
#19962

But not any sanitizer modes (i.e. address/memory)
After I enable msan/asan mode in dist like CL374974 do. There are some std libraries (runtime, reflect, runtime/pprof, constraints etc,.) failed both the sanitizers.

I think we need builder that runs msan/asan and clear all failures in msan/asan mode.

CC @rsc @ianlancetaylor @aclements @cherrymui

@cherrymui
Copy link
Member

Do you want to build the toolchain (the compiler, linker, assembler) in msan/asan mode, or the standard library? They are two separate things. It's unclear from your message what you want.

For the former (the toolchain), there may not be much value for it. Those sanitizers are mostly for catching errors for mixed C/Go programs. And the toolchains are pure-Go programs.

For the latter, there may be some value. Not for catching bugs in the standard library, but to make sure the instrumentation is consistent and won't fail when user importing those packages.

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 30, 2021
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Dec 30, 2021
@ianlancetaylor
Copy link
Contributor

The goal here is to run the tests with -msan or -asan. I agree that the purpose of this would be to catch cases where we aren't correctly instrumenting Go code, or where assembler code in the standard library isn't correctly recording its memory writes. This seems like a reasonable thing to do though it doesn't seem high priority.

For example, it's a good point that go test -msan reflect fails today:

Uninitialized bytes in __msan_check_mem_is_initialized at offset 0 inside [0x00c00000e5d0, 8)
==1261422==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x505b68  (/tmp/go-build2132864169/b001/reflect.test+0x505b68)

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/tmp/go-build2132864169/b001/reflect.test+0x505b68) 
Exiting
FAIL	reflect	1.917s
FAIL

It fails because it thinks that the memory accessed by the compiler-constructed method reflect_test.(*Outer).M does what appears to be a read of uninitialized memory. I'm confident that the problem is actually that nothing told msan that the memory was initialized, but that does still seem like a bug worth fixing.

@mengzhuo mengzhuo changed the title build: run memory/address sanitizer toolchain build: add memory/address sanitizer builder Dec 31, 2021
@mengzhuo
Copy link
Contributor Author

Do you want to build the toolchain (the compiler, linker, assembler) in msan/asan mode, or the standard library? They are two separate things. It's unclear from your message what you want.

For the former (the toolchain), there may not be much value for it. Those sanitizers are mostly for catching errors for mixed C/Go programs. And the toolchains are pure-Go programs.

For the latter, there may be some value. Not for catching bugs in the standard library, but to make sure the instrumentation is consistent and won't fail when user importing those packages.

I mean the latter one. Thanks for clearing things up. :)

@gopherbot
Copy link

Change https://golang.org/cl/375254 mentions this issue: src: add sanitizer.bash

@gopherbot
Copy link

Change https://golang.org/cl/374496 mentions this issue: dashboard: add msan/asan builder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. new-builder
Projects
None yet
Development

No branches or pull requests

5 participants