Reland "Add start & end markers around |str_stack| for use by Crash service."

Crash can locate particular entries in a stack by scanning for start &
end markers with unusual values.  We add start & end markers around the
|str_stack| copy of the LogMessage string, created when we're about to
crash the process due to a FATAL level log message.

This was originally landed with review:
https://chromium-review.googlesource.com/c/1201344
which was reverted because MSan bots (correctly) failed due to the test's
scanning of uninitialized stack memory. Since the test can't avoid doing
that, we simply disable it under both ASan and MSan.

Bug: 802393
Change-Id: Icd87f258aeb9556ab8e68d3e3431e80c01f1d3f0
Reviewed-on: https://chromium-review.googlesource.com/c/1292597Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601399}

base/logging.cc

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "base/stl_util.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
@@ -831,8 +832,17 @@ LogMessage::~LogMessage() {
       tracker->RecordLogMessage(str_newline);
 
     // Ensure the first characters of the string are on the stack so they
-    // are contained in minidumps for diagnostic purposes.
-    DEBUG_ALIAS_FOR_CSTR(str_stack, str_newline.c_str(), 1024);
+    // are contained in minidumps for diagnostic purposes. We place start
+    // and end marker values at either end, so we can scan captured stacks
+    // for the data easily.
+    struct {
+      uint32_t start_marker = 0xbedead01;
+      char data[1024];
+      uint32_t end_marker = 0x5050dead;
+    } str_stack;
+    base::strlcpy(str_stack.data, str_newline.data(),
+                  base::size(str_stack.data));
+    base::debug::Alias(&str_stack);
 
     if (log_assert_handler_stack.IsCreated() &&
         !log_assert_handler_stack.Get().empty()) {

base/logging_unittest.cc

@@ -829,6 +829,56 @@ TEST_F(LoggingTest, LogPrefix) {
   log_string_ptr = nullptr;
 }
 
+#if !defined(ADDRESS_SANITIZER) && !defined(MEMORY_SANITIZER)
+// Since we scan potentially uninitialized portions of the stack, we can't run
+// this test under any sanitizer that checks for uninitialized reads.
+TEST_F(LoggingTest, LogMessageMarkersOnStack) {
+  const uint32_t kLogStartMarker = 0xbedead01;
+  const uint32_t kLogEndMarker = 0x5050dead;
+  const char kTestMessage[] = "Oh noes! I have crashed! 💩";
+
+  uint32_t stack_start = 0;
+
+  // Install a LogAssertHandler which will scan between |stack_start| and its
+  // local-scope stack for the start & end markers, and verify the message.
+  ScopedLogAssertHandler assert_handler(base::BindRepeating(
+      [](uint32_t* stack_start_ptr, const char* file, int line,
+         const base::StringPiece message, const base::StringPiece stack_trace) {
+        uint32_t stack_end;
+        uint32_t* stack_end_ptr = &stack_end;
+
+        // Scan the stack for the expected markers.
+        uint32_t* start_marker = nullptr;
+        uint32_t* end_marker = nullptr;
+        for (uint32_t* ptr = stack_end_ptr; ptr <= stack_start_ptr; ++ptr) {
+          if (*ptr == kLogStartMarker)
+            start_marker = ptr;
+          else if (*ptr == kLogEndMarker)
+            end_marker = ptr;
+        }
+
+        // Verify that start & end markers were found, somewhere, in-between
+        // this and the LogAssertHandler scope, in the LogMessage destructor's
+        // stack frame.
+        ASSERT_TRUE(start_marker);
+        ASSERT_TRUE(end_marker);
+
+        // Verify that the |message| is found in-between the markers.
+        const char* start_char_marker =
+            reinterpret_cast<char*>(start_marker + 1);
+        const char* end_char_marker = reinterpret_cast<char*>(end_marker);
+
+        const base::StringPiece stack_view(start_char_marker,
+                                           end_char_marker - start_char_marker);
+        ASSERT_FALSE(stack_view.find(message) == base::StringPiece::npos);
+      },
+      &stack_start));
+
+  // Trigger a log assertion, with a test message we can check for.
+  LOG(FATAL) << kTestMessage;
+}
+#endif  // !defined(ADDRESS_SANITIZER)
+
 }  // namespace
 
 }  // namespace logging