iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.58k stars 3.88k forks source link

Build failure with LTO: examples/cpp/pyperf/PyPerfUtil.cc:29:20: error: ‘PYPERF_BPF_PROGRAM’ violates the C++ One Definition Rule [-Werror=odr] #5091

Open hhoffstaette opened 2 months ago

hhoffstaette commented 2 months ago

Gentoo found a problem when building with LTO + -Werror=odr, see here for the bug.

I have reproduced this with 0.31.0 and despite the fact that PyPerf is "only" an example, I am reporting it since it often indicates header confusion/mixups.

[156/156] : && /usr/bin/x86_64-pc-linux-gnu-g++ -pipe -march=native -O2 -flto -fuse-linker-plugin -Wall  -fPIC -Wl,-O1,--as-needed,-z,now,-z,pack-relative-relocs -flto=auto -fuse-linker-plugin    -rdynamic examples/cpp/pyperf/CMakeFiles/PyPerf.dir/PyPerf.cc.o examples/cpp/pyperf/CMakeFiles/PyPerf.dir/PyPerfUtil.cc.o examples/cpp/pyperf/CMakeFiles/PyPerf.dir/PyPerfBPFProgram.cc.o examples/cpp/pyperf/CMakeFiles/PyPerf.dir/PyPerfLoggingHelper.cc.o examples/cpp/pyperf/CMakeFiles/PyPerf.dir/PyPerfDefaultPrinter.cc.o examples/cpp/pyperf/CMakeFiles/PyPerf.dir/Py36Offsets.cc.o -o examples/cpp/pyperf/PyPerf  -Wl,-rpath,/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0_build/src/cc:/usr/lib/llvm/18/lib64  src/cc/libbcc.a  src/cc/libbcc.so.0.31.0  src/cc/libbcc-loader-static.a  -lelf  -lz  src/cc/frontends/clang/libclang_frontend.a  -Wl,--whole-archive  /usr/lib/llvm/18/lib64/libclang-cpp.so  /usr/lib/llvm/18/lib64/libLLVM.so.18.1  -Wl,--no-whole-archive  -lelf  -llzma  -lbpf  src/cc/usdt/libusdt-static.a  src/cc/api/libapi-static.a && :
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/examples/cpp/pyperf/PyPerfUtil.cc:29:20: warning: 'PYPERF_BPF_PROGRAM' violates the C++ One Definition Rule [-Wodr]
   29 | extern std::string PYPERF_BPF_PROGRAM;
      |                    ^
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/examples/cpp/pyperf/PyPerfBPFProgram.cc:11:26: note: type 'const struct string' itself violates the C++ One Definition Rule
   11 | extern const std::string PYPERF_BPF_PROGRAM = R"(
      |                          ^
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/examples/cpp/pyperf/PyPerfBPFProgram.cc:11:26: note: 'PYPERF_BPF_PROGRAM' was previously declared here
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/examples/cpp/pyperf/PyPerfUtil.cc:28:21: warning: 'kPy36OffsetConfig' violates the C++ One Definition Rule [-Wodr]
   28 | extern OffsetConfig kPy36OffsetConfig;
      |                     ^
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/examples/cpp/pyperf/Py36Offsets.cc:11:27: note: type 'const struct OffsetConfig' itself violates the C++ One Definition Rule
   11 | extern const OffsetConfig kPy36OffsetConfig = {
      |                           ^
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/examples/cpp/pyperf/PyPerfType.h:74:3: note: the incompatible type is defined here
   74 | } OffsetConfig;
      |   ^
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/examples/cpp/pyperf/Py36Offsets.cc:11:27: note: 'kPy36OffsetConfig' was previously declared here
   11 | extern const OffsetConfig kPy36OffsetConfig = {
      |                           ^
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/src/cc/compat/linux/bpf.h:1061:6: warning: type 'bpf_attach_type' violates the C++ One Definition Rule [-Wodr]
 1061 | enum bpf_attach_type {
      |      ^
/usr/include/bpf/uapi/linux/bpf.h:1061:6: note: an enum with different value name is defined in another translation unit
 1061 | enum bpf_attach_type {
      |      ^
/tmp/portage/dev-util/bcc-0.31.0/work/bcc-0.31.0/src/cc/compat/linux/bpf.h:1118:9: note: name 'BPF_TRACE_KPROBE_SESSION' differs from name '__MAX_BPF_ATTACH_TYPE' defined in another translation unit
 1118 |         BPF_TRACE_KPROBE_SESSION,
      |         ^
/usr/include/bpf/uapi/linux/bpf.h:1118:9: note: mismatching definition
 1118 |         __MAX_BPF_ATTACH_TYPE
      |         ^
hhoffstaette commented 3 weeks ago

Had a few spare cycles to look into this since it didn't seem too difficult.

The mismatched typedefs are a completely valid C++ complaint and easily fixed:

diff --git a/examples/cpp/pyperf/PyPerfUtil.cc b/examples/cpp/pyperf/PyPerfUtil.cc
index 312d0181..329dcf88 100644
--- a/examples/cpp/pyperf/PyPerfUtil.cc
+++ b/examples/cpp/pyperf/PyPerfUtil.cc
@@ -25,8 +25,8 @@
 namespace ebpf {
 namespace pyperf {

-extern OffsetConfig kPy36OffsetConfig;
-extern std::string PYPERF_BPF_PROGRAM;
+extern const OffsetConfig kPy36OffsetConfig;
+extern const std::string PYPERF_BPF_PROGRAM;

 const static int kPerfBufSizePages = 32;

"extern const" might seem a bit weird at first but works correctly, provided that the symbol is eventually defined once - which is the case here.

The last warning turns out to be a problem with cmake generating a likely mismatching compat/linux/bpf.h even when the build is explicitly told to use an external libbpf and accompanying headers. This seems helpful at first, but is unexpectedly wrong: when I tell the build to use an external libbpf/uapi, I really mean it and don't want random mystery meat from the vendored libbpf snapshot. The generated header contains a symbol that is not in any released libbpf as of today, hence the mismatch. Removing the section in src/cc/CMakeLists.txt fixes it:

diff --git a/src/cc/CMakeLists.txt b/src/cc/CMakeLists.txt
index 104eff0e..486bedcb 100644
--- a/src/cc/CMakeLists.txt
+++ b/src/cc/CMakeLists.txt
@@ -18,12 +18,6 @@ endif (LIBDEBUGINFOD_FOUND AND ENABLE_LIBDEBUGINFOD)
 # todo: if check for kernel version
 if (CMAKE_USE_LIBBPF_PACKAGE AND LIBBPF_FOUND)
   include_directories(${LIBBPF_INCLUDE_DIRS})
-  # create up-to-date linux/bpf.h from virtual_bpf.h (remove string wrapper);
-  # when libbpf is built as a submodule we use its version of linux/bpf.h
-  # so this does similar for the libbpf package, removing reliance on the
-  # system uapi header which can be out of date.
-  execute_process(COMMAND sh -c "cd ${CMAKE_CURRENT_SOURCE_DIR}/compat/linux && grep -ve '\\*\\*\\*\\*' virtual_bpf.h > bpf.h")
-  include_directories(${CMAKE_CURRENT_SOURCE_DIR}/compat)
 else()
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/libbpf/include)
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/libbpf/include/uapi)

With these changes the PyPerf example builds perfectly fine against an external libbpf using LTO.