bpftrace: only print kernel headers cannot be found warning on error

need more time to figure how to deal with kheaders cleanly
This commit is contained in:
Dominique Martinet 2024-06-23 21:09:52 +09:00
parent 0bf9c7660f
commit 75316f49bb
3 changed files with 283 additions and 0 deletions

View File

@ -0,0 +1,59 @@
From 76950d08a3da13dd9e51b2fbef3532c3de79265b Mon Sep 17 00:00:00 2001
From: Dominique Martinet <asmadeus@codewreck.org>
Date: Fri, 14 Jun 2024 23:19:05 +0900
Subject: [PATCH] utils: fix kernel headers not found warning (#3242)
The code would make ksrc and kobj empty then try to print them,
this would always print empty strings.
Store is_dir() checks as bool instead and use these, the strings
cannot be empty if the check passed.
(cherry picked from commit f937803c2ad156ab90b9194965dbfb62bef1ff80)
---
(mostly to avoid conflicts with the next patch)
src/utils.cpp | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/utils.cpp b/src/utils.cpp
index bda722136588..c358a401fc83 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -738,15 +738,11 @@ std::tuple<std::string, std::string> get_kernel_dirs(
auto ksrc = kdir + "/source";
auto kobj = kdir + "/build";
- // if one of source/ or build/ is not present - try to use the other one for both.
- if (!is_dir(ksrc)) {
- ksrc = "";
- }
- if (!is_dir(kobj)) {
- kobj = "";
- }
- if (ksrc.empty() && kobj.empty())
- {
+ // if one of source/ or build/ is not present - try to use the other one for
+ // both.
+ auto has_ksrc = is_dir(ksrc);
+ auto has_kobj = is_dir(kobj);
+ if (!has_ksrc && !has_kobj) {
LOG(WARNING) << "Could not find kernel headers in " << ksrc << " or "
<< kobj
<< ". To specify a particular path to kernel headers, set the "
@@ -757,12 +753,9 @@ std::tuple<std::string, std::string> get_kernel_dirs(
"file at /sys/kernel/kheaders.tar.xz";
return std::make_tuple("", "");
}
- if (ksrc.empty())
- {
+ if (!has_ksrc) {
ksrc = kobj;
- }
- else if (kobj.empty())
- {
+ } else if (!has_kobj) {
kobj = ksrc;
}
--
2.45.2

View File

@ -0,0 +1,220 @@
From 5208c6275e65d94d0ed169ca2b253602c78c15a8 Mon Sep 17 00:00:00 2001
From: Dominique Martinet <asmadeus@codewreck.org>
Date: Fri, 14 Jun 2024 22:32:43 +0900
Subject: [PATCH] kernel headers: only print kheaders not found warning if
parsing failed
Current code would print kheaders not found as soon as the code has any
include, even if the include worked.
This delays printing the warning until we know if parsing succeeded or
not, and only prints it if parsing failed.
Also update the message to give clearer extraction instructions
---
CHANGELOG.md | 2 ++
src/CMakeLists.txt | 3 ++-
src/main.cpp | 40 ++++++++++++++++++++++++----------------
src/utils.cpp | 45 +++++++++++++++++++++------------------------
src/utils.h | 5 +++--
5 files changed, 52 insertions(+), 43 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a4aca8b6c85d..181c1bffc9f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,8 @@ ## Unreleased
#### Added
#### Changed
+- Only print kernel headers not found message if parsing fails
+ - [#3265](https://github.com/bpftrace/bpftrace/pull/3265)
#### Deprecated
#### Removed
#### Fixed
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index eadb11207052..7b637835afd9 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -90,11 +90,12 @@ endif()
if (KERNEL_HEADERS_DIR)
MESSAGE(STATUS "Using KERNEL_HEADERS_DIR=${KERNEL_HEADERS_DIR}")
- target_compile_definitions(runtime PUBLIC KERNEL_HEADERS_DIR="${KERNEL_HEADERS_DIR}")
endif()
+target_compile_definitions(runtime PUBLIC KERNEL_HEADERS_DIR="${KERNEL_HEADERS_DIR}")
if (NOT SYSTEM_INCLUDE_PATHS EQUAL "auto")
MESSAGE(STATUS "Using SYSTEM_INCLUDE_PATHS=${SYSTEM_INCLUDE_PATHS}")
endif()
+target_compile_definitions(runtime PUBLIC SYSTEM_INCLUDE_PATHS="${SYSTEM_INCLUDE_PATHS}")
execute_process(
COMMAND git describe --abbrev=4 --dirty --tags
diff --git a/src/main.cpp b/src/main.cpp
index 3c532b3aa767..7918f90b90ab 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -420,24 +420,20 @@ static std::optional<struct timespec> get_delta_taitime()
bool should_clang_parse = !(driver.root.get()->c_definitions.empty() &&
bpftrace.btf_set_.empty());
-
- if (should_clang_parse)
- {
+ if (should_clang_parse) {
ClangParser clang;
+ bool found_kernel_headers;
+ std::string ksrc, kobj;
+ struct utsname utsname;
std::vector<std::string> extra_flags;
- {
- struct utsname utsname;
- uname(&utsname);
- std::string ksrc, kobj;
- auto kdirs = get_kernel_dirs(utsname);
- ksrc = std::get<0>(kdirs);
- kobj = std::get<1>(kdirs);
+ uname(&utsname);
+ found_kernel_headers = get_kernel_dirs(utsname, ksrc, kobj);
- if (ksrc != "")
- {
- extra_flags = get_kernel_cflags(
- utsname.machine, ksrc, kobj, bpftrace.kconfig);
- }
+ if (found_kernel_headers)
+ {
+ extra_flags = get_kernel_cflags(
+ utsname.machine, ksrc, kobj, bpftrace.kconfig);
+ found_kernel_headers = true;
}
extra_flags.push_back("-include");
extra_flags.push_back(CLANG_WORKAROUNDS_H);
@@ -453,8 +449,20 @@ static std::optional<struct timespec> get_delta_taitime()
extra_flags.push_back(file);
}
- if (!clang.parse(driver.root.get(), bpftrace, extra_flags))
+ if (!clang.parse(driver.root.get(), bpftrace, extra_flags)) {
+ if (!found_kernel_headers) {
+ LOG(WARNING)
+ << "Could not find kernel headers in " << ksrc << " / " << kobj
+ << ". To specify a particular path to kernel headers, set the env "
+ << "variables BPFTRACE_KERNEL_SOURCE and, optionally, "
+ << "BPFTRACE_KERNEL_BUILD if the kernel was built in a different "
+ << "directory than its source. You can also point the variable to "
+ << "a directory with built-in headers extracted from the following "
+ << "snippet:\nmodprobe kheaders && tar -C <directory> -xf "
+ << "/sys/kernel/kheaders.tar.xz";
+ }
return nullptr;
+ }
}
err = driver.parse();
diff --git a/src/utils.cpp b/src/utils.cpp
index c358a401fc83..06d7fa95ff6e 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -700,8 +700,8 @@ bool is_dir(const std::string& path)
return std_filesystem::is_directory(buf, ec);
}
-// get_kernel_dirs returns {ksrc, kobj} - directories for pristine and
-// generated kernel sources.
+// get_kernel_dirs fills {ksrc, kobj} - directories for pristine and
+// generated kernel sources - and returns if they were found.
//
// When the kernel was built in its source tree ksrc == kobj, however when
// the kernel was build in a different directory than its source, ksrc != kobj.
@@ -714,44 +714,41 @@ bool is_dir(const std::string& path)
//
// /lib/modules/`uname -r`/build/
//
-// {"", ""} is returned if no trace of kernel headers was found at all.
-// Both ksrc and kobj are guaranteed to be != "", if at least some trace of kernel sources was found.
-std::tuple<std::string, std::string> get_kernel_dirs(
- const struct utsname &utsname)
+// false is returned if no trace of kernel headers was found at all, with the guessed
+// location set anyway for later warning.
+// Both ksrc and kobj are guaranteed to be != ""
+bool get_kernel_dirs(const struct utsname &utsname,
+ std::string &ksrc,
+ std::string &kobj)
{
-#ifdef KERNEL_HEADERS_DIR
- return {KERNEL_HEADERS_DIR, KERNEL_HEADERS_DIR};
-#endif
+ ksrc = kobj = std::string(KERNEL_HEADERS_DIR);
+ if (!ksrc.empty())
+ return true;
const char *kpath_env = ::getenv("BPFTRACE_KERNEL_SOURCE");
if (kpath_env)
{
+ ksrc = std::string(kpath_env);
const char *kpath_build_env = ::getenv("BPFTRACE_KERNEL_BUILD");
- if (!kpath_build_env)
+ if (kpath_build_env)
{
- kpath_build_env = kpath_env;
+ kobj = std::string(kpath_build_env);
+ } else {
+ kobj = ksrc;
}
- return std::make_tuple(kpath_env, kpath_build_env);
+ return true;
}
std::string kdir = std::string("/lib/modules/") + utsname.release;
- auto ksrc = kdir + "/source";
- auto kobj = kdir + "/build";
+ ksrc = kdir + "/source";
+ kobj = kdir + "/build";
// if one of source/ or build/ is not present - try to use the other one for
// both.
auto has_ksrc = is_dir(ksrc);
auto has_kobj = is_dir(kobj);
if (!has_ksrc && !has_kobj) {
- LOG(WARNING) << "Could not find kernel headers in " << ksrc << " or "
- << kobj
- << ". To specify a particular path to kernel headers, set the "
- "env variables BPFTRACE_KERNEL_SOURCE and, optionally, "
- "BPFTRACE_KERNEL_BUILD if the kernel was built in a "
- "different directory than its source. To create kernel "
- "headers run 'modprobe kheaders', which will create a tar "
- "file at /sys/kernel/kheaders.tar.xz";
- return std::make_tuple("", "");
+ return false;
}
if (!has_ksrc) {
ksrc = kobj;
@@ -759,7 +756,7 @@ std::tuple<std::string, std::string> get_kernel_dirs(
kobj = ksrc;
}
- return std::make_tuple(ksrc, kobj);
+ return true;
}
const std::string &is_deprecated(const std::string &str)
diff --git a/src/utils.h b/src/utils.h
index bc78bd2176b5..9bd5395eab22 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -186,8 +186,9 @@ std::vector<int> get_online_cpus();
std::vector<int> get_possible_cpus();
bool is_dir(const std::string &path);
bool file_exists_and_ownedby_root(const char *f);
-std::tuple<std::string, std::string> get_kernel_dirs(
- const struct utsname &utsname);
+bool get_kernel_dirs(const struct utsname &utsname,
+ std::string &ksrc,
+ std::string &kobj);
std::vector<std::string> get_kernel_cflags(const char *uname_machine,
const std::string &ksrc,
const std::string &kobj,
--
2.45.2

View File

@ -51,6 +51,10 @@ stdenv.mkDerivation rec {
./tcp-bt-no-includes.patch
# https://github.com/bpftrace/bpftrace/pull/3262 (merged)
./runqlat-bt-no-includes.patch
# https://github.com/bpftrace/bpftrace/pull/3242 (merged)
./kheaders-not-found-message-fix.patch
# https://github.com/bpftrace/bpftrace/pull/3265
./kheaders-not-found-message-only-on-error.patch
];
# Pull BPF scripts into $PATH (next to their bcc program equivalents), but do