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

[Julien Grall <julien@xxxxxxx>]

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@xxxxxxxx>
>
> --- 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