From patchwork Wed Aug 16 14:13:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 22146 Received: (qmail 49805 invoked by alias); 16 Aug 2017 14:13:39 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 42340 invoked by uid 89); 16 Aug 2017 14:13:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=sk:INTERNA X-HELO: mail-qk0-f179.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=iGpRiRj213php8LpCNf/9bdQkEp4iG7tFmR4DY8SjAA=; b=aRXrGGZ05Zv4y/vCsCgTWMObDBpxRXem+R+0+0piyfcZDh8hdD5SJ0z2lveO+ZW16O fJiY5DX6MWireffHZmGu/mrXAEjgqscO+WOBs5CpCdJoT94T1QWVIhDFB7d0fSOSM0Xn kUsF1iNhF75Ia9eA80J6zo3KyIC97c45sovKD3ZO/AYCavLdpxTYbiFLe3+DMOhIaoJY n/6Czu19N1zHmvhe/sp4u8/ugaZSJVWEw7MNEv7qxzXM1Ie7e59GJ3On87XQoy3I+tJy d5IHAySpv4JoISIAlFm0JGVQMZkR1q2FbjMSY1RAM85NbuzxeDMBg+ewb9U6mrjNUvSS qk3w== X-Gm-Message-State: AHYfb5iKyhHwRlOd0MgrG69ObhXOg2nLOr844bQoUQFd8LGNg5nEjJga NEVVMfht8Iwrh08/CQwmFg== X-Received: by 10.55.79.77 with SMTP id d74mr2404367qkb.183.1502892808683; Wed, 16 Aug 2017 07:13:28 -0700 (PDT) Subject: Re: [PATCH] mips/o32: fix internal_syscall5/6/7 To: Joseph Myers , "Maciej W. Rozycki" Cc: Aurelien Jarno , libc-alpha@sourceware.org References: <20170815115055.29375-1-aurelien@aurel32.net> <601ef1a8-f7f5-ce53-722c-b6dbaad2831d@linaro.org> <20170815193344.sszs3vmc7ahkspvx@aurel32.net> <20170815200812.6kmv554yfga2x4al@aurel32.net> From: Adhemerval Zanella Message-ID: <35e73925-532c-bc95-53ca-005f3dbd130b@linaro.org> Date: Wed, 16 Aug 2017 11:13:25 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: On 16/08/2017 10:44, Joseph Myers wrote: > On Wed, 16 Aug 2017, Maciej W. Rozycki wrote: > >> On Tue, 15 Aug 2017, Joseph Myers wrote: >> >>> In which case having a volatile integer variable with value 4, declaring a >>> VLA whose size is that variable, and storing a pointer to that VLA in a >>> variable, would be an alternative to alloca to force a frame pointer, but >>> with deallocation happening when the scope ends rather than the function >>> ending (and the syscall macro has its own scope, so using it inside a loop >>> wouldn't be a problem). >> >> I suspect using volatile variables will cause unnecessary memory traffic. >> Passing the size specifier through an empty `asm' might give better code; >> also I think we can use 0 as the size requested, not to decrease the stack >> pointer unnecessarily, e.g.: > > Sure, as long as (a) the compiler can't know the size is actually constant > and (b) it can't know the VLA isn't actually used, as if it can tell > either of those things it can optimize away the variable stack allocation. > >> Also I wonder if there's actually a dependable way to have GCC itself >> allocate the argument space we require. For example if we set `s' to 1 >> above instead for `internal_syscall6', then would `0($sp)' and `4($sp)' be >> valid to place arguments #5 and #6 at respectively without the subsequent >> $sp adjustment we currently have in the syscall `asm' or would it be UB? > > You can't tell whether the compiler might have allocated other variables > on the stack after the dynamic adjustment - that is, whether any > particular offset from sp is in fact unused or not. > What about the below? I can use some help to see if I am handling all the required ABI requirements for the __libc_do_syscall, but on an qemu emulated system I see no regression on basic tests (including some cancellation one from glibc to see the syscall is correctly unwinded) and tst-rwlock15 also does not fail anymore. diff --git a/sysdeps/unix/sysv/linux/mips/mips32/Makefile b/sysdeps/unix/sysv/linux/mips/mips32/Makefile index 33b4615..cbdf032 100644 --- a/sysdeps/unix/sysv/linux/mips/mips32/Makefile +++ b/sysdeps/unix/sysv/linux/mips/mips32/Makefile @@ -1,8 +1,26 @@ +ifeq ($(subdir),elf) +sysdep-dl-routines += libc-do-syscall +endif + ifeq ($(subdir),conform) # For bugs 17786 and 21278. conformtest-xfail-conds += mips-o32-linux endif +ifeq ($(subdir),io) +sysdep_routines += libc-do-syscall +endif + +ifeq ($(subdir),nptl) +libpthread-sysdep_routines += libc-do-syscall +libpthread-shared-only-routines += libc-do-syscall +endif + +ifeq ($(subdir),rt) +librt-sysdep_routines += libc-do-syscall +librt-shared-only-routines += libc-do-syscall +endif + ifeq ($(subdir),stdlib) tests += bug-getcontext-mips-gp endif diff --git a/sysdeps/unix/sysv/linux/mips/mips32/libc-do-syscall.S b/sysdeps/unix/sysv/linux/mips/mips32/libc-do-syscall.S new file mode 100644 index 0000000..a7184d9 --- /dev/null +++ b/sysdeps/unix/sysv/linux/mips/mips32/libc-do-syscall.S @@ -0,0 +1,54 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library. If not, see + . */ + +#include +#include +#include +#include + + +/* long int __libc_do_syscall (long int, ...) */ + +#define FRAMESZ 32 + + .text + .set nomips16 + .hidden __libc_do_syscall +ENTRY(__libc_do_syscall) + move $2, $4 + move $4, $5 + move $5, $6 + move $6, $7 + lw $7, 16(sp) + lw $8, 20(sp) + lw $9, 24(sp) + lw $10,28(sp) + .set noreorder + PTR_SUBU sp, FRAMESZ + cfi_adjust_cfa_offset (FRAMESZ) + sw $8, 16(sp) + sw $9, 20(sp) + sw $10,24(sp) + syscall + PTR_ADDU sp, FRAMESZ + cfi_adjust_cfa_offset (-FRAMESZ) + .set reorder + beq $7, $0, 1f + subu $2, $0, $2 +1: jr ra + nop +END (__libc_do_syscall) diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h index e9e3ee7..3a8920a 100644 --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h @@ -121,13 +121,13 @@ # define INTERNAL_SYSCALL_MIPS16(number, err, nr, args...) \ internal_syscall##nr ("lw\t%0, %2\n\t", \ "R" (number), \ - 0, err, args) + SYS_ify(name), err, args) #else /* !__mips16 */ # define INTERNAL_SYSCALL(name, err, nr, args...) \ internal_syscall##nr ("li\t%0, %2\t\t\t# " #name "\n\t", \ "IK" (SYS_ify (name)), \ - 0, err, args) + SYS_ify(name), err, args) # define INTERNAL_SYSCALL_NCS(number, err, nr, args...) \ internal_syscall##nr (MOVE32 "\t%0, %2\n\t", \ @@ -136,6 +136,7 @@ #endif /* !__mips16 */ + #define internal_syscall0(v0_init, input, number, err, dummy...) \ ({ \ long _sys_result; \ @@ -262,109 +263,41 @@ _sys_result; \ }) -/* We need to use a frame pointer for the functions in which we - adjust $sp around the syscall, or debug information and unwind - information will be $sp relative and thus wrong during the syscall. As - of GCC 4.7, this is sufficient. */ -#define FORCE_FRAME_POINTER \ - void *volatile __fp_force __attribute__ ((unused)) = alloca (4) +long int __libc_do_syscall (long int, ...) attribute_hidden; #define internal_syscall5(v0_init, input, number, err, \ arg1, arg2, arg3, arg4, arg5) \ ({ \ - long _sys_result; \ - \ - FORCE_FRAME_POINTER; \ - { \ - register long __s0 asm ("$16") __attribute__ ((unused)) \ - = (number); \ - register long __v0 asm ("$2"); \ - register long __a0 asm ("$4") = (long) (arg1); \ - register long __a1 asm ("$5") = (long) (arg2); \ - register long __a2 asm ("$6") = (long) (arg3); \ - register long __a3 asm ("$7") = (long) (arg4); \ - __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ - "subu\t$29, 32\n\t" \ - "sw\t%6, 16($29)\n\t" \ - v0_init \ - "syscall\n\t" \ - "addiu\t$29, 32\n\t" \ - ".set\treorder" \ - : "=r" (__v0), "+r" (__a3) \ - : input, "r" (__a0), "r" (__a1), "r" (__a2), \ - "r" ((long) (arg5)) \ - : __SYSCALL_CLOBBERS); \ - err = __a3; \ - _sys_result = __v0; \ - } \ + long int _sys_result; \ + _sys_result = __libc_do_syscall (number, arg1, arg2, arg3, \ + arg4, arg5); \ + err = _sys_result > -4096UL ? 1 : 0; \ + if (err) \ + _sys_result = -_sys_result; \ _sys_result; \ }) #define internal_syscall6(v0_init, input, number, err, \ arg1, arg2, arg3, arg4, arg5, arg6) \ ({ \ - long _sys_result; \ - \ - FORCE_FRAME_POINTER; \ - { \ - register long __s0 asm ("$16") __attribute__ ((unused)) \ - = (number); \ - register long __v0 asm ("$2"); \ - register long __a0 asm ("$4") = (long) (arg1); \ - register long __a1 asm ("$5") = (long) (arg2); \ - register long __a2 asm ("$6") = (long) (arg3); \ - register long __a3 asm ("$7") = (long) (arg4); \ - __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ - "subu\t$29, 32\n\t" \ - "sw\t%6, 16($29)\n\t" \ - "sw\t%7, 20($29)\n\t" \ - v0_init \ - "syscall\n\t" \ - "addiu\t$29, 32\n\t" \ - ".set\treorder" \ - : "=r" (__v0), "+r" (__a3) \ - : input, "r" (__a0), "r" (__a1), "r" (__a2), \ - "r" ((long) (arg5)), "r" ((long) (arg6)) \ - : __SYSCALL_CLOBBERS); \ - err = __a3; \ - _sys_result = __v0; \ - } \ + long int _sys_result; \ + _sys_result = __libc_do_syscall (number, arg1, arg2, arg3, \ + arg4, arg5, arg6); \ + err = _sys_result > -4096UL ? 1 : 0; \ + if (err) \ + _sys_result = -_sys_result; \ _sys_result; \ }) #define internal_syscall7(v0_init, input, number, err, \ arg1, arg2, arg3, arg4, arg5, arg6, arg7) \ ({ \ - long _sys_result; \ - \ - FORCE_FRAME_POINTER; \ - { \ - register long __s0 asm ("$16") __attribute__ ((unused)) \ - = (number); \ - register long __v0 asm ("$2"); \ - register long __a0 asm ("$4") = (long) (arg1); \ - register long __a1 asm ("$5") = (long) (arg2); \ - register long __a2 asm ("$6") = (long) (arg3); \ - register long __a3 asm ("$7") = (long) (arg4); \ - __asm__ volatile ( \ - ".set\tnoreorder\n\t" \ - "subu\t$29, 32\n\t" \ - "sw\t%6, 16($29)\n\t" \ - "sw\t%7, 20($29)\n\t" \ - "sw\t%8, 24($29)\n\t" \ - v0_init \ - "syscall\n\t" \ - "addiu\t$29, 32\n\t" \ - ".set\treorder" \ - : "=r" (__v0), "+r" (__a3) \ - : input, "r" (__a0), "r" (__a1), "r" (__a2), \ - "r" ((long) (arg5)), "r" ((long) (arg6)), "r" ((long) (arg7)) \ - : __SYSCALL_CLOBBERS); \ - err = __a3; \ - _sys_result = __v0; \ - } \ + long int _sys_result; \ + _sys_result = __libc_do_syscall (number, arg1, arg2, arg3, \ + arg4, arg5, arg6, arg7); \ + err = _sys_result > -4096UL ? 1 : 0; \ + if (err) \ + _sys_result = -_sys_result; \ _sys_result; \ })