[cfe-dev] support __attribute__((weak)) for non-primitive types

[James Y Knight via cfe-dev]

In my opinion, this sounds like a hack to work around a problem that might
be better solved outside the compiler. Or, at the very least, I think some
more justification as to why this is the best solution would be helpful.

Restating the problem:
You have a tool which generates a "vmlinux.h" header file based on the
currently-running kernel (e.g. by parsing /sys/kernel/btf/vmlinux). This
allows users to avoid needing a copy of the kernel header files, which may
not be readily available. However, the program cannot extract #defines --
only types -- because defines aren't recorded in the kernel BTF debug info.
So, despite having an easy way to generate vmlinux.h, users do still want
to include the actual kernel headers sometimes, in order to access these
constants. You want to allow that.

Your proposed solution is:
Modify the compiler so that it can ignore the duplicate struct
definitions, in order to allow users to include both the kernel headers and
the generated vmlinux.h header, even though they don't cooperate with
each-other.

But -- is that really necessary? It seems a bit odd to invent a compiler
feature to work around non-cooperating headers. Even more so when all the
headers are part of the same project.

Here's some ideas of possible alternatives. You may well have already
thought of these and rejected them, but if so, it would be nice to hear why.

1) Modify the original headers to have appropriate #ifndefs, in order to
avoid conflicting.

2) Tell users that they can either include either the autogenerated
"vmlinux.h", OR set the pragma and include normal kernel headers. But not
both. AFAICT, the vmlinux.h file is not really version-independent --
except that it uses the "preserve_access_index" pragma in order to allow
field accesses to be adjusted at load-time to match the actual struct
layout. Doesn't putting the same pragma around the normal kernel headers
work, too?

3) Automatically extract #defines from kernel headers (e.g. using -dD to
print all macro-expansions) to create a new header with those values. (Or
defines and inline functions. Or everything but struct definitions. Or
something along those lines). Have people include that, instead.

...Although, IIUC, you want use the kernel-internal headers e.g.
"include/linux/socket.h", not the stable "uapi" headers, e.g.
"include/uapi/linux/socket.h". But can't those defines change arbitrarily?
Doesn't that ruin the version-independence aspect? So, maybe you want:

4) Enhance BPF/CO:RE to support defines as
compile-time-unknown/runtime-constants [at least some useful subset of
them]. Then they too can be fixed up at BPF load-time, instead of hoping
that the values never change.

Though, the example you give is "TCP" (IPPROTO_TCP?) which *is* in the
uapi. Maybe you do need only the stable uapi defines? In which case,

5) Create a set of modified uapi headers for use with bpf, which omits
struct definitions, and instead has #include vmlinux.h at the top instead.
Distribute with libbpf-dev, perhaps?



On Mon, Aug 23, 2021 at 7:17 PM Y Song via cfe-dev <cfe-dev at lists.llvm.org>
wrote:

> Hi,
>
> This is a BPF use case. We would like to check whether
> __attribute__((weak)) support for non-primitive types would be
> possible in clang or not. For example, if we have two types like
>    struct t { int a; } __attribute__((weak));
>    struct t { int a; };
>
>    typedef unsigned __u32;
>    typedef unsigned __u32 __attribute__((weak));
> the compilation should not fail. It should just ignore weak definitions.
>
> In BPF, to support CO-RE (compile once and run everywhere), we
> generate a *generic* vmlinux.h
> (
> https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html
> )
> which is included in the bpf program. The bpf loader (libbpf) will
> adjust field offsets properly based on host record layout so the same
> program can run on different kernels which may have different record
> layouts.
>
> But vmlinux.h does not support macros (and simple static inline helper
> functions in kernel header files). Currently, users need to define
> these macros and static inline functions explicitly in their bpf
> program. What we are thinking is to permit users to include kernel
> header files containing these macros/static inline functions. But this
> may introduce duplicated types as these types have been defined in
> vmlinux.h.
>
> For example, we may have:
>     socket.h:
>         struct socket { ... };
>         #define TCP    6
>     vmlinux.h:
>         struct socket { ... };
>     bpf.c:
>         #include <socket.h>
>         #include <vmlinux.h>
>         ... TCP ...
>
> In this case, struct "socket" is defined twice. If we can have something
> like
>     bpf.c:
>       #pragma clang attribute push (__attribute__((weak)), apply_to =
> record)
>       #include <socket.h>
>       #pragma clang attribute pop
>       #include <vmlinux.h>
>         ... TCP ...
>
>
> Then bpf program can get header file macro definitions without copying
> explicitly.
>
> We need support for "apply_to = typedef" in the above pragma, so we
> can deal with typedef types as well.
>
> Another issue is related to what if two types are not compatible. For
> example,
>     struct t { int a; } __attribute__((weak));
>     struct t { int a; int b; };
> I think we could have a flag to control whether we allow incompatible
> weak type or not.
>
> It would be good if I can get some suggestions/directions on whether
> such a feature is possible or not for clang frontend.
>
> Thanks,
>
> Yonghong
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20210830/a8b38a84/attachment.html>