| Message ID | 878snajjh2.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sh: use generic strncpy() | expand |
Hi Morimoto-san, Karl, On Wed, Dec 18, 2019 at 6:22 AM Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Current SH will get below warning at strncpy() > > In file included from ${LINUX}/arch/sh/include/asm/string.h:3, > from ${LINUX}/include/linux/string.h:20, > from ${LINUX}/include/linux/bitmap.h:9, > from ${LINUX}/include/linux/nodemask.h:95, > from ${LINUX}/include/linux/mmzone.h:17, > from ${LINUX}/include/linux/gfp.h:6, > from ${LINUX}/innclude/linux/slab.h:15, > from ${LINUX}/linux/drivers/mmc/host/vub300.c:38: > ${LINUX}/drivers/mmc/host/vub300.c: In function 'new_system_port_status': > ${LINUX}/arch/sh/include/asm/string_32.h:51:42: warning: array subscript\ > 80 is above array bounds of 'char[26]' [-Warray-bounds] > : "0" (__dest), "1" (__src), "r" (__src+__n) > ~~~~~^~~~ > > In general, strncpy() should behave like below. > > char dest[10]; > char *src = "12345"; > > strncpy(dest, src, 10); > // dest = {'1', '2', '3', '4', '5', > '\0','\0','\0','\0','\0'} > > But, current SH strnpy() has 2 issues. > 1st is it will access to out-of-memory (= src + 10). I believe this is not correct: the code does not really access memory beyond the end of the source string. (Recent) gcc just thinks so, because "__src+__n" is used as a parameter to the routine. > 2nd is it needs big fixup for it, and maintenance __asm__ > code is difficult. Yeah, the padding is missing. > To solve these issues, this patch simply uses generic strncpy() > instead of architecture specific one. That will definitely fix the issue, as we assume the generic implementation is correct ;-) Now, I've just tried, naively, to enable CONFIG_STRING_SELFTEST=y in my rts7751r2d build (without your patch), and boot it in qemu: String selftests succeeded Woops, turns out lib/test_string.c does not have any testcases for strncpy()... So adding test code for the corner cases may be a valuable contribution. Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert Thank you for your review > > But, current SH strnpy() has 2 issues. > > 1st is it will access to out-of-memory (= src + 10). > > I believe this is not correct: the code does not really access memory > beyond the end of the source string. (Recent) gcc just thinks so, > because "__src+__n" is used as a parameter to the routine. I see. Will fix in v2 Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/arch/sh/include/asm/string_32.h b/arch/sh/include/asm/string_32.h index 3558b1d..be3f9a0 100644 --- a/arch/sh/include/asm/string_32.h +++ b/arch/sh/include/asm/string_32.h @@ -28,32 +28,6 @@ static inline char *strcpy(char *__dest, const char *__src) return __xdest; } -#define __HAVE_ARCH_STRNCPY -static inline char *strncpy(char *__dest, const char *__src, size_t __n) -{ - register char *__xdest = __dest; - unsigned long __dummy; - - if (__n == 0) - return __xdest; - - __asm__ __volatile__( - "1:\n" - "mov.b @%1+, %2\n\t" - "mov.b %2, @%0\n\t" - "cmp/eq #0, %2\n\t" - "bt/s 2f\n\t" - " cmp/eq %5,%1\n\t" - "bf/s 1b\n\t" - " add #1, %0\n" - "2:" - : "=r" (__dest), "=r" (__src), "=&z" (__dummy) - : "0" (__dest), "1" (__src), "r" (__src+__n) - : "memory", "t"); - - return __xdest; -} - #define __HAVE_ARCH_STRCMP static inline int strcmp(const char *__cs, const char *__ct) {