Implement LogLineLowStack() properly.

Avoid stack-based buffers used by the log formatting in
__android_log_print().

Move the VLOG(threads) in Thread::InitStackHwm() after the
low stack check as it's not safe to actually log before
that; even the StringPrintf() is using a stack-based buffer.

Bug: 18830897
Change-Id: I13b2166438e871c52ab91dabfe98f2200fd7c1cf

2 files changed, 26 insertions, 8 deletions

diff --git a/runtime/base/logging.cc b/runtime/base/logging.cc
index b781d60..7c2ef71 100644
--- a/runtime/base/logging.cc
+++ b/runtime/base/logging.cc
@@ -16,6 +16,7 @@
 
 #include "logging.h"
 
+#include <limits>
 #include <sstream>
 
 #include "base/mutex.h"
@@ -239,8 +240,25 @@ void LogMessage::LogLine(const char* file, unsigned int line, LogSeverity log_se
 void LogMessage::LogLineLowStack(const char* file, unsigned int line, LogSeverity log_severity,
                                  const char* message) {
 #ifdef HAVE_ANDROID_OS
-  // TODO: be more conservative on stack usage here.
-  LogLine(file, line, log_severity, message);
+  // Use android_writeLog() to avoid stack-based buffers used by android_printLog().
+  const char* tag = ProgramInvocationShortName();
+  int priority = kLogSeverityToAndroidLogPriority[log_severity];
+  char* buf = nullptr;
+  size_t buf_size = 0u;
+  if (priority == ANDROID_LOG_FATAL) {
+    // Allocate buffer for snprintf(buf, buf_size, "%s:%u] %s", file, line, message) below.
+    // If allocation fails, fall back to printing only the message.
+    buf_size = strlen(file) + 1 /* ':' */ + std::numeric_limits<typeof(line)>::max_digits10 +
+        2 /* "] " */ + strlen(message) + 1 /* terminating 0 */;
+    buf = reinterpret_cast<char*>(malloc(buf_size));
+  }
+  if (buf != nullptr) {
+    snprintf(buf, buf_size, "%s:%u] %s", file, line, message);
+    android_writeLog(priority, tag, buf);
+    free(buf);
+  } else {
+    android_writeLog(priority, tag, message);
+  }
 #else
   static const char* log_characters = "VDIWEFF";
   CHECK_EQ(strlen(log_characters), INTERNAL_FATAL + 1U);
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 4a7103b..5ff7490 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -515,12 +515,6 @@ bool Thread::InitStackHwm() {
   size_t read_guard_size;
   GetThreadStack(tlsPtr_.pthread_self, &read_stack_base, &read_stack_size, &read_guard_size);
 
-  // This is included in the SIGQUIT output, but it's useful here for thread debugging.
-  VLOG(threads) << StringPrintf("Native stack is at %p (%s with %s guard)",
-                                read_stack_base,
-                                PrettySize(read_stack_size).c_str(),
-                                PrettySize(read_guard_size).c_str());
-
   tlsPtr_.stack_begin = reinterpret_cast<uint8_t*>(read_stack_base);
   tlsPtr_.stack_size = read_stack_size;
 
@@ -537,6 +531,12 @@ bool Thread::InitStackHwm() {
     return false;
   }
 
+  // This is included in the SIGQUIT output, but it's useful here for thread debugging.
+  VLOG(threads) << StringPrintf("Native stack is at %p (%s with %s guard)",
+                                read_stack_base,
+                                PrettySize(read_stack_size).c_str(),
+                                PrettySize(read_guard_size).c_str());
+
   // Set stack_end_ to the bottom of the stack saving space of stack overflows
 
   Runtime* runtime = Runtime::Current();