Clean up abort.
* A dlmalloc usage error shouldn't call abort(3) because we want to
cause a SIGSEGV by writing the address dlmalloc didn't like to an
address the kernel won't like, so that debuggerd will dump the
memory around the address that upset dlmalloc.
* Switch to the simpler FreeBSD/NetBSD style of registering stdio
cleanup. Hopefully this will let us simplify more of the stdio
implementation.
* Clear the stdio cleanup handler before we abort because of a dlmalloc
corruption error. This fixes the reported bug, where we'd hang inside
dlmalloc because the stdio cleanup reentered dlmalloc.
Bug: 9301265
Change-Id: Ief31b389455d6876e5a68f0f5429567d37277dbc
diff --git a/libc/Android.mk b/libc/Android.mk
index c98bf9e..49d93af 100644
--- a/libc/Android.mk
+++ b/libc/Android.mk
@@ -6,7 +6,6 @@
# =========================================================
libc_common_src_files := \
$(syscall_src) \
- unistd/abort.c \
unistd/alarm.c \
unistd/exec.c \
unistd/fnmatch.c \
@@ -192,6 +191,7 @@
netbsd/nameser/ns_samedomain.c \
libc_bionic_src_files := \
+ bionic/abort.cpp \
bionic/assert.cpp \
bionic/brk.cpp \
bionic/dirent.cpp \
diff --git a/libc/bionic/abort.cpp b/libc/bionic/abort.cpp
new file mode 100644
index 0000000..6fcdfda
--- /dev/null
+++ b/libc/bionic/abort.cpp
@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 1985 Regents of the University of California.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include "atexit.h"
+
+__LIBC_HIDDEN__ void (*__cleanup)();
+
+#ifdef __arm__
+extern "C" __LIBC_HIDDEN__ void __libc_android_abort()
+#else
+void abort()
+#endif
+{
+ // Don't block SIGABRT to give any signal handler a chance; we ignore
+ // any errors -- X311J doesn't allow abort to return anyway.
+ sigset_t mask;
+ sigfillset(&mask);
+ sigdelset(&mask, SIGABRT);
+ sigprocmask(SIG_SETMASK, &mask, NULL);
+
+ // POSIX requires we flush stdio buffers on abort.
+ if (__cleanup) {
+ (*__cleanup)();
+ }
+
+ raise(SIGABRT);
+
+ // If SIGABRT ignored, or caught and the handler returns,
+ // remove the SIGABRT signal handler and raise SIGABRT again.
+ struct sigaction sa;
+ sa.sa_handler = SIG_DFL;
+ sa.sa_flags = SA_RESTART;
+ sigemptyset(&sa.sa_mask);
+ sigaction(SIGABRT, &sa, &sa);
+ sigprocmask(SIG_SETMASK, &mask, NULL);
+ raise(SIGABRT);
+ _exit(1);
+}
diff --git a/libc/bionic/dlmalloc.c b/libc/bionic/dlmalloc.c
index 51c62a7..78f2e1d 100644
--- a/libc/bionic/dlmalloc.c
+++ b/libc/bionic/dlmalloc.c
@@ -28,13 +28,17 @@
// Ugly inclusion of C file so that bionic specific #defines configure dlmalloc.
#include "../upstream-dlmalloc/malloc.c"
+extern void (*__cleanup)();
+
static void __bionic_heap_corruption_error(const char* function) {
- __libc_fatal("@@@ ABORTING: heap corruption detected by %s", function);
+ __cleanup = NULL; // The heap is corrupt. We can forget trying to shut down stdio.
+ __libc_fatal("heap corruption detected by %s", function);
}
static void __bionic_heap_usage_error(const char* function, void* address) {
- __libc_fatal("@@@ ABORTING: invalid address or address of corrupt block %p passed to %s",
+ __libc_fatal_no_abort("invalid address or address of corrupt block %p passed to %s",
address, function);
- // So that we can get a memory dump around the specific address.
+ // So that debuggerd gives us a memory dump around the specific address.
+ // TODO: improve the debuggerd protocol so we can tell it to dump an address when we abort.
*((int**) 0xdeadbaad) = (int*) address;
}
diff --git a/libc/bionic/libc_logging.cpp b/libc/bionic/libc_logging.cpp
index 74e599c..ffc5335 100644
--- a/libc/bionic/libc_logging.cpp
+++ b/libc/bionic/libc_logging.cpp
@@ -488,13 +488,10 @@
__libc_fatal("FORTIFY_SOURCE: %s. Calling abort().", msg);
}
-void __libc_fatal(const char* format, ...) {
+static void __libc_fatal(const char* format, va_list args) {
char msg[1024];
BufferOutputStream os(msg, sizeof(msg));
- va_list args;
- va_start(args, format);
out_vformat(os, format, args);
- va_end(args);
// TODO: log to stderr for the benefit of "adb shell" users.
@@ -502,7 +499,20 @@
__libc_write_log(ANDROID_LOG_FATAL, "libc", msg);
__libc_set_abort_message(msg);
+}
+void __libc_fatal_no_abort(const char* format, ...) {
+ va_list args;
+ va_start(args, format);
+ __libc_fatal(format, args);
+ va_end(args);
+}
+
+void __libc_fatal(const char* format, ...) {
+ va_list args;
+ va_start(args, format);
+ __libc_fatal(format, args);
+ va_end(args);
abort();
}
diff --git a/libc/private/libc_logging.h b/libc/private/libc_logging.h
index e62ddf2..f69e2ed 100644
--- a/libc/private/libc_logging.h
+++ b/libc/private/libc_logging.h
@@ -79,6 +79,15 @@
__printflike(1, 2);
//
+// Formats a message to the log (priority 'fatal'), but doesn't abort.
+// Used by the malloc implementation to ensure that debuggerd dumps memory
+// around the bad address.
+//
+
+__LIBC_HIDDEN__ void __libc_fatal_no_abort(const char* format, ...)
+ __printflike(1, 2);
+
+//
// Formatting routines for the C library's internal debugging.
// Unlike the usual alternatives, these don't allocate.
//
diff --git a/libc/stdio/findfp.c b/libc/stdio/findfp.c
index 76ed5ee..863235b 100644
--- a/libc/stdio/findfp.c
+++ b/libc/stdio/findfp.c
@@ -171,10 +171,9 @@
#endif
/*
- * exit() and abort() call _cleanup() through the callback registered
- * with __atexit_register_cleanup(), set whenever we open or buffer a
- * file. This chicanery is done so that programs that do not use stdio
- * need not link it all in.
+ * exit() calls _cleanup() through *__cleanup, set whenever we
+ * open or buffer a file. This chicanery is done so that programs
+ * that do not use stdio need not link it all in.
*
* The name `_cleanup' is, alas, fairly well known outside stdio.
*/
@@ -201,7 +200,7 @@
_FILEEXT_SETUP(usual+i, usualext+i);
}
/* make sure we clean up on exit */
- __atexit_register_cleanup(_cleanup); /* conservative */
+ __cleanup = _cleanup; /* conservative */
__sdidinit = 1;
out:
_THREAD_PRIVATE_MUTEX_UNLOCK(__sinit_mutex);
diff --git a/libc/stdio/local.h b/libc/stdio/local.h
index 664cec1..e3a40bc 100644
--- a/libc/stdio/local.h
+++ b/libc/stdio/local.h
@@ -58,7 +58,10 @@
int __sflags(const char *, int *);
int __vfprintf(FILE *, const char *, __va_list);
-extern void __atexit_register_cleanup(void (*)(void));
+/*
+ * Function to clean up streams, called from abort() and exit().
+ */
+extern void (*__cleanup)(void);
extern int __sdidinit;
/*
diff --git a/libc/stdio/makebuf.c b/libc/stdio/makebuf.c
index d47e27c..362cf05 100644
--- a/libc/stdio/makebuf.c
+++ b/libc/stdio/makebuf.c
@@ -65,7 +65,6 @@
fp->_bf._size = 1;
return;
}
- __atexit_register_cleanup(_cleanup);
flags |= __SMBF;
fp->_bf._base = fp->_p = p;
fp->_bf._size = size;
diff --git a/libc/stdio/setvbuf.c b/libc/stdio/setvbuf.c
index 2fb76af..eb0ec68 100644
--- a/libc/stdio/setvbuf.c
+++ b/libc/stdio/setvbuf.c
@@ -124,8 +124,7 @@
flags |= __SNPT;
/*
- * Fix up the FILE fields, and set __cleanup for output flush on
- * exit (since we are buffered in some way).
+ * Fix up the FILE fields.
*/
if (mode == _IOLBF)
flags |= __SLBF;
@@ -148,7 +147,6 @@
fp->_w = 0;
}
FUNLOCKFILE(fp);
- __atexit_register_cleanup(_cleanup);
return (ret);
}
diff --git a/libc/stdlib/atexit.c b/libc/stdlib/atexit.c
index 23a2636..8f5bd2d 100644
--- a/libc/stdlib/atexit.c
+++ b/libc/stdlib/atexit.c
@@ -44,7 +44,8 @@
* Function pointers are stored in a linked list of pages. The list
* is initially empty, and pages are allocated on demand. The first
* function pointer in the first allocated page (the last one in
- * the linked list) is reserved for the cleanup function.
+ * the linked list) was reserved for the cleanup function.
+ * TODO: switch to the regular FreeBSD/NetBSD atexit implementation.
*
* Outside the following functions, all pages are mprotect()'ed
* to prevent unintentional/malicious corruption.
@@ -172,42 +173,3 @@
}
_ATEXIT_UNLOCK();
}
-
-/*
- * Register the cleanup function
- */
-void
-__atexit_register_cleanup(void (*func)(void))
-{
- struct atexit *p;
- int pgsize = getpagesize();
-
- if (pgsize < (int)sizeof(*p))
- return;
- _ATEXIT_LOCK();
- p = __atexit;
- while (p != NULL && p->next != NULL)
- p = p->next;
- if (p == NULL) {
- p = mmap(NULL, pgsize, PROT_READ | PROT_WRITE,
- MAP_ANON | MAP_PRIVATE, -1, 0);
- if (p == MAP_FAILED)
- goto unlock;
- p->ind = 1;
- p->max = (pgsize - ((char *)&p->fns[0] - (char *)p)) /
- sizeof(p->fns[0]);
- p->next = NULL;
- __atexit = p;
- if (__atexit_invalid)
- __atexit_invalid = 0;
- } else {
- if (mprotect(p, pgsize, PROT_READ | PROT_WRITE))
- goto unlock;
- }
- p->fns[0].fn_ptr.std_func = func;
- p->fns[0].fn_arg = NULL;
- p->fns[0].fn_dso = NULL;
- mprotect(p, pgsize, PROT_READ);
-unlock:
- _ATEXIT_UNLOCK();
-}
diff --git a/libc/unistd/abort.c b/libc/unistd/abort.c
deleted file mode 100644
index 4a349bd..0000000
--- a/libc/unistd/abort.c
+++ /dev/null
@@ -1,92 +0,0 @@
-/* $OpenBSD: abort.c,v 1.14 2005/08/08 08:05:36 espie Exp $ */
-/*
- * Copyright (c) 1985 Regents of the University of California.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <signal.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include "thread_private.h"
-#include "atexit.h"
-
-#ifdef __arm__
-__LIBC_HIDDEN__ void
-__libc_android_abort(void)
-#else
-void
-abort(void)
-#endif
-{
- struct atexit *p = __atexit;
- static int cleanup_called = 0;
- sigset_t mask;
-
-
- sigfillset(&mask);
- /*
- * don't block SIGABRT to give any handler a chance; we ignore
- * any errors -- X311J doesn't allow abort to return anyway.
- */
- sigdelset(&mask, SIGABRT);
-
- (void)sigprocmask(SIG_SETMASK, &mask, (sigset_t *)NULL);
-
- /*
- * POSIX requires we flush stdio buffers on abort
- */
- if (cleanup_called == 0) {
- while (p != NULL && p->next != NULL)
- p = p->next;
- /* the check for fn_dso == NULL is mostly paranoia */
- if (p != NULL && p->fns[0].fn_dso == NULL &&
- p->fns[0].fn_ptr.std_func != NULL) {
- cleanup_called = 1;
- (*p->fns[0].fn_ptr.std_func)();
- }
- }
-
- raise(SIGABRT);
-
- /*
- * if SIGABRT ignored, or caught and the handler returns, do
- * it again, only harder.
- */
- {
- struct sigaction sa;
-
- sa.sa_handler = SIG_DFL;
- sa.sa_flags = SA_RESTART;
- sigemptyset(&sa.sa_mask);
-
- (void)sigaction( SIGABRT, &sa, &sa );
- }
-
- (void)sigprocmask(SIG_SETMASK, &mask, (sigset_t *)NULL);
- raise(SIGABRT);
- _exit(1);
-}
