randconfig broken on choice

randconfig broken on choice

[Matthieu CASTET]

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

Hi,

I am using v3.1-rc4 and make randconfig look broken for choice.
The choice config is always the same or choice entry can have more than one entry.


After some debugging, it seems in randomize_choice_values, we don't clean
SYMBOL_VALID for choice entry.
And we don't take "sym->def[S_DEF_USER].tri" but the "default sym->cur.tri".

The following patch seems to fix the problem.


Matthieu

[-- Attachment #2: randconfig.diff --]
[-- Type: text/x-diff, Size: 492 bytes --]

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 59b667c..08331f8 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -1045,6 +1045,10 @@ static void randomize_choice_values(struct symbol *csym)
 		else {
 			sym->def[S_DEF_USER].tri = no;
 		}
+		sym->flags |= SYMBOL_DEF_USER;
+		/* clear VALID to get value calculated */
+		sym->flags &= ~(SYMBOL_VALID);
+
 	}
 	csym->flags |= SYMBOL_DEF_USER;
 	/* clear VALID to get value calculated */

Re: randconfig broken on choice

[Arnaud Lacombe]

Hi,

On Tue, Sep 6, 2011 at 4:52 AM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> Hi,
>
> I am using v3.1-rc4 and make randconfig look broken for choice.
> The choice config is always the same or choice entry can have more than one entry.
>
Could you includes testcases showing that behavior, please ?

Btw, try also to include patch inlined, not multipart. That makes
review easier. I know, gmail is not making that easy, but git
send-email works just fine.

Thanks,
 - Arnaud

>
> After some debugging, it seems in randomize_choice_values, we don't clean
> SYMBOL_VALID for choice entry.
> And we don't take "sym->def[S_DEF_USER].tri" but the "default sym->cur.tri".
>
> The following patch seems to fix the problem.
>
>
> Matthieu
>

Re: randconfig broken on choice

[Matthieu CASTET]

Hi,

Arnaud Lacombe a écrit :
> Hi,
> 
> On Tue, Sep 6, 2011 at 4:52 AM, Matthieu CASTET
> <matthieu.castet@parrot.com> wrote:
>> Hi,
>>
>> I am using v3.1-rc4 and make randconfig look broken for choice.
>> The choice config is always the same or choice entry can have more than one entry.
>>
> Could you includes testcases showing that behavior, please ?
$ make randconfig

$ grep -e SLUB -e SLAB -e SLOB .config
# CONFIG_SLAB is not set
CONFIG_SLUB=y
CONFIG_SLOB=y
# CONFIG_FAILSLAB is not set

We have 2 choice selected (SLUB and SLOB). You may run randconfig (more than one
time to get this result).

Matthieu

Re: randconfig broken on choice

[Arnaud Lacombe]

Hi,

On Tue, Sep 6, 2011 at 11:13 AM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> Hi,
>
> Arnaud Lacombe a écrit :
>> Hi,
>>
>> On Tue, Sep 6, 2011 at 4:52 AM, Matthieu CASTET
>> <matthieu.castet@parrot.com> wrote:
>>> Hi,
>>>
>>> I am using v3.1-rc4 and make randconfig look broken for choice.
>>> The choice config is always the same or choice entry can have more than one entry.
>>>
>> Could you includes testcases showing that behavior, please ?
> $ make randconfig
>
> $ grep -e SLUB -e SLAB -e SLOB .config
> # CONFIG_SLAB is not set
> CONFIG_SLUB=y
> CONFIG_SLOB=y
> # CONFIG_FAILSLAB is not set
>
> We have 2 choice selected (SLUB and SLOB). You may run randconfig (more than one
> time to get this result).
>
ok, I reproduced it with:

while true; do
    make randconfig
    CONFIG_SLAB=; CONFIG_SLOB=; CONFIG_SLUB=
    source .config
    if [ "${CONFIG_SLAB}${CONFIG_SLOB}${CONFIG_SLUB}" != "y" ]; then
        break
    fi
    sleep 1
done

I wonder what set the flags originally...

 - Arnaud

> Matthieu
>

Re: randconfig broken on choice

[Arnaud Lacombe]

Hi,

On Tue, Sep 6, 2011 at 11:32 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Tue, Sep 6, 2011 at 11:13 AM, Matthieu CASTET
> <matthieu.castet@parrot.com> wrote:
>> Hi,
>>
>> Arnaud Lacombe a écrit :
>>> Hi,
>>>
>>> On Tue, Sep 6, 2011 at 4:52 AM, Matthieu CASTET
>>> <matthieu.castet@parrot.com> wrote:
>>>> Hi,
>>>>
>>>> I am using v3.1-rc4 and make randconfig look broken for choice.
>>>> The choice config is always the same or choice entry can have more than one entry.
>>>>
>>> Could you includes testcases showing that behavior, please ?
>> $ make randconfig
>>
>> $ grep -e SLUB -e SLAB -e SLOB .config
>> # CONFIG_SLAB is not set
>> CONFIG_SLUB=y
>> CONFIG_SLOB=y
>> # CONFIG_FAILSLAB is not set
>>
>> We have 2 choice selected (SLUB and SLOB). You may run randconfig (more than one
>> time to get this result).
>>
> ok, I reproduced it with:
>
> while true; do
>    make randconfig
>    CONFIG_SLAB=; CONFIG_SLOB=; CONFIG_SLUB=
>    source .config
>    if [ "${CONFIG_SLAB}${CONFIG_SLOB}${CONFIG_SLUB}" != "y" ]; then
>        break
>    fi
>    sleep 1
> done
>
> I wonder what set the flags originally...
>
This is interesting; I'm always getting the following:

CONFIG_SLUB=y
CONFIG_SLOB=y

never CONFIG_SLAB and another one. Actually, the default of the choice
ends up being selected along with the random one. Is this what you
mean by "we don't take "sym->def[S_DEF_USER].tri" but the "default
sym->cur.tri"" ? However, I do not see this behavior with the
following reduced testcase:

config EXPERT
        bool
        default y

choice
        prompt "Choose SLAB allocator"
        default SLUB

config SLAB
        bool "SLAB"

config SLUB
        bool "SLUB (Unqueued Allocator)"

config SLOB
        depends on EXPERT
        bool "SLOB (Simple Allocator)"

endchoice

Btw, you say that "we don't clean SYMBOL_VALID for choice entry", but
AFAIU the code, we explicitly set the not-randomly-chosen choice value
to 'no', so it's value should be valid.

 - Arnaud

Re: randconfig broken on choice

[Matthieu CASTET]

Hi Arnaud,

Arnaud Lacombe a écrit :
> Hi,
> 
> On Tue, Sep 6, 2011 at 11:32 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> Hi,
>>
>> On Tue, Sep 6, 2011 at 11:13 AM, Matthieu CASTET
>> <matthieu.castet@parrot.com> wrote:
>>> Hi,
>>>
>>> Arnaud Lacombe a écrit :
>>>> Hi,
>>>>
>> ok, I reproduced it with:
>>
>> while true; do
>>    make randconfig
>>    CONFIG_SLAB=; CONFIG_SLOB=; CONFIG_SLUB=
>>    source .config
>>    if [ "${CONFIG_SLAB}${CONFIG_SLOB}${CONFIG_SLUB}" != "y" ]; then
>>        break
>>    fi
>>    sleep 1
>> done
>>
>> I wonder what set the flags originally...
>>
> This is interesting; I'm always getting the following:
> 
> CONFIG_SLUB=y
> CONFIG_SLOB=y
> 
> never CONFIG_SLAB and another one. Actually, the default of the choice
> ends up being selected along with the random one. Is this what you
> mean by "we don't take "sym->def[S_DEF_USER].tri" but the "default
> sym->cur.tri"" ?
Yes, what I mean is that we don't take the value set by randomize_choice_values,
 but some default values.
I wonder why the other one is random (I don't search who set cur value).

You can see the problem with the attached patch

and do make randconfig | grep -e "SL[UAO]B,"


 However, I do not see this behavior with the
> following reduced testcase:
> 
> 
Yes that's weird.

> Btw, you say that "we don't clean SYMBOL_VALID for choice entry", but
> AFAIU the code, we explicitly set the not-randomly-chosen choice value
> to 'no', so it's value should be valid.
> 
I don't understand what you mean.

What I saw is that randomize_choice_values modify sym->def[S_DEF_USER],
but because it is already marked valid, we never use sym->def[S_DEF_USER] in the
code writing the .config.


Matthieu


diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index b633bdb..eecb7af 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -147,6 +147,9 @@ struct property *sym_get_env_prop(struct symbol *sym);

 static inline tristate sym_get_tristate_value(struct symbol *sym)
 {
+	printf("name %s, cur=%d(%s) user=%d(%s)\n", sym->name,
+			sym->curr.tri, sym->flags & SYMBOL_VALID?"valid":"invalid",
+			sym->def[S_DEF_USER].tri, sym->flags & SYMBOL_DEF_USER?"valid":"invalid");
 	return sym->curr.tri;
 }

Re: randconfig broken on choice

[Alexander Kriegisch]

Matthieu, Arnaud,

I wrote a shell script [1] which creates thousands or random configs in 
order to find and eliminate warnings in our configuration (missing 
dependencies etc.). We use this as a way of fuzzing, i.e. to perform 
heuristic tests because it is impossible to cover all combinations.

We use kconfig 3.1-rc9 in our project. First we had the problem Matthieu 
described for rc4, because the randconfigs only covered a small subset 
of possibilities due to its buggy handling of "choice" entries.

The good news is that Mattheiu's patch [2] effectively eliminated the 
problems, and we could successfully remove all inconsistencies in our 
configuration.

So I would like to push this patch. We would be happy to see it 
integrated upstream. Thanks! :-)

[1] http://freetz.org/browser/trunk/tools/developer/create-kconfig-warnings
[2] 
http://freetz.org/browser/trunk/tools/make/patches/340-fix_randconfig_choice.kconfig.patch
-- 
Alexander Kriegisch
http://freetz.org


> I am using v3.1-rc4 and make randconfig look broken for choice.
> The choice config is always the same or choice entry can have more than one entry.
>
> After some debugging, it seems in randomize_choice_values, we don't clean
> SYMBOL_VALID for choice entry.
> And we don't take "sym->def[S_DEF_USER].tri" but the "default sym->cur.tri".
>
> The following patch seems to fix the problem.
>
> Matthieu
>
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 59b667c..08331f8 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -1045,6 +1045,10 @@ static void randomize_choice_values(struct symbol *csym)
>  		else {
>  			sym->def[S_DEF_USER].tri = no;
>  		}
> +		sym->flags |= SYMBOL_DEF_USER;
> +		/* clear VALID to get value calculated */
> +		sym->flags &= ~(SYMBOL_VALID);
> +
>  	}
>  	csym->flags |= SYMBOL_DEF_USER;
>  	/* clear VALID to get value calculated */