On Tue, May 5, 2015 at 11:13 AM, Neil Roberts <n...@linux.intel.com> wrote:
> Jason Ekstrand <ja...@jlekstrand.net> writes:
>
>> +#define list_for_each_entry(type, pos, head, member)                    \
>> +   for (type *pos = container_of((head)->next, pos, member);            \
>> +     &pos->member != (head);                                         \
>> +     pos = container_of(pos->member.next, pos, member))
>
> I think this is technically invalid. The container_of macro dereferences
> the pos pointer but at this point it is undefined so I think the
> compiler is allowed to do whatever it wants here. There is a comment
> above the container_of macro saying exactly that.

Right.  I think I've come across that too.  We also have a LIST_ELEM
(or maybe ITEM) macro in the file that does the type-based thing.
I'll fix it up to use that instead.
--Jason

> Some quick testing with clang shows that it actually takes advantage of
> this undefined behaviour and doesn't add the offset to pos to get the
> container and it all falls apart.
>
> A while ago we added the wayland linked-list implementation to Cogl and
> we ran into a similar problem. We actually ended up changing the
> container_of macro to take a type instead of a sample pointer because
> this is such an easy trap to fall into.
>
> https://git.gnome.org/browse/cogl/commit/?id=1efed1e0a2bce706eb490197
>
> Regards,
> - Neil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to