Linux kbuild/kconfig development
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox