On Wed, Feb 9, 2022 at 4:37 PM Delyan Kratunov <delyank@xxxxxx> wrote: > > As reported in [0] and [1], kernel and userspace can sometimes disagree > on the definition of a typedef (in particular, the size). > This leads to trouble when userspace maps the memory of a bpf program > and reads/writes to it assuming a different memory layout. > > This commit now uses the libbpf sized ints logic when emitting the > skeleton. This resolves int types to int32_t-like equivalents and > ensures that typedefs are not just emitted verbatim. > > The drive-by selftest changes fix format specifier issues > due to the definitions of [us]64 and (u)int64_t differing in how > many longs they use (long long int vs long int on x86_64). > > [0]: https://github.com/iovisor/bcc/pull/3777 > [1]: Closes: https://github.com/libbpf/libbpf/issues/433 > > Signed-off-by: Delyan Kratunov <delyank@xxxxxx> > --- > tools/bpf/bpftool/gen.c | 3 +++ > .../testing/selftests/bpf/prog_tests/skeleton.c | 16 ++++++++-------- > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c > index eacfc6a2060d..18c3f755ad88 100644 > --- a/tools/bpf/bpftool/gen.c > +++ b/tools/bpf/bpftool/gen.c > @@ -146,6 +146,7 @@ static int codegen_datasec_def(struct bpf_object *obj, > .field_name = var_ident, > .indent_level = 2, > .strip_mods = strip_mods, > + .sizedints = true, > ); > int need_off = sec_var->offset, align_off, align; > __u32 var_type_id = var->type; > @@ -751,6 +752,7 @@ static int do_skeleton(int argc, char **argv) > #ifndef %2$s \n\ > #define %2$s \n\ > \n\ > + #include <inttypes.h> \n\ if Alexei's patch set will go in first (very likely), you'll need to rebase and make sure that you don't include either inttypes.h or stdint.h for kernel mode, as those headers don't exist there > #include <stdlib.h> \n\ > #include <bpf/bpf.h> \n\ > #include <bpf/skel_internal.h> \n\ > @@ -770,6 +772,7 @@ static int do_skeleton(int argc, char **argv) > #define %2$s \n\ > \n\ > #include <errno.h> \n\ > + #include <inttypes.h> \n\ seems like inttypes.h just includes stdint.h, I'd just include stdint.h directly > #include <stdlib.h> \n\ > #include <bpf/libbpf.h> \n\ > \n\ > diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c > index 180afd632f4c..9894e1b39211 100644 > --- a/tools/testing/selftests/bpf/prog_tests/skeleton.c > +++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c > @@ -43,13 +43,13 @@ void test_skeleton(void) > /* validate values are pre-initialized correctly */ > CHECK(data->in1 != -1, "in1", "got %d != exp %d\n", data->in1, -1); > CHECK(data->out1 != -1, "out1", "got %d != exp %d\n", data->out1, -1); > - CHECK(data->in2 != -1, "in2", "got %lld != exp %lld\n", data->in2, -1LL); > - CHECK(data->out2 != -1, "out2", "got %lld != exp %lld\n", data->out2, -1LL); > + CHECK(data->in2 != -1, "in2", "got %"PRId64" != exp %lld\n", data->in2, -1LL); > + CHECK(data->out2 != -1, "out2", "got %"PRId64" != exp %lld\n", data->out2, -1LL); we don't use PRIxxx ugliness anywhere in selftests or libbpf code base, it would be easier to just convert this to ASSERT_EQ() > > CHECK(bss->in3 != 0, "in3", "got %d != exp %d\n", bss->in3, 0); > CHECK(bss->out3 != 0, "out3", "got %d != exp %d\n", bss->out3, 0); > - CHECK(bss->in4 != 0, "in4", "got %lld != exp %lld\n", bss->in4, 0LL); > - CHECK(bss->out4 != 0, "out4", "got %lld != exp %lld\n", bss->out4, 0LL); > + CHECK(bss->in4 != 0, "in4", "got %"PRId64" != exp %lld\n", bss->in4, 0LL); > + CHECK(bss->out4 != 0, "out4", "got %"PRId64" != exp %lld\n", bss->out4, 0LL); [...]
