Skip to content

Commit

Permalink
[MDEV-9127] Crash reporter often fails to show the query that crashed
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cvicentiu committed Jul 12, 2016
1 parent 406fe77 commit 7d4a7d8
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 6 deletions.
2 changes: 1 addition & 1 deletion include/my_stacktrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 26 additions & 4 deletions mysys/stacktrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,26 +129,47 @@ 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);

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)
Expand Down Expand Up @@ -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
{
Expand All @@ -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__*/

Expand Down
23 changes: 22 additions & 1 deletion sql/signal_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -254,6 +265,16 @@ extern "C" sig_handler handle_fatal_signal(int sig)
"\"mlockall\" bugs.\n");
}

if (print_invalid_query_pointer)
{
my_safe_printf_stderr(
"\nWe think the query pointer is invalid, but we will try "
"to print it anyway. \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)
{
Expand Down

3 comments on commit 7d4a7d8

@brad0
Copy link
Contributor

@brad0 brad0 commented on 7d4a7d8 Aug 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks building on OpenBSD...

/home/ports/pobj/mariadb-10.0.27/mariadb-10.0.27/sql/signal_handler.cc: In function 'void handle_fatal_signal(int)':
/home/ports/pobj/mariadb-10.0.27/mariadb-10.0.27/sql/signal_handler.cc:274: error: 'thd' was not declared in this scope

It looks like some or all of the code in the chunk starting at line 268 and ending at 277 should be wrapped in #ifdef HAVE_STACKTRACE.

@vuvova
Copy link
Member

@vuvova vuvova commented on 7d4a7d8 Aug 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, f81f985

@brad0
Copy link
Contributor

@brad0 brad0 commented on 7d4a7d8 Aug 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That commit does fix the build. Thank you.

Please sign in to comment.