Enable MSAN support again #46348
Enable MSAN support again #46348
Conversation
|
I believe JITLink has support for TLS relocation so we might get that for free when we enable it soon ™️ https://groups.google.com/g/llvm-dev/c/iTAdTFQ-ql8 |
Analogous to (and dependent on) #46336, but for msan. The biggest change is a workaround for LLVM's lack for TLS relocation support (I had a fix for that about 7 years ago at https://reviews.llvm.org/D8815, but that was x86_64-linux-gnu only, while this workaround works everywhere - though we should consider resurrecting my patch for performance at some point). The rest is mostly build fixes and plumbing to get the sanitizer flags through to the dependencies.
| @@ -856,8 +856,11 @@ uint8_t *RTDyldMemoryManagerJL::allocateCodeSection(uintptr_t Size, | |||
| StringRef SectionName) | |||
| { | |||
| // allocating more than one code section can confuse libunwind. | |||
| #if defined(_COMPILER_MSAN_ENABLED_) | |||
| // TODO: Figure out why msan needs this. | |||
| assert(!code_allocated); | |||
There was a problem hiding this comment.
Why is this assert now only used when MSAN is present? The memory manager can't handle recursive allocations anyways, so the assert guards against that?
There was a problem hiding this comment.
Sorry, this was a typo. Obviously the condition should be inverted.
There was a problem hiding this comment.
Even so, I'm surprised that simply disabling the assert works for MSAN at all. Whenever I've disabled that assert the memory manager dies later on with a deeper error.
There was a problem hiding this comment.
I don't know - I haven't looked into it further. You're welcome to it ;)
| @@ -5,7 +5,7 @@ OPENBLAS_GIT_URL := https://github.com/xianyi/OpenBLAS.git | |||
| OPENBLAS_TAR_URL = https://api.github.com/repos/xianyi/OpenBLAS/tarball/$1 | |||
| $(eval $(call git-external,openblas,OPENBLAS,,,$(BUILDDIR))) | |||
|
|
|||
| OPENBLAS_BUILD_OPTS := CC="$(CC)" FC="$(FC)" LD="$(LD)" RANLIB="$(RANLIB)" TARGET=$(OPENBLAS_TARGET_ARCH) BINARY=$(BINARY) | |||
| OPENBLAS_BUILD_OPTS := CC="$(CC) $(SANITIZE_OPTS)" FC="$(FC) $(SANITIZE_OPTS) -L/home/keno/julia-msan/usr/lib" LD="$(LD) $(SANITIZE_LDFLAGS)" RANLIB="$(RANLIB)" BINARY=$(BINARY) | |||
There was a problem hiding this comment.
-L/home/keno/julia-msan/usr/lib
Is that supposed to be something else?
|
This commit breaks build from source of suite-sparse on macOS because system linker (Apple's ld) does not recognise the |
Analogous to (and dependent on) #46336, but for msan. The biggest
change is a workaround for LLVM's lack for TLS relocation support
(I had a fix for that about 7 years ago at https://reviews.llvm.org/D8815,
but that was x86_64-linux-gnu only, while this workaround works
everywhere - though we should consider resurrecting my patch for
performance at some point). The rest is mostly build fixes and
plumbing to get the sanitizer flags through to the dependencies.