diff mbox series

[v4,3/4] selftests/vDSO: Use KHDR_INCLUDES to locate UAPI headers for vdso_test_getrandom

Message ID 20240827132018.88854-4-xry111@xry111.site (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series LoongArch: Implement getrandom() in vDSO | expand

Commit Message

Xi Ruoyao Aug. 27, 2024, 1:20 p.m. UTC
Building test_vdso_getrandom currently leads to following issue:

    In file included from /home/xry111/git-repos/linux/tools/include/linux/compiler_types.h:36,
                     from /home/xry111/git-repos/linux/include/uapi/linux/stddef.h:5,
                     from /home/xry111/git-repos/linux/include/uapi/linux/posix_types.h:5,
                     from /usr/include/asm/sigcontext.h:12,
                     from /usr/include/bits/sigcontext.h:30,
                     from /usr/include/signal.h:301,
                     from vdso_test_getrandom.c:14:
    /home/xry111/git-repos/linux/tools/include/linux/compiler-gcc.h:3:2: error: #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
        3 | #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
          |  ^~~~~

It's because the compiler_types.h inclusion in
include/uapi/linux/stddef.h is expected to be removed by the
header_install.sh script, as compiler_types.h shouldn't be used from the
user space.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---
 tools/testing/selftests/vDSO/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jason A. Donenfeld Aug. 27, 2024, 1:58 p.m. UTC | #1
On Tue, Aug 27, 2024 at 09:20:16PM +0800, Xi Ruoyao wrote:
> Building test_vdso_getrandom currently leads to following issue:
> 
>     In file included from /home/xry111/git-repos/linux/tools/include/linux/compiler_types.h:36,
>                      from /home/xry111/git-repos/linux/include/uapi/linux/stddef.h:5,
>                      from /home/xry111/git-repos/linux/include/uapi/linux/posix_types.h:5,
>                      from /usr/include/asm/sigcontext.h:12,
>                      from /usr/include/bits/sigcontext.h:30,
>                      from /usr/include/signal.h:301,
>                      from vdso_test_getrandom.c:14:
>     /home/xry111/git-repos/linux/tools/include/linux/compiler-gcc.h:3:2: error: #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
>         3 | #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
>           |  ^~~~~
> 
> It's because the compiler_types.h inclusion in
> include/uapi/linux/stddef.h is expected to be removed by the
> header_install.sh script, as compiler_types.h shouldn't be used from the
> user space.
 
Hmm. If I run this on my current 6.10-based system, I get:

$ make
  CC       vdso_standalone_test_x86
  CC       vdso_test_getrandom
vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
   43 |         struct vgetrandom_opaque_params params;
      |                                         ^~~~~~

Because KHDR_INCLUDES is /usr/include instead.

Christophe, any suggestions on this one? And any idea why loongarch is
hitting it, but not x86 or ppc?

Jason
LEROY Christophe Aug. 27, 2024, 2:07 p.m. UTC | #2
Le 27/08/2024 à 15:58, Jason A. Donenfeld a écrit :
> On Tue, Aug 27, 2024 at 09:20:16PM +0800, Xi Ruoyao wrote:
>> Building test_vdso_getrandom currently leads to following issue:
>>
>>      In file included from /home/xry111/git-repos/linux/tools/include/linux/compiler_types.h:36,
>>                       from /home/xry111/git-repos/linux/include/uapi/linux/stddef.h:5,
>>                       from /home/xry111/git-repos/linux/include/uapi/linux/posix_types.h:5,
>>                       from /usr/include/asm/sigcontext.h:12,
>>                       from /usr/include/bits/sigcontext.h:30,
>>                       from /usr/include/signal.h:301,
>>                       from vdso_test_getrandom.c:14:
>>      /home/xry111/git-repos/linux/tools/include/linux/compiler-gcc.h:3:2: error: #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
>>          3 | #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
>>            |  ^~~~~
>>
>> It's because the compiler_types.h inclusion in
>> include/uapi/linux/stddef.h is expected to be removed by the
>> header_install.sh script, as compiler_types.h shouldn't be used from the
>> user space.
>   
> Hmm. If I run this on my current 6.10-based system, I get:
> 
> $ make
>    CC       vdso_standalone_test_x86
>    CC       vdso_test_getrandom
> vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
>     43 |         struct vgetrandom_opaque_params params;
>        |                                         ^~~~~~
> 
> Because KHDR_INCLUDES is /usr/include instead.
> 
> Christophe, any suggestions on this one? And any idea why loongarch is
> hitting it, but not x86 or ppc?
> 


Can you 'make clean' then provide the output of 'make V=1' ?

On powerpc I get:

$ make  ARCH=powerpc CROSS_COMPILE=ppc-linux- 
SODIUM="-I/tmp/sodium/usr/local/include/ -L/tmp/sodium/usr/local/lib/ 
-lsodium" V=1
ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_gettimeofday.c 
parse_vdso.c  -o 
/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_gettimeofday
ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_getcpu.c 
parse_vdso.c  -o 
/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_getcpu
ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_abi.c parse_vdso.c 
-o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_abi
ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_clock_getres.c  -o 
/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_clock_getres
ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE=  -ldl  vdso_test_correctness.c 
-o 
/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_correctness
ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE= -isystem 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include 
-isystem 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi 
    vdso_test_getrandom.c parse_vdso.c  -o 
/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_getrandom
ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE= -idirafter 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include 
-idirafter 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../arch/powerpc/include 
-idirafter 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../include 
-D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 
-Wa,--noexecstack -I/tmp/sodium/usr/local/include/ 
-L/tmp/sodium/usr/local/lib/ -lsodium    vdso_test_chacha.c 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/arch/powerpc/vdso/vgetrandom-chacha.S 
  -o 
/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_chacha
$

Thanks
Christophe
Jason A. Donenfeld Aug. 27, 2024, 2:15 p.m. UTC | #3
On Tue, Aug 27, 2024 at 02:07:59PM +0000, LEROY Christophe wrote:
> Le 27/08/2024 à 15:58, Jason A. Donenfeld a écrit :
> > On Tue, Aug 27, 2024 at 09:20:16PM +0800, Xi Ruoyao wrote:
> >> Building test_vdso_getrandom currently leads to following issue:
> >>
> >>      In file included from /home/xry111/git-repos/linux/tools/include/linux/compiler_types.h:36,
> >>                       from /home/xry111/git-repos/linux/include/uapi/linux/stddef.h:5,
> >>                       from /home/xry111/git-repos/linux/include/uapi/linux/posix_types.h:5,
> >>                       from /usr/include/asm/sigcontext.h:12,
> >>                       from /usr/include/bits/sigcontext.h:30,
> >>                       from /usr/include/signal.h:301,
> >>                       from vdso_test_getrandom.c:14:
> >>      /home/xry111/git-repos/linux/tools/include/linux/compiler-gcc.h:3:2: error: #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
> >>          3 | #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
> >>            |  ^~~~~
> >>
> >> It's because the compiler_types.h inclusion in
> >> include/uapi/linux/stddef.h is expected to be removed by the
> >> header_install.sh script, as compiler_types.h shouldn't be used from the
> >> user space.
> >   
> > Hmm. If I run this on my current 6.10-based system, I get:
> > 
> > $ make
> >    CC       vdso_standalone_test_x86
> >    CC       vdso_test_getrandom
> > vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
> >     43 |         struct vgetrandom_opaque_params params;
> >        |                                         ^~~~~~
> > 
> > Because KHDR_INCLUDES is /usr/include instead.
> > 
> > Christophe, any suggestions on this one? And any idea why loongarch is
> > hitting it, but not x86 or ppc?
> > 
> 
> 
> Can you 'make clean' then provide the output of 'make V=1' ?
 
With*out* this patch, the output is:

gcc -std=gnu99 -D_GNU_SOURCE= -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/include -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../include/uapi    vdso_test_getrandom.c parse_vdso.c  -o /home/zx2c4/Projects/random-linux/tools/testing/selftests/vDSO/vdso_test_getrandom

*With* this patch, the output is:

gcc -std=gnu99 -D_GNU_SOURCE= -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/include -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include    vdso_test_getrandom.c parse_vdso.c  -o /home/zx2c4/Projects/random-linux/tools/testing/selftests/vDSO/vdso_test_getrandom
vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
   43 |         struct vgetrandom_opaque_params params;
      |                                         ^~~~~~

$ ls /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include
headers_check.pl  Makefile

Since I don't build in there, this directory is empty.

Jason
Xi Ruoyao Aug. 27, 2024, 2:41 p.m. UTC | #4
On Tue, 2024-08-27 at 16:15 +0200, Jason A. Donenfeld wrote:

/* snip */

> gcc -std=gnu99 -D_GNU_SOURCE= -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/include -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include    vdso_test_getrandom.c parse_vdso.c  -o /home/zx2c4/Projects/random-linux/tools/testing/selftests/vDSO/vdso_test_getrandom
> vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
>    43 |         struct vgetrandom_opaque_params params;
>       |                                         ^~~~~~
> 
> $ ls /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include
> headers_check.pl  Makefile
> 
> Since I don't build in there, this directory is empty.

In the toplevel Makefile:

kselftest-%: headers FORCE
    $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*

So running "make kselftest-all" to build the self tests should have
already caused make to build the "headers" target, which puts the
headers into usr/include.

I don't think it's supported to build self tests w/o invoking the
toplevel Makefile: many other self tests use KHDR_INCLUDES as well, so
generally building with something like "make -C tools/testing/selftests"
just won't work.
Christophe Leroy Aug. 27, 2024, 2:50 p.m. UTC | #5
Le 27/08/2024 à 16:41, Xi Ruoyao a écrit :
> On Tue, 2024-08-27 at 16:15 +0200, Jason A. Donenfeld wrote:
> 
> /* snip */
> 
>> gcc -std=gnu99 -D_GNU_SOURCE= -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/include -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include    vdso_test_getrandom.c parse_vdso.c  -o /home/zx2c4/Projects/random-linux/tools/testing/selftests/vDSO/vdso_test_getrandom
>> vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
>>     43 |         struct vgetrandom_opaque_params params;
>>        |                                         ^~~~~~
>>
>> $ ls /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include
>> headers_check.pl  Makefile
>>
>> Since I don't build in there, this directory is empty.
> 
> In the toplevel Makefile:
> 
> kselftest-%: headers FORCE
>      $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
> 
> So running "make kselftest-all" to build the self tests should have
> already caused make to build the "headers" target, which puts the
> headers into usr/include.
> 
> I don't think it's supported to build self tests w/o invoking the
> toplevel Makefile: many other self tests use KHDR_INCLUDES as well, so
> generally building with something like "make -C tools/testing/selftests"
> just won't work.
> 

My usr/include/ is also empty (only Makefile and headers_check.pl) and 
building directly in tools/testing/selftests/vDSO works for me.

The command is:

ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE= -isystem 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include 
-isystem 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi 
    vdso_test_getrandom.c parse_vdso.c  -o 
/home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_getrandom

I believe I get the needed headers through : -isystem 
/home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi

Christophe

PS: By the way, did you see the -DBULID_VDSO for the chacha test ? Don't 
know the impact though ....
Jason A. Donenfeld Aug. 27, 2024, 3 p.m. UTC | #6
On Tue, Aug 27, 2024 at 04:50:59PM +0200, Christophe Leroy wrote:
> 
> 
> Le 27/08/2024 à 16:41, Xi Ruoyao a écrit :
> > On Tue, 2024-08-27 at 16:15 +0200, Jason A. Donenfeld wrote:
> > 
> > /* snip */
> > 
> >> gcc -std=gnu99 -D_GNU_SOURCE= -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/include -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include    vdso_test_getrandom.c parse_vdso.c  -o /home/zx2c4/Projects/random-linux/tools/testing/selftests/vDSO/vdso_test_getrandom
> >> vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
> >>     43 |         struct vgetrandom_opaque_params params;
> >>        |                                         ^~~~~~
> >>
> >> $ ls /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include
> >> headers_check.pl  Makefile
> >>
> >> Since I don't build in there, this directory is empty.
> > 
> > In the toplevel Makefile:
> > 
> > kselftest-%: headers FORCE
> >      $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
> > 
> > So running "make kselftest-all" to build the self tests should have
> > already caused make to build the "headers" target, which puts the
> > headers into usr/include.
> > 
> > I don't think it's supported to build self tests w/o invoking the
> > toplevel Makefile: many other self tests use KHDR_INCLUDES as well, so
> > generally building with something like "make -C tools/testing/selftests"
> > just won't work.
> > 
> 
> My usr/include/ is also empty (only Makefile and headers_check.pl) and 
> building directly in tools/testing/selftests/vDSO works for me.
> 
> The command is:
> 
> ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE= -isystem 
> /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include 
> -isystem 
> /home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi 
>     vdso_test_getrandom.c parse_vdso.c  -o 
> /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_getrandom
> 
> I believe I get the needed headers through : -isystem 
> /home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi

The effect of this patch is to replace include/uapi with usr/include, so
it will break for you too.

What I'm wondering is why yours and mine work like that, while Ruoyao's
breaks. He makes a good argument as to why this patch is the "right
way", even if it breaks our workflow...

> 
> Christophe
> 
> PS: By the way, did you see the -DBULID_VDSO for the chacha test ? Don't 
> know the impact though ....

Yes and https://lore.kernel.org/all/20240827145454.3317093-1-Jason@zx2c4.com/
Xi Ruoyao Aug. 27, 2024, 3:05 p.m. UTC | #7
On Tue, 2024-08-27 at 17:00 +0200, Jason A. Donenfeld wrote:
> The effect of this patch is to replace include/uapi with usr/include, so
> it will break for you too.
> 
> What I'm wondering is why yours and mine work like that, while Ruoyao's
> breaks. He makes a good argument as to why this patch is the "right
> way", even if it breaks our workflow...

Because arch/loongarch/include/uapi/asm/sigcontext.h includes
<linux/posix_types.h>, but the files for x86 and ppc do not.

I cannot see how this inclusion is useful anyway, so maybe I can just
remove the inclusion and paper over the real issue for now?
Jason A. Donenfeld Aug. 27, 2024, 3:10 p.m. UTC | #8
On Tue, Aug 27, 2024 at 11:05:14PM +0800, Xi Ruoyao wrote:
> On Tue, 2024-08-27 at 17:00 +0200, Jason A. Donenfeld wrote:
> > The effect of this patch is to replace include/uapi with usr/include, so
> > it will break for you too.
> > 
> > What I'm wondering is why yours and mine work like that, while Ruoyao's
> > breaks. He makes a good argument as to why this patch is the "right
> > way", even if it breaks our workflow...
> 
> Because arch/loongarch/include/uapi/asm/sigcontext.h includes
> <linux/posix_types.h>, but the files for x86 and ppc do not.
> 
> I cannot see how this inclusion is useful anyway, so maybe I can just
> remove the inclusion and paper over the real issue for now?

The kselftest people might disagree with papering it over and may prefer
your patch, but your solution does sound better to me...
Christophe Leroy Aug. 27, 2024, 3:12 p.m. UTC | #9
Le 27/08/2024 à 17:00, Jason A. Donenfeld a écrit :
> On Tue, Aug 27, 2024 at 04:50:59PM +0200, Christophe Leroy wrote:
>>
>>
>> Le 27/08/2024 à 16:41, Xi Ruoyao a écrit :
>>> On Tue, 2024-08-27 at 16:15 +0200, Jason A. Donenfeld wrote:
>>>
>>> /* snip */
>>>
>>>> gcc -std=gnu99 -D_GNU_SOURCE= -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../tools/include -isystem /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include    vdso_test_getrandom.c parse_vdso.c  -o /home/zx2c4/Projects/random-linux/tools/testing/selftests/vDSO/vdso_test_getrandom
>>>> vdso_test_getrandom.c:43:41: error: field ‘params’ has incomplete type
>>>>      43 |         struct vgetrandom_opaque_params params;
>>>>         |                                         ^~~~~~
>>>>
>>>> $ ls /home/zx2c4/Projects/random-linux/tools/testing/selftests/../../../usr/include
>>>> headers_check.pl  Makefile
>>>>
>>>> Since I don't build in there, this directory is empty.
>>>
>>> In the toplevel Makefile:
>>>
>>> kselftest-%: headers FORCE
>>>       $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
>>>
>>> So running "make kselftest-all" to build the self tests should have
>>> already caused make to build the "headers" target, which puts the
>>> headers into usr/include.
>>>
>>> I don't think it's supported to build self tests w/o invoking the
>>> toplevel Makefile: many other self tests use KHDR_INCLUDES as well, so
>>> generally building with something like "make -C tools/testing/selftests"
>>> just won't work.
>>>
>>
>> My usr/include/ is also empty (only Makefile and headers_check.pl) and
>> building directly in tools/testing/selftests/vDSO works for me.
>>
>> The command is:
>>
>> ppc-linux-gcc -std=gnu99 -D_GNU_SOURCE= -isystem
>> /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include
>> -isystem
>> /home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi
>>      vdso_test_getrandom.c parse_vdso.c  -o
>> /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_getrandom
>>
>> I believe I get the needed headers through : -isystem
>> /home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi
> 
> The effect of this patch is to replace include/uapi with usr/include, so
> it will break for you too.
> 
> What I'm wondering is why yours and mine work like that, while Ruoyao's
> breaks. He makes a good argument as to why this patch is the "right
> way", even if it breaks our workflow...

Ah yes he is probably right.

Then I'll have to first do:

make CROSS_COMPILE=powerpc64-linux- ARCH=powerpc headers

Then everything should be fine.

Christophe

> 
>>
>> Christophe
>>
>> PS: By the way, did you see the -DBULID_VDSO for the chacha test ? Don't
>> know the impact though ....
> 
> Yes and https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20240827145454.3317093-1-Jason%40zx2c4.com%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C627846cc0a0e429e45c508dcc6a90690%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638603676502990226%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=H7PPFv8QGHsb9xh3J%2FzyeFrpvDu2uSKqx4ZwrPNwC2s%3D&reserved=0
Xi Ruoyao Aug. 27, 2024, 3:28 p.m. UTC | #10
On Tue, 2024-08-27 at 17:10 +0200, Jason A. Donenfeld wrote:
> On Tue, Aug 27, 2024 at 11:05:14PM +0800, Xi Ruoyao wrote:
> > On Tue, 2024-08-27 at 17:00 +0200, Jason A. Donenfeld wrote:
> > > The effect of this patch is to replace include/uapi with usr/include, so
> > > it will break for you too.
> > > 
> > > What I'm wondering is why yours and mine work like that, while Ruoyao's
> > > breaks. He makes a good argument as to why this patch is the "right
> > > way", even if it breaks our workflow...
> > 
> > Because arch/loongarch/include/uapi/asm/sigcontext.h includes
> > <linux/posix_types.h>, but the files for x86 and ppc do not.
> > 
> > I cannot see how this inclusion is useful anyway, so maybe I can just
> > remove the inclusion and paper over the real issue for now?
> 
> The kselftest people might disagree with papering it over and may prefer
> your patch, but your solution does sound better to me...

Oops the papering over does not really work because the compiler picks
up the sigcontext.h already installed to /usr/include/asm/sigcontext.h.
So, if we want to use the "updated" version of sigcontext.h, we still
have to add KHDR_INCLUDES to pick up
kernel-tree/usr/include/asm/sigcontext.h instead.

Or, I can add $(KHDR_INCLUDES) but also keep
-isystem $(top_srcdir)/include/uapi, so "make -C tools/testing/selftests
TARGETS=vDSO" will still (happens to) work on x86 and ppc (without
headers in kernel-tree/usr).

If you agree I'll use this approach in v5.
Jason A. Donenfeld Aug. 27, 2024, 3:29 p.m. UTC | #11
On Tue, Aug 27, 2024 at 5:29 PM Xi Ruoyao <xry111@xry111.site> wrote:
> Or, I can add $(KHDR_INCLUDES) but also keep
> -isystem $(top_srcdir)/include/uapi, so "make -C tools/testing/selftests
> TARGETS=vDSO" will still (happens to) work on x86 and ppc (without
> headers in kernel-tree/usr).

Oh, the porquenolosdos solution. That sounds good to me.

Jason
Jason A. Donenfeld Aug. 28, 2024, 11:36 a.m. UTC | #12
On Tue, Aug 27, 2024 at 05:29:56PM +0200, Jason A. Donenfeld wrote:
> On Tue, Aug 27, 2024 at 5:29 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > Or, I can add $(KHDR_INCLUDES) but also keep
> > -isystem $(top_srcdir)/include/uapi, so "make -C tools/testing/selftests
> > TARGETS=vDSO" will still (happens to) work on x86 and ppc (without
> > headers in kernel-tree/usr).
> 
> Oh, the porquenolosdos solution. That sounds good to me.


Does the below work for you?

From 0a769491e0193cdf9728a23d02be5e6be975b513 Mon Sep 17 00:00:00 2001
From: Xi Ruoyao <xry111@xry111.site>
Date: Wed, 28 Aug 2024 13:29:57 +0200
Subject: [PATCH] selftests/vDSO: use KHDR_INCLUDES to locate UAPI headers for
 vdso_test_getrandom

Building test_vdso_getrandom currently leads to following issue:

    In file included from /home/xry111/git-repos/linux/tools/include/linux/compiler_types.h:36,
                     from /home/xry111/git-repos/linux/include/uapi/linux/stddef.h:5,
                     from /home/xry111/git-repos/linux/include/uapi/linux/posix_types.h:5,
                     from /usr/include/asm/sigcontext.h:12,
                     from /usr/include/bits/sigcontext.h:30,
                     from /usr/include/signal.h:301,
                     from vdso_test_getrandom.c:14:
    /home/xry111/git-repos/linux/tools/include/linux/compiler-gcc.h:3:2: error: #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
        3 | #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
          |  ^~~~~

It's because the compiler_types.h inclusion in
include/uapi/linux/stddef.h is expected to be removed by the
header_install.sh script, as compiler_types.h shouldn't be used from
user space.

Add KHDR_INCLUDES before the existing include/uapi inclusion so that
usr/include takes precedence if it's populated.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/vDSO/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index d1452c7d6d4f..13a626ef64f7 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -39,6 +39,7 @@ $(OUTPUT)/vdso_test_correctness: LDFLAGS += -ldl

 $(OUTPUT)/vdso_test_getrandom: parse_vdso.c
 $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
+					 $(KHDR_INCLUDES) \
                                          -isystem $(top_srcdir)/include/uapi

 $(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
--
2.46.0
Xi Ruoyao Aug. 28, 2024, noon UTC | #13
On Wed, 2024-08-28 at 13:36 +0200, Jason A. Donenfeld wrote:
> On Tue, Aug 27, 2024 at 05:29:56PM +0200, Jason A. Donenfeld wrote:
> > On Tue, Aug 27, 2024 at 5:29 PM Xi Ruoyao <xry111@xry111.site> wrote:
> > > Or, I can add $(KHDR_INCLUDES) but also keep
> > > -isystem $(top_srcdir)/include/uapi, so "make -C tools/testing/selftests
> > > TARGETS=vDSO" will still (happens to) work on x86 and ppc (without
> > > headers in kernel-tree/usr).
> > 
> > Oh, the porquenolosdos solution. That sounds good to me.
> 
> 
> Does the below work for you?

Yes, it works.

> From 0a769491e0193cdf9728a23d02be5e6be975b513 Mon Sep 17 00:00:00 2001
> From: Xi Ruoyao <xry111@xry111.site>
> Date: Wed, 28 Aug 2024 13:29:57 +0200
> Subject: [PATCH] selftests/vDSO: use KHDR_INCLUDES to locate UAPI headers for
>  vdso_test_getrandom
> 
> Building test_vdso_getrandom currently leads to following issue:
> 
>     In file included from /home/xry111/git-repos/linux/tools/include/linux/compiler_types.h:36,
>                      from /home/xry111/git-repos/linux/include/uapi/linux/stddef.h:5,
>                      from /home/xry111/git-repos/linux/include/uapi/linux/posix_types.h:5,
>                      from /usr/include/asm/sigcontext.h:12,
>                      from /usr/include/bits/sigcontext.h:30,
>                      from /usr/include/signal.h:301,
>                      from vdso_test_getrandom.c:14:
>     /home/xry111/git-repos/linux/tools/include/linux/compiler-gcc.h:3:2: error: #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
>         3 | #error "Please don't include <linux/compiler-gcc.h> directly, include <linux/compiler.h> instead."
>           |  ^~~~~
> 
> It's because the compiler_types.h inclusion in
> include/uapi/linux/stddef.h is expected to be removed by the
> header_install.sh script, as compiler_types.h shouldn't be used from
> user space.
> 
> Add KHDR_INCLUDES before the existing include/uapi inclusion so that
> usr/include takes precedence if it's populated.
> 
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  tools/testing/selftests/vDSO/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> index d1452c7d6d4f..13a626ef64f7 100644
> --- a/tools/testing/selftests/vDSO/Makefile
> +++ b/tools/testing/selftests/vDSO/Makefile
> @@ -39,6 +39,7 @@ $(OUTPUT)/vdso_test_correctness: LDFLAGS += -ldl
> 
>  $(OUTPUT)/vdso_test_getrandom: parse_vdso.c
>  $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
> +					 $(KHDR_INCLUDES) \
>                                           -isystem $(top_srcdir)/include/uapi
> 
>  $(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
> --
> 2.46.0
> 
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 180854eb9fec..e346890d4c91 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -38,7 +38,7 @@  $(OUTPUT)/vdso_test_correctness: LDFLAGS += -ldl
 
 $(OUTPUT)/vdso_test_getrandom: parse_vdso.c
 $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
-                                         -isystem $(top_srcdir)/include/uapi
+                                         $(KHDR_INCLUDES)
 
 $(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
 $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \