[PATCH] D12707: [PATCH] [sanitizers] Add MSan support for AArch64

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 9 04:10:33 PDT 2015


rengolin added inline comments.

================
Comment at: lib/Transforms/Instrumentation/MemorySanitizer.cpp:125
@@ +124,3 @@
+// AArch64 has 3 VMA sizes: 39, 42 and 48.
+#ifndef SANITIZER_AARCH64_VMA
+# define SANITIZER_AARCH64_VMA 39
----------------
samsonov wrote:
> eugenis wrote:
> > What's that?
> > It's not even attached to any cmake setting.
> > Does it mean that a typical clang binary can target all of x86/arm/ppc/etc, 32/64, but only one fixed aarch64 kernel configuration?
> > 
> > I really think this should be a compiler flag.
> > 
> > And it should be possible to remove the corresponding macro in compiler-rt as well by exporting this setting through a global variable in the compiled objects (see __msan_keep_going or the ASAN_FLEXIBLE_MAPPING thing).
> > 
> +1.
> 
> I'm sorry that we didn't figure it out promptly during reviews where this define was added to another sanitizers. We should rework this - building compiler and runtimes with a secret define is not the way to go.
> What's that?

An unfortunate choice. We're in need of a better one.

> It's not even attached to any cmake setting.

CFLAGS. I know...

> Does it mean that a typical clang binary can target all of x86/arm/ppc/etc, 32/64, but only one fixed aarch64 kernel configuration?

I think so. If we want to target both 39 and 42, we'd have to create two separate libraries every time. With 48 coming to be the default in a year or so, we'd have to keep three. If that's acceptable, than I think we should create them all every time. If not, we need a way to select them at build time.

> I really think this should be a compiler flag.

Can you expand on that? This is a temporary implementation detail that is very specific to AArch64, and one that will disappear soon.

Currently, it *is* a compiler flag. Not a pretty one, but one that we expect to be used *very* rarely. In the future, when it all becomes 48 bits, it'll be used even less, and possibly deprecated altogether when we give up supporting the "old" VMA configurations.

Creating an -mvma or anything like that would be kinda pointless, and harder to revert later.


http://reviews.llvm.org/D12707





More information about the llvm-commits mailing list