[PATCH 1/2] Arm32: correct string.h functions for "int" -> "unsigned char" conversion

Jan Beulich posted 2 patches 3 years, 5 months ago
[PATCH 1/2] Arm32: correct string.h functions for "int" -> "unsigned char" conversion
Posted by Jan Beulich 3 years, 5 months ago
While Arm64 does so uniformly, for Arm32 only strchr() currently handles
this properly. Add the necessary conversion also to strrchr(), memchr(),
and memset().

As to the placement in memset(): Putting the new insn at the beginning
of the function could perhaps be deemed more "obvious", but the code
reachable without ever making it to the "1" label only ever does byte
stores.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/arm/arm32/lib/memchr.S
+++ b/xen/arch/arm/arm32/lib/memchr.S
@@ -14,6 +14,7 @@
 	.text
 	.align	5
 ENTRY(memchr)
+	and	r1, r1, #0xff
 1:	subs	r2, r2, #1
 	bmi	2f
 	ldrb	r3, [r0], #1
--- a/xen/arch/arm/arm32/lib/memset.S
+++ b/xen/arch/arm/arm32/lib/memset.S
@@ -21,7 +21,8 @@ ENTRY(memset)
 /*
  * we know that the pointer in ip is aligned to a word boundary.
  */
-1:	orr	r1, r1, r1, lsl #8
+1:	and	r1, r1, #0xff
+	orr	r1, r1, r1, lsl #8
 	orr	r1, r1, r1, lsl #16
 	mov	r3, r1
 	cmp	r2, #16
--- a/xen/arch/arm/arm32/lib/strrchr.S
+++ b/xen/arch/arm/arm32/lib/strrchr.S
@@ -14,6 +14,7 @@
 		.text
 		.align	5
 ENTRY(strrchr)
+		and	r1, r1, #0xff
 		mov	r3, #0
 1:		ldrb	r2, [r0], #1
 		teq	r2, r1
Re: [PATCH 1/2] Arm32: correct string.h functions for "int" -> "unsigned char" conversion
Posted by Julien Grall 3 years, 5 months ago
Hi Jan,

On 19/08/2022 08:49, Jan Beulich wrote:
> While Arm64 does so uniformly, for Arm32 only strchr() currently handles
> this properly. Add the necessary conversion also to strrchr(), memchr(),
> and memset().
> 
> As to the placement in memset(): Putting the new insn at the beginning
> of the function could perhaps be deemed more "obvious", but the code
> reachable without ever making it to the "1" label only ever does byte
> stores.
So the assumption here is the rest of the code will always use byte 
stores. Given that this issue has been present for a long time, I think 
it would be wiser to do the conversion at the start of the function.

The changes in memchr() and strrchr() looks fine to me.

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/arm/arm32/lib/memchr.S
> +++ b/xen/arch/arm/arm32/lib/memchr.S
> @@ -14,6 +14,7 @@
>   	.text
>   	.align	5
>   ENTRY(memchr)
> +	and	r1, r1, #0xff
>   1:	subs	r2, r2, #1
>   	bmi	2f
>   	ldrb	r3, [r0], #1
> --- a/xen/arch/arm/arm32/lib/memset.S
> +++ b/xen/arch/arm/arm32/lib/memset.S
> @@ -21,7 +21,8 @@ ENTRY(memset)
>   
>    * we know that the pointer in ip is aligned to a word boundary.
>    */
> -1:	orr	r1, r1, r1, lsl #8
> +1:	and	r1, r1, #0xff
> +	orr	r1, r1, r1, lsl #8
>   	orr	r1, r1, r1, lsl #16
>   	mov	r3, r1
>   	cmp	r2, #16
> --- a/xen/arch/arm/arm32/lib/strrchr.S
> +++ b/xen/arch/arm/arm32/lib/strrchr.S
> @@ -14,6 +14,7 @@
>   		.text
>   		.align	5
>   ENTRY(strrchr)
> +		and	r1, r1, #0xff
>   		mov	r3, #0
>   1:		ldrb	r2, [r0], #1
>   		teq	r2, r1
> 

Cheers,

-- 
Julien Grall