From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 9FDA03857819 for ; Tue, 8 Dec 2020 12:27:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9FDA03857819 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5154A1FB; Tue, 8 Dec 2020 04:27:27 -0800 (PST) Received: from [192.168.1.19] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9E0193F68F; Tue, 8 Dec 2020 04:27:26 -0800 (PST) Subject: Re: [PATCH] aarch64: Add ILP32 ABI support in assembly To: Kinsey Moore , newlib@sourceware.org References: <20201201183107.17039-1-kinsey.moore@oarcorp.com> From: Richard Earnshaw Message-ID: Date: Tue, 8 Dec 2020 12:27:25 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201201183107.17039-1-kinsey.moore@oarcorp.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3498.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Dec 2020 12:27:29 -0000 This is heading in the right direction, but I have two issues with it at present. 1) You need to also handle size_t paramenters (length parameters for routines like memcpy) - you will need something similar, but this isn't a pointer, so probably needs a similar macro to be defined. 2) I'd really prefer that this change were made first to the Arm Optimized Routines repository; we can then sync the fixed/updated versions from there into newlib and avoid fragmentation or potential merge issues in future by doing that. R. On 01/12/2020 18:31, Kinsey Moore wrote: > This adds sanitization of the padding bits of pointer arguments when > compiled using the ILP32 ABI as per aapcs64. > --- > newlib/libc/machine/aarch64/machine/asm.h | 39 +++++++++++++++++++++++ > newlib/libc/machine/aarch64/memchr.S | 4 +++ > newlib/libc/machine/aarch64/memcmp.S | 4 +++ > newlib/libc/machine/aarch64/memcpy.S | 4 +++ > newlib/libc/machine/aarch64/memmove.S | 4 +++ > newlib/libc/machine/aarch64/memset.S | 3 ++ > newlib/libc/machine/aarch64/setjmp.S | 4 +++ > newlib/libc/machine/aarch64/strchr.S | 3 ++ > newlib/libc/machine/aarch64/strchrnul.S | 3 ++ > newlib/libc/machine/aarch64/strcmp.S | 4 +++ > newlib/libc/machine/aarch64/strcpy.S | 4 +++ > newlib/libc/machine/aarch64/strlen.S | 3 ++ > newlib/libc/machine/aarch64/strncmp.S | 4 +++ > newlib/libc/machine/aarch64/strnlen.S | 3 ++ > newlib/libc/machine/aarch64/strrchr.S | 3 ++ > 15 files changed, 89 insertions(+) > create mode 100644 newlib/libc/machine/aarch64/machine/asm.h > > diff --git a/newlib/libc/machine/aarch64/machine/asm.h b/newlib/libc/machine/aarch64/machine/asm.h > new file mode 100644 > index 000000000..92b36510f > --- /dev/null > +++ b/newlib/libc/machine/aarch64/machine/asm.h > @@ -0,0 +1,39 @@ > +/* > + Copyright (c) 2020 Kinsey Moore > + All rights reserved. > + > + Redistribution and use in source and binary forms, with or without > + modification, are permitted provided that the following conditions > + are met: > + 1. Redistributions of source code must retain the above copyright > + notice, this list of conditions and the following disclaimer. > + 2. Redistributions in binary form must reproduce the above copyright > + notice, this list of conditions and the following disclaimer in the > + documentation and/or other materials provided with the distribution. > + 3. The name of the company may not be used to endorse or promote > + products derived from this software without specific prior written > + permission. > + > + THIS SOFTWARE IS PROVIDED BY ARM LTD ``AS IS'' AND ANY EXPRESS OR IMPLIED > + WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF > + MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. > + IN NO EVENT SHALL ARM LTD BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > + SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED > + TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR > + PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING > + NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS > + SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > + > +#ifndef ASM_H > +#define ASM_H > + > + .macro ptr_param reg_num:req > +#ifdef __ILP32__ > + /* Sanitize padding bits of pointer arguments as per aapcs64 */ > + mov w\reg_num, w\reg_num > +#endif > + .endm > + > +#endif /* ASM_H */ > diff --git a/newlib/libc/machine/aarch64/memchr.S b/newlib/libc/machine/aarch64/memchr.S > index 53f5d6bc0..424d5e537 100644 > --- a/newlib/libc/machine/aarch64/memchr.S > +++ b/newlib/libc/machine/aarch64/memchr.S > @@ -31,6 +31,9 @@ > #if (defined (__OPTIMIZE_SIZE__) || defined (PREFER_SIZE_OVER_SPEED)) > /* See memchr-stub.c */ > #else > + > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64 > @@ -79,6 +82,7 @@ > .endm > > def_fn memchr > + ptr_param 0 > /* Do not dereference srcin if no bytes to compare. */ > cbz cntin, .Lzero_length > /* > diff --git a/newlib/libc/machine/aarch64/memcmp.S b/newlib/libc/machine/aarch64/memcmp.S > index 605d99365..8e19718f8 100644 > --- a/newlib/libc/machine/aarch64/memcmp.S > +++ b/newlib/libc/machine/aarch64/memcmp.S > @@ -58,6 +58,8 @@ > /* See memcmp-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64, unaligned accesses. > @@ -90,6 +92,8 @@ > .endm > > def_fn memcmp p2align=6 > + ptr_param 0 > + ptr_param 1 > subs limit, limit, 8 > b.lo L(less8) > > diff --git a/newlib/libc/machine/aarch64/memcpy.S b/newlib/libc/machine/aarch64/memcpy.S > index 463bad0a1..f35fc0055 100644 > --- a/newlib/libc/machine/aarch64/memcpy.S > +++ b/newlib/libc/machine/aarch64/memcpy.S > @@ -62,6 +62,8 @@ > /* See memcpy-stub.c */ > #else > > +#include > + > #define dstin x0 > #define src x1 > #define count x2 > @@ -105,6 +107,8 @@ > */ > > def_fn memcpy p2align=6 > + ptr_param 0 > + ptr_param 1 > prfm PLDL1KEEP, [src] > add srcend, src, count > add dstend, dstin, count > diff --git a/newlib/libc/machine/aarch64/memmove.S b/newlib/libc/machine/aarch64/memmove.S > index 597a8c8e9..9e0430d6c 100644 > --- a/newlib/libc/machine/aarch64/memmove.S > +++ b/newlib/libc/machine/aarch64/memmove.S > @@ -61,6 +61,8 @@ > /* See memmove-stub.c */ > #else > > +#include > + > .macro def_fn f p2align=0 > .text > .p2align \p2align > @@ -94,6 +96,8 @@ > */ > > def_fn memmove, 6 > + ptr_param 0 > + ptr_param 1 > sub tmp1, dstin, src > cmp count, 96 > ccmp tmp1, count, 2, hi > diff --git a/newlib/libc/machine/aarch64/memset.S b/newlib/libc/machine/aarch64/memset.S > index 103e3f8bb..216673895 100644 > --- a/newlib/libc/machine/aarch64/memset.S > +++ b/newlib/libc/machine/aarch64/memset.S > @@ -62,6 +62,8 @@ > /* See memset-stub.c */ > #else > > +#include > + > #define dstin x0 > #define val x1 > #define valw w1 > @@ -87,6 +89,7 @@ > > def_fn memset p2align=6 > > + ptr_param 0 > dup v0.16B, valw > add dstend, dstin, count > > diff --git a/newlib/libc/machine/aarch64/setjmp.S b/newlib/libc/machine/aarch64/setjmp.S > index 0856145bf..1032ae9ea 100644 > --- a/newlib/libc/machine/aarch64/setjmp.S > +++ b/newlib/libc/machine/aarch64/setjmp.S > @@ -26,6 +26,8 @@ > SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > */ > > +#include > + > #define GPR_LAYOUT \ > REG_PAIR (x19, x20, 0); \ > REG_PAIR (x21, x22, 16); \ > @@ -45,6 +47,7 @@ > .global setjmp > .type setjmp, %function > setjmp: > + ptr_param 0 > mov x16, sp > #define REG_PAIR(REG1, REG2, OFFS) stp REG1, REG2, [x0, OFFS] > #define REG_ONE(REG1, OFFS) str REG1, [x0, OFFS] > @@ -60,6 +63,7 @@ setjmp: > .global longjmp > .type longjmp, %function > longjmp: > + ptr_param 0 > #define REG_PAIR(REG1, REG2, OFFS) ldp REG1, REG2, [x0, OFFS] > #define REG_ONE(REG1, OFFS) ldr REG1, [x0, OFFS] > GPR_LAYOUT > diff --git a/newlib/libc/machine/aarch64/strchr.S b/newlib/libc/machine/aarch64/strchr.S > index 2448dbc7d..ba8f4e924 100644 > --- a/newlib/libc/machine/aarch64/strchr.S > +++ b/newlib/libc/machine/aarch64/strchr.S > @@ -31,6 +31,8 @@ > /* See strchr-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64 > @@ -83,6 +85,7 @@ > .endm > > def_fn strchr > + ptr_param 0 > /* Magic constant 0x40100401 to allow us to identify which lane > matches the requested byte. Magic constant 0x80200802 used > similarly for NUL termination. */ > diff --git a/newlib/libc/machine/aarch64/strchrnul.S b/newlib/libc/machine/aarch64/strchrnul.S > index a0ac13b7f..385f3b537 100644 > --- a/newlib/libc/machine/aarch64/strchrnul.S > +++ b/newlib/libc/machine/aarch64/strchrnul.S > @@ -31,6 +31,8 @@ > /* See strchrnul-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64 > @@ -79,6 +81,7 @@ > .endm > > def_fn strchrnul > + ptr_param 0 > /* Magic constant 0x40100401 to allow us to identify which lane > matches the termination condition. */ > mov wtmp2, #0x0401 > diff --git a/newlib/libc/machine/aarch64/strcmp.S b/newlib/libc/machine/aarch64/strcmp.S > index e2bef2d49..f9ac9e317 100644 > --- a/newlib/libc/machine/aarch64/strcmp.S > +++ b/newlib/libc/machine/aarch64/strcmp.S > @@ -33,6 +33,8 @@ > /* See strcmp-stub.c */ > #else > > +#include > + > .macro def_fn f p2align=0 > .text > .p2align \p2align > @@ -68,6 +70,8 @@ > > /* Start of performance-critical section -- one 64B cache line. */ > def_fn strcmp p2align=6 > + ptr_param 0 > + ptr_param 1 > eor tmp1, src1, src2 > mov zeroones, #REP8_01 > tst tmp1, #7 > diff --git a/newlib/libc/machine/aarch64/strcpy.S b/newlib/libc/machine/aarch64/strcpy.S > index e5405f253..b74b54881 100644 > --- a/newlib/libc/machine/aarch64/strcpy.S > +++ b/newlib/libc/machine/aarch64/strcpy.S > @@ -31,6 +31,8 @@ > /* See strchr-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64, unaligned accesses, min page size 4k. > @@ -112,6 +114,8 @@ > #define MIN_PAGE_SIZE (1 << MIN_PAGE_P2) > > def_fn STRCPY p2align=6 > + ptr_param 0 > + ptr_param 1 > /* For moderately short strings, the fastest way to do the copy is to > calculate the length of the string in the same way as strlen, then > essentially do a memcpy of the result. This avoids the need for > diff --git a/newlib/libc/machine/aarch64/strlen.S b/newlib/libc/machine/aarch64/strlen.S > index 872d136ef..75ff1a6eb 100644 > --- a/newlib/libc/machine/aarch64/strlen.S > +++ b/newlib/libc/machine/aarch64/strlen.S > @@ -28,6 +28,8 @@ > /* See strlen-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64, unaligned accesses, min page size 4k. > @@ -105,6 +107,7 @@ > boundary. */ > > def_fn strlen p2align=6 > + ptr_param 0 > and tmp1, srcin, MIN_PAGE_SIZE - 1 > mov zeroones, REP8_01 > cmp tmp1, MIN_PAGE_SIZE - 16 > diff --git a/newlib/libc/machine/aarch64/strncmp.S b/newlib/libc/machine/aarch64/strncmp.S > index ffdabc260..08e2b5f43 100644 > --- a/newlib/libc/machine/aarch64/strncmp.S > +++ b/newlib/libc/machine/aarch64/strncmp.S > @@ -28,6 +28,8 @@ > /* See strcmp-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64 > @@ -75,6 +77,8 @@ > nop /* Pad so that the loop below fits a cache line. */ > .endr > def_fn strncmp > + ptr_param 0 > + ptr_param 1 > cbz limit, .Lret0 > eor tmp1, src1, src2 > mov zeroones, #REP8_01 > diff --git a/newlib/libc/machine/aarch64/strnlen.S b/newlib/libc/machine/aarch64/strnlen.S > index c255c3f7c..1f3c0740f 100644 > --- a/newlib/libc/machine/aarch64/strnlen.S > +++ b/newlib/libc/machine/aarch64/strnlen.S > @@ -30,6 +30,8 @@ > /* See strlen-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64 > @@ -80,6 +82,7 @@ > ret > > def_fn strnlen > + ptr_param 0 > cbz limit, .Lhit_limit > mov zeroones, #REP8_01 > bic src, srcin, #15 > diff --git a/newlib/libc/machine/aarch64/strrchr.S b/newlib/libc/machine/aarch64/strrchr.S > index d64fc09b1..b55d28118 100644 > --- a/newlib/libc/machine/aarch64/strrchr.S > +++ b/newlib/libc/machine/aarch64/strrchr.S > @@ -31,6 +31,8 @@ > /* See strchr-stub.c */ > #else > > +#include > + > /* Assumptions: > * > * ARMv8-a, AArch64 > @@ -89,6 +91,7 @@ > .endm > > def_fn strrchr > + ptr_param 0 > /* Magic constant 0x40100401 to allow us to identify which lane > matches the requested byte. Magic constant 0x80200802 used > similarly for NUL termination. */ >