[vlc-devel] [PATCH] objects: ensure accessing payload parents via container_of is correct

[Rémi Denis-Courmont]

Le 26 juillet 2017 11:04:54 GMT+03:00, Steve Lhomme <robux4 at gmail.com> a écrit :
>As a side note, the standard has this example:
>***
>struct s { int n; double d[]; };
>
>struct s t1 = { 0 };          // valid
>struct s t2 = { 1, { 4.2 }};  // invalid
>t1.n = 4;                     // valid
>t1.d[0] = 4.2;                // might be undefined behavior
>
>The initialization of t2 is invalid (and violates a constraint)
>because struct s is treated as if it did not contain member d. The
>assignment to t1.d[0] is probably undefined behavior, but it is
>possible that
>
>sizeof (struct s) >= offsetof(struct s, d) + sizeof (double)
>
>in which case the assignment would be legitimate. Nevertheless, it
>cannot appear in strictly conforming code.
>***
>
>So it seems possible, according to the standard, that sizeof(struct s)
>and offsetof(struct s, d) do not have the same value.
>
>On Wed, Jul 26, 2017 at 9:39 AM, Steve Lhomme <robux4 at videolabs.io>
>wrote:
>> In those cases the parent of a payload is accessed via the
>container_of() macro
>> that removes the size of the containing structure of the payload
>pointer. This
>> size comes from offsetof() the flexible array element at the end of
>the parent
>> structure.
>> The compiler is supposed to treat the flexible array element has
>having no size
>> in the structure (except when accessed).
>> ---
>>  src/misc/objects.c | 3 +++
>>  src/misc/objres.c  | 2 ++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/src/misc/objects.c b/src/misc/objects.c
>> index 91eebdaf20..e953e38a64 100644
>> --- a/src/misc/objects.c
>> +++ b/src/misc/objects.c
>> @@ -186,6 +186,9 @@ void *vlc_custom_create (vlc_object_t *parent,
>size_t length,
>>       * and zeroes the rest.
>>       */
>>      assert (length >= sizeof (vlc_object_t));
>> +    static_assert( sizeof(vlc_object_internals_t) ==
>> +                   offsetof(vlc_object_internals_t, aligned_end),
>> +                  "flexible array size is not ignored" );
>>
>>      vlc_object_internals_t *priv = malloc (sizeof (*priv) + length);
>>      if (unlikely(priv == NULL))
>> diff --git a/src/misc/objres.c b/src/misc/objres.c
>> index 1afaccb700..b9aa87f30d 100644
>> --- a/src/misc/objres.c
>> +++ b/src/misc/objres.c
>> @@ -51,6 +51,8 @@ void *vlc_objres_new(size_t size, void
>(*release)(void *))
>>          errno = ENOMEM;
>>          return NULL;
>>      }
>> +    static_assert( sizeof(struct vlc_res) == offsetof(struct
>vlc_res, payload),
>> +                   "flexible array size is not ignored" );
>>
>>      struct vlc_res *res = malloc(sizeof (*res) + size);
>>      if (unlikely(res == NULL))
>> --
>> 2.12.1
>>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

It is possible if the alignment of double is less than the alignment of a struct. For instance, you could have all structures aligned to 16 bytes by ABI while double is aligned to 8 bytes (or less).

It is not clear to me that this is legal for max_align_t though. It would mean that the size of a structure is padded to a larger alignment than the actual alignment of the structure... Which sound somewhat contradictory.

And if this is legal, then vlc_externals and vlc_internals macros are broken, and the aligned_end addition needs reverting - as are probably a few other things.
-- 
Rémi Denis-Courmont
Typed on an inconvenient virtual keyboard
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170726/f4608230/attachment.html>