[Commits] 79716e0: [MDEV-9127] Crash reporter often fails to show the query that crashed

Vicentiu Ciorbaru vicentiu at mariadb.org
Mon May 30 23:21:52 EEST 2016


revision-id: 79716e07f15e8dabca4d2647ff5dbf189935581a (mariadb-10.0.25-6-g79716e0)
parent(s): eaf5376f808b3d71c442fb6b95048c4dcd684b08
author: Vicențiu Ciorbaru
committer: Vicențiu Ciorbaru
timestamp: 2016-05-30 23:21:22 +0300
message:

[MDEV-9127] Crash reporter often fails to show the query that crashed

Addreses are not necessarily between heap_start && heap_end. Malloc
calls using mmap can place pointers outside these bounds. In this case,
we'll warn the user that the query pointer is potentially invalid.
However, we'll attempt to print the data anyway after we're done
printing everything else.

---
 include/my_stacktrace.h |  2 +-
 mysys/stacktrace.c      | 30 ++++++++++++++++++++++++++----
 sql/signal_handler.cc   | 21 ++++++++++++++++++++-
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/include/my_stacktrace.h b/include/my_stacktrace.h
index fb2525e..fad6e53 100644
--- a/include/my_stacktrace.h
+++ b/include/my_stacktrace.h
@@ -45,7 +45,7 @@ C_MODE_START
 #if defined(HAVE_STACKTRACE) || defined(HAVE_BACKTRACE)
 void my_init_stacktrace();
 void my_print_stacktrace(uchar* stack_bottom, ulong thread_stack);
-void my_safe_print_str(const char* val, int max_len);
+int my_safe_print_str(const char* val, int max_len);
 void my_write_core(int sig);
 #if BACKTRACE_DEMANGLE
 char *my_demangle(const char *mangled_name, int *status);
diff --git a/mysys/stacktrace.c b/mysys/stacktrace.c
index 613911e..746b99d 100644
--- a/mysys/stacktrace.c
+++ b/mysys/stacktrace.c
@@ -129,13 +129,32 @@ static int safe_print_str(const char *addr, int max_len)
 
 #endif
 
-void my_safe_print_str(const char* val, int max_len)
+/*
+  Attempt to print a char * pointer as a string.
+
+  SYNOPSIS
+    Prints either until the end of string ('\0'), or max_len characters have
+    been printed.
+
+  RETURN VALUE
+    0  Pointer was within the heap address space.
+       The string was printed fully, or until the end of the heap address space.
+    1  Pointer is outside the heap address space. Printed as invalid.
+
+  NOTE
+    On some systems, we can have valid pointers outside the heap address space.
+    This is through the use of mmap inside malloc calls. When this function
+    returns 1, it does not mean 100% that the pointer is corrupted.
+*/
+
+int my_safe_print_str(const char* val, int max_len)
 {
   char *heap_end;
 
 #ifdef __linux__
+  // Try and make use of /proc filesystem to safely print memory contents.
   if (!safe_print_str(val, max_len))
-    return;
+    return 0;
 #endif
 
   heap_end= (char*) sbrk(0);
@@ -143,12 +162,14 @@ void my_safe_print_str(const char* val, int max_len)
   if (!PTR_SANE(val))
   {
     my_safe_printf_stderr("%s", "is an invalid pointer");
-    return;
+    return 1;
   }
 
   for (; max_len && PTR_SANE(val) && *val; --max_len)
     my_write_stderr((val++), 1);
   my_safe_printf_stderr("%s", "\n");
+
+  return 0;
 }
 
 #if defined(HAVE_PRINTSTACK)
@@ -728,7 +749,7 @@ void my_write_core(int unused)
 }
 
 
-void my_safe_print_str(const char *val, int len)
+int my_safe_print_str(const char *val, int len)
 {
   __try
   {
@@ -738,6 +759,7 @@ void my_safe_print_str(const char *val, int len)
   {
     my_safe_printf_stderr("%s", "is an invalid string pointer");
   }
+  return 0;
 }
 #endif /*__WIN__*/
 
diff --git a/sql/signal_handler.cc b/sql/signal_handler.cc
index bb1e632..7b89b9a 100644
--- a/sql/signal_handler.cc
+++ b/sql/signal_handler.cc
@@ -65,6 +65,12 @@ extern "C" sig_handler handle_fatal_signal(int sig)
 #ifdef HAVE_STACKTRACE
   THD *thd;
 #endif
+  /*
+     This flag remembers if the query pointer was found invalid.
+     We will try and print the query at the end of the signal handler, in case
+     we're wrong.
+  */
+  bool print_invalid_query_pointer= false;
 
   if (segfaulted)
   {
@@ -190,7 +196,12 @@ extern "C" sig_handler handle_fatal_signal(int sig)
       "Some pointers may be invalid and cause the dump to abort.\n");
 
     my_safe_printf_stderr("Query (%p): ", thd->query());
-    my_safe_print_str(thd->query(), MY_MIN(65536U, thd->query_length()));
+    if (my_safe_print_str(thd->query(), MY_MIN(65536U, thd->query_length())))
+    {
+      // Query was found invalid. We will try to print it at the end.
+      print_invalid_query_pointer= true;
+    }
+
     my_safe_printf_stderr("\nConnection ID (thread ID): %lu\n",
                           (ulong) thd->thread_id);
     my_safe_printf_stderr("Status: %s\n\n", kreason);
@@ -254,6 +265,14 @@ extern "C" sig_handler handle_fatal_signal(int sig)
       "\"mlockall\" bugs.\n");
   }
 
+  if (print_invalid_query_pointer)
+  {
+    my_safe_printf_stderr("\nTrying to print the invalid query pointer. \n"
+                          "Query: ");
+    my_write_stderr(thd->query(), MY_MIN(65536U, thd->query_length()));
+    my_safe_printf_stderr("\n\n");
+  }
+
 #ifdef HAVE_WRITE_CORE
   if (test_flags & TEST_CORE_ON_SIGNAL)
   {


More information about the commits mailing list