[vlc-devel] [PATCH 0/3] Respect C++ ODR (first patches)

[Rémi Denis-Courmont]

Le 23 avril 2018 00:23:40 GMT+03:00, Romain Vimont <rom1v at videolabs.io> a écrit :
>On Sun, Apr 22, 2018 at 07:28:48PM +0300, Rémi Denis-Courmont wrote:
>> Le dimanche 22 avril 2018, 18:03:35 EEST Romain Vimont a écrit :
>> > In C++, there cannot be more than one definition of a single type
>in the
>> > entire program:
>> >
><http://en.cppreference.com/w/cpp/language/definition#One_Definition_Rule>
>> > 
>> > This has been reported by #17078 and #18033.
>> > 
>> > To fix the issue:
>> >  1. In C++ code, declare *_sys_t types in a separate namespace,
>either
>> >     unnamed if it's used only in one translation unit, or named
>otherwise.
>> >  2. Remove the *_sys_t typedefs in include/vlc_common.h.
>> >  3. Declare *_sys_t typedefs locally for C modules.
>> >  4. Declare references to private data as "void *" instead of
>"*_sys_t *".
>> >  5. Adapt code that now dereferences "void *" or relied on the
>*_sys_t
>> >     forward declarations.
>> 
>> This might fix the compilation, but as long as the same typename is
>defined 
>> multiple times, the debugging will remain broken.
>
>At least, the program behavior is not undefined due to ODR anymore.
>
>It seems to me that it also solves debugging "confusion", at least as I
>experienced it.
>
>For example, on commit 5a20d22~ (parent of 5a20d22), in
>modules/demux/mkv/mkv.cpp:Demux, it is clear that gdb decodes p_sys as
>a
>demux_sys_t structure from modules/demux/sid.cpp:
>
>    (gdb) p p_sys->i_pcr
>    There is no member named i_pcr.
>    (gdb) p *p_sys
>$1 = {player = 0x7fff9240f700, config = {clockDefault = (unknown:
>2160103168), clockForced = 255,
>       ...
>
>On the contrary, after applying the ODR patch for demux_sys_t ([PATCH
>1/3]), gdb correctly decodes it against the right demux_sys_t from
>modules/demux/mkv/demux.hpp:
>
>    (gdb) p p_sys->i_pcr
>    $1 = 0
>    (gdb) p *p_sys
>$2 = {_vptr.demux_sys_t = 0x7fff9240f700, demuxer = @0x7fff80c14b10,
>b_seekable = true, b_fastseekable = true, i_pts = 1, i_pcr = 0,
>       ...
>
>> Also, as far as aliasing problems are concerned, there is another
>potential 
>> separate problem. I am not sure if treating objects alternatively as 
>> vlc_object_t and as ${MODULE_TYPE}_t is valid by C++ either. In fact,
>I am not 
>> even sure if it is valid in C.
>
>I agree there might be aliasing problems. Maybe we should compile with
>-fno-strict-aliasing for now, unless we are convinced that there are no
>strict aliasing violations or that they have been fixed, what do you
>think?
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

No ways we disable strict aliasing. This will not only diable optimizations that have been working for ten years, but it will also disable aliasing warnings, AFAIK (since the necessary compiler analysis will not be done).

We just need to keep LTO disabled and, at least for C++, dynamic linking enabled for plugins.
-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.