[compiler-rt] Replace deprecated os_trace calls on mac #138908

DanBlackwell · 2025-05-07T16:39:39Z

Currently there are deprecation warnings suppressed for os_trace; this patch replaces all uses with os_log_error.

rdar://140295247

github-actions · 2025-05-07T16:39:57Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-05-07T16:40:32Z

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Dan Blackwell (DanBlackwell)

Changes

Currently there are deprecation warnings suppressed for os_trace; this patch replaces all uses with os_log_error.

rdar://140295247

Full diff: https://github.com/llvm/llvm-project/pull/138908.diff

1 Files Affected:

(modified) compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp (+20-24)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index 0b8a75391136d..f02f78c4e19b4 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -844,30 +844,26 @@ void LogMessageOnPrintf(const char *str) {
 
 void LogFullErrorReport(const char *buffer) {
 #if !SANITIZER_GO
-  // Log with os_trace. This will make it into the crash log.
-#if SANITIZER_OS_TRACE
-#pragma clang diagnostic push
-// os_trace is deprecated.
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-  if (GetMacosAlignedVersion() >= MacosVersion(10, 10)) {
-    // os_trace requires the message (format parameter) to be a string literal.
-    if (internal_strncmp(SanitizerToolName, "AddressSanitizer",
-                         sizeof("AddressSanitizer") - 1) == 0)
-      os_trace("Address Sanitizer reported a failure.");
-    else if (internal_strncmp(SanitizerToolName, "UndefinedBehaviorSanitizer",
-                              sizeof("UndefinedBehaviorSanitizer") - 1) == 0)
-      os_trace("Undefined Behavior Sanitizer reported a failure.");
-    else if (internal_strncmp(SanitizerToolName, "ThreadSanitizer",
-                              sizeof("ThreadSanitizer") - 1) == 0)
-      os_trace("Thread Sanitizer reported a failure.");
-    else
-      os_trace("Sanitizer tool reported a failure.");
-
-    if (common_flags()->log_to_syslog)
-      os_trace("Consult syslog for more information.");
-  }
-#pragma clang diagnostic pop
-#endif
+#  if SANITIZER_OS_TRACE
+   // Log with os_log_error. This will make it into the crash log.
+   if (GetMacosAlignedVersion() >= MacosVersion(10, 10)) {
+     if (internal_strncmp(SanitizerToolName, "AddressSanitizer",
+                          sizeof("AddressSanitizer") - 1) == 0)
+       os_log_error(OS_LOG_DEFAULT, "Address Sanitizer reported a failure.");
+     else if (internal_strncmp(SanitizerToolName, "UndefinedBehaviorSanitizer",
+                               sizeof("UndefinedBehaviorSanitizer") - 1) == 0)
+     os_log_error(OS_LOG_DEFAULT,
+                  "Undefined Behavior Sanitizer reported a failure.");
+     else if (internal_strncmp(SanitizerToolName, "ThreadSanitizer",
+                               sizeof("ThreadSanitizer") - 1) == 0)
+       os_log_error(OS_LOG_DEFAULT, "Thread Sanitizer reported a failure.");
+     else
+       os_log_error(OS_LOG_DEFAULT, "Sanitizer tool reported a failure.");
+
+     if (common_flags()->log_to_syslog)
+       os_log_error(OS_LOG_DEFAULT, "Consult syslog for more information.");
+   }
+#  endif // SANITIZER_OS_TRACE
 
   // Log to syslog.
   // The logging on OS X may call pthread_create so we need the threading

wrotki

LGTM

thetruestblue · 2025-05-08T16:53:55Z

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

-#endif
+#  if SANITIZER_OS_TRACE
+   // Log with os_log_error. This will make it into the crash log.
+   if (GetMacosAlignedVersion() >= MacosVersion(10, 10)) {


Did we test this on 10.10? Is this the proper version for the new call?

I've now bumped the check to 10.12, but also the minimum supported macOS for sanitizers looks to be 10.13 anyway:

llvm-project/compiler-rt/cmake/config-ix.cmake

Line 475 in 790ce0e

set(DEFAULT_SANITIZER_MIN_OSX_VERSION 10.13)

.

thetruestblue · 2025-05-08T16:55:22Z

I think the right thing here may be to find out when the deprecation occurred and call the new calls after that version, but maintain the os_trace calls for the older versions.

github-actions · 2025-05-09T12:35:26Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

View the diff from clang-format here.

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index c1a4d5ac3..d9ff0c961 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -844,26 +844,26 @@ void LogMessageOnPrintf(const char *str) {
 
 void LogFullErrorReport(const char *buffer) {
 #if !SANITIZER_GO
-#  if SANITIZER_OS_TRACE
-   // Log with os_log_error. This will make it into the crash log.
-   if (GetMacosAlignedVersion() >= MacosVersion(10, 12)) {
-     if (internal_strncmp(SanitizerToolName, "AddressSanitizer",
-                          sizeof("AddressSanitizer") - 1) == 0)
-       os_log_error(OS_LOG_DEFAULT, "Address Sanitizer reported a failure.");
-     else if (internal_strncmp(SanitizerToolName, "UndefinedBehaviorSanitizer",
-                               sizeof("UndefinedBehaviorSanitizer") - 1) == 0)
-     os_log_error(OS_LOG_DEFAULT,
-                  "Undefined Behavior Sanitizer reported a failure.");
-     else if (internal_strncmp(SanitizerToolName, "ThreadSanitizer",
-                               sizeof("ThreadSanitizer") - 1) == 0)
-       os_log_error(OS_LOG_DEFAULT, "Thread Sanitizer reported a failure.");
-     else
-       os_log_error(OS_LOG_DEFAULT, "Sanitizer tool reported a failure.");
-
-     if (common_flags()->log_to_syslog)
-       os_log_error(OS_LOG_DEFAULT, "Consult syslog for more information.");
-   }
-#  endif // SANITIZER_OS_TRACE
+#    if SANITIZER_OS_TRACE
+  // Log with os_log_error. This will make it into the crash log.
+  if (GetMacosAlignedVersion() >= MacosVersion(10, 12)) {
+    if (internal_strncmp(SanitizerToolName, "AddressSanitizer",
+                         sizeof("AddressSanitizer") - 1) == 0)
+      os_log_error(OS_LOG_DEFAULT, "Address Sanitizer reported a failure.");
+    else if (internal_strncmp(SanitizerToolName, "UndefinedBehaviorSanitizer",
+                              sizeof("UndefinedBehaviorSanitizer") - 1) == 0)
+      os_log_error(OS_LOG_DEFAULT,
+                   "Undefined Behavior Sanitizer reported a failure.");
+    else if (internal_strncmp(SanitizerToolName, "ThreadSanitizer",
+                              sizeof("ThreadSanitizer") - 1) == 0)
+      os_log_error(OS_LOG_DEFAULT, "Thread Sanitizer reported a failure.");
+    else
+      os_log_error(OS_LOG_DEFAULT, "Sanitizer tool reported a failure.");
+
+    if (common_flags()->log_to_syslog)
+      os_log_error(OS_LOG_DEFAULT, "Consult syslog for more information.");
+  }
+#    endif  // SANITIZER_OS_TRACE
 
   // Log to syslog.
   // The logging on OS X may call pthread_create so we need the threading

DanBlackwell · 2025-05-09T17:17:37Z

I think the right thing here may be to find out when the deprecation occurred and call the new calls after that version, but maintain the os_trace calls for the older versions.

As noted, it seems that these new os_log_error calls are available in all supported macOS above the minimum version required for sanitizer support:

llvm-project/compiler-rt/cmake/config-ix.cmake

Line 475 in 790ce0e

set(DEFAULT_SANITIZER_MIN_OSX_VERSION 10.13)

.

yln

Good cleanup, thanks!

I have a few nits.

yln · 2025-05-12T15:23:49Z

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

-    // os_trace requires the message (format parameter) to be a string literal.
+#    if SANITIZER_OS_TRACE
+  // Log with os_log_error. This will make it into the crash log.
+  if (GetMacosAlignedVersion() >= MacosVersion(10, 12)) {


it seems that these new os_log_error calls are available in all supported macOS above the minimum version required for sanitizer support: ma

Could we completely omit the check then? No point in trying to preserve runtime compatibility with < macOS 10.13.

Have removed the check now.

yln · 2025-05-12T15:24:41Z

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

-  if (GetMacosAlignedVersion() >= MacosVersion(10, 10)) {
-    // os_trace requires the message (format parameter) to be a string literal.
+#    if SANITIZER_OS_TRACE
+  // Log with os_log_error. This will make it into the crash log.


If you haven't, please double-check this is still true with the new API.
Is there any difference in crash log format?

Behaviour seems identical - note that these changes only affect the single message that gets output on crash. The actual Asan crash report is unaffected.

yln · 2025-05-12T15:35:59Z

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-  if (GetMacosAlignedVersion() >= MacosVersion(10, 10)) {
-    // os_trace requires the message (format parameter) to be a string literal.
+#    if SANITIZER_OS_TRACE


We can now also remove the SANITIZER_OS_TRACE macro and #include <os/trace.h> above.

This file already includes #include <os/log.h>.

[compiler-rt] Replace deprecated os_trace calls on mac

598eb65

llvmbot added compiler-rt compiler-rt:sanitizer labels May 7, 2025

wrotki approved these changes May 7, 2025

View reviewed changes

thetruestblue requested review from yln and thetruestblue May 8, 2025 16:48

thetruestblue reviewed May 8, 2025

View reviewed changes

Updated 10.10 to 10.12 for min version

86bb81f

Formatting fix

42ccb48

yln requested changes May 12, 2025

View reviewed changes

yln reviewed May 12, 2025

View reviewed changes

yln assigned yln and DanBlackwell and unassigned yln May 12, 2025

DanBlackwell added 2 commits May 13, 2025 15:44

Addressed review comments

96dc7b0

Formatting fixes

38b7749

DanBlackwell requested a review from yln May 13, 2025 14:51

yln approved these changes May 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[compiler-rt] Replace deprecated os_trace calls on mac #138908

[compiler-rt] Replace deprecated os_trace calls on mac #138908

DanBlackwell commented May 7, 2025

github-actions bot commented May 7, 2025

llvmbot commented May 7, 2025

wrotki left a comment

thetruestblue May 8, 2025

DanBlackwell May 9, 2025

thetruestblue commented May 8, 2025

github-actions bot commented May 9, 2025

DanBlackwell commented May 9, 2025

yln left a comment

yln May 12, 2025

DanBlackwell May 13, 2025

yln May 12, 2025

DanBlackwell May 13, 2025

yln May 12, 2025

DanBlackwell May 13, 2025

[compiler-rt] Replace deprecated os_trace calls on mac #138908

Are you sure you want to change the base?

[compiler-rt] Replace deprecated os_trace calls on mac #138908

Conversation

DanBlackwell commented May 7, 2025

github-actions bot commented May 7, 2025

llvmbot commented May 7, 2025

wrotki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thetruestblue commented May 8, 2025

github-actions bot commented May 9, 2025

DanBlackwell commented May 9, 2025

yln left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment