This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] aarch64: fix errno address calculation in SYSCALL_ERROR_HANDLER()
- From: Yury Norov <ynorov at caviumnetworks dot com>
- To: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- Cc: <libc-alpha at sourceware dot org>
- Date: Tue, 7 Feb 2017 20:24:15 +0530
- Subject: Re: [PATCH] aarch64: fix errno address calculation in SYSCALL_ERROR_HANDLER()
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Yuri dot Norov at caviumnetworks dot com;
- References: <1486468092-20986-1-git-send-email-ynorov@caviumnetworks.com> <3a715aee-1afe-94af-d6ab-77394d5e45e0@linaro.org>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On Tue, Feb 07, 2017 at 10:10:42AM -0200, Adhemerval Zanella wrote:
> If it is an user visible issue it requires a bugzilla report (although I am not
> sure in this situation since I think this issue should arise only for ilp32).
This is not a user-visible bug because arm64/ilp32 is not a user
visible feature at now. If you think that I should create a bug, I can do it.
> Also, errno for mmap in particular should be covered on 'posix/tst-mmap.c'.
> When/how exactly this issues is triggered? It could be a good thing to have
> at least a regression check or increase coverage in an existing test.
This is not mmap bug. This is SYSCALL_ERROR_HANDLER() bug that triggered in
the mmap16 test. The macro is used in generation of wrappers for pseudo
syscalls. Bug is triggered for syscall sys_write(). Kernel returns ENOSPC
for it, as intended, and triggers the invocation of SYSCALL_ERROR_HANDLER().
113 while (1) {
114 ret = write(parentfd, buf, FS_BLOCKSIZE);
115 if (ret < 0) {
116 if (errno == ENOSPC) {
117 break;
118 } else {
119 tst_brkm(TBROK | TERRNO, cleanup,
120 "write failed unexpectedly");
121 }
122 }
123 }
The macro stores error code at the errno address:
# define SYSCALL_ERROR_HANDLER \
.Lsyscall_error: \
adrp x1, :gottprel:errno; \
neg w2, w0; \
ldr x1, [x1, :gottprel_lo12:errno]; \
mrs x3, tpidr_el0; \
mov x0, -1; \
str w2, [x1, x3]; \
RET;
Assembler translates it to next commands in test mmap16 (with my comments):
│0xf77a9ba4 adrp x1, 0xf77bf000
│0xf77a9ba8 neg w2, w0 // 28
│0xf77a9bac ldr x1, [x1,#8048] // $x1==0
│0xf77a9bb0 mrs x3, tpidr_el0 // 0xf77ec350
│0xf77a9bb4 mov x0, #0xffffffffffffffff // #-1
│0xf77a9bb8 str w2, [x1,x3] // [0xf77ec350] == 28
│0xf77a9bbc ret
The code that retrieves errno is written in C, and can be found in
errno_location(). For ilp32 it looks like this:
│0xf77aca10 adrp x0, 0xf77bf000
│0xf77aca14 ldr w0, [x0,#4024] // $w0==0x10
│0xf77aca18 mrs x7, tpidr_el0 // $x7==0xf77ec350
│0xf77aca1c add w0, w0, w7 // $w0==0xf77ec360
│0xf77aca20 ret
The problem is in the 'ldr x1, [x1,#8048]' instruction. It
should be 'ldr w1, [x1,#4024]' for ilp32. The address $x1+8048
becomes valid occasionally, and contains '0'. Error code is therefore
stored at 0xf77ec350 while the proper location for it is 0xf77ec360.
After it errno still contains the code of previous error which is
ENOENT, and test fails.
There's the patch 389d1f1b (Partial ILP32 support for aarch64) in
glibc tree that contains the bunch of fixes of this sort for arm64,
but this one is missed.
If you think there should be the specific test for this case, I can
write it. But I think it should be the test for pseudo syscalls
wrapper, not the part of mmap() or write() test.
Yury
> On 07/02/2017 09:48, Yury Norov wrote:
> > This patch fixes the last regression in LTP lite scenario (mmap16) comparing
> > to lp64 in my source trees [1, 2]. The fix has been suggested back in 2015 [3]
> > but was never applied, so I reinvented the weel while debugging mmap16.
> >
> > [1] https://github.com/norov/glibc/tree/dev9
> > [2] https://github.com/norov/linux/tree/ilp32-20170203
> > [3] https://sourceware.org/ml/libc-alpha/2015-03/msg00587.html
> >
> > * sysdeps/unix/sysv/linux/aarch64/sysdep.h: use PTR_REG() for offset
> > calculation in SYSCALL_ERROR_HANDLER().
> >
> > ---
> > sysdeps/unix/sysv/linux/aarch64/sysdep.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> > index 1ffabc2..d926e19 100644
> > --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> > @@ -108,7 +108,7 @@
> > .Lsyscall_error: \
> > adrp x1, :gottprel:errno; \
> > neg w2, w0; \
> > - ldr x1, [x1, :gottprel_lo12:errno]; \
> > + ldr PTR_REG (1), [x1, :gottprel_lo12:errno]; \
> > mrs x3, tpidr_el0; \
> > mov x0, -1; \
> > str w2, [x1, x3]; \
> >