Re: sparse: ARRAY_SIZE and sparse array initialization

[Hans Verkuil <hverkuil@xs4all.nl>]

Hi Chris,

On 03/30/2014 08:10 AM, Christopher Li wrote:
> On Fri, Mar 28, 2014 at 3:50 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> Is there any chance that the three issues I reported will be fixed? If not,
>> then I'll work around it in the kernel code.
>>
> 
> Most likely it is a sparse issue. Can you generate a minimal stand alone
> test case that expose this bug? I try to simplify it as following, but
> it does not
> reproduce the error.
> 
> 
> #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> 
> static const char *v4l2_type_names[] = {
>         [1]    = "mmap",
>         [9] = "userptr",
>         [2] = "overlay",
>         [3] = "dmabuf",
> };
> 
> #define prt_names(a, arr) (((unsigned)(a)) < ARRAY_SIZE(arr) ? arr[a]
> : "unknown")
> 
> 
> extern void prt(const char *name);
> 
> static void v4l_print_requestbuffers(const void *arg, int write_only)
> {
>         const struct v4l2_requestbuffers *p = arg;
> 
>         prt(prt_names(3, v4l2_type_names));
> }
> 
> 
> If you have the test case, you are welcome to submit a patch to
> add the test case.

I experimented a bit more and it turned out that the use of EXPORT_SYMBOL
for the array is what causes the problem. Reproducible with this:

#include <linux/kernel.h>

#define prt_names(a, arr) (((unsigned)(a)) < ARRAY_SIZE(arr) ? arr[a] : "unknown")

extern const char *v4l2_names[];

const char *v4l2_names[] = {
        [1]    = "mmap",
        [9] = "userptr",
        [2] = "overlay",
        [3] = "dmabuf",
};
EXPORT_SYMBOL(v4l2_names);

extern void prt(const char *name);

static void v4l_print_requestbuffers(const void *arg, int write_only)
{
        prt(prt_names(3, v4l2_names));
}


And with this sparse command:

sparse  -nostdinc -isystem /usr/lib/gcc/x86_64-linux-gnu/4.8/include -Iarch/x86/include -Iarch/x86/include/generated  -Iinclude -Iarch/x86/include/uapi -Iarch/x86/include/generated/uapi -Iinclude/uapi -Iinclude/generated/uapi -include include/linux/kconfig.h -D__KERNEL__ x.c

If I remove EXPORT_SYMBOL all is well. EXPORT_SYMBOL expands to this:

extern typeof(v4l2_names) v4l2_names;
extern __attribute__((externally_visible)) void *__crc_v4l2_names __attribute__((weak));
static const unsigned long __kcrctab_v4l2_names __attribute__((__used__)) __attribute__((section("___kcrctab" "" "+" "v4l2_names"), unused)) = (unsigned long) &__crc_v4l2_names;
static const char __kstrtab_v4l2_names[] __attribute__((section("__ksymtab_strings"), aligned(1))) = "v4l2_names";
extern const struct kernel_symbol __ksymtab_v4l2_names;
__attribute__((externally_visible)) const struct kernel_symbol __ksymtab_v4l2_names __attribute__((__used__)) __attribute__((section("___ksymtab" "" "+" "v4l2_names"), unused)) = { (unsigned long)&v4l2_names, __kstrtab_v4l2_names };

I did some more research and the key is the first line: 

extern typeof(v4l2_names) v4l2_names;

If I add that to your test case:

static const char *v4l2_type_names[] = {
        [1]    = "mmap",
        [9] = "userptr",
        [2] = "overlay",
        [3] = "dmabuf",
};
extern typeof(v4l2_type_names) v4l2_type_names;

Then I get the same error with your test case.

The smallest test case I can make is this:

====== extern-array.c ======
extern const char *v4l2_type_names[];
const char *v4l2_type_names[] = {
        "test"
};
extern const char *v4l2_type_names[];

static void test(void)
{
	unsigned sz = sizeof(v4l2_type_names);
}
/*
 * check-name: duplicate extern array
 *
 * check-error-start
 * check-error-end
 */
====== extern-array.c ======

If I leave out the both 'extern' declarations I get:

warning: symbol 'v4l2_type_names' was not declared. Should it be static?

Which is a correct warning.

If I leave out the second 'extern' only, then all is fine. If I add both
'extern' declarations, then I get:

error: cannot size expression

which is clearly a sparse bug somewhere.

Regards,

	Hans