am 04cdfa67: Merge "Clean up the x86 and x86_64 _exit_with_stack_teardown implementations."

* commit '04cdfa67c7e4755ddbcd1d907f00d3cbdf6cd1f2':
  Clean up the x86 and x86_64 _exit_with_stack_teardown implementations.

libc/arch-x86/bionic/_exit_with_stack_teardown.S

@@ -1,28 +1,22 @@
 #include <asm/unistd.h>
 #include <machine/asm.h>
 
-// void _exit_with_stack_teardown(void *stackBase, int stackSize, int *retCode)
+// void _exit_with_stack_teardown(void* stackBase, int stackSize, int retCode)
 ENTRY(_exit_with_stack_teardown)
-    /* we can trash %ebx here since this call should never return. */
-    /* We can also take advantage of the fact that the linux syscall trap
-     * handler saves all the registers, so we don't need a stack to keep
-     * the retCode argument for exit while doing the munmap */
-
-    /* TODO(dmtriyz): No one expects this code to return, so even if
-     * munmap fails, we have to exit. This should probably be fixed, but
-     * since ARM side does the same thing, leave it as is.
-     */
-    mov     4(%esp), %ebx             /* stackBase */
-    mov     8(%esp), %ecx             /* stackSize */
-    mov     12(%esp), %edx           /* retCode, not used for munmap */
+    // We can trash %ebx here since this call should never return.
+    // We can also take advantage of the fact that the linux syscall trap
+    // handler saves all the registers, so we don't need a stack to keep
+    // the retCode argument for exit while doing the munmap */
+    mov     4(%esp), %ebx             // stackBase
+    mov     8(%esp), %ecx             // stackSize
     mov     $__NR_munmap, %eax
     int     $0x80
-    mov     %edx, %ebx                /* retrieve the retCode */
+
+    // If munmap failed, we ignore the failure and exit anyway.
+
+    mov     %edx, %ebx                // retCode
     movl    $__NR_exit, %eax
     int     $0x80
-    /* exit does not return */
-    /* can't have a ret here since we no longer have a usable stack. Seems
-     * that presently, 'hlt' will cause the program to segfault.. but this
-     * should never happen :) */
-    hlt
+
+    // The exit syscall does not return.
 END(_exit_with_stack_teardown)

libc/arch-x86_64/bionic/_exit_with_stack_teardown.S

@@ -28,29 +28,20 @@
 
 #include <asm/unistd.h>
 #include <machine/asm.h>
-/*
- * void _exit_with_stack_teardown(void *stackBase, int stackSize, int *retCode)
- */
 
+// void _exit_with_stack_teardown(void* stackBase, int stackSize, int retCode)
 ENTRY(_exit_with_stack_teardown)
-    /* we can trash %rbx here since this call should never return. */
-    /* We can also take advantage of the fact that the linux syscall trap
-     * handler saves all the registers, so we don't need a stack to keep
-     * the retCode argument for exit while doing the munmap */
-
-    /* TODO(dmtriyz): No one expects this code to return, so even if
-     * munmap fails, we have to exit. This should probably be fixed, but
-     * since ARM side does the same thing, leave it as is.
-     */
-
-    /* args passed through registers */
-    mov     $__NR_munmap, %eax        /* shouldn't change %rdx (retCode) */
+    // We take advantage of the fact that the linux syscall trap
+    // handler saves all the registers, so we don't need to save
+    // the retCode argument for exit(2) while doing the munmap(2).
+    mov     $__NR_munmap, %eax
     syscall
-    mov     %rdx, %rdi                /* retrieve the retCode */
+
+    // If munmap failed, ignore the failure and exit anyway.
+
+    mov     %rdx, %rdi        // retCode
     mov     $__NR_exit, %eax
     syscall
-    /* exit does not return */
-    /* can't have a ret here since we no longer have a usable stack. Seems
-     * that presently, 'hlt' will cause the program to segfault.. but this
-     * should never happen :) */
-    hlt
+
+    // The exit syscall does not return.
+END(_exit_with_stack_teardown)

libc/bionic/pthread.c

@@ -146,10 +146,13 @@ void pthread_exit(void * retval)
     (void)sigprocmask(SIG_SETMASK, &mask, (sigset_t *)NULL);
 
     // destroy the thread stack
-    if (user_stack)
+    if (user_stack) {
         _exit_thread((int)retval);
-    else
+    } else {
+        // We need to munmap the stack we're running on before calling exit.
+        // That's not something we can do in C.
         _exit_with_stack_teardown(stack_base, stack_size, (int)retval);
+    }
 }
 
 /* a mutex is implemented as a 32-bit integer holding the following fields