* Re: [PATCH] perf bpf: 8 byte align bpil data
2022-06-14 1:47 [PATCH] perf bpf: 8 byte align bpil data Ian Rogers
@ 2022-06-27 17:17 ` Ian Rogers
2022-06-27 20:56 ` Andrii Nakryiko
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Ian Rogers @ 2022-06-27 17:17 UTC (permalink / raw)
To: Dave Marchevsky
Cc: Stephane Eranian, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, bpf, netdev, linux-kernel,
linux-perf-users, Song Liu, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Yonghong Song, John Fastabend,
KP Singh, Quentin Monnet
On Mon, Jun 13, 2022 at 6:47 PM Ian Rogers <irogers@google.com> wrote:
>
> bpil data is accessed assuming 64-bit alignment resulting in undefined
> behavior as the data is just byte aligned. With an -fsanitize=undefined
> build the following errors are observed:
>
> $ sudo perf record -a sleep 1
> util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> 0x55f61084520f: note: pointer points here
> a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> ^
> util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> 0x55f61084522f: note: pointer points here
> ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> ^
> util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> 0x55f61084523f: note: pointer points here
> 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
>
> Correct this by rouding up the data sizes and aligning the pointers.
Happy Monday, polite ping!
Thanks,
Ian
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/bpf-utils.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
> index e271e05e51bc..80b1d2b3729b 100644
> --- a/tools/perf/util/bpf-utils.c
> +++ b/tools/perf/util/bpf-utils.c
> @@ -149,11 +149,10 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> count = bpf_prog_info_read_offset_u32(&info, desc->count_offset);
> size = bpf_prog_info_read_offset_u32(&info, desc->size_offset);
>
> - data_len += count * size;
> + data_len += roundup(count * size, sizeof(__u64));
> }
>
> /* step 3: allocate continuous memory */
> - data_len = roundup(data_len, sizeof(__u64));
> info_linear = malloc(sizeof(struct perf_bpil) + data_len);
> if (!info_linear)
> return ERR_PTR(-ENOMEM);
> @@ -180,7 +179,7 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> bpf_prog_info_set_offset_u64(&info_linear->info,
> desc->array_offset,
> ptr_to_u64(ptr));
> - ptr += count * size;
> + ptr += roundup(count * size, sizeof(__u64));
> }
>
> /* step 5: call syscall again to get required arrays */
> --
> 2.36.1.476.g0c4daa206d-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf bpf: 8 byte align bpil data
2022-06-14 1:47 [PATCH] perf bpf: 8 byte align bpil data Ian Rogers
2022-06-27 17:17 ` Ian Rogers
@ 2022-06-27 20:56 ` Andrii Nakryiko
2022-06-28 8:14 ` Jiri Olsa
2022-06-28 8:17 ` olsajiri
3 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2022-06-27 20:56 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Dave Marchevsky, Quentin Monnet, linux-perf-use.,
open list, Networking, bpf, Stephane Eranian
On Mon, Jun 13, 2022 at 6:47 PM Ian Rogers <irogers@google.com> wrote:
>
> bpil data is accessed assuming 64-bit alignment resulting in undefined
> behavior as the data is just byte aligned. With an -fsanitize=undefined
> build the following errors are observed:
>
> $ sudo perf record -a sleep 1
> util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> 0x55f61084520f: note: pointer points here
> a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> ^
> util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> 0x55f61084522f: note: pointer points here
> ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> ^
> util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> 0x55f61084523f: note: pointer points here
> 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
>
> Correct this by rouding up the data sizes and aligning the pointers.
typo: rounding
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
Makes sense.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> tools/perf/util/bpf-utils.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
> index e271e05e51bc..80b1d2b3729b 100644
> --- a/tools/perf/util/bpf-utils.c
> +++ b/tools/perf/util/bpf-utils.c
> @@ -149,11 +149,10 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> count = bpf_prog_info_read_offset_u32(&info, desc->count_offset);
> size = bpf_prog_info_read_offset_u32(&info, desc->size_offset);
>
> - data_len += count * size;
> + data_len += roundup(count * size, sizeof(__u64));
> }
>
> /* step 3: allocate continuous memory */
> - data_len = roundup(data_len, sizeof(__u64));
> info_linear = malloc(sizeof(struct perf_bpil) + data_len);
> if (!info_linear)
> return ERR_PTR(-ENOMEM);
> @@ -180,7 +179,7 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> bpf_prog_info_set_offset_u64(&info_linear->info,
> desc->array_offset,
> ptr_to_u64(ptr));
> - ptr += count * size;
> + ptr += roundup(count * size, sizeof(__u64));
> }
>
> /* step 5: call syscall again to get required arrays */
> --
> 2.36.1.476.g0c4daa206d-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf bpf: 8 byte align bpil data
2022-06-14 1:47 [PATCH] perf bpf: 8 byte align bpil data Ian Rogers
2022-06-27 17:17 ` Ian Rogers
2022-06-27 20:56 ` Andrii Nakryiko
@ 2022-06-28 8:14 ` Jiri Olsa
2022-06-28 15:11 ` Arnaldo Carvalho de Melo
2022-06-28 8:17 ` olsajiri
3 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2022-06-28 8:14 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Dave Marchevsky, Quentin Monnet, linux-perf-users,
linux-kernel, netdev, bpf, Stephane Eranian
On Mon, Jun 13, 2022 at 06:47:14PM -0700, Ian Rogers wrote:
> bpil data is accessed assuming 64-bit alignment resulting in undefined
> behavior as the data is just byte aligned. With an -fsanitize=undefined
> build the following errors are observed:
>
> $ sudo perf record -a sleep 1
> util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> 0x55f61084520f: note: pointer points here
> a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> ^
> util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> 0x55f61084522f: note: pointer points here
> ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> ^
> util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> 0x55f61084523f: note: pointer points here
> 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
>
> Correct this by rouding up the data sizes and aligning the pointers.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/bpf-utils.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
> index e271e05e51bc..80b1d2b3729b 100644
> --- a/tools/perf/util/bpf-utils.c
> +++ b/tools/perf/util/bpf-utils.c
> @@ -149,11 +149,10 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> count = bpf_prog_info_read_offset_u32(&info, desc->count_offset);
> size = bpf_prog_info_read_offset_u32(&info, desc->size_offset);
>
> - data_len += count * size;
> + data_len += roundup(count * size, sizeof(__u64));
> }
>
> /* step 3: allocate continuous memory */
> - data_len = roundup(data_len, sizeof(__u64));
> info_linear = malloc(sizeof(struct perf_bpil) + data_len);
> if (!info_linear)
> return ERR_PTR(-ENOMEM);
> @@ -180,7 +179,7 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> bpf_prog_info_set_offset_u64(&info_linear->info,
> desc->array_offset,
> ptr_to_u64(ptr));
> - ptr += count * size;
> + ptr += roundup(count * size, sizeof(__u64));
this one depends on info_linear->data being alligned(8), right?
should we make sure it's allways the case like in the patch
below, or it's superfluous?
thanks,
jirka
---
diff --git a/tools/perf/util/bpf-utils.h b/tools/perf/util/bpf-utils.h
index 86a5055cdfad..1aba76c44116 100644
--- a/tools/perf/util/bpf-utils.h
+++ b/tools/perf/util/bpf-utils.h
@@ -60,7 +60,7 @@ struct perf_bpil {
/* which arrays are included in data */
__u64 arrays;
struct bpf_prog_info info;
- __u8 data[];
+ __u8 data[] __attribute__((aligned(8)));
};
struct perf_bpil *
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] perf bpf: 8 byte align bpil data
2022-06-28 8:14 ` Jiri Olsa
@ 2022-06-28 15:11 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-06-28 15:11 UTC (permalink / raw)
To: Jiri Olsa
Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Namhyung Kim, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Dave Marchevsky,
Quentin Monnet, linux-perf-users, linux-kernel, netdev, bpf,
Stephane Eranian
Em Tue, Jun 28, 2022 at 10:14:52AM +0200, Jiri Olsa escreveu:
> On Mon, Jun 13, 2022 at 06:47:14PM -0700, Ian Rogers wrote:
> > bpil data is accessed assuming 64-bit alignment resulting in undefined
> > behavior as the data is just byte aligned. With an -fsanitize=undefined
> > build the following errors are observed:
> >
> > $ sudo perf record -a sleep 1
> > util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> > 0x55f61084520f: note: pointer points here
> > a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> > ^
> > util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> > 0x55f61084522f: note: pointer points here
> > ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> > ^
> > util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> > 0x55f61084523f: note: pointer points here
> > 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
> >
> > Correct this by rouding up the data sizes and aligning the pointers.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/util/bpf-utils.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
> > index e271e05e51bc..80b1d2b3729b 100644
> > --- a/tools/perf/util/bpf-utils.c
> > +++ b/tools/perf/util/bpf-utils.c
> > @@ -149,11 +149,10 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> > count = bpf_prog_info_read_offset_u32(&info, desc->count_offset);
> > size = bpf_prog_info_read_offset_u32(&info, desc->size_offset);
> >
> > - data_len += count * size;
> > + data_len += roundup(count * size, sizeof(__u64));
> > }
> >
> > /* step 3: allocate continuous memory */
> > - data_len = roundup(data_len, sizeof(__u64));
> > info_linear = malloc(sizeof(struct perf_bpil) + data_len);
> > if (!info_linear)
> > return ERR_PTR(-ENOMEM);
> > @@ -180,7 +179,7 @@ get_bpf_prog_info_linear(int fd, __u64 arrays)
> > bpf_prog_info_set_offset_u64(&info_linear->info,
> > desc->array_offset,
> > ptr_to_u64(ptr));
> > - ptr += count * size;
> > + ptr += roundup(count * size, sizeof(__u64));
>
> this one depends on info_linear->data being alligned(8), right?
>
> should we make sure it's allways the case like in the patch
> below, or it's superfluous?
>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/bpf-utils.h b/tools/perf/util/bpf-utils.h
> index 86a5055cdfad..1aba76c44116 100644
> --- a/tools/perf/util/bpf-utils.h
> +++ b/tools/perf/util/bpf-utils.h
> @@ -60,7 +60,7 @@ struct perf_bpil {
> /* which arrays are included in data */
> __u64 arrays;
> struct bpf_prog_info info;
> - __u8 data[];
> + __u8 data[] __attribute__((aligned(8)));
> };
>
> struct perf_bpil *
⬢[acme@toolbox perf-urgent]$ pahole -C perf_bpil ~/bin/perf
struct perf_bpil {
__u32 info_len; /* 0 4 */
__u32 data_len; /* 4 4 */
__u64 arrays; /* 8 8 */
struct bpf_prog_info info __attribute__((__aligned__(8))); /* 16 224 */
/* XXX last struct has 4 bytes of padding */
/* --- cacheline 3 boundary (192 bytes) was 48 bytes ago --- */
__u8 data[]; /* 240 0 */
/* size: 240, cachelines: 4, members: 5 */
/* paddings: 1, sum paddings: 4 */
/* forced alignments: 1 */
/* last cacheline: 48 bytes */
} __attribute__((__aligned__(8)));
⬢[acme@toolbox perf-urgent]$
Humm, lotsa explicit alignments already?
Looking at the sources:
struct perf_bpil {
/* size of struct bpf_prog_info, when the tool is compiled */
__u32 info_len;
/* total bytes allocated for data, round up to 8 bytes */
__u32 data_len;
/* which arrays are included in data */
__u64 arrays;
struct bpf_prog_info info;
__u8 data[];
};
Interesting, where is pahole finding those aligned attributes? Ok
'struct bpf_prog_info' in tools/include/uapi/linux/bpf.h has aligned(8)
for the whole struct, so perf_bpil's info gets that.
sp that data right after 'info' is 8 byte alignedas
sizeof(bpf_prog_info) is a multiple of 8 bytes.
So I think I can apply the patch as-is and leave making sure data is
8-byte aligned for later.
Doing that now.
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf bpf: 8 byte align bpil data
2022-06-14 1:47 [PATCH] perf bpf: 8 byte align bpil data Ian Rogers
` (2 preceding siblings ...)
2022-06-28 8:14 ` Jiri Olsa
@ 2022-06-28 8:17 ` olsajiri
2022-06-28 15:15 ` Ian Rogers
3 siblings, 1 reply; 8+ messages in thread
From: olsajiri @ 2022-06-28 8:17 UTC (permalink / raw)
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Dave Marchevsky, Quentin Monnet, linux-perf-users,
linux-kernel, netdev, bpf, Stephane Eranian
On Mon, Jun 13, 2022 at 06:47:14PM -0700, Ian Rogers wrote:
> bpil data is accessed assuming 64-bit alignment resulting in undefined
> behavior as the data is just byte aligned. With an -fsanitize=undefined
> build the following errors are observed:
I need to add -w to get the clean build with that, do you see that as well?
$ make EXTRA_CFLAGS='-fsanitize=undefined -w'
>
> $ sudo perf record -a sleep 1
> util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> 0x55f61084520f: note: pointer points here
> a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> ^
> util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> 0x55f61084522f: note: pointer points here
> ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> ^
> util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> 0x55f61084523f: note: pointer points here
> 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
and I'm also getting another error in:
[root@krava perf]# ./perf record -a sleep 1
util/synthetic-events.c:1202:11: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
0x00000286f7ea: note: pointer points here
20 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
util/synthetic-events.c:1203:18: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
0x00000286f7ea: note: pointer points here
20 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
util/synthetic-events.c:1206:46: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
0x00000286f7ea: note: pointer points here
20 00 01 00 01 00 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
^
/home/jolsa/kernel/linux-perf/tools/include/asm-generic/bitops/atomic.h:10:29: runtime error: load of misaligned address 0x00000286f7f2 for type 'long unsigned int', which requires 8 byte alignment
0x00000286f7f2: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 51 00 00 00 00 00
^
are you going to address this one as well?
the reason for this one is that 'data' in struct perf_record_cpu_map_data
is not alligned(8), so that's why I raised the question in my other reply ;-)
I wonder we should mark all tools/lib/perf/include/perf/event.h types
as packed to prevent any compiler padding
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf bpf: 8 byte align bpil data
2022-06-28 8:17 ` olsajiri
@ 2022-06-28 15:15 ` Ian Rogers
2022-06-28 15:35 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Ian Rogers @ 2022-06-28 15:15 UTC (permalink / raw)
To: olsajiri
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Dave Marchevsky, Quentin Monnet, linux-perf-users,
linux-kernel, netdev, bpf, Stephane Eranian
On Tue, Jun 28, 2022 at 1:41 AM <olsajiri@gmail.com> wrote:
>
> On Mon, Jun 13, 2022 at 06:47:14PM -0700, Ian Rogers wrote:
> > bpil data is accessed assuming 64-bit alignment resulting in undefined
> > behavior as the data is just byte aligned. With an -fsanitize=undefined
> > build the following errors are observed:
>
> I need to add -w to get the clean build with that, do you see that as well?
>
> $ make EXTRA_CFLAGS='-fsanitize=undefined -w'
I don't recall needing this, but I was stacking fixes which may explain it.
> >
> > $ sudo perf record -a sleep 1
> > util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> > 0x55f61084520f: note: pointer points here
> > a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> > ^
> > util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> > 0x55f61084522f: note: pointer points here
> > ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> > ^
> > util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> > 0x55f61084523f: note: pointer points here
> > 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
>
>
> and I'm also getting another error in:
>
> [root@krava perf]# ./perf record -a sleep 1
> util/synthetic-events.c:1202:11: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> 0x00000286f7ea: note: pointer points here
> 20 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> util/synthetic-events.c:1203:18: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> 0x00000286f7ea: note: pointer points here
> 20 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> util/synthetic-events.c:1206:46: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> 0x00000286f7ea: note: pointer points here
> 20 00 01 00 01 00 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> /home/jolsa/kernel/linux-perf/tools/include/asm-generic/bitops/atomic.h:10:29: runtime error: load of misaligned address 0x00000286f7f2 for type 'long unsigned int', which requires 8 byte alignment
> 0x00000286f7f2: note: pointer points here
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 51 00 00 00 00 00
> ^
>
> are you going to address this one as well?
>
>
> the reason for this one is that 'data' in struct perf_record_cpu_map_data
> is not alligned(8), so that's why I raised the question in my other reply ;-)
>
> I wonder we should mark all tools/lib/perf/include/perf/event.h types
> as packed to prevent any compiler padding
I already sent out a fix and some improvements related to this:
https://lore.kernel.org/lkml/20220614143353.1559597-1-irogers@google.com/
Could you take a look?
I'm not sure about aligned and packed. I tried to minimize it in the
change above. The issue is that taking the address of a variable in a
packed struct results in an unaligned pointer. To address this in the
fix above I changed the functions to pass pointers to the whole
struct.
Thanks,
Ian
> thanks,
> jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf bpf: 8 byte align bpil data
2022-06-28 15:15 ` Ian Rogers
@ 2022-06-28 15:35 ` Jiri Olsa
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2022-06-28 15:35 UTC (permalink / raw)
To: Ian Rogers
Cc: olsajiri, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Dave Marchevsky, Quentin Monnet, linux-perf-users,
linux-kernel, netdev, bpf, Stephane Eranian
On Tue, Jun 28, 2022 at 08:15:04AM -0700, Ian Rogers wrote:
> On Tue, Jun 28, 2022 at 1:41 AM <olsajiri@gmail.com> wrote:
> >
> > On Mon, Jun 13, 2022 at 06:47:14PM -0700, Ian Rogers wrote:
> > > bpil data is accessed assuming 64-bit alignment resulting in undefined
> > > behavior as the data is just byte aligned. With an -fsanitize=undefined
> > > build the following errors are observed:
> >
> > I need to add -w to get the clean build with that, do you see that as well?
> >
> > $ make EXTRA_CFLAGS='-fsanitize=undefined -w'
>
> I don't recall needing this, but I was stacking fixes which may explain it.
>
> > >
> > > $ sudo perf record -a sleep 1
> > > util/bpf-event.c:310:22: runtime error: load of misaligned address 0x55f61084520f for type '__u64', which requires 8 byte alignment
> > > 0x55f61084520f: note: pointer points here
> > > a8 fe ff ff 3c 51 d3 c0 ff ff ff ff 04 84 d3 c0 ff ff ff ff d8 aa d3 c0 ff ff ff ff a4 c0 d3 c0
> > > ^
> > > util/bpf-event.c:311:20: runtime error: load of misaligned address 0x55f61084522f for type '__u32', which requires 4 byte alignment
> > > 0x55f61084522f: note: pointer points here
> > > ff ff ff ff c7 17 00 00 f1 02 00 00 1f 04 00 00 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00
> > > ^
> > > util/bpf-event.c:198:33: runtime error: member access within misaligned address 0x55f61084523f for type 'const struct bpf_func_info', which requires 4 byte alignment
> > > 0x55f61084523f: note: pointer points here
> > > 58 04 00 00 00 00 00 00 0f 00 00 00 63 02 00 00 3b 00 00 00 ab 02 00 00 44 00 00 00 14 03 00 00
> >
> >
> > and I'm also getting another error in:
> >
> > [root@krava perf]# ./perf record -a sleep 1
> > util/synthetic-events.c:1202:11: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> > 0x00000286f7ea: note: pointer points here
> > 20 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ^
> > util/synthetic-events.c:1203:18: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> > 0x00000286f7ea: note: pointer points here
> > 20 00 01 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ^
> > util/synthetic-events.c:1206:46: runtime error: member access within misaligned address 0x00000286f7ea for type 'struct perf_record_record_cpu_map', which requires 8 byte alignment
> > 0x00000286f7ea: note: pointer points here
> > 20 00 01 00 01 00 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ^
> > /home/jolsa/kernel/linux-perf/tools/include/asm-generic/bitops/atomic.h:10:29: runtime error: load of misaligned address 0x00000286f7f2 for type 'long unsigned int', which requires 8 byte alignment
> > 0x00000286f7f2: note: pointer points here
> > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 51 00 00 00 00 00
> > ^
> >
> > are you going to address this one as well?
> >
> >
> > the reason for this one is that 'data' in struct perf_record_cpu_map_data
> > is not alligned(8), so that's why I raised the question in my other reply ;-)
> >
> > I wonder we should mark all tools/lib/perf/include/perf/event.h types
> > as packed to prevent any compiler padding
>
> I already sent out a fix and some improvements related to this:
> https://lore.kernel.org/lkml/20220614143353.1559597-1-irogers@google.com/
> Could you take a look?
ok, I overlooked that one
>
> I'm not sure about aligned and packed. I tried to minimize it in the
> change above. The issue is that taking the address of a variable in a
> packed struct results in an unaligned pointer. To address this in the
> fix above I changed the functions to pass pointers to the whole
> struct.
ok, will check,
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread