Message ID | 1399273875-8403-1-git-send-email-a.ryabinin@samsung.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Andrey, On Mon, May 05, 2014 at 11:11:13AM +0400, Andrey Ryabinin wrote: > memset doesn't work right for following example: > > signed char c = 0xF0; > memset(addr, c, size); > > Variable c is signed, so after typcasting to int the value will be 0xFFFFFFF0. > This value will be passed through r1 regitster to memset function. > memset doesn't zero out upper bytes in r1, so memory will be filled > with 0xFFFFFFF0 instead of expected 0xF0F0F0F0. > --- a/arch/arm/lib/memset.S > +++ b/arch/arm/lib/memset.S > @@ -22,7 +22,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 int is to be converted to unsigned char in memset, would having above change immediately upon entry to memset rather than at a place where it won't always execute make intention clearer ? (although it doesn't make difference) ubfx r1, r1, #0, #8 would have given the needed typecasting, but seems it is available only on ARMv6T2 & above. Regards Afzal
On 05/08/14 03:42, Afzal Mohammed wrote: > > int is to be converted to unsigned char in memset, would having above > change immediately upon entry to memset rather than at a place where it > won't always execute make intention clearer ? (although it doesn't make > difference) > I think it's better to keep it near other manipulations with r1. Plus this will save us from executing extra instruction on 'memset -> 6 -> 5 -> return' path (memset for size <= 3). > ubfx r1, r1, #0, #8 would have given the needed typecasting, but seems > it is available only on ARMv6T2 & above. > Indeed. It might be wrapped it with #if #else, but it will be a bit ugly, probably not worth to do.
On 05/08/14 12:38, Vladimir Murzin wrote: > Vladimir Murzin <murzin.v <at> gmail.com> writes: > >> >> Andrey Ryabinin <a.ryabinin <at> samsung.com> writes: >> >>> >>> memset doesn't work right for following example: >>> >>> signed char c = 0xF0; >>> memset(addr, c, size); >>> >>> Variable c is signed, so after typcasting to int the value will be > 0xFFFFFFF0. >>> This value will be passed through r1 regitster to memset function. >>> memset doesn't zero out upper bytes in r1, so memory will be filled >>> with 0xFFFFFFF0 instead of expected 0xF0F0F0F0. >> >> It behaves the same as a generic implementation in lib/string.c, moreover >> gcc built-in behaves the same. So it looks like expected behavior and POSIX >> Programmer's Manual (man 3posix memset) explicitly says that "c" is >> converted to unsigned char. >> >> Cheers >> Vladimir >> >> > Hi Vladimir. Please, use reply to all next time, otherwise I might miss your mails. > Sorry, had a bad coffee when posted it ;) It behaves /different/, but it is > here for many years... Your are right, it's here for many years and wasn't noticed, probably because, there are no such users, that call memset with not zero upper bytes in second argument. Though, I can't tell this for sure, there are might be a few of such calls. So this fix probably doesn't change anything for current kernel, however it might help to avoid problems in future. Also, I think would be good to have the same behavior on different architectures (arm64/x86 works correctly for example above). > doesn't this change break something? iow, it possible > that some user rely on this behavior... > I don't think that such user exist, cause such user must know implementation details of memset on ARM and rely on them. If such user exist that's insane user and should be fixed. > Vladimir >
diff --git a/arch/arm/lib/memset.S b/arch/arm/lib/memset.S index 94b0650..a010f76 100644 --- a/arch/arm/lib/memset.S +++ b/arch/arm/lib/memset.S @@ -22,7 +22,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
memset doesn't work right for following example: signed char c = 0xF0; memset(addr, c, size); Variable c is signed, so after typcasting to int the value will be 0xFFFFFFF0. This value will be passed through r1 regitster to memset function. memset doesn't zero out upper bytes in r1, so memory will be filled with 0xFFFFFFF0 instead of expected 0xF0F0F0F0. Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com> --- arch/arm/lib/memset.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)