diff mbox

Save final 'error' in __nptl_setxid_error()

Message ID fc3f011b-c690-7ea6-cd2e-8bf59a996724@dektech.com.au
State New, archived
Headers

Commit Message

Peter Zelezny Sept. 19, 2017, 7:55 a.m. UTC 
  Hi,

I recently got an abort() inside allocatestack.c::__nptl_setxid_error().
Unfortunately it's impossible to see the last error from the syscall (from kernel). It would be really invaluable to see this number, but it tends to be optimized out into registers, which are then clobbered by abort() itself.

I suggest the newest 'error' is placed into the cmdp structure (into memory), so that core dumps will contain this value.

Would something like this (patch attached) be reasonable?

Comments

Florian Weimer Sept. 19, 2017, 9:43 a.m. UTC | #1 
On 09/19/2017 09:55 AM, Peter Zelezny wrote:
> -      if (olderror != -1)
> +      if (olderror != -1) {
> +	/* save error to memory so it's not lost in coredumps.  */
> +	cmdp->error = error;
>   	/* Mismatch between current and previous results.  */
>   	abort ();
> +	}

Would you please change the formatting so that the { } are on lines by 
themselves (see the surrounding code for examples)?

I think it's better to store the error code in a new local volatile int 
variable, so that both old and new error code are accessible.  The old 
error code seems quite valuable, too.

Thanks,
Florian
diff mbox

Patch

--- glibc.orig/nptl/allocatestack.c	2017-09-19 12:58:11.456588117 +1000
+++ glibc/nptl/allocatestack.c	2017-09-19 12:59:43.906355264 +1000
@@ -1083,9 +1083,12 @@  __nptl_setxid_error (struct xid_command
       int olderror = cmdp->error;
       if (olderror == error)
 	break;
-      if (olderror != -1)
+      if (olderror != -1) {
+	/* save error to memory so it's not lost in coredumps.  */
+	cmdp->error = error;
 	/* Mismatch between current and previous results.  */
 	abort ();
+	}
     }
   while (atomic_compare_and_exchange_bool_acq (&cmdp->error, error, -1));
 }