All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Schier <nsc@kernel.org>
To: Andrew Jones <andrew.jones@linux.dev>
Cc: linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	nathan@kernel.org, andriy.shevchenko@linux.intel.com,
	rdunlap@infradead.org, julianbraha@gmail.com
Subject: Re: [PATCH v4] kconfig: add kconfig-sym-check static checker
Date: Tue, 2 Jun 2026 15:05:32 +0200	[thread overview]
Message-ID: <ah7VHBwxnn4pp0G1@levanger> (raw)
In-Reply-To: <20260527142703.107110-1-andrew.jones@linux.dev>

On Wed, May 27, 2026 at 09:27:03AM -0500, Andrew Jones wrote:
> Add 'make kconfig-sym-check', a static checker that finds Kconfig
> symbols referenced in expressions (select, depends on, default, etc.)
> but never defined via config/menuconfig anywhere in the tree. New
> dangling symbols are reported as errors (exit 1) unless they are
> listed in an exclusion file, e.g.
> 
>  KCONFIG_SYM_CHECK_EXCLUDES=sym-check-excludes make kconfig-sym-check
> 
> The exclusion file lists one symbol per line; blank lines and lines
> starting with '#' are ignored.
> 
> The checker also warns about uppercase N/Y/M used as tristate literal
> values following the same logic as checkpatch.
> 
> This new static checker is the script used for [1] with a few
> improvements to avoid some false positives.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216748 [1]
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
> Tested-by: Julian Braha <julianbraha@gmail.com>
> ---
> 
> Note, v1 was based on next-20260508 and this checker found 14 dangling
> symbols on that base. v4 is based on next-20260526 and there are now 15.
> EZX_PCAP has recently emerged as a new dangling symbol after commit
> b12d1da45f12 ("mfd: ezx-pcap: Remove unused driver") was merged.
> 
> v4:
>   - improved stripping of $(macro) expansions [Sashiko]
>   - improved failure when run without srctree parameter
>   - Added Julian's t-b
> 
> v3:
>   - Filter out scripts/kconfig/tests Kconfig files since they may
>     be wrong on purpose and indeed there is a 'config Y' in there
>     which would mask improper use of 'Y'. [Julian and Randy]
>   - Fixed breakage introduced in v2 when attempting to be too
>     clever...
>   - More changes from another sashiko review which required the
>     Perl to get even uglier. So ugly that I enlisted Claude to
>     help generate it.
>   - Added a sentence to the commit message to describe the excludes
>     file format.
> 
> v2:
>   - Added Andy's and Randy's tags
>   - Accept srctree as first argument so the Makefile can drop 'cd $(srctree)' [Nathan]
>   - Replace git ls-files with git+find fallback [Nathan and Andy]
>   - Changes thanks to sashiko's review
>     - strip quoted strings before inline comments to avoid '#' inside a string
>     - use [^)]* instead of .* in macro strip regex to avoid greedy match
>       eating tokens between adjacent $(macro) expansions
> 
>  Makefile                             |  23 +++--
>  scripts/kconfig/kconfig-sym-check.pl | 132 +++++++++++++++++++++++++++
>  2 files changed, 146 insertions(+), 9 deletions(-)
>  create mode 100755 scripts/kconfig/kconfig-sym-check.pl
> 
> diff --git a/Makefile b/Makefile
> index d59f703f9797..60c30116b95e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -293,6 +293,7 @@ version_h := include/generated/uapi/linux/version.h
>  clean-targets := %clean mrproper cleandocs
>  no-dot-config-targets := $(clean-targets) \
>  			 cscope gtags TAGS tags help% %docs check% coccicheck \
> +			 kconfig-sym-check \
>  			 $(version_h) headers headers_% archheaders archscripts \
>  			 %asm-generic kernelversion %src-pkg dt_binding_check \
>  			 outputmakefile rustavailable rustfmt rustfmtcheck \
> @@ -1821,14 +1822,15 @@ help:
>  	 echo  '                    (default: $(INSTALL_HDR_PATH))'; \
>  	 echo  ''
>  	@echo  'Static analysers:'
> -	@echo  '  checkstack      - Generate a list of stack hogs and consider all functions'
> -	@echo  '                    with a stack size larger than MINSTACKSIZE (default: 100)'
> -	@echo  '  versioncheck    - Sanity check on version.h usage'
> -	@echo  '  includecheck    - Check for duplicate included header files'
> -	@echo  '  headerdep       - Detect inclusion cycles in headers'
> -	@echo  '  coccicheck      - Check with Coccinelle'
> -	@echo  '  clang-analyzer  - Check with clang static analyzer'
> -	@echo  '  clang-tidy      - Check with clang-tidy'
> +	@echo  '  checkstack        - Generate a list of stack hogs and consider all functions'
> +	@echo  '                      with a stack size larger than MINSTACKSIZE (default: 100)'
> +	@echo  '  versioncheck      - Sanity check on version.h usage'
> +	@echo  '  includecheck      - Check for duplicate included header files'
> +	@echo  '  headerdep         - Detect inclusion cycles in headers'
> +	@echo  '  coccicheck        - Check with Coccinelle'
> +	@echo  '  kconfig-sym-check - Check for dangling Kconfig symbol references'
> +	@echo  '  clang-analyzer    - Check with clang static analyzer'
> +	@echo  '  clang-tidy        - Check with clang-tidy'
>  	@echo  ''
>  	@echo  'Tools:'
>  	@echo  '  nsdeps          - Generate missing symbol namespace dependencies'
> @@ -2272,7 +2274,7 @@ endif
>  # Scripts to check various things for consistency
>  # ---------------------------------------------------------------------------
>  
> -PHONY += includecheck versioncheck coccicheck
> +PHONY += includecheck versioncheck coccicheck kconfig-sym-check
>  
>  includecheck:
>  	find $(srctree)/* $(RCS_FIND_IGNORE) \
> @@ -2287,6 +2289,9 @@ versioncheck:
>  coccicheck:
>  	$(Q)$(BASH) $(srctree)/scripts/$@
>  
> +kconfig-sym-check:
> +	$(Q)$(PERL) $(srctree)/scripts/kconfig/kconfig-sym-check.pl $(srctree) $(KCONFIG_SYM_CHECK_EXCLUDES)
> +
>  PHONY += checkstack kernelrelease kernelversion image_name
>  
>  # UML needs a little special treatment here.  It wants to use the host
> diff --git a/scripts/kconfig/kconfig-sym-check.pl b/scripts/kconfig/kconfig-sym-check.pl
> new file mode 100755
> index 000000000000..daa5285fdefc
> --- /dev/null
> +++ b/scripts/kconfig/kconfig-sym-check.pl
> @@ -0,0 +1,132 @@
> +#!/usr/bin/env perl
> +# SPDX-License-Identifier: GPL-2.0
> +
> +use warnings;
> +use strict;
> +
> +my $srctree = shift @ARGV;
> +unless (defined $srctree) {

unless (defined $srctree && -d $srctree) {

would also catch common attempts like 'kconfig-sym-check.pl --help' (at
least in most cases).


> +	$srctree = `git rev-parse --show-toplevel 2>/dev/null`;
> +	chomp $srctree;
> +	my $msg = "Usage: $0 <srctree> [excludes file]\n";
> +	$msg .= "Please provide <srctree>.";
> +	$msg .= " Is it '$srctree'?" if $srctree;
> +	$msg .= "\n";
> +	die $msg;
> +}
> +my $kconfig_sym_check_excludes = defined $ARGV[0] ? $ARGV[0] : undef;
> +
> +sub indent_depth {
> +	my ($ws) = @_;
> +	my $col = 0;
> +	for my $c (split //, $ws) {
> +		$col = $c eq "\t" ? int($col / 8) * 8 + 8 : $col + 1;
> +	}
> +	return $col;
> +}
> +
> +my @files = `git -C \Q$srctree\E ls-files '*Kconfig*' 2>/dev/null`;
> +if (@files) {
> +	chomp @files;
> +	@files = map { "$srctree/$_" } @files;
> +} else {
> +	@files = `find \Q$srctree\E -name '*Kconfig*'`;
> +	chomp @files;
> +}
> +
> +@files = grep { !m{/scripts/kconfig/tests/} } @files;
> +
> +my %configs = ();
> +my %refs = ();
> +
> +foreach my $file (@files) {
> +	open F, $file or die "Cannot open $file: $!";
> +
> +	my $help = 0;
> +	my $help_level;
> +	my $level;
> +
> +	while (<F>) {
> +		chomp;
> +
> +		while (/\\\s*$/) {
> +			s/\\\s*$/ /;
> +			my $cont = <F> // last;
> +			chomp $cont;
> +			$_ .= $cont;
> +		}
> +
> +		next if /^\s*$/;
> +		next if /^\s*#/;
> +
> +		/^(\s*)/;
> +		$level = indent_depth($1);
> +
> +		if ($help && $level < $help_level) {
> +			$help = 0;
> +		}
> +
> +		next if ($help);
> +
> +		if (/^\s*(help|\-\-\-help\-\-\-)$/) {
> +			$help = 1;
> +			my $next;
> +			while (defined($next = <F>)) {
> +				last unless $next =~ /^\s*(?:#.*)?$/;
> +			}
> +			last unless defined $next;
> +			$next =~ /^(\s*)/;
> +			if (indent_depth($1) >= $level) {
> +				$help_level = indent_depth($1);
> +			} else {
> +				$help = 0;
> +			}
> +			$_ = $next;
> +			redo;
> +		}
> +
> +		if (/^\s*(config|menuconfig)\s+([a-zA-Z0-9_]+)\s*(#.*)?$/) {

Sashiko claims that named choices are not catched here (but I could not
find any).  And all current findings of kconfig-sym-check look valid to
me, thus I don't see a strong need to adjust here for now.

> +			$configs{$2}++;
> +			next;
> +		}
> +
> +		if (/^\s*(default|def_bool|def_tristate|select|depends\s+on|imply|visible\s+if|range|if|bool|tristate|int|hex|string|prompt)\s+(.+)\s*$/) {
> +			my $s = $2;
> +			$s =~ s/"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'//g;
> +			$s =~ s/#.*//;
> +			$s =~ s/\$\((?:[^()]*|\((?:[^()]*|\([^()]*\))*\))*\)//g;
> +			$s =~ s/%%[^%]*%%//g;
> +			my @syms = split /[^a-zA-Z0-9_]+/, $s;
> +			map {
> +				$refs{$_}++ if (/[a-zA-Z]/ && $_ ne "if" && $_ ne "y" && $_ ne "n" && $_ ne "m" && !/^0[xX][0-9a-fA-F]+$/);
> +			} @syms
> +		}
> +	}
> +
> +	close F;
> +}
> +
> +my %known_syms = ();
> +if (defined $kconfig_sym_check_excludes) {
> +	my $file = $kconfig_sym_check_excludes;
> +	open(F, "<", $file) or die "Cannot open $file: $!";
> +	while (<F>) {
> +		chomp;
> +		next if /^\s*$/;
> +		next if /^\s*#/;
> +		$known_syms{$1}++ if (/^\s*([a-zA-Z0-9_]+)\s*(#.*)?$/);
> +	}
> +}
> +
> +my $ret = 0;
> +foreach my $k (sort keys %refs) {
> +	next if (exists $configs{$k} || exists $known_syms{$k});
> +
> +	print "$k";
> +	print " - warning: '$k' is probably not what you want; Kconfig tristate literals are always lowercase ('n', 'y', 'm')" if ($k eq "N" || $k eq "Y" || $k eq "M");
> +	print "\n";

Just bike-shedding:  I'd liked if kconfig-sym-check would output the
origin of its findings (gcc error style would help common tooling to
jump there, too).  But finding stray ever-undefined symbols is a good
thing, and finding these is no magic.

As my perl is in a quite bad state, I cannot review in-depth.  Would you
be available for maintaining the script yourself (= entry in
MAINTAINERS)?

Tested-by: Nicolas Schier <nsc@kernel.org>
Acked-by: Nicolas Schier <nsc@kernel.org>

      reply	other threads:[~2026-06-02 13:05 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27 14:27 [PATCH v4] kconfig: add kconfig-sym-check static checker Andrew Jones
2026-06-02 13:05 ` Nicolas Schier [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ah7VHBwxnn4pp0G1@levanger \
    --to=nsc@kernel.org \
    --cc=andrew.jones@linux.dev \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=julianbraha@gmail.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.