This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 08/08] nptl: arm: Fix Race conditions in pthread cancellation (BZ#12683)
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Phil Blundell <pb at pbcl dot net>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Wed, 2 Sep 2015 18:23:52 -0300
- Subject: Re: [PATCH 08/08] nptl: arm: Fix Race conditions in pthread cancellation (BZ#12683)
- Authentication-results: sourceware.org; auth=none
- References: <55E4C300 dot 9080800 at linaro dot org> <1441148926 dot 1680 dot 51 dot camel at pbcl dot net>
Hi,
Thanks for the review.
On 01-09-2015 20:08, Phil Blundell wrote:
> On Mon, 2015-08-31 at 18:11 -0300, Adhemerval Zanella wrote:
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/arm/syscall_cancel.S
>> @@ -0,0 +1,69 @@
>> +/* Cancellable syscall wrapper - aarch64 version.
>
> I guess this should say "arm version".
Indeed, I will change that.
>
>> + .thumb
>> + .syntax unified
>
> I think this (at least the .thumb part) should be inside #ifdef
> __thumb2__. Otherwise this will break compiling on ARMv4 which I think
> otherwise still works.
Based on other ARM code I think only .thumb part, I will change that.
>
>> +
>> +ENTRY (__syscall_cancel_arch)
>> + .fnstart
>> + mov ip,sp
>> + stmfd sp!,{r4,r5,r6,r7,lr}
>> + .save {r4,r5,r6,r7,lr}
>
> Why are you stacking r7 and lr here?
Rechecking and for 'r7' it is not really required. However afaik to use
the glibc cancellation mechanism based on libgcc unwind for arm it
requires to at least save the 'lr'. I have not debug in details, but
omitting lr save I see failures with this mechanism for
tst-cancel{4,5,17,18}x.
>
> p.
>
>