Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH 3/3] Fix insn.c misaligned address error | Date | Tue, 30 Jul 2019 09:17:21 +0000 |
| |
From: Adrian Hunter > Sent: 30 July 2019 08:53 > On 30/07/19 3:47 AM, Masami Hiramatsu wrote: > > On Mon, 29 Jul 2019 11:22:34 +0300 > > Adrian Hunter <adrian.hunter@intel.com> wrote: > > > >> On 27/07/19 12:46 PM, Masami Hiramatsu wrote: > >>> On Fri, 26 Jul 2019 16:38:06 -0300 > >>> Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > >>> > >>>> Em Wed, Jul 24, 2019 at 11:45:12AM -0700, Numfor Mbiziwo-Tiapo escreveu: > >>>>> The ubsan (undefined behavior sanitizer) version of perf throws an > >>>>> error on the 'x86 instruction decoder - new instructions' function > >>>>> of perf test. > >>>>> > >>>>> To reproduce this run: > >>>>> make -C tools/perf USE_CLANG=1 EXTRA_CFLAGS="-fsanitize=undefined" > >>>>> > >>>>> then run: tools/perf/perf test 62 -v > >>>>> > >>>>> The error occurs in the __get_next macro (line 34) where an int is > >>>>> read from a potentially unaligned address. Using memcpy instead of > >>>>> assignment from an unaligned pointer. > >>>> > >>>> Since this came from the kernel, don't we have to fix it there as well? > >>>> Masami, Adrian? > >>> > >>> I guess we don't need it, since x86 can access "unaligned address" and > >>> x86 insn decoder in kernel runs only on x86. I'm not sure about perf's > >>> that part. Maybe if we run it on other arch as cross-arch application, > >>> it may cause unaligned pointer issue. > >> > >> Yes, theoretically Intel PT decoding can be done on any arch. > >> > >> But the memcpy is probably sub-optimal for x86, so the patch as it stands > >> does not seem suitable. I notice the kernel has get_unaligned() and > >> put_unaligned(). > >> > >> Obviously it would be better for a patch to be accepted to > >> arch/x86/lib/insn.c also. > > > > Hmm, then I rather like memcpy() for arch/x86/lib/insn.c, because it runs only > > on x86. > > Yes, I was wrong about memcpy, and it is simpler for perf tools than > dragging out get_unaligned().
It may well make the generated code worse because some optimisations won't happen because they would need to be done before memcpy() gets inlined. I've certainly seen cases where a #define generates significantly better code than an inline function.
David
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
| |