am 3da47942: am 944ea1a3: Merge "Clean up abort."

* commit '3da47942d3fdeb6f39cf84b03c75cfeb69f91449':
  Clean up abort.

libc/Android.mk

@@ -6,7 +6,6 @@ include $(LOCAL_PATH)/arch-$(TARGET_ARCH)/syscalls.mk
 # =========================================================
 libc_common_src_files := \
 	$(syscall_src) \
-	unistd/abort.c \
 	unistd/alarm.c \
 	unistd/exec.c \
 	unistd/fnmatch.c \
@@ -192,6 +191,7 @@ libc_common_src_files := \
 	netbsd/nameser/ns_samedomain.c \
 
 libc_bionic_src_files := \
+    bionic/abort.cpp \
     bionic/assert.cpp \
     bionic/brk.cpp \
     bionic/dirent.cpp \

libc/bionic/abort.cpp

@@ -1,4 +1,3 @@
-/*	$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.
@@ -31,62 +30,38 @@
 #include <signal.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include "thread_private.h"
 #include "atexit.h"
 
+__LIBC_HIDDEN__ void (*__cleanup)();
+
 #ifdef __arm__
-__LIBC_HIDDEN__ void
+extern "C" __LIBC_HIDDEN__ void __libc_android_abort()
-__libc_android_abort(void)
 #else
-void
+void abort()
-abort(void)
 #endif
 {
-	struct atexit *p = __atexit;
+  // Don't block SIGABRT to give any signal handler a chance; we ignore
-	static int cleanup_called = 0;
+  // any errors -- X311J doesn't allow abort to return anyway.
-	sigset_t mask;
+  sigset_t mask;
+  sigfillset(&mask);
+  sigdelset(&mask, SIGABRT);
+  sigprocmask(SIG_SETMASK, &mask, NULL);
 
+  // POSIX requires we flush stdio buffers on abort.
+  if (__cleanup) {
+    (*__cleanup)();
+  }
 
-	sigfillset(&mask);
+  raise(SIGABRT);
-	/*
-	 * 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);
+  // If SIGABRT ignored, or caught and the handler returns,
-
+  // remove the SIGABRT signal handler and raise SIGABRT again.
-	/*
+  struct sigaction sa;
-	 * POSIX requires we flush stdio buffers on abort
+  sa.sa_handler = SIG_DFL;
-	 */
+  sa.sa_flags   = SA_RESTART;
-	if (cleanup_called == 0) {
+  sigemptyset(&sa.sa_mask);
-		while (p != NULL && p->next != NULL)
+  sigaction(SIGABRT, &sa, &sa);
-			p = p->next;
+  sigprocmask(SIG_SETMASK, &mask, NULL);
-		/* the check for fn_dso == NULL is mostly paranoia */
+  raise(SIGABRT);
-		if (p != NULL && p->fns[0].fn_dso == NULL &&
+  _exit(1);
-		    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);
 }

libc/bionic/dlmalloc.c

@@ -28,13 +28,17 @@ static void __bionic_heap_usage_error(const char* function, void* address);
 // 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;
 }

libc/bionic/libc_logging.cpp

@@ -488,13 +488,10 @@ void __fortify_chk_fail(const char *msg, uint32_t tag) {
   __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 @@ void __libc_fatal(const char* format, ...) {
   __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();
 }
 

libc/private/libc_logging.h

@@ -78,6 +78,15 @@ __LIBC_HIDDEN__ void __libc_set_abort_message(const char* msg);
 __LIBC_HIDDEN__ __noreturn void __libc_fatal(const char* format, ...)
     __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.

libc/stdio/findfp.c

@@ -171,10 +171,9 @@ f_prealloc(void)
 #endif
 
 /*
- * exit() and abort() call _cleanup() through the callback registered
+ * exit() calls _cleanup() through *__cleanup, set whenever we
- * with __atexit_register_cleanup(), set whenever we open or buffer a
+ * open or buffer a file.  This chicanery is done so that programs
- * file. This chicanery is done so that programs that do not use stdio
+ * that do not use stdio need not link it all in.
- * need not link it all in.
  *
  * The name `_cleanup' is, alas, fairly well known outside stdio.
  */
@@ -201,7 +200,7 @@ __sinit(void)
 		_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);

libc/stdio/local.h

@@ -58,7 +58,10 @@ int	__swsetup(FILE *);
 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;
 
 /*

libc/stdio/makebuf.c

@@ -65,7 +65,6 @@ __smakebuf(FILE *fp)
 		fp->_bf._size = 1;
 		return;
 	}
-	__atexit_register_cleanup(_cleanup);
 	flags |= __SMBF;
 	fp->_bf._base = fp->_p = p;
 	fp->_bf._size = size;

libc/stdio/setvbuf.c

@@ -124,8 +124,7 @@ nbf:
 		flags |= __SNPT;
 
 	/*
-	 * Fix up the FILE fields, and set __cleanup for output flush on
+	 * Fix up the FILE fields.
-	 * exit (since we are buffered in some way).
 	 */
 	if (mode == _IOLBF)
 		flags |= __SLBF;
@@ -148,7 +147,6 @@ nbf:
 		fp->_w = 0;
 	}
 	FUNLOCKFILE(fp);
-	__atexit_register_cleanup(_cleanup);
 
 	return (ret);
 }

libc/stdlib/atexit.c

@@ -44,7 +44,8 @@ struct atexit *__atexit;
  * 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 @@ __cxa_finalize(void *dso)
 	}
 	_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();
-}