selftests: hid: trouble building with clang due to missing header

selftests: hid: trouble building with clang due to missing header

[Justin Stitt]

Hi, I'd like to get some help with building the kselftest target.

I am running into some warnings within the hid tree:
| progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct
hid_bpf_ctx' will \
|       not be visible outside of this function [-Werror,-Wvisibility]
|     9 | extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
|       |                                      ^
| progs/hid.c:23:35: error: incompatible pointer types passing 'struct
hid_bpf_ctx *' \
|       to parameter of type 'struct hid_bpf_ctx *'
[-Werror,-Wincompatible-pointer-types]
|    23 |         __u8 *rw_data = hid_bpf_get_data(hid_ctx, 0 /*
offset */, 3 /* size */);

This warning, amongst others, is due to some symbol not being included.
In this case, `struct hid_bpf_ctx` is not being defined anywhere that I
can see inside of the testing tree itself.

Instead, `struct hid_bpf_ctx` is defined and implemented at
`include/linux/hid_bpf.h`. AFAIK, I cannot just include this header as
the tools directory is a separate entity from kbuild and these tests are
meant to be built/ran without relying on kernel headers. Am I correct in
this assumption? At any rate, the include itself doesn't work. How can I
properly include this struct definition and fix the warning(s)?

Please note that we cannot just forward declare the struct as it is
being dereferenced and would then yield a completely different
error/warning for an incomplete type. We need the entire implementation
for the struct included.

Other symbols also defined in `include/linux/hid_bpf.h` that we need are
`struct hid_report_type` and `HID_BPF_FLAG...`

Here's the invocation I am running to build kselftest:
`$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
-j128 V=1 -C tools/testing/selftests`

If anyone is currently getting clean builds of kselftest with clang,
what invocation works for you?



Link: https://github.com/ClangBuiltLinux/linux/issues/1698
Full-build-log:
https://gist.github.com/JustinStitt/b217f6e47c1d762e5e1cc6c3532f1bbb
(V=1)

Thanks.
Justin

Re: selftests: hid: trouble building with clang due to missing header

[Nick Desaulniers]

+ Ben, author of commit dbb60c8a26da ("selftests: add tests for the
HID-bpf initial implementation")

On Tue, Aug 22, 2023 at 1:34 PM Justin Stitt <justinstitt@google.com> wrote:
>
> Hi, I'd like to get some help with building the kselftest target.
>
> I am running into some warnings within the hid tree:
> | progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct
> hid_bpf_ctx' will \
> |       not be visible outside of this function [-Werror,-Wvisibility]
> |     9 | extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
> |       |                                      ^
> | progs/hid.c:23:35: error: incompatible pointer types passing 'struct
> hid_bpf_ctx *' \
> |       to parameter of type 'struct hid_bpf_ctx *'
> [-Werror,-Wincompatible-pointer-types]
> |    23 |         __u8 *rw_data = hid_bpf_get_data(hid_ctx, 0 /*
> offset */, 3 /* size */);
>
> This warning, amongst others, is due to some symbol not being included.
> In this case, `struct hid_bpf_ctx` is not being defined anywhere that I
> can see inside of the testing tree itself.
>
> Instead, `struct hid_bpf_ctx` is defined and implemented at
> `include/linux/hid_bpf.h`. AFAIK, I cannot just include this header as
> the tools directory is a separate entity from kbuild and these tests are
> meant to be built/ran without relying on kernel headers. Am I correct in
> this assumption? At any rate, the include itself doesn't work. How can I
> properly include this struct definition and fix the warning(s)?
>
> Please note that we cannot just forward declare the struct as it is
> being dereferenced and would then yield a completely different
> error/warning for an incomplete type. We need the entire implementation
> for the struct included.
>
> Other symbols also defined in `include/linux/hid_bpf.h` that we need are
> `struct hid_report_type` and `HID_BPF_FLAG...`
>
> Here's the invocation I am running to build kselftest:
> `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> -j128 V=1 -C tools/testing/selftests`
>
> If anyone is currently getting clean builds of kselftest with clang,
> what invocation works for you?
>
>
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> Full-build-log:
> https://gist.github.com/JustinStitt/b217f6e47c1d762e5e1cc6c3532f1bbb
> (V=1)
>
> Thanks.
> Justin



-- 
Thanks,
~Nick Desaulniers

Re: selftests: hid: trouble building with clang due to missing header

[Benjamin Tissoires]

Justin,

On Tue, Aug 22, 2023 at 10:44 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> + Ben, author of commit dbb60c8a26da ("selftests: add tests for the
> HID-bpf initial implementation")
>
> On Tue, Aug 22, 2023 at 1:34 PM Justin Stitt <justinstitt@google.com> wrote:
> >
> > Hi, I'd like to get some help with building the kselftest target.
> >
> > I am running into some warnings within the hid tree:
> > | progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct
> > hid_bpf_ctx' will \
> > |       not be visible outside of this function [-Werror,-Wvisibility]
> > |     9 | extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
> > |       |                                      ^
> > | progs/hid.c:23:35: error: incompatible pointer types passing 'struct
> > hid_bpf_ctx *' \
> > |       to parameter of type 'struct hid_bpf_ctx *'
> > [-Werror,-Wincompatible-pointer-types]
> > |    23 |         __u8 *rw_data = hid_bpf_get_data(hid_ctx, 0 /*
> > offset */, 3 /* size */);
> >
> > This warning, amongst others, is due to some symbol not being included.
> > In this case, `struct hid_bpf_ctx` is not being defined anywhere that I
> > can see inside of the testing tree itself.
> >
> > Instead, `struct hid_bpf_ctx` is defined and implemented at
> > `include/linux/hid_bpf.h`. AFAIK, I cannot just include this header as
> > the tools directory is a separate entity from kbuild and these tests are
> > meant to be built/ran without relying on kernel headers. Am I correct in
> > this assumption? At any rate, the include itself doesn't work. How can I
> > properly include this struct definition and fix the warning(s)?
> >
> > Please note that we cannot just forward declare the struct as it is
> > being dereferenced and would then yield a completely different
> > error/warning for an incomplete type. We need the entire implementation
> > for the struct included.
> >
> > Other symbols also defined in `include/linux/hid_bpf.h` that we need are
> > `struct hid_report_type` and `HID_BPF_FLAG...`
> >
> > Here's the invocation I am running to build kselftest:
> > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > -j128 V=1 -C tools/testing/selftests`

I think I fixed the same issue in the script I am running to launch
those tests in a VM. This was in commit
f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).

And in the commit log, I wrote:
```
According to commit 01d6c48a828b ("Documentation: kselftest:
"make headers" is a prerequisite"), running the kselftests requires
to run "make headers" first.
```

So my assumption is that you also need to run "make headers" with the
proper flags before compiling the selftests themselves (I might be
wrong but that's how I read the commit).

Cheers,
Benjamin

> >
> > If anyone is currently getting clean builds of kselftest with clang,
> > what invocation works for you?
> >
> >
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> > Full-build-log:
> > https://gist.github.com/JustinStitt/b217f6e47c1d762e5e1cc6c3532f1bbb
> > (V=1)
> >
> > Thanks.
> > Justin
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
>

Re: selftests: hid: trouble building with clang due to missing header

[Justin Stitt]

On Tue, Aug 22, 2023 at 1:52 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Justin,
>
> On Tue, Aug 22, 2023 at 10:44 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > + Ben, author of commit dbb60c8a26da ("selftests: add tests for the
> > HID-bpf initial implementation")
> >
> > On Tue, Aug 22, 2023 at 1:34 PM Justin Stitt <justinstitt@google.com> wrote:
> > >
> > > Hi, I'd like to get some help with building the kselftest target.
> > >
> > > I am running into some warnings within the hid tree:
> > > | progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct
> > > hid_bpf_ctx' will \
> > > |       not be visible outside of this function [-Werror,-Wvisibility]
> > > |     9 | extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
> > > |       |                                      ^
> > > | progs/hid.c:23:35: error: incompatible pointer types passing 'struct
> > > hid_bpf_ctx *' \
> > > |       to parameter of type 'struct hid_bpf_ctx *'
> > > [-Werror,-Wincompatible-pointer-types]
> > > |    23 |         __u8 *rw_data = hid_bpf_get_data(hid_ctx, 0 /*
> > > offset */, 3 /* size */);
> > >
> > > This warning, amongst others, is due to some symbol not being included.
> > > In this case, `struct hid_bpf_ctx` is not being defined anywhere that I
> > > can see inside of the testing tree itself.
> > >
> > > Instead, `struct hid_bpf_ctx` is defined and implemented at
> > > `include/linux/hid_bpf.h`. AFAIK, I cannot just include this header as
> > > the tools directory is a separate entity from kbuild and these tests are
> > > meant to be built/ran without relying on kernel headers. Am I correct in
> > > this assumption? At any rate, the include itself doesn't work. How can I
> > > properly include this struct definition and fix the warning(s)?
> > >
> > > Please note that we cannot just forward declare the struct as it is
> > > being dereferenced and would then yield a completely different
> > > error/warning for an incomplete type. We need the entire implementation
> > > for the struct included.
> > >
> > > Other symbols also defined in `include/linux/hid_bpf.h` that we need are
> > > `struct hid_report_type` and `HID_BPF_FLAG...`
> > >
> > > Here's the invocation I am running to build kselftest:
> > > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > > -j128 V=1 -C tools/testing/selftests`
>
> I think I fixed the same issue in the script I am running to launch
> those tests in a VM. This was in commit
> f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).
>
> And in the commit log, I wrote:
> ```
> According to commit 01d6c48a828b ("Documentation: kselftest:
> "make headers" is a prerequisite"), running the kselftests requires
> to run "make headers" first.
> ```
>
> So my assumption is that you also need to run "make headers" with the
> proper flags before compiling the selftests themselves (I might be
> wrong but that's how I read the commit).

In my original email I pasted the invocation I used which includes the
headers target. What are the "proper flags" in this case?

>
> Cheers,
> Benjamin
>
> > >
> > > If anyone is currently getting clean builds of kselftest with clang,
> > > what invocation works for you?
> > >
> > >
> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1698
> > > Full-build-log:
> > > https://gist.github.com/JustinStitt/b217f6e47c1d762e5e1cc6c3532f1bbb
> > > (V=1)
> > >
> > > Thanks.
> > > Justin
> >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
> >
>

Re: selftests: hid: trouble building with clang due to missing header

[Benjamin Tissoires]

On Tue, Aug 22, 2023 at 10:57 PM Justin Stitt <justinstitt@google.com> wrote:
>
[...]
> > > > Here's the invocation I am running to build kselftest:
> > > > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > > > -j128 V=1 -C tools/testing/selftests`
> >
> > I think I fixed the same issue in the script I am running to launch
> > those tests in a VM. This was in commit
> > f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).
> >
> > And in the commit log, I wrote:
> > ```
> > According to commit 01d6c48a828b ("Documentation: kselftest:
> > "make headers" is a prerequisite"), running the kselftests requires
> > to run "make headers" first.
> > ```
> >
> > So my assumption is that you also need to run "make headers" with the
> > proper flags before compiling the selftests themselves (I might be
> > wrong but that's how I read the commit).
>
> In my original email I pasted the invocation I used which includes the
> headers target. What are the "proper flags" in this case?
>

"make LLVM=1 ARCH=x86_64 headers" no?

But now I'm starting to wonder if that was not the intent of your
combined "make mrproper headers". I honestly never tried to combine
the 2. It's worth a try to split them I would say.

Cheers,
Benjamin

Re: selftests: hid: trouble building with clang due to missing header

[Benjamin Tissoires]

On Tue, Aug 22, 2023 at 11:06 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Tue, Aug 22, 2023 at 10:57 PM Justin Stitt <justinstitt@google.com> wrote:
> >
> [...]
> > > > > Here's the invocation I am running to build kselftest:
> > > > > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > > > > -j128 V=1 -C tools/testing/selftests`
> > >
> > > I think I fixed the same issue in the script I am running to launch
> > > those tests in a VM. This was in commit
> > > f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).
> > >
> > > And in the commit log, I wrote:
> > > ```
> > > According to commit 01d6c48a828b ("Documentation: kselftest:
> > > "make headers" is a prerequisite"), running the kselftests requires
> > > to run "make headers" first.
> > > ```
> > >
> > > So my assumption is that you also need to run "make headers" with the
> > > proper flags before compiling the selftests themselves (I might be
> > > wrong but that's how I read the commit).
> >
> > In my original email I pasted the invocation I used which includes the
> > headers target. What are the "proper flags" in this case?
> >
>
> "make LLVM=1 ARCH=x86_64 headers" no?
>
> But now I'm starting to wonder if that was not the intent of your
> combined "make mrproper headers". I honestly never tried to combine
> the 2. It's worth a try to split them I would say.

Apologies, I just tested it, and it works (combining the 2).

Which kernel are you trying to test?
I tested your 2 commands on v6.5-rc7 and it just works.

FTR:
$> git checkout v6.5-rc7
$> make LLVM=1 ARCH=x86_64 mrproper headers
$> make LLVM=1 ARCH=x86_64 -j8 -C tools/testing/selftests TARGETS=hid

->   BINARY   hid_bpf

Cheers,
Benjamin

Re: selftests: hid: trouble building with clang due to missing header

[Justin Stitt]

On Tue, Aug 22, 2023 at 2:15 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Tue, Aug 22, 2023 at 11:06 PM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > On Tue, Aug 22, 2023 at 10:57 PM Justin Stitt <justinstitt@google.com> wrote:
> > >
> > [...]
> > > > > > Here's the invocation I am running to build kselftest:
> > > > > > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > > > > > -j128 V=1 -C tools/testing/selftests`
> > > >
> > > > I think I fixed the same issue in the script I am running to launch
> > > > those tests in a VM. This was in commit
> > > > f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).
> > > >
> > > > And in the commit log, I wrote:
> > > > ```
> > > > According to commit 01d6c48a828b ("Documentation: kselftest:
> > > > "make headers" is a prerequisite"), running the kselftests requires
> > > > to run "make headers" first.
> > > > ```
> > > >
> > > > So my assumption is that you also need to run "make headers" with the
> > > > proper flags before compiling the selftests themselves (I might be
> > > > wrong but that's how I read the commit).
> > >
> > > In my original email I pasted the invocation I used which includes the
> > > headers target. What are the "proper flags" in this case?
> > >
> >
> > "make LLVM=1 ARCH=x86_64 headers" no?
> >
> > But now I'm starting to wonder if that was not the intent of your
> > combined "make mrproper headers". I honestly never tried to combine
> > the 2. It's worth a try to split them I would say.
>
> Apologies, I just tested it, and it works (combining the 2).
>
> Which kernel are you trying to test?
> I tested your 2 commands on v6.5-rc7 and it just works.

I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)

I ran these exact commands:
|    $ make mrproper
|    $ make LLVM=1 ARCH=x86_64 headers
|    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
TARGETS=hid &> out

and here's the contents of `out` (still warnings/errors):
https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d

I have a feeling I'm doing something fundamentally incorrectly. Any ideas?

To be clear, I can build the Kernel itself just fine across many
configs and architectures. I have, at the very least, the dependencies
required to accomplish that.

>
> FTR:
> $> git checkout v6.5-rc7
> $> make LLVM=1 ARCH=x86_64 mrproper headers
> $> make LLVM=1 ARCH=x86_64 -j8 -C tools/testing/selftests TARGETS=hid
>
> ->   BINARY   hid_bpf
>
> Cheers,
> Benjamin
>

Re: selftests: hid: trouble building with clang due to missing header

[Benjamin Tissoires]

On Tue, Aug 22, 2023 at 11:26 PM Justin Stitt <justinstitt@google.com> wrote:
>
> On Tue, Aug 22, 2023 at 2:15 PM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > On Tue, Aug 22, 2023 at 11:06 PM Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 10:57 PM Justin Stitt <justinstitt@google.com> wrote:
> > > >
> > > [...]
> > > > > > > Here's the invocation I am running to build kselftest:
> > > > > > > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > > > > > > -j128 V=1 -C tools/testing/selftests`
> > > > >
> > > > > I think I fixed the same issue in the script I am running to launch
> > > > > those tests in a VM. This was in commit
> > > > > f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).
> > > > >
> > > > > And in the commit log, I wrote:
> > > > > ```
> > > > > According to commit 01d6c48a828b ("Documentation: kselftest:
> > > > > "make headers" is a prerequisite"), running the kselftests requires
> > > > > to run "make headers" first.
> > > > > ```
> > > > >
> > > > > So my assumption is that you also need to run "make headers" with the
> > > > > proper flags before compiling the selftests themselves (I might be
> > > > > wrong but that's how I read the commit).
> > > >
> > > > In my original email I pasted the invocation I used which includes the
> > > > headers target. What are the "proper flags" in this case?
> > > >
> > >
> > > "make LLVM=1 ARCH=x86_64 headers" no?
> > >
> > > But now I'm starting to wonder if that was not the intent of your
> > > combined "make mrproper headers". I honestly never tried to combine
> > > the 2. It's worth a try to split them I would say.
> >
> > Apologies, I just tested it, and it works (combining the 2).
> >
> > Which kernel are you trying to test?
> > I tested your 2 commands on v6.5-rc7 and it just works.
>
> I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)
>
> I ran these exact commands:
> |    $ make mrproper
> |    $ make LLVM=1 ARCH=x86_64 headers
> |    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
> TARGETS=hid &> out
>
> and here's the contents of `out` (still warnings/errors):
> https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d
>
> I have a feeling I'm doing something fundamentally incorrectly. Any ideas?

Sigh... there is a high chance my Makefile is not correct and uses the
installed headers (I was running the exact same commands, but on a
v6.4-rc7+ kernel).

But sorry, it will have to wait for tomorrow if you want me to have a
look at it. It's 11:35 PM here, and I need to go to bed

Cheers,
Benjamin

>
> To be clear, I can build the Kernel itself just fine across many
> configs and architectures. I have, at the very least, the dependencies
> required to accomplish that.
>
> >
> > FTR:
> > $> git checkout v6.5-rc7
> > $> make LLVM=1 ARCH=x86_64 mrproper headers
> > $> make LLVM=1 ARCH=x86_64 -j8 -C tools/testing/selftests TARGETS=hid
> >
> > ->   BINARY   hid_bpf
> >
> > Cheers,
> > Benjamin
> >
>

Re: selftests: hid: trouble building with clang due to missing header

[Justin Stitt]

On Tue, Aug 22, 2023 at 2:36 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Tue, Aug 22, 2023 at 11:26 PM Justin Stitt <justinstitt@google.com> wrote:
> >
> > On Tue, Aug 22, 2023 at 2:15 PM Benjamin Tissoires
> > <benjamin.tissoires@redhat.com> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 11:06 PM Benjamin Tissoires
> > > <benjamin.tissoires@redhat.com> wrote:
> > > >
> > > > On Tue, Aug 22, 2023 at 10:57 PM Justin Stitt <justinstitt@google.com> wrote:
> > > > >
> > > > [...]
> > > > > > > > Here's the invocation I am running to build kselftest:
> > > > > > > > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > > > > > > > -j128 V=1 -C tools/testing/selftests`
> > > > > >
> > > > > > I think I fixed the same issue in the script I am running to launch
> > > > > > those tests in a VM. This was in commit
> > > > > > f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).
> > > > > >
> > > > > > And in the commit log, I wrote:
> > > > > > ```
> > > > > > According to commit 01d6c48a828b ("Documentation: kselftest:
> > > > > > "make headers" is a prerequisite"), running the kselftests requires
> > > > > > to run "make headers" first.
> > > > > > ```
> > > > > >
> > > > > > So my assumption is that you also need to run "make headers" with the
> > > > > > proper flags before compiling the selftests themselves (I might be
> > > > > > wrong but that's how I read the commit).
> > > > >
> > > > > In my original email I pasted the invocation I used which includes the
> > > > > headers target. What are the "proper flags" in this case?
> > > > >
> > > >
> > > > "make LLVM=1 ARCH=x86_64 headers" no?
> > > >
> > > > But now I'm starting to wonder if that was not the intent of your
> > > > combined "make mrproper headers". I honestly never tried to combine
> > > > the 2. It's worth a try to split them I would say.
> > >
> > > Apologies, I just tested it, and it works (combining the 2).
> > >
> > > Which kernel are you trying to test?
> > > I tested your 2 commands on v6.5-rc7 and it just works.
> >
> > I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)
> >
> > I ran these exact commands:
> > |    $ make mrproper
> > |    $ make LLVM=1 ARCH=x86_64 headers
> > |    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
> > TARGETS=hid &> out
> >
> > and here's the contents of `out` (still warnings/errors):
> > https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d
> >
> > I have a feeling I'm doing something fundamentally incorrectly. Any ideas?
>
> Sigh... there is a high chance my Makefile is not correct and uses the
> installed headers (I was running the exact same commands, but on a
> v6.4-rc7+ kernel).
>
> But sorry, it will have to wait for tomorrow if you want me to have a
> look at it. It's 11:35 PM here, and I need to go to bed
Take it easy. Thanks for the prompt responses here! I'd like to get
the entire kselftest make target building with Clang so that we can
close [1].

>
> Cheers,
> Benjamin
>
> >
> > To be clear, I can build the Kernel itself just fine across many
> > configs and architectures. I have, at the very least, the dependencies
> > required to accomplish that.
> >
> > >
> > > FTR:
> > > $> git checkout v6.5-rc7
> > > $> make LLVM=1 ARCH=x86_64 mrproper headers
> > > $> make LLVM=1 ARCH=x86_64 -j8 -C tools/testing/selftests TARGETS=hid
> > >
> > > ->   BINARY   hid_bpf
> > >
> > > Cheers,
> > > Benjamin
> > >
> >
>

[1]: https://github.com/ClangBuiltLinux/linux/issues/1910

Re: selftests: hid: trouble building with clang due to missing header

[Justin Stitt]

On Tue, Aug 22, 2023 at 02:38:29PM -0700, Justin Stitt wrote:
> On Tue, Aug 22, 2023 at 2:36 PM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > On Tue, Aug 22, 2023 at 11:26 PM Justin Stitt <justinstitt@google.com> wrote:
> > >
> > > On Tue, Aug 22, 2023 at 2:15 PM Benjamin Tissoires
> > > <benjamin.tissoires@redhat.com> wrote:
> > > >
> > > > On Tue, Aug 22, 2023 at 11:06 PM Benjamin Tissoires
> > > > <benjamin.tissoires@redhat.com> wrote:
> > > > >
> > > > > On Tue, Aug 22, 2023 at 10:57 PM Justin Stitt <justinstitt@google.com> wrote:
> > > > > >
> > > > > [...]
> > > > > > > > > Here's the invocation I am running to build kselftest:
> > > > > > > > > `$ make LLVM=1 ARCH=x86_64 mrproper headers && make LLVM=1 ARCH=x86_64
> > > > > > > > > -j128 V=1 -C tools/testing/selftests`
> > > > > > >
> > > > > > > I think I fixed the same issue in the script I am running to launch
> > > > > > > those tests in a VM. This was in commit
> > > > > > > f9abdcc617dad5f14bbc2ebe96ee99f3e6de0c4e (in the v6.5-rc+ series).
> > > > > > >
> > > > > > > And in the commit log, I wrote:
> > > > > > > ```
> > > > > > > According to commit 01d6c48a828b ("Documentation: kselftest:
> > > > > > > "make headers" is a prerequisite"), running the kselftests requires
> > > > > > > to run "make headers" first.
> > > > > > > ```
> > > > > > >
> > > > > > > So my assumption is that you also need to run "make headers" with the
> > > > > > > proper flags before compiling the selftests themselves (I might be
> > > > > > > wrong but that's how I read the commit).
> > > > > >
> > > > > > In my original email I pasted the invocation I used which includes the
> > > > > > headers target. What are the "proper flags" in this case?
> > > > > >
> > > > >
> > > > > "make LLVM=1 ARCH=x86_64 headers" no?
> > > > >
> > > > > But now I'm starting to wonder if that was not the intent of your
> > > > > combined "make mrproper headers". I honestly never tried to combine
> > > > > the 2. It's worth a try to split them I would say.
> > > >
> > > > Apologies, I just tested it, and it works (combining the 2).
> > > >
> > > > Which kernel are you trying to test?
> > > > I tested your 2 commands on v6.5-rc7 and it just works.
> > >
> > > I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)
> > >
> > > I ran these exact commands:
> > > |    $ make mrproper
> > > |    $ make LLVM=1 ARCH=x86_64 headers
> > > |    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
> > > TARGETS=hid &> out
> > >
> > > and here's the contents of `out` (still warnings/errors):
> > > https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d
> > >
> > > I have a feeling I'm doing something fundamentally incorrectly. Any ideas?
> >
> > Sigh... there is a high chance my Makefile is not correct and uses the
> > installed headers (I was running the exact same commands, but on a
> > v6.4-rc7+ kernel).
> >
> > But sorry, it will have to wait for tomorrow if you want me to have a
> > look at it. It's 11:35 PM here, and I need to go to bed
> Take it easy. Thanks for the prompt responses here! I'd like to get
> the entire kselftest make target building with Clang so that we can
> close [1].
>
> >
> > Cheers,
> > Benjamin
> >
> > >
> > > To be clear, I can build the Kernel itself just fine across many
> > > configs and architectures. I have, at the very least, the dependencies
> > > required to accomplish that.
> > >
> > > >
> > > > FTR:
> > > > $> git checkout v6.5-rc7
> > > > $> make LLVM=1 ARCH=x86_64 mrproper headers
> > > > $> make LLVM=1 ARCH=x86_64 -j8 -C tools/testing/selftests TARGETS=hid
> > > >
> > > > ->   BINARY   hid_bpf
> > > >
> > > > Cheers,
> > > > Benjamin
> > > >
> > >
> >
>
> [1]: https://github.com/ClangBuiltLinux/linux/issues/1910

Erhm, I meant [1]: https://github.com/ClangBuiltLinux/linux/issues/1698

Re: selftests: hid: trouble building with clang due to missing header

[Benjamin Tissoires]

On Tue, Aug 22, 2023 at 11:42 PM Justin Stitt <justinstitt@google.com> wrote:
> > > > > Which kernel are you trying to test?
> > > > > I tested your 2 commands on v6.5-rc7 and it just works.
> > > >
> > > > I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)
> > > >
> > > > I ran these exact commands:
> > > > |    $ make mrproper
> > > > |    $ make LLVM=1 ARCH=x86_64 headers
> > > > |    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
> > > > TARGETS=hid &> out
> > > >
> > > > and here's the contents of `out` (still warnings/errors):
> > > > https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d
> > > >
> > > > I have a feeling I'm doing something fundamentally incorrectly. Any ideas?
> > >
> > > Sigh... there is a high chance my Makefile is not correct and uses the
> > > installed headers (I was running the exact same commands, but on a
> > > v6.4-rc7+ kernel).
> > >
> > > But sorry, it will have to wait for tomorrow if you want me to have a
> > > look at it. It's 11:35 PM here, and I need to go to bed
> > Take it easy. Thanks for the prompt responses here! I'd like to get
> > the entire kselftest make target building with Clang so that we can
> > close [1].

Sorry I got urgent matters to tackle yesterday.

It took me a while to understand what was going on, and I finally found
it.

struct hid_bpf_ctx is internal to the kernel, and so is declared in
vmlinux.h, that we generate through BTF. But to generate the vmlinux.h
with the correct symbols, these need to be present in the running
kernel.
And that's where we had a fundamental difference: I was running my
compilations on a kernel v6.3+ (6.4.11) with that symbol available, and
you are probably not.

The bpf folks are using a clever trick to force the compilation[2]. And
I think the following patch would work for you:

---
 From bb9eccb7a896ba4b3a35ed12a248e6d6cfed2df6 Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <bentiss@kernel.org>
Date: Fri, 25 Aug 2023 10:02:32 +0200
Subject: [PATCH] selftests/hid: ensure we can compile the tests on kernels
  pre-6.3

For the hid-bpf tests to compile, we need to have the definition of
struct hid_bpf_ctx. This definition is an internal one from the kernel
and it is supposed to be defined in the generated vmlinux.h.

This vmlinux.h header is generated based on the currently running kernel
or if the kernel was already compiled in the tree. If you just compile
the selftests without compiling the kernel beforehand and you are running
on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx
definition.

Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h
to force the definition of that symbol in case we don't find it in the
BTF.

Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
  tools/testing/selftests/hid/progs/hid.c       |  3 ---
  .../selftests/hid/progs/hid_bpf_helpers.h     | 20 +++++++++++++++++++
  2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
index 88c593f753b5..1e558826b809 100644
--- a/tools/testing/selftests/hid/progs/hid.c
+++ b/tools/testing/selftests/hid/progs/hid.c
@@ -1,8 +1,5 @@
  // SPDX-License-Identifier: GPL-2.0
  /* Copyright (c) 2022 Red hat */
-#include "vmlinux.h"
-#include <bpf/bpf_helpers.h>
-#include <bpf/bpf_tracing.h>
  #include "hid_bpf_helpers.h"
  
  char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
index 4fff31dbe0e7..749097f8f4d9 100644
--- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
+++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
@@ -5,6 +5,26 @@
  #ifndef __HID_BPF_HELPERS_H
  #define __HID_BPF_HELPERS_H
  
+/* "undefine" structs in vmlinux.h, because we "override" them below */
+#define hid_bpf_ctx hid_bpf_ctx___not_used
+#include "vmlinux.h"
+#undef hid_bpf_ctx
+
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+
+struct hid_bpf_ctx {
+	__u32 index;
+	const struct hid_device *hid;
+	__u32 allocated_size;
+	enum hid_report_type report_type;
+	union {
+		__s32 retval;
+		__s32 size;
+	};
+};
+
  /* following are kfuncs exported by HID for HID-BPF */
  extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
  			      unsigned int offset,
-- 
2.41.0
---

Would you mind testing it?

Cheers,
Benjamin

[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/bpf/progs/bpf_iter.h#n3

Re: selftests: hid: trouble building with clang due to missing header

[Eduard Zingerman]

On Fri, 2023-08-25 at 10:08 +0200, Benjamin Tissoires wrote:
> 
> On Tue, Aug 22, 2023 at 11:42 PM Justin Stitt <justinstitt@google.com> wrote:
> > > > > > Which kernel are you trying to test?
> > > > > > I tested your 2 commands on v6.5-rc7 and it just works.
> > > > > 
> > > > > I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)
> > > > > 
> > > > > I ran these exact commands:
> > > > > >    $ make mrproper
> > > > > >    $ make LLVM=1 ARCH=x86_64 headers
> > > > > >    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
> > > > > TARGETS=hid &> out
> > > > > 
> > > > > and here's the contents of `out` (still warnings/errors):
> > > > > https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d
> > > > > 
> > > > > I have a feeling I'm doing something fundamentally incorrectly. Any ideas?
> > > > 
> > > > Sigh... there is a high chance my Makefile is not correct and uses the
> > > > installed headers (I was running the exact same commands, but on a
> > > > v6.4-rc7+ kernel).
> > > > 
> > > > But sorry, it will have to wait for tomorrow if you want me to have a
> > > > look at it. It's 11:35 PM here, and I need to go to bed
> > > Take it easy. Thanks for the prompt responses here! I'd like to get
> > > the entire kselftest make target building with Clang so that we can
> > > close [1].
> 
> Sorry I got urgent matters to tackle yesterday.
> 
> It took me a while to understand what was going on, and I finally found
> it.
> 
> struct hid_bpf_ctx is internal to the kernel, and so is declared in
> vmlinux.h, that we generate through BTF. But to generate the vmlinux.h
> with the correct symbols, these need to be present in the running
> kernel.
> And that's where we had a fundamental difference: I was running my
> compilations on a kernel v6.3+ (6.4.11) with that symbol available, and
> you are probably not.
> 
> The bpf folks are using a clever trick to force the compilation[2]. And
> I think the following patch would work for you:

Hi Benjamin, Justin,

You might want to take a look at these two links:
[1] https://nakryiko.com/posts/bpf-core-reference-guide/#handling-incompatible-field-and-type-changes
[2] https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html#dealing-with-kernel-version-and-configuration-differences

The short version is: CO-RE relocation handling logic in libbpf
ignores suffixes of form '___something' for type and field names.

So, the following should accomplish the same as the trick with
#define/#undef:

    #include "vmlinux.h"
    ...
    struct hid_bpf_ctx___local {
        __u32 index;
        const struct hid_device *hid;
        __u32 allocated_size;
        enum hid_report_type report_type;
        union {
            __s32 retval;
            __s32 size;
        };
    
    };
    ...
    extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx,
                                  unsigned int offset, ...)

However, if the kernel does not have `hid_bpf_ctx` definition would
the test `progs/hid.c` still make sense?

When I tried to build hid tests locally I run into similar errors:

    ...
      CLNG-BPF hid.bpf.o
    In file included from progs/hid.c:6:
    progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct hid_bpf_ctx' \
           will not be visible outside of this function [-Werror,-Wvisibility]
    extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
    ...

And there is indeed no `hid_bpf_ctx` in my vmlinux.h.
However, after enabling CONFIG_HID_BPF in kernel config the
`hid_bpf_ctx` appears in vmlinux.h, and I can compile HID selftests
w/o issues.

> 
> ---
>  From bb9eccb7a896ba4b3a35ed12a248e6d6cfed2df6 Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <bentiss@kernel.org>
> Date: Fri, 25 Aug 2023 10:02:32 +0200
> Subject: [PATCH] selftests/hid: ensure we can compile the tests on kernels
>   pre-6.3
> 
> For the hid-bpf tests to compile, we need to have the definition of
> struct hid_bpf_ctx. This definition is an internal one from the kernel
> and it is supposed to be defined in the generated vmlinux.h.
> 
> This vmlinux.h header is generated based on the currently running kernel
> or if the kernel was already compiled in the tree. If you just compile
> the selftests without compiling the kernel beforehand and you are running
> on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx
> definition.
> 
> Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h
> to force the definition of that symbol in case we don't find it in the
> BTF.
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
>   tools/testing/selftests/hid/progs/hid.c       |  3 ---
>   .../selftests/hid/progs/hid_bpf_helpers.h     | 20 +++++++++++++++++++
>   2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
> index 88c593f753b5..1e558826b809 100644
> --- a/tools/testing/selftests/hid/progs/hid.c
> +++ b/tools/testing/selftests/hid/progs/hid.c
> @@ -1,8 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /* Copyright (c) 2022 Red hat */
> -#include "vmlinux.h"
> -#include <bpf/bpf_helpers.h>
> -#include <bpf/bpf_tracing.h>
>   #include "hid_bpf_helpers.h"
>   
>   char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> index 4fff31dbe0e7..749097f8f4d9 100644
> --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> @@ -5,6 +5,26 @@
>   #ifndef __HID_BPF_HELPERS_H
>   #define __HID_BPF_HELPERS_H
>   
> +/* "undefine" structs in vmlinux.h, because we "override" them below */
> +#define hid_bpf_ctx hid_bpf_ctx___not_used
> +#include "vmlinux.h"
> +#undef hid_bpf_ctx
> +
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +
> +struct hid_bpf_ctx {
> +	__u32 index;
> +	const struct hid_device *hid;
> +	__u32 allocated_size;
> +	enum hid_report_type report_type;
> +	union {
> +		__s32 retval;
> +		__s32 size;
> +	};
> +};
> +
>   /* following are kfuncs exported by HID for HID-BPF */
>   extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
>   			      unsigned int offset,

Re: selftests: hid: trouble building with clang due to missing header

[Justin Stitt]

On Fri, Aug 25, 2023 at 6:01 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2023-08-25 at 10:08 +0200, Benjamin Tissoires wrote:
> >
> > On Tue, Aug 22, 2023 at 11:42 PM Justin Stitt <justinstitt@google.com> wrote:
> > > > > > > Which kernel are you trying to test?
> > > > > > > I tested your 2 commands on v6.5-rc7 and it just works.
> > > > > >
> > > > > > I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)
> > > > > >
> > > > > > I ran these exact commands:
> > > > > > >    $ make mrproper
> > > > > > >    $ make LLVM=1 ARCH=x86_64 headers
> > > > > > >    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
> > > > > > TARGETS=hid &> out
> > > > > >
> > > > > > and here's the contents of `out` (still warnings/errors):
> > > > > > https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d
> > > > > >
> > > > > > I have a feeling I'm doing something fundamentally incorrectly. Any ideas?
> > > > >
> > > > > Sigh... there is a high chance my Makefile is not correct and uses the
> > > > > installed headers (I was running the exact same commands, but on a
> > > > > v6.4-rc7+ kernel).
> > > > >
> > > > > But sorry, it will have to wait for tomorrow if you want me to have a
> > > > > look at it. It's 11:35 PM here, and I need to go to bed
> > > > Take it easy. Thanks for the prompt responses here! I'd like to get
> > > > the entire kselftest make target building with Clang so that we can
> > > > close [1].
> >
> > Sorry I got urgent matters to tackle yesterday.
> >
> > It took me a while to understand what was going on, and I finally found
> > it.
> >
> > struct hid_bpf_ctx is internal to the kernel, and so is declared in
> > vmlinux.h, that we generate through BTF. But to generate the vmlinux.h
> > with the correct symbols, these need to be present in the running
> > kernel.
> > And that's where we had a fundamental difference: I was running my
> > compilations on a kernel v6.3+ (6.4.11) with that symbol available, and
> > you are probably not.
> >
> > The bpf folks are using a clever trick to force the compilation[2]. And
> > I think the following patch would work for you:
>
> Hi Benjamin, Justin,
>
> You might want to take a look at these two links:
> [1] https://nakryiko.com/posts/bpf-core-reference-guide/#handling-incompatible-field-and-type-changes
> [2] https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html#dealing-with-kernel-version-and-configuration-differences
>
> The short version is: CO-RE relocation handling logic in libbpf
> ignores suffixes of form '___something' for type and field names.
>
> So, the following should accomplish the same as the trick with
> #define/#undef:
>
>     #include "vmlinux.h"
>     ...
>     struct hid_bpf_ctx___local {
>         __u32 index;
>         const struct hid_device *hid;
>         __u32 allocated_size;
>         enum hid_report_type report_type;
>         union {
>             __s32 retval;
>             __s32 size;
>         };
>
>     };
>     ...
>     extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx,
>                                   unsigned int offset, ...)
>
> However, if the kernel does not have `hid_bpf_ctx` definition would
> the test `progs/hid.c` still make sense?
>
> When I tried to build hid tests locally I run into similar errors:
>
>     ...
>       CLNG-BPF hid.bpf.o
>     In file included from progs/hid.c:6:
>     progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct hid_bpf_ctx' \
>            will not be visible outside of this function [-Werror,-Wvisibility]
>     extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
>     ...
>
> And there is indeed no `hid_bpf_ctx` in my vmlinux.h.
> However, after enabling CONFIG_HID_BPF in kernel config the
> `hid_bpf_ctx` appears in vmlinux.h, and I can compile HID selftests
> w/o issues.

Even with enabling this configuration option I was unable to get clean
builds of the HID selftests. I proposed a 4th patch on top of
Benjamin's n=3 patch series here [1] using the #def/#undef pattern.

>
> >
> > ---
> >  From bb9eccb7a896ba4b3a35ed12a248e6d6cfed2df6 Mon Sep 17 00:00:00 2001
> > From: Benjamin Tissoires <bentiss@kernel.org>
> > Date: Fri, 25 Aug 2023 10:02:32 +0200
> > Subject: [PATCH] selftests/hid: ensure we can compile the tests on kernels
> >   pre-6.3
> >
> > For the hid-bpf tests to compile, we need to have the definition of
> > struct hid_bpf_ctx. This definition is an internal one from the kernel
> > and it is supposed to be defined in the generated vmlinux.h.
> >
> > This vmlinux.h header is generated based on the currently running kernel
> > or if the kernel was already compiled in the tree. If you just compile
> > the selftests without compiling the kernel beforehand and you are running
> > on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx
> > definition.
> >
> > Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h
> > to force the definition of that symbol in case we don't find it in the
> > BTF.
> >
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> >   tools/testing/selftests/hid/progs/hid.c       |  3 ---
> >   .../selftests/hid/progs/hid_bpf_helpers.h     | 20 +++++++++++++++++++
> >   2 files changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
> > index 88c593f753b5..1e558826b809 100644
> > --- a/tools/testing/selftests/hid/progs/hid.c
> > +++ b/tools/testing/selftests/hid/progs/hid.c
> > @@ -1,8 +1,5 @@
> >   // SPDX-License-Identifier: GPL-2.0
> >   /* Copyright (c) 2022 Red hat */
> > -#include "vmlinux.h"
> > -#include <bpf/bpf_helpers.h>
> > -#include <bpf/bpf_tracing.h>
> >   #include "hid_bpf_helpers.h"
> >
> >   char _license[] SEC("license") = "GPL";
> > diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > index 4fff31dbe0e7..749097f8f4d9 100644
> > --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > @@ -5,6 +5,26 @@
> >   #ifndef __HID_BPF_HELPERS_H
> >   #define __HID_BPF_HELPERS_H
> >
> > +/* "undefine" structs in vmlinux.h, because we "override" them below */
> > +#define hid_bpf_ctx hid_bpf_ctx___not_used
> > +#include "vmlinux.h"
> > +#undef hid_bpf_ctx
> > +
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +
> > +
> > +struct hid_bpf_ctx {
> > +     __u32 index;
> > +     const struct hid_device *hid;
> > +     __u32 allocated_size;
> > +     enum hid_report_type report_type;
> > +     union {
> > +             __s32 retval;
> > +             __s32 size;
> > +     };
> > +};
> > +
> >   /* following are kfuncs exported by HID for HID-BPF */
> >   extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
> >                             unsigned int offset,
>

[1]: https://lore.kernel.org/all/20230825182316.m2ksjoxe4s7dsapn@google.com/

Thanks
Justin

Re: selftests: hid: trouble building with clang due to missing header

[Eduard Zingerman]

On Fri, 2023-08-25 at 11:36 -0700, Justin Stitt wrote:
> On Fri, Aug 25, 2023 at 6:01 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Fri, 2023-08-25 at 10:08 +0200, Benjamin Tissoires wrote:
> > > 
> > > On Tue, Aug 22, 2023 at 11:42 PM Justin Stitt <justinstitt@google.com> wrote:
> > > > > > > > Which kernel are you trying to test?
> > > > > > > > I tested your 2 commands on v6.5-rc7 and it just works.
> > > > > > > 
> > > > > > > I'm also on v6.5-rc7 (706a741595047797872e669b3101429ab8d378ef)
> > > > > > > 
> > > > > > > I ran these exact commands:
> > > > > > > >    $ make mrproper
> > > > > > > >    $ make LLVM=1 ARCH=x86_64 headers
> > > > > > > >    $ make LLVM=1 ARCH=x86_64 -j128 -C tools/testing/selftests
> > > > > > > TARGETS=hid &> out
> > > > > > > 
> > > > > > > and here's the contents of `out` (still warnings/errors):
> > > > > > > https://gist.github.com/JustinStitt/d0c30180a2a2e046c32d5f0ce5f59c6d
> > > > > > > 
> > > > > > > I have a feeling I'm doing something fundamentally incorrectly. Any ideas?
> > > > > > 
> > > > > > Sigh... there is a high chance my Makefile is not correct and uses the
> > > > > > installed headers (I was running the exact same commands, but on a
> > > > > > v6.4-rc7+ kernel).
> > > > > > 
> > > > > > But sorry, it will have to wait for tomorrow if you want me to have a
> > > > > > look at it. It's 11:35 PM here, and I need to go to bed
> > > > > Take it easy. Thanks for the prompt responses here! I'd like to get
> > > > > the entire kselftest make target building with Clang so that we can
> > > > > close [1].
> > > 
> > > Sorry I got urgent matters to tackle yesterday.
> > > 
> > > It took me a while to understand what was going on, and I finally found
> > > it.
> > > 
> > > struct hid_bpf_ctx is internal to the kernel, and so is declared in
> > > vmlinux.h, that we generate through BTF. But to generate the vmlinux.h
> > > with the correct symbols, these need to be present in the running
> > > kernel.
> > > And that's where we had a fundamental difference: I was running my
> > > compilations on a kernel v6.3+ (6.4.11) with that symbol available, and
> > > you are probably not.
> > > 
> > > The bpf folks are using a clever trick to force the compilation[2]. And
> > > I think the following patch would work for you:
> > 
> > Hi Benjamin, Justin,
> > 
> > You might want to take a look at these two links:
> > [1] https://nakryiko.com/posts/bpf-core-reference-guide/#handling-incompatible-field-and-type-changes
> > [2] https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html#dealing-with-kernel-version-and-configuration-differences
> > 
> > The short version is: CO-RE relocation handling logic in libbpf
> > ignores suffixes of form '___something' for type and field names.
> > 
> > So, the following should accomplish the same as the trick with
> > #define/#undef:
> > 
> >     #include "vmlinux.h"
> >     ...
> >     struct hid_bpf_ctx___local {
> >         __u32 index;
> >         const struct hid_device *hid;
> >         __u32 allocated_size;
> >         enum hid_report_type report_type;
> >         union {
> >             __s32 retval;
> >             __s32 size;
> >         };
> > 
> >     };
> >     ...
> >     extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx___local *ctx,
> >                                   unsigned int offset, ...)
> > 
> > However, if the kernel does not have `hid_bpf_ctx` definition would
> > the test `progs/hid.c` still make sense?
> > 
> > When I tried to build hid tests locally I run into similar errors:
> > 
> >     ...
> >       CLNG-BPF hid.bpf.o
> >     In file included from progs/hid.c:6:
> >     progs/hid_bpf_helpers.h:9:38: error: declaration of 'struct hid_bpf_ctx' \
> >            will not be visible outside of this function [-Werror,-Wvisibility]
> >     extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
> >     ...
> > 
> > And there is indeed no `hid_bpf_ctx` in my vmlinux.h.
> > However, after enabling CONFIG_HID_BPF in kernel config the
> > `hid_bpf_ctx` appears in vmlinux.h, and I can compile HID selftests
> > w/o issues.
> 
> Even with enabling this configuration option I was unable to get clean
> builds of the HID selftests. I proposed a 4th patch on top of
> Benjamin's n=3 patch series here [1] using the #def/#undef pattern.

What are the remaining errors?
Could you please share your .config (e.g. as a gist).

As I said, when your kernel does not have `struct hid_bpf_ctx`,
sure you can define these data structures in the test itself,
but the test would loose it's meaning. If kernel is built
w/o HID BPF support there is no sense in compiling this test.

> 
> > 
> > > 
> > > ---
> > >  From bb9eccb7a896ba4b3a35ed12a248e6d6cfed2df6 Mon Sep 17 00:00:00 2001
> > > From: Benjamin Tissoires <bentiss@kernel.org>
> > > Date: Fri, 25 Aug 2023 10:02:32 +0200
> > > Subject: [PATCH] selftests/hid: ensure we can compile the tests on kernels
> > >   pre-6.3
> > > 
> > > For the hid-bpf tests to compile, we need to have the definition of
> > > struct hid_bpf_ctx. This definition is an internal one from the kernel
> > > and it is supposed to be defined in the generated vmlinux.h.
> > > 
> > > This vmlinux.h header is generated based on the currently running kernel
> > > or if the kernel was already compiled in the tree. If you just compile
> > > the selftests without compiling the kernel beforehand and you are running
> > > on a 6.2 kernel, you'll end up with a vmlinux.h without the hid_bpf_ctx
> > > definition.
> > > 
> > > Use the clever trick from tools/testing/selftests/bpf/progs/bpf_iter.h
> > > to force the definition of that symbol in case we don't find it in the
> > > BTF.
> > > 
> > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > > ---
> > >   tools/testing/selftests/hid/progs/hid.c       |  3 ---
> > >   .../selftests/hid/progs/hid_bpf_helpers.h     | 20 +++++++++++++++++++
> > >   2 files changed, 20 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c
> > > index 88c593f753b5..1e558826b809 100644
> > > --- a/tools/testing/selftests/hid/progs/hid.c
> > > +++ b/tools/testing/selftests/hid/progs/hid.c
> > > @@ -1,8 +1,5 @@
> > >   // SPDX-License-Identifier: GPL-2.0
> > >   /* Copyright (c) 2022 Red hat */
> > > -#include "vmlinux.h"
> > > -#include <bpf/bpf_helpers.h>
> > > -#include <bpf/bpf_tracing.h>
> > >   #include "hid_bpf_helpers.h"
> > > 
> > >   char _license[] SEC("license") = "GPL";
> > > diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > > index 4fff31dbe0e7..749097f8f4d9 100644
> > > --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > > +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h
> > > @@ -5,6 +5,26 @@
> > >   #ifndef __HID_BPF_HELPERS_H
> > >   #define __HID_BPF_HELPERS_H
> > > 
> > > +/* "undefine" structs in vmlinux.h, because we "override" them below */
> > > +#define hid_bpf_ctx hid_bpf_ctx___not_used
> > > +#include "vmlinux.h"
> > > +#undef hid_bpf_ctx
> > > +
> > > +#include <bpf/bpf_helpers.h>
> > > +#include <bpf/bpf_tracing.h>
> > > +
> > > +
> > > +struct hid_bpf_ctx {
> > > +     __u32 index;
> > > +     const struct hid_device *hid;
> > > +     __u32 allocated_size;
> > > +     enum hid_report_type report_type;
> > > +     union {
> > > +             __s32 retval;
> > > +             __s32 size;
> > > +     };
> > > +};
> > > +
> > >   /* following are kfuncs exported by HID for HID-BPF */
> > >   extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx,
> > >                             unsigned int offset,
> > 
> 
> [1]: https://lore.kernel.org/all/20230825182316.m2ksjoxe4s7dsapn@google.com/
> 
> Thanks
> Justin