<div dir="ltr">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.<div><div><br></div><div>Restating the problem:</div><div><div>You have a tool which generates a "vmlinux.h" header file based on the currently-running kernel (e.g. by parsing <span style="font-variant-ligatures:no-common-ligatures;color:rgb(0,0,0);font-family:Menlo;font-size:11px">/sys/kernel/btf/vmlinux</span>). 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.</div></div><div><br></div><div>Your proposed solution is:</div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>1) Modify the original headers to have appropriate #ifndefs, in order to avoid conflicting.<br></div><div><br></div><div><div>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?</div><div><br></div><div>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.<br></div></div><div><br></div><div><div>...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:</div><div><br></div></div><div>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.<br></div></div><div><br></div><div>Though, the example you give is "TCP" (IPPROTO_TCP?) which <i>is</i> in the uapi. Maybe you do need only the stable uapi defines? In which case,<br></div><div><br></div><div><div>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?<br></div><div><br></div></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 23, 2021 at 7:17 PM Y Song via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
This is a BPF use case. We would like to check whether<br>
__attribute__((weak)) support for non-primitive types would be<br>
possible in clang or not. For example, if we have two types like<br>
struct t { int a; } __attribute__((weak));<br>
struct t { int a; };<br>
<br>
typedef unsigned __u32;<br>
typedef unsigned __u32 __attribute__((weak));<br>
the compilation should not fail. It should just ignore weak definitions.<br>
<br>
In BPF, to support CO-RE (compile once and run everywhere), we<br>
generate a *generic* vmlinux.h<br>
(<a href="https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html" rel="noreferrer" target="_blank">https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html</a>)<br>
which is included in the bpf program. The bpf loader (libbpf) will<br>
adjust field offsets properly based on host record layout so the same<br>
program can run on different kernels which may have different record<br>
layouts.<br>
<br>
But vmlinux.h does not support macros (and simple static inline helper<br>
functions in kernel header files). Currently, users need to define<br>
these macros and static inline functions explicitly in their bpf<br>
program. What we are thinking is to permit users to include kernel<br>
header files containing these macros/static inline functions. But this<br>
may introduce duplicated types as these types have been defined in<br>
vmlinux.h.<br>
<br>
For example, we may have:<br>
socket.h:<br>
struct socket { ... };<br>
#define TCP 6<br>
vmlinux.h:<br>
struct socket { ... };<br>
bpf.c:<br>
#include <socket.h><br>
#include <vmlinux.h><br>
... TCP ...<br>
<br>
In this case, struct "socket" is defined twice. If we can have something like<br>
bpf.c:<br>
#pragma clang attribute push (__attribute__((weak)), apply_to = record)<br>
#include <socket.h><br>
#pragma clang attribute pop<br>
#include <vmlinux.h><br>
... TCP ...<br>
<br>
<br>
Then bpf program can get header file macro definitions without copying<br>
explicitly.<br>
<br>
We need support for "apply_to = typedef" in the above pragma, so we<br>
can deal with typedef types as well.<br>
<br>
Another issue is related to what if two types are not compatible. For example,<br>
struct t { int a; } __attribute__((weak));<br>
struct t { int a; int b; };<br>
I think we could have a flag to control whether we allow incompatible<br>
weak type or not.<br>
<br>
It would be good if I can get some suggestions/directions on whether<br>
such a feature is possible or not for clang frontend.<br>
<br>
Thanks,<br>
<br>
Yonghong<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>