Skip to contentSkip to conversation

kconfig: warn if an unknown symbol is selected

PATCH In review Paul Bolle 2 participants 5 messages Sep 30, 2014 – Jan 29, 2015 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

Paul Bolle

author
#0 lore raw

PATCH 1/1 kconfig: warn if an unknown symbol is selected

A select of an unknown symbol is basically treated as a nop and is silently skipped. This is annoying if the selected symbol contains a typo. It can also hide the fact that a treewide symbol cleanup was only done partially.

There are also a few cases were this might have been done on purpose. But that anti-pattern should be discouraged. Almost all select statements point to a known and reachable symbol. So people will likely assume that any selected symbol is actually set. Selects that violate this assumption can only be spotted by checking multiple Kconfig files, often across architectures. It is unlikely that people will do this regularly.

So let's warn when we notice a select of a symbol that is not known in the configuration we're creating.

---
First sent as an RFC in https://lkml.org/lkml/2014/9/26/887 (and
https://lkml.org/lkml/2014/9/26/886 for the cover letter).

Small modification to the commit explanation in comparison to the RFC, but identical patch. Minor edits to the cover-letter too.

 scripts/kconfig/conf.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
scripts/kconfig/conf.c +41 -0
index fef75fc756f4..de8406287531 100644
@@ -446,6 +446,45 @@ static void check_conf(struct menu *menu)
 		check_conf(child);
 }
 
+static void check_selects(struct menu *menu)
+{
+	struct symbol *sym, *sel;
+	struct property *prop;
+
+	while (menu) {
+		sym = menu->sym;
+
+		if (sym && !sym_is_choice(sym) &&
+		    sym->type != S_UNKNOWN &&
+		    sym->flags & SYMBOL_WRITE) {
+			for_all_properties(sym, prop, P_SELECT) {
+				sel = prop->expr->left.sym;
+				if (sel->type == S_UNKNOWN &&
+				    expr_calc_value(prop->visible.expr) != no) {
+					fprintf(stderr, "%s:%d:warning: ",
+						prop->file->name,
+						prop->lineno);
+					fprintf(stderr,
+						"'%s' selects unknown symbol '%s'\n",
+						sym->name,
+						sel->name);
+				}
+			}
+		}
+
+		if (menu->list) {
+			menu = menu->list;
+		} else if (menu->next) {
+			menu = menu->next;
+		} else while ((menu = menu->parent)) {
+			if (menu->next) {
+				menu = menu->next;
+				break;
+			}
+		}
+	}
+}
+
 static struct option long_opts[] = {
 	{"oldaskconfig",    no_argument,       NULL, oldaskconfig},
 	{"oldconfig",       no_argument,       NULL, oldconfig},
@@ -681,6 +720,8 @@ int main(int ac, char **av)
 		break;
 	}
 
+	check_selects(rootmenu.list);
+
 	if (sync_kconfig) {
 		/* silentoldconfig is used during the build so we shall update autoconf.
 		 * All other commands are only used to generate a config.
Signature · 1 line
1.9.3
author
#1 lore raw

Hi Yann,

Quoted 16 lines · Paul Bolle

A select of an unknown symbol is basically treated as a nop and is silently skipped. This is annoying if the selected symbol contains a typo. It can also hide the fact that a treewide symbol cleanup was only done partially.

There are also a few cases were this might have been done on purpose. But that anti-pattern should be discouraged. Almost all select statements point to a known and reachable symbol. So people will likely assume that any selected symbol is actually set. Selects that violate this assumption can only be spotted by checking multiple Kconfig files, often across architectures. It is unlikely that people will do this regularly.

So let's warn when we notice a select of a symbol that is not known in the configuration we're creating.

Quoted 8 lines

---
First sent as an RFC in https://lkml.org/lkml/2014/9/26/887 (and
https://lkml.org/lkml/2014/9/26/886 for the cover letter).

Small modification to the commit explanation in comparison to the RFC, but identical patch. Minor edits to the cover-letter too.

I've gotten no reaction from you yet on this patch nor on the preceding RFC. There have been a handful of other kconfig patches flying by the last half year or so (at least, that I know of). None of those have gotten any reaction from you, as far as I can tell.

Do you expect to handle this patch (and whatever has been submitted lately by other people) in the near future?

Quoted 54 lines
 scripts/kconfig/conf.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
scripts/kconfig/conf.c +39 -0
index fef75fc756f4..de8406287531 100644
@@ -446,6 +446,45 @@ static void check_conf(struct menu *menu)
 		check_conf(child);
 }
 
+static void check_selects(struct menu *menu)
+{
+	struct symbol *sym, *sel;
+	struct property *prop;
+
+	while (menu) {
+		sym = menu->sym;
+
+		if (sym && !sym_is_choice(sym) &&
+		    sym->type != S_UNKNOWN &&
+		    sym->flags & SYMBOL_WRITE) {
+			for_all_properties(sym, prop, P_SELECT) {
+				sel = prop->expr->left.sym;
+				if (sel->type == S_UNKNOWN &&
+				    expr_calc_value(prop->visible.expr) != no) {
+					fprintf(stderr, "%s:%d:warning: ",
+						prop->file->name,
+						prop->lineno);
+					fprintf(stderr,
+						"'%s' selects unknown symbol '%s'\n",
+						sym->name,
+						sel->name);
+				}
+			}
+		}
+
+		if (menu->list) {
+			menu = menu->list;
+		} else if (menu->next) {
+			menu = menu->next;
+		} else while ((menu = menu->parent)) {
+			if (menu->next) {
+				menu = menu->next;
+				break;
+			}
+		}
+	}
+}
+
 static struct option long_opts[] = {
 	{"oldaskconfig",    no_argument,       NULL, oldaskconfig},
 	{"oldconfig",       no_argument,       NULL, oldconfig},

scripts/kconfig/conf.c int main(int ac, char **av) · line 727

Quoted 8 lines
scripts/kconfig/conf.c +2 -0
@@ -681,6 +720,8 @@ int main(int ac, char **av)
 		break;
 	}
 
+	check_selects(rootmenu.list);
+
 	if (sync_kconfig) {
 		/* silentoldconfig is used during the build so we shall update autoconf.
 		 * All other commands are only used to generate a config.

Regards,

Paul Bolle

A message that is not in the archive (sent off-list or not yet received): 1422438897.5666.23.camel@x220

#3 lore raw
Quoted 5 lines · Paul Bolle

Now that we've been told Yann has disappeared, would you consider taking this patch into one of your trees? It would be nice to have people actually use it for a while.

Or should we first clean up (most of) the warnings it generates? There

It seems your fixes have been accepted in the meantime, but there is one new:
*** Default configuration is based on 'x86_64_defconfig' drivers/misc/cxl/Kconfig:8:warning: 'CXL_BASE' selects unknown symbol 'PPC_COPRO_BASE'

PPC_COPRO_BASE is not known on x86, but at the same time, there is no way for CXL_BASE to be selected on x86:

config CXL_BASE
bool
default n
select PPC_COPRO_BASE

config CXL

	tristate "Support for IBM Coherent Accelerators (CXL)"
	depends on PPC_POWERNV && PCI_MSI
	select CXL_BASE
	default m

Shouldn't we only warn about a select when it is triggered? An allyesconfig would still report all bogus selects for given architecture.

Michal

author
#4 lore raw
Quoted 21 lines · Michal Marek
Quoted 5 lines · Paul Bolle

Now that we've been told Yann has disappeared, would you consider taking this patch into one of your trees? It would be nice to have people actually use it for a while.

Or should we first clean up (most of) the warnings it generates? There

It seems your fixes have been accepted in the meantime, but there is one new:
*** Default configuration is based on 'x86_64_defconfig' drivers/misc/cxl/Kconfig:8:warning: 'CXL_BASE' selects unknown symbol 'PPC_COPRO_BASE'

PPC_COPRO_BASE is not known on x86, but at the same time, there is no way for CXL_BASE to be selected on x86:

config CXL_BASE
bool
default n
select PPC_COPRO_BASE

Quoted 8 lines

config CXL

	tristate "Support for IBM Coherent Accelerators (CXL)"
	depends on PPC_POWERNV && PCI_MSI
	select CXL_BASE
	default m

Shouldn't we only warn about a select when it is triggered? An allyesconfig would still report all bogus selects for given architecture.

Good catch! I think checking only for selects done by Kconfig symbols that are not "n" might do the trick. Expect a v2 one of these days.

The current load of issues (for allyesconfig on all arches except um) is pasted at the bottom of this message (with some comments added). Looking at that load I think it might be preferable to first most of fix those issues before adding this to kconfig.

Paul Bolle

alpha:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION'

REMOTEPROC should be ARM or OMAP specific?

lib/Kconfig.debug:1024:warning: 'DEBUG_ATOMIC_SLEEP' selects unknown symbol 'PREEMPT_COUNT'

Perhaps fixed by selecting PREEMPT_COUNT only for architectures that support it.

arc:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' arm:
arch/arm/mach-mvebu/Kconfig:40:warning: 'MACH_ARMADA_375' selects unknown symbol 'ARM_ERRATA_753970' arch/arm/mach-mvebu/Kconfig:55:warning: 'MACH_ARMADA_38X' selects unknown symbol 'ARM_ERRATA_753970'

I'm trying for quite some time to get this fixed.

arm64:
arch/arm64/Kconfig:183:warning: 'ARCH_TEGRA' selects unknown symbol 'HAVE_SMP' arch/arm64/Kconfig:193:warning: 'ARCH_TEGRA_132_SOC' selects unknown symbol 'USB_ARCH_HAS_EHCI'

Patches pending, I've been told.

drivers/irqchip/Kconfig:9:warning: 'ARM_GIC' selects unknown symbol 'MULTI_IRQ_HANDLER' drivers/irqchip/Kconfig:23:warning: 'ARM_GIC_V3' selects unknown symbol 'MULTI_IRQ_HANDLER'

MULTI_IRQ_HANDLER is arm specific.

avr32:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' blackfin:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' c6x:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' cris:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' frv:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' lib/Kconfig.debug:1024:warning: 'DEBUG_ATOMIC_SLEEP' selects unknown symbol 'PREEMPT_COUNT' hexagon:
arch/hexagon/Kconfig:22:warning: 'HEXAGON' selects unknown symbol 'NO_IOPORT_MAP'

Fixing obscure architectures requires patience!

drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' lib/Kconfig.debug:1024:warning: 'DEBUG_ATOMIC_SLEEP' selects unknown symbol 'PREEMPT_COUNT' ia64:
drivers/acpi/Kconfig:165:warning: 'ACPI_PROCESSOR' selects unknown symbol 'CPU_IDLE'

Perhaps, again, fixed by only selecting CPU_IDLE on architectures that support it.

drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' m32r:
m68k:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' metag:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' microblaze:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' mips:
mn10300:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' nios2:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' openrisc:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' parisc:
sound/pci/Kconfig:28:warning: 'SND_ALS300' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:53:warning: 'SND_ALI5451' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:158:warning: 'SND_AZT3328' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:466:warning: 'SND_EMU10K1' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:482:warning: 'SND_EMU10K1X' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:516:warning: 'SND_ES1938' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:528:warning: 'SND_ES1968' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:615:warning: 'SND_ICE1712' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:703:warning: 'SND_MAESTRO3' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:821:warning: 'SND_SONICVIBES' selects unknown symbol 'ZONE_DMA' sound/pci/Kconfig:833:warning: 'SND_TRIDENT' selects unknown symbol 'ZONE_DMA'

I guess one or more cases of !PARISC might do the trick. But the underlying problem is quite complicated. See commit 80ab8eae70e5 ("ALSA:
Enable CONFIG_ZONE_DMA for smaller PCI DMA masks"). Rather nice quote from Andrew Morton about adding these selects (instead of fixing the underlying problem):

    But then we'll just forget about it :(

(https://bugzilla.kernel.org/show_bug.cgi?id=68221#c19 ).

drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' powerpc:
s390:
score:
arch/score/Kconfig:28:warning: 'MACH_SPCT6600' selects unknown symbol 'SYS_SUPPORTS_32BIT_KERNEL'

Fixing obscure architectures requires patience!

sh:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' sparc:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' tile:
unicore32:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION' x86:
xtensa:
drivers/remoteproc/Kconfig:10:warning: 'REMOTEPROC' selects unknown symbol 'VIRTUALIZATION'

#5 lore raw
Quoted 31 lines · Paul Bolle
Quoted 21 lines · Michal Marek
Quoted 5 lines · Paul Bolle

Now that we've been told Yann has disappeared, would you consider taking this patch into one of your trees? It would be nice to have people actually use it for a while.

Or should we first clean up (most of) the warnings it generates? There

It seems your fixes have been accepted in the meantime, but there is one new:
*** Default configuration is based on 'x86_64_defconfig' drivers/misc/cxl/Kconfig:8:warning: 'CXL_BASE' selects unknown symbol 'PPC_COPRO_BASE'

PPC_COPRO_BASE is not known on x86, but at the same time, there is no way for CXL_BASE to be selected on x86:

config CXL_BASE
bool
default n
select PPC_COPRO_BASE

Quoted 8 lines

config CXL

	tristate "Support for IBM Coherent Accelerators (CXL)"
	depends on PPC_POWERNV && PCI_MSI
	select CXL_BASE
	default m

Shouldn't we only warn about a select when it is triggered? An allyesconfig would still report all bogus selects for given architecture.

Quoted 7 lines

Good catch! I think checking only for selects done by Kconfig symbols that are not "n" might do the trick. Expect a v2 one of these days.

The current load of issues (for allyesconfig on all arches except um) is pasted at the bottom of this message (with some comments added). Looking at that load I think it might be preferable to first most of fix those issues before adding this to kconfig.

Maybe. OTOH, you have no warnings on x86 and powerpc and you already have patches for the arm(64) warnings, so I think this can go in.

Michal