Remove maximum OSX version for sanitizers

[cjappl]

Remove a block preventing newer versions of the MacOS SDK from being selected for compiling the sanitizers.

As a backup proposal (if this is unacceptable), I think we should consider increasing the maximum version to (minimally) 10.15, or 14.0 (latest as of writing) to allow the sanitizers to intercept more modern calls like aligned_alloc and shared_mutex, introduced in 10.15 and 10.12 respectively.

History

The limitation on the max version of OSX was committed here, originally set to 10.9:
41c5288

It's not a good idea to build the sanitizers with e.g. -DCMAKE_OSX_DEPLOYMENT_TARGET=10.12, because some deprecated functions that we intercept will cause build errors. Let's limit the allowed deployment targets to 10.9 (which is the default anyway), and warn when it's set above."

Later, it was updated to be 10.10 in this commit:
b87fc09

Bump default value for SANITIZER_MIN_OSX_VERSION to 10.10 (from 10.9). TSan does not work on macOS 10.9 and a nice error message is preferable to an "unreferenced symbol" error when loading the TSan runtime.

We could try to only bump the deployment target for TSan, but we would have to invest into adding support for this to our CMake build and it does not seem worth it. macOS 10.10 was released in 2014.

This means the original error was that the sanitizers could not build with the errors produced in a newer version than 10.9, and later 10.10.

Why we may want to go higher than 10.10 (or not limit at all)

There are some sanitizers that do not intercept functions that they could because these APIs do not exist in the version we are targeting.

Take for example aligned_alloc, introduced in 10.15:

> cat /Library/Developer/CommandLineTools/SDKs/MacOSX14.0.sdk/usr/include/malloc/_malloc.h
...

void *aligned_alloc(size_t __alignment, size_t __size) ... __OSX_AVAILABLE(10.15) ... 

...

We have a few interceptors that cannot intercept aligned_alloc:

> rg INTERCEPT_ALIGNED_ALLOC
compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
495:#define SANITIZER_INTERCEPT_ALIGNED_ALLOC (!SI_MAC)

compiler-rt/lib/lsan/lsan_interceptors.cpp
145:#if SANITIZER_INTERCEPT_ALIGNED_ALLOC
151:#define LSAN_MAYBE_INTERCEPT_ALIGNED_ALLOC INTERCEPT_FUNCTION(aligned_alloc)
153:#define LSAN_MAYBE_INTERCEPT_ALIGNED_ALLOC
555:  LSAN_MAYBE_INTERCEPT_ALIGNED_ALLOC;

TSan seemingly would use aligned_alloc if it were available to it (but does not use the SANITIZER_INTERCEPT_ALIGNED_ALLOC call)

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

#if !SANITIZER_APPLE
TSAN_INTERCEPTOR(void*, aligned_alloc, uptr align, uptr sz) {
  if (in_symbolizer())
    return InternalAlloc(sz, nullptr, align);
  SCOPED_INTERCEPTOR_RAW(aligned_alloc, align, sz);
  return user_aligned_alloc(thr, pc, align, sz);
}
...
#endif

ASan shows aligned_alloc in their linux interceptors, but no such call exists in their mac interceptors

> cat compiler-rt/lib/asan/asan_malloc_linux.cpp | grep -C 2 "aligned_alloc"

#if SANITIZER_INTERCEPT_ALIGNED_ALLOC
INTERCEPTOR(void*, aligned_alloc, uptr boundary, uptr size) {
  GET_STACK_TRACE_MALLOC;
  return asan_aligned_alloc(boundary, size, &stack);
}
#endif // SANITIZER_INTERCEPT_ALIGNED_ALLOC

> cat compiler-rt/lib/sanitizer_common/sanitizer_malloc_mac.inc | grep -C 2 "aligned_alloc"
... (nothing)

Additionally, any future interceptor that wants to intercept calls introduced later than 10.10 will be blocked. For instance, in my own development I have been blocked intercepting shared_mutex (introduced 10.12) and aligned_alloc (introduced in 10.15).

Testing

I have tested 3 versions, the existing 10.10, 10.15 (the latest I personally care about), and 14.0 (the latest MacOS available as of writing). I was attempting to confirm that no regressions happened in the newer versions, and the limitation could be removed.

Each of these tests I:

1. Ran check-ubsan
2. Ran check-asan
3. Ran check-ubsan
4. Compiled clang, compiler-rt and llvm-symbolizer

Summary

No versions tested that were newer than 10.10 had any regressions or errors compiling. check-asan failed 137 tests against all versions, remaining the same, the others had no failures.

The newer versions (10.15 and 14.0) did report a number of deprecation warnings, but no compilation errors.

Full testing results

Testing done on this machine:

> system_profiler SPSoftwareDataType SPHardwareDataType
Software:

    System Software Overview:

      System Version: macOS 14.1.1 (23B81)
      Kernel Version: Darwin 23.1.0
      Boot Volume: Macintosh HD
      Boot Mode: Normal
      Computer Name: ...
      User Name: ...
      Secure Virtual Memory: Enabled
      System Integrity Protection: Enabled
      Time since boot: 22 days, 4 hours, 43 minutes

Hardware:

    Hardware Overview:

      Model Name: MacBook Pro
      Model Identifier: MacBookPro17,1
      Model Number: Z11C000E4LL/A
      Chip: Apple M1
      Total Number of Cores: 8 (4 performance and 4 efficiency)
      Memory: 16 GB
      System Firmware Version: 10151.41.12
      OS Loader Version: 10151.41.12

With MacOS version 10.10 (Existing default today)

cmake -G Ninja \
      -DCMAKE_BUILD_TYPE=Release \
      -DBUILD_SHARED_LIBS=ON \
      -DCOMPILER_RT_BUILD_SANITIZERS=ON \
      -DLLVM_ENABLE_PROJECTS="clang;compiler-rt" \
      -DLLVM_TARGETS_TO_BUILD=Native \
      -DSANITIZER_MIN_OSX_VERSION="10.10" \
      ../llvm

> cat CMakeCache.txt | grep SANITIZER_MIN
SANITIZER_MIN_OSX_VERSION:STRING=10.10

UBSan

> ninja check-ubsan

Total Discovered Tests: 261
  Unsupported      :  29 (11.11%)
  Passed           : 231 (88.51%)
  Expectedly Failed:   1 (0.38%)

3 warning(s) in tests

No failures

ASan

> ninja check-asan
Testing Time: 198.77s

Total Discovered Tests: 748
  Unsupported      : 237 (31.68%)
  Passed           : 372 (49.73%)
  Expectedly Failed:   2 (0.27%)
  Failed           : 137 (18.32%)

Failed: 137 tests

TSan

> ninja check-tsan

Total Discovered Tests: 441
  Unsupported      :  57 (12.93%)
  Passed           : 383 (86.85%)
  Expectedly Failed:   1 (0.23%)

1 warning(s) in tests

No failures

Building main targets

> ninja clang compiler-rt llvm-symbolizer
 ...
> echo $?
0

Succeeds

With MacOS version 10.15

cmake -G Ninja \
      -DCMAKE_BUILD_TYPE=Release \
      -DBUILD_SHARED_LIBS=ON \
      -DCOMPILER_RT_BUILD_SANITIZERS=ON \
      -DLLVM_ENABLE_PROJECTS="clang;compiler-rt" \
      -DLLVM_TARGETS_TO_BUILD=Native \
      -DSANITIZER_MIN_OSX_VERSION="10.15" \
      ../llvm

> cat CMakeCache.txt | grep SANITIZER_MIN
SANITIZER_MIN_OSX_VERSION:STRING=10.15

UBSan

> ninja check-ubsan

Total Discovered Tests: 261
  Unsupported      :  29 (11.11%)
  Passed           : 231 (88.51%)
  Expectedly Failed:   1 (0.38%)

3 warning(s) in tests

No failures

ASan

> ninja check-asan

Total Discovered Tests: 748
  Unsupported      : 237 (31.68%)
  Passed           : 372 (49.73%)
  Expectedly Failed:   2 (0.27%)
  Failed           : 137 (18.32%)

Failed: 137 tests - SAME AS 10.10

TSan

> ninja check-tsan

Total Discovered Tests: 441
  Unsupported      :  57 (12.93%)
  Passed           : 383 (86.85%)
  Expectedly Failed:   1 (0.23%)

1 warning(s) in tests

No failures

Building main targets

> ninja clang compiler-rt llvm-symbolizer
 ...
> echo $?
0

Succeeds

With MacOS version 14.0 (latest released)

cmake -G Ninja \
      -DCMAKE_BUILD_TYPE=Release \
      -DBUILD_SHARED_LIBS=ON \
      -DCOMPILER_RT_BUILD_SANITIZERS=ON \
      -DLLVM_ENABLE_PROJECTS="clang;compiler-rt" \
      -DLLVM_TARGETS_TO_BUILD=Native \
      -DSANITIZER_MIN_OSX_VERSION="14.0" \
      ../llvm

> cat CMakeCache.txt | grep SANITIZER_MIN
SANITIZER_MIN_OSX_VERSION:STRING=14.0

UBSan

> ninja check-ubsan

Total Discovered Tests: 261
  Unsupported      :  29 (11.11%)
  Passed           : 231 (88.51%)
  Expectedly Failed:   1 (0.38%)

3 warning(s) in tests

No failures

ASan

> ninja check-asan

Total Discovered Tests: 748
  Unsupported      : 237 (31.68%)
  Passed           : 372 (49.73%)
  Expectedly Failed:   2 (0.27%)
  Failed           : 137 (18.32%)

Failed: 137 tests - SAME AS 10.10

TSan

> ninja check-tsan
Total Discovered Tests: 261
  Unsupported      :  29 (11.11%)
  Passed           : 231 (88.51%)
  Expectedly Failed:   1 (0.38%)

3 warning(s) in tests

No failures

Building main targets

> ninja clang compiler-rt llvm-symbolizer
 ...
> echo $?
0

Succeeds

Future PRs

To not overload this PR, I omitted these, but they should be done in quick succession, which I'm happy to do.

A few sanitizers unnecessarily do not include interceptors for aligned_alloc which could be easily fixed.

1. Improve the SANITIZER_INTERCEPT_ALIGNED_ALLOC (!SI_MAC) to be version dependent, if the version is greater than 10.15, this should evaluate to true
2. Change TSAN_INTERCEPTOR to use the SANITIZER_INTERCEPT_ALIGNED_ALLOC macro, currently it is blocked out behind just !SANITIZER_APPLE
3. Add aligned_alloc to compiler-rt/lib/sanitizer_common/sanitizer_malloc_mac.inc, as it is missing, but all other allocations exist there.

Lastly, we may want to consider increasing the default version from 10.10:

  set(DEFAULT_SANITIZER_MIN_OSX_VERSION 10.10)

See "Why we may want to go higher" for details on these changes.

[cjappl]

Pinging people that were involved in my previous review (#74394) as this is in the same area of the code and is related to the same things (OSX, Sanitizers, CMake).

@petrhosek @yln @vitalybuka

Thanks in advance for any thoughts.

[yln]

Thank you Chris for this detailed write up. We really appreciate open source contributions to make sanitizers work better on Darwin.

I am happy with this PR since it just removes the "clamp max min version at some arbitrary (old) value". The default stays the same.

Comment on:

There are some sanitizers that do not intercept functions that they could because these APIs do not exist in the version we are targeting.

I don't think this is necessarily true. There is 2 issues to consider: building and availability at runtime.

• For building, if we want to intercept a newly-introduced API we usually "forward declare" the API (copy the minimal set of declarations necessary from the headers) in compiler-rt itself. This enables us to add interceptors for APIs without requiring all builders of compiler-rt to use the new SDK that declares this API.
• For running: we can include interceptors for APIs that are not available on our minimal deployment target. This intercepter will then be present in the resulting dylib, but will be dormant (never called) when the dylib is used on an older system that doesn't have the intercept target. If we copied the declaration, then we need to ensure the declaration of the intercept target has the right availability annotations so REAL(xxx) is indirected through a pointer (when availability > minimum deployment target). If we get this wrong we will get an undefined reference error at dylib load time. An example for this is TSan's interceptor for os_unfair_lock_lock with availability macOS 10.12 while our minimum deployment target is macOS 10.10.

intercept more modern calls like aligned_alloc

I am not 100% certain if there isn't a more tricky reason, but my guess for aligned_alloc not being intercepted is that simply no one has done the work to ensure it works yet. Again, we really appreciate open source contributions in this area.

On a different note: I am open to bumping the minimal deployment target (even if not strictly necessary) if it eases the maintenance burden.

cc: @thetruestblue @wrotki @rsundahl

[cjappl]

I don't think this is necessarily true. There is 2 issues to consider: building and availability at runtime.

Thanks for the clarity on this point, that makes a lot of sense! I was very surprised to think that these were so "behind" the newer OS.

For building, if we want to intercept a newly-introduced API we usually "forward declare" the API (copy the minimal set of declarations necessary from the headers) in compiler-rt itself. This enables us to add interceptors for APIs without requiring all builders of compiler-rt to use the new SDK that declares this API.

@yln could you point me to an example of this? I'm trying to find one for os_unfair_lock_lock but can't seem to find the forward declaration in the llvm-project source tree.

Understanding that will help me........

... aligned_alloc not being intercepted is that simply no one has done the work to ensure it works yet. Again, we really appreciate open source contributions in this area.

....do this! Happy to follow up with some PRs to do some of this, I'm in the area.

Lastly:

On a different note: I am open to bumping the minimal deployment target (even if not strictly necessary) if it eases the maintenance burden.

Let me know if you'd like to see this in this review, or a future one, and what version to move it to. As mentioned in the wall of text there are some deprecation warnings in newer versions while building, but no build errors.

[yln]

For building, if we want to intercept a newly-introduced API we usually "forward declare" the API (copy the minimal set of declarations necessary from the headers) in compiler-rt itself. This enables us to add interceptors for APIs without requiring all builders of compiler-rt to use the new SDK that declares this API.

@yln could you point me to an example of this? I'm trying to find one for os_unfair_lock_lock but can't seem to find the forward declaration in the llvm-project source tree.

The libdispatch APIs here (added in e0e0244):

llvm-project/compiler-rt/lib/tsan/rtl/tsan_dispatch_defs.h

Line 17 in 8246b2e

typedef struct dispatch_object_s {} *dispatch_object_t;

Albeit the reason wasn't to intercept a new API, but to reduce our dependencies on the SDK.

On a different note: I am open to bumping the minimal deployment target (even if not strictly necessary) if it eases the maintenance burden.

Let me know if you'd like to see this in this review, or a future one, and what version to move it to. As mentioned in the wall of text there are some deprecation warnings in newer versions while building, but no build errors.

In a separate PR please and with some justification "What does it buy us?". This will require extensive qualification on our side. I am deferring to @thetruestblue who already did some investigation on this.

[cjappl]

The libdispatch APIs here (added in e0e0244):

Great, thanks Julian! Very helpful.

In a separate PR please and with some justification "What does it buy us?". This will require extensive qualification on our side. I am deferring to @thetruestblue who already did some investigation on this.

Sounds good, I will leave this PR as it is now. I will also defer this version bump PR to someone else, as this decision is above my paygrade. However I am more than happy to be involved or help test, all someone has to do is reach out.

[cjappl]

Pinging @yln @vitalybuka @petrhosek @thetruestblue

For more reviews, or merging if it's appropriate.

Thank you

[yln]

Thank you, Chris!