LLVM_DEFINITIONS args broken if arg contains "="

[benhsmith]

I wasn't able to build bcc because the LLVM_DEFINITIONS from my llvm build was parsed incorrectly, resulting in invalid compiler flags.

The LLVM_DEFINITIONS that caused the problem for me was -D_GLIBCXX_USE_CXX11_ABI=0 -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS

The resulting compiler flags were -D_GLIBCXX_USE_CXX11_ABI=0 -D_GLIBCXX_USE_CXX11_ABI="0 -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS"

Note the quote" after -D_GLIBCXX_USE_CXX11_ABI= . It looks like cmake is misparsing _GLIBCXX_USE_CXX11_ABI and setting all the args following as it's value, rather than as separate args.

I have a fix but I don't have permission to push up a branch so I couldn't figure out how to create a pull request. Here's the diff:

diff --git a/src/cc/CMakeLists.txt b/src/cc/CMakeLists.txt
index 0f02ad6..67c1d49 100644
--- a/src/cc/CMakeLists.txt
+++ b/src/cc/CMakeLists.txt
@@ -12,7 +12,12 @@ include_directories(${LIBELF_INCLUDE_DIRS})
 # todo: if check for kernel version
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/libbpf/include)
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/libbpf/include/uapi)
-add_definitions(${LLVM_DEFINITIONS})
+
+# add_definitions has a problem parsing "-D_GLIBCXX_USE_CXX11_ABI=0", this is safer
+string(REPLACE "-D" "" LLVM_DEFINITIONS ${LLVM_DEFINITIONS})
+separate_arguments(LLVM_DEFINITIONS)
+add_compile_definitions(${LLVM_DEFINITIONS})
+
 configure_file(libbcc.pc.in libbcc.pc @ONLY)
 configure_file(bcc_version.h.in bcc_version.h @ONLY)

[yonghong-song]

@benhsmith do you know why this happen?

I tried this patch on my local env with cmake 3.14.2. Somehow separate_arguments does not work for me

-- -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
   <=== original LLVM_DEFINITIONS
-- _GNU_SOURCE _DEBUG __STDC_CONSTANT_MACROS __STDC_FORMAT_MACROS __STDC_LIMIT_MACROS
   <=== after string REPLACE
-- _GNU_SOURCE_DEBUG__STDC_CONSTANT_MACROS__STDC_FORMAT_MACROS__STDC_LIMIT_MACROS
   <=== after separate arguments

Do you know why?

[benhsmith]

@yonghong-song what you might be seeing is that separate_arguments converts the string to a ; separated list of arguments. After separate_arguments, LLVM_DEFINITIONS should actually contain _GNU_SOURCE;_DEBUG;__STDC_CONSTANT_MACROS;__STDC_FORMAT_MACROS;__STDC_LIMIT_MACROS If you print this out as

message(${LLVM_DEFINITIONS})

cmake treats it as a list. If you include quotes:

message("${LLVM_DEFINITIONS}")

it should display with the semicolons.

[benhsmith]

I just discovered that add_compile_definitions doesn't seem to exist until cmake 3.12, so it's not a good choice. Here's another patch that also works but is compatible with more versions of cmake:

diff --git a/src/cc/CMakeLists.txt b/src/cc/CMakeLists.txt
index 0f02ad62..4021c662 100644
--- a/src/cc/CMakeLists.txt
+++ b/src/cc/CMakeLists.txt
@@ -12,7 +12,11 @@ include_directories(${LIBELF_INCLUDE_DIRS})
 # todo: if check for kernel version
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/libbpf/include)
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/libbpf/include/uapi)
-add_definitions(${LLVM_DEFINITIONS})
+
+# add_definitions has a problem parsing "-D_GLIBCXX_USE_CXX11_ABI=0", this is safer
+separate_arguments(LLVM_DEFINITIONS)
+add_compile_options(${LLVM_DEFINITIONS})
+
 configure_file(libbcc.pc.in libbcc.pc @ONLY)
 configure_file(bcc_version.h.in bcc_version.h @ONLY)

[yonghong-song]

@benhsmith Thanks for explanation. Indeed, message("${LLVM_DEFINITIONS}") print out properly. Your above latest change looks good. Could you create a pull request? The doc is here: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request

It might be the best for you to try to create a pull request since in the future you might make more contributions to bcc or other github projects. Thanks!

[benhsmith]

@yonghong-song yes, I wanted to create a pull request but I can't push my branch to the repo. Is the correct way to fork the repo and create a branch in my fork?

[benhsmith]

@yonghong-song thanks for your help. https://github.com/iovisor/bcc/pull/3052