From patchwork Fri Oct 13 05:16:54 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-Patchwork-Submitter: Peter Zelezny <peter.zelezny@dektech.com.au>
X-Patchwork-Id: 23525
Received: (qmail 32691 invoked by alias); 13 Oct 2017 05:17:00 -0000
Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <libc-alpha.sourceware.org>
List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org>
List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/libc-alpha/>
List-Post: <mailto:libc-alpha@sourceware.org>
List-Help: <mailto:libc-alpha-help@sourceware.org>,
	<http://sourceware.org/ml/#faqs>
Sender: libc-alpha-owner@sourceware.org
Delivered-To: mailing list libc-alpha@sourceware.org
Received: (qmail 32664 invoked by uid 89); 13 Oct 2017 05:16:59 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-11.3 required=5.0 tests=AWL, BAYES_00,
	GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD,
	SPF_PASS autolearn=ham version=3.3.2 spammy=UD:com.au, UD:au,
	H*F:D*com.au, H*F:D*au
X-HELO: mail.dektech.com.au
Subject: Re: [PATCH] Save final 'error' in __nptl_setxid_error()
From: Peter Zelezny <peter.zelezny@dektech.com.au>
To: libc-alpha@sourceware.org
References: <fc3f011b-c690-7ea6-cd2e-8bf59a996724@dektech.com.au>
	<81af73be-b7d4-46b2-0d78-470803e74bea@redhat.com>
	<27ef5a73-04ef-6a35-767e-d8ffe0935962@dektech.com.au>
Cc: Florian Weimer <fweimer@redhat.com>
Message-ID: <e56319f8-a4f3-a6d8-9ed7-a6ce492e259b@dektech.com.au>
Date: Fri, 13 Oct 2017 16:16:54 +1100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
	Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <27ef5a73-04ef-6a35-767e-d8ffe0935962@dektech.com.au>

On 20/09/17 10:46, Peter Zelezny wrote:

> On 19/09/17 19:43, Florian Weimer wrote:
>
>> 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
>
> I hope I got the indentation right now, copying nearby code (one tab 
> and two spaces).
>
> Yes, perhaps just saving this to a local stack int would be better, 
> like this?
>
> Cheers,
> -Peter.
>

Any other opinions or concerns on this patch?

Cheers,
-Peter.

--- 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
@@ -1084,8 +1084,12 @@ __nptl_setxid_error (struct xid_command
       if (olderror == error)
 	break;
       if (olderror != -1)
-	/* Mismatch between current and previous results.  */
-	abort ();
+	{
+	  /* save 'error' to memory so it's not lost in coredumps.  */
+	  volatile int xid_err __attribute__((unused)) = error;
+	  /* Mismatch between current and previous results.  */
+	  abort ();
+	}
     }
   while (atomic_compare_and_exchange_bool_acq (&cmdp->error, error, -1));
 }