[PATCH] D39888: [lsan] Only use VM_MEMORY_OS_ALLOC_ONCE on Darwin versions that support it

[Rainer Orth via Phabricator via llvm-commits]

ro added a comment.

In https://reviews.llvm.org/D39888#926120, @kubamracek wrote:

>




> As said previously, LSan is only tested on 10.11+ and I don't think anyone is going to support it on older macOS versions. If you just want to get it to compile (and not worry about it it not working), copying the constant from the header is the easiest way to go. But it's up to you.

So the effect of copying the constant or wrapping the code using it in #ifdef as my original patch did is effectively the same.  While I don't
understand the opposition to the #undef route, I'm fine either way.  It had been my understanding from some comments early in this
thread that the code wouldn't work reliably without VM_MEMORY_OS_ALLOC_ONCE support.  However, my test results in the gcc
tree have been that there were just a hand of failures.  I still haven't been able to build any current llvm version on OS X 10.7.  While
testing there would be more thorough, I cannot really justify the inordinate amount of time I'm spending there...

So if you're fine with some way of disabling either the lsan common code (e.g. via the CAN_SANITIZE_LEAKS route) or just avoiding
the undefined VM_MEMORY_OS_ALLOC_ONCE (via #ifdef or a private definition), I'll provide a patch.

> // Before Xcode 4.5, the Darwin linker doesn't reliably support undefined
>  // weak symbols.  Mac OS X 10.9/Darwin 13 is the first release only supported
>  // by Xcode >= 4.5.
>  #elif SANITIZER_MAC && \
> 
>   __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1090 && !SANITIZER_GO
> 
>   Can you explain? Is this the same problem? Another problem? If it's not specific to LSan then please move to a separate patch.

This is a separate problem indeed, which I hit only after some workaround for the first was in place.  Between the previous and current
import of libsanitizer into gcc, the sanitizer code started to assume that undefined weak symbols work on OS X, which isn't true for
older versions.

> Secondarily, I believe the code should still be working even when VM_MEMORY_OS_ALLOC_ONCE is not provided by the kernel: We will simply not find any VM_MEMORY_OS_ALLOC_ONCE regions and move on.

This agrees with the 10.7 testresults I get (although on a limited test set).


https://reviews.llvm.org/D39888