Re: kbuild: Invalid configuration generated for dependent choices by'conf' but not by 'mconf', with reason

[Ulf Magnusson]

On Thu, Dec 6, 2012 at 1:07 AM, Ulf Magnusson <ulfalizer.lkml2@xxxxxxxxx> wrote:
> Hi,
>
> Consider the following Kconfig file:
>
> config MODULES
>     def_bool y
>
> choice
>         tristate "C1"
> config A
>         tristate "A"
> config B
>         tristate "B"
> endchoice
>
> choice
>     bool "C2" if A
> config C
>     bool "C"
> config D
>     bool "D"
> endchoice
>
> When this Kconfig file is used with a configuration file containing
> just "CONFIG_A=y", conf generates the following invalid configuration:
>
> CONFIG_MODULES=y
> CONFIG_A=m
> # CONFIG_B is not set
> CONFIG_C=y
> # CONFIG_D is not set
>
> mconf on the other hand generates the correct configuration:
>
> CONFIG_MODULES=y
> CONFIG_A=y
> # CONFIG_B is not set
> CONFIG_C=y
> # CONFIG_D is not set
>
> To see this, compare the .config files generated by the following commands:
>
> $ scripts/kconfig/conf --defconfig=bug_config bug_Kconfig
>
> $ cp bug_config .config
> $ scripts/kconfig/mconf bug_Kconfig
>
> The root cause of the problem is the sym_calc_value(csym) call at the
> end of conf_set_all_new_symbols(). Since the second 'choice' depends
> on A, that call causes the value of A to be calculated. But this also
> causes SYMBOL_VALID to be set on A, preventing it from being
> reevaluated later when the configuration is written in conf_write().
> This reevaluation is required to get the correct value since only by
> then will the choice have SYMBOL_DEF_USER set on it (set in
> set_all_choice_values()).
>
> Some possible fixes:
>  (1) Add another sym_clear_all_valid() call at the end of
> conf_set_all_new_symbols(). This is kinda wasteful.
>  (2) Just randomize_choice_values() seems to actually need the
> sym_calc_value(csym). It could probably be moved into that function,
> perhaps doing the additional sym_clear_all_valid() there unless
> there's something neater you could do.
>
> It would probably be safer if someone more familiar with the
> randomization stuff wrote the actual patch.
>
> Linux version is 3.7.0-rc4, though this bug has likely been around a while.
>
> /Ulf

This bug causes an incorrect configuration to be generated for
Blackfin with the following defconfigs by the way:

CM-BF537U_defconfig
BF548-EZKIT_defconfig
BF527-EZKIT_defconfig
BF527-EZKIT-V2_defconfig
TCM-BF537_defconfig

For example:

$ make ARCH=blackfin CM-BF537U_defconfig
$ cp .config ._config
$ cp arch/blackfin/configs/CM-BF537U_defconfig .config
$ make ARCH=blackfin menuconfig
*save and exit*
$ diff -u .config ._config
--- .config    2012-12-08 08:46:07.598588422 +0100
+++ ._config    2012-12-08 08:44:03.566584623 +0100
@@ -997,9 +997,9 @@
 # CONFIG_USB_MV_UDC is not set
 # CONFIG_USB_M66592 is not set
 # CONFIG_USB_NET2272 is not set
-CONFIG_USB_LIBCOMPOSITE=y
+CONFIG_USB_LIBCOMPOSITE=m
 # CONFIG_USB_ZERO is not set
-CONFIG_USB_ETH=y
+CONFIG_USB_ETH=m
 CONFIG_USB_ETH_RNDIS=y
 # CONFIG_USB_ETH_EEM is not set
 # CONFIG_USB_G_NCM is not set

The defconfig itself contains "CONFIG_USB_ETH=y", which 'conf'
incorrectly truncates.

I noticed the bug because it causes the only test suite failures in
Kconfiglib (https://github.com/ulfalizer/Kconfiglib) with Linus'
latest kernel, in the tests that compare output between Kconfiglib and
the scripts/kconfig/*conf utilities. (I'm in the process of updating
Kconfiglib. I have no intention to get it into the kernel - just a
handy tool.)

/Ulf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/