Arm32: correct string.h functions for "int" -> "unsigned char" conversion

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

Posted by Jan Beulich 1 year, 8 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 is apparently deemed more "obvious". It could be placed
later, as the code reachable without ever making it to the "1" label
only ever does byte stores.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: For memset() use the "more obvious" adjustment.

--- 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
@@ -15,6 +15,7 @@
 	.align	5
 
 ENTRY(memset)
+	and	r1, r1, #0xff
 	ands	r3, r0, #3		@ 1 unaligned?
 	mov	ip, r0			@ preserve r0 as return value
 	bne	6f			@ 1
--- 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 v2] Arm32: correct string.h functions for "int" -> "unsigned char" conversion

Posted by Bertrand Marquis 1 year, 8 months ago

Hi Jan,

> On 24 Aug 2022, at 13:33, Jan Beulich <jbeulich@suse.com> 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 is apparently deemed more "obvious". It could be placed
> later, as the code reachable without ever making it to the "1" label
> only ever does byte stores.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand
> ---
> v2: For memset() use the "more obvious" adjustment.
> 
> --- 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
> @@ -15,6 +15,7 @@
> 	.align	5
> 
> ENTRY(memset)
> +	and	r1, r1, #0xff
> 	ands	r3, r0, #3		@ 1 unaligned?
> 	mov	ip, r0			@ preserve r0 as return value
> 	bne	6f			@ 1
> --- 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 v2] Arm32: correct string.h functions for "int" -> "unsigned char" conversion

Posted by Julien Grall 1 year, 8 months ago


On 24/08/2022 13:44, Bertrand Marquis wrote:
>> On 24 Aug 2022, at 13:33, Jan Beulich <jbeulich@suse.com> 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 is apparently deemed more "obvious". It could be placed
>> later, as the code reachable without ever making it to the "1" label
>> only ever does byte stores.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

It is now committed.

Cheers,

-- 
Julien Grall

Re: [PATCH v2] Arm32: correct string.h functions for "int" -> "unsigned char" conversion

Posted by Jan Beulich 1 year, 8 months ago

On 25.08.2022 16:31, Julien Grall wrote:
> On 24/08/2022 13:44, Bertrand Marquis wrote:
>>> On 24 Aug 2022, at 13:33, Jan Beulich <jbeulich@suse.com> 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 is apparently deemed more "obvious". It could be placed
>>> later, as the code reachable without ever making it to the "1" label
>>> only ever does byte stores.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> It is now committed.

But then perhaps not pushed yet?

Jan

Re: [PATCH v2] Arm32: correct string.h functions for "int" -> "unsigned char" conversion

Posted by Julien Grall 1 year, 8 months ago

Hi Jan,

On 25/08/2022 15:34, Jan Beulich wrote:
> On 25.08.2022 16:31, Julien Grall wrote:
>> On 24/08/2022 13:44, Bertrand Marquis wrote:
>>>> On 24 Aug 2022, at 13:33, Jan Beulich <jbeulich@suse.com> 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 is apparently deemed more "obvious". It could be placed
>>>> later, as the code reachable without ever making it to the "1" label
>>>> only ever does byte stores.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>
>> It is now committed.
> 
> But then perhaps not pushed yet?

Yes. I tend to send the message just after I apply it. I will push when 
I am done with a batch (in this case, this is the only patch I pushed).

Cheers,

-- 
Julien Grall