From: Michal Marek <mmarek@suse.cz>
To: Martin Walch <walch.martin@web.de>
Cc: linux-kbuild@vger.kernel.org
Subject: Re: [PATCH v4] Kconfig: Remove bad inference rules expr_eliminate_dups2()
Date: Mon, 25 May 2015 16:05:16 +0800 [thread overview]
Message-ID: <5562D7BC.3050604@suse.cz> (raw)
In-Reply-To: <47745287.pcmKhRVW0u@tacticalops>
Dne 22.5.2015 v 19:41 Martin Walch napsal(a):
> From: Martin Walch <walch.martin@web.de>
>
> expr_eliminate_dups2() in scripts/kconfig/expr.c applies two invalid
> inference rules:
>
> (FOO || BAR) && (!FOO && !BAR) -> n
> (FOO && BAR) || (!FOO || !BAR) -> y
>
> They would be correct in propositional logic, but this is a three-valued
> logic, and here it is wrong in that it changes semantics. It becomes
> immediately visible when assigning the value 1 to both, FOO and BAR:
>
> (FOO || BAR) && (!FOO && !BAR)
> -> min(max(1, 1), min(2-1, 2-1)) = min(1, 1) = 1
>
> while n evaluates to 0 and
>
> (FOO && BAR) || (!FOO || !BAR)
> -> max(min(1, 1), max(2-1, 2-1)) = max(1, 1) = 1
>
> with y evaluating to 2.
>
> Fix it by removing expr_eliminate_dups2() and the functions that have no
> use anywhere else: expr_extract_eq_and(), expr_extract_eq_or(),
> and expr_extract_eq() from scripts/kconfig/expr.c
>
> Currently the bug is not triggered in mainline, so this patch does not
> modify the configuration space there. To observe the bug consider this
> example:
>
> config MODULES
> def_bool y
> option modules
>
> config FOO
> def_tristate m
>
> config BAR
> def_tristate m
>
> config TEST1
> def_tristate y
> depends on (FOO || BAR) && (!FOO && !BAR)
>
> if TEST1 = n
> comment "TEST1 broken"
> endif
>
> config TEST2
> def_tristate y
> depends on (FOO && BAR) || (!FOO || !BAR)
>
> if TEST2 = y
> comment "TEST2 broken"
> endif
>
> config TEST3
> def_tristate y
> depends on m && !m
>
> if TEST3 = n
> comment "TEST3 broken"
> endif
>
> TEST1, TEST2 and TEST3 should all evaluate to m, but without the patch,
> none of them does. It is probably not obvious that TEST3 is the same bug,
> but it becomes clear when considering what happens internally to the
> expression
> m && !m":
> First it expands to
> (m && MODULES) && !(m && MODULES),
> then it is transformed into
> (m && MODULES) && (!m || !MODULES),
> and finally due to the bug it is replaced with n.
>
> As a side effect, this patch reduces code size in expr.c by roughly 10%
> and slightly improves startup time for all configuration frontends.
>
> Signed-off-by: Martin Walch <walch.martin@web.de>
> ---
> Version 3:
> Earlier versions fixed some comments, too. Remove those bits and focus
> on the logical problem.
>
> Version 4:
> Add test case to commit description.
Thanks, applied now!
Michal
prev parent reply other threads:[~2015-05-25 8:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-17 2:14 [PATCH v3] Kconfig: Remove bad inference rules expr_eliminate_dups2() Martin Walch
2015-05-18 7:36 ` Michal Marek
2015-05-18 9:38 ` Martin Walch
2015-05-21 6:39 ` Michal Marek
2015-05-22 11:41 ` [PATCH v4] " Martin Walch
2015-05-25 8:05 ` Michal Marek [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5562D7BC.3050604@suse.cz \
--to=mmarek@suse.cz \
--cc=linux-kbuild@vger.kernel.org \
--cc=walch.martin@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.