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

[Rémi Denis-Courmont]

Le 26 juillet 2017 11:43:09 GMT+03:00, Steve Lhomme <robux4 at gmail.com> a écrit :
>On Wed, Jul 26, 2017 at 10:35 AM, Rémi Denis-Courmont <remi at remlab.net>
>wrote:
>> Le 26 juillet 2017 10:39:01 GMT+03:00, Steve Lhomme
><robux4 at videolabs.io> a
>> écrit :
>>>
>>> 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))
>>
>>
>> Nack. This assertion is always true. If you want to check that nobody
>breaks
>
>It's not always true. I do have the assertion fail in some cases. And
>as said in my follow-up email. There is no strict guarantee in the
>standard that this is true.
>
>> aligned_end the sensible assertion is alignof(internals) >=
>> alignof(max_align_t). But it would be incredibly stupid and reckless
>for
>> somebody to make aligned_end not the end of the structure, so that
>seems
>> pointless too.
>> --
>> Rémi Denis-Courmont
>> Typed on an inconvenient virtual keyboard
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

Well, if it ain't always true, then it can't be relied upon, so the assertion does not make sense either way.

4f91e6bc15d8b72deb1f761ca857a33d672c91ef should be reverted instead in that case.
-- 
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/6418775f/attachment.html>