* [PATCH v3 0/4] Compile-test UAPI and kernel headers
@ 2019-06-27 16:38 Masahiro Yamada
2019-06-27 16:38 ` [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained Masahiro Yamada
2019-06-28 4:57 ` [PATCH v3 0/4] Compile-test UAPI and kernel headers Masahiro Yamada
0 siblings, 2 replies; 6+ messages in thread
From: Masahiro Yamada @ 2019-06-27 16:38 UTC (permalink / raw)
To: linux-kbuild
Cc: Song Liu, Jakub Kicinski, linux-doc, Palmer Dabbelt,
Alexei Starovoitov, Masahiro Yamada, linux-riscv, Sam Ravnborg,
Kees Cook, xdp-newbies, Daniel Borkmann, Jonathan Corbet,
Anton Vorontsov, John Fastabend, Yonghong Song, Albert Ou,
Jesper Dangaard Brouer, Jani Nikula, Michal Marek,
linux-mediatek, Matthias Brugger, linux-arm-kernel, Tony Luck,
netdev, linux-kernel, David S. Miller, Colin Cross, bpf,
Martin KaFai Lau
1/4: Compile-test exported headers (reworked in v2)
2/4: fix a flaw I noticed when I was working on this series.
Avoid generating intermediate wrappers.
3/4: maybe useful for 4/4 and in some other places.
Add header-test-pattern-y syntax.
4/4: Compile-test kernel-space headers in include/.
v2: compile as many headers as possible.
v3: exclude more headers causing build errors
Masahiro Yamada (4):
kbuild: compile-test UAPI headers to ensure they are self-contained
kbuild: do not create wrappers for header-test-y
kbuild: support header-test-pattern-y
kbuild: compile-test kernel headers to ensure they are self-contained
.gitignore | 1 -
Documentation/dontdiff | 1 -
Documentation/kbuild/makefiles.txt | 13 +-
Makefile | 4 +-
include/Kbuild | 1250 ++++++++++++++++++++++++++++
init/Kconfig | 22 +
scripts/Makefile.build | 10 +-
scripts/Makefile.lib | 13 +-
scripts/cc-system-headers.sh | 8 +
usr/.gitignore | 1 -
usr/Makefile | 2 +
usr/include/.gitignore | 3 +
usr/include/Makefile | 134 +++
13 files changed, 1449 insertions(+), 13 deletions(-)
create mode 100644 include/Kbuild
create mode 100755 scripts/cc-system-headers.sh
create mode 100644 usr/include/.gitignore
create mode 100644 usr/include/Makefile
--
2.17.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained 2019-06-27 16:38 [PATCH v3 0/4] Compile-test UAPI and kernel headers Masahiro Yamada @ 2019-06-27 16:38 ` Masahiro Yamada 2019-06-28 15:43 ` Sam Ravnborg 2019-06-30 1:35 ` Masahiro Yamada 2019-06-28 4:57 ` [PATCH v3 0/4] Compile-test UAPI and kernel headers Masahiro Yamada 1 sibling, 2 replies; 6+ messages in thread From: Masahiro Yamada @ 2019-06-27 16:38 UTC (permalink / raw) To: linux-kbuild Cc: Song Liu, Michal Marek, bpf, Daniel Borkmann, Jani Nikula, netdev, Palmer Dabbelt, Alexei Starovoitov, linux-kernel, Masahiro Yamada, Albert Ou, Yonghong Song, linux-riscv, Sam Ravnborg, Martin KaFai Lau Multiple people have suggested compile-testing UAPI headers to ensure they can be really included from user-space. "make headers_check" is obviously not enough to catch bugs, and we often leak references to kernel-space definition to user-space. Use the new header-test-y syntax to implement it. Please note exported headers are compile-tested with a completely different set of compiler flags. The header search path is set to $(objtree)/usr/include since exported headers should not include unexported ones. We use -std=gnu89 for the kernel space since the kernel code highly depends on GNU extensions. On the other hand, UAPI headers should be written in more standardized C, so they are compiled with -std=c90. This will emit errors if C++ style comments, the keyword 'inline', etc. are used. Please use C style comments (/* ... */), '__inline__', etc. in UAPI headers. There is additional compiler requirement to enable this test because many of UAPI headers include <stdlib.h>, <sys/ioctl.h>, <sys/time.h>, etc. directly or indirectly. You cannot use kernel.org pre-built toolchains [1] since they lack <stdlib.h>. I added scripts/cc-system-headers.sh to check the system header availability, which CONFIG_UAPI_HEADER_TEST depends on. For now, a lot of headers need to be excluded because they cannot be compiled standalone, but this is a good start point. [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/index.html Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v3: None Changes in v2: - Add CONFIG_CPU_{BIG,LITTLE}_ENDIAN guard to avoid build error - Use 'header-test-' instead of 'no-header-test' - Avoid weird 'find' warning when cleaning Makefile | 2 +- init/Kconfig | 11 +++ scripts/cc-system-headers.sh | 8 +++ usr/.gitignore | 1 - usr/Makefile | 2 + usr/include/.gitignore | 3 + usr/include/Makefile | 134 +++++++++++++++++++++++++++++++++++ 7 files changed, 159 insertions(+), 2 deletions(-) create mode 100755 scripts/cc-system-headers.sh create mode 100644 usr/include/.gitignore create mode 100644 usr/include/Makefile diff --git a/Makefile b/Makefile index 1f35aca4fe05..f23516980796 100644 --- a/Makefile +++ b/Makefile @@ -1363,7 +1363,7 @@ CLEAN_DIRS += $(MODVERDIR) include/ksym CLEAN_FILES += modules.builtin.modinfo # Directories & files removed with 'make mrproper' -MRPROPER_DIRS += include/config usr/include include/generated \ +MRPROPER_DIRS += include/config include/generated \ arch/$(SRCARCH)/include/generated .tmp_objdiff MRPROPER_FILES += .config .config.old .version \ Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \ diff --git a/init/Kconfig b/init/Kconfig index df5bba27e3fe..667d68e1cdf4 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -105,6 +105,17 @@ config HEADER_TEST If you are a developer or tester and want to ensure the requested headers are self-contained, say Y here. Otherwise, choose N. +config UAPI_HEADER_TEST + bool "Compile test UAPI headers" + depends on HEADER_TEST && HEADERS_INSTALL + depends on $(success,$(srctree)/scripts/cc-system-headers.sh $(CC)) + help + Compile test headers exported to user-space to ensure they are + self-contained, i.e. compilable as standalone units. + + If you are a developer or tester and want to ensure the exported + headers are self-contained, say Y here. Otherwise, choose N. + config LOCALVERSION string "Local version - append to kernel release" help diff --git a/scripts/cc-system-headers.sh b/scripts/cc-system-headers.sh new file mode 100755 index 000000000000..1b3db369828c --- /dev/null +++ b/scripts/cc-system-headers.sh @@ -0,0 +1,8 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0-only + +cat << "END" | $@ -E -x c - -o /dev/null >/dev/null 2>&1 +#include <stdlib.h> +#include <sys/ioctl.h> +#include <sys/time.h> +END diff --git a/usr/.gitignore b/usr/.gitignore index 8e48117a3f3d..be5eae1df7eb 100644 --- a/usr/.gitignore +++ b/usr/.gitignore @@ -7,4 +7,3 @@ initramfs_data.cpio.gz initramfs_data.cpio.bz2 initramfs_data.cpio.lzma initramfs_list -include diff --git a/usr/Makefile b/usr/Makefile index 4a70ae43c9cb..6a89eb019275 100644 --- a/usr/Makefile +++ b/usr/Makefile @@ -56,3 +56,5 @@ $(deps_initramfs): klibcdirs $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y) $(call if_changed,initfs) + +subdir-$(CONFIG_UAPI_HEADER_TEST) += include diff --git a/usr/include/.gitignore b/usr/include/.gitignore new file mode 100644 index 000000000000..a0991ff4402b --- /dev/null +++ b/usr/include/.gitignore @@ -0,0 +1,3 @@ +* +!.gitignore +!Makefile diff --git a/usr/include/Makefile b/usr/include/Makefile new file mode 100644 index 000000000000..58ce96fa1701 --- /dev/null +++ b/usr/include/Makefile @@ -0,0 +1,134 @@ +# SPDX-License-Identifier: GPL-2.0-only + +# Unlike the kernel space, uapi headers are written in standard C. +# - Forbid C++ style comments +# - Use '__inline__', '__asm__' instead of 'inline', 'asm' +# +# -std=c90 (equivalent to -ansi) catches the violation of those. +# We cannot go as far as adding -Wpedantic since it emits too many warnings. +# +# REVISIT: re-consider the proper set of compiler flags for uapi compile-test. + +UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration + +override c_flags = $(UAPI_CFLAGS) -Wp,-MD,$(depfile) -I$(objtree)/usr/include + +# The following are excluded for now because they fail to build. +# The cause of errors are mostly missing include directives. +# Check one by one, and send a patch to each subsystem. +# +# Do not add a new header to the blacklist without legitimate reason. +# Please consider to fix the header first. +header-test- += asm/ipcbuf.h +header-test- += asm/msgbuf.h +header-test- += asm/sembuf.h +header-test- += asm/shmbuf.h +header-test- += asm/signal.h +header-test- += asm/ucontext.h +header-test- += drm/vmwgfx_drm.h +header-test- += linux/am437x-vpfe.h +header-test- += linux/android/binderfs.h +header-test- += linux/android/binder.h +header-test-$(CONFIG_CPU_BIG_ENDIAN) += linux/byteorder/big_endian.h +header-test-$(CONFIG_CPU_LITTLE_ENDIAN) += linux/byteorder/little_endian.h +header-test- += linux/coda.h +header-test- += linux/coda_psdev.h +header-test- += linux/dvb/audio.h +header-test- += linux/dvb/osd.h +header-test- += linux/elfcore.h +header-test- += linux/errqueue.h +header-test- += linux/fsmap.h +header-test- += linux/hdlc/ioctl.h +header-test- += linux/jffs2.h +header-test- += linux/kexec.h +header-test- += linux/matroxfb.h +header-test- += linux/netfilter_bridge/ebtables.h +header-test- += linux/netfilter_ipv4/ipt_LOG.h +header-test- += linux/netfilter_ipv6/ip6t_LOG.h +header-test- += linux/nfc.h +header-test- += linux/nilfs2_ondisk.h +header-test- += linux/omap3isp.h +header-test- += linux/omapfb.h +header-test- += linux/patchkey.h +header-test- += linux/phonet.h +header-test- += linux/reiserfs_xattr.h +header-test- += linux/scc.h +header-test- += linux/sctp.h +header-test- += linux/signal.h +header-test- += linux/sysctl.h +header-test- += linux/usb/audio.h +header-test- += linux/ivtv.h +header-test- += linux/v4l2-mediabus.h +header-test- += linux/v4l2-subdev.h +header-test- += linux/videodev2.h +header-test- += linux/vm_sockets.h +header-test- += misc/ocxl.h +header-test- += mtd/mtd-abi.h +header-test- += scsi/scsi_bsg_fc.h +header-test- += scsi/scsi_netlink_fc.h +header-test- += scsi/scsi_netlink.h +header-test- += sound/asequencer.h +header-test- += sound/asound.h +header-test- += sound/asoc.h +header-test- += sound/compress_offload.h +header-test- += sound/emu10k1.h +header-test- += sound/sfnt_info.h +header-test- += sound/sof/eq.h +header-test- += sound/sof/fw.h +header-test- += sound/sof/header.h +header-test- += sound/sof/manifest.h +header-test- += sound/sof/trace.h +header-test- += xen/evtchn.h +header-test- += xen/gntdev.h +header-test- += xen/privcmd.h + +# more headers are broken in some architectures + +ifeq ($(SRCARCH),arc) +header-test- += linux/bpf_perf_event.h +endif + +ifeq ($(SRCARCH),ia64) +header-test- += asm/setup.h +header-test- += asm/sigcontext.h +header-test- += asm/perfmon.h +header-test- += asm/perfmon_default_smpl.h +header-test- += linux/if_bonding.h +endif + +ifeq ($(SRCARCH),mips) +header-test- += asm/stat.h +endif + +ifeq ($(SRCARCH),powerpc) +header-test- += asm/stat.h +header-test- += linux/bpf_perf_event.h +endif + +ifeq ($(SRCARCH),riscv) +header-test- += linux/bpf_perf_event.h +endif + +ifeq ($(SRCARCH),s390) +header-test- += asm/runtime_instr.h +header-test- += asm/zcrypt.h +endif + +ifeq ($(SRCARCH),sparc) +header-test- += asm/stat.h +header-test- += asm/uctx.h +header-test- += asm/fbio.h +header-test- += asm/openpromio.h +endif + +# asm-generic/*.h is used by asm/*.h, and should not be included directly +header-test- += asm-generic/% + +# The rest are compile-tested +header-test-y += $(filter-out $(header-test-), \ + $(patsubst $(obj)/%,%, $(wildcard \ + $(addprefix $(obj)/, *.h */*.h */*/*.h */*/*/*.h)))) + +# For GNU Make <= 4.2.1, $(wildcard $(obj)/*/) matches to not only directories +# but also regular files. Use $(filter %/, ...) just in case. +clean-dirs += $(patsubst $(obj)/%/,%,$(filter %/, $(wildcard $(obj)/*/))) -- 2.17.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained 2019-06-27 16:38 ` [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained Masahiro Yamada @ 2019-06-28 15:43 ` Sam Ravnborg 2019-06-30 1:27 ` Masahiro Yamada 2019-06-30 1:35 ` Masahiro Yamada 1 sibling, 1 reply; 6+ messages in thread From: Sam Ravnborg @ 2019-06-28 15:43 UTC (permalink / raw) To: Masahiro Yamada Cc: Song Liu, Michal Marek, bpf, Daniel Borkmann, linux-kbuild, Jani Nikula, netdev, Palmer Dabbelt, Alexei Starovoitov, linux-kernel, Albert Ou, Yonghong Song, linux-riscv, Martin KaFai Lau Hi Masahiro. On Fri, Jun 28, 2019 at 01:38:59AM +0900, Masahiro Yamada wrote: > Multiple people have suggested compile-testing UAPI headers to ensure > they can be really included from user-space. "make headers_check" is > obviously not enough to catch bugs, and we often leak references to > kernel-space definition to user-space. > > Use the new header-test-y syntax to implement it. Please note exported > headers are compile-tested with a completely different set of compiler > flags. The header search path is set to $(objtree)/usr/include since > exported headers should not include unexported ones. This patchset introduce a new set of tests for uapi headers. Can we somehow consolidate so we have only one way to verify the uapi headers? It can be confusing for users that they see errors from testing the uapi headers during normal build and a new class or error if they run a "make headers_check" sometimes later. This can be a next step to consolidate this. With the suggestions below considered you can add my: Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > We use -std=gnu89 for the kernel space since the kernel code highly > depends on GNU extensions. On the other hand, UAPI headers should be > written in more standardized C, so they are compiled with -std=c90. > This will emit errors if C++ style comments, the keyword 'inline', etc. > are used. Please use C style comments (/* ... */), '__inline__', etc. > in UAPI headers. > > There is additional compiler requirement to enable this test because > many of UAPI headers include <stdlib.h>, <sys/ioctl.h>, <sys/time.h>, > etc. directly or indirectly. You cannot use kernel.org pre-built > toolchains [1] since they lack <stdlib.h>. > > I added scripts/cc-system-headers.sh to check the system header > availability, which CONFIG_UAPI_HEADER_TEST depends on. > > For now, a lot of headers need to be excluded because they cannot > be compiled standalone, but this is a good start point. > > [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/index.html > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > Changes in v3: None > Changes in v2: > - Add CONFIG_CPU_{BIG,LITTLE}_ENDIAN guard to avoid build error > - Use 'header-test-' instead of 'no-header-test' > - Avoid weird 'find' warning when cleaning > > Makefile | 2 +- > init/Kconfig | 11 +++ > scripts/cc-system-headers.sh | 8 +++ > usr/.gitignore | 1 - > usr/Makefile | 2 + > usr/include/.gitignore | 3 + > usr/include/Makefile | 134 +++++++++++++++++++++++++++++++++++ > 7 files changed, 159 insertions(+), 2 deletions(-) > create mode 100755 scripts/cc-system-headers.sh > create mode 100644 usr/include/.gitignore > create mode 100644 usr/include/Makefile > > diff --git a/Makefile b/Makefile > index 1f35aca4fe05..f23516980796 100644 > --- a/Makefile > +++ b/Makefile > @@ -1363,7 +1363,7 @@ CLEAN_DIRS += $(MODVERDIR) include/ksym > CLEAN_FILES += modules.builtin.modinfo > > # Directories & files removed with 'make mrproper' > -MRPROPER_DIRS += include/config usr/include include/generated \ > +MRPROPER_DIRS += include/config include/generated \ > arch/$(SRCARCH)/include/generated .tmp_objdiff > MRPROPER_FILES += .config .config.old .version \ > Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \ > diff --git a/init/Kconfig b/init/Kconfig > index df5bba27e3fe..667d68e1cdf4 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -105,6 +105,17 @@ config HEADER_TEST > If you are a developer or tester and want to ensure the requested > headers are self-contained, say Y here. Otherwise, choose N. > > +config UAPI_HEADER_TEST > + bool "Compile test UAPI headers" > + depends on HEADER_TEST && HEADERS_INSTALL > + depends on $(success,$(srctree)/scripts/cc-system-headers.sh $(CC)) > + help > + Compile test headers exported to user-space to ensure they are > + self-contained, i.e. compilable as standalone units. > + > + If you are a developer or tester and want to ensure the exported > + headers are self-contained, say Y here. Otherwise, choose N. > + > config LOCALVERSION > string "Local version - append to kernel release" > help > diff --git a/scripts/cc-system-headers.sh b/scripts/cc-system-headers.sh > new file mode 100755 > index 000000000000..1b3db369828c > --- /dev/null > +++ b/scripts/cc-system-headers.sh > @@ -0,0 +1,8 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0-only > + > +cat << "END" | $@ -E -x c - -o /dev/null >/dev/null 2>&1 > +#include <stdlib.h> > +#include <sys/ioctl.h> > +#include <sys/time.h> > +END Add comment to this file that explains that it is used to verify that the toolchain provides the minimal set of header-files required by uapi headers. > diff --git a/usr/.gitignore b/usr/.gitignore > index 8e48117a3f3d..be5eae1df7eb 100644 > --- a/usr/.gitignore > +++ b/usr/.gitignore > @@ -7,4 +7,3 @@ initramfs_data.cpio.gz > initramfs_data.cpio.bz2 > initramfs_data.cpio.lzma > initramfs_list > -include > diff --git a/usr/Makefile b/usr/Makefile > index 4a70ae43c9cb..6a89eb019275 100644 > --- a/usr/Makefile > +++ b/usr/Makefile > @@ -56,3 +56,5 @@ $(deps_initramfs): klibcdirs > $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs > $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y) > $(call if_changed,initfs) > + > +subdir-$(CONFIG_UAPI_HEADER_TEST) += include > diff --git a/usr/include/.gitignore b/usr/include/.gitignore > new file mode 100644 > index 000000000000..a0991ff4402b > --- /dev/null > +++ b/usr/include/.gitignore > @@ -0,0 +1,3 @@ > +* > +!.gitignore > +!Makefile > diff --git a/usr/include/Makefile b/usr/include/Makefile > new file mode 100644 > index 000000000000..58ce96fa1701 > --- /dev/null > +++ b/usr/include/Makefile > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +# Unlike the kernel space, uapi headers are written in standard C. > +# - Forbid C++ style comments > +# - Use '__inline__', '__asm__' instead of 'inline', 'asm' > +# > +# -std=c90 (equivalent to -ansi) catches the violation of those. > +# We cannot go as far as adding -Wpedantic since it emits too many warnings. > +# > +# REVISIT: re-consider the proper set of compiler flags for uapi compile-test. > + > +UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration > + > +override c_flags = $(UAPI_CFLAGS) -Wp,-MD,$(depfile) -I$(objtree)/usr/include > + > +# The following are excluded for now because they fail to build. > +# The cause of errors are mostly missing include directives. > +# Check one by one, and send a patch to each subsystem. > +# > +# Do not add a new header to the blacklist without legitimate reason. > +# Please consider to fix the header first. Maybe add comment that the alphabetical sort by filename must be preserved. Not too relevant, as we hopefully do not see files being added. > +header-test- += asm/ipcbuf.h > +header-test- += asm/msgbuf.h Consider same syntax like in include/Makefile where you use header-test-<tab><tab>...+= file Then the alignment looks betters. > +header-test- += asm/sembuf.h > +header-test- += asm/shmbuf.h > +header-test- += asm/signal.h > +header-test- += asm/ucontext.h > +header-test- += drm/vmwgfx_drm.h > +header-test- += linux/am437x-vpfe.h > +header-test- += linux/android/binderfs.h > +header-test- += linux/android/binder.h > +header-test-$(CONFIG_CPU_BIG_ENDIAN) += linux/byteorder/big_endian.h > +header-test-$(CONFIG_CPU_LITTLE_ENDIAN) += linux/byteorder/little_endian.h > +header-test- += linux/coda.h ... List is shorter than I feared. Seems quite doable to get down to a small number of files. > + > +# more headers are broken in some architectures > + > +ifeq ($(SRCARCH),arc) > +header-test- += linux/bpf_perf_event.h > +endif Again a manageable number. > + > + > +# asm-generic/*.h is used by asm/*.h, and should not be included directly > +header-test- += asm-generic/% > + > +# The rest are compile-tested > +header-test-y += $(filter-out $(header-test-), \ > + $(patsubst $(obj)/%,%, $(wildcard \ > + $(addprefix $(obj)/, *.h */*.h */*/*.h */*/*/*.h)))) Could you use header-test-pattern-y here? Sam _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained 2019-06-28 15:43 ` Sam Ravnborg @ 2019-06-30 1:27 ` Masahiro Yamada 0 siblings, 0 replies; 6+ messages in thread From: Masahiro Yamada @ 2019-06-30 1:27 UTC (permalink / raw) To: Sam Ravnborg Cc: Song Liu, Michal Marek, bpf, Daniel Borkmann, Linux Kbuild mailing list, Jani Nikula, Networking, Palmer Dabbelt, Alexei Starovoitov, Linux Kernel Mailing List, Albert Ou, Yonghong Song, linux-riscv, Martin KaFai Lau Hi Sam, On Sat, Jun 29, 2019 at 12:44 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Masahiro. > > On Fri, Jun 28, 2019 at 01:38:59AM +0900, Masahiro Yamada wrote: > > Multiple people have suggested compile-testing UAPI headers to ensure > > they can be really included from user-space. "make headers_check" is > > obviously not enough to catch bugs, and we often leak references to > > kernel-space definition to user-space. > > > > Use the new header-test-y syntax to implement it. Please note exported > > headers are compile-tested with a completely different set of compiler > > flags. The header search path is set to $(objtree)/usr/include since > > exported headers should not include unexported ones. > > This patchset introduce a new set of tests for uapi headers. > Can we somehow consolidate so we have only one way to verify the uapi > headers? > It can be confusing for users that they see errors from testing the > uapi headers during normal build and a new class or error if they > run a "make headers_check" sometimes later. Good point. I had also noticed some over-wrap between this feature and scripts/headers_check.pl For example, check_include will be unneeded. I expect "make headers_check" will be deprecated sooner or later. > This can be a next step to consolidate this. > With the suggestions below considered you can add my: > Reviewed-by: Sam Ravnborg <sam@ravnborg.org> > > > > > We use -std=gnu89 for the kernel space since the kernel code highly > > depends on GNU extensions. On the other hand, UAPI headers should be > > written in more standardized C, so they are compiled with -std=c90. > > This will emit errors if C++ style comments, the keyword 'inline', etc. > > are used. Please use C style comments (/* ... */), '__inline__', etc. > > in UAPI headers. > > > > There is additional compiler requirement to enable this test because > > many of UAPI headers include <stdlib.h>, <sys/ioctl.h>, <sys/time.h>, > > etc. directly or indirectly. You cannot use kernel.org pre-built > > toolchains [1] since they lack <stdlib.h>. > > > > I added scripts/cc-system-headers.sh to check the system header > > availability, which CONFIG_UAPI_HEADER_TEST depends on. > > > > For now, a lot of headers need to be excluded because they cannot > > be compiled standalone, but this is a good start point. > > > > [1] https://mirrors.edge.kernel.org/pub/tools/crosstool/index.html > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > --- > > > > Changes in v3: None > > Changes in v2: > > - Add CONFIG_CPU_{BIG,LITTLE}_ENDIAN guard to avoid build error > > - Use 'header-test-' instead of 'no-header-test' > > - Avoid weird 'find' warning when cleaning > > > > Makefile | 2 +- > > init/Kconfig | 11 +++ > > scripts/cc-system-headers.sh | 8 +++ > > usr/.gitignore | 1 - > > usr/Makefile | 2 + > > usr/include/.gitignore | 3 + > > usr/include/Makefile | 134 +++++++++++++++++++++++++++++++++++ > > 7 files changed, 159 insertions(+), 2 deletions(-) > > create mode 100755 scripts/cc-system-headers.sh > > create mode 100644 usr/include/.gitignore > > create mode 100644 usr/include/Makefile > > > > diff --git a/Makefile b/Makefile > > index 1f35aca4fe05..f23516980796 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1363,7 +1363,7 @@ CLEAN_DIRS += $(MODVERDIR) include/ksym > > CLEAN_FILES += modules.builtin.modinfo > > > > # Directories & files removed with 'make mrproper' > > -MRPROPER_DIRS += include/config usr/include include/generated \ > > +MRPROPER_DIRS += include/config include/generated \ > > arch/$(SRCARCH)/include/generated .tmp_objdiff > > MRPROPER_FILES += .config .config.old .version \ > > Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \ > > diff --git a/init/Kconfig b/init/Kconfig > > index df5bba27e3fe..667d68e1cdf4 100644 > > --- a/init/Kconfig > > +++ b/init/Kconfig > > @@ -105,6 +105,17 @@ config HEADER_TEST > > If you are a developer or tester and want to ensure the requested > > headers are self-contained, say Y here. Otherwise, choose N. > > > > +config UAPI_HEADER_TEST > > + bool "Compile test UAPI headers" > > + depends on HEADER_TEST && HEADERS_INSTALL > > + depends on $(success,$(srctree)/scripts/cc-system-headers.sh $(CC)) > > + help > > + Compile test headers exported to user-space to ensure they are > > + self-contained, i.e. compilable as standalone units. > > + > > + If you are a developer or tester and want to ensure the exported > > + headers are self-contained, say Y here. Otherwise, choose N. > > + > > config LOCALVERSION > > string "Local version - append to kernel release" > > help > > diff --git a/scripts/cc-system-headers.sh b/scripts/cc-system-headers.sh > > new file mode 100755 > > index 000000000000..1b3db369828c > > --- /dev/null > > +++ b/scripts/cc-system-headers.sh > > @@ -0,0 +1,8 @@ > > +#!/bin/sh > > +# SPDX-License-Identifier: GPL-2.0-only > > + > > +cat << "END" | $@ -E -x c - -o /dev/null >/dev/null 2>&1 > > +#include <stdlib.h> > > +#include <sys/ioctl.h> > > +#include <sys/time.h> > > +END > > Add comment to this file that explains that it is used to verify that the > toolchain provides the minimal set of header-files required by uapi > headers. Will do. > > diff --git a/usr/.gitignore b/usr/.gitignore > > index 8e48117a3f3d..be5eae1df7eb 100644 > > --- a/usr/.gitignore > > +++ b/usr/.gitignore > > @@ -7,4 +7,3 @@ initramfs_data.cpio.gz > > initramfs_data.cpio.bz2 > > initramfs_data.cpio.lzma > > initramfs_list > > -include > > diff --git a/usr/Makefile b/usr/Makefile > > index 4a70ae43c9cb..6a89eb019275 100644 > > --- a/usr/Makefile > > +++ b/usr/Makefile > > @@ -56,3 +56,5 @@ $(deps_initramfs): klibcdirs > > $(obj)/$(datafile_y): $(obj)/gen_init_cpio $(deps_initramfs) klibcdirs > > $(Q)$(initramfs) -l $(ramfs-input) > $(obj)/$(datafile_d_y) > > $(call if_changed,initfs) > > + > > +subdir-$(CONFIG_UAPI_HEADER_TEST) += include > > diff --git a/usr/include/.gitignore b/usr/include/.gitignore > > new file mode 100644 > > index 000000000000..a0991ff4402b > > --- /dev/null > > +++ b/usr/include/.gitignore > > @@ -0,0 +1,3 @@ > > +* > > +!.gitignore > > +!Makefile > > diff --git a/usr/include/Makefile b/usr/include/Makefile > > new file mode 100644 > > index 000000000000..58ce96fa1701 > > --- /dev/null > > +++ b/usr/include/Makefile > > @@ -0,0 +1,134 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > + > > +# Unlike the kernel space, uapi headers are written in standard C. > > +# - Forbid C++ style comments > > +# - Use '__inline__', '__asm__' instead of 'inline', 'asm' > > +# > > +# -std=c90 (equivalent to -ansi) catches the violation of those. > > +# We cannot go as far as adding -Wpedantic since it emits too many warnings. > > +# > > +# REVISIT: re-consider the proper set of compiler flags for uapi compile-test. > > + > > +UAPI_CFLAGS := -std=c90 -Wall -Werror=implicit-function-declaration > > + > > +override c_flags = $(UAPI_CFLAGS) -Wp,-MD,$(depfile) -I$(objtree)/usr/include > > + > > +# The following are excluded for now because they fail to build. > > +# The cause of errors are mostly missing include directives. > > +# Check one by one, and send a patch to each subsystem. > > +# > > +# Do not add a new header to the blacklist without legitimate reason. > > +# Please consider to fix the header first. > > Maybe add comment that the alphabetical sort by filename must be preserved. > Not too relevant, as we hopefully do not see files being added. > > > +header-test- += asm/ipcbuf.h > > +header-test- += asm/msgbuf.h > Consider same syntax like in include/Makefile where you use > header-test-<tab><tab>...+= file > > Then the alignment looks betters. Probably I can do that, but this is not so important because our ultimate goal is (almost) no blacklist. > > + > > +# The rest are compile-tested > > +header-test-y += $(filter-out $(header-test-), \ > > + $(patsubst $(obj)/%,%, $(wildcard \ > > + $(addprefix $(obj)/, *.h */*.h */*/*.h */*/*/*.h)))) > Could you use header-test-pattern-y here? header-test-pattern-y does not work here because it only matches to $(srctree)/$(src)/* All headers under usr/include/ are generated one, and located in $(objtree)/$(obj)/. Thanks. -- Best Regards Masahiro Yamada _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained 2019-06-27 16:38 ` [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained Masahiro Yamada 2019-06-28 15:43 ` Sam Ravnborg @ 2019-06-30 1:35 ` Masahiro Yamada 1 sibling, 0 replies; 6+ messages in thread From: Masahiro Yamada @ 2019-06-30 1:35 UTC (permalink / raw) To: Linux Kbuild mailing list Cc: Song Liu, Michal Marek, Daniel Borkmann, Jani Nikula, Networking, Martin KaFai Lau, Palmer Dabbelt, Alexei Starovoitov, Linux Kernel Mailing List, Albert Ou, Yonghong Song, bpf, Sam Ravnborg, linux-riscv On Fri, Jun 28, 2019 at 1:40 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Multiple people have suggested compile-testing UAPI headers to ensure > they can be really included from user-space. "make headers_check" is > obviously not enough to catch bugs, and we often leak references to > kernel-space definition to user-space. > > Use the new header-test-y syntax to implement it. Please note exported > headers are compile-tested with a completely different set of compiler > flags. The header search path is set to $(objtree)/usr/include since > exported headers should not include unexported ones. > > We use -std=gnu89 for the kernel space since the kernel code highly > depends on GNU extensions. On the other hand, UAPI headers should be > written in more standardized C, so they are compiled with -std=c90. > This will emit errors if C++ style comments, the keyword 'inline', etc. > are used. Please use C style comments (/* ... */), '__inline__', etc. > in UAPI headers. > > There is additional compiler requirement to enable this test because > many of UAPI headers include <stdlib.h>, <sys/ioctl.h>, <sys/time.h>, > etc. directly or indirectly. You cannot use kernel.org pre-built > toolchains [1] since they lack <stdlib.h>. > > I added scripts/cc-system-headers.sh to check the system header > availability, which CONFIG_UAPI_HEADER_TEST depends on. Perhaps, we could use scripts/cc-can-link.sh for this purpose. The intention is slightly different, but a compiler to link user-space programs must provide necessary standard headers. -- Best Regards Masahiro Yamada _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/4] Compile-test UAPI and kernel headers 2019-06-27 16:38 [PATCH v3 0/4] Compile-test UAPI and kernel headers Masahiro Yamada 2019-06-27 16:38 ` [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained Masahiro Yamada @ 2019-06-28 4:57 ` Masahiro Yamada 1 sibling, 0 replies; 6+ messages in thread From: Masahiro Yamada @ 2019-06-28 4:57 UTC (permalink / raw) To: Linux Kbuild mailing list Cc: Song Liu, Jakub Kicinski, open list:DOCUMENTATION, Palmer Dabbelt, Alexei Starovoitov, linux-riscv, Sam Ravnborg, Jesper Dangaard Brouer, xdp-newbies, Daniel Borkmann, Jonathan Corbet, Anton Vorontsov, John Fastabend, Yonghong Song, Albert Ou, Kees Cook, Jani Nikula, Tony Luck, moderated list:ARM/Mediatek SoC support, Matthias Brugger, linux-arm-kernel, Michal Marek, Networking, Linux Kernel Mailing List, Martin KaFai Lau, Colin Cross, bpf, David S. Miller On Fri, Jun 28, 2019 at 1:41 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > 1/4: Compile-test exported headers (reworked in v2) > > 2/4: fix a flaw I noticed when I was working on this series. > Avoid generating intermediate wrappers. > > 3/4: maybe useful for 4/4 and in some other places. > Add header-test-pattern-y syntax. > > 4/4: Compile-test kernel-space headers in include/. > v2: compile as many headers as possible. > v3: exclude more headers causing build errors I push this series to git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git header-test-v3 for somebody who wants to test it. > > Masahiro Yamada (4): > kbuild: compile-test UAPI headers to ensure they are self-contained > kbuild: do not create wrappers for header-test-y > kbuild: support header-test-pattern-y > kbuild: compile-test kernel headers to ensure they are self-contained > > .gitignore | 1 - > Documentation/dontdiff | 1 - > Documentation/kbuild/makefiles.txt | 13 +- > Makefile | 4 +- > include/Kbuild | 1250 ++++++++++++++++++++++++++++ > init/Kconfig | 22 + > scripts/Makefile.build | 10 +- > scripts/Makefile.lib | 13 +- > scripts/cc-system-headers.sh | 8 + > usr/.gitignore | 1 - > usr/Makefile | 2 + > usr/include/.gitignore | 3 + > usr/include/Makefile | 134 +++ > 13 files changed, 1449 insertions(+), 13 deletions(-) > create mode 100644 include/Kbuild > create mode 100755 scripts/cc-system-headers.sh > create mode 100644 usr/include/.gitignore > create mode 100644 usr/include/Makefile > > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Best Regards Masahiro Yamada _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-30 1:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-27 16:38 [PATCH v3 0/4] Compile-test UAPI and kernel headers Masahiro Yamada 2019-06-27 16:38 ` [PATCH v3 1/4] kbuild: compile-test UAPI headers to ensure they are self-contained Masahiro Yamada 2019-06-28 15:43 ` Sam Ravnborg 2019-06-30 1:27 ` Masahiro Yamada 2019-06-30 1:35 ` Masahiro Yamada 2019-06-28 4:57 ` [PATCH v3 0/4] Compile-test UAPI and kernel headers Masahiro Yamada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).