Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190724184512.162887-1-nums@google.com> <20190724184512.162887-4-nums@google.com>
 <c256d9b6702b4d37a0c21b93f319b476@AcuMS.aculab.com>
In-Reply-To: <c256d9b6702b4d37a0c21b93f319b476@AcuMS.aculab.com>
From:   Ian Rogers <irogers@google.com>
Date:   Thu, 25 Jul 2019 14:18:25 -0700
Message-ID: <CAP-5=fW4v4NDe-aGS+RNLktTtUK9mHVRKn46XPw=S8Akh6hoyA@mail.gmail.com>
Subject: Re: [PATCH 3/3] Fix insn.c misaligned address error
To:     David Laight <David.Laight@aculab.com>
Cc:     Numfor Mbiziwo-Tiapo <nums@google.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "acme@kernel.org" <acme@kernel.org>,
        "alexander.shishkin@linux.intel.com" 
        <alexander.shishkin@linux.intel.com>,
        "jolsa@redhat.com" <jolsa@redhat.com>,
        "namhyung@kernel.org" <namhyung@kernel.org>,
        "songliubraving@fb.com" <songliubraving@fb.com>,
        "mbd@fb.com" <mbd@fb.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "eranian@google.com" <eranian@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jul 25, 2019 at 6:06 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Numfor Mbiziwo-Tiapo
> > Sent: 24 July 2019 19:45
> >
> > 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.
> ...
> >  #define __get_next(t, insn)  \
> > -     ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; })
> > +     ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); \
> > +             insn->next_byte += sizeof(t); r; })
>
> Isn't there a get_unaligned_u32() (or similar) that can be used?


memcpy is a compiler intrinsic. get_unaligned_u32 would mean either a
'if (sizeof(t) == sizeof(u32)) get_unaligned_u32(.. ' for all sizes or
changing all call sites of __get_next. Numfor's change feels right as
it is the least invasive.

Thanks,
Ian Rogers
(resent to make plain text)

>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)