RE: AArch64 ILP32 strcmp bug

[Kinsey Moore <kinsey.moore@oarcorp.com>]

[-- Attachment #1: Type: text/plain, Size: 2339 bytes --]

-----Original Message-----
From: Richard Earnshaw <Richard.Earnshaw@foss.arm.com> 
Sent: Wednesday, November 25, 2020 05:29
To: Kinsey Moore <kinsey.moore@oarcorp.com>; newlib@sourceware.org
Subject: Re: AArch64 ILP32 strcmp bug

>On 20/07/2020 14:52, Kinsey Moore wrote:
>> Hi,
>> It appears that the hand-coded assembly in AArch64 strcmp does not sanitize the incoming address parameters in x0 and x1 when compiled for AArch64 ILP32. Based on my reading of the AArch64 Procedure Call Specification and GCC's output for similar function signatures, the callee is responsible for sanitization of the pointer addresses. I encountered this because I have a struct containing a pointer and length returned from another function that happens to get packed into a single register (x0) and GCC passes this unmodified into strcmp as the first argument.
>> 
>> According to the aapcs64: "Any part of a register or a stack slot that is not used for an argument (padding bits) has unspecified content at the callee entry point."
>> 
>> I suspect this is a problem for the majority of hand-written AArch64 assembly in newlib.
>> 
>> Please let me know if I missed something.
>
> Apologies, somehow this message got marked as read although it never received a response.
>
> I don't think we've really added support for ILP32 to newlib.  This may just be one corner of a fairly large can of worms.
> 
> The Arm/AArch64 optimized assembly routines are really just copies (provided that they've been kept up-to-date) of the code that Arm publishes as part of its Arm Optimized Routines package (https://github.com/ARM-software/optimized-routines); but even those do not have ILP32 support at this time.  The best place to address this is to raise an issue there.

Thanks for taking a look at this. I did see some flags related to ILP32 elsewhere in newlib which made me think it was supported but, as you mention, none of the assembly is setup for it. In the interim, I've been working around this by adding -DPREFER_SIZE_OVER_SPEED to the newlib CFLAGS to force use of compiled C sources instead of hand-written assembly. The patch to automatically specify this for RTEMS is attached, but I suspect that this may be applicable for all AArch64 targets that wish to take advantage of ILP32. I'll create an issue upstream.

Kinsey

[-- Attachment #2: newlib-ilp32.patch --]
[-- Type: application/octet-stream, Size: 945 bytes --]

--- a/newlib/configure.host	2020-11-25 08:38:24.751655619 -0600
+++ b/newlib/configure.host	2020-11-25 08:54:25.911655619 -0600
@@ -664,6 +664,16 @@
 newlib_cflags="${newlib_cflags} -DCLOCK_PROVIDED -DMALLOC_PROVIDED -DEXIT_PROVIDED -DSIGNAL_PROVIDED -DGETREENT_PROVIDED -DREENTRANT_SYSCALLS_PROVIDED -DHAVE_NANOSLEEP -DHAVE_BLKSIZE -DHAVE_FCNTL -DHAVE_ASSERT_FUNC"
         # turn off unsupported items in posix directory 
 	newlib_cflags="${newlib_cflags} -D_NO_GETLOGIN -D_NO_GETPWENT -D_NO_GETUT -D_NO_GETPASS -D_NO_SIGSET -D_NO_WORDEXP -D_NO_POPEN -D_NO_POSIX_SPAWN"
+
+	# The hand-written assembly in newlib does not currently support AArch64/ILP32 ABI
+	# Build from C source instead
+	case "${host}" in
+		aarch64*-*-rtems*)
+			newlib_cflags="${newlib_cflags} -DPREFER_SIZE_OVER_SPEED"
+			;;
+		*)
+			;;
+	esac
 	;;
 # VxWorks supplies its own version of malloc, and the newlib one
 # doesn't work because VxWorks does not have sbrk.