Skip to content

Commit

Permalink
libbpf-cargo: return stderr from clang
Browse files Browse the repository at this point in the history
Update `build_and_generate` to return a `CompilationOutput` owning the stderr
of the compiler process. This gives consumers access to the stderr to output as
they see fit.

Provides a `stderr` method that calls `String::from_utf8_lossy`. The primary
consumer is `sched-ext/scx` which outputs each line with
`println!("cargo:warning={}", l)`, showing the warnings quite nicely in the
Cargo output when there isn't an error.

Test plan:
- Pointed that Cargo.lock at this repo and built.
  • Loading branch information
JakeHillion authored and danielocfb committed Nov 13, 2024
1 parent aff9d72 commit 9aac71c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 31 deletions.
66 changes: 42 additions & 24 deletions libbpf-cargo/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@ use tempfile::tempdir;
use crate::metadata;
use crate::metadata::UnprocessedObj;

/// Contains information about a successful compilation.
#[derive(Debug)]
pub struct CompilationOutput {
stderr: Vec<u8>,
}

impl CompilationOutput {
// Only used in libbpf-cargo library
#[allow(dead_code)]
/// Read the stderr from the compilation
pub fn stderr(&self) -> &[u8] {
&self.stderr
}
}

fn check_progs(objs: &[UnprocessedObj]) -> Result<()> {
let mut set = HashSet::with_capacity(objs.len());
for obj in objs {
Expand Down Expand Up @@ -171,7 +186,7 @@ fn compile_one(
out: &Path,
clang: &Path,
clang_args: &[OsString],
) -> Result<()> {
) -> Result<CompilationOutput> {
if debug {
println!("Building {}", source.display());
}
Expand Down Expand Up @@ -234,7 +249,12 @@ fn compile_one(
// system specific and temporary paths. That can render our generated
// skeletons unstable, potentially rendering them unsuitable for inclusion
// in version control systems. So strip this information.
strip_dwarf_info(out).with_context(|| format!("Failed to strip object file {}", out.display()))
strip_dwarf_info(out)
.with_context(|| format!("Failed to strip object file {}", out.display()))?;

Ok(CompilationOutput {
stderr: output.stderr,
})
}

fn compile(
Expand All @@ -243,31 +263,31 @@ fn compile(
clang: &Path,
mut clang_args: Vec<OsString>,
target_dir: &Path,
) -> Result<()> {
) -> Result<Vec<CompilationOutput>> {
let header_dir = extract_libbpf_headers_to_disk(target_dir)?;
if let Some(dir) = header_dir {
clang_args.push(OsString::from("-I"));
clang_args.push(dir.into_os_string());
}

for obj in objs {
let stem = obj.path.file_stem().with_context(|| {
format!(
"Could not calculate destination name for obj={}",
obj.path.display()
)
})?;

let mut dest_name = stem.to_os_string();
dest_name.push(".o");

let mut dest_path = obj.out.to_path_buf();
dest_path.push(&dest_name);
fs::create_dir_all(&obj.out)?;
compile_one(debug, &obj.path, &dest_path, clang, &clang_args)?;
}
objs.iter()
.map(|obj| -> Result<_> {
let stem = obj.path.file_stem().with_context(|| {
format!(
"Could not calculate destination name for obj={}",
obj.path.display()
)
})?;

Ok(())
let mut dest_name = stem.to_os_string();
dest_name.push(".o");

let mut dest_path = obj.out.to_path_buf();
dest_path.push(&dest_name);
fs::create_dir_all(&obj.out)?;
compile_one(debug, &obj.path, &dest_path, clang, &clang_args)
})
.collect::<Result<_, _>>()
}

fn extract_clang_or_default(clang: Option<&PathBuf>) -> PathBuf {
Expand Down Expand Up @@ -316,7 +336,7 @@ pub(crate) fn build_single(
clang: Option<&PathBuf>,
skip_clang_version_checks: bool,
mut clang_args: Vec<OsString>,
) -> Result<()> {
) -> Result<CompilationOutput> {
let clang = extract_clang_or_default(clang);
check_clang(debug, &clang, skip_clang_version_checks)?;
let header_parent_dir = tempdir()?;
Expand All @@ -331,9 +351,7 @@ pub(crate) fn build_single(
// BPF. See https://lkml.org/lkml/2020/2/21/1000.
clang_args.push(OsString::from("-fno-stack-protector"));

compile_one(debug, source, out, &clang, &clang_args)?;

Ok(())
compile_one(debug, source, out, &clang, &clang_args)
}

#[test]
Expand Down
14 changes: 7 additions & 7 deletions libbpf-cargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
)]
#![deny(unsafe_op_in_unsafe_fn)]

pub use build::CompilationOutput;

use std::ffi::OsStr;
use std::ffi::OsString;
use std::path::Path;
Expand Down Expand Up @@ -210,17 +212,17 @@ impl SkeletonBuilder {
}

/// Build BPF programs and generate the skeleton at path `output`
pub fn build_and_generate<P: AsRef<Path>>(&mut self, output: P) -> Result<()> {
self.build()?;
pub fn build_and_generate<P: AsRef<Path>>(&mut self, output: P) -> Result<CompilationOutput> {
let comp_output = self.build()?;
self.generate(output)?;

Ok(())
Ok(comp_output)
}

// Build BPF programs without generating a skeleton.
//
// [`SkeletonBuilder::source`] must be set for this to succeed.
pub fn build(&mut self) -> Result<()> {
pub fn build(&mut self) -> Result<CompilationOutput> {
let source = self
.source
.as_ref()
Expand Down Expand Up @@ -257,9 +259,7 @@ impl SkeletonBuilder {
self.skip_clang_version_check,
self.clang_args.clone(),
)
.with_context(|| format!("failed to build `{}`", source.display()))?;

Ok(())
.with_context(|| format!("failed to build `{}`", source.display()))
}

// Generate a skeleton at path `output` without building BPF programs.
Expand Down

0 comments on commit 9aac71c

Please sign in to comment.