Emil Velikov <emil.l.veli...@gmail.com> writes: > On 18 April 2017 at 13:55, Pekka Paalanen <ppaala...@gmail.com> wrote: >> On Mon, 27 Feb 2017 13:26:11 +0000 >> Emil Velikov <emil.l.veli...@gmail.com> wrote: >> >>> Hi Julien, >>> >>> On 27 February 2017 at 12:08, Julien Isorce <julien.iso...@gmail.com> wrote: >>> > Hi, >>> > >>> > Since 2012 commit ccff74971203b533bf16b46b49a9e61753f75e6c it is said: >>> > "sample must be initialized, or else the result is undefined" in the >>> > description of mesa/src/util/list.h::container_of . >>> > >>> > But I can find a few places where it is used without initializing that >>> > second parameter, i.e. like: >>> > >>> > struct A a; >>> > container_of(ptr, a, member); >>> > >>> > Then I can add the "= NULL" but should it be just >>> > container_of(ptr, struct A, member); >>> > like in the kernel and some other places in mesa ? >>> > >>> Strictly peaking these are toolchain (ASAN iirc) bugs, since there is >>> no pointer deref, as we're doing pointer arithmetic. >> >> Hi Emil, >> >> that's what people would usually think. It used to work with GCC. Then >> came Clang. >> >>> Afaict the general decision was to to merge the patch(es) since they >>> will make actual bugs stand out amongst the noise. In the long run, >>> it's better to fix the tool (ASAN/other) than trying to "fix" all the >>> cases in mesa and dozens of other projects. But until then patches are >>> safe enough ;-) >>> >>> That's my take on it, at least. >> >> It depends on how container_of() has been defined. For more details, >> see e.g.: >> https://cgit.freedesktop.org/wayland/wayland/commit/?id=a18e34417ba3fefeb81d891235e8ebf394a20a74 >> >> The comment in the code in Mesa is correct for the fallback >> implementation it has. Maybe things rely on the fallback implementation >> never being used when compiling with Clang. >> >> Julien, some alternatives for container_of use a pointer, others expect >> a type instead. I believe one must use the correct form. >> > Thanks for the correction Pekka - yes, the issue (to deref or not) is > implementation specific. > A 'minor' detail that I've missed.
The issue isn't whether it derefs the pointer or not. The undefined version looks like this: #define wl_container_of(ptr, sample, member) (__typeof__(sample))((char *)(ptr) - ((char *)&(sample)->member - (char *)(sample))) and the problem is that it expects an uninitialized variable (sample) to have consistent values across two references ( (char *)&(sample)->member and (char *)sample ), but referencing an uninitialized variable even once is undefined behavior. Kristian > Seems like we use the more prone one - having the second argument > being a pointer as, opposed to a type. > Perhaps we should reconsider and update mesa, sooner rather than later. > > Thanks again! > Emil > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev