Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 this issue Aug 25, 2024 · 1 comment

Comments

@hhoffstaette
Copy link
Contributor

hhoffstaette commented Aug 25, 2024

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
Copy link
Contributor Author

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.

hhoffstaette added a commit to hhoffstaette/bcc that referenced this issue Nov 26, 2024
This creates unnecessary header confusion and valid complaints
e.g. when building with LTO. One could argue that there could be
an explicit version check for whatever libbpf version bcc needs,
but silently trying to be helpful by doing something else is
pretty much wrong.

This is one part of fixing iovisor#5091

Signed-off-by: Holger Hoffstätte <[email protected]>
hhoffstaette added a commit to hhoffstaette/bcc that referenced this issue Nov 26, 2024
This is not just good hygiene but also silences valid complaints
about mismatched definitions when building with LTO.

This is one part of fixing iovisor#5091

Signed-off-by: Holger Hoffstätte <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant