Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Mon, 4 Mar 2019 14:05:01 +0100 | Subject | Re: [m68k:master 1174/1174] arch/m68k/include/asm/string.h:72:25: warning: '__builtin_memcpy' forming offset 8 is out of the bounds [0, 7] |
| |
On Mon, Mar 4, 2019 at 1:44 PM kbuild test robot <lkp@intel.com> wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/geert/linux-m68k.git master > head: e223cadc191661c67cb419b3a53c7854ecc39e8b > commit: e223cadc191661c67cb419b3a53c7854ecc39e8b [1174/1174] Merge tag 'v5.0' > config: m68k-allmodconfig (attached as .config) > compiler: m68k-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > git checkout e223cadc191661c67cb419b3a53c7854ecc39e8b > # save the attached .config to linux build tree > GCC_VERSION=8.2.0 make.cross ARCH=m68k > > All warnings (new ones prefixed by >>): > > In file included from include/linux/string.h:20, > from include/linux/bitmap.h:9, > from include/linux/nodemask.h:95, > from include/linux/mmzone.h:17, > from include/linux/gfp.h:6, > from include/linux/umh.h:4, > from include/linux/kmod.h:22, > from include/linux/module.h:13, > from drivers/nvme/target/admin-cmd.c:15: > In function 'memcpy_and_pad', > inlined from 'nvmet_execute_identify_ctrl' at drivers/nvme/target/admin-cmd.c:309:2: > >> arch/m68k/include/asm/string.h:72:25: warning: '__builtin_memcpy' forming offset 8 is out of the bounds [0, 7] [-Warray-bounds] > #define memcpy(d, s, n) __builtin_memcpy(d, s, n) > ^~~~~~~~~~~~~~~~~~~~~~~~~ > include/linux/string.h:456:3: note: in expansion of macro 'memcpy' > memcpy(dest, src, dest_len); > ^~~~~~ > > vim +/__builtin_memcpy +72 arch/m68k/include/asm/string.h > > ea61bc46 Greg Ungerer 2010-09-07 69 > ea61bc46 Greg Ungerer 2010-09-07 70 #define __HAVE_ARCH_MEMCPY > ea61bc46 Greg Ungerer 2010-09-07 71 extern void *memcpy(void *, const void *, __kernel_size_t); > ea61bc46 Greg Ungerer 2010-09-07 @72 #define memcpy(d, s, n) __builtin_memcpy(d, s, n) > ea61bc46 Greg Ungerer 2010-09-07 73 > > :::::: The code at line 72 was first introduced by commit > :::::: ea61bc461d09e8d331a307916530aaae808c72a2 m68k/m68knommu: merge MMU and non-MMU string.h > > :::::: TO: Greg Ungerer <gerg@snapgear.com> > :::::: CC: Geert Uytterhoeven <geert@linux-m68k.org>
Yep, that's a funny one, which I saw myself this morning, and decided to dive into. Apparently this is due 1) the recently added -ffreestanding and 2) the kernel version being shorter than 8 characters (indeed, it didn't happen with allmodconfig on v5.0.0-rcX).
The offending code is:
memcpy_and_pad(id->fr, sizeof(id->fr), UTS_RELEASE, strlen(UTS_RELEASE), ' ');
with UTS_RELEASE being "5.0.0+", which calls into:
static inline void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, int pad) { if (dest_len > count) {
Of course this branch is taken, right?
memcpy(dest, src, count); memset(dest + count, pad, dest_len - count); } else memcpy(dest, src, dest_len); }
This assembles to:
.LC1: .string "5.0.0+"
| drivers/nvme/target/admin-cmd.c:311: memcpy_and_pad(id->fr, sizeof(id->fr), pea .LC1 | jsr strlen |
Woops, gcc no longer optimizes this away, due to -ffreestanding.
| drivers/nvme/target/admin-cmd.c:311: memcpy_and_pad(id->fr, sizeof(id->fr), lea (64,%a3),%a1 |, ret, _7 | include/linux/string.h:452: if (dest_len > count) { lea (16,%sp),%sp |, moveq #7,%d1 |, cmp.l %d0,%d1 | _6, jcs .L53 |
And we end up with retaining both branches:
| include/linux/string.h:453: memcpy(dest, src, count); move.l %d0,-(%sp) | _6, pea .LC1 | move.l %a1,-(%sp) | _7, move.l %d0,-12(%fp) |, move.l %a1,-16(%fp) |, jsr memcpy | | include/linux/string.h:454: memset(dest + count, pad, dest_len - count); move.l -12(%fp),%d0 |, moveq #8,%d1 |, sub.l %d0,%d1 | _6, move.l %d1,-(%sp) |, pea 32.w | move.l -16(%fp),%a1 |, pea (%a1,%d0.l) | jsr memset | lea (24,%sp),%sp |, jra .L54 | .L53: | include/linux/string.h:456: memcpy(dest, src, dest_len); move.l .LC1,(%a1) | MEM[(void *)"5.0.0+"], MEM[(void *)_7] move.l .LC1+4,4(%a1) | MEM[(void *)"5.0.0+"], MEM[(void *)_7]
But given the warning, the compiler must have devised that taking the second branch would read beyond the source buffer???
.L54:
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
| |