* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 8:29 ` Geert Uytterhoeven
0 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 8:29 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Morimoto-san,
On Tue, Dec 17, 2019 at 7:09 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> We get too many below strncpy() warning on SH.
> Can someone solve it ?
> I don't remember SH assembler code / can't test it...
I never touched SH assembler code at all.
But it looks a bit like RISCified m68k, so let's give it a try ;-)
> In file included from /home/morimoto/WORK/linux/arch/sh/include/asm/string.h:3,
> from /home/morimoto/WORK/linux/include/linux/string.h:20,
> from /home/morimoto/WORK/linux/include/linux/bitmap.h:9,
> from /home/morimoto/WORK/linux/include/linux/nodemask.h:95,
> from /home/morimoto/WORK/linux/include/linux/mmzone.h:17,
> from /home/morimoto/WORK/linux/include/linux/gfp.h:6,
> from /home/morimoto/WORK/linux/include/linux/slab.h:15,
> from /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c:38:
> /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> /home/morimoto/WORK/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)
> ~~~~~^~~~
Yeah, these array warnings are (sometimes) a PITA.
> 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;
> }
My first thought was to just replace "__src+__n" by "__dest+__n", and
change the "cmp/eq" from "%1" (current src) to "%0" (current dst).
However, "%0" isn't incremented until the branch delay slot of the loop.
So I had to move the increment up, and fill the branch delay slot with a nop.
Untested (it-compiles-so-it-must-be-perfect ;-) whitespace-damaged patch:
--- a/arch/sh/include/asm/string_32.h
+++ b/arch/sh/include/asm/string_32.h
@@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
char *__src, size_t __n)
__asm__ __volatile__(
"1:\n"
"mov.b @%1+, %2\n\t"
- "mov.b %2, @%0\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"
+ " cmp/eq %5,%0\n\t"
"bf/s 1b\n\t"
- " add #1, %0\n"
+ " nop\n"
"2:"
: "=r" (__dest), "=r" (__src), "=&z" (__dummy)
- : "0" (__dest), "1" (__src), "r" (__src+__n)
+ : "0" (__dest), "1" (__src), "r" (__dest+__n)
: "memory", "t");
return __xdest;
Does this make sense?
Can it be improved, by putting something useful in the delay slot?
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 8:29 ` Geert Uytterhoeven
@ 2019-12-17 8:37 ` Kuninori Morimoto
-1 siblings, 0 replies; 37+ messages in thread
From: Kuninori Morimoto @ 2019-12-17 8:37 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Geert
Thank you for your feedback
> --- a/arch/sh/include/asm/string_32.h
> +++ b/arch/sh/include/asm/string_32.h
> @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> char *__src, size_t __n)
> __asm__ __volatile__(
> "1:\n"
> "mov.b @%1+, %2\n\t"
> - "mov.b %2, @%0\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"
> + " cmp/eq %5,%0\n\t"
> "bf/s 1b\n\t"
> - " add #1, %0\n"
> + " nop\n"
> "2:"
> : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> - : "0" (__dest), "1" (__src), "r" (__src+__n)
> + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> : "memory", "t");
>
> return __xdest;
>
> Does this make sense?
> Can it be improved, by putting something useful in the delay slot?
Unfortunately, I can't test it :(
Can someone test it ?
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 8:37 ` Kuninori Morimoto
0 siblings, 0 replies; 37+ messages in thread
From: Kuninori Morimoto @ 2019-12-17 8:37 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Geert
Thank you for your feedback
> --- a/arch/sh/include/asm/string_32.h
> +++ b/arch/sh/include/asm/string_32.h
> @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> char *__src, size_t __n)
> __asm__ __volatile__(
> "1:\n"
> "mov.b @%1+, %2\n\t"
> - "mov.b %2, @%0\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"
> + " cmp/eq %5,%0\n\t"
> "bf/s 1b\n\t"
> - " add #1, %0\n"
> + " nop\n"
> "2:"
> : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> - : "0" (__dest), "1" (__src), "r" (__src+__n)
> + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> : "memory", "t");
>
> return __xdest;
>
> Does this make sense?
> Can it be improved, by putting something useful in the delay slot?
Unfortunately, I can't test it :(
Can someone test it ?
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 8:37 ` Kuninori Morimoto
@ 2019-12-17 8:43 ` Geert Uytterhoeven
-1 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 8:43 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Morimoto-san,
On Tue, Dec 17, 2019 at 9:37 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Unfortunately, I can't test it :(
> Can someone test it ?
make rts7751r2dplus_defconfig
qemu-system-sh4 qemu-system-sh4 -M r2d -serial null -serial mon:stdio \
-nographic -no-reboot -m 256 \
-append "panic=1 HOST=sh4 console=ttySC1 noiotrap" \
-kernel arch/sh/boot/zImage
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 8:43 ` Geert Uytterhoeven
0 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 8:43 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Morimoto-san,
On Tue, Dec 17, 2019 at 9:37 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> Unfortunately, I can't test it :(
> Can someone test it ?
make rts7751r2dplus_defconfig
qemu-system-sh4 qemu-system-sh4 -M r2d -serial null -serial mon:stdio \
-nographic -no-reboot -m 256 \
-append "panic=1 HOST=sh4 console=ttySC1 noiotrap" \
-kernel arch/sh/boot/zImage
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 8:29 ` Geert Uytterhoeven
@ 2019-12-17 8:40 ` Geert Uytterhoeven
-1 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 8:40 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Morimoto-san,
On Tue, Dec 17, 2019 at 9:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Dec 17, 2019 at 7:09 AM Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
> > We get too many below strncpy() warning on SH.
> > Can someone solve it ?
> > I don't remember SH assembler code / can't test it...
>
> I never touched SH assembler code at all.
> But it looks a bit like RISCified m68k, so let's give it a try ;-)
>
> > In file included from /home/morimoto/WORK/linux/arch/sh/include/asm/string.h:3,
> > from /home/morimoto/WORK/linux/include/linux/string.h:20,
> > from /home/morimoto/WORK/linux/include/linux/bitmap.h:9,
> > from /home/morimoto/WORK/linux/include/linux/nodemask.h:95,
> > from /home/morimoto/WORK/linux/include/linux/mmzone.h:17,
> > from /home/morimoto/WORK/linux/include/linux/gfp.h:6,
> > from /home/morimoto/WORK/linux/include/linux/slab.h:15,
> > from /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c:38:
> > /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> > /home/morimoto/WORK/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)
> > ~~~~~^~~~
>
> Yeah, these array warnings are (sometimes) a PITA.
>
> > 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;
> > }
>
>
> My first thought was to just replace "__src+__n" by "__dest+__n", and
> change the "cmp/eq" from "%1" (current src) to "%0" (current dst).
> However, "%0" isn't incremented until the branch delay slot of the loop.
> So I had to move the increment up, and fill the branch delay slot with a nop.
>
> Untested (it-compiles-so-it-must-be-perfect ;-) whitespace-damaged patch:
>
> --- a/arch/sh/include/asm/string_32.h
> +++ b/arch/sh/include/asm/string_32.h
> @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> char *__src, size_t __n)
> __asm__ __volatile__(
> "1:\n"
> "mov.b @%1+, %2\n\t"
> - "mov.b %2, @%0\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"
> + " cmp/eq %5,%0\n\t"
> "bf/s 1b\n\t"
> - " add #1, %0\n"
> + " nop\n"
> "2:"
> : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> - : "0" (__dest), "1" (__src), "r" (__src+__n)
> + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> : "memory", "t");
>
> return __xdest;
>
> Does this make sense?
> Can it be improved, by putting something useful in the delay slot?
BTW, there seems to be a serious security issue with this strncpy()
implementation: while it never writes more than n bytes in the
destination buffer, it doesn't pad the destination buffer with zeroes if
the source string is shorter than the buffer size. This will leak
data.
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 8:40 ` Geert Uytterhoeven
0 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 8:40 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Morimoto-san,
On Tue, Dec 17, 2019 at 9:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Dec 17, 2019 at 7:09 AM Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
> > We get too many below strncpy() warning on SH.
> > Can someone solve it ?
> > I don't remember SH assembler code / can't test it...
>
> I never touched SH assembler code at all.
> But it looks a bit like RISCified m68k, so let's give it a try ;-)
>
> > In file included from /home/morimoto/WORK/linux/arch/sh/include/asm/string.h:3,
> > from /home/morimoto/WORK/linux/include/linux/string.h:20,
> > from /home/morimoto/WORK/linux/include/linux/bitmap.h:9,
> > from /home/morimoto/WORK/linux/include/linux/nodemask.h:95,
> > from /home/morimoto/WORK/linux/include/linux/mmzone.h:17,
> > from /home/morimoto/WORK/linux/include/linux/gfp.h:6,
> > from /home/morimoto/WORK/linux/include/linux/slab.h:15,
> > from /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c:38:
> > /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> > /home/morimoto/WORK/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)
> > ~~~~~^~~~
>
> Yeah, these array warnings are (sometimes) a PITA.
>
> > 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;
> > }
>
>
> My first thought was to just replace "__src+__n" by "__dest+__n", and
> change the "cmp/eq" from "%1" (current src) to "%0" (current dst).
> However, "%0" isn't incremented until the branch delay slot of the loop.
> So I had to move the increment up, and fill the branch delay slot with a nop.
>
> Untested (it-compiles-so-it-must-be-perfect ;-) whitespace-damaged patch:
>
> --- a/arch/sh/include/asm/string_32.h
> +++ b/arch/sh/include/asm/string_32.h
> @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> char *__src, size_t __n)
> __asm__ __volatile__(
> "1:\n"
> "mov.b @%1+, %2\n\t"
> - "mov.b %2, @%0\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"
> + " cmp/eq %5,%0\n\t"
> "bf/s 1b\n\t"
> - " add #1, %0\n"
> + " nop\n"
> "2:"
> : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> - : "0" (__dest), "1" (__src), "r" (__src+__n)
> + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> : "memory", "t");
>
> return __xdest;
>
> Does this make sense?
> Can it be improved, by putting something useful in the delay slot?
BTW, there seems to be a serious security issue with this strncpy()
implementation: while it never writes more than n bytes in the
destination buffer, it doesn't pad the destination buffer with zeroes if
the source string is shorter than the buffer size. This will leak
data.
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 8:40 ` Geert Uytterhoeven
@ 2019-12-17 8:51 ` Kuninori Morimoto
-1 siblings, 0 replies; 37+ messages in thread
From: Kuninori Morimoto @ 2019-12-17 8:51 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Geert
Cc Yoshinori-san
> > --- a/arch/sh/include/asm/string_32.h
> > +++ b/arch/sh/include/asm/string_32.h
> > @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> > char *__src, size_t __n)
> > __asm__ __volatile__(
> > "1:\n"
> > "mov.b @%1+, %2\n\t"
> > - "mov.b %2, @%0\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"
> > + " cmp/eq %5,%0\n\t"
> > "bf/s 1b\n\t"
> > - " add #1, %0\n"
> > + " nop\n"
> > "2:"
> > : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> > - : "0" (__dest), "1" (__src), "r" (__src+__n)
> > + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> > : "memory", "t");
> >
> > return __xdest;
> >
> > Does this make sense?
> > Can it be improved, by putting something useful in the delay slot?
>
> BTW, there seems to be a serious security issue with this strncpy()
> implementation: while it never writes more than n bytes in the
> destination buffer, it doesn't pad the destination buffer with zeroes if
> the source string is shorter than the buffer size. This will leak
> data.
Yeah...
I can only do is "Reporting issue" to SH ML, unfortunately...
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 8:51 ` Kuninori Morimoto
0 siblings, 0 replies; 37+ messages in thread
From: Kuninori Morimoto @ 2019-12-17 8:51 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
Hi Geert
Cc Yoshinori-san
> > --- a/arch/sh/include/asm/string_32.h
> > +++ b/arch/sh/include/asm/string_32.h
> > @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> > char *__src, size_t __n)
> > __asm__ __volatile__(
> > "1:\n"
> > "mov.b @%1+, %2\n\t"
> > - "mov.b %2, @%0\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"
> > + " cmp/eq %5,%0\n\t"
> > "bf/s 1b\n\t"
> > - " add #1, %0\n"
> > + " nop\n"
> > "2:"
> > : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> > - : "0" (__dest), "1" (__src), "r" (__src+__n)
> > + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> > : "memory", "t");
> >
> > return __xdest;
> >
> > Does this make sense?
> > Can it be improved, by putting something useful in the delay slot?
>
> BTW, there seems to be a serious security issue with this strncpy()
> implementation: while it never writes more than n bytes in the
> destination buffer, it doesn't pad the destination buffer with zeroes if
> the source string is shorter than the buffer size. This will leak
> data.
Yeah...
I can only do is "Reporting issue" to SH ML, unfortunately...
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 8:51 ` Kuninori Morimoto
@ 2019-12-17 9:09 ` Karl Nasrallah
-1 siblings, 0 replies; 37+ messages in thread
From: Karl Nasrallah @ 2019-12-17 9:09 UTC (permalink / raw)
To: kuninori.morimoto.gx, geert; +Cc: ysato, dalias, linux-sh, linux-renesas-soc
Hello,
Give me a day or so and I can do the following things:
1) Write you all a brand new standards-conforming strncpy in SH4 asm like this that is easier to read
2) Compile it with sh4-elf-GCC 9.2
3) Test it on a real SH4 (SH7750/SH7750R-like)
The warning, if it shows up in my test, would likely then be a GCC thing--I have an idea of what it's doing, but I'll be sure after that.
Unfortunately it's 4AM here on the other side of the world right now...
-Karl
-----Original Message-----
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>; Rich Felker <dalias@libc.org>; Linux-SH <linux-sh@vger.kernel.org>; Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Sent: Tue, Dec 17, 2019 3:51 am
Subject: Re: can someone solve string_32.h issue for SH ?
Hi Geert
Cc Yoshinori-san
> > --- a/arch/sh/include/asm/string_32.h
> > +++ b/arch/sh/include/asm/string_32.h
> > @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> > char *__src, size_t __n)
> > __asm__ __volatile__(
> > "1:\n"
> > "mov.b @%1+, %2\n\t"
> > - "mov.b %2, @%0\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"
> > + " cmp/eq %5,%0\n\t"
> > "bf/s 1b\n\t"
> > - " add #1, %0\n"
> > + " nop\n"
> > "2:"
> > : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> > - : "0" (__dest), "1" (__src), "r" (__src+__n)
> > + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> > : "memory", "t");
> >
> > return __xdest;
> >
> > Does this make sense?
> > Can it be improved, by putting something useful in the delay slot?
>
> BTW, there seems to be a serious security issue with this strncpy()
> implementation: while it never writes more than n bytes in the
> destination buffer, it doesn't pad the destination buffer with zeroes if
> the source string is shorter than the buffer size. This will leak
> data.
Yeah...
I can only do is "Reporting issue" to SH ML, unfortunately...
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 9:09 ` Karl Nasrallah
0 siblings, 0 replies; 37+ messages in thread
From: Karl Nasrallah @ 2019-12-17 9:09 UTC (permalink / raw)
To: kuninori.morimoto.gx, geert; +Cc: ysato, dalias, linux-sh, linux-renesas-soc
Hello,
Give me a day or so and I can do the following things:
1) Write you all a brand new standards-conforming strncpy in SH4 asm like this that is easier to read
2) Compile it with sh4-elf-GCC 9.2
3) Test it on a real SH4 (SH7750/SH7750R-like)
The warning, if it shows up in my test, would likely then be a GCC thing--I have an idea of what it's doing, but I'll be sure after that.
Unfortunately it's 4AM here on the other side of the world right now...
-Karl
-----Original Message-----
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>; Rich Felker <dalias@libc.org>; Linux-SH <linux-sh@vger.kernel.org>; Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Sent: Tue, Dec 17, 2019 3:51 am
Subject: Re: can someone solve string_32.h issue for SH ?
Hi Geert
Cc Yoshinori-san
> > --- a/arch/sh/include/asm/string_32.h
> > +++ b/arch/sh/include/asm/string_32.h
> > @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> > char *__src, size_t __n)
> > __asm__ __volatile__(
> > "1:\n"
> > "mov.b @%1+, %2\n\t"
> > - "mov.b %2, @%0\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"
> > + " cmp/eq %5,%0\n\t"
> > "bf/s 1b\n\t"
> > - " add #1, %0\n"
> > + " nop\n"
> > "2:"
> > : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> > - : "0" (__dest), "1" (__src), "r" (__src+__n)
> > + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> > : "memory", "t");
> >
> > return __xdest;
> >
> > Does this make sense?
> > Can it be improved, by putting something useful in the delay slot?
>
> BTW, there seems to be a serious security issue with this strncpy()
> implementation: while it never writes more than n bytes in the
> destination buffer, it doesn't pad the destination buffer with zeroes if
> the source string is shorter than the buffer size. This will leak
> data.
Yeah...
I can only do is "Reporting issue" to SH ML, unfortunately...
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 9:09 ` Karl Nasrallah
@ 2019-12-17 22:16 ` Karl Nasrallah
-1 siblings, 0 replies; 37+ messages in thread
From: Karl Nasrallah @ 2019-12-17 22:16 UTC (permalink / raw)
To: kuninori.morimoto.gx, geert; +Cc: ysato, dalias, linux-sh, linux-renesas-soc
Hello!
I have a strncpy for you.
static inline char *strncpy(char *__dest, const char *__src, size_t __n)
{
char * retval = __dest;
const char * __dest_end = __dest + __n - 1;
// size_t is always unsigned
if(__n == 0)
{
return retval;
}
__asm__ __volatile__ (
"strncpy_start:\n\t"
"mov.b @%[src]+,r0\n\t"
"cmp/eq #0,r0\n\t" // cmp/eq #imm8,r0 is its own instruction
"bt.s strncpy_pad\n\t" // Done with the string
"cmp/eq %[dest],%[dest_end]\n\t" // This takes care of the size parameter in only one instruction ;)
"bt.s strncpy_end\n\t"
"mov.b r0,@%[dest]\n\t"
"bra strncpy_start\n\t"
"add #1,%[dest]\n\t" // mov.b R0,@Rn+ is SH2A only, but we can fill the delay slot with the offset
"strncpy_pad:\n\t"
"bt.s strncpy_end\n\t"
"mov.b r0,@%[dest]\n\t"
"add #1,%[dest]\n\t"
"bra strncpy_pad\n\t"
"cmp/eq %[dest],%[dest_end]\n\t"
"strncpy_end:\n\t" // All done
: [src] "+r" (__src), [dest] "+r" (__dest)
: [dest_end] "r" (__dest_end)
: "t"
);
return retval;
}
Tested with sh4-elf-gcc 9.2.0 on a real SH7750/SH7750R-compatible system. No warnings, behaves exactly as per linux (dot) die (dot) net/man/3/strncpy and I optimized it with some tricks I devised from writing extremely optimized x86. If there are any doubts as to the authenticity, note that I am the sole author of this project: github (dot) com/KNNSpeed/AVX-Memmove
Hope this helps!
-Karl
(P.S. Consider this code public domain. If for whatever reason that doesn't fly, then I give the Linux kernel community explicit permission to use it as they see fit.)
-----Original Message-----
From: Karl Nasrallah <knnspeed@aol.com>
To: kuninori.morimoto.gx <kuninori.morimoto.gx@renesas.com>; geert <geert@linux-m68k.org>
Cc: ysato <ysato@users.sourceforge.jp>; dalias <dalias@libc.org>; linux-sh <linux-sh@vger.kernel.org>; linux-renesas-soc <linux-renesas-soc@vger.kernel.org>
Sent: Tue, Dec 17, 2019 4:09 am
Subject: Re: can someone solve string_32.h issue for SH ?
Hello,
Give me a day or so and I can do the following things:
1) Write you all a brand new standards-conforming strncpy in SH4 asm like this that is easier to read
2) Compile it with sh4-elf-GCC 9.2
3) Test it on a real SH4 (SH7750/SH7750R-like)
The warning, if it shows up in my test, would likely then be a GCC thing--I have an idea of what it's doing, but I'll be sure after that.
Unfortunately it's 4AM here on the other side of the world right now...
-Karl
-----Original Message-----
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>; Rich Felker <dalias@libc.org>; Linux-SH <linux-sh@vger.kernel.org>; Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Sent: Tue, Dec 17, 2019 3:51 am
Subject: Re: can someone solve string_32.h issue for SH ?
Hi Geert
Cc Yoshinori-san
> > --- a/arch/sh/include/asm/string_32.h
> > +++ b/arch/sh/include/asm/string_32.h
> > @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> > char *__src, size_t __n)
> > __asm__ __volatile__(
> > "1:\n"
> > "mov.b @%1+, %2\n\t"
> > - "mov.b %2, @%0\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"
> > + " cmp/eq %5,%0\n\t"
> > "bf/s 1b\n\t"
> > - " add #1, %0\n"
> > + " nop\n"
> > "2:"
> > : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> > - : "0" (__dest), "1" (__src), "r" (__src+__n)
> > + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> > : "memory", "t");
> >
> > return __xdest;
> >
> > Does this make sense?
> > Can it be improved, by putting something useful in the delay slot?
>
> BTW, there seems to be a serious security issue with this strncpy()
> implementation: while it never writes more than n bytes in the
> destination buffer, it doesn't pad the destination buffer with zeroes if
> the source string is shorter than the buffer size. This will leak
> data.
Yeah...
I can only do is "Reporting issue" to SH ML, unfortunately...
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 22:16 ` Karl Nasrallah
0 siblings, 0 replies; 37+ messages in thread
From: Karl Nasrallah @ 2019-12-17 22:16 UTC (permalink / raw)
To: kuninori.morimoto.gx, geert; +Cc: ysato, dalias, linux-sh, linux-renesas-soc
Hello!
I have a strncpy for you.
static inline char *strncpy(char *__dest, const char *__src, size_t __n)
{
char * retval = __dest;
const char * __dest_end = __dest + __n - 1;
// size_t is always unsigned
if(__n == 0)
{
return retval;
}
__asm__ __volatile__ (
"strncpy_start:\n\t"
"mov.b @%[src]+,r0\n\t"
"cmp/eq #0,r0\n\t" // cmp/eq #imm8,r0 is its own instruction
"bt.s strncpy_pad\n\t" // Done with the string
"cmp/eq %[dest],%[dest_end]\n\t" // This takes care of the size parameter in only one instruction ;)
"bt.s strncpy_end\n\t"
"mov.b r0,@%[dest]\n\t"
"bra strncpy_start\n\t"
"add #1,%[dest]\n\t" // mov.b R0,@Rn+ is SH2A only, but we can fill the delay slot with the offset
"strncpy_pad:\n\t"
"bt.s strncpy_end\n\t"
"mov.b r0,@%[dest]\n\t"
"add #1,%[dest]\n\t"
"bra strncpy_pad\n\t"
"cmp/eq %[dest],%[dest_end]\n\t"
"strncpy_end:\n\t" // All done
: [src] "+r" (__src), [dest] "+r" (__dest)
: [dest_end] "r" (__dest_end)
: "t"
);
return retval;
}
Tested with sh4-elf-gcc 9.2.0 on a real SH7750/SH7750R-compatible system. No warnings, behaves exactly as per linux (dot) die (dot) net/man/3/strncpy and I optimized it with some tricks I devised from writing extremely optimized x86. If there are any doubts as to the authenticity, note that I am the sole author of this project: github (dot) com/KNNSpeed/AVX-Memmove
Hope this helps!
-Karl
(P.S. Consider this code public domain. If for whatever reason that doesn't fly, then I give the Linux kernel community explicit permission to use it as they see fit.)
-----Original Message-----
From: Karl Nasrallah <knnspeed@aol.com>
To: kuninori.morimoto.gx <kuninori.morimoto.gx@renesas.com>; geert <geert@linux-m68k.org>
Cc: ysato <ysato@users.sourceforge.jp>; dalias <dalias@libc.org>; linux-sh <linux-sh@vger.kernel.org>; linux-renesas-soc <linux-renesas-soc@vger.kernel.org>
Sent: Tue, Dec 17, 2019 4:09 am
Subject: Re: can someone solve string_32.h issue for SH ?
Hello,
Give me a day or so and I can do the following things:
1) Write you all a brand new standards-conforming strncpy in SH4 asm like this that is easier to read
2) Compile it with sh4-elf-GCC 9.2
3) Test it on a real SH4 (SH7750/SH7750R-like)
The warning, if it shows up in my test, would likely then be a GCC thing--I have an idea of what it's doing, but I'll be sure after that.
Unfortunately it's 4AM here on the other side of the world right now...
-Karl
-----Original Message-----
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>; Rich Felker <dalias@libc.org>; Linux-SH <linux-sh@vger.kernel.org>; Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Sent: Tue, Dec 17, 2019 3:51 am
Subject: Re: can someone solve string_32.h issue for SH ?
Hi Geert
Cc Yoshinori-san
> > --- a/arch/sh/include/asm/string_32.h
> > +++ b/arch/sh/include/asm/string_32.h
> > @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> > char *__src, size_t __n)
> > __asm__ __volatile__(
> > "1:\n"
> > "mov.b @%1+, %2\n\t"
> > - "mov.b %2, @%0\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"
> > + " cmp/eq %5,%0\n\t"
> > "bf/s 1b\n\t"
> > - " add #1, %0\n"
> > + " nop\n"
> > "2:"
> > : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> > - : "0" (__dest), "1" (__src), "r" (__src+__n)
> > + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> > : "memory", "t");
> >
> > return __xdest;
> >
> > Does this make sense?
> > Can it be improved, by putting something useful in the delay slot?
>
> BTW, there seems to be a serious security issue with this strncpy()
> implementation: while it never writes more than n bytes in the
> destination buffer, it doesn't pad the destination buffer with zeroes if
> the source string is shorter than the buffer size. This will leak
> data.
Yeah...
I can only do is "Reporting issue" to SH ML, unfortunately...
Thank you for your help !!
Best regards
---
Kuninori Morimoto
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 22:16 ` Karl Nasrallah
@ 2019-12-17 23:13 ` Rich Felker
-1 siblings, 0 replies; 37+ messages in thread
From: Rich Felker @ 2019-12-17 23:13 UTC (permalink / raw)
To: Karl Nasrallah
Cc: kuninori.morimoto.gx, geert, ysato, linux-sh, linux-renesas-soc
On Tue, Dec 17, 2019 at 10:16:28PM +0000, Karl Nasrallah wrote:
> Hello!
>
> I have a strncpy for you.
>
> static inline char *strncpy(char *__dest, const char *__src, size_t __n)
> {
> char * retval = __dest;
> const char * __dest_end = __dest + __n - 1;
>
> // size_t is always unsigned
> if(__n = 0)
> {
> return retval;
> }
>
> __asm__ __volatile__ (
> "strncpy_start:\n\t"
> "mov.b @%[src]+,r0\n\t"
> "cmp/eq #0,r0\n\t" // cmp/eq #imm8,r0 is its own instruction
> "bt.s strncpy_pad\n\t" // Done with the string
> "cmp/eq %[dest],%[dest_end]\n\t" // This takes care of the size parameter in only one instruction ;)
> "bt.s strncpy_end\n\t"
> "mov.b r0,@%[dest]\n\t"
> "bra strncpy_start\n\t"
> "add #1,%[dest]\n\t" // mov.b R0,@Rn+ is SH2A only, but we can fill the delay slot with the offset
> "strncpy_pad:\n\t"
> "bt.s strncpy_end\n\t"
> "mov.b r0,@%[dest]\n\t"
> "add #1,%[dest]\n\t"
> "bra strncpy_pad\n\t"
> "cmp/eq %[dest],%[dest_end]\n\t"
> "strncpy_end:\n\t" // All done
> : [src] "+r" (__src), [dest] "+r" (__dest)
> : [dest_end] "r" (__dest_end)
> : "t"
> );
>
> return retval;
> }
>
> Tested with sh4-elf-gcc 9.2.0 on a real SH7750/SH7750R-compatible
> system. No warnings, behaves exactly as per linux (dot) die (dot)
> net/man/3/strncpy and I optimized it with some tricks I devised from
> writing extremely optimized x86. If there are any doubts as to the
> authenticity, note that I am the sole author of this project: github
> (dot) com/KNNSpeed/AVX-Memmove
You're using r0 explicitly in the asm but I don't see where you're
reserving it for your use. You need it either on the clobbers or bound
to a dummy output with earlyclobber.
Rich
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 23:13 ` Rich Felker
0 siblings, 0 replies; 37+ messages in thread
From: Rich Felker @ 2019-12-17 23:13 UTC (permalink / raw)
To: Karl Nasrallah
Cc: kuninori.morimoto.gx, geert, ysato, linux-sh, linux-renesas-soc
On Tue, Dec 17, 2019 at 10:16:28PM +0000, Karl Nasrallah wrote:
> Hello!
>
> I have a strncpy for you.
>
> static inline char *strncpy(char *__dest, const char *__src, size_t __n)
> {
> char * retval = __dest;
> const char * __dest_end = __dest + __n - 1;
>
> // size_t is always unsigned
> if(__n == 0)
> {
> return retval;
> }
>
> __asm__ __volatile__ (
> "strncpy_start:\n\t"
> "mov.b @%[src]+,r0\n\t"
> "cmp/eq #0,r0\n\t" // cmp/eq #imm8,r0 is its own instruction
> "bt.s strncpy_pad\n\t" // Done with the string
> "cmp/eq %[dest],%[dest_end]\n\t" // This takes care of the size parameter in only one instruction ;)
> "bt.s strncpy_end\n\t"
> "mov.b r0,@%[dest]\n\t"
> "bra strncpy_start\n\t"
> "add #1,%[dest]\n\t" // mov.b R0,@Rn+ is SH2A only, but we can fill the delay slot with the offset
> "strncpy_pad:\n\t"
> "bt.s strncpy_end\n\t"
> "mov.b r0,@%[dest]\n\t"
> "add #1,%[dest]\n\t"
> "bra strncpy_pad\n\t"
> "cmp/eq %[dest],%[dest_end]\n\t"
> "strncpy_end:\n\t" // All done
> : [src] "+r" (__src), [dest] "+r" (__dest)
> : [dest_end] "r" (__dest_end)
> : "t"
> );
>
> return retval;
> }
>
> Tested with sh4-elf-gcc 9.2.0 on a real SH7750/SH7750R-compatible
> system. No warnings, behaves exactly as per linux (dot) die (dot)
> net/man/3/strncpy and I optimized it with some tricks I devised from
> writing extremely optimized x86. If there are any doubts as to the
> authenticity, note that I am the sole author of this project: github
> (dot) com/KNNSpeed/AVX-Memmove
You're using r0 explicitly in the asm but I don't see where you're
reserving it for your use. You need it either on the clobbers or bound
to a dummy output with earlyclobber.
Rich
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
2019-12-17 8:29 ` Geert Uytterhoeven
@ 2019-12-17 8:50 ` Geert Uytterhoeven
-1 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 8:50 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
On Tue, Dec 17, 2019 at 9:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Dec 17, 2019 at 7:09 AM Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
> > We get too many below strncpy() warning on SH.
> > Can someone solve it ?
> > I don't remember SH assembler code / can't test it...
>
> I never touched SH assembler code at all.
> But it looks a bit like RISCified m68k, so let's give it a try ;-)
>
> > In file included from /home/morimoto/WORK/linux/arch/sh/include/asm/string.h:3,
> > from /home/morimoto/WORK/linux/include/linux/string.h:20,
> > from /home/morimoto/WORK/linux/include/linux/bitmap.h:9,
> > from /home/morimoto/WORK/linux/include/linux/nodemask.h:95,
> > from /home/morimoto/WORK/linux/include/linux/mmzone.h:17,
> > from /home/morimoto/WORK/linux/include/linux/gfp.h:6,
> > from /home/morimoto/WORK/linux/include/linux/slab.h:15,
> > from /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c:38:
> > /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> > /home/morimoto/WORK/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)
> > ~~~~~^~~~
>
> Yeah, these array warnings are (sometimes) a PITA.
>
> > 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;
> > }
>
>
> My first thought was to just replace "__src+__n" by "__dest+__n", and
> change the "cmp/eq" from "%1" (current src) to "%0" (current dst).
> However, "%0" isn't incremented until the branch delay slot of the loop.
> So I had to move the increment up, and fill the branch delay slot with a nop.
>
> Untested (it-compiles-so-it-must-be-perfect ;-) whitespace-damaged patch:
>
> --- a/arch/sh/include/asm/string_32.h
> +++ b/arch/sh/include/asm/string_32.h
> @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> char *__src, size_t __n)
> __asm__ __volatile__(
> "1:\n"
> "mov.b @%1+, %2\n\t"
> - "mov.b %2, @%0\n\t"
> + "mov.b %2, @%0+\n\t"
While this compiles for SH2, it doesn't for SH4 :-(
> "cmp/eq #0, %2\n\t"
> "bt/s 2f\n\t"
> - " cmp/eq %5,%1\n\t"
> + " cmp/eq %5,%0\n\t"
> "bf/s 1b\n\t"
> - " add #1, %0\n"
> + " nop\n"
> "2:"
> : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> - : "0" (__dest), "1" (__src), "r" (__src+__n)
> + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> : "memory", "t");
>
> return __xdest;
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
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: can someone solve string_32.h issue for SH ?
@ 2019-12-17 8:50 ` Geert Uytterhoeven
0 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 8:50 UTC (permalink / raw)
To: Kuninori Morimoto; +Cc: Yoshinori Sato, Rich Felker, Linux-SH, Linux-Renesas
On Tue, Dec 17, 2019 at 9:29 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Dec 17, 2019 at 7:09 AM Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
> > We get too many below strncpy() warning on SH.
> > Can someone solve it ?
> > I don't remember SH assembler code / can't test it...
>
> I never touched SH assembler code at all.
> But it looks a bit like RISCified m68k, so let's give it a try ;-)
>
> > In file included from /home/morimoto/WORK/linux/arch/sh/include/asm/string.h:3,
> > from /home/morimoto/WORK/linux/include/linux/string.h:20,
> > from /home/morimoto/WORK/linux/include/linux/bitmap.h:9,
> > from /home/morimoto/WORK/linux/include/linux/nodemask.h:95,
> > from /home/morimoto/WORK/linux/include/linux/mmzone.h:17,
> > from /home/morimoto/WORK/linux/include/linux/gfp.h:6,
> > from /home/morimoto/WORK/linux/include/linux/slab.h:15,
> > from /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c:38:
> > /home/morimoto/WORK/linux/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> > /home/morimoto/WORK/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)
> > ~~~~~^~~~
>
> Yeah, these array warnings are (sometimes) a PITA.
>
> > 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;
> > }
>
>
> My first thought was to just replace "__src+__n" by "__dest+__n", and
> change the "cmp/eq" from "%1" (current src) to "%0" (current dst).
> However, "%0" isn't incremented until the branch delay slot of the loop.
> So I had to move the increment up, and fill the branch delay slot with a nop.
>
> Untested (it-compiles-so-it-must-be-perfect ;-) whitespace-damaged patch:
>
> --- a/arch/sh/include/asm/string_32.h
> +++ b/arch/sh/include/asm/string_32.h
> @@ -40,15 +40,15 @@ static inline char *strncpy(char *__dest, const
> char *__src, size_t __n)
> __asm__ __volatile__(
> "1:\n"
> "mov.b @%1+, %2\n\t"
> - "mov.b %2, @%0\n\t"
> + "mov.b %2, @%0+\n\t"
While this compiles for SH2, it doesn't for SH4 :-(
> "cmp/eq #0, %2\n\t"
> "bt/s 2f\n\t"
> - " cmp/eq %5,%1\n\t"
> + " cmp/eq %5,%0\n\t"
> "bf/s 1b\n\t"
> - " add #1, %0\n"
> + " nop\n"
> "2:"
> : "=r" (__dest), "=r" (__src), "=&z" (__dummy)
> - : "0" (__dest), "1" (__src), "r" (__src+__n)
> + : "0" (__dest), "1" (__src), "r" (__dest+__n)
> : "memory", "t");
>
> return __xdest;
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
^ permalink raw reply [flat|nested] 37+ messages in thread