Message-Id: <20221205163157.269335-4-nick.alcock@oracle.com>
Date:   Mon,  5 Dec 2022 16:31:47 +0000
From:   Nick Alcock <nick.alcock@...cle.com>
To:     mcgrof@...nel.org, masahiroy@...nel.org
Cc:     linux-modules@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
        linux-kernel@...r.kernel.org, arnd@...db.de,
        akpm@...ux-foundation.org, eugene.loh@...cle.com,
        kris.van.hees@...cle.com
Subject: [PATCH v10 03/13] kbuild: add tristate checker

This new check target uses the tristate.conf machinery just added, and
modules.builtin.objs, to identify source files that have MODULE_*
declarations despite not being modules according to Kconfig.  After
commit 8b41fc4454e ("kbuild: create modules.builtin without
Makefile.modbuiltin or tristate.conf"), all such declarations will cause
modprobe to misidentify their containing object file as a module when it
is not, which might cause it to spuriously fail when trying to load
something that is built in to the kernel.

Only source files compiled by the current configuration are checked, so
a make allyesconfig is probably a good thing to do before running this:
output if you don't do that won't be wrong but will probably be
incomplete.  Note that it is quite possible for things to be modular on
only some architectures, in which case a make allyesconfig build will
give false positive reports on other arches.  (This is rare, but does
apply to e.g. KVM on aarch64.)

The checking is done by comparing modules.builtin.obj to a new
check-tristates.objs, which is built in a way more or less identical to
the way modules.builtin used to be built before commit 8b41fc4454e; the
principal difference is that it's only built when running the checker,
so doesn't slow down existing builds.

This file is similar in structure to modules.builtin.objs, and
constructed in a similar way to the way modules.builtin used to be built
before commit 8b41fc4454e, via tristate.conf inclusion and recursive
concatenation up the tree.

Signed-off-by: Nick Alcock <nick.alcock@...cle.com>
---

Notes:
    v10: adapted from modules_thick.builtin code: adjusted to use
         modules.builtin.objs, turned into a tristate checker.  Top-level
         Kbuild changes no longer necessary.

 .gitignore                 |  1 +
 Documentation/dontdiff     |  1 +
 Makefile                   | 23 ++++++++++++++--
 scripts/Kbuild.include     |  6 ++++
 scripts/Makefile.lib       |  8 +++++-
 scripts/check-tristates.mk | 56 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 scripts/check-tristates.mk

diff --git a/.gitignore b/.gitignore
index ef8665c64f21..75f246e0565f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -52,6 +52,7 @@
 *.zst
 Module.symvers
 modules.order
+check-tristates.objs
 
 #
 # Top-level generic files
diff --git a/Documentation/dontdiff b/Documentation/dontdiff
index ed1fbc711f33..17686f59039c 100644
--- a/Documentation/dontdiff
+++ b/Documentation/dontdiff
@@ -183,6 +183,7 @@ modules.builtin
 modules.builtin.*
 modules.nsdeps
 modules.order
+check-tristates.objs
 modversions.h*
 nconf
 nconf-cfg
diff --git a/Makefile b/Makefile
index 248f780cb75b..4d8a4c231cc1 100644
--- a/Makefile
+++ b/Makefile
@@ -1689,6 +1689,7 @@ help:
 	@echo  '  coccicheck      - Check with Coccinelle'
 	@echo  '  clang-analyzer  - Check with clang static analyzer'
 	@echo  '  clang-tidy      - Check with clang-tidy'
+	@echo  '  tristatecheck   - Check for non-tristates with MODULE_ declarations'
 	@echo  ''
 	@echo  'Tools:'
 	@echo  '  nsdeps          - Generate missing symbol namespace dependencies'
@@ -2012,7 +2013,7 @@ clean: $(clean-dirs)
 		-o -name '*.lex.c' -o -name '*.tab.[ch]' \
 		-o -name '*.asn1.[ch]' \
 		-o -name '*.symtypes' -o -name 'modules.order' \
-		-o -name '.tmp_*' \
+		-o -name 'check-tristates.objs' -o -name '.tmp_*' \
 		-o -name '*.c.[012]*.*' \
 		-o -name '*.ll' \
 		-o -name '*.gcno' \
@@ -2082,7 +2083,25 @@ coccicheck:
 export_report:
 	$(PERL) $(srctree)/scripts/export_report.pl
 
-PHONY += checkstack kernelrelease kernelversion image_name
+tristatecheck: SHELL=/bin/bash
+tristatecheck: check-tristates.objs modules.builtin.objs
+	$(Q) for name in $$(comm -23 <(sed 's,:,,' modules.builtin.objs | tr ' ' '\n' | sort -u) \
+				    <(sed 's,:,,' check-tristates.objs | tr ' ' '\n' | sort -u)); do \
+		grep -w $$name modules.builtin.objs | while read -r line; do \
+			case $$line in \
+			$$name:\ ) printf "%s " "$$line" | sed 's,: ,,';; \
+			$$name:\ *) printf "%s " "$$line" | sed 's,^.*: ,,';; \
+			*) echo $$name;; \
+			esac; \
+		done; \
+	     done | tr ' ' '\n' | sort -u | while read -r name; do \
+		test -f $${name%.o}.c && echo $${name%.o}.c; \
+	     done | xargs grep -l '^ *MODULE_' || true
+
+check-tristates.objs: $(build-dir)
+	$(Q)$(MAKE) $(tristatecheck)=$^ tristates-file=$@
+
+PHONY += checkstack kernelrelease kernelversion image_name tristatecheck
 
 # UML needs a little special treatment here.  It wants to use the host
 # toolchain, so needs $(SUBARCH) passed to checkstack.pl.  Everyone
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 2bc08ace38a3..8042c067312e 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -78,6 +78,12 @@ endef
 # $(Q)$(MAKE) $(build)=dir
 build := -f $(srctree)/scripts/Makefile.build obj
 
+###
+# Shorthand for $(Q)$(MAKE) -f scripts/check-tristates.mk need-tristates=1 obj=
+# Usage:
+# $(Q)$(MAKE) $(tristatecheck)=dir
+tristatecheck := -f $(srctree)/scripts/check-tristates.mk need-tristates=1 obj
+
 ###
 # Shorthand for $(Q)$(MAKE) -f scripts/Makefile.dtbinst obj=
 # Usage:
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index f7e5a83572fa..3bbfa99259cd 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -39,11 +39,17 @@ else
 obj-m := $(filter-out %/, $(obj-m))
 endif
 
-ifdef need-builtin
+ifdef need-tristates
+subdir-ym	:= $(sort $(subdir-y) $(subdir-m) \
+			$(patsubst %/,%, $(filter %/, $(obj-y) $(obj-m) $(obj-Y) $(obj-M))))
 obj-y		:= $(patsubst %/, %/built-in.a, $(obj-y))
 else
+ifdef need-builtin
+obj-y          := $(patsubst %/, %/built-in.a, $(obj-y))
+else
 obj-y		:= $(filter-out %/, $(obj-y))
 endif
+endif
 
 # Expand $(foo-objs) $(foo-y) etc. by replacing their individuals
 suffix-search = $(strip $(foreach s, $3, $($(1:%$(strip $2)=%$s))))
diff --git a/scripts/check-tristates.mk b/scripts/check-tristates.mk
new file mode 100644
index 000000000000..fa9fadb79143
--- /dev/null
+++ b/scripts/check-tristates.mk
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: GPL-2.0
+# ==========================================================================
+# Generating check-tristates.objs
+# ==========================================================================
+
+src := $(obj)
+
+PHONY := __tristates
+__tristates:
+
+include include/config/auto.conf
+# tristate.conf sets tristate variables to uppercase 'Y' or 'M'
+# That way, we get the list of built-in modules in obj-Y
+include include/config/tristate.conf
+
+include scripts/Kbuild.include
+
+ifdef building_out_of_srctree
+# Create output directory if not already present
+_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
+endif
+
+# The filename Kbuild has precedence over Makefile
+kbuild-dir := $(if $(filter /%,$(src)),$(src),$(srctree)/$(src))
+kbuild-file := $(if $(wildcard $(kbuild-dir)/Kbuild),$(kbuild-dir)/Kbuild,$(kbuild-dir)/Makefile)
+include $(kbuild-file)
+
+include scripts/Makefile.lib
+
+check-tristates-subdirs := $(patsubst %,%/check-tristates.objs, $(subdir-ym))
+check-tristates-target  := $(obj)/check-tristates.objs
+
+__tristates: $(obj)/$(tristates-file) $(subdir-ym)
+	@:
+
+$(check-tristates-target): $(subdir-ym) FORCE
+	$(Q) rm -f $@
+	$(Q) $(foreach mod-o, $(filter %.o,$(obj-Y)),\
+		printf "%s: " $(addprefix $(obj)/,$(mod-o)) >> $@; \
+		printf " %s" $(sort $(strip $(addprefix $(obj)/,$($(mod-o:.o=-objs)) \
+			$($(mod-o:.o=-y)) $($(mod-o:.o=-Y))))) >> $@; \
+		printf "\n" >> $@; ) \
+	cat /dev/null $(check-tristates-subdirs) >> $@;
+
+PHONY += FORCE
+
+FORCE:
+
+# Descending
+# ---------------------------------------------------------------------------
+
+PHONY += $(subdir-ym)
+$(subdir-ym):
+	$(Q)$(MAKE) $(tristatecheck)=$@ tristates-file=$(tristates-file)
+
+.PHONY: $(PHONY)
-- 
2.38.0.266.g481848f278

Powered by blists - more mailing lists