Re: [PATCH v1 1/2] perf record: Add 8-byte aligned event type PERF_RECORD_COMPRESSED2
From: Namhyung Kim
Date: Thu Feb 27 2025 - 03:21:19 EST
On Wed, Feb 26, 2025 at 10:20:36PM -0800, Ian Rogers wrote:
> On Wed, Feb 26, 2025 at 10:04 PM Andi Kleen <ak@xxxxxxxxxxxxxxx> wrote:
> >
> > On Wed, Feb 26, 2025 at 09:34:06PM -0800, Chun-Tse Shao wrote:
> > > The original PERF_RECORD_COMPRESS is not 8-byte aligned, which can cause
> > > asan runtime error:
> >
> > It seems pointless. Most architectures have cheap unaligned accesses
> > these days.
> >
> > Just disable that error?
>
> The perf_event_header in perf_event.h is:
> ```
> struct perf_event_header {
> __u32 type;
> __u16 misc;
> __u16 size;
> };
> ```
> so it is assuming at least 4-byte alignment. 8-byte alignment is
> assumed in many places in tools/lib/perf/include/perf/event.h. We pad
> events to ensure the alignment in about 30 places already:
> ```
> $ grep -r PERF_ALIGN tools/perf|grep u64|wc -l
> 32
> ```
I vaguely remember that it needs 8 bytes alignment to deal with partial
mmap-ed data on 32-bit machines so that it can make sure the header is
not across the mmap boundary.
Thanks,
Namhyung
> Having sanitizers I think is a must, if we allow unaligned events we'd
> need to introduce helper functions or memcpys to workaround the
> unaligned undefined behavior. I think the padding is a less worse
> alternative and one that was already picked.
>
> Thanks,
> Ian