[Sanitizer] Bump macOS deployment target for sanitizer unit test binary to support C++17 requirements.

[thetruestblue]

Summary

This patch fixes a test failure on Apple caused by changing standard to c++17.
sanitizer_allocator_test.cpp requires language features introducied in 10.13 for c++17.
After initial investigation, it was not clear how to add this flag to a single file:
https://reviews.llvm.org/D133878

Becuase of this, we have upped the min version of this test suite to 10.13, the min version necessary to support necessary language features.

We felt this was a better option than upping the min version of the product to support a single test.
We are raising deployment target for a single test suite, rather than the product.

[wrotki]

LGTM

[wrotki]

I mean, I'm good accepting it too

[rsundahl]

This looks like the best way to stay with c++17 so LGTM.

[yln]

LGTM, with nits:

[Apple][Sanitizer] Up min version of macos to support necessary language features for c++17

Please update the commit message so it mentions that we raising the deployment target for the test suite, not the the product, e.g.:

"[Sanitizer] Bump macOS deployment target for sanitizer unit test binary to support C++17 requirements"

compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt

106

Is the if ever not true after the bump to C++17? Is there risk to doing this unconditionally? If not, I would prefer to do it unconditionally.

[thetruestblue]

C++ standard can be manually overwritten and manually defined. As the codebase stands, without this overwrite, no this is never not true. And in the case of someone running Apple tests, I'm not sure *why* this would happen. But because the the mechanism exists, I added a conditional. I agree that guarding tests against every potential case might be unnecessary.

[thetruestblue]

removed conditionalw

[thetruestblue]

Updating commit message

[wrotki]

Reapproving commit message change

[rsundahl]

LGTM

[thakis]

Out of interest, what's the problem without this? Sanitizer tests were passing on our bots before this change too as far as I can tell.

compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt

 endif()
 
 if(APPLE)
   list(APPEND SANITIZER_TEST_CFLAGS_COMMON ${DARWIN_osx_CFLAGS})
   list(APPEND SANITIZER_TEST_LINK_FLAGS_COMMON ${DARWIN_osx_LINK_FLAGS})
 
   add_weak_symbols("sanitizer_common" WEAK_SYMBOL_LINK_FLAGS)
   list(APPEND SANITIZER_TEST_LINK_FLAGS_COMMON ${WEAK_SYMBOL_LINK_FLAGS})
+  # For c++17 sanitizer_allocator_test requires language features introduced in macos 10.13
+  list(APPEND SANITIZER_TEST_CFLAGS_COMMON "-mmacosx-version-min=10.13")
 endif()
 
 # MSVC linker is allocating 1M for the stack by default, which is not
 # enough for the unittests. Some unittests require more than 2M.
 # The default stack size for clang is 8M.
 if(MSVC)
   list(APPEND SANITIZER_TEST_LINK_FLAGS_COMMON -Wl,/STACK:0xC00000)
 endif()