Move HAVE_BUILTIN_BSWAP includes to separate header
authorDave Marchevsky <davemarchevsky@fb.com>
Fri, 30 Apr 2021 05:00:05 +0000 (22:00 -0700)
committeryonghong-song <ys114321@gmail.com>
Fri, 30 Apr 2021 23:51:30 +0000 (16:51 -0700)
As reported in #3366, on newer kernels bcc complains about macro
redefinition when compiling bpf programs:

```
include/linux/compiler-clang.h:46:9: warning: '__HAVE_BUILTIN_BSWAP64__' macro redefined [-Wmacro-redefined]
\#define __HAVE_BUILTIN_BSWAP64__
        ^
<command line>:5:9: note: previous definition is here
\#define __HAVE_BUILTIN_BSWAP64__ 1
```

Since these macros are passed in as `-D` cflags, they appear first
before any \#define statements in code. Since an [upstream kernel
patch](https://lore.kernel.org/linux-csky/20210226161151.2629097-1-arnd@kernel.org/)
added these defines in a kernel header, we see the warning.

This patch moves these definitions to a separate 'virtual' header that's included
after virtual_bpf.h and adds an ifndef guard. As a result, newer kernels
with the patch will not trigger the warning, while older kernels will
not lose the definition.

This should be safe based on my digging - some existing bcc programs use
`__builtin_bswap` methods, but without checking HAVE_BUILTIN_BSWAP.
Macros that may be conditionally defined based on HAVE_BUILTIN_BSWAP,
like those in `bpf_endian.h`, aren't. If a similar macro or struct def
in virtual_bpf.h - or any header it pulls in - changes depending on
HAVE_BUILTIN_BSWAP this could cause problems on older kernels, but I
don't believe that this is the case, or will be based on how
infrequently the defines are checked.

src/cc/export/bpf_workaround.h [new file with mode: 0644]
src/cc/exported_files.cc
src/cc/frontends/clang/kbuild_helper.cc

diff --git a/src/cc/export/bpf_workaround.h b/src/cc/export/bpf_workaround.h
new file mode 100644 (file)
index 0000000..732eab1
--- /dev/null
@@ -0,0 +1,11 @@
+R"********(
+#ifndef __HAVE_BUILTIN_BSWAP16__
+#define __HAVE_BUILTIN_BSWAP16__
+#endif
+#ifndef __HAVE_BUILTIN_BSWAP32__
+#define __HAVE_BUILTIN_BSWAP32__
+#endif
+#ifndef __HAVE_BUILTIN_BSWAP64__
+#define __HAVE_BUILTIN_BSWAP64__
+#endif
+)********"
index b9818e1..ec2c7d9 100644 (file)
@@ -30,6 +30,10 @@ map<string, const char *> ExportedFiles::headers_ = {
     #include "compat/linux/virtual_bpf.h"
   },
   {
+    "/virtual/include/bcc/bpf_workaround.h",
+    #include "export/bpf_workaround.h"
+  },
+  {
     "/virtual/include/bcc/proto.h",
     #include "export/proto.h"
   },
index 5bed272..5c57c13 100644 (file)
@@ -123,9 +123,6 @@ int KBuildHelper::get_flags(const char *uname_machine, vector<string> *cflags) {
   cflags->push_back("-include");
   cflags->push_back("./include/linux/kconfig.h");
   cflags->push_back("-D__KERNEL__");
-  cflags->push_back("-D__HAVE_BUILTIN_BSWAP16__");
-  cflags->push_back("-D__HAVE_BUILTIN_BSWAP32__");
-  cflags->push_back("-D__HAVE_BUILTIN_BSWAP64__");
   cflags->push_back("-DKBUILD_MODNAME=\"bcc\"");
 
   // If ARCH env variable is set, pass this along.