[compiler-rt] [asan] Use InternalMmapVector for error_message_buffer, reallocate wh… (PR #77488)

via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 8 18:21:18 PST 2024


Enna1 wrote:

ping!

BTW, do you think move `error_message_buffer` into `ScopedInErrorReport` as a class member is a better approach?
IMHO, this approach is more clear. 

But with this approach, there will be a behavior change:
- Currently, if asan and ubsan both enabled, ubsan error report will be mixed with asan error report, and passed to error report callback
- With this approach, if asan and ubsan both enabled, ubsan error report will **not** be mixed with asan error report, only asan error report will be passed to error report callback

WDYT?

``` diff
--- a/compiler-rt/lib/asan/asan_report.cpp
+++ b/compiler-rt/lib/asan/asan_report.cpp
@@ -24,6 +24,7 @@
 // -------------------- User-specified callbacks ----------------- {{{1
 static void (*error_report_callback)(const char*);
-static char *error_message_buffer = nullptr;
-static uptr error_message_buffer_pos = 0;
+typedef InternalMmapVectorNoCtor<char, true> ErrorMessageBuffer;
+static ErrorMessageBuffer *error_message_buf_ptr = nullptr;
 static Mutex error_message_buf_mutex;
 static const unsigned kAsanBuggyPcPoolSize = 25;
 static __sanitizer::atomic_uintptr_t AsanBuggyPcPool[kAsanBuggyPcPoolSize];
 
 void AppendToErrorMessageBuffer(const char *buffer) {
   Lock l(&error_message_buf_mutex);
-  if (!error_message_buffer) {
-    error_message_buffer =
-      (char*)MmapOrDieQuietly(kErrorMessageBufferSize, __func__);
-    error_message_buffer_pos = 0;
-  }
-  uptr length = internal_strlen(buffer);
-  RAW_CHECK(kErrorMessageBufferSize >= error_message_buffer_pos);
-  uptr remaining = kErrorMessageBufferSize - error_message_buffer_pos;
-  internal_strncpy(error_message_buffer + error_message_buffer_pos,
-                   buffer, remaining);
-  error_message_buffer[kErrorMessageBufferSize - 1] = '\0';
-  // FIXME: reallocate the buffer instead of truncating the message.
-  error_message_buffer_pos += Min(remaining, length);
+  if (!error_message_buf_ptr)
+    return;
+  uptr error_message_buf_len = error_message_buf_ptr->size();
+  uptr buffer_len = internal_strlen(buffer);
+  error_message_buf_ptr->resize(error_message_buf_len + buffer_len);
+  internal_memcpy(error_message_buf_ptr->data() + error_message_buf_len, buffer,
+                  buffer_len);
 }
 
 // ---------------------- Helper functions ----------------------- {{{1
@@ -130,6 +125,11 @@ class ScopedInErrorReport {
     // We can lock them only here to avoid self-deadlock in case of
     // recursive reports.
     asanThreadRegistry().Lock();
+    {
+      Lock l(&error_message_buf_mutex);
+      error_message_buffer_.Initialize(kErrorMessageBufferSize);
+      error_message_buf_ptr = &error_message_buffer_;
+    }
     Printf(
         "=================================================================\n");
   }
@@ -156,22 +156,16 @@ class ScopedInErrorReport {
     if (common_flags()->print_module_map == 2)
       DumpProcessMap();
 
-    // Copy the message buffer so that we could start logging without holding a
-    // lock that gets acquired during printing.
-    InternalMmapVector<char> buffer_copy(kErrorMessageBufferSize);
     {
       Lock l(&error_message_buf_mutex);
-      internal_memcpy(buffer_copy.data(),
-                      error_message_buffer, kErrorMessageBufferSize);
-      // Clear error_message_buffer so that if we find other errors
-      // we don't re-log this error.
-      error_message_buffer_pos = 0;
+      error_message_buf_ptr = nullptr;
+      error_message_buffer_.push_back('\0');
     }
 
-    LogFullErrorReport(buffer_copy.data());
+    LogFullErrorReport(error_message_buffer_.data());
 
     if (error_report_callback) {
-      error_report_callback(buffer_copy.data());
+      error_report_callback(error_message_buffer_.data());
     }
 
     if (halt_on_error_ && common_flags()->abort_on_error) {
@@ -179,7 +173,7 @@ class ScopedInErrorReport {
       // FIXME: implement "compact" error format, possibly without, or with
       // highly compressed stack traces?
       // FIXME: or just use the summary line as abort message?
-      SetAbortMessage(buffer_copy.data());
+      SetAbortMessage(error_message_buffer_.data());
     }
 
     // In halt_on_error = false mode, reset the current error object (before
@@ -205,6 +199,7 @@ class ScopedInErrorReport {
 
  private:
   ScopedErrorReportLock error_report_lock_;
+  ErrorMessageBuffer error_message_buffer_;
   // Error currently being reported. This enables the destructor to interact
   // with the debugger and point it to an error description.
   static ErrorDescription current_error_;
```

https://github.com/llvm/llvm-project/pull/77488


More information about the llvm-commits mailing list