* [RFC v3 0/3] add kconfirm
@ 2026-05-16 21:53 Julian Braha
2026-05-16 21:53 ` [RFC PATCH v3 1/3] scripts: " Julian Braha
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Julian Braha @ 2026-05-16 21:53 UTC (permalink / raw)
To: nathan, nsc
Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda,
corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel,
rust-for-linux, linux-doc, linux-kbuild, Julian Braha
Hi all,
kconfirm has shrunk a lot since v2!
Okay back to the RFC...
kconfirm is a tool to detect misusage of Kconfig. It detects dead code,
constant conditions, and invalid (reverse) ranges. There are also optional
checks to detect config options that select visible config options, and to
check for dead links in the help texts.
Following this discussion:
https://lore.kernel.org/all/20260405122749.4990dcb538d457769a3276e0@linux-foundation.org/
in which Andrew brought up the possibility of moving kconfirm in-tree,
I've prepared this RFC to do so. See also kconfirm's introduction to the
mailing list:
https://lore.kernel.org/all/6ec4df6d-1445-48ca-8f54-1d1a83c4716d@gmail.com/
False Alarms:
kconfirm aims for zero false-positives, which is currently true for the
default checks (as far as I'm aware - but there are hundreds to go
through); this is not really possible for dead link checks, as this
depends on an internet connection, and we do not attempt to bypass bot
blocks. For this reason, dead link checking is disabled by default, but
I've provided an example below of how to enable it. Additionally, you can
view my previous message to the mailing list with hand-verified dead links
here:
https://lore.kernel.org/all/6732bf08-41ee-40c4-83b2-4ae8bc0da7cf@gmail.com/
Additionally, there is an optional check to detect config options that
select visible config options, as requested by Jani during the review of
the first version of this RFC:
https://lore.kernel.org/all/dcb7439832f0bb35598fba653d922b5f6a4d0058@intel.com/
Even after deduplicating across architectures, there are well over 1,000
instances of these select-visible cases, and I suspect that, despite the
Kconfig documentation saying select-visible should be avoided, some
exceptions will be made. So, I have left this check disabled by default,
keeping in line with the goal of having a low-noise checker. If interested
in using it, I have included an example below of how to enable this check.
Current State of Alarms:
On Linux v7.1-rc3 (which this RFC is based), there are 489 alarms coming
from the default set of checks, and an additional 1,789 alarms if enabling
the optional select-visible check. These counts are with deduplication
across all architectures, a change that was made to the tool's CLI from
RFC v1 to RFC v2. The last time I checked linux-next (next-20260427),
there were 81 unique dead links.
The most critical check is the dead default statements, which has surfaced
a few misconfiguration bugs (fortunately, just for kunit tests), see
examples:
https://lore.kernel.org/all/20260323124118.1414913-1-julianbraha@gmail.com/
and:
https://lore.kernel.org/all/20260323123536.1413732-1-julianbraha@gmail.com/
But hopefully kconfirm can ease maintenance and we can prevent more of
these from making it into the tree in the future.
Use it:
You can test out kconfirm with this patch series by compiling and running
kconfirm like this:
`make kconfirm`
To enable the select-visible check:
`KCONFIRM_ARGS="--enable-check select_visible" make kconfirm`
And to enable dead link checks in the help texts:
`KCONFIRM_ARGS="--enable-check dead_link" make kconfirm`
kconfirm by default runs on the same architecture as the kernel build
would; though additional architectures can be enabled by passing
`--enable-arch` and the default architecture can be disabled using
`--disable-arch`. Alarms are tagged with the affected architecture. For
alarms that appear in multiple of the enabled architectures, they are
deduplicated and tagged like: [X86] or [X86, ARM].
Dependencies will need to first be downloaded from crates.io by running
the `cargo vendor` command in scripts/kconfirm/
Requested feedback:
1. I would like to know if anyone thinks that the select-visible check
should be enabled by default.
2. I'm still hoping for some usage feedback!
Thanks,
Julian Braha
---
Changes since v2:
- Reduce Rust dependencies significantly (follows Demi's suggestions):
- from 6 direct dependencies to 1
- from 107 indirect dependencies to 4
- Replace ureq crate with usage of system libcurl (thanks Demi)
- Replace clap crate with FFI bindings to libc's getopt_long (also Demi)
- Remove crates env_logger, regex
- Switch from vendoring dependencies to requiring users to first download
outside of Make (as suggested by Miguel)
- Various makefile improvements (as pointed out by Nicolas):
- Fix out-of-tree builds
- Only delete kconfirm artifacts with 'distclean' and 'mrproper'
- Add myself as maintainer of kconfirm (as discussed with Nicolas)
- Remove dedicated code license file (pointed out by Jani)
- Update documentation to explain tool setup
- Add hint to users to check documentation and download tool dependencies
- Address sashiko's many code-level and documentation suggestions:
- Follow the kernel's rust import style
- Fix a dead_range/duplicate_range alarm mixup
- Fix potential duplicates in default value style check
- Avoid panicking on errors
- Clarify parse failure check usage in documentation
- Fix typo in documentation
- Can now enable architectures and disable the default (host) architecture in the CLI
Link to v2:
https://lore.kernel.org/all/20260509203808.1142311-1-julianbraha@gmail.com/
Changes since v1:
- vendored dependencies instead of requiring an internet connection
- removed Cargo.lock
- replaced reqwest dependency with smaller ureq
- removed rustls, expect user to have openssl instead
- added select-visible check based on Jani's feature request
- added invalid (reverse) range check
- deduplicating alarms that appear for multiple architectures
- `make clean` no longer deletes kconfirm's build artifacts
- typo fixes in documentation
- added patch description for the main "add kconfirm" patch (patch 1/2)
Link to v1:
https://lore.kernel.org/all/20260427174429.779474-1-julianbraha@gmail.com/
---
Julian Braha (3):
scripts: add kconfirm
Documentation: add kconfirm
MAINTAINERS: create entry for kconfirm
Documentation/dev-tools/index.rst | 1 +
Documentation/dev-tools/kconfirm.rst | 222 ++++++
MAINTAINERS | 6 +
Makefile | 15 +-
scripts/Makefile | 2 +-
scripts/kconfirm/.gitignore | 3 +
scripts/kconfirm/Cargo.lock | 60 ++
scripts/kconfirm/Cargo.toml | 12 +
scripts/kconfirm/Makefile | 14 +
scripts/kconfirm/kconfirm-lib/Cargo.toml | 12 +
scripts/kconfirm/kconfirm-lib/src/analyze.rs | 643 ++++++++++++++++
scripts/kconfirm/kconfirm-lib/src/checks.rs | 701 ++++++++++++++++++
scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs | 182 +++++
.../kconfirm/kconfirm-lib/src/dead_links.rs | 138 ++++
scripts/kconfirm/kconfirm-lib/src/lib.rs | 62 ++
scripts/kconfirm/kconfirm-lib/src/output.rs | 111 +++
.../kconfirm/kconfirm-lib/src/symbol_table.rs | 223 ++++++
scripts/kconfirm/kconfirm-linux/Cargo.toml | 10 +
.../kconfirm/kconfirm-linux/src/getopt_ffi.rs | 99 +++
scripts/kconfirm/kconfirm-linux/src/lib.rs | 78 ++
scripts/kconfirm/kconfirm-linux/src/main.rs | 192 +++++
21 files changed, 2781 insertions(+), 5 deletions(-)
create mode 100644 Documentation/dev-tools/kconfirm.rst
create mode 100644 scripts/kconfirm/.gitignore
create mode 100644 scripts/kconfirm/Cargo.lock
create mode 100644 scripts/kconfirm/Cargo.toml
create mode 100644 scripts/kconfirm/Makefile
create mode 100644 scripts/kconfirm/kconfirm-lib/Cargo.toml
create mode 100644 scripts/kconfirm/kconfirm-lib/src/analyze.rs
create mode 100644 scripts/kconfirm/kconfirm-lib/src/checks.rs
create mode 100644 scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs
create mode 100644 scripts/kconfirm/kconfirm-lib/src/dead_links.rs
create mode 100644 scripts/kconfirm/kconfirm-lib/src/lib.rs
create mode 100644 scripts/kconfirm/kconfirm-lib/src/output.rs
create mode 100644 scripts/kconfirm/kconfirm-lib/src/symbol_table.rs
create mode 100644 scripts/kconfirm/kconfirm-linux/Cargo.toml
create mode 100644 scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs
create mode 100644 scripts/kconfirm/kconfirm-linux/src/lib.rs
create mode 100644 scripts/kconfirm/kconfirm-linux/src/main.rs
--
2.53.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-16 21:53 [RFC v3 0/3] add kconfirm Julian Braha @ 2026-05-16 21:53 ` Julian Braha 2026-05-17 6:10 ` Demi Marie Obenour 2026-05-17 6:28 ` Miguel Ojeda 2026-05-16 21:53 ` [RFC PATCH v3 2/3] Documentation: " Julian Braha ` (3 subsequent siblings) 4 siblings, 2 replies; 20+ messages in thread From: Julian Braha @ 2026-05-16 21:53 UTC (permalink / raw) To: nathan, nsc Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild, Julian Braha Add kconfirm into scripts/ kconfirm is a static analysis tool with various checks for Kconfig, and intended to have zero false alarms by default. These default checks currently include dead code, constant conditions, and invalid (reverse) ranges. There are also optional checks for dead links in the help texts, and for config options that select visible config options. Checks are performed on the same architecture as the kernel build, using a single thread. More architectures can be enabled by passing `--enable-arch`. Alarms are tagged using the architectures' config options, like so: [X86] if specific to x86, or [X86, ARM] if the alarm appears for both x86 and arm. Each alarm gets a single line (deduplicated across architectures) and is formatted like this: [<SEVERITY>] [<ARCH_1>, <ARCH_2>] config <OPTION_NAME>: <alarm message> The tool source contains two Rust packages: kconfirm-lib and kconfirm-linux. kconfirm-lib is the underlying library that analyzes Kconfig code, and formats alarms for usability. It analyzes the entire Linux Kconfig spec, including all architectures. This package exposes the symbol table that it constructs so that other tools can import this library, and make use of it for their own Kconfig analyses. kconfirm-linux imports kconfirm-lib, and provides the CLI, which is intended for either manual usage, or integration with the Linux build system so that users can simply run `make kconfirm` from the root. kconfirm-linux also handles some of the specificities of how Kconfig is used in the Linux tree, in contrast to other open source software. E.g. the way that each architecture has its own Kconfig and Kconfig.debug files. The tool's dependencies need to be downloaded from crates.io by running `cargo vendor` in scripts/kconfirm/ before building and running the tool. Signed-off-by: Julian Braha <julianbraha@gmail.com> --- Makefile | 15 +- scripts/Makefile | 2 +- scripts/kconfirm/.gitignore | 3 + scripts/kconfirm/Cargo.lock | 60 ++ scripts/kconfirm/Cargo.toml | 12 + scripts/kconfirm/Makefile | 14 + scripts/kconfirm/kconfirm-lib/Cargo.toml | 12 + scripts/kconfirm/kconfirm-lib/src/analyze.rs | 643 ++++++++++++++++ scripts/kconfirm/kconfirm-lib/src/checks.rs | 701 ++++++++++++++++++ scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs | 182 +++++ .../kconfirm/kconfirm-lib/src/dead_links.rs | 138 ++++ scripts/kconfirm/kconfirm-lib/src/lib.rs | 62 ++ scripts/kconfirm/kconfirm-lib/src/output.rs | 111 +++ .../kconfirm/kconfirm-lib/src/symbol_table.rs | 223 ++++++ scripts/kconfirm/kconfirm-linux/Cargo.toml | 10 + .../kconfirm/kconfirm-linux/src/getopt_ffi.rs | 99 +++ scripts/kconfirm/kconfirm-linux/src/lib.rs | 78 ++ scripts/kconfirm/kconfirm-linux/src/main.rs | 192 +++++ 18 files changed, 2552 insertions(+), 5 deletions(-) create mode 100644 scripts/kconfirm/.gitignore create mode 100644 scripts/kconfirm/Cargo.lock create mode 100644 scripts/kconfirm/Cargo.toml create mode 100644 scripts/kconfirm/Makefile create mode 100644 scripts/kconfirm/kconfirm-lib/Cargo.toml create mode 100644 scripts/kconfirm/kconfirm-lib/src/analyze.rs create mode 100644 scripts/kconfirm/kconfirm-lib/src/checks.rs create mode 100644 scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs create mode 100644 scripts/kconfirm/kconfirm-lib/src/dead_links.rs create mode 100644 scripts/kconfirm/kconfirm-lib/src/lib.rs create mode 100644 scripts/kconfirm/kconfirm-lib/src/output.rs create mode 100644 scripts/kconfirm/kconfirm-lib/src/symbol_table.rs create mode 100644 scripts/kconfirm/kconfirm-linux/Cargo.toml create mode 100644 scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs create mode 100644 scripts/kconfirm/kconfirm-linux/src/lib.rs create mode 100644 scripts/kconfirm/kconfirm-linux/src/main.rs diff --git a/Makefile b/Makefile index b7b80e84e1eb..99aaed5bdbc5 100644 --- a/Makefile +++ b/Makefile @@ -296,7 +296,7 @@ no-dot-config-targets := $(clean-targets) \ $(version_h) headers headers_% archheaders archscripts \ %asm-generic kernelversion %src-pkg dt_binding_check \ outputmakefile rustavailable rustfmt rustfmtcheck \ - run-command + run-command kconfirm no-sync-config-targets := $(no-dot-config-targets) %install modules_sign kernelrelease \ image_name single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s %/ @@ -536,6 +536,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump READELF = $(CROSS_COMPILE)readelf STRIP = $(CROSS_COMPILE)strip endif +CARGO = cargo RUSTC = rustc RUSTDOC = rustdoc RUSTFMT = rustfmt @@ -633,7 +634,7 @@ export RUSTC_BOOTSTRAP := 1 export CLIPPY_CONF_DIR := $(srctree) export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG -export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK +export CARGO RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK export HOSTRUSTC KBUILD_HOSTRUSTFLAGS export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX @@ -1705,7 +1706,7 @@ MRPROPER_FILES += include/config include/generated \ vmlinux-gdb.py \ rpmbuild \ rust/libmacros.so rust/libmacros.dylib \ - rust/libpin_init_internal.so rust/libpin_init_internal.dylib + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \ # clean - Delete most, but leave enough to build external modules # @@ -2227,7 +2228,7 @@ endif # Scripts to check various things for consistency # --------------------------------------------------------------------------- -PHONY += includecheck versioncheck coccicheck +PHONY += includecheck versioncheck coccicheck kconfirm includecheck: find $(srctree)/* $(RCS_FIND_IGNORE) \ @@ -2242,6 +2243,12 @@ versioncheck: coccicheck: $(Q)$(BASH) $(srctree)/scripts/$@ + +kconfirm: + $(Q)$(MAKE) -C $(srctree)/scripts/kconfirm srctree=$(abs_srctree) SRCARCH=$(SRCARCH) || \ + (printf "\n kconfirm failed to compile and run. Have you set up its dependencies yet?\n See Documentation/dev-tools/kconfirm.rst\n\n" && false) +clean-dirs += scripts/kconfirm + PHONY += checkstack kernelrelease kernelversion image_name # UML needs a little special treatment here. It wants to use the host diff --git a/scripts/Makefile b/scripts/Makefile index 3434a82a119f..460655bd2de1 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -66,4 +66,4 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux subdir-$(CONFIG_SECURITY_IPE) += ipe # Let clean descend into subdirs -subdir- += basic dtc gdb kconfig mod +subdir- += basic dtc gdb kconfig kconfirm mod diff --git a/scripts/kconfirm/.gitignore b/scripts/kconfirm/.gitignore new file mode 100644 index 000000000000..f63ee0251591 --- /dev/null +++ b/scripts/kconfirm/.gitignore @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only +/target +/vendor diff --git a/scripts/kconfirm/Cargo.lock b/scripts/kconfirm/Cargo.lock new file mode 100644 index 000000000000..d90bc7d2e2a3 --- /dev/null +++ b/scripts/kconfirm/Cargo.lock @@ -0,0 +1,60 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "bytecount" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e" + +[[package]] +name = "kconfirm-lib" +version = "0.10.0" +dependencies = [ + "nom-kconfig", +] + +[[package]] +name = "kconfirm-linux" +version = "0.10.0" +dependencies = [ + "kconfirm-lib", + "nom-kconfig", +] + +[[package]] +name = "memchr" +version = "2.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" + +[[package]] +name = "nom" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df9761775871bdef83bee530e60050f7e54b1105350d6884eb0fb4f46c2f9405" +dependencies = [ + "memchr", +] + +[[package]] +name = "nom-kconfig" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a0220bb2c8e5ad29b06fe0f75a276affeb10c9504726bf46d81fef78d69b1e3" +dependencies = [ + "nom", + "nom_locate", +] + +[[package]] +name = "nom_locate" +version = "5.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b577e2d69827c4740cba2b52efaad1c4cc7c73042860b199710b3575c68438d" +dependencies = [ + "bytecount", + "memchr", + "nom", +] diff --git a/scripts/kconfirm/Cargo.toml b/scripts/kconfirm/Cargo.toml new file mode 100644 index 000000000000..5880b06c4116 --- /dev/null +++ b/scripts/kconfirm/Cargo.toml @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0 +[workspace] +members = ["kconfirm-lib", "kconfirm-linux"] +resolver = "3" + +[workspace.package] +rust-version = "1.85.0" + +[workspace.dependencies] +nom-kconfig = { version = "0.11", default-features = false, features = [ + "display", +] } diff --git a/scripts/kconfirm/Makefile b/scripts/kconfirm/Makefile new file mode 100644 index 000000000000..6a0b7389103e --- /dev/null +++ b/scripts/kconfirm/Makefile @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0 +# kconfirm makefile + +TARGET := kconfirm + +# Extra arguments forwarded to kconfirm. +# Example: make kconfirm KCONFIRM_ARGS="--enable-check dead_links" +KCONFIRM_ARGS ?= + +$(TARGET): + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS) + + +clean-files += target vendor diff --git a/scripts/kconfirm/kconfirm-lib/Cargo.toml b/scripts/kconfirm/kconfirm-lib/Cargo.toml new file mode 100644 index 000000000000..dd3d7cb1aa1d --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/Cargo.toml @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0 +[package] +name = "kconfirm-lib" +version = "0.10.0" +edition = "2024" +rust-version.workspace = true + +[dependencies] +nom-kconfig = { workspace = true } + +[features] +default = [] diff --git a/scripts/kconfirm/kconfirm-lib/src/analyze.rs b/scripts/kconfirm/kconfirm-lib/src/analyze.rs new file mode 100644 index 000000000000..24798581dc3d --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/src/analyze.rs @@ -0,0 +1,643 @@ +// SPDX-License-Identifier: GPL-2.0-only +use crate::AnalysisArgs; +use crate::Check; +use crate::SymbolTable; +use crate::dead_links; +use crate::dead_links::LinkStatus; +use crate::dead_links::check_link; +use crate::output::Finding; +use crate::output::Severity; +use crate::symbol_table::ChoiceData; +use nom_kconfig::Attribute::*; +use nom_kconfig::Entry; +use nom_kconfig::attribute::DefaultAttribute; +use nom_kconfig::attribute::Expression; +use nom_kconfig::attribute::Imply; +use nom_kconfig::attribute::Select; +use nom_kconfig::attribute::r#type::Type; +use nom_kconfig::entry::Choice; +use nom_kconfig::entry::Config; +use nom_kconfig::entry::If; +use nom_kconfig::entry::Menu; +use nom_kconfig::entry::Source; +use std::collections::HashSet; +use std::option::Option; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +enum FunctionalAttributes { + // only tracking the attributes that affect the semantics, e.g. not help texts + Dependencies, + Selects, + Implies, + Ranges, + Defaults, +} + +struct AttributeGroupingChecker { + current_group: Option<FunctionalAttributes>, + finished_groups: HashSet<FunctionalAttributes>, +} + +impl AttributeGroupingChecker { + fn new() -> Self { + Self { + current_group: None, + finished_groups: HashSet::new(), + } + } + + // doesn't modify `findings` if the style check is disabled + fn check( + &mut self, + group: FunctionalAttributes, + args: &AnalysisArgs, + findings: &mut Vec<Finding>, + symbol: &str, + arch: &String, + message: String, + ) { + if !args.is_enabled(Check::UngroupedAttribute) { + return; + } + + match self.current_group { + // still contiguous + Some(current) if current == group => {} + + // start of group + None => { + self.current_group = Some(group); + } + + Some(current) => { + // the previous group finished + self.finished_groups.insert(current); + + // we've already finished this group, it's ungrouped + if self.finished_groups.contains(&group) { + findings.push(Finding { + severity: Severity::Style, + check: Check::UngroupedAttribute, + symbol: Some(symbol.to_string()), + message, + arch: arch.to_owned(), + }); + } + + // switch to the new group + self.current_group = Some(group); + } + } + } +} + +struct DeadLinkChecker { + visited_links: HashSet<String>, +} + +impl DeadLinkChecker { + fn new() -> Self { + Self { + visited_links: HashSet::new(), + } + } + + fn check_text( + &mut self, + text: &str, + args: &AnalysisArgs, + findings: &mut Vec<Finding>, + symbol: Option<&str>, + arch: &String, + context: &str, + ) { + if !args.is_enabled(Check::DeadLink) { + return; + } + + let links = dead_links::find_links(text); + + if links.is_empty() { + return; + } + + for link in links { + // avoid rechecking identical links + if !self.visited_links.insert(link.clone()) { + continue; + } + + let status = check_link(&link); + if status != LinkStatus::Ok && status != LinkStatus::ProbablyBlocked { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadLink, + symbol: symbol.map(|s| s.to_string()), + message: format!( + "{} contains link {} with status {:?}", + context, link, status + ), + arch: arch.to_owned(), + }); + } + } + } +} + +#[derive(Clone)] +pub struct Context { + pub arch: String, + pub definition_condition: Vec<Expression>, + pub visibility: Vec<Option<Expression>>, + pub dependencies: Vec<Expression>, + pub in_choice: bool, +} + +impl Context { + fn with_arch(arch: String) -> Context { + Context { + arch, + definition_condition: vec![], + visibility: vec![], + dependencies: vec![], + in_choice: false, + } + } + + fn child(&self) -> Self { + self.clone() + } + + fn with_dep(mut self, dep: Expression) -> Self { + self.dependencies.push(dep); + self + } + + fn with_visibility(mut self, cond: Option<Expression>) -> Self { + self.visibility.push(cond); + self + } + + fn with_definition(mut self, cond: Expression) -> Self { + self.definition_condition.push(cond); + self + } + + fn in_choice(mut self) -> Self { + self.in_choice = true; + self + } +} + +fn recurse_entries( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + entries: Vec<Entry>, + ctx: Context, + findings: &mut Vec<Finding>, +) { + for entry in entries { + process_entry(args, symtab, entry, ctx.clone(), findings); + } +} + +pub fn analyze( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + arch: String, + entries: Vec<Entry>, +) -> Vec<Finding> { + let mut findings = Vec::new(); + + let ctx = Context::with_arch(arch); + + recurse_entries(args, symtab, entries, ctx, &mut findings); + + findings +} + +fn handle_config( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + entry: Config, + ctx: &Context, + findings: &mut Vec<Finding>, +) { + let config_symbol = entry.symbol; + + let mut child_ctx = ctx.child(); + + let mut config_type = None; + let mut kconfig_dependencies = Vec::new(); + let mut kconfig_selects: Vec<Select> = Vec::new(); + let mut kconfig_implies: Vec<Imply> = Vec::new(); + let mut kconfig_ranges = Vec::new(); + let mut kconfig_defaults = Vec::new(); + let mut found_prompt = false; + + /* + * style check: ungrouped attributes + * - need to check that dependencies, selects, ranges, and defaults are each kept together. + */ + let mut attribute_grouping_checker = AttributeGroupingChecker::new(); + let mut dead_link_checker = DeadLinkChecker::new(); + for attribute in entry.attributes { + match attribute { + Type(kconfig_type) => match kconfig_type.r#type.clone() { + // hybrid type definition and default + Type::DefBool(db) => { + let default_attribute: DefaultAttribute = DefaultAttribute { + expression: db.clone(), + r#if: kconfig_type.clone().r#if, + }; + + kconfig_defaults.push(default_attribute); + config_type = Some(kconfig_type); + + // NOTE: as a style, we prefer to keep the hybrid default-typedef with the standalone defaults + attribute_grouping_checker.check( + FunctionalAttributes::Defaults, + args, + findings, + &config_symbol, + &ctx.arch, + format!("ungrouped default {}", db), + ); + } + Type::Bool(unconditional_prompt) => { + if unconditional_prompt.is_some() { + found_prompt = true; + } + config_type = Some(kconfig_type); + } + + // hybrid type definition and default + Type::DefTristate(dt) => { + // NOTE: as a style, we prefer to keep the hybrid default-typedef with the standalone defaults + attribute_grouping_checker.check( + FunctionalAttributes::Defaults, + args, + findings, + &config_symbol, + &ctx.arch, + format!("ungrouped default {}", &dt), + ); + + let default_attribute: DefaultAttribute = DefaultAttribute { + expression: dt, + r#if: kconfig_type.clone().r#if, + }; + + kconfig_defaults.push(default_attribute); + config_type = Some(kconfig_type); + } + Type::Tristate(unconditional_prompt) => { + if unconditional_prompt.is_some() { + found_prompt = true; + } + + config_type = Some(kconfig_type.clone()) + } + Type::Hex(unconditional_prompt) => { + if unconditional_prompt.is_some() { + found_prompt = true; + } + + config_type = Some(kconfig_type); + } + Type::Int(unconditional_prompt) => { + if unconditional_prompt.is_some() { + found_prompt = true; + } + config_type = Some(kconfig_type); + } + Type::String(unconditional_prompt) => { + if unconditional_prompt.is_some() { + found_prompt = true; + } + config_type = Some(kconfig_type); + } + }, + Default(default) => { + attribute_grouping_checker.check( + FunctionalAttributes::Defaults, + args, + findings, + &config_symbol, + &ctx.arch, + format!("ungrouped default {}", &default), + ); + + kconfig_defaults.push(default); + } + + DependsOn(depends_on) => { + attribute_grouping_checker.check( + FunctionalAttributes::Dependencies, + args, + findings, + &config_symbol, + &ctx.arch, + format!("ungrouped dependency {}", &depends_on), + ); + + kconfig_dependencies.push(depends_on); + } + Select(select) => { + attribute_grouping_checker.check( + FunctionalAttributes::Selects, + args, + findings, + &config_symbol, + &ctx.arch, + format!("ungrouped select {}", &select), + ); + + kconfig_selects.push(select); + } + Imply(imply) => { + attribute_grouping_checker.check( + FunctionalAttributes::Implies, + args, + findings, + &config_symbol, + &ctx.arch, + format!("ungrouped imply {}", imply), + ); + + kconfig_implies.push(imply); + + // TODO: may be relevant for nonvisible config options when building an SMT model... + } + // NOTE: range bounds are inclusive + Range(r) => { + attribute_grouping_checker.check( + FunctionalAttributes::Ranges, + args, + findings, + &config_symbol, + &ctx.arch, + format!("ungrouped range {}", r), + ); + + kconfig_ranges.push(r); + } + Help(h) => { + // doing nothing for menu help right now + + dead_link_checker.check_text( + &h, + args, + findings, + Some(&config_symbol), + &ctx.arch, + "help text", + ); + } + + Modules => { + // the modules attribute designates this config option as the one that determines if the `m` state is available for tristates options. + + // just making a special note of this in the symtab for now... + symtab.modules_option = Some(config_symbol.clone()); + } + + // the prompt's option `if` determines "visibility" + Prompt(prompt) => { + // TODO: once we have SMT solving, we can also check if the prompt condition is always true or never true (and therefore, effectively unconditional) + + found_prompt = true; + if let Some(c) = prompt.r#if { + child_ctx = child_ctx.with_visibility(Some(c)); + } + } + Transitional => { + // doing nothing for transitional right now + } + Optional | Visible(_) | Requires(_) | Option(_) => { + eprintln!("Error: unexpected attribute encountered: {:?}", attribute); + + if cfg!(debug_assertions) { + panic!(); + } + } + } + } + + if !found_prompt { + child_ctx = child_ctx.with_visibility(None); + } + + // there can be multiple entries that get merged. so we need to do the same for our symtab. + let kconfig_type = config_type.clone().map(|c| c.r#type); + + // at the time of writing this, linux's kconfig only uses Bool inside Choice. + // however, the kconfig documentation doesn't specify whether or not this is guaranteed to be the case. + // we add this check to ensure that we don't cause undefined behavior in future linux versions if something changes... + if child_ctx.in_choice { + if let Some(kt) = &kconfig_type { + match kt { + Type::Bool(_) | Type::DefBool(_) => { + // expected in a choice... + } + + _ => { + // TODO: old versions of linux (like 5.4.4) have tristates in the choice + // - u-boot also currently has hex options in the choice! + eprintln!( + "Error: found something unexpected in a choice-statement: {:?}", + kt + ); + } + } + } + } + + // at the end, add the file's cur_dependencies to this var's invididual dependencies. + kconfig_dependencies.extend(child_ctx.dependencies.clone()); + symtab.merge_insert_new_solved( + config_symbol.clone(), + kconfig_type, + kconfig_dependencies, + //z3_dependency, + kconfig_ranges, + kconfig_defaults, + child_ctx.visibility.clone(), + child_ctx.arch.clone(), + child_ctx.definition_condition.clone(), + None, + kconfig_selects + .clone() + .into_iter() + .map(|sel| (sel.symbol, sel.r#if)) + .collect(), + kconfig_implies + .into_iter() + .map(|imply| (imply.symbol.to_string(), imply.r#if)) + .collect(), + ); + // TODO: file a github issue, imply can never imply a constant (this is technically parsing incorrectly) + + // TODO: when SMT solving, we may need to keep track of the implies the same way we keep track of selects, + // in cases when the implied config option is non-visible + + // need to add the select condition to the definedness condition if it exists + for select in kconfig_selects { + match select.r#if { + None => symtab.merge_insert_new_solved( + select.symbol, + None, + Vec::new(), + Vec::new(), + Vec::new(), + Vec::new(), + child_ctx.arch.clone(), + child_ctx.definition_condition.clone(), + Some((config_symbol.clone(), None)), + Vec::new(), + Vec::new(), + ), + Some(select_condition) => { + symtab.merge_insert_new_solved( + select.symbol, + None, + Vec::new(), + Vec::new(), + Vec::new(), + Vec::new(), + child_ctx.arch.clone(), + child_ctx.definition_condition.clone(), + Some((config_symbol.clone(), Some(select_condition))), + Vec::new(), + Vec::new(), + ); + } + } + } +} + +fn handle_menu( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + entry: Menu, + ctx: &Context, + findings: &mut Vec<Finding>, +) { + // menus can set the visibility of their menu items + + let mut child_ctx = ctx.child(); + + for dep in entry.depends_on { + child_ctx = child_ctx.with_dep(dep.clone()); + child_ctx = child_ctx.with_visibility(Some(dep)); // not a typo, the config options inside of a menu are only visible if the menu's dependencies are satisfied + } + + let nested_entries = entry.entries; + + recurse_entries(args, symtab, nested_entries, child_ctx.clone(), findings); +} + +fn handle_choice( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + entry: Choice, + ctx: &Context, + findings: &mut Vec<Finding>, +) { + let mut child_ctx = ctx.child(); + child_ctx = child_ctx.in_choice(); + + // we are going to add the dependencies of the choice to the dependencies of the entries. + // we start with the dependencies inherited from the file + let mut choice_visibility_condition = None; + let mut defaults = Vec::new(); + for attribute in entry.options { + match attribute { + DependsOn(depends_on) => { + child_ctx = child_ctx.with_dep(depends_on); + } + + Default(default) => { + defaults.push(default); + } + + // the prompt's `if` determines visibility + Prompt(prompt) => { + choice_visibility_condition = prompt.r#if; + if let Some(i) = choice_visibility_condition.clone() { + child_ctx = child_ctx.with_visibility(Some(i)); + } + } + _ => { + // skip + } + } + } + + // all of the variables in the choice menu + //let mut contained_vars = Vec::with_capacity(c.entries.len()); + let nested_entries = entry.entries; + + recurse_entries(args, symtab, nested_entries, child_ctx.clone(), findings); + + let choice_data = ChoiceData { + //inner_vars: contained_vars, + arch: child_ctx.arch.clone(), + visibility: choice_visibility_condition, + dependencies: child_ctx.dependencies, + defaults, + }; + symtab.choices.push(choice_data); +} + +fn handle_if( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + entry: If, + ctx: &Context, + findings: &mut Vec<Finding>, +) { + let mut child_ctx = ctx.child(); + child_ctx = child_ctx.with_definition(entry.condition.clone()); + child_ctx = child_ctx.with_dep(entry.condition); + let nested_entries = entry.entries; + + recurse_entries(args, symtab, nested_entries, child_ctx, findings); +} + +fn handle_source( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + entry: Source, + ctx: &Context, + findings: &mut Vec<Finding>, +) { + let sourced_kconfig = entry.kconfigs; + + for sourced_kconfig in sourced_kconfig { + recurse_entries(args, symtab, sourced_kconfig.entries, ctx.clone(), findings); + } +} + +pub fn process_entry( + args: &AnalysisArgs, + symtab: &mut SymbolTable, + entry: Entry, + ctx: Context, + findings: &mut Vec<Finding>, +) { + // NOTE: in general, each handler should update the context as it encounters that construct. + // e.g. Context.in_choice() should be called at the start of handle_choice(), not right before call to process_entry() when a choice is found and process_entry is called + match entry { + Entry::Config(c) | Entry::MenuConfig(c) => { + handle_config(args, symtab, c, &ctx, findings); + } + Entry::Menu(m) => handle_menu(args, symtab, m, &ctx, findings), + Entry::Choice(c) => handle_choice(args, symtab, c, &ctx, findings), + Entry::If(i) => handle_if(args, symtab, i, &ctx, findings), + Entry::Source(s) => handle_source(args, symtab, s, &ctx, findings), + Entry::Comment(_) => {} + Entry::MainMenu(_) => {} + _ => {} + } +} diff --git a/scripts/kconfirm/kconfirm-lib/src/checks.rs b/scripts/kconfirm/kconfirm-lib/src/checks.rs new file mode 100644 index 000000000000..2ad67f4390ea --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/src/checks.rs @@ -0,0 +1,701 @@ +// SPDX-License-Identifier: GPL-2.0-only +use crate::output::Finding; +use crate::output::Severity; +use crate::symbol_table::AttributeDef; +use crate::symbol_table::TypeInfo; +use nom_kconfig::attribute::Expression; +use nom_kconfig::attribute::range::RangeBound; +use std::collections::HashSet; +use std::num::ParseIntError; +use std::str::FromStr; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Check { + FailedParse, + UngroupedAttribute, // check for duplicate default values, and ungrouped attributes + DeadLink, // check for dead links in the help texts + SelectVisible, + // need SMT solving before we can detect select-undefineds + //SelectUndefined, + DuplicateDependency, + DuplicateRange, + DeadRange, + DuplicateSelect, + DeadSelect, + DeadDefault, + ConstantCondition, + DuplicateDefault, + DuplicateDefaultValue, + DuplicateImply, + DeadImply, + ReverseRange, +} + +impl Check { + pub fn as_str(self) -> &'static str { + match self { + Check::FailedParse => "failed_parse", + Check::UngroupedAttribute => "ungrouped_attribute", + Check::DeadLink => "dead_link", + Check::SelectVisible => "select_visible", + Check::DuplicateDependency => "duplicate_dependency", + Check::DuplicateRange => "duplicate_range", + Check::DeadRange => "dead_range", + Check::DuplicateSelect => "duplicate_select", + Check::DeadSelect => "dead_select", + Check::DeadDefault => "dead_default", + Check::ConstantCondition => "constant_condition", + Check::DuplicateDefault => "duplicate_default", + Check::DuplicateDefaultValue => "duplicate_default_value", + Check::DuplicateImply => "duplicate_imply", + Check::DeadImply => "dead_imply", + Check::ReverseRange => "reverse_range", + } + } +} + +#[derive(Debug)] +pub struct ParseCheckError { + pub input: String, +} + +impl std::fmt::Display for ParseCheckError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "unknown check '{}'", self.input) + } +} + +impl std::error::Error for ParseCheckError {} + +impl FromStr for Check { + type Err = ParseCheckError; + + fn from_str(name: &str) -> Result<Self, Self::Err> { + match name { + "failed_parse" => Ok(Check::FailedParse), + "ungrouped_attribute" => Ok(Check::UngroupedAttribute), + "dead_link" => Ok(Check::DeadLink), + "select_visible" => Ok(Check::SelectVisible), + "duplicate_dependency" => Ok(Check::DuplicateDependency), + "duplicate_range" => Ok(Check::DuplicateRange), + "dead_range" => Ok(Check::DeadRange), + "duplicate_select" => Ok(Check::DuplicateSelect), + "dead_select" => Ok(Check::DeadSelect), + "dead_default" => Ok(Check::DeadDefault), + "constant_condition" => Ok(Check::ConstantCondition), + "duplicate_default" => Ok(Check::DuplicateDefault), + "duplicate_default_value" => Ok(Check::DuplicateDefaultValue), + "duplicate_imply" => Ok(Check::DuplicateImply), + "dead_imply" => Ok(Check::DeadImply), + "reverse_range" => Ok(Check::ReverseRange), + _ => Err(ParseCheckError { + input: name.to_string(), + }), + } + } +} + +#[derive(Clone, Debug)] +pub struct AnalysisArgs { + // check for duplicate default values + pub enabled_checks: HashSet<Check>, +} + +impl AnalysisArgs { + pub fn is_enabled(&self, check: Check) -> bool { + self.enabled_checks.contains(&check) + } +} + +// returns an Error if a hex range bound cannot be parsed as an u64 +pub fn check_reverse_ranges(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { + let mut findings = Vec::new(); + + for range in &info.kconfig_ranges { + // returns an Error if a hex range bound cannot be parsed as an u64 + fn range_bound_to_int(range_bound: &RangeBound) -> Result<i128, ParseIntError> { + match range_bound { + RangeBound::Number(b) => { + return Ok(b.to_owned() as i128); + } + RangeBound::Hex(b_str) => { + let trimmed = b_str.trim_start_matches("0x").trim_start_matches("0X"); + + return i128::from_str_radix(trimmed, 16); + } + RangeBound::Variable(_) => { + // for now, the caller is expected not to pass these cases. + unreachable!("not handling variable ranges until SMT solving"); + } + RangeBound::Symbol(_) => { + // TODO: need SMT solving for this case + // for now, the caller is expected not to pass these cases. + unreachable!("not handling CONFIG ranges until SMT solving"); + } + } + } + + if matches!(range.lower_bound, RangeBound::Symbol(_)) + || matches!(range.upper_bound, RangeBound::Symbol(_)) + { + // not handling these cases until SMT solving. + // don't return though, because we stil want to check the other ranges. + continue; + } + + let maybe_lower_bound = range_bound_to_int(&range.lower_bound); + let maybe_upper_bound = range_bound_to_int(&range.upper_bound); + + match (maybe_lower_bound, maybe_upper_bound) { + (Ok(lower_bound), Ok(upper_bound)) => { + if lower_bound > upper_bound { + let message = format!( + "reverse range {} for config option: {}, no value is valid", + range.to_string(), + var_symbol, + ); + findings.push(Finding { + severity: Severity::Warning, + check: Check::ReverseRange, + symbol: Some(var_symbol.to_owned()), + arch: arch.to_owned(), + message, + }); + } + } + (Result::Err(_), _) | (_, Result::Err(_)) => { + eprintln!( + "Error: couldn't parse hex range bound as i128 for config option: {}", + var_symbol + ); + // still want to check the other range bounds + continue; + } + } + } + + findings +} + +pub fn check_constant_conditions( + arch: &String, + var_symbol: &str, + info: &AttributeDef, +) -> Vec<Finding> { + let mut findings = Vec::new(); + let default_conditions: Vec<&Expression> = info + .kconfig_defaults + .iter() + .filter_map(|conditional_default| conditional_default.r#if.as_ref()) + .collect(); + + check_conditions( + arch, + &mut findings, + &var_symbol, + &info.kconfig_dependencies, + default_conditions, + "default", + ); + + let select_conditions: Vec<&Expression> = info + .selects + .iter() + .filter_map(|conditional_select| conditional_select.1.as_ref()) + .collect(); + + check_conditions( + arch, + &mut findings, + var_symbol, + &info.kconfig_dependencies, + select_conditions, + "select", + ); + + let imply_conditions: Vec<&Expression> = info + .implies + .iter() + .filter_map(|imp| imp.1.as_ref()) + .collect(); + + check_conditions( + arch, + &mut findings, + var_symbol, + &info.kconfig_dependencies, + imply_conditions, + "imply", + ); + + let range_conditions: Vec<&Expression> = info + .kconfig_ranges + .iter() + .filter_map(|conditional_range| conditional_range.r#if.as_ref()) + .collect(); + + check_conditions( + arch, + &mut findings, + var_symbol, + &info.kconfig_dependencies, + range_conditions, + "range", + ); + + fn check_conditions( + arch: &String, + findings: &mut Vec<Finding>, + symbol: &str, + kconfig_dependencies: &[Expression], + attribute_conditions: Vec<&Expression>, + context: &str, + ) { + for attribute_condition in attribute_conditions.into_iter() { + if kconfig_dependencies.contains(attribute_condition) { + let message = format!( + "constant {} condition 'if {}' for config option: {}, this condition is a dependency and will always be true", + context, + attribute_condition.to_string(), + symbol, + ); + findings.push(Finding { + severity: Severity::Warning, + check: Check::ConstantCondition, + symbol: Some(symbol.to_owned()), + arch: arch.to_owned(), + message, + }); + } + } + } + findings +} + +pub fn check_variable_info( + args: &AnalysisArgs, + var_symbol: &str, + arch_specific: &String, + info: &AttributeDef, +) -> Vec<Finding> { + let mut findings = Vec::new(); + + if args.is_enabled(Check::DuplicateDependency) { + findings.extend(check_duplicate_dependencies( + arch_specific, + var_symbol, + info, + )); + } + + if args.is_enabled(Check::DuplicateImply) { + findings.extend(check_duplicate_implies(arch_specific, var_symbol, info)); + } + + if args.is_enabled(Check::DuplicateRange) { + findings.extend(check_duplicate_ranges(arch_specific, var_symbol, info)); + } + + if args.is_enabled(Check::DuplicateSelect) { + findings.extend(check_duplicate_selects(arch_specific, var_symbol, info)); + } + + if args.is_enabled(Check::ConstantCondition) { + findings.extend(check_constant_conditions(arch_specific, var_symbol, info)); + } + + if args.is_enabled(Check::DeadDefault) + || args.is_enabled(Check::DuplicateDefault) + || args.is_enabled(Check::DuplicateDefaultValue) + { + findings.extend(check_defaults(arch_specific, var_symbol, info, args)); + } + + if args.is_enabled(Check::ReverseRange) { + findings.extend(check_reverse_ranges(arch_specific, var_symbol, info)); + } + + findings +} + +// TODO: also check if a config option in one arch unconditionally references a config option that only exists in another arch (need SMT for this first) +pub fn check_select_visible(var_symbol: &str, info: &TypeInfo) -> Vec<Finding> { + let mut findings = Vec::new(); + + // only interested in the options that are selected + if info.selected_by.is_empty() { + return Vec::new(); + } + + for (selector, select_info) in &info.selected_by { + for (arch, _cond) in select_info { + // NOTE: we don't care if the select is conditional or unconditional, just the selectee's visibility + + // at this point, we know that `selector` unconditionally selects `var_symbol` + // now, we need to check if `var_symbol` is unconditionally visible + + let message = format!( + "selects the visible {}; consider using 'depends on' or 'imply' instead", + var_symbol + ); + + // match the architecture that the select happens under with the architecture of the unconditional visibility + match info.attribute_defs.get(arch) { + None => { + // not selected in this architecture + } + Some(cur_arch_attribute_def) => { + for (if_conditions, attributes) in cur_arch_attribute_def { + if if_conditions.is_empty() && attributes.visibility.is_empty() { + // empty visiblity means that it is unconditionally visible, within the current arch (assuming arch is not `None`) + + findings.push(Finding { + severity: Severity::Warning, + check: Check::SelectVisible, + symbol: Some(selector.to_owned()), + message: message.clone(), + arch: arch.to_owned(), + }); + } + } + } + } + } + } + + findings +} + +fn is_duplicate<T: Eq + std::hash::Hash>(set: &mut HashSet<T>, key: T) -> bool { + !set.insert(key) +} + +fn check_duplicate_dependencies( + arch_specific: &String, + var_symbol: &str, + info: &AttributeDef, +) -> Vec<Finding> { + let mut findings = Vec::new(); + let mut seen = HashSet::new(); + + for dep in &info.kconfig_dependencies { + if is_duplicate(&mut seen, dep.to_string()) { + let message = format!("duplicate dependency on {}", dep.to_string()); + findings.push(Finding { + severity: Severity::Warning, + check: Check::DuplicateDependency, + symbol: Some(var_symbol.to_owned()), + message, + arch: arch_specific.to_owned(), + }); + } + } + + findings +} + +fn check_duplicate_implies(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { + let mut findings = Vec::new(); + + // symbols implied unconditionally + let mut unconditional: HashSet<String> = HashSet::new(); + + // (symbol, condition) + let mut conditional: HashSet<(String, String)> = HashSet::new(); + + for imp in &info.implies { + let imply_var = imp.0.clone(); + + match &imp.1 { + Some(cond) => { + let cond_str = cond.to_string(); + + // duplicate conditional imply + if !conditional.insert((imply_var.clone(), cond_str.clone())) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DuplicateImply, + symbol: Some(var_symbol.to_owned()), + message: format!( + "duplicate imply of {:?} with condition {}", + imp.0, cond_str + ), + arch: arch.to_owned(), + }); + } + + // conditional imply is dead if unconditional exists + if unconditional.contains(&imply_var) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadImply, + symbol: Some(var_symbol.to_owned()), + message: format!("dead imply of {:?}", imp), + arch: arch.to_owned(), + }); + } + } + + None => { + // duplicate unconditional imply + if !unconditional.insert(imply_var.clone()) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DuplicateImply, + symbol: Some(var_symbol.to_owned()), + message: format!("duplicate imply of {:?}", imp), + arch: arch.to_owned(), + }); + } + + // previous conditionals with same symbol are dead + for (sym, _) in &conditional { + if sym == &imply_var { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadImply, + symbol: Some(var_symbol.to_owned()), + message: format!("dead imply of {:?}", imp), + arch: arch.to_owned(), + }); + } + } + } + } + } + + findings +} + +fn check_duplicate_ranges(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { + let mut findings = Vec::new(); + + // unconditional ranges by bounds + let mut unconditional: HashSet<String> = HashSet::new(); + + // (bounds, condition) + let mut conditional: HashSet<(String, String)> = HashSet::new(); + + for range in &info.kconfig_ranges { + // uniquely identify the range bounds + let range_key = format!("{} {}", range.lower_bound, range.upper_bound); + + match &range.r#if { + Some(cond) => { + let cond_str = cond.to_string(); + + // duplicate conditional range + if !conditional.insert((range_key.clone(), cond_str.clone())) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DuplicateRange, + symbol: Some(var_symbol.to_owned()), + message: format!("duplicate range {:?} with condition {}", range, cond_str), + arch: arch.to_owned(), + }); + } + + // conditional range is dead if unconditional exists + if unconditional.contains(&range_key) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadRange, + symbol: Some(var_symbol.to_owned()), + message: format!("dead range of {:?}", range), + arch: arch.to_owned(), + }); + } + } + + None => { + // duplicate unconditional range + if !unconditional.insert(range_key.clone()) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadRange, + symbol: Some(var_symbol.to_owned()), + message: format!("duplicate range {:?}", range), + arch: arch.to_owned(), + }); + } + + // previous conditionals with same bounds are dead + for (bounds, _) in &conditional { + if bounds == &range_key { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadRange, + symbol: Some(var_symbol.to_owned()), + message: format!("dead range of {:?}", range), + arch: arch.to_owned(), + }); + } + } + } + } + } + + findings +} + +fn check_duplicate_selects(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { + let mut findings = Vec::new(); + + // symbols selected unconditionally + let mut unconditional: HashSet<String> = HashSet::new(); + + // (symbol, condition) + let mut conditional: HashSet<(String, String)> = HashSet::new(); + + for select in &info.selects { + let select_var = select.0.clone(); + + match &select.1 { + Some(cond) => { + let cond_str = cond.to_string(); + + // duplicate conditional select + if !conditional.insert((select_var.clone(), cond_str.clone())) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DuplicateSelect, + symbol: Some(var_symbol.to_owned()), + message: format!( + "duplicate select of {:?} with condition {}", + select.0, cond_str + ), + arch: arch.to_owned(), + }); + } + + // conditional is dead if unconditional exists + if unconditional.contains(&select_var) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadSelect, + symbol: Some(var_symbol.to_owned()), + message: format!("dead select of {:?}", select.0), + arch: arch.to_owned(), + }); + } + } + + None => { + // duplicate unconditional select + if !unconditional.insert(select_var.clone()) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DuplicateSelect, + symbol: Some(var_symbol.to_owned()), + message: format!("duplicate select of {:?}", select.0), + arch: arch.to_owned(), + }); + } + + // any previous conditional selects are now dead too + for (sym, _) in &conditional { + if sym == &select_var { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadSelect, + symbol: Some(var_symbol.to_owned()), + message: format!("dead select of {:?}", select.0), + arch: arch.to_owned(), + }); + } + } + } + } + } + + findings +} + +#[allow(clippy::collapsible_if)] +fn check_defaults( + arch: &String, + var_symbol: &str, + info: &AttributeDef, + args: &AnalysisArgs, +) -> Vec<Finding> { + let mut findings = Vec::new(); + let mut seen_conditions = HashSet::new(); + let mut seen_values = HashSet::new(); + let mut already_unconditional = false; + + for default in &info.kconfig_defaults { + let val_str = default.expression.to_string(); + + let has_real_condition = match &default.r#if { + Some(cond) => { + let cond_str = cond.to_string(); + !cond_str.is_empty() + } + None => false, + }; + + let is_value_dup = if has_real_condition { + is_duplicate(&mut seen_values, val_str.clone()) + } else { + false + }; + + if already_unconditional && args.is_enabled(Check::DeadDefault) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadDefault, + symbol: Some(var_symbol.to_owned()), + message: format!("dead default of {}", val_str), + arch: arch.to_owned(), + }); + } + + if args.is_enabled(Check::DuplicateDefaultValue) { + if default.r#if.is_some() && is_value_dup { + findings.push(Finding { + severity: Severity::Style, + check: Check::DuplicateDefaultValue, + symbol: Some(var_symbol.to_owned()), + message: format!( + "duplicate default value of {}; consider combining the conditions with a logical-or: ||", + val_str + ), + arch: arch.to_owned(), + }); + } + } + + match &default.r#if { + Some(cond) => { + if is_duplicate(&mut seen_conditions, cond.to_string()) { + if is_value_dup { + if args.is_enabled(Check::DuplicateDefault) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DuplicateDefault, + symbol: Some(var_symbol.to_owned()), + message: format!("duplicate default condition of {:?}", cond), + arch: arch.to_owned(), + }); + } + } else { + if args.is_enabled(Check::DeadDefault) { + findings.push(Finding { + severity: Severity::Warning, + check: Check::DeadDefault, + symbol: Some(var_symbol.to_owned()), + message: format!("dead default of {}", val_str), + arch: arch.to_owned(), + }); + } + } + } + } + None => { + already_unconditional = true; + } + } + } + + findings +} diff --git a/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs b/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs new file mode 100644 index 000000000000..d458010cc3f1 --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: GPL-2.0-only +use core::ffi::c_void; +use std::ffi::CStr; +use std::ffi::CString; +use std::os::raw::c_char; +use std::os::raw::c_int; +use std::os::raw::c_long; +use std::sync::OnceLock; + +static CURL_INIT: OnceLock<()> = OnceLock::new(); + +#[repr(C)] +pub struct CURL { + _private: [u8; 0], +} + +type CURLcode = c_int; +type CURLoption = u32; +type CURLINFO = u32; + +const CURLE_OK: CURLcode = 0; + +const CURL_GLOBAL_DEFAULT: c_long = 3; + +const CURLOPT_URL: CURLoption = 10002; +const CURLOPT_NOBODY: CURLoption = 44; +const CURLOPT_TIMEOUT: CURLoption = 13; +const CURLOPT_FOLLOWLOCATION: CURLoption = 52; +const CURLOPT_USERAGENT: CURLoption = 10018; +const CURLOPT_HEADERFUNCTION: CURLoption = 20079; +const CURLOPT_HEADERDATA: CURLoption = 10029; + +const CURLINFO_RESPONSE_CODE: CURLINFO = 0x200002; + +#[link(name = "curl")] +unsafe extern "C" {} + +unsafe extern "C" { + fn curl_global_init(flags: c_long) -> CURLcode; + + fn curl_easy_init() -> *mut CURL; + + fn curl_easy_cleanup(handle: *mut CURL); + + fn curl_easy_perform(handle: *mut CURL) -> CURLcode; + + fn curl_easy_strerror(code: CURLcode) -> *const c_char; + + fn curl_easy_setopt(handle: *mut CURL, option: CURLoption, ...) -> CURLcode; + + fn curl_easy_getinfo(handle: *mut CURL, info: CURLINFO, ...) -> CURLcode; +} + +fn init_curl() { + CURL_INIT.get_or_init(|| unsafe { + curl_global_init(CURL_GLOBAL_DEFAULT); + }); +} + +fn curl_error(code: CURLcode) -> String { + unsafe { + let ptr = curl_easy_strerror(code); + + if ptr.is_null() { + return format!("curl error {}", code); + } + + CStr::from_ptr(ptr).to_string_lossy().into_owned() + } +} + +struct HeaderCapture { + location: Option<String>, +} + +extern "C" fn header_callback( + buffer: *mut c_char, + size: usize, + nitems: usize, + userdata: *mut c_void, +) -> usize { + let total = size * nitems; + + unsafe { + let bytes = std::slice::from_raw_parts(buffer as *const u8, total); + + if let Ok(header) = std::str::from_utf8(bytes) { + let lower = header.to_ascii_lowercase(); + + if lower.starts_with("location:") { + if let Some((_, value)) = header.split_once(':') { + let capture = &mut *(userdata as *mut HeaderCapture); + + capture.location = Some(value.trim().to_string()); + } + } + } + } + + total +} + +#[derive(Debug)] +pub struct HttpResponse { + pub response_code: u16, + pub location: Option<String>, +} + +pub fn head_request(url: &str) -> Result<HttpResponse, String> { + init_curl(); + + unsafe { + let curl = curl_easy_init(); + + if curl.is_null() { + return Err("curl_easy_init failed".into()); + } + + let url_c = match CString::new(url) { + Ok(v) => v, + Err(_) => { + curl_easy_cleanup(curl); + + return Err("invalid URL".into()); + } + }; + + let ua_c = CString::new("link-checker/1.0").unwrap(); + + let mut headers = HeaderCapture { location: None }; + + macro_rules! setopt { + ($opt:expr, $val:expr) => {{ + let rc = curl_easy_setopt(curl, $opt, $val); + + if rc != CURLE_OK { + curl_easy_cleanup(curl); + + return Err(curl_error(rc)); + } + }}; + } + + setopt!(CURLOPT_URL, url_c.as_ptr()); + setopt!(CURLOPT_NOBODY, 1 as c_long); + setopt!(CURLOPT_TIMEOUT, 10 as c_long); + setopt!(CURLOPT_FOLLOWLOCATION, 0 as c_long); + setopt!(CURLOPT_USERAGENT, ua_c.as_ptr()); + + setopt!( + CURLOPT_HEADERFUNCTION, + header_callback as extern "C" fn(_, _, _, _) -> _ + ); + + setopt!(CURLOPT_HEADERDATA, &mut headers as *mut _ as *mut c_void); + + let rc = curl_easy_perform(curl); + + if rc != CURLE_OK { + curl_easy_cleanup(curl); + + return Err(curl_error(rc)); + } + + let mut response_code: c_long = 0; + + let rc = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &mut response_code); + + if rc != CURLE_OK { + curl_easy_cleanup(curl); + + return Err(curl_error(rc)); + } + + curl_easy_cleanup(curl); + + Ok(HttpResponse { + response_code: response_code as u16, + location: headers.location, + }) + } +} diff --git a/scripts/kconfirm/kconfirm-lib/src/dead_links.rs b/scripts/kconfirm/kconfirm-lib/src/dead_links.rs new file mode 100644 index 000000000000..47bbd5c09114 --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/src/dead_links.rs @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0-only +use crate::curl_ffi::head_request; +use std::collections::HashSet; + +#[derive(PartialEq, Debug)] +pub enum LinkStatus { + Ok, + ProbablyBlocked, + Redirected(String), + NotFound, + ServerError, + Unreachable(String), + UnsupportedScheme(String), +} + +pub fn check_link(url: &str) -> LinkStatus { + if let Some(scheme) = url.split("://").next() { + match scheme { + "http" | "https" => return check_http(url), + + "git" | "ftp" => { + return LinkStatus::UnsupportedScheme(scheme.into()); + } + + _ => { + return LinkStatus::UnsupportedScheme(scheme.into()); + } + } + } + + LinkStatus::Unreachable("invalid URL".into()) +} + +fn check_http(url: &str) -> LinkStatus { + let response = match head_request(url) { + Ok(r) => r, + Err(e) => return LinkStatus::Unreachable(e), + }; + + match response.response_code { + 200..=299 => LinkStatus::Ok, + + 301 | 302 => LinkStatus::Redirected(response.location.unwrap_or_else(|| "unknown".into())), + + 403 | 429 => LinkStatus::ProbablyBlocked, + + 404 => LinkStatus::NotFound, + + 500..=599 => LinkStatus::ServerError, + + _ => LinkStatus::ProbablyBlocked, + } +} + +pub fn find_links(text: &str) -> Vec<String> { + fn is_scheme_char(c: u8) -> bool { + c.is_ascii_alphanumeric() || matches!(c, b'+' | b'-' | b'.') + } + + fn is_url_terminator(c: u8) -> bool { + c.is_ascii_whitespace() + || matches!( + c, + b'"' | b'\'' | b'<' | b'>' | b'(' | b')' | b'[' | b']' | b'{' | b'}' + ) + } + + let bytes = text.as_bytes(); + + let mut links = Vec::new(); + let mut seen = HashSet::new(); + + let mut i = 0; + + while i + 3 < bytes.len() { + if bytes[i] == b':' && bytes[i + 1] == b'/' && bytes[i + 2] == b'/' { + // walk backward to find scheme start + let mut start = i; + + while start > 0 && is_scheme_char(bytes[start - 1]) { + start -= 1; + } + + // require non-empty scheme + if start == i { + i += 3; + continue; + } + + // first char must be alphabetic + if !bytes[start].is_ascii_alphabetic() { + i += 3; + continue; + } + + // walk forward to url end + let mut end = i + 3; + + while end < bytes.len() && !is_url_terminator(bytes[end]) { + end += 1; + } + + let mut url = &text[start..end]; + + // trim trailing punctuation + url = url.trim_end_matches(&['.', ',', ';', ':', '!', '?'][..]); + + // trim unmatched markdown + while let Some(last) = url.chars().last() { + let trim = match last { + ')' => url.matches('(').count() < url.matches(')').count(), + + ']' => url.matches('[').count() < url.matches(']').count(), + + '}' => url.matches('{').count() < url.matches('}').count(), + + _ => false, + }; + + if trim { + url = &url[..url.len() - last.len_utf8()]; + } else { + break; + } + } + + if seen.insert(url) { + links.push(url.to_string()); + } + + i = end; + } else { + i += 1; + } + } + + links +} diff --git a/scripts/kconfirm/kconfirm-lib/src/lib.rs b/scripts/kconfirm/kconfirm-lib/src/lib.rs new file mode 100644 index 000000000000..6be0199f0785 --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/src/lib.rs @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: GPL-2.0-only +use analyze::analyze; +pub use checks::AnalysisArgs; +pub use checks::Check; +pub use checks::check_select_visible; +pub use checks::check_variable_info; +use nom_kconfig::Entry; +use nom_kconfig::KconfigInput; +use nom_kconfig::parse_kconfig; +use output::*; +use symbol_table::*; +mod analyze; +mod checks; +mod curl_ffi; +mod dead_links; +pub mod output; +pub mod symbol_table; + +pub fn check_kconfig( + args: AnalysisArgs, + kconfig_files: Vec<(String, KconfigInput)>, +) -> Vec<Finding> { + let mut findings = Vec::new(); + let mut symbol_table = SymbolTable::new(); + + for (arch_config_option, kconfig_file) in kconfig_files { + match parse_kconfig(kconfig_file) { + Ok(parsed) => { + let entries: Vec<Entry> = parsed.1.entries; + findings.extend(analyze( + &args, + &mut symbol_table, + arch_config_option, + entries, + )); + } + Err(e) => { + findings.push(Finding { + severity: Severity::Fatal, + check: Check::FailedParse, + symbol: None, + message: format!("Failed to parse kconfig, error is: {}", e), + arch: arch_config_option, + }); + } + } + } + + for (var_symbol, type_info) in &symbol_table.raw { + for (arch_specific, redefinitions) in &type_info.attribute_defs { + for (_definition_condition, info) in redefinitions { + findings.extend(check_variable_info(&args, var_symbol, arch_specific, info)); + } + } + + if args.is_enabled(Check::SelectVisible) { + findings.extend(check_select_visible(var_symbol, type_info)); + } + } + + findings +} diff --git a/scripts/kconfirm/kconfirm-lib/src/output.rs b/scripts/kconfirm/kconfirm-lib/src/output.rs new file mode 100644 index 000000000000..e0d8bf8342d5 --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/src/output.rs @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-2.0-only +use crate::Check; +use std::fmt; + +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] +pub enum Severity { + Fatal, + Error, // will be used for known bugs, e.g. unmet dependencies + Warning, + Style, +} + +#[derive(Debug)] +pub struct Finding { + pub severity: Severity, + pub check: Check, + pub symbol: Option<String>, + pub message: String, + pub arch: String, +} + +impl Finding { + fn fmt_with_arches(&self, f: &mut fmt::Formatter, arches: &[&str]) -> fmt::Result { + let arch_part = if arches.is_empty() { + String::new() + } else { + format!(" [{}]", arches.join(", ")) + }; + + match &self.symbol { + Some(s) => write!( + f, + "{} [{}]{} config {}: {}", + self.severity, + self.check.as_str(), + arch_part, + s, + self.message + ), + None => write!( + f, + "{} [{}]{} {}", + self.severity, + self.check.as_str(), + arch_part, + self.message + ), + } + } +} + +impl fmt::Display for Finding { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.fmt_with_arches(f, &[]) + } +} + +pub fn print_findings(mut findings: Vec<Finding>) { + findings.sort_by(|a, b| { + ( + &a.severity, + a.check.as_str(), + &a.symbol, + &a.message, + &a.arch, + ) + .cmp(&( + &b.severity, + b.check.as_str(), + &b.symbol, + &b.message, + &b.arch, + )) + }); + + for group in findings.chunk_by(|a, b| { + a.severity == b.severity + && a.check.as_str() == b.check.as_str() + && a.symbol == b.symbol + && a.message == b.message + }) { + let head = &group[0]; + + let mut arches: Vec<&str> = Vec::new(); + for f in group { + if arches.last() != Some(&f.arch.as_str()) { + arches.push(&f.arch); + } + } + + // Use a small wrapper so we can call our custom formatter via println! + struct Wrap<'a>(&'a Finding, &'a [&'a str]); + impl fmt::Display for Wrap<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt_with_arches(f, self.1) + } + } + println!("{}", Wrap(head, &arches)); + } +} + +impl fmt::Display for Severity { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Severity::Fatal => write!(f, "FATAL "), + Severity::Error => write!(f, "ERROR "), + Severity::Warning => write!(f, "WARNING"), + Severity::Style => write!(f, "STYLE "), + } + } +} diff --git a/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs b/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs new file mode 100644 index 000000000000..48abb46c1945 --- /dev/null +++ b/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs @@ -0,0 +1,223 @@ +// SPDX-License-Identifier: GPL-2.0-only +use nom_kconfig::attribute::DefaultAttribute; +use nom_kconfig::attribute::Expression; +use nom_kconfig::attribute::OrExpression; +use nom_kconfig::attribute::Range; +use nom_kconfig::attribute::r#type::Type; +use std::collections::HashMap; +use std::collections::hash_map; + +type KconfigSymbol = String; +type Arch = String; +type Cond = Option<Expression>; + +// NOTE: we cannot add these elements to the solver until we've processed all variables, +// because we need to know all of the selectors. +#[derive(Debug, Clone)] +pub struct TypeInfo { + pub kconfig_type: Option<Type>, // 'None' when we don't know the type (e.g. if it's a dangling reference) + + // maps the selector to an (ARCH, select_cond) + // - if the ARCH is None, then it's not arch-specific + // if the select_cond is None, then it's unconditional + pub selected_by: HashMap<KconfigSymbol, Vec<(Arch, Cond)>>, // .0 only selects it when .1 is true. + + // there is one of these per entry (each entry expected to have a different definedness condition) + // maps architecture option name (or none if not arch-specific) to: + // [([condition], config definition)] + // - NOTE: there can be multiple partial definitions under the same condition, or mutually-exclusive conditions, or a subset condition. + pub attribute_defs: HashMap<Arch, Vec<(Vec<Expression>, AttributeDef)>>, // the innermost `Vec<Expression>` represents each nested condition that was reached (we will eventually need to AND them all) +} + +// everything is a vector because we may encounter multiple over time, +// so we won't know until the end what the condition is. +#[derive(Debug, Clone)] +pub struct AttributeDef { + pub kconfig_dependencies: Vec<OrExpression>, + pub kconfig_ranges: Vec<Range>, + pub kconfig_defaults: Vec<DefaultAttribute>, + pub visibility: Vec<Option<OrExpression>>, + pub selects: Vec<(KconfigSymbol, Cond)>, + pub implies: Vec<(KconfigSymbol, Cond)>, +} + +impl TypeInfo { + fn new_empty() -> Self { + Self { + kconfig_type: None, + selected_by: HashMap::new(), + attribute_defs: HashMap::new(), + } + } + + // TODO: we should consider having separate functions for: + // 1. merge-inserting a redef of attributes (NOTE: the type definition is actually part of the redef, but we aren't handling type-redefinitions for now) + // 2. selectors + fn insert( + &mut self, + kconfig_type: Option<Type>, + raw_constraints: Vec<OrExpression>, + kconfig_ranges: Vec<Range>, + kconfig_defaults: Vec<DefaultAttribute>, + visibility: Vec<Option<OrExpression>>, + arch: String, + definition_condition: Vec<OrExpression>, + selected_by: Option<(KconfigSymbol, Cond)>, + selects: Vec<(KconfigSymbol, Cond)>, + implies: Vec<(KconfigSymbol, Cond)>, + ) { + // type merge + match (&self.kconfig_type, &kconfig_type) { + (None, Some(_)) => self.kconfig_type = kconfig_type.clone(), + (Some(_), Some(new)) if Some(new) != self.kconfig_type.as_ref() => { + // TODO: not doing anything with redefined types yet. + // later, we will want to consider e.g. bool/def_bool the same type (and possibly int/hex?) but not bool/tristate, so we need to build out typechecking. + } + _ => {} + } + + // selected_by merge + if let Some(sb) = selected_by { + merge_selected_by(&mut self.selected_by, arch.clone(), sb); + } + + // variable_info merge: + // we only want to add an attribute redefinition if the things in the attribute def aren't empty + // (the visibility is just additional info to capture) + if (&kconfig_type).is_some() // we need to ensure that we have an empty definition here if the config option had a type definition + || !raw_constraints.is_empty() + || !kconfig_ranges.is_empty() + || !kconfig_defaults.is_empty() + || !selects.is_empty() + || !implies.is_empty() + { + insert_variable_info( + &mut self.attribute_defs, + arch, + definition_condition, + AttributeDef { + kconfig_dependencies: raw_constraints, + kconfig_ranges, + kconfig_defaults, + visibility, + selects, + implies, + }, + ); + } + } +} + +// the visibility and the dependencies will each need to be AND'd (separately) +// the defaults should each be handled separately. +pub struct ChoiceData { + //pub inner_vars: Vec<String>, + pub arch: Arch, + pub visibility: Cond, + pub dependencies: Vec<OrExpression>, // this is the menu's dependencies (and inherited dependencies from the file) + pub defaults: Vec<DefaultAttribute>, // these are each of the conditional defaults for the choice +} + +// NOTE: it might be better if TypeInfo is an enum with a single value, +// e.g. Unsolved(kconfig_raw) and Solved(z3_ast) +pub struct SymbolTable { + pub raw: HashMap<KconfigSymbol, TypeInfo>, + pub choices: Vec<ChoiceData>, + pub modules_option: Option<KconfigSymbol>, // None until we find the modules attribute in exactly 1 config option +} + +impl SymbolTable { + pub fn new() -> Self { + SymbolTable { + raw: HashMap::new(), + choices: Vec::new(), + modules_option: None, + } + } + + pub fn from_parts( + raw: HashMap<KconfigSymbol, TypeInfo>, + choices: Vec<ChoiceData>, + modules_option: Option<KconfigSymbol>, + ) -> Self { + SymbolTable { + raw, + choices, + modules_option, + } + } + + pub fn merge_insert_new_solved( + &mut self, + var: KconfigSymbol, + kconfig_type: Option<Type>, + raw_constraints: Vec<OrExpression>, + kconfig_ranges: Vec<Range>, + kconfig_defaults: Vec<DefaultAttribute>, + visibility: Vec<Option<OrExpression>>, + arch: Arch, + definition_condition: Vec<OrExpression>, + selected_by: Option<(KconfigSymbol, Cond)>, + selects: Vec<(KconfigSymbol, Cond)>, + implies: Vec<(KconfigSymbol, Cond)>, + ) { + let entry = self.raw.entry(var.clone()); + + match entry { + hash_map::Entry::Vacant(v) => { + let mut t = TypeInfo::new_empty(); + t.insert( + kconfig_type, + raw_constraints, + kconfig_ranges, + kconfig_defaults, + visibility, + arch, + definition_condition, + selected_by, + selects, + implies, + ); + v.insert(t); + } + + hash_map::Entry::Occupied(mut o) => { + let t = o.get_mut(); + + t.insert( + kconfig_type, + raw_constraints, + kconfig_ranges, + kconfig_defaults, + visibility, + arch, + definition_condition, + selected_by, + selects, + implies, + ); + } + } + } +} + +fn merge_selected_by( + map: &mut HashMap<String, Vec<(Arch, Cond)>>, + arch: Arch, + selected_by: (KconfigSymbol, Cond), +) { + map.entry(selected_by.0) + .or_default() // empty vec + .push((arch, selected_by.1)); +} + +fn insert_variable_info( + map: &mut HashMap<Arch, Vec<(Vec<Expression>, AttributeDef)>>, + arch: Arch, + definition_condition: Vec<Expression>, + info: AttributeDef, +) { + map.entry(arch) + .or_default() // empty vec + .push((definition_condition, info)); +} diff --git a/scripts/kconfirm/kconfirm-linux/Cargo.toml b/scripts/kconfirm/kconfirm-linux/Cargo.toml new file mode 100644 index 000000000000..9516399e1dae --- /dev/null +++ b/scripts/kconfirm/kconfirm-linux/Cargo.toml @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0 +[package] +name = "kconfirm-linux" +version = "0.10.0" +edition = "2024" +rust-version.workspace = true + +[dependencies] +kconfirm-lib = { path = "../kconfirm-lib" } +nom-kconfig = { workspace = true } diff --git a/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs b/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs new file mode 100644 index 000000000000..227faa17b962 --- /dev/null +++ b/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0-only +use std::env; +use std::ffi::CStr; +use std::ffi::CString; +use std::os::raw::c_char; +use std::os::raw::c_int; +use std::ptr; + +pub const REQUIRED_ARGUMENT: c_int = 1; + +#[repr(C)] +pub struct option { + pub name: *const c_char, + pub has_arg: c_int, + pub flag: *mut c_int, + pub val: c_int, +} + +#[link(name = "c")] +unsafe extern "C" { + fn getopt_long( + argc: c_int, + argv: *mut *mut c_char, + optstring: *const c_char, + longopts: *const option, + longindex: *mut c_int, + ) -> c_int; + + static mut optarg: *mut c_char; + static mut optind: c_int; +} + +pub struct Getopt { + _cstrings: Vec<CString>, + argv: Vec<*mut c_char>, + argc: c_int, +} + +impl Getopt { + pub fn new() -> Self { + let raw_args: Vec<String> = env::args().collect(); + + let cstrings: Vec<CString> = raw_args + .iter() + .map(|s| CString::new(s.as_str()).unwrap()) + .collect(); + + let mut argv: Vec<*mut c_char> = + cstrings.iter().map(|s| s.as_ptr() as *mut c_char).collect(); + + argv.push(ptr::null_mut()); + + let argc = (argv.len() - 1) as c_int; + + Self { + _cstrings: cstrings, + argv, + argc, + } + } + + pub fn reset(&mut self) { + unsafe { + optind = 1; + } + } + + pub fn next( + &mut self, + optstring: &CStr, + longopts: &[option], + ) -> Option<Result<(char, Option<String>), String>> { + unsafe { + let c = getopt_long( + self.argc, + self.argv.as_mut_ptr(), + optstring.as_ptr(), + longopts.as_ptr(), + ptr::null_mut(), + ); + + if c == -1 { + return None; + } + + if c == '?' as c_int { + return Some(Err("invalid argument".into())); + } + + let arg = if optarg.is_null() { + None + } else { + Some(CStr::from_ptr(optarg).to_string_lossy().into_owned()) + }; + + Some(Ok((c as u8 as char, arg))) + } + } +} diff --git a/scripts/kconfirm/kconfirm-linux/src/lib.rs b/scripts/kconfirm/kconfirm-linux/src/lib.rs new file mode 100644 index 000000000000..f52399d2c9e5 --- /dev/null +++ b/scripts/kconfirm/kconfirm-linux/src/lib.rs @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: GPL-2.0-only +use nom_kconfig::KconfigFile; +use std::io; +use std::path::PathBuf; + +pub const ALL_ARCHITECTURES: [&str; 21] = [ + "arm", + "arm64", + "x86", + "riscv", + "mips", + "xtensa", + "sparc", + "alpha", + "arc", + "csky", + "hexagon", + "loongarch", + "m68k", + "microblaze", + "nios2", + "openrisc", + "parisc", + "powerpc", + "s390", + "sh", + "um", +]; + +// each architecture has its own directory, and config option. +// most are the same, but powerpc / ppc and um / uml are not. +// this maps the directory to the config option +pub fn arch_dir_to_config(arch_dir: &str) -> String { + match arch_dir { + "powerpc" => String::from("PPC"), + "um" => String::from("UML"), + _ => String::from(arch_dir).to_uppercase(), + } +} + +pub struct LinuxKconfig { + pub arch_config_option: String, + pub kconfig_file: KconfigFile, + pub file_contents: String, +} + +// collects the root kconfig file, and all of the arch-specific kconfig files +pub fn collect_kconfig_root_files( + archs: Vec<String>, + linux_source: PathBuf, +) -> io::Result<Vec<LinuxKconfig>> { + let mut all_root_kconfig_files = Vec::new(); + + // add the root kconfig file + let root_kconfig_path = PathBuf::from("Kconfig"); // doesn't include the arch: arch/x86/Kconfig + let root_kconfig_file = KconfigFile::new(linux_source.clone(), root_kconfig_path.clone()); + + for arch_dir in archs { + let mut cur_root_kconfig_file = root_kconfig_file.clone(); + + if arch_dir == "um" { + // this is only used by the 'um' architecture to include arch/x86/um/Kconfig + cur_root_kconfig_file.add_local_var("HEADER_ARCH", "x86"); + } + + cur_root_kconfig_file.add_local_var("SRCARCH", &arch_dir); + + let linux_kconfig = LinuxKconfig { + arch_config_option: arch_dir_to_config(&arch_dir), + file_contents: root_kconfig_file.read_to_string()?, + kconfig_file: cur_root_kconfig_file, + }; + + all_root_kconfig_files.push(linux_kconfig); + } + + Ok(all_root_kconfig_files) +} diff --git a/scripts/kconfirm/kconfirm-linux/src/main.rs b/scripts/kconfirm/kconfirm-linux/src/main.rs new file mode 100644 index 000000000000..03554a94f57c --- /dev/null +++ b/scripts/kconfirm/kconfirm-linux/src/main.rs @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: GPL-2.0-only +use crate::getopt_ffi::Getopt; +use crate::getopt_ffi::REQUIRED_ARGUMENT; +use crate::getopt_ffi::option; +use kconfirm_lib::AnalysisArgs; +use kconfirm_lib::Check; +use kconfirm_lib::check_kconfig; +use kconfirm_lib::output::print_findings; +use kconfirm_linux::ALL_ARCHITECTURES; +use kconfirm_linux::collect_kconfig_root_files; +use nom_kconfig::KconfigInput; +use std::collections::HashSet; +use std::io; +use std::path::PathBuf; +use std::ptr; +use std::str::FromStr; +mod getopt_ffi; + +fn split_csv_arg(dst: &mut Vec<String>, value: &str) { + dst.extend( + value + .split(',') + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()), + ); +} + +#[derive(Debug)] +pub struct Args { + pub linux_path: PathBuf, + pub enable_arch: Vec<String>, + pub disable_arch: Vec<String>, + pub enable_check: Vec<String>, + pub disable_check: Vec<String>, +} + +pub fn parse_args() -> Result<Args, String> { + let mut linux_path: Option<PathBuf> = None; + let mut enable_arch = Vec::new(); + let mut disable_arch = Vec::new(); + let mut enable_check = Vec::new(); + let mut disable_check = Vec::new(); + + let long_options = [ + option { + name: c"linux-path".as_ptr(), + has_arg: REQUIRED_ARGUMENT, + flag: ptr::null_mut(), + val: 'l' as _, + }, + option { + name: c"enable-arch".as_ptr(), + has_arg: REQUIRED_ARGUMENT, + flag: ptr::null_mut(), + val: 'a' as _, + }, + option { + name: c"disable-arch".as_ptr(), + has_arg: REQUIRED_ARGUMENT, + flag: ptr::null_mut(), + val: 'x' as _, + }, + option { + name: c"enable-check".as_ptr(), + has_arg: REQUIRED_ARGUMENT, + flag: ptr::null_mut(), + val: 'e' as _, + }, + option { + name: c"disable-check".as_ptr(), + has_arg: REQUIRED_ARGUMENT, + flag: ptr::null_mut(), + val: 'd' as _, + }, + option { + name: ptr::null(), + has_arg: 0, + flag: ptr::null_mut(), + val: 0, + }, + ]; + + let mut getopt = Getopt::new(); + + getopt.reset(); + + while let Some(result) = getopt.next(c"l:a:x:e:d:", &long_options) { + let (opt, arg) = result?; + + match opt { + 'l' => { + linux_path = Some(PathBuf::from(arg.unwrap())); + } + + 'a' => { + split_csv_arg(&mut enable_arch, &arg.unwrap()); + } + + 'x' => { + split_csv_arg(&mut disable_arch, &arg.unwrap()); + } + + 'e' => { + split_csv_arg(&mut enable_check, &arg.unwrap()); + } + + 'd' => { + split_csv_arg(&mut disable_check, &arg.unwrap()); + } + + _ => {} + } + } + + let linux_path = linux_path.ok_or("--linux-path is required")?; + + if enable_arch.is_empty() { + return Err("--enable-arch is required".into()); + } + + Ok(Args { + linux_path, + enable_arch, + disable_arch, + enable_check, + disable_check, + }) +} + +fn main() -> io::Result<()> { + let cli_args = parse_args().unwrap_or_else(|e| { + eprintln!("error: {e}"); + std::process::exit(1); + }); + let mut enabled_checks: HashSet<Check> = [ + Check::DuplicateDependency, + Check::DuplicateRange, + Check::DeadRange, + Check::DuplicateSelect, + Check::DeadDefault, + Check::ConstantCondition, + Check::DuplicateDefault, + Check::DuplicateImply, + Check::ReverseRange, + ] + .into_iter() + .collect(); // apply --enable-check + for name in &cli_args.enable_check { + if let Ok(c) = Check::from_str(name) { + enabled_checks.insert(c); + } else { + eprintln!("Error: check {} does not exist", name); + std::process::exit(1); + } + } // apply --disable-check + for name in &cli_args.disable_check { + if let Ok(c) = Check::from_str(name) { + enabled_checks.remove(&c); + } else { + eprintln!("Error: check {} does not exist", name); + std::process::exit(1); + } + } + let analysis_args = AnalysisArgs { enabled_checks }; + let mut selected_arches: HashSet<String> = cli_args.enable_arch.iter().cloned().collect(); // apply --disable-arch + for arch in &cli_args.disable_arch { + selected_arches.remove(arch); + } + for desired_arch in &selected_arches { + if !ALL_ARCHITECTURES.contains(&desired_arch.as_str()) { + eprintln!("Error: unexpected architecture, please pass one of the following:"); + for available_arch in ALL_ARCHITECTURES { + eprint!("{} ", available_arch); + } + eprintln!(""); + std::process::exit(1); + } + } + let kconfig_files = + collect_kconfig_root_files(selected_arches.into_iter().collect(), cli_args.linux_path)?; + let kconfig_inputs = kconfig_files + .iter() + .map(|kconfig| { + let kconfig_input = + KconfigInput::new_extra(&kconfig.file_contents, kconfig.kconfig_file.clone()); + (kconfig.arch_config_option.clone(), kconfig_input) + }) + .collect(); + let findings = check_kconfig(analysis_args, kconfig_inputs); + print_findings(findings); + Ok(()) +} -- 2.53.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-16 21:53 ` [RFC PATCH v3 1/3] scripts: " Julian Braha @ 2026-05-17 6:10 ` Demi Marie Obenour 2026-05-17 9:58 ` Miguel Ojeda 2026-05-17 6:28 ` Miguel Ojeda 1 sibling, 1 reply; 20+ messages in thread From: Demi Marie Obenour @ 2026-05-17 6:10 UTC (permalink / raw) To: Julian Braha, nathan, nsc Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild [-- Attachment #1.1.1: Type: text/plain, Size: 91429 bytes --] On 5/16/26 17:53, Julian Braha wrote: > Add kconfirm into scripts/ > > kconfirm is a static analysis tool with various checks for Kconfig, and > intended to have zero false alarms by default. These default checks > currently include dead code, constant conditions, and invalid (reverse) > ranges. > > There are also optional checks for dead links in the help texts, and for > config options that select visible config options. > > Checks are performed on the same architecture as the kernel build, using > a single thread. More architectures can be enabled by passing > `--enable-arch`. Alarms are tagged using the architectures' config options, > like so: [X86] if specific to x86, or [X86, ARM] if the alarm appears for > both x86 and arm. > > Each alarm gets a single line (deduplicated across architectures) and is > formatted like this: > [<SEVERITY>] [<ARCH_1>, <ARCH_2>] config <OPTION_NAME>: <alarm message> > > The tool source contains two Rust packages: kconfirm-lib and > kconfirm-linux. > > kconfirm-lib is the underlying library that analyzes Kconfig code, and > formats alarms for usability. It analyzes the entire Linux Kconfig spec, > including all architectures. This package exposes the symbol table that it > constructs so that other tools can import this library, and make use of it > for their own Kconfig analyses. > > kconfirm-linux imports kconfirm-lib, and provides the CLI, which is > intended for either manual usage, or integration with the Linux build > system so that users can simply run `make kconfirm` from the root. > kconfirm-linux also handles some of the specificities of how Kconfig is > used in the Linux tree, in contrast to other open source software. E.g. > the way that each architecture has its own Kconfig and Kconfig.debug > files. > > The tool's dependencies need to be downloaded from crates.io by running > `cargo vendor` in scripts/kconfirm/ before building and running the tool. > > Signed-off-by: Julian Braha <julianbraha@gmail.com> > --- > Makefile | 15 +- > scripts/Makefile | 2 +- > scripts/kconfirm/.gitignore | 3 + > scripts/kconfirm/Cargo.lock | 60 ++ > scripts/kconfirm/Cargo.toml | 12 + > scripts/kconfirm/Makefile | 14 + > scripts/kconfirm/kconfirm-lib/Cargo.toml | 12 + > scripts/kconfirm/kconfirm-lib/src/analyze.rs | 643 ++++++++++++++++ > scripts/kconfirm/kconfirm-lib/src/checks.rs | 701 ++++++++++++++++++ > scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs | 182 +++++ > .../kconfirm/kconfirm-lib/src/dead_links.rs | 138 ++++ > scripts/kconfirm/kconfirm-lib/src/lib.rs | 62 ++ > scripts/kconfirm/kconfirm-lib/src/output.rs | 111 +++ > .../kconfirm/kconfirm-lib/src/symbol_table.rs | 223 ++++++ > scripts/kconfirm/kconfirm-linux/Cargo.toml | 10 + > .../kconfirm/kconfirm-linux/src/getopt_ffi.rs | 99 +++ > scripts/kconfirm/kconfirm-linux/src/lib.rs | 78 ++ > scripts/kconfirm/kconfirm-linux/src/main.rs | 192 +++++ > 18 files changed, 2552 insertions(+), 5 deletions(-) > create mode 100644 scripts/kconfirm/.gitignore > create mode 100644 scripts/kconfirm/Cargo.lock > create mode 100644 scripts/kconfirm/Cargo.toml > create mode 100644 scripts/kconfirm/Makefile > create mode 100644 scripts/kconfirm/kconfirm-lib/Cargo.toml > create mode 100644 scripts/kconfirm/kconfirm-lib/src/analyze.rs > create mode 100644 scripts/kconfirm/kconfirm-lib/src/checks.rs > create mode 100644 scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs > create mode 100644 scripts/kconfirm/kconfirm-lib/src/dead_links.rs > create mode 100644 scripts/kconfirm/kconfirm-lib/src/lib.rs > create mode 100644 scripts/kconfirm/kconfirm-lib/src/output.rs > create mode 100644 scripts/kconfirm/kconfirm-lib/src/symbol_table.rs > create mode 100644 scripts/kconfirm/kconfirm-linux/Cargo.toml > create mode 100644 scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs > create mode 100644 scripts/kconfirm/kconfirm-linux/src/lib.rs > create mode 100644 scripts/kconfirm/kconfirm-linux/src/main.rs > > diff --git a/Makefile b/Makefile > index b7b80e84e1eb..99aaed5bdbc5 100644 > --- a/Makefile > +++ b/Makefile > @@ -296,7 +296,7 @@ no-dot-config-targets := $(clean-targets) \ > $(version_h) headers headers_% archheaders archscripts \ > %asm-generic kernelversion %src-pkg dt_binding_check \ > outputmakefile rustavailable rustfmt rustfmtcheck \ > - run-command > + run-command kconfirm > no-sync-config-targets := $(no-dot-config-targets) %install modules_sign kernelrelease \ > image_name > single-targets := %.a %.i %.ko %.lds %.ll %.lst %.mod %.o %.rsi %.s %/ > @@ -536,6 +536,7 @@ OBJDUMP = $(CROSS_COMPILE)objdump > READELF = $(CROSS_COMPILE)readelf > STRIP = $(CROSS_COMPILE)strip > endif > +CARGO = cargo > RUSTC = rustc > RUSTDOC = rustdoc > RUSTFMT = rustfmt > @@ -633,7 +634,7 @@ export RUSTC_BOOTSTRAP := 1 > export CLIPPY_CONF_DIR := $(srctree) > > export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC HOSTPKG_CONFIG > -export RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK > +export CARGO RUSTC RUSTDOC RUSTFMT RUSTC_OR_CLIPPY_QUIET RUSTC_OR_CLIPPY BINDGEN LLVM_LINK > export HOSTRUSTC KBUILD_HOSTRUSTFLAGS > export CPP AR NM STRIP OBJCOPY OBJDUMP READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL > export PERL PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX > @@ -1705,7 +1706,7 @@ MRPROPER_FILES += include/config include/generated \ > vmlinux-gdb.py \ > rpmbuild \ > rust/libmacros.so rust/libmacros.dylib \ > - rust/libpin_init_internal.so rust/libpin_init_internal.dylib > + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \ > > # clean - Delete most, but leave enough to build external modules > # > @@ -2227,7 +2228,7 @@ endif > # Scripts to check various things for consistency > # --------------------------------------------------------------------------- > > -PHONY += includecheck versioncheck coccicheck > +PHONY += includecheck versioncheck coccicheck kconfirm > > includecheck: > find $(srctree)/* $(RCS_FIND_IGNORE) \ > @@ -2242,6 +2243,12 @@ versioncheck: > coccicheck: > $(Q)$(BASH) $(srctree)/scripts/$@ > > + > +kconfirm: > + $(Q)$(MAKE) -C $(srctree)/scripts/kconfirm srctree=$(abs_srctree) SRCARCH=$(SRCARCH) || \ > + (printf "\n kconfirm failed to compile and run. Have you set up its dependencies yet?\n See Documentation/dev-tools/kconfirm.rst\n\n" && false) > +clean-dirs += scripts/kconfirm > + > PHONY += checkstack kernelrelease kernelversion image_name > > # UML needs a little special treatment here. It wants to use the host > diff --git a/scripts/Makefile b/scripts/Makefile > index 3434a82a119f..460655bd2de1 100644 > --- a/scripts/Makefile > +++ b/scripts/Makefile > @@ -66,4 +66,4 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux > subdir-$(CONFIG_SECURITY_IPE) += ipe > > # Let clean descend into subdirs > -subdir- += basic dtc gdb kconfig mod > +subdir- += basic dtc gdb kconfig kconfirm mod > diff --git a/scripts/kconfirm/.gitignore b/scripts/kconfirm/.gitignore > new file mode 100644 > index 000000000000..f63ee0251591 > --- /dev/null > +++ b/scripts/kconfirm/.gitignore > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +/target > +/vendor > diff --git a/scripts/kconfirm/Cargo.lock b/scripts/kconfirm/Cargo.lock > new file mode 100644 > index 000000000000..d90bc7d2e2a3 > --- /dev/null > +++ b/scripts/kconfirm/Cargo.lock > @@ -0,0 +1,60 @@ > +# This file is automatically @generated by Cargo. > +# It is not intended for manual editing. > +version = 4 > + > +[[package]] > +name = "bytecount" > +version = "0.6.9" > +source = "registry+https://github.com/rust-lang/crates.io-index" > +checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e" > + > +[[package]] > +name = "kconfirm-lib" > +version = "0.10.0" > +dependencies = [ > + "nom-kconfig", > +] > + > +[[package]] > +name = "kconfirm-linux" > +version = "0.10.0" > +dependencies = [ > + "kconfirm-lib", > + "nom-kconfig", > +] > + > +[[package]] > +name = "memchr" > +version = "2.8.0" > +source = "registry+https://github.com/rust-lang/crates.io-index" > +checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" > + > +[[package]] > +name = "nom" > +version = "8.0.0" > +source = "registry+https://github.com/rust-lang/crates.io-index" > +checksum = "df9761775871bdef83bee530e60050f7e54b1105350d6884eb0fb4f46c2f9405" > +dependencies = [ > + "memchr", > +] > + > +[[package]] > +name = "nom-kconfig" > +version = "0.11.0" > +source = "registry+https://github.com/rust-lang/crates.io-index" > +checksum = "5a0220bb2c8e5ad29b06fe0f75a276affeb10c9504726bf46d81fef78d69b1e3" > +dependencies = [ > + "nom", > + "nom_locate", > +] > + > +[[package]] > +name = "nom_locate" > +version = "5.0.0" > +source = "registry+https://github.com/rust-lang/crates.io-index" > +checksum = "0b577e2d69827c4740cba2b52efaad1c4cc7c73042860b199710b3575c68438d" > +dependencies = [ > + "bytecount", > + "memchr", > + "nom", > +] > diff --git a/scripts/kconfirm/Cargo.toml b/scripts/kconfirm/Cargo.toml > new file mode 100644 > index 000000000000..5880b06c4116 > --- /dev/null > +++ b/scripts/kconfirm/Cargo.toml > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: GPL-2.0 > +[workspace] > +members = ["kconfirm-lib", "kconfirm-linux"] > +resolver = "3" > + > +[workspace.package] > +rust-version = "1.85.0" > + > +[workspace.dependencies] > +nom-kconfig = { version = "0.11", default-features = false, features = [ > + "display", > +] } > diff --git a/scripts/kconfirm/Makefile b/scripts/kconfirm/Makefile > new file mode 100644 > index 000000000000..6a0b7389103e > --- /dev/null > +++ b/scripts/kconfirm/Makefile > @@ -0,0 +1,14 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# kconfirm makefile > + > +TARGET := kconfirm > + > +# Extra arguments forwarded to kconfirm. > +# Example: make kconfirm KCONFIRM_ARGS="--enable-check dead_links" > +KCONFIRM_ARGS ?= > + > +$(TARGET): > + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS) > + > + > +clean-files += target vendor > diff --git a/scripts/kconfirm/kconfirm-lib/Cargo.toml b/scripts/kconfirm/kconfirm-lib/Cargo.toml > new file mode 100644 > index 000000000000..dd3d7cb1aa1d > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/Cargo.toml > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: GPL-2.0 > +[package] > +name = "kconfirm-lib" > +version = "0.10.0" > +edition = "2024" > +rust-version.workspace = true > + > +[dependencies] > +nom-kconfig = { workspace = true } > + > +[features] > +default = [] > diff --git a/scripts/kconfirm/kconfirm-lib/src/analyze.rs b/scripts/kconfirm/kconfirm-lib/src/analyze.rs > new file mode 100644 > index 000000000000..24798581dc3d > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/src/analyze.rs > @@ -0,0 +1,643 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use crate::AnalysisArgs; > +use crate::Check; > +use crate::SymbolTable; > +use crate::dead_links; > +use crate::dead_links::LinkStatus; > +use crate::dead_links::check_link; > +use crate::output::Finding; > +use crate::output::Severity; > +use crate::symbol_table::ChoiceData; > +use nom_kconfig::Attribute::*; > +use nom_kconfig::Entry; > +use nom_kconfig::attribute::DefaultAttribute; > +use nom_kconfig::attribute::Expression; > +use nom_kconfig::attribute::Imply; > +use nom_kconfig::attribute::Select; > +use nom_kconfig::attribute::r#type::Type; > +use nom_kconfig::entry::Choice; > +use nom_kconfig::entry::Config; > +use nom_kconfig::entry::If; > +use nom_kconfig::entry::Menu; > +use nom_kconfig::entry::Source; > +use std::collections::HashSet; > +use std::option::Option; > + > +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] > +enum FunctionalAttributes { > + // only tracking the attributes that affect the semantics, e.g. not help texts > + Dependencies, > + Selects, > + Implies, > + Ranges, > + Defaults, > +} > + > +struct AttributeGroupingChecker { > + current_group: Option<FunctionalAttributes>, > + finished_groups: HashSet<FunctionalAttributes>, > +} > + > +impl AttributeGroupingChecker { > + fn new() -> Self { > + Self { > + current_group: None, > + finished_groups: HashSet::new(), > + } > + } > + > + // doesn't modify `findings` if the style check is disabled > + fn check( > + &mut self, > + group: FunctionalAttributes, > + args: &AnalysisArgs, > + findings: &mut Vec<Finding>, > + symbol: &str, > + arch: &String, > + message: String, > + ) { > + if !args.is_enabled(Check::UngroupedAttribute) { > + return; > + } > + > + match self.current_group { > + // still contiguous > + Some(current) if current == group => {} > + > + // start of group > + None => { > + self.current_group = Some(group); > + } > + > + Some(current) => { > + // the previous group finished > + self.finished_groups.insert(current); > + > + // we've already finished this group, it's ungrouped > + if self.finished_groups.contains(&group) { > + findings.push(Finding { > + severity: Severity::Style, > + check: Check::UngroupedAttribute, > + symbol: Some(symbol.to_string()), > + message, > + arch: arch.to_owned(), > + }); > + } > + > + // switch to the new group > + self.current_group = Some(group); > + } > + } > + } > +} > + > +struct DeadLinkChecker { > + visited_links: HashSet<String>, > +} > + > +impl DeadLinkChecker { > + fn new() -> Self { > + Self { > + visited_links: HashSet::new(), > + } > + } > + > + fn check_text( > + &mut self, > + text: &str, > + args: &AnalysisArgs, > + findings: &mut Vec<Finding>, > + symbol: Option<&str>, > + arch: &String, > + context: &str, > + ) { > + if !args.is_enabled(Check::DeadLink) { > + return; > + } > + > + let links = dead_links::find_links(text); > + > + if links.is_empty() { > + return; > + } > + > + for link in links { > + // avoid rechecking identical links > + if !self.visited_links.insert(link.clone()) { > + continue; > + } > + > + let status = check_link(&link); > + if status != LinkStatus::Ok && status != LinkStatus::ProbablyBlocked { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadLink, > + symbol: symbol.map(|s| s.to_string()), > + message: format!( > + "{} contains link {} with status {:?}", > + context, link, status > + ), > + arch: arch.to_owned(), > + }); > + } > + } > + } > +} > + > +#[derive(Clone)] > +pub struct Context { > + pub arch: String, > + pub definition_condition: Vec<Expression>, > + pub visibility: Vec<Option<Expression>>, > + pub dependencies: Vec<Expression>, > + pub in_choice: bool, > +} > + > +impl Context { > + fn with_arch(arch: String) -> Context { > + Context { > + arch, > + definition_condition: vec![], > + visibility: vec![], > + dependencies: vec![], > + in_choice: false, > + } > + } > + > + fn child(&self) -> Self { > + self.clone() > + } > + > + fn with_dep(mut self, dep: Expression) -> Self { > + self.dependencies.push(dep); > + self > + } > + > + fn with_visibility(mut self, cond: Option<Expression>) -> Self { > + self.visibility.push(cond); > + self > + } > + > + fn with_definition(mut self, cond: Expression) -> Self { > + self.definition_condition.push(cond); > + self > + } > + > + fn in_choice(mut self) -> Self { > + self.in_choice = true; > + self > + } > +} > + > +fn recurse_entries( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + entries: Vec<Entry>, > + ctx: Context, > + findings: &mut Vec<Finding>, > +) { > + for entry in entries { > + process_entry(args, symtab, entry, ctx.clone(), findings); > + } > +} > + > +pub fn analyze( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + arch: String, > + entries: Vec<Entry>, > +) -> Vec<Finding> { > + let mut findings = Vec::new(); > + > + let ctx = Context::with_arch(arch); > + > + recurse_entries(args, symtab, entries, ctx, &mut findings); > + > + findings > +} > + > +fn handle_config( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + entry: Config, > + ctx: &Context, > + findings: &mut Vec<Finding>, > +) { > + let config_symbol = entry.symbol; > + > + let mut child_ctx = ctx.child(); > + > + let mut config_type = None; > + let mut kconfig_dependencies = Vec::new(); > + let mut kconfig_selects: Vec<Select> = Vec::new(); > + let mut kconfig_implies: Vec<Imply> = Vec::new(); > + let mut kconfig_ranges = Vec::new(); > + let mut kconfig_defaults = Vec::new(); > + let mut found_prompt = false; > + > + /* > + * style check: ungrouped attributes > + * - need to check that dependencies, selects, ranges, and defaults are each kept together. > + */ > + let mut attribute_grouping_checker = AttributeGroupingChecker::new(); > + let mut dead_link_checker = DeadLinkChecker::new(); > + for attribute in entry.attributes { > + match attribute { > + Type(kconfig_type) => match kconfig_type.r#type.clone() { > + // hybrid type definition and default > + Type::DefBool(db) => { > + let default_attribute: DefaultAttribute = DefaultAttribute { > + expression: db.clone(), > + r#if: kconfig_type.clone().r#if, > + }; > + > + kconfig_defaults.push(default_attribute); > + config_type = Some(kconfig_type); > + > + // NOTE: as a style, we prefer to keep the hybrid default-typedef with the standalone defaults > + attribute_grouping_checker.check( > + FunctionalAttributes::Defaults, > + args, > + findings, > + &config_symbol, > + &ctx.arch, > + format!("ungrouped default {}", db), > + ); > + } > + Type::Bool(unconditional_prompt) => { > + if unconditional_prompt.is_some() { > + found_prompt = true; > + } > + config_type = Some(kconfig_type); > + } > + > + // hybrid type definition and default > + Type::DefTristate(dt) => { > + // NOTE: as a style, we prefer to keep the hybrid default-typedef with the standalone defaults > + attribute_grouping_checker.check( > + FunctionalAttributes::Defaults, > + args, > + findings, > + &config_symbol, > + &ctx.arch, > + format!("ungrouped default {}", &dt), > + ); > + > + let default_attribute: DefaultAttribute = DefaultAttribute { > + expression: dt, > + r#if: kconfig_type.clone().r#if, > + }; > + > + kconfig_defaults.push(default_attribute); > + config_type = Some(kconfig_type); > + } > + Type::Tristate(unconditional_prompt) => { > + if unconditional_prompt.is_some() { > + found_prompt = true; > + } > + > + config_type = Some(kconfig_type.clone()) > + } > + Type::Hex(unconditional_prompt) => { > + if unconditional_prompt.is_some() { > + found_prompt = true; > + } > + > + config_type = Some(kconfig_type); > + } > + Type::Int(unconditional_prompt) => { > + if unconditional_prompt.is_some() { > + found_prompt = true; > + } > + config_type = Some(kconfig_type); > + } > + Type::String(unconditional_prompt) => { > + if unconditional_prompt.is_some() { > + found_prompt = true; > + } > + config_type = Some(kconfig_type); > + } > + }, > + Default(default) => { > + attribute_grouping_checker.check( > + FunctionalAttributes::Defaults, > + args, > + findings, > + &config_symbol, > + &ctx.arch, > + format!("ungrouped default {}", &default), > + ); > + > + kconfig_defaults.push(default); > + } > + > + DependsOn(depends_on) => { > + attribute_grouping_checker.check( > + FunctionalAttributes::Dependencies, > + args, > + findings, > + &config_symbol, > + &ctx.arch, > + format!("ungrouped dependency {}", &depends_on), > + ); > + > + kconfig_dependencies.push(depends_on); > + } > + Select(select) => { > + attribute_grouping_checker.check( > + FunctionalAttributes::Selects, > + args, > + findings, > + &config_symbol, > + &ctx.arch, > + format!("ungrouped select {}", &select), > + ); > + > + kconfig_selects.push(select); > + } > + Imply(imply) => { > + attribute_grouping_checker.check( > + FunctionalAttributes::Implies, > + args, > + findings, > + &config_symbol, > + &ctx.arch, > + format!("ungrouped imply {}", imply), > + ); > + > + kconfig_implies.push(imply); > + > + // TODO: may be relevant for nonvisible config options when building an SMT model... > + } > + // NOTE: range bounds are inclusive > + Range(r) => { > + attribute_grouping_checker.check( > + FunctionalAttributes::Ranges, > + args, > + findings, > + &config_symbol, > + &ctx.arch, > + format!("ungrouped range {}", r), > + ); > + > + kconfig_ranges.push(r); > + } > + Help(h) => { > + // doing nothing for menu help right now > + > + dead_link_checker.check_text( > + &h, > + args, > + findings, > + Some(&config_symbol), > + &ctx.arch, > + "help text", > + ); > + } > + > + Modules => { > + // the modules attribute designates this config option as the one that determines if the `m` state is available for tristates options. > + > + // just making a special note of this in the symtab for now... > + symtab.modules_option = Some(config_symbol.clone()); > + } > + > + // the prompt's option `if` determines "visibility" > + Prompt(prompt) => { > + // TODO: once we have SMT solving, we can also check if the prompt condition is always true or never true (and therefore, effectively unconditional) > + > + found_prompt = true; > + if let Some(c) = prompt.r#if { > + child_ctx = child_ctx.with_visibility(Some(c)); > + } > + } > + Transitional => { > + // doing nothing for transitional right now > + } > + Optional | Visible(_) | Requires(_) | Option(_) => { > + eprintln!("Error: unexpected attribute encountered: {:?}", attribute); > + > + if cfg!(debug_assertions) { > + panic!(); > + } > + } > + } > + } > + > + if !found_prompt { > + child_ctx = child_ctx.with_visibility(None); > + } > + > + // there can be multiple entries that get merged. so we need to do the same for our symtab. > + let kconfig_type = config_type.clone().map(|c| c.r#type); > + > + // at the time of writing this, linux's kconfig only uses Bool inside Choice. > + // however, the kconfig documentation doesn't specify whether or not this is guaranteed to be the case. > + // we add this check to ensure that we don't cause undefined behavior in future linux versions if something changes... > + if child_ctx.in_choice { > + if let Some(kt) = &kconfig_type { > + match kt { > + Type::Bool(_) | Type::DefBool(_) => { > + // expected in a choice... > + } > + > + _ => { > + // TODO: old versions of linux (like 5.4.4) have tristates in the choice > + // - u-boot also currently has hex options in the choice! > + eprintln!( > + "Error: found something unexpected in a choice-statement: {:?}", > + kt > + ); > + } > + } > + } > + } > + > + // at the end, add the file's cur_dependencies to this var's invididual dependencies. > + kconfig_dependencies.extend(child_ctx.dependencies.clone()); > + symtab.merge_insert_new_solved( > + config_symbol.clone(), > + kconfig_type, > + kconfig_dependencies, > + //z3_dependency, > + kconfig_ranges, > + kconfig_defaults, > + child_ctx.visibility.clone(), > + child_ctx.arch.clone(), > + child_ctx.definition_condition.clone(), > + None, > + kconfig_selects > + .clone() > + .into_iter() > + .map(|sel| (sel.symbol, sel.r#if)) > + .collect(), > + kconfig_implies > + .into_iter() > + .map(|imply| (imply.symbol.to_string(), imply.r#if)) > + .collect(), > + ); > + // TODO: file a github issue, imply can never imply a constant (this is technically parsing incorrectly) > + > + // TODO: when SMT solving, we may need to keep track of the implies the same way we keep track of selects, > + // in cases when the implied config option is non-visible > + > + // need to add the select condition to the definedness condition if it exists > + for select in kconfig_selects { > + match select.r#if { > + None => symtab.merge_insert_new_solved( > + select.symbol, > + None, > + Vec::new(), > + Vec::new(), > + Vec::new(), > + Vec::new(), > + child_ctx.arch.clone(), > + child_ctx.definition_condition.clone(), > + Some((config_symbol.clone(), None)), > + Vec::new(), > + Vec::new(), > + ), > + Some(select_condition) => { > + symtab.merge_insert_new_solved( > + select.symbol, > + None, > + Vec::new(), > + Vec::new(), > + Vec::new(), > + Vec::new(), > + child_ctx.arch.clone(), > + child_ctx.definition_condition.clone(), > + Some((config_symbol.clone(), Some(select_condition))), > + Vec::new(), > + Vec::new(), > + ); > + } > + } > + } > +} > + > +fn handle_menu( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + entry: Menu, > + ctx: &Context, > + findings: &mut Vec<Finding>, > +) { > + // menus can set the visibility of their menu items > + > + let mut child_ctx = ctx.child(); > + > + for dep in entry.depends_on { > + child_ctx = child_ctx.with_dep(dep.clone()); > + child_ctx = child_ctx.with_visibility(Some(dep)); // not a typo, the config options inside of a menu are only visible if the menu's dependencies are satisfied > + } > + > + let nested_entries = entry.entries; > + > + recurse_entries(args, symtab, nested_entries, child_ctx.clone(), findings); > +} > + > +fn handle_choice( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + entry: Choice, > + ctx: &Context, > + findings: &mut Vec<Finding>, > +) { > + let mut child_ctx = ctx.child(); > + child_ctx = child_ctx.in_choice(); > + > + // we are going to add the dependencies of the choice to the dependencies of the entries. > + // we start with the dependencies inherited from the file > + let mut choice_visibility_condition = None; > + let mut defaults = Vec::new(); > + for attribute in entry.options { > + match attribute { > + DependsOn(depends_on) => { > + child_ctx = child_ctx.with_dep(depends_on); > + } > + > + Default(default) => { > + defaults.push(default); > + } > + > + // the prompt's `if` determines visibility > + Prompt(prompt) => { > + choice_visibility_condition = prompt.r#if; > + if let Some(i) = choice_visibility_condition.clone() { > + child_ctx = child_ctx.with_visibility(Some(i)); > + } > + } > + _ => { > + // skip > + } > + } > + } > + > + // all of the variables in the choice menu > + //let mut contained_vars = Vec::with_capacity(c.entries.len()); > + let nested_entries = entry.entries; > + > + recurse_entries(args, symtab, nested_entries, child_ctx.clone(), findings); > + > + let choice_data = ChoiceData { > + //inner_vars: contained_vars, > + arch: child_ctx.arch.clone(), > + visibility: choice_visibility_condition, > + dependencies: child_ctx.dependencies, > + defaults, > + }; > + symtab.choices.push(choice_data); > +} > + > +fn handle_if( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + entry: If, > + ctx: &Context, > + findings: &mut Vec<Finding>, > +) { > + let mut child_ctx = ctx.child(); > + child_ctx = child_ctx.with_definition(entry.condition.clone()); > + child_ctx = child_ctx.with_dep(entry.condition); > + let nested_entries = entry.entries; > + > + recurse_entries(args, symtab, nested_entries, child_ctx, findings); > +} > + > +fn handle_source( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + entry: Source, > + ctx: &Context, > + findings: &mut Vec<Finding>, > +) { > + let sourced_kconfig = entry.kconfigs; > + > + for sourced_kconfig in sourced_kconfig { > + recurse_entries(args, symtab, sourced_kconfig.entries, ctx.clone(), findings); > + } > +} > + > +pub fn process_entry( > + args: &AnalysisArgs, > + symtab: &mut SymbolTable, > + entry: Entry, > + ctx: Context, > + findings: &mut Vec<Finding>, > +) { > + // NOTE: in general, each handler should update the context as it encounters that construct. > + // e.g. Context.in_choice() should be called at the start of handle_choice(), not right before call to process_entry() when a choice is found and process_entry is called > + match entry { > + Entry::Config(c) | Entry::MenuConfig(c) => { > + handle_config(args, symtab, c, &ctx, findings); > + } > + Entry::Menu(m) => handle_menu(args, symtab, m, &ctx, findings), > + Entry::Choice(c) => handle_choice(args, symtab, c, &ctx, findings), > + Entry::If(i) => handle_if(args, symtab, i, &ctx, findings), > + Entry::Source(s) => handle_source(args, symtab, s, &ctx, findings), > + Entry::Comment(_) => {} > + Entry::MainMenu(_) => {} > + _ => {} > + } > +} > diff --git a/scripts/kconfirm/kconfirm-lib/src/checks.rs b/scripts/kconfirm/kconfirm-lib/src/checks.rs > new file mode 100644 > index 000000000000..2ad67f4390ea > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/src/checks.rs > @@ -0,0 +1,701 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use crate::output::Finding; > +use crate::output::Severity; > +use crate::symbol_table::AttributeDef; > +use crate::symbol_table::TypeInfo; > +use nom_kconfig::attribute::Expression; > +use nom_kconfig::attribute::range::RangeBound; > +use std::collections::HashSet; > +use std::num::ParseIntError; > +use std::str::FromStr; > + > +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] > +pub enum Check { > + FailedParse, > + UngroupedAttribute, // check for duplicate default values, and ungrouped attributes > + DeadLink, // check for dead links in the help texts > + SelectVisible, > + // need SMT solving before we can detect select-undefineds > + //SelectUndefined, > + DuplicateDependency, > + DuplicateRange, > + DeadRange, > + DuplicateSelect, > + DeadSelect, > + DeadDefault, > + ConstantCondition, > + DuplicateDefault, > + DuplicateDefaultValue, > + DuplicateImply, > + DeadImply, > + ReverseRange, > +} > + > +impl Check { > + pub fn as_str(self) -> &'static str { > + match self { > + Check::FailedParse => "failed_parse", > + Check::UngroupedAttribute => "ungrouped_attribute", > + Check::DeadLink => "dead_link", > + Check::SelectVisible => "select_visible", > + Check::DuplicateDependency => "duplicate_dependency", > + Check::DuplicateRange => "duplicate_range", > + Check::DeadRange => "dead_range", > + Check::DuplicateSelect => "duplicate_select", > + Check::DeadSelect => "dead_select", > + Check::DeadDefault => "dead_default", > + Check::ConstantCondition => "constant_condition", > + Check::DuplicateDefault => "duplicate_default", > + Check::DuplicateDefaultValue => "duplicate_default_value", > + Check::DuplicateImply => "duplicate_imply", > + Check::DeadImply => "dead_imply", > + Check::ReverseRange => "reverse_range", > + } > + } > +} > + > +#[derive(Debug)] > +pub struct ParseCheckError { > + pub input: String, > +} > + > +impl std::fmt::Display for ParseCheckError { > + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { > + write!(f, "unknown check '{}'", self.input) > + } > +} > + > +impl std::error::Error for ParseCheckError {} > + > +impl FromStr for Check { > + type Err = ParseCheckError; > + > + fn from_str(name: &str) -> Result<Self, Self::Err> { > + match name { > + "failed_parse" => Ok(Check::FailedParse), > + "ungrouped_attribute" => Ok(Check::UngroupedAttribute), > + "dead_link" => Ok(Check::DeadLink), > + "select_visible" => Ok(Check::SelectVisible), > + "duplicate_dependency" => Ok(Check::DuplicateDependency), > + "duplicate_range" => Ok(Check::DuplicateRange), > + "dead_range" => Ok(Check::DeadRange), > + "duplicate_select" => Ok(Check::DuplicateSelect), > + "dead_select" => Ok(Check::DeadSelect), > + "dead_default" => Ok(Check::DeadDefault), > + "constant_condition" => Ok(Check::ConstantCondition), > + "duplicate_default" => Ok(Check::DuplicateDefault), > + "duplicate_default_value" => Ok(Check::DuplicateDefaultValue), > + "duplicate_imply" => Ok(Check::DuplicateImply), > + "dead_imply" => Ok(Check::DeadImply), > + "reverse_range" => Ok(Check::ReverseRange), > + _ => Err(ParseCheckError { > + input: name.to_string(), > + }), > + } > + } > +} > + > +#[derive(Clone, Debug)] > +pub struct AnalysisArgs { > + // check for duplicate default values > + pub enabled_checks: HashSet<Check>, > +} > + > +impl AnalysisArgs { > + pub fn is_enabled(&self, check: Check) -> bool { > + self.enabled_checks.contains(&check) > + } > +} > + > +// returns an Error if a hex range bound cannot be parsed as an u64 > +pub fn check_reverse_ranges(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { > + let mut findings = Vec::new(); > + > + for range in &info.kconfig_ranges { > + // returns an Error if a hex range bound cannot be parsed as an u64 > + fn range_bound_to_int(range_bound: &RangeBound) -> Result<i128, ParseIntError> { > + match range_bound { > + RangeBound::Number(b) => { > + return Ok(b.to_owned() as i128); > + } > + RangeBound::Hex(b_str) => { > + let trimmed = b_str.trim_start_matches("0x").trim_start_matches("0X"); > + > + return i128::from_str_radix(trimmed, 16); > + } > + RangeBound::Variable(_) => { > + // for now, the caller is expected not to pass these cases. > + unreachable!("not handling variable ranges until SMT solving"); > + } > + RangeBound::Symbol(_) => { > + // TODO: need SMT solving for this case > + // for now, the caller is expected not to pass these cases. > + unreachable!("not handling CONFIG ranges until SMT solving"); > + } > + } > + } > + > + if matches!(range.lower_bound, RangeBound::Symbol(_)) > + || matches!(range.upper_bound, RangeBound::Symbol(_)) > + { > + // not handling these cases until SMT solving. > + // don't return though, because we stil want to check the other ranges. > + continue; > + } > + > + let maybe_lower_bound = range_bound_to_int(&range.lower_bound); > + let maybe_upper_bound = range_bound_to_int(&range.upper_bound); > + > + match (maybe_lower_bound, maybe_upper_bound) { > + (Ok(lower_bound), Ok(upper_bound)) => { > + if lower_bound > upper_bound { > + let message = format!( > + "reverse range {} for config option: {}, no value is valid", > + range.to_string(), > + var_symbol, > + ); > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::ReverseRange, > + symbol: Some(var_symbol.to_owned()), > + arch: arch.to_owned(), > + message, > + }); > + } > + } > + (Result::Err(_), _) | (_, Result::Err(_)) => { > + eprintln!( > + "Error: couldn't parse hex range bound as i128 for config option: {}", > + var_symbol > + ); > + // still want to check the other range bounds > + continue; > + } > + } > + } > + > + findings > +} > + > +pub fn check_constant_conditions( > + arch: &String, > + var_symbol: &str, > + info: &AttributeDef, > +) -> Vec<Finding> { > + let mut findings = Vec::new(); > + let default_conditions: Vec<&Expression> = info > + .kconfig_defaults > + .iter() > + .filter_map(|conditional_default| conditional_default.r#if.as_ref()) > + .collect(); > + > + check_conditions( > + arch, > + &mut findings, > + &var_symbol, > + &info.kconfig_dependencies, > + default_conditions, > + "default", > + ); > + > + let select_conditions: Vec<&Expression> = info > + .selects > + .iter() > + .filter_map(|conditional_select| conditional_select.1.as_ref()) > + .collect(); > + > + check_conditions( > + arch, > + &mut findings, > + var_symbol, > + &info.kconfig_dependencies, > + select_conditions, > + "select", > + ); > + > + let imply_conditions: Vec<&Expression> = info > + .implies > + .iter() > + .filter_map(|imp| imp.1.as_ref()) > + .collect(); > + > + check_conditions( > + arch, > + &mut findings, > + var_symbol, > + &info.kconfig_dependencies, > + imply_conditions, > + "imply", > + ); > + > + let range_conditions: Vec<&Expression> = info > + .kconfig_ranges > + .iter() > + .filter_map(|conditional_range| conditional_range.r#if.as_ref()) > + .collect(); > + > + check_conditions( > + arch, > + &mut findings, > + var_symbol, > + &info.kconfig_dependencies, > + range_conditions, > + "range", > + ); > + > + fn check_conditions( > + arch: &String, > + findings: &mut Vec<Finding>, > + symbol: &str, > + kconfig_dependencies: &[Expression], > + attribute_conditions: Vec<&Expression>, > + context: &str, > + ) { > + for attribute_condition in attribute_conditions.into_iter() { > + if kconfig_dependencies.contains(attribute_condition) { > + let message = format!( > + "constant {} condition 'if {}' for config option: {}, this condition is a dependency and will always be true", > + context, > + attribute_condition.to_string(), > + symbol, > + ); > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::ConstantCondition, > + symbol: Some(symbol.to_owned()), > + arch: arch.to_owned(), > + message, > + }); > + } > + } > + } > + findings > +} > + > +pub fn check_variable_info( > + args: &AnalysisArgs, > + var_symbol: &str, > + arch_specific: &String, > + info: &AttributeDef, > +) -> Vec<Finding> { > + let mut findings = Vec::new(); > + > + if args.is_enabled(Check::DuplicateDependency) { > + findings.extend(check_duplicate_dependencies( > + arch_specific, > + var_symbol, > + info, > + )); > + } > + > + if args.is_enabled(Check::DuplicateImply) { > + findings.extend(check_duplicate_implies(arch_specific, var_symbol, info)); > + } > + > + if args.is_enabled(Check::DuplicateRange) { > + findings.extend(check_duplicate_ranges(arch_specific, var_symbol, info)); > + } > + > + if args.is_enabled(Check::DuplicateSelect) { > + findings.extend(check_duplicate_selects(arch_specific, var_symbol, info)); > + } > + > + if args.is_enabled(Check::ConstantCondition) { > + findings.extend(check_constant_conditions(arch_specific, var_symbol, info)); > + } > + > + if args.is_enabled(Check::DeadDefault) > + || args.is_enabled(Check::DuplicateDefault) > + || args.is_enabled(Check::DuplicateDefaultValue) > + { > + findings.extend(check_defaults(arch_specific, var_symbol, info, args)); > + } > + > + if args.is_enabled(Check::ReverseRange) { > + findings.extend(check_reverse_ranges(arch_specific, var_symbol, info)); > + } > + > + findings > +} > + > +// TODO: also check if a config option in one arch unconditionally references a config option that only exists in another arch (need SMT for this first) > +pub fn check_select_visible(var_symbol: &str, info: &TypeInfo) -> Vec<Finding> { > + let mut findings = Vec::new(); > + > + // only interested in the options that are selected > + if info.selected_by.is_empty() { > + return Vec::new(); > + } > + > + for (selector, select_info) in &info.selected_by { > + for (arch, _cond) in select_info { > + // NOTE: we don't care if the select is conditional or unconditional, just the selectee's visibility > + > + // at this point, we know that `selector` unconditionally selects `var_symbol` > + // now, we need to check if `var_symbol` is unconditionally visible > + > + let message = format!( > + "selects the visible {}; consider using 'depends on' or 'imply' instead", > + var_symbol > + ); > + > + // match the architecture that the select happens under with the architecture of the unconditional visibility > + match info.attribute_defs.get(arch) { > + None => { > + // not selected in this architecture > + } > + Some(cur_arch_attribute_def) => { > + for (if_conditions, attributes) in cur_arch_attribute_def { > + if if_conditions.is_empty() && attributes.visibility.is_empty() { > + // empty visiblity means that it is unconditionally visible, within the current arch (assuming arch is not `None`) > + > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::SelectVisible, > + symbol: Some(selector.to_owned()), > + message: message.clone(), > + arch: arch.to_owned(), > + }); > + } > + } > + } > + } > + } > + } > + > + findings > +} > + > +fn is_duplicate<T: Eq + std::hash::Hash>(set: &mut HashSet<T>, key: T) -> bool { > + !set.insert(key) > +} > + > +fn check_duplicate_dependencies( > + arch_specific: &String, > + var_symbol: &str, > + info: &AttributeDef, > +) -> Vec<Finding> { > + let mut findings = Vec::new(); > + let mut seen = HashSet::new(); > + > + for dep in &info.kconfig_dependencies { > + if is_duplicate(&mut seen, dep.to_string()) { > + let message = format!("duplicate dependency on {}", dep.to_string()); > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DuplicateDependency, > + symbol: Some(var_symbol.to_owned()), > + message, > + arch: arch_specific.to_owned(), > + }); > + } > + } > + > + findings > +} > + > +fn check_duplicate_implies(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { > + let mut findings = Vec::new(); > + > + // symbols implied unconditionally > + let mut unconditional: HashSet<String> = HashSet::new(); > + > + // (symbol, condition) > + let mut conditional: HashSet<(String, String)> = HashSet::new(); > + > + for imp in &info.implies { > + let imply_var = imp.0.clone(); > + > + match &imp.1 { > + Some(cond) => { > + let cond_str = cond.to_string(); > + > + // duplicate conditional imply > + if !conditional.insert((imply_var.clone(), cond_str.clone())) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DuplicateImply, > + symbol: Some(var_symbol.to_owned()), > + message: format!( > + "duplicate imply of {:?} with condition {}", > + imp.0, cond_str > + ), > + arch: arch.to_owned(), > + }); > + } > + > + // conditional imply is dead if unconditional exists > + if unconditional.contains(&imply_var) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadImply, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead imply of {:?}", imp), > + arch: arch.to_owned(), > + }); > + } > + } > + > + None => { > + // duplicate unconditional imply > + if !unconditional.insert(imply_var.clone()) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DuplicateImply, > + symbol: Some(var_symbol.to_owned()), > + message: format!("duplicate imply of {:?}", imp), > + arch: arch.to_owned(), > + }); > + } > + > + // previous conditionals with same symbol are dead > + for (sym, _) in &conditional { > + if sym == &imply_var { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadImply, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead imply of {:?}", imp), > + arch: arch.to_owned(), > + }); > + } > + } > + } > + } > + } > + > + findings > +} > + > +fn check_duplicate_ranges(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { > + let mut findings = Vec::new(); > + > + // unconditional ranges by bounds > + let mut unconditional: HashSet<String> = HashSet::new(); > + > + // (bounds, condition) > + let mut conditional: HashSet<(String, String)> = HashSet::new(); > + > + for range in &info.kconfig_ranges { > + // uniquely identify the range bounds > + let range_key = format!("{} {}", range.lower_bound, range.upper_bound); > + > + match &range.r#if { > + Some(cond) => { > + let cond_str = cond.to_string(); > + > + // duplicate conditional range > + if !conditional.insert((range_key.clone(), cond_str.clone())) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DuplicateRange, > + symbol: Some(var_symbol.to_owned()), > + message: format!("duplicate range {:?} with condition {}", range, cond_str), > + arch: arch.to_owned(), > + }); > + } > + > + // conditional range is dead if unconditional exists > + if unconditional.contains(&range_key) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadRange, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead range of {:?}", range), > + arch: arch.to_owned(), > + }); > + } > + } > + > + None => { > + // duplicate unconditional range > + if !unconditional.insert(range_key.clone()) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadRange, > + symbol: Some(var_symbol.to_owned()), > + message: format!("duplicate range {:?}", range), > + arch: arch.to_owned(), > + }); > + } > + > + // previous conditionals with same bounds are dead > + for (bounds, _) in &conditional { > + if bounds == &range_key { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadRange, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead range of {:?}", range), > + arch: arch.to_owned(), > + }); > + } > + } > + } > + } > + } > + > + findings > +} > + > +fn check_duplicate_selects(arch: &String, var_symbol: &str, info: &AttributeDef) -> Vec<Finding> { > + let mut findings = Vec::new(); > + > + // symbols selected unconditionally > + let mut unconditional: HashSet<String> = HashSet::new(); > + > + // (symbol, condition) > + let mut conditional: HashSet<(String, String)> = HashSet::new(); > + > + for select in &info.selects { > + let select_var = select.0.clone(); > + > + match &select.1 { > + Some(cond) => { > + let cond_str = cond.to_string(); > + > + // duplicate conditional select > + if !conditional.insert((select_var.clone(), cond_str.clone())) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DuplicateSelect, > + symbol: Some(var_symbol.to_owned()), > + message: format!( > + "duplicate select of {:?} with condition {}", > + select.0, cond_str > + ), > + arch: arch.to_owned(), > + }); > + } > + > + // conditional is dead if unconditional exists > + if unconditional.contains(&select_var) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadSelect, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead select of {:?}", select.0), > + arch: arch.to_owned(), > + }); > + } > + } > + > + None => { > + // duplicate unconditional select > + if !unconditional.insert(select_var.clone()) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DuplicateSelect, > + symbol: Some(var_symbol.to_owned()), > + message: format!("duplicate select of {:?}", select.0), > + arch: arch.to_owned(), > + }); > + } > + > + // any previous conditional selects are now dead too > + for (sym, _) in &conditional { > + if sym == &select_var { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadSelect, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead select of {:?}", select.0), > + arch: arch.to_owned(), > + }); > + } > + } > + } > + } > + } > + > + findings > +} > + > +#[allow(clippy::collapsible_if)] > +fn check_defaults( > + arch: &String, > + var_symbol: &str, > + info: &AttributeDef, > + args: &AnalysisArgs, > +) -> Vec<Finding> { > + let mut findings = Vec::new(); > + let mut seen_conditions = HashSet::new(); > + let mut seen_values = HashSet::new(); > + let mut already_unconditional = false; > + > + for default in &info.kconfig_defaults { > + let val_str = default.expression.to_string(); > + > + let has_real_condition = match &default.r#if { > + Some(cond) => { > + let cond_str = cond.to_string(); > + !cond_str.is_empty() > + } > + None => false, > + }; > + > + let is_value_dup = if has_real_condition { > + is_duplicate(&mut seen_values, val_str.clone()) > + } else { > + false > + }; > + > + if already_unconditional && args.is_enabled(Check::DeadDefault) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadDefault, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead default of {}", val_str), > + arch: arch.to_owned(), > + }); > + } > + > + if args.is_enabled(Check::DuplicateDefaultValue) { > + if default.r#if.is_some() && is_value_dup { > + findings.push(Finding { > + severity: Severity::Style, > + check: Check::DuplicateDefaultValue, > + symbol: Some(var_symbol.to_owned()), > + message: format!( > + "duplicate default value of {}; consider combining the conditions with a logical-or: ||", > + val_str > + ), > + arch: arch.to_owned(), > + }); > + } > + } > + > + match &default.r#if { > + Some(cond) => { > + if is_duplicate(&mut seen_conditions, cond.to_string()) { > + if is_value_dup { > + if args.is_enabled(Check::DuplicateDefault) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DuplicateDefault, > + symbol: Some(var_symbol.to_owned()), > + message: format!("duplicate default condition of {:?}", cond), > + arch: arch.to_owned(), > + }); > + } > + } else { > + if args.is_enabled(Check::DeadDefault) { > + findings.push(Finding { > + severity: Severity::Warning, > + check: Check::DeadDefault, > + symbol: Some(var_symbol.to_owned()), > + message: format!("dead default of {}", val_str), > + arch: arch.to_owned(), > + }); > + } > + } > + } > + } > + None => { > + already_unconditional = true; > + } > + } > + } > + > + findings > +} > diff --git a/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs b/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs > new file mode 100644 > index 000000000000..d458010cc3f1 > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/src/curl_ffi.rs > @@ -0,0 +1,182 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use core::ffi::c_void; > +use std::ffi::CStr; > +use std::ffi::CString; > +use std::os::raw::c_char; > +use std::os::raw::c_int; > +use std::os::raw::c_long; > +use std::sync::OnceLock; > + > +static CURL_INIT: OnceLock<()> = OnceLock::new(); > + > +#[repr(C)] > +pub struct CURL { > + _private: [u8; 0], > +} > + > +type CURLcode = c_int; > +type CURLoption = u32; > +type CURLINFO = u32; > + > +const CURLE_OK: CURLcode = 0; > + > +const CURL_GLOBAL_DEFAULT: c_long = 3; > + > +const CURLOPT_URL: CURLoption = 10002; > +const CURLOPT_NOBODY: CURLoption = 44; > +const CURLOPT_TIMEOUT: CURLoption = 13; > +const CURLOPT_FOLLOWLOCATION: CURLoption = 52; > +const CURLOPT_USERAGENT: CURLoption = 10018; > +const CURLOPT_HEADERFUNCTION: CURLoption = 20079; > +const CURLOPT_HEADERDATA: CURLoption = 10029; > + > +const CURLINFO_RESPONSE_CODE: CURLINFO = 0x200002; > + > +#[link(name = "curl")] > +unsafe extern "C" {} > + > +unsafe extern "C" { > + fn curl_global_init(flags: c_long) -> CURLcode; > + > + fn curl_easy_init() -> *mut CURL; > + > + fn curl_easy_cleanup(handle: *mut CURL); > + > + fn curl_easy_perform(handle: *mut CURL) -> CURLcode; > + > + fn curl_easy_strerror(code: CURLcode) -> *const c_char; > + > + fn curl_easy_setopt(handle: *mut CURL, option: CURLoption, ...) -> CURLcode; > + > + fn curl_easy_getinfo(handle: *mut CURL, info: CURLINFO, ...) -> CURLcode; > +} > + > +fn init_curl() { > + CURL_INIT.get_or_init(|| unsafe { > + curl_global_init(CURL_GLOBAL_DEFAULT); > + }); > +} > + > +fn curl_error(code: CURLcode) -> String { > + unsafe { > + let ptr = curl_easy_strerror(code); > + > + if ptr.is_null() { > + return format!("curl error {}", code); > + } > + > + CStr::from_ptr(ptr).to_string_lossy().into_owned() > + } > +} > + > +struct HeaderCapture { > + location: Option<String>, > +} > + > +extern "C" fn header_callback( > + buffer: *mut c_char, > + size: usize, > + nitems: usize, > + userdata: *mut c_void, > +) -> usize { > + let total = size * nitems; > + > + unsafe { > + let bytes = std::slice::from_raw_parts(buffer as *const u8, total); > + > + if let Ok(header) = std::str::from_utf8(bytes) { > + let lower = header.to_ascii_lowercase(); > + > + if lower.starts_with("location:") { > + if let Some((_, value)) = header.split_once(':') { > + let capture = &mut *(userdata as *mut HeaderCapture); > + > + capture.location = Some(value.trim().to_string()); > + } > + } > + } > + } > + > + total > +} > + > +#[derive(Debug)] > +pub struct HttpResponse { > + pub response_code: u16, > + pub location: Option<String>, > +} > + > +pub fn head_request(url: &str) -> Result<HttpResponse, String> { > + init_curl(); > + > + unsafe { > + let curl = curl_easy_init(); > + > + if curl.is_null() { > + return Err("curl_easy_init failed".into()); > + } > + > + let url_c = match CString::new(url) { > + Ok(v) => v, > + Err(_) => { > + curl_easy_cleanup(curl); > + > + return Err("invalid URL".into()); > + } > + }; > + > + let ua_c = CString::new("link-checker/1.0").unwrap(); > + > + let mut headers = HeaderCapture { location: None }; > + > + macro_rules! setopt { > + ($opt:expr, $val:expr) => {{ > + let rc = curl_easy_setopt(curl, $opt, $val); > + > + if rc != CURLE_OK { > + curl_easy_cleanup(curl); > + > + return Err(curl_error(rc)); > + } > + }}; > + } > + > + setopt!(CURLOPT_URL, url_c.as_ptr()); > + setopt!(CURLOPT_NOBODY, 1 as c_long); > + setopt!(CURLOPT_TIMEOUT, 10 as c_long); > + setopt!(CURLOPT_FOLLOWLOCATION, 0 as c_long); > + setopt!(CURLOPT_USERAGENT, ua_c.as_ptr()); > + > + setopt!( > + CURLOPT_HEADERFUNCTION, > + header_callback as extern "C" fn(_, _, _, _) -> _ > + ); > + > + setopt!(CURLOPT_HEADERDATA, &mut headers as *mut _ as *mut c_void); > + > + let rc = curl_easy_perform(curl); > + > + if rc != CURLE_OK { > + curl_easy_cleanup(curl); > + > + return Err(curl_error(rc)); > + } > + > + let mut response_code: c_long = 0; > + > + let rc = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &mut response_code); > + > + if rc != CURLE_OK { > + curl_easy_cleanup(curl); > + > + return Err(curl_error(rc)); > + } > + > + curl_easy_cleanup(curl); > + > + Ok(HttpResponse { > + response_code: response_code as u16, > + location: headers.location, > + }) > + } > +} > diff --git a/scripts/kconfirm/kconfirm-lib/src/dead_links.rs b/scripts/kconfirm/kconfirm-lib/src/dead_links.rs > new file mode 100644 > index 000000000000..47bbd5c09114 > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/src/dead_links.rs > @@ -0,0 +1,138 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use crate::curl_ffi::head_request; > +use std::collections::HashSet; > + > +#[derive(PartialEq, Debug)] > +pub enum LinkStatus { > + Ok, > + ProbablyBlocked, > + Redirected(String), > + NotFound, > + ServerError, > + Unreachable(String), > + UnsupportedScheme(String), > +} > + > +pub fn check_link(url: &str) -> LinkStatus { > + if let Some(scheme) = url.split("://").next() { > + match scheme { > + "http" | "https" => return check_http(url), > + > + "git" | "ftp" => { > + return LinkStatus::UnsupportedScheme(scheme.into()); > + } > + > + _ => { > + return LinkStatus::UnsupportedScheme(scheme.into()); > + } > + } > + } > + > + LinkStatus::Unreachable("invalid URL".into()) > +} > + > +fn check_http(url: &str) -> LinkStatus { > + let response = match head_request(url) { > + Ok(r) => r, > + Err(e) => return LinkStatus::Unreachable(e), > + }; > + > + match response.response_code { > + 200..=299 => LinkStatus::Ok, > + > + 301 | 302 => LinkStatus::Redirected(response.location.unwrap_or_else(|| "unknown".into())), > + > + 403 | 429 => LinkStatus::ProbablyBlocked, > + > + 404 => LinkStatus::NotFound, > + > + 500..=599 => LinkStatus::ServerError, > + > + _ => LinkStatus::ProbablyBlocked, > + } > +} > + > +pub fn find_links(text: &str) -> Vec<String> { > + fn is_scheme_char(c: u8) -> bool { > + c.is_ascii_alphanumeric() || matches!(c, b'+' | b'-' | b'.') > + } > + > + fn is_url_terminator(c: u8) -> bool { > + c.is_ascii_whitespace() > + || matches!( > + c, > + b'"' | b'\'' | b'<' | b'>' | b'(' | b')' | b'[' | b']' | b'{' | b'}' > + ) > + } > + > + let bytes = text.as_bytes(); > + > + let mut links = Vec::new(); > + let mut seen = HashSet::new(); > + > + let mut i = 0; > + > + while i + 3 < bytes.len() { > + if bytes[i] == b':' && bytes[i + 1] == b'/' && bytes[i + 2] == b'/' { > + // walk backward to find scheme start > + let mut start = i; > + > + while start > 0 && is_scheme_char(bytes[start - 1]) { > + start -= 1; > + } > + > + // require non-empty scheme > + if start == i { > + i += 3; > + continue; > + } > + > + // first char must be alphabetic > + if !bytes[start].is_ascii_alphabetic() { > + i += 3; > + continue; > + } > + > + // walk forward to url end > + let mut end = i + 3; > + > + while end < bytes.len() && !is_url_terminator(bytes[end]) { > + end += 1; > + } > + > + let mut url = &text[start..end]; > + > + // trim trailing punctuation > + url = url.trim_end_matches(&['.', ',', ';', ':', '!', '?'][..]); > + > + // trim unmatched markdown > + while let Some(last) = url.chars().last() { > + let trim = match last { > + ')' => url.matches('(').count() < url.matches(')').count(), > + > + ']' => url.matches('[').count() < url.matches(']').count(), > + > + '}' => url.matches('{').count() < url.matches('}').count(), > + > + _ => false, > + }; > + > + if trim { > + url = &url[..url.len() - last.len_utf8()]; > + } else { > + break; > + } > + } > + > + if seen.insert(url) { > + links.push(url.to_string()); > + } > + > + i = end; > + } else { > + i += 1; > + } > + } > + > + links > +} > diff --git a/scripts/kconfirm/kconfirm-lib/src/lib.rs b/scripts/kconfirm/kconfirm-lib/src/lib.rs > new file mode 100644 > index 000000000000..6be0199f0785 > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/src/lib.rs > @@ -0,0 +1,62 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use analyze::analyze; > +pub use checks::AnalysisArgs; > +pub use checks::Check; > +pub use checks::check_select_visible; > +pub use checks::check_variable_info; > +use nom_kconfig::Entry; > +use nom_kconfig::KconfigInput; > +use nom_kconfig::parse_kconfig; > +use output::*; > +use symbol_table::*; > +mod analyze; > +mod checks; > +mod curl_ffi; > +mod dead_links; > +pub mod output; > +pub mod symbol_table; > + > +pub fn check_kconfig( > + args: AnalysisArgs, > + kconfig_files: Vec<(String, KconfigInput)>, > +) -> Vec<Finding> { > + let mut findings = Vec::new(); > + let mut symbol_table = SymbolTable::new(); > + > + for (arch_config_option, kconfig_file) in kconfig_files { > + match parse_kconfig(kconfig_file) { > + Ok(parsed) => { > + let entries: Vec<Entry> = parsed.1.entries; > + findings.extend(analyze( > + &args, > + &mut symbol_table, > + arch_config_option, > + entries, > + )); > + } > + Err(e) => { > + findings.push(Finding { > + severity: Severity::Fatal, > + check: Check::FailedParse, > + symbol: None, > + message: format!("Failed to parse kconfig, error is: {}", e), > + arch: arch_config_option, > + }); > + } > + } > + } > + > + for (var_symbol, type_info) in &symbol_table.raw { > + for (arch_specific, redefinitions) in &type_info.attribute_defs { > + for (_definition_condition, info) in redefinitions { > + findings.extend(check_variable_info(&args, var_symbol, arch_specific, info)); > + } > + } > + > + if args.is_enabled(Check::SelectVisible) { > + findings.extend(check_select_visible(var_symbol, type_info)); > + } > + } > + > + findings > +} > diff --git a/scripts/kconfirm/kconfirm-lib/src/output.rs b/scripts/kconfirm/kconfirm-lib/src/output.rs > new file mode 100644 > index 000000000000..e0d8bf8342d5 > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/src/output.rs > @@ -0,0 +1,111 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use crate::Check; > +use std::fmt; > + > +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] > +pub enum Severity { > + Fatal, > + Error, // will be used for known bugs, e.g. unmet dependencies > + Warning, > + Style, > +} > + > +#[derive(Debug)] > +pub struct Finding { > + pub severity: Severity, > + pub check: Check, > + pub symbol: Option<String>, > + pub message: String, > + pub arch: String, > +} > + > +impl Finding { > + fn fmt_with_arches(&self, f: &mut fmt::Formatter, arches: &[&str]) -> fmt::Result { > + let arch_part = if arches.is_empty() { > + String::new() > + } else { > + format!(" [{}]", arches.join(", ")) > + }; > + > + match &self.symbol { > + Some(s) => write!( > + f, > + "{} [{}]{} config {}: {}", > + self.severity, > + self.check.as_str(), > + arch_part, > + s, > + self.message > + ), > + None => write!( > + f, > + "{} [{}]{} {}", > + self.severity, > + self.check.as_str(), > + arch_part, > + self.message > + ), > + } > + } > +} > + > +impl fmt::Display for Finding { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + self.fmt_with_arches(f, &[]) > + } > +} > + > +pub fn print_findings(mut findings: Vec<Finding>) { > + findings.sort_by(|a, b| { > + ( > + &a.severity, > + a.check.as_str(), > + &a.symbol, > + &a.message, > + &a.arch, > + ) > + .cmp(&( > + &b.severity, > + b.check.as_str(), > + &b.symbol, > + &b.message, > + &b.arch, > + )) > + }); > + > + for group in findings.chunk_by(|a, b| { > + a.severity == b.severity > + && a.check.as_str() == b.check.as_str() > + && a.symbol == b.symbol > + && a.message == b.message > + }) { > + let head = &group[0]; > + > + let mut arches: Vec<&str> = Vec::new(); > + for f in group { > + if arches.last() != Some(&f.arch.as_str()) { > + arches.push(&f.arch); > + } > + } > + > + // Use a small wrapper so we can call our custom formatter via println! > + struct Wrap<'a>(&'a Finding, &'a [&'a str]); > + impl fmt::Display for Wrap<'_> { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + self.0.fmt_with_arches(f, self.1) > + } > + } > + println!("{}", Wrap(head, &arches)); > + } > +} > + > +impl fmt::Display for Severity { > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > + match self { > + Severity::Fatal => write!(f, "FATAL "), > + Severity::Error => write!(f, "ERROR "), > + Severity::Warning => write!(f, "WARNING"), > + Severity::Style => write!(f, "STYLE "), > + } > + } > +} > diff --git a/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs b/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs > new file mode 100644 > index 000000000000..48abb46c1945 > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-lib/src/symbol_table.rs > @@ -0,0 +1,223 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use nom_kconfig::attribute::DefaultAttribute; > +use nom_kconfig::attribute::Expression; > +use nom_kconfig::attribute::OrExpression; > +use nom_kconfig::attribute::Range; > +use nom_kconfig::attribute::r#type::Type; > +use std::collections::HashMap; > +use std::collections::hash_map; > + > +type KconfigSymbol = String; > +type Arch = String; > +type Cond = Option<Expression>; > + > +// NOTE: we cannot add these elements to the solver until we've processed all variables, > +// because we need to know all of the selectors. > +#[derive(Debug, Clone)] > +pub struct TypeInfo { > + pub kconfig_type: Option<Type>, // 'None' when we don't know the type (e.g. if it's a dangling reference) > + > + // maps the selector to an (ARCH, select_cond) > + // - if the ARCH is None, then it's not arch-specific > + // if the select_cond is None, then it's unconditional > + pub selected_by: HashMap<KconfigSymbol, Vec<(Arch, Cond)>>, // .0 only selects it when .1 is true. > + > + // there is one of these per entry (each entry expected to have a different definedness condition) > + // maps architecture option name (or none if not arch-specific) to: > + // [([condition], config definition)] > + // - NOTE: there can be multiple partial definitions under the same condition, or mutually-exclusive conditions, or a subset condition. > + pub attribute_defs: HashMap<Arch, Vec<(Vec<Expression>, AttributeDef)>>, // the innermost `Vec<Expression>` represents each nested condition that was reached (we will eventually need to AND them all) > +} > + > +// everything is a vector because we may encounter multiple over time, > +// so we won't know until the end what the condition is. > +#[derive(Debug, Clone)] > +pub struct AttributeDef { > + pub kconfig_dependencies: Vec<OrExpression>, > + pub kconfig_ranges: Vec<Range>, > + pub kconfig_defaults: Vec<DefaultAttribute>, > + pub visibility: Vec<Option<OrExpression>>, > + pub selects: Vec<(KconfigSymbol, Cond)>, > + pub implies: Vec<(KconfigSymbol, Cond)>, > +} > + > +impl TypeInfo { > + fn new_empty() -> Self { > + Self { > + kconfig_type: None, > + selected_by: HashMap::new(), > + attribute_defs: HashMap::new(), > + } > + } > + > + // TODO: we should consider having separate functions for: > + // 1. merge-inserting a redef of attributes (NOTE: the type definition is actually part of the redef, but we aren't handling type-redefinitions for now) > + // 2. selectors > + fn insert( > + &mut self, > + kconfig_type: Option<Type>, > + raw_constraints: Vec<OrExpression>, > + kconfig_ranges: Vec<Range>, > + kconfig_defaults: Vec<DefaultAttribute>, > + visibility: Vec<Option<OrExpression>>, > + arch: String, > + definition_condition: Vec<OrExpression>, > + selected_by: Option<(KconfigSymbol, Cond)>, > + selects: Vec<(KconfigSymbol, Cond)>, > + implies: Vec<(KconfigSymbol, Cond)>, > + ) { > + // type merge > + match (&self.kconfig_type, &kconfig_type) { > + (None, Some(_)) => self.kconfig_type = kconfig_type.clone(), > + (Some(_), Some(new)) if Some(new) != self.kconfig_type.as_ref() => { > + // TODO: not doing anything with redefined types yet. > + // later, we will want to consider e.g. bool/def_bool the same type (and possibly int/hex?) but not bool/tristate, so we need to build out typechecking. > + } > + _ => {} > + } > + > + // selected_by merge > + if let Some(sb) = selected_by { > + merge_selected_by(&mut self.selected_by, arch.clone(), sb); > + } > + > + // variable_info merge: > + // we only want to add an attribute redefinition if the things in the attribute def aren't empty > + // (the visibility is just additional info to capture) > + if (&kconfig_type).is_some() // we need to ensure that we have an empty definition here if the config option had a type definition > + || !raw_constraints.is_empty() > + || !kconfig_ranges.is_empty() > + || !kconfig_defaults.is_empty() > + || !selects.is_empty() > + || !implies.is_empty() > + { > + insert_variable_info( > + &mut self.attribute_defs, > + arch, > + definition_condition, > + AttributeDef { > + kconfig_dependencies: raw_constraints, > + kconfig_ranges, > + kconfig_defaults, > + visibility, > + selects, > + implies, > + }, > + ); > + } > + } > +} > + > +// the visibility and the dependencies will each need to be AND'd (separately) > +// the defaults should each be handled separately. > +pub struct ChoiceData { > + //pub inner_vars: Vec<String>, > + pub arch: Arch, > + pub visibility: Cond, > + pub dependencies: Vec<OrExpression>, // this is the menu's dependencies (and inherited dependencies from the file) > + pub defaults: Vec<DefaultAttribute>, // these are each of the conditional defaults for the choice > +} > + > +// NOTE: it might be better if TypeInfo is an enum with a single value, > +// e.g. Unsolved(kconfig_raw) and Solved(z3_ast) > +pub struct SymbolTable { > + pub raw: HashMap<KconfigSymbol, TypeInfo>, > + pub choices: Vec<ChoiceData>, > + pub modules_option: Option<KconfigSymbol>, // None until we find the modules attribute in exactly 1 config option > +} > + > +impl SymbolTable { > + pub fn new() -> Self { > + SymbolTable { > + raw: HashMap::new(), > + choices: Vec::new(), > + modules_option: None, > + } > + } > + > + pub fn from_parts( > + raw: HashMap<KconfigSymbol, TypeInfo>, > + choices: Vec<ChoiceData>, > + modules_option: Option<KconfigSymbol>, > + ) -> Self { > + SymbolTable { > + raw, > + choices, > + modules_option, > + } > + } > + > + pub fn merge_insert_new_solved( > + &mut self, > + var: KconfigSymbol, > + kconfig_type: Option<Type>, > + raw_constraints: Vec<OrExpression>, > + kconfig_ranges: Vec<Range>, > + kconfig_defaults: Vec<DefaultAttribute>, > + visibility: Vec<Option<OrExpression>>, > + arch: Arch, > + definition_condition: Vec<OrExpression>, > + selected_by: Option<(KconfigSymbol, Cond)>, > + selects: Vec<(KconfigSymbol, Cond)>, > + implies: Vec<(KconfigSymbol, Cond)>, > + ) { > + let entry = self.raw.entry(var.clone()); > + > + match entry { > + hash_map::Entry::Vacant(v) => { > + let mut t = TypeInfo::new_empty(); > + t.insert( > + kconfig_type, > + raw_constraints, > + kconfig_ranges, > + kconfig_defaults, > + visibility, > + arch, > + definition_condition, > + selected_by, > + selects, > + implies, > + ); > + v.insert(t); > + } > + > + hash_map::Entry::Occupied(mut o) => { > + let t = o.get_mut(); > + > + t.insert( > + kconfig_type, > + raw_constraints, > + kconfig_ranges, > + kconfig_defaults, > + visibility, > + arch, > + definition_condition, > + selected_by, > + selects, > + implies, > + ); > + } > + } > + } > +} > + > +fn merge_selected_by( > + map: &mut HashMap<String, Vec<(Arch, Cond)>>, > + arch: Arch, > + selected_by: (KconfigSymbol, Cond), > +) { > + map.entry(selected_by.0) > + .or_default() // empty vec > + .push((arch, selected_by.1)); > +} > + > +fn insert_variable_info( > + map: &mut HashMap<Arch, Vec<(Vec<Expression>, AttributeDef)>>, > + arch: Arch, > + definition_condition: Vec<Expression>, > + info: AttributeDef, > +) { > + map.entry(arch) > + .or_default() // empty vec > + .push((definition_condition, info)); > +} > diff --git a/scripts/kconfirm/kconfirm-linux/Cargo.toml b/scripts/kconfirm/kconfirm-linux/Cargo.toml > new file mode 100644 > index 000000000000..9516399e1dae > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-linux/Cargo.toml > @@ -0,0 +1,10 @@ > +# SPDX-License-Identifier: GPL-2.0 > +[package] > +name = "kconfirm-linux" > +version = "0.10.0" > +edition = "2024" > +rust-version.workspace = true > + > +[dependencies] > +kconfirm-lib = { path = "../kconfirm-lib" } > +nom-kconfig = { workspace = true } > diff --git a/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs b/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs > new file mode 100644 > index 000000000000..227faa17b962 > --- /dev/null > +++ b/scripts/kconfirm/kconfirm-linux/src/getopt_ffi.rs > @@ -0,0 +1,99 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +use std::env; > +use std::ffi::CStr; > +use std::ffi::CString; > +use std::os::raw::c_char; > +use std::os::raw::c_int; > +use std::ptr; > + > +pub const REQUIRED_ARGUMENT: c_int = 1; > + > +#[repr(C)] > +pub struct option { > + pub name: *const c_char, > + pub has_arg: c_int, > + pub flag: *mut c_int, > + pub val: c_int, > +} > + > +#[link(name = "c")] > +unsafe extern "C" { > + fn getopt_long( > + argc: c_int, > + argv: *mut *mut c_char, > + optstring: *const c_char, > + longopts: *const option, > + longindex: *mut c_int, > + ) -> c_int; > + > + static mut optarg: *mut c_char; > + static mut optind: c_int; > +} > + > +pub struct Getopt { > + _cstrings: Vec<CString>, > + argv: Vec<*mut c_char>, > + argc: c_int, > +} > + > +impl Getopt { > + pub fn new() -> Self { > + let raw_args: Vec<String> = env::args().collect(); > + > + let cstrings: Vec<CString> = raw_args > + .iter() > + .map(|s| CString::new(s.as_str()).unwrap()) > + .collect(); This panics if the arguments aren't valid UTF-8. Using env::args_os() might be better. getopt_long() doesn't care if the arguments are valid UTF-8 or not. > + let mut argv: Vec<*mut c_char> = > + cstrings.iter().map(|s| s.as_ptr() as *mut c_char).collect(); > + > + argv.push(ptr::null_mut()); > + > + let argc = (argv.len() - 1) as c_int; > + > + Self { > + _cstrings: cstrings, > + argv, > + argc, > + } > + } > + > + pub fn reset(&mut self) { > + unsafe { > + optind = 1; > + } > + } > + > + pub fn next( > + &mut self, > + optstring: &CStr, > + longopts: &[option], > + ) -> Option<Result<(char, Option<String>), String>> { > + unsafe { > + let c = getopt_long( > + self.argc, > + self.argv.as_mut_ptr(), > + optstring.as_ptr(), > + longopts.as_ptr(), > + ptr::null_mut(), > + ); > + > + if c == -1 { > + return None; > + } > + > + if c == '?' as c_int { if c == c_int::from(b'?') { Might be a bit clearer. > + return Some(Err("invalid argument".into())); You can just std::process::exit() here. getopt_long() will have printed an error message. This assumes that the > + } > + > + let arg = if optarg.is_null() { > + None > + } else { > + Some(CStr::from_ptr(optarg).to_string_lossy().into_owned()) > + }; I'd prefer to return an OsString here. > + Some(Ok((c as u8 as char, arg))) > + } > + } > +} I think it is simpler to just inline all of this code into its single call-site. The safety of the code is obvious in context, and you can avoid checking for impossible errors. For instance, since all of the options have required arguments, it really is safe to dereference optarg without any null check. Yes, this is C with Rust syntax, but it's *simple* C with Rust syntax. As an aside, I don't know if you want to accept abbreviated long options. Most tools using getopt_long() allow them. If you don't, I can show you how to reject them. -- Sincerely, Demi Marie Obenour (she/her/hers) [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 7253 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 6:10 ` Demi Marie Obenour @ 2026-05-17 9:58 ` Miguel Ojeda 2026-05-17 20:25 ` Demi Marie Obenour 0 siblings, 1 reply; 20+ messages in thread From: Miguel Ojeda @ 2026-05-17 9:58 UTC (permalink / raw) To: Demi Marie Obenour Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sun, May 17, 2026 at 8:10 AM Demi Marie Obenour <demiobenour@gmail.com> wrote: > > I think it is simpler to just inline all of this code into its > single call-site. The safety of the code is obvious in context, > and you can avoid checking for impossible errors. For instance, > since all of the options have required arguments, it really is safe > to dereference optarg without any null check. If we are going to have unsafe code, then let's please build safe abstractions wherever possible, just like we do elsewhere. We should also write `// SAFETY` comments and enable the lints that catch that etc., just like elsewhere too. (This is not to say we should use `getopt` instead of something like `clap` -- as soon as we start using `cargo vendor`, then it makes sense to at least consider having a set of vetted, well-known crates to write Rust tools in-tree, as I mentioned in v1.) Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 9:58 ` Miguel Ojeda @ 2026-05-17 20:25 ` Demi Marie Obenour 2026-05-17 22:53 ` Miguel Ojeda 0 siblings, 1 reply; 20+ messages in thread From: Demi Marie Obenour @ 2026-05-17 20:25 UTC (permalink / raw) To: Miguel Ojeda Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild [-- Attachment #1.1.1: Type: text/plain, Size: 1229 bytes --] On 5/17/26 05:58, Miguel Ojeda wrote: > On Sun, May 17, 2026 at 8:10 AM Demi Marie Obenour > <demiobenour@gmail.com> wrote: >> >> I think it is simpler to just inline all of this code into its >> single call-site. The safety of the code is obvious in context, >> and you can avoid checking for impossible errors. For instance, >> since all of the options have required arguments, it really is safe >> to dereference optarg without any null check. > > If we are going to have unsafe code, then let's please build safe > abstractions wherever possible, just like we do elsewhere. We should > also write `// SAFETY` comments and enable the lints that catch that > etc., just like elsewhere too. > > (This is not to say we should use `getopt` instead of something like > `clap` -- as soon as we start using `cargo vendor`, then it makes > sense to at least consider having a set of vetted, well-known crates > to write Rust tools in-tree, as I mentioned in v1.) I was hoping for Linux to avoid the Rust trend of downloading tons of third-party crates, with all the supply-chain risks that entails. Hence the idea of using getopt and system C libraries. -- Sincerely, Demi Marie Obenour (she/her/hers) [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 7253 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 20:25 ` Demi Marie Obenour @ 2026-05-17 22:53 ` Miguel Ojeda 0 siblings, 0 replies; 20+ messages in thread From: Miguel Ojeda @ 2026-05-17 22:53 UTC (permalink / raw) To: Demi Marie Obenour Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sun, May 17, 2026 at 10:25 PM Demi Marie Obenour <demiobenour@gmail.com> wrote: > > I was hoping for Linux to avoid the Rust trend of downloading tons > of third-party crates, with all the supply-chain risks that entails. I completely agree -- it is why I said a well-known, vetted set of crates. That is, we should decide on e.g. a single CLI arg parser, a single logger, etc. for most of our tools, and ideally they should be well-known crates (ideally already trusted via use in the compiler itself). Moreover, they should be pinned with `--locked` or similar (like we already recommend for `bindgen-cli`), so that we only ever use something that matches the hash in the lockfile that would be committed in the tree. Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-16 21:53 ` [RFC PATCH v3 1/3] scripts: " Julian Braha 2026-05-17 6:10 ` Demi Marie Obenour @ 2026-05-17 6:28 ` Miguel Ojeda 2026-05-17 7:32 ` Demi Marie Obenour 2026-05-17 9:28 ` Nathan Chancellor 1 sibling, 2 replies; 20+ messages in thread From: Miguel Ojeda @ 2026-05-17 6:28 UTC (permalink / raw) To: Julian Braha Cc: nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote: > > +CARGO = cargo Question to Kbuild: would it hurt to hardcore `--offline` here? If someone within Make actually ever needs Cargo to fetch something, then they should be very explicit about it (in which case we could have another variable etc.). > - rust/libpin_init_internal.so rust/libpin_init_internal.dylib > + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \ Spurious change? > +$(TARGET): > + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS) This probably does not work in `O=` builds or in cases where the `srctree` is read-only (please see my other reply on the docs patch). > +// SPDX-License-Identifier: GPL-2.0-only > +use crate::AnalysisArgs; This appears to use a quite different, custom Rust style. If this is going to be developed in-tree, then we should do our best to follow the guidelines: https://docs.kernel.org/rust/coding-guidelines.html For instance, a few key points are not followed here: - Public items are not documented. When they are, they seem to use comments instead of actual docs. Markdown is not used either. - No examples, doctests or tests. - No `// SAFETY` comments for unsafe code. > +unsafe extern "C" { > + fn curl_global_init(flags: c_long) -> CURLcode; > + > + fn curl_easy_init() -> *mut CURL; > + > + fn curl_easy_cleanup(handle: *mut CURL); > + > + fn curl_easy_perform(handle: *mut CURL) -> CURLcode; > + > + fn curl_easy_strerror(code: CURLcode) -> *const c_char; > + > + fn curl_easy_setopt(handle: *mut CURL, option: CURLoption, ...) -> CURLcode; > + > + fn curl_easy_getinfo(handle: *mut CURL, info: CURLINFO, ...) -> CURLcode; > +} I like minimizing dependencies, but since we require vendored dependencies anyway, then it may be simpler to use a common ("standard") Rust dependency for these things. Then we can all agree on a particular one and use that when the same need arises. In fact, it seems like FFI here and in the other file is the only source of `unsafe` code, no? We could perhaps even `forbid` it otherwise. If we truly want to minimize dependencies even if we have vendored ones, then we could call into the `curl` CLI instead, just like we e.g. call into `bindgen` instead of using its library. Thanks! Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 6:28 ` Miguel Ojeda @ 2026-05-17 7:32 ` Demi Marie Obenour 2026-05-17 9:30 ` Miguel Ojeda 2026-05-17 9:28 ` Nathan Chancellor 1 sibling, 1 reply; 20+ messages in thread From: Demi Marie Obenour @ 2026-05-17 7:32 UTC (permalink / raw) To: Miguel Ojeda, Julian Braha Cc: nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild [-- Attachment #1.1.1: Type: text/plain, Size: 3067 bytes --] On 5/17/26 02:28, Miguel Ojeda wrote: > On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote: >> >> +CARGO = cargo > > Question to Kbuild: would it hurt to hardcore `--offline` here? > > If someone within Make actually ever needs Cargo to fetch something, > then they should be very explicit about it (in which case we could > have another variable etc.). > >> - rust/libpin_init_internal.so rust/libpin_init_internal.dylib >> + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \ > > Spurious change? > >> +$(TARGET): >> + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS) > > This probably does not work in `O=` builds or in cases where the > `srctree` is read-only (please see my other reply on the docs patch). > >> +// SPDX-License-Identifier: GPL-2.0-only >> +use crate::AnalysisArgs; > > This appears to use a quite different, custom Rust style. If this is > going to be developed in-tree, then we should do our best to follow > the guidelines: > > https://docs.kernel.org/rust/coding-guidelines.html > > For instance, a few key points are not followed here: > > - Public items are not documented. When they are, they seem to use > comments instead of actual docs. Markdown is not used either. > > - No examples, doctests or tests. > > - No `// SAFETY` comments for unsafe code. > >> +unsafe extern "C" { >> + fn curl_global_init(flags: c_long) -> CURLcode; >> + >> + fn curl_easy_init() -> *mut CURL; >> + >> + fn curl_easy_cleanup(handle: *mut CURL); >> + >> + fn curl_easy_perform(handle: *mut CURL) -> CURLcode; >> + >> + fn curl_easy_strerror(code: CURLcode) -> *const c_char; >> + >> + fn curl_easy_setopt(handle: *mut CURL, option: CURLoption, ...) -> CURLcode; >> + >> + fn curl_easy_getinfo(handle: *mut CURL, info: CURLINFO, ...) -> CURLcode; >> +} > > I like minimizing dependencies, but since we require vendored > dependencies anyway, then it may be simpler to use a common > ("standard") Rust dependency for these things. Then we can all agree > on a particular one and use that when the same need arises. Using a ton of vendored dependencies would make for unreviewable patches. It's also the kind of thing that gives distro packagers headaches. > In fact, it seems like FFI here and in the other file is the only > source of `unsafe` code, no? We could perhaps even `forbid` it > otherwise. I don't want to pull in a huge library like clap. A simple getopt_long implementation could also be an option. > If we truly want to minimize dependencies even if we have vendored > ones, then we could call into the `curl` CLI instead, just like we > e.g. call into `bindgen` instead of using its library. I'm the one who suggested using FFI here and for command-line parsing. The command-line interface would also work. -- Sincerely, Demi Marie Obenour (she/her/hers) [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 7253 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 7:32 ` Demi Marie Obenour @ 2026-05-17 9:30 ` Miguel Ojeda 2026-05-17 9:32 ` Demi Marie Obenour 0 siblings, 1 reply; 20+ messages in thread From: Miguel Ojeda @ 2026-05-17 9:30 UTC (permalink / raw) To: Demi Marie Obenour Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sun, May 17, 2026 at 9:32 AM Demi Marie Obenour <demiobenour@gmail.com> wrote: > > Using a ton of vendored dependencies would make for unreviewable > patches. I am referring to `cargo vendor` here, not to copying in-tree -- we already discussed that in previous versions. Please note that I was arguing for avoiding actual vendoring in previous versions... > I'm the one who suggested using FFI here and for command-line parsing. > The command-line interface would also work. Yes, I suggested the CLI in v1, and then you mentioned the library in v2. Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 9:30 ` Miguel Ojeda @ 2026-05-17 9:32 ` Demi Marie Obenour 2026-05-17 9:48 ` Miguel Ojeda 0 siblings, 1 reply; 20+ messages in thread From: Demi Marie Obenour @ 2026-05-17 9:32 UTC (permalink / raw) To: Miguel Ojeda Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild [-- Attachment #1.1.1: Type: text/plain, Size: 842 bytes --] On 5/17/26 05:30, Miguel Ojeda wrote: > On Sun, May 17, 2026 at 9:32 AM Demi Marie Obenour > <demiobenour@gmail.com> wrote: >> >> Using a ton of vendored dependencies would make for unreviewable >> patches. > > I am referring to `cargo vendor` here, not to copying in-tree -- we > already discussed that in previous versions. That's true. Some distros (Fedora, Debian) don't like that either, but that's a bigger ecosystem-wide concern. > Please note that I was arguing for avoiding actual vendoring in > previous versions... > >> I'm the one who suggested using FFI here and for command-line parsing. >> The command-line interface would also work. > > Yes, I suggested the CLI in v1, and then you mentioned the library in v2. CLI would also work just as well. -- Sincerely, Demi Marie Obenour (she/her/hers) [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 7253 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 9:32 ` Demi Marie Obenour @ 2026-05-17 9:48 ` Miguel Ojeda 0 siblings, 0 replies; 20+ messages in thread From: Miguel Ojeda @ 2026-05-17 9:48 UTC (permalink / raw) To: Demi Marie Obenour Cc: Julian Braha, nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sun, May 17, 2026 at 11:33 AM Demi Marie Obenour <demiobenour@gmail.com> wrote: > > That's true. Some distros (Fedora, Debian) don't like that either, > but that's a bigger ecosystem-wide concern. Do you mean that distributions would like to package this tool on their own? If so, I don't see why packagers would be blocked. Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 1/3] scripts: add kconfirm 2026-05-17 6:28 ` Miguel Ojeda 2026-05-17 7:32 ` Demi Marie Obenour @ 2026-05-17 9:28 ` Nathan Chancellor 1 sibling, 0 replies; 20+ messages in thread From: Nathan Chancellor @ 2026-05-17 9:28 UTC (permalink / raw) To: Miguel Ojeda Cc: Julian Braha, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sun, May 17, 2026 at 08:28:16AM +0200, Miguel Ojeda wrote: > On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote: > > > > +CARGO = cargo > > Question to Kbuild: would it hurt to hardcore `--offline` here? > > If someone within Make actually ever needs Cargo to fetch something, > then they should be very explicit about it (in which case we could > have another variable etc.). No, I don't think so. I think there would need to be a very compelling reason for connecting to the network during the build process. Although, we would need to handle someone passing CARGO via the make command line so that '--offline' does not get blown away. > > - rust/libpin_init_internal.so rust/libpin_init_internal.dylib > > + rust/libpin_init_internal.so rust/libpin_init_internal.dylib \ > > Spurious change? Maybe 'scripts/kconfirm' used to be here? Another thing I just realized: scripts/kconfirm is going to mess with shell autocompletion for some people, as scripts/kc<tab> will currently always complete to scripts/kconfig. Not sure if that will be that big of a deal but I know Linus has complained about that in the past. > > +$(TARGET): > > + $(CARGO) run --release --offline -p kconfirm-linux -- --linux-path $(srctree) --enable-arch $(SRCARCH) $(KCONFIRM_ARGS) > > This probably does not work in `O=` builds or in cases where the > `srctree` is read-only (please see my other reply on the docs patch). Yeah, it seems like this wants something like '--target-dir $(obj)' or '--target-dir $(objtree)/scripts/kconfirm'? I don't find this to be particularly readable either (I am more used to "build then run" as two separate steps) but maybe that is because I am just not familiar with Rust projects. Cheers, Nathan ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC PATCH v3 2/3] Documentation: add kconfirm 2026-05-16 21:53 [RFC v3 0/3] add kconfirm Julian Braha 2026-05-16 21:53 ` [RFC PATCH v3 1/3] scripts: " Julian Braha @ 2026-05-16 21:53 ` Julian Braha 2026-05-17 6:05 ` Miguel Ojeda 2026-05-16 21:53 ` [RFC PATCH v3 3/3] MAINTAINERS: create entry for kconfirm Julian Braha ` (2 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Julian Braha @ 2026-05-16 21:53 UTC (permalink / raw) To: nathan, nsc Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild, Julian Braha Add usage documentation and a brief description for kconfirm to Documentation/dev-tools/ --- Documentation/dev-tools/index.rst | 1 + Documentation/dev-tools/kconfirm.rst | 222 +++++++++++++++++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 Documentation/dev-tools/kconfirm.rst diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst index 59cbb77b33ff..130ebc0d7282 100644 --- a/Documentation/dev-tools/index.rst +++ b/Documentation/dev-tools/index.rst @@ -40,3 +40,4 @@ Documentation/process/debugging/index.rst autofdo propeller container + kconfirm diff --git a/Documentation/dev-tools/kconfirm.rst b/Documentation/dev-tools/kconfirm.rst new file mode 100644 index 000000000000..8790672c9a87 --- /dev/null +++ b/Documentation/dev-tools/kconfirm.rst @@ -0,0 +1,222 @@ +.. SPDX-License-Identifier: GPL-2.0-only +.. Copyright (C) 2026 Julian Braha <julianbraha@gmail.com> + +======== +kconfirm +======== + +kconfirm is a static analysis tool for the kernel's Kconfig. It checks +the entire tree-wide Kconfig, and reports misusage like dead code. In the +case of dead default statements, these can be a code smell. + +kconfirm has some additional, optional checks. The first is for dead links +in the Kconfig help texts. Since this has a high potential for false +positives (due to websites blocking bots) and slows down runtime +significantly, it is disabled by default. + +Another optional check is for config options that select visible config +options. Examples of how to enable the optional checks are included +below. + +kconfirm is written in Rust and lives in ``scripts/kconfirm``. Other +than the dead link checks, kconfirm aims for zero false positives. + +By default, kconfirm checks the same architecture as your kernel build, +but you can also enable checking more architectures with +``--enable-arch`` or disable checking your default architecture with +``--disable-arch``. Alarms are deduplicated across all affected +architectures; kconfirm displays a tag with the corresponding Kconfig +architecture config option names. For example, ``[RISCV]`` indicates +that an alarm is specific to RISC-V, while ``[ARM, X86]`` indicates that +an alarm affects both arm and x86. Running on each architecture will take +approximately one minute on modern consumer hardware. + +**NOTE**: kconfirm does not modify or compile the source tree; it is +strictly a static checker. + + +Getting Started +=============== + + +kconfirm's Minimum Supported Rust Version (MSRV) is v1.85.0, because +it uses Rust edition 2024, and this is the earliest supported version. + +kconfirm requires the Cargo package manager and an internet connection +to download its dependencies from crates.io. + +In ``scripts/kconfirm/`` run the following to download the dependencies:: + + cargo vendor + +Then, kconfirm can be built and run from the top of the +kernel source tree:: + + make kconfirm + +The compiled ``kconfirm-linux`` binary will be available in +``scripts/kconfirm/target/release/``. + +The default checks currently cover dead code analysis, as well as invalid +(reverse) ranges and constant conditions. ``select_visible`` and +``dead_links`` must be turned on explicitly with ``--enable-check``; +conversely, any default check can be turned off with ``--disable-check``. Both +options accept either a comma-separated list or repeated flags, so the +following two invocations are equivalent:: + + kconfirm-linux --linux-path . --enable-check select_visible,dead_link + kconfirm-linux --linux-path . --enable-check select_visible --enable-check dead_link + + +Options +======= + +**NOTE**: kconfirm's arguments must be provided in the ``KCONFIRM_ARGS`` +environment variable if running with ``make``. See `Examples`_. + +Available options: + +``--linux-path PATH`` + The path to the linux source tree to analyze. ``make`` uses this + option to pass the current linux tree, but this option can be used + when running the tool directly with another source tree. + See `Examples`_. + +``--enable-check CHECK[,CHECK...]`` + + Enable one or more checks in addition to the default set. May be + given multiple times, or as a single comma-separated list. See + `Available checks`_ below for valid names. + +``--disable-check CHECK[,CHECK...]`` + + Disable one or more checks from the default set. May be given + multiple times, or as a single comma-separated list. + +``--enable-arch ARCH[,ARCH...]`` + + Enable one or more architectures in addition to the default + architecture. May be given multiple times, or as a single + comma-separated list. + +``--disable-arch ARCH[,ARCH...]`` + + Disable one or more architectures from the default set. May be given + multiple times, or as a single comma-separated list. + +``-h, --help`` + + Show the help message and exit. + +``-V, --version`` + + Show version information and exit. + + +Available checks +================ + +Each check has a string name that is accepted by ``--enable`` and +``--disable``. Checks marked *(default)* are enabled unless turned off +explicitly. + +``duplicate_dependency`` *(default)* + + Reports duplicated ``depends on`` entries on a single Kconfig symbol. + +``duplicate_range`` *(default)* + + Reports duplicated ``range`` entries on a single Kconfig symbol. + +``dead_range`` *(default)* + + Reports ``range`` entries that will never be evaluated, due to an + unconditional range entry. + +``duplicate_select`` *(default)* + + Reports duplicated ``select`` entries on a single Kconfig symbol. + +``dead_select`` *(default)* + + Reports dead ``select`` entries that will never be evaluated, due to an + unconditional select entry of the same config option. + +``duplicate_imply`` *(default)* + + Reports duplicated ``imply`` entries on a single Kconfig symbol. + +``dead_imply`` *(default)* + + Reports dead ``imply`` entries that will never be evaluated, due to an + unconditional imply entry for the same config option. + +``duplicate_default`` *(default)* + + Reports duplicated ``default`` entries on a single Kconfig symbol. + +``dead_default`` *(default)* + + Reports ``default`` entries that can never be selected, for example + because their condition is unsatisfiable. + +``constant_condition`` *(default)* + + Reports conditions for any entries that always evaluate to ``true``. + +``reverse_range`` *(default)* + + Reports invalid ranges for int and hex configuration options. + +``failed_parse`` *(default)* + + Reports a parsing failure of the Kconfig. Cannot be disabled. + +``select_visible`` + + Reports configuration options that ``select`` a config option that is + visible to users. + +``dead_link`` + + Reports broken URLs found in Kconfig help text. Because this + performs network requests it can be quite slow, and is disabled by + default. May also have false positives. + +``ungrouped_attribute`` + + Reports ungrouped entries, like ``select`` and ``depends on``. + This is a style check, and is disabled by default. + +``duplicate_default_value`` + + Reports duplicate default values that have different conditions. + Suggests combining the conditions using a logical-or ``||``. + This is a style check, and is disabled by default. + + +Examples +======== + +Compile (as needed) and run on the current tree:: + + make kconfirm + +To additionally enable the dead link and select-visible checks:: + + make kconfirm KCONFIRM_ARGS="--enable-check=dead_link,select_visible" + +To disable a check (here, ``duplicate_dependency``) while keeping the +rest of the default set:: + + make kconfirm KCONFIRM_ARGS="--disable-check duplicate_dependency" + +To enable an architecture (here, ``RISC-V``) while keeping the +default architecture enabled:: + + make kconfirm KCONFIRM_ARGS="--enable-arch riscv" + +To run the default checks against a kernel tree separate from the +current directory, such as ``~/repos/linux``:: + + scripts/kconfirm/target/release/kconfirm-linux --linux-path ~/repos/linux -- 2.53.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 2/3] Documentation: add kconfirm 2026-05-16 21:53 ` [RFC PATCH v3 2/3] Documentation: " Julian Braha @ 2026-05-17 6:05 ` Miguel Ojeda 2026-05-17 9:40 ` Nathan Chancellor 0 siblings, 1 reply; 20+ messages in thread From: Miguel Ojeda @ 2026-05-17 6:05 UTC (permalink / raw) To: Julian Braha Cc: nathan, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote: > > +kconfirm's Minimum Supported Rust Version (MSRV) is v1.85.0, because > +it uses Rust edition 2024, and this is the earliest supported version. Note: this means it will be the first code within the kernel tree using the new edition. I think it is fine, since in general no one should be copying code from here to kernel code or vice versa. (For context for others: code in one edition can have different behavior than in another edition, and thus it is risky to mix them up by mistake). > +In ``scripts/kconfirm/`` run the following to download the dependencies:: > + > + cargo vendor I am not sure how important this is for `scripts/` and/or `tools/` (Kbuild may have a policy), but this should probably handle `O=` builds. In some cases, the source tree may even be read-only, i.e. we wouldn't be able to create `target/` there. Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 2/3] Documentation: add kconfirm 2026-05-17 6:05 ` Miguel Ojeda @ 2026-05-17 9:40 ` Nathan Chancellor 2026-05-17 12:35 ` Miguel Ojeda 0 siblings, 1 reply; 20+ messages in thread From: Nathan Chancellor @ 2026-05-17 9:40 UTC (permalink / raw) To: Miguel Ojeda Cc: Julian Braha, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sun, May 17, 2026 at 08:05:30AM +0200, Miguel Ojeda wrote: > On Sat, May 16, 2026 at 11:54 PM Julian Braha <julianbraha@gmail.com> wrote: > > +In ``scripts/kconfirm/`` run the following to download the dependencies:: > > + > > + cargo vendor > > I am not sure how important this is for `scripts/` and/or `tools/` > (Kbuild may have a policy), but this should probably handle `O=` > builds. > > In some cases, the source tree may even be read-only, i.e. we wouldn't > be able to create `target/` there. I guess this is kind of a weird/unique situation. I agree that the files generated by 'cargo run' should absolutely be contained within the build folder; at that point, $(srctree) could be read only and I would consider it rude not to respect the user's choice of build directory. For 'cargo vendor' however, I am not sure. They are source files and I would expect that running 'cargo vendor' would be more considered part of preparing the source tree, rather than the build one (so it should not be read only). At the same time, it might be safer for dependency updates and internal consistency that they are confined to the build folder. I guess we would only want to remove them with a 'distclean', rather than 'mrproper' or 'clean', in that case, to avoid requiring users to constantly run 'cargo vendor'. It might be more ergonomic for this to be a Kbuild target ('kconfirmvendor'?) so that this could be handled automatically based on the user's build command. Additionally, can we detect explicitly when dependencies are not properly vendored and error with a more helpful error message? The build command in patch 1 just throws up its hands when the build fails and asks if the dependencies have been set up but if we provided our own vendoring build target, we could add some canary that says we vendored successfully and if that is not present, error before even running the build and say "hey, you need to explicitly run this target before you build". -- Cheers, Nathan ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC PATCH v3 2/3] Documentation: add kconfirm 2026-05-17 9:40 ` Nathan Chancellor @ 2026-05-17 12:35 ` Miguel Ojeda 0 siblings, 0 replies; 20+ messages in thread From: Miguel Ojeda @ 2026-05-17 12:35 UTC (permalink / raw) To: Nathan Chancellor Cc: Julian Braha, nsc, jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On Sun, May 17, 2026 at 11:40 AM Nathan Chancellor <nathan@kernel.org> wrote: > > I guess this is kind of a weird/unique situation. I agree that the files > generated by 'cargo run' should absolutely be contained within the build > folder; at that point, $(srctree) could be read only and I would > consider it rude not to respect the user's choice of build directory. > For 'cargo vendor' however, I am not sure. They are source files and I > would expect that running 'cargo vendor' would be more considered part > of preparing the source tree, rather than the build one (so it should > not be read only). That would simplify things, yeah. We could always start there and see if someone needs it. > At the same time, it might be safer for dependency updates and internal > consistency that they are confined to the build folder. I guess we would > only want to remove them with a 'distclean', rather than 'mrproper' or > 'clean', in that case, to avoid requiring users to constantly run > 'cargo vendor'. It might be more ergonomic for this to be a Kbuild > target ('kconfirmvendor'?) so that this could be handled automatically > based on the user's build command. Yeah, it is a bit painful to not have the usual Kbuild variables/infrastructure around... On the other hand, it is a nice property to know that nothing called via `make` will ever connect (or need to connect) to the Internet. Hmm... Perhaps a good middle ground would be having something in the name that makes it obvious it will connect, e.g. `fetch` like Git? Or, if people feel strongly about the property mentioned, then something like an environment variable that needs to be set to allow it (with a message printed about it if it is not set). If this were allowed, i.e. if we are OK having things in `make` that fetch stuff and put it in the build folder (only in certain targets, of course), then we could actually think about doing more things that we didn't so far, such as other setup-like targets, e.g. preparing kernel.org toolchains, setting up a Rust toolchain via `rustup` (including `bindgen` etc.), and so on and so forth. > Additionally, can we detect explicitly when dependencies are not > properly vendored and error with a more helpful error message? The build > command in patch 1 just throws up its hands when the build fails and > asks if the dependencies have been set up but if we provided our own > vendoring build target, we could add some canary that says we vendored > successfully and if that is not present, error before even running the > build and say "hey, you need to explicitly run this target before you > build". +1, good error messages help a lot. Something like `rustavailable` that prints which particular thing is missing is great (that one even tries to warn about some problematic versions testing for bugs -- hopefully we don't need `autoconf`... :). Cheers, Miguel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC PATCH v3 3/3] MAINTAINERS: create entry for kconfirm 2026-05-16 21:53 [RFC v3 0/3] add kconfirm Julian Braha 2026-05-16 21:53 ` [RFC PATCH v3 1/3] scripts: " Julian Braha 2026-05-16 21:53 ` [RFC PATCH v3 2/3] Documentation: " Julian Braha @ 2026-05-16 21:53 ` Julian Braha 2026-05-16 22:36 ` [RFC v3 0/3] add kconfirm Julian Braha 2026-05-17 6:14 ` Demi Marie Obenour 4 siblings, 0 replies; 20+ messages in thread From: Julian Braha @ 2026-05-16 21:53 UTC (permalink / raw) To: nathan, nsc Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild, Julian Braha Add myself as maintainer of kconfirm. Signed-off-by: Julian Braha <julianbraha@gmail.com> --- MAINTAINERS | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index b2040011a386..8f4f5a009228 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13824,6 +13824,12 @@ F: Documentation/kbuild/kconfig* F: scripts/Kconfig.include F: scripts/kconfig/ +KCONFIRM +M: Julian Braha <julianbraha@gmail.com> +S: Maintained +F: Documentation/dev-tools/kconfirm.rst +F: scripts/kconfirm/ + KCORE M: Omar Sandoval <osandov@osandov.com> L: linux-debuggers@vger.kernel.org -- 2.53.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [RFC v3 0/3] add kconfirm 2026-05-16 21:53 [RFC v3 0/3] add kconfirm Julian Braha ` (2 preceding siblings ...) 2026-05-16 21:53 ` [RFC PATCH v3 3/3] MAINTAINERS: create entry for kconfirm Julian Braha @ 2026-05-16 22:36 ` Julian Braha 2026-05-17 6:14 ` Demi Marie Obenour 4 siblings, 0 replies; 20+ messages in thread From: Julian Braha @ 2026-05-16 22:36 UTC (permalink / raw) To: nathan, nsc Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, demiobenour, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On 5/16/26 22:53, Julian Braha wrote: > Dependencies will need to first be downloaded from crates.io by running > the `cargo vendor` command in scripts/kconfirm/ Ugh, sorry all. I didn't realize that the linux gitignore blocks all dot files. So, the scripts/kconfirm/.cargo/config.toml wasn't checked in correctly due to this. It should have contained the following: ``` [source.crates-io] replace-with = "vendored-sources" [source.vendored-sources] directory = "vendor" ``` And Nicolas, I think this was why v2 didn't compile for you. - Julian Braha ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v3 0/3] add kconfirm 2026-05-16 21:53 [RFC v3 0/3] add kconfirm Julian Braha ` (3 preceding siblings ...) 2026-05-16 22:36 ` [RFC v3 0/3] add kconfirm Julian Braha @ 2026-05-17 6:14 ` Demi Marie Obenour 2026-05-17 23:21 ` Julian Braha 4 siblings, 1 reply; 20+ messages in thread From: Demi Marie Obenour @ 2026-05-17 6:14 UTC (permalink / raw) To: Julian Braha, nathan, nsc Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild [-- Attachment #1.1.1: Type: text/plain, Size: 457 bytes --] On 5/16/26 17:53, Julian Braha wrote: > Hi all, > > kconfirm has shrunk a lot since v2! Thanks for dropping so many of the dependencies! It might be able to shrink it further by using the existing C Kconfig parser. This has the advantage that it ensures kconfirm and Kconfig will interpret the Kconfig files the same way. I'm not sure if that would be too much of a change. That's up to you. -- Sincerely, Demi Marie Obenour (she/her/hers) [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 7253 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC v3 0/3] add kconfirm 2026-05-17 6:14 ` Demi Marie Obenour @ 2026-05-17 23:21 ` Julian Braha 0 siblings, 0 replies; 20+ messages in thread From: Julian Braha @ 2026-05-17 23:21 UTC (permalink / raw) To: Demi Marie Obenour, nathan, nsc Cc: jani.nikula, akpm, gary, ljs, arnd, gregkh, masahiroy, ojeda, corbet, qingfang.deng, yann.prono, ej, linux-kernel, rust-for-linux, linux-doc, linux-kbuild On 5/17/26 07:14, Demi Marie Obenour wrote: >> Hi all, >> >> kconfirm has shrunk a lot since v2! > Thanks for dropping so many of the dependencies! > > It might be able to shrink it further by using the existing C Kconfig > parser. This has the advantage that it ensures kconfirm and Kconfig > will interpret the Kconfig files the same way. I'm not sure if > that would be too much of a change. That's up to you. Hi Demi, I did look into the in-tree parser after your suggestion in v2. What I discovered was that this parser performs too much evaluation of the kconfig code during parsing, making it unsuitable for purposes of static analysis: The in-tree parser doesn't actually output a parse tree that we can traverse and analyze, which is how kconfirm currently works. Instead, semantic actions directly construct the symbol table *during parsing*, with that symbol table being different from ours. I think(?) this makes the overall process faster (which is great for real-world kernel builds), but for static analysis purposes, we really need to preserve as much of the underlying code as we can. For example, we don't even preprocess variables, because this allows us to analyze more regardless of the host and target. (Well, architecture is the one exception there because we need to resolve imports/"source".) I do want to point out that Yann, the author of kconfirm's parsing library (nom-kconfig) is CC'd on these RFCs and has done an awesome job of supporting the parser and kconfirm's usage of it. He also helped reduce the number of indirect dependencies pulled in by the parser following your feedback in RFC v2. - Julian Braha ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-05-17 23:22 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-16 21:53 [RFC v3 0/3] add kconfirm Julian Braha 2026-05-16 21:53 ` [RFC PATCH v3 1/3] scripts: " Julian Braha 2026-05-17 6:10 ` Demi Marie Obenour 2026-05-17 9:58 ` Miguel Ojeda 2026-05-17 20:25 ` Demi Marie Obenour 2026-05-17 22:53 ` Miguel Ojeda 2026-05-17 6:28 ` Miguel Ojeda 2026-05-17 7:32 ` Demi Marie Obenour 2026-05-17 9:30 ` Miguel Ojeda 2026-05-17 9:32 ` Demi Marie Obenour 2026-05-17 9:48 ` Miguel Ojeda 2026-05-17 9:28 ` Nathan Chancellor 2026-05-16 21:53 ` [RFC PATCH v3 2/3] Documentation: " Julian Braha 2026-05-17 6:05 ` Miguel Ojeda 2026-05-17 9:40 ` Nathan Chancellor 2026-05-17 12:35 ` Miguel Ojeda 2026-05-16 21:53 ` [RFC PATCH v3 3/3] MAINTAINERS: create entry for kconfirm Julian Braha 2026-05-16 22:36 ` [RFC v3 0/3] add kconfirm Julian Braha 2026-05-17 6:14 ` Demi Marie Obenour 2026-05-17 23:21 ` Julian Braha
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox