netdev - Re: [PATCH bpf-next 1/2] btf: resolve enum fwds in btf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Mar 2019 10:39:37 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Edward Cree <ecree@...arflare.com>
Cc:     Yonghong Song <yhs@...com>, Andrii Nakryiko <andriin@...com>,
        Kernel Team <Kernel-team@...com>,
        Alexei Starovoitov <ast@...com>,
        "acme@...nel.org" <acme@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>
Subject: Re: [PATCH bpf-next 1/2] btf: resolve enum fwds in btf_dedup

On Mon, Mar 11, 2019 at 10:27 AM Edward Cree <ecree@...arflare.com> wrote:
>
> On 11/03/19 16:34, Andrii Nakryiko wrote:
> > The only thing we should consider is that enums can have different
> > sizes. And enum size is part of enum's forward declaration. So unlike
> > struct/union fwd, enum's fwd has extra info. I don't think it's
> > possible to specify in C (enum is always 4 bytes),
> Not always, see e.g. N1458 §6.7.2.2.4:

Yeah, you are right, sorry. I was mostly trying to point out that C
itself doesn't allow programmer to specify size, while C++ does.


> > Each enumerated type shall be compatible with char, a signed integer
> > type, or an unsigned integer type. The choice of type is implementation-
> > defined,¹²⁸⁾ but shall be capable of representing the values of all the
> > members of the enumeration. [...]
> The enumeration _constants_ have type int (§6.7.2.2.3), but the enum type
> itself has implementation-defined size, per the above.
> C doesn't allow to forward-declare enumerators (§6.7.2.3.3), nor even to

There is a GCC extension that allows to forward-declare enums:
https://gcc.gnu.org/onlinedocs/gcc/Incomplete-Enums.html#Incomplete-Enums

Kernel actually has examples of using this (see patch description
regrading irqchip_irq_state).

> reference them from the enumerator-list in the declaration, since the
> enumerator type is incomplete until the closing brace (§6.7.2.2.4).
> Footnote 128 adds that "An implementation may delay the choice of which
> integer type until all enumeration constants have been seen."
> It appears, in any case, that no forward-declaration could be required in
> BTF, since an enum type's BTF record does not reference other types.  With
> something like
>     enum foo {
>         BAR = sizeof(enum foo *),
>     };
> which is not valid C thanks to §6.7.2.3.3 (but many compilers will accept
> it, e.g. gcc without -pedantic), the BTF record would read
>     kind=enum name_off=&"foo" vlen=1
>         name_off=&"BAR" val=8
> (assuming 64-bit) which does not require any forward-declaring.
>
> I don't know about the C++ situation, though.

C++ allows to forward-declare enums only if as part of declaration you
also specify underlying integer type.

>
> -Ed
>
> PS: It's not really clear to me what the rationale for §6.7.2.3.3 is,
> since all the problems with incomplete enum types also arise with other
> incomplete types; why 'enum foo *' can't be treated like 'struct quux *'

Seems like GCC's extension allows exactly this.

> I don't know.  But this isn't comp.std.c...