[FFmpeg-devel] [PATCH] lavfi: get rid of FF_INTERNAL_FIELDS

[Andreas Rheinhardt]

Nicolas George:
> Andreas Rheinhardt (12023-02-01):
>> PS: Upon rethinking, it is not only b) that contains undefined
>> behaviour; it is b)-d) as well as the current code. The reason is that
>> the type of AVFilterLink as seen in the files with FF_INTERNAL_FIELDS is
>> not compatible with the type of AVFilterLink from the files without
>> FF_INTERNAL_FIELDS. This also makes derived types, like the types of
>> function declarations containing pointers to AVFilterLink incompatible
>> and this is a violation of 6.2.7(2) of C11. I wonder if this will become
>> a real problem with lto someday.
> 
> No: read the second half of the previous paragraph: two structures with
> common first fields are compatible types. What we have been using is a
> deliberately supported construct.
> 

"Moreover, two structure, union, or enumerated types declared in
separate translation units are compatible if their tags and members
satisfy the following requirements: If one is declared with a tag, the
other shall be declared with the same tag. If both are completed
anywhere within their respective translation units, then the following
additional requirements apply: there shall be a one-to-one
correspondence between their members such that each pair of
corresponding members are declared with compatible types; if one member
of the pair is declared with an alignment specifier, the other is
declared with an equivalent alignment specifier; and if one member of
the pair is declared with a name, the other is declared with the same
name. For two structures, corresponding members shall be declared in the
same order."

1. There is no one-to-one correspondence (aka a bijection) between the
elements of the complete and of the public structure.
2. In the case of AVFilterLink, there is not even an injection from the
public to the FF_INTERNAL_FIELDS structure (due to the former having a
big reserved array).
3. The fact that these structures share a common initial sequence does
not mean that this is legal. There are some guarantees regarding common
initial sequences in 6.5.2.3 (6):

"One special guarantee is made in order to simplify the use of unions:
if a union contains several structures that share a common initial
sequence (see below), and if the union object currently contains one of
these structures, it is permitted to inspect the common initial part of
any of them anywhere that a declaration of the completed type of the
union is visible. Two structures share a common initial sequence if
corresponding members have compatible types (and, for bit-fields, the
same widths) for a sequence of one or more initial members."

But there just is no union between the FF_INTERNAL_FIELDS
!defined(FF_INTERNAL_FIELDS) structures in the whole codebase.
Furthermore, said guarantee is only for inspecting, i.e. reading. For
example, for the following two structs sharing a common initial sequence,

struct Small {
    uint64_t foo;
    uint32_t bar;
};
struct Big {
    uint64_t foo;
    uint32_t bar;
    uint32_t baz;
};

if one had a union { struct Small; struct Big; }, a write to Small.bar
may clobber Big.baz.

- Andreas