Re: [PATCH] Use a zero-size array in the struct devres

[Catalin Marinas]

Tejun,

Tejun Heo <htejun@xxxxxxxxx> wrote:
> Catalin Marinas wrote:
>> Commit 9ac7849e35f705830f7b016ff272b0ff1f7ff759 adds the devres
>> structure containing a flexible unsigned long long array, with a
>> comment about the guaranteed alignment. According to the gcc manual,
>> flexible arrays are considered incomplete types and it is an error to
>> ask for their alignment. This patch makes data[] a zero-size array.
>
> The gcc manual says nothing about alignment.  What it says is sizeof()
> doesn't work.  If flexible array doesn't honor the alignment of the
> declared type, it would be useless for its designed purpose of
> supporting *array* of dynamically-determined length.

Chapter 5.31 (http://gcc.gnu.org/onlinedocs/gcc/Alignment.html) states
that "it is an error to ask for the alignment of an incomplete type"
and flexible array members have incomplete type (according to ISO C99
as described in http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html).

It sounds to me like the alignment of an incomplete type is not
guaranteed (as you can't even enquire about it, though I might be
wrong). This is probably dependent on the compiler as well - with
gcc-3.3 on x86, __alignof__(unsigned long long) is 8 but the
"offsetof(struct test, data)" below is 12 (and it doesn't make any
difference whether it is a 0-size array or not):

struct test {
    unsigned long a;
    unsigned long b;
    unsigned long c;
    unsigned long long data[];
};

>> I came across this when trying to build kmemleak as the modified
>> container_of macro tries to get the size of the devres.data member and
>> sizeof cannot be applied to incomplete types.
>
> If kmemleak's container_of cannot deal with the current struct devres,
> it means it can't deal with flexible arrays at all.  Flexible array
> being the standard as opposed to 0 size array gcc extension, I guess a
> lot of people would prefer flexible array.

I'm OK with flexible arrays (kmemleak uses them as well) but not as a
member of structure getting passed to the container_of macro.

I did a quick grep through the kernel and it looks like Linux mainly
uses 0-size rather than flexible arrays at the end of a structure
(>500 vs ~14). This might be for historical reasons but it's not a big
issue in modifying them.

> It would be best if kmemleak's container_of can be modified to work with
> flexible arrays.  If that's not possible, I guess it needs wider
> discussion to determine whether to ditch flexible array for
> kmemleak.

Kmemleak needs the sizeof the member passed to container_of and I'm
not aware of any __builtin_* predicate that would detect incomplete
types.

-- 
Catalin
-
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/