Merge pull request #76 from aya-rs/logging

log: append rather than truncate

.github/workflows/lint.yml

@@ -27,7 +27,7 @@ jobs:
 
       - name: Run clippy
         run: |
-          cargo clippy --workspace -- --deny warnings
+          cargo clippy --all-targets --workspace -- --deny warnings
 
   lint-nightly:
     runs-on: ubuntu-20.04

src/bin/bpf-linker.rs

@@ -9,9 +9,8 @@ use simplelog::{
     ColorChoice, Config, LevelFilter, SimpleLogger, TermLogger, TerminalMode, WriteLogger,
 };
 use std::{
-    collections::HashSet,
     env,
-    fs::{self, File},
+    fs::{self, OpenOptions},
     path::PathBuf,
     str::FromStr,
 };
@@ -154,9 +153,29 @@ fn main() {
             arg
         }
     });
-    let cli = CommandLine::parse_from(args);
+    let CommandLine {
+        target,
+        cpu,
+        cpu_features,
+        output,
+        emit,
+        libs,
+        optimize,
+        export_symbols,
+        log_file,
+        log_level,
+        unroll_loops,
+        ignore_inline_never,
+        dump_module,
+        llvm_args,
+        disable_expand_memcpy_in_order,
+        disable_memory_builtins,
+        inputs,
+        export,
+        _debug,
+    } = Parser::parse_from(args);
 
-    if cli.inputs.is_empty() {
+    if inputs.is_empty() {
         error("no input files", clap::error::ErrorKind::TooFewValues);
     }
 
@@ -170,9 +189,9 @@ fn main() {
         },
         _ => None,
     };
-    let log_level = cli.log_level.or(env_log_level).unwrap_or(LevelFilter::Warn);
-    if let Some(path) = cli.log_file.clone() {
-        let log_file = match File::create(path) {
+    let log_level = log_level.or(env_log_level).unwrap_or(LevelFilter::Warn);
+    if let Some(log_file) = log_file {
+        let log_file = match OpenOptions::new().create(true).append(true).open(log_file) {
             Ok(f) => f,
             Err(e) => {
                 error(
@@ -198,38 +217,22 @@ fn main() {
         env::args().collect::<Vec<_>>().join(" ")
     );
 
-    let CommandLine {
-        target,
-        cpu,
-        cpu_features,
-        inputs,
-        output,
-        emit,
-        libs,
-        optimize,
-        export_symbols,
-        unroll_loops,
-        ignore_inline_never,
-        dump_module,
-        llvm_args,
-        disable_expand_memcpy_in_order,
-        disable_memory_builtins,
-        mut export,
-        ..
-    } = cli;
-
-    let mut export_symbols = export_symbols
-        .map(|path| match fs::read_to_string(path) {
-            Ok(symbols) => symbols
-                .lines()
-                .map(|s| s.to_string())
-                .collect::<HashSet<_>>(),
-            Err(e) => {
-                error(&e.to_string(), clap::error::ErrorKind::Io);
-            }
-        })
-        .unwrap_or_else(HashSet::new);
-    export_symbols.extend(export.drain(..));
+    let export_symbols = export_symbols
+        .map(fs::read_to_string)
+        .transpose()
+        .unwrap_or_else(|e| {
+            error(&e.to_string(), clap::error::ErrorKind::Io);
+        });
+
+    // TODO: the data is owned by this call frame; we could make this zero-alloc.
+    let export_symbols = export_symbols
+        .as_deref()
+        .into_iter()
+        .flat_map(str::lines)
+        .map(str::to_owned)
+        .chain(export)
+        .map(Into::into)
+        .collect();
 
     let options = LinkerOptions {
         target,
@@ -270,7 +273,7 @@ mod test {
     // `--export` flag.
     #[test]
     fn test_export_input_args() {
-        let args = vec![
+        let args = [
             "bpf-linker",
             "--export",
             "foo",
@@ -289,17 +292,17 @@ mod test {
             "--target=bpf",
             "--emit=asm",
         ];
-        let cli = CommandLine::parse_from(args);
-        assert_eq!(cli.export, vec!["foo", "bar"]);
+        let CommandLine { inputs, export, .. } = Parser::parse_from(args);
+        assert_eq!(export, ["foo", "bar"]);
         assert_eq!(
-            cli.inputs,
-            vec![PathBuf::from("symbols.o"), PathBuf::from("rcgu.o")]
+            inputs,
+            [PathBuf::from("symbols.o"), PathBuf::from("rcgu.o")]
         );
     }
 
     #[test]
     fn test_export_delimiter() {
-        let args = vec![
+        let args = [
             "bpf-linker",
             "--export",
             "foo,bar",
@@ -320,14 +323,11 @@ mod test {
             "--target=bpf",
             "--emit=asm",
         ];
-        let cli = CommandLine::parse_from(args);
-        assert_eq!(
-            cli.export,
-            vec!["foo", "bar", "ayy", "lmao", "lol", "rotfl"]
-        );
+        let CommandLine { inputs, export, .. } = Parser::parse_from(args);
+        assert_eq!(export, ["foo", "bar", "ayy", "lmao", "lol", "rotfl"]);
         assert_eq!(
-            cli.inputs,
-            vec![PathBuf::from("symbols.o"), PathBuf::from("rcgu.o")]
+            inputs,
+            [PathBuf::from("symbols.o"), PathBuf::from("rcgu.o")]
         );
     }
 }

src/linker.rs

@@ -6,6 +6,7 @@ use llvm_sys::prelude::*;
 use llvm_sys::target_machine::*;
 use log::*;
 use std::{
+    borrow::Cow,
     collections::HashSet,
     ffi::{CStr, CString},
     fs::File,
@@ -193,7 +194,7 @@ pub struct LinkerOptions {
     /// Optimization level.
     pub optimize: OptLevel,
     /// Set of symbol names to export.
-    pub export_symbols: HashSet<String>,
+    pub export_symbols: HashSet<Cow<'static, str>>,
     /// Whether to aggressively unroll loops. Useful for older kernels that don't support loops.
     pub unroll_loops: bool,
     /// Remove `noinline` attributes from functions. Useful for kernels before 5.8 that don't
@@ -411,8 +412,8 @@ impl Linker {
         if !self.options.disable_memory_builtins {
             self.options.export_symbols.extend(
                 ["memcpy", "memmove", "memset", "memcmp", "bcmp"]
-                    .iter()
-                    .map(|&s| s.to_owned()),
+                    .into_iter()
+                    .map(Into::into),
             );
         };
         debug!(
@@ -469,19 +470,22 @@ impl Linker {
     }
 
     fn llvm_init(&mut self) {
-        let mut args = vec!["bpf-linker".to_string()];
+        let mut args = Vec::<Cow<str>>::new();
+        args.push("bpf-linker".into());
         if self.options.unroll_loops {
             // setting cmdline arguments is the only way to customize the unroll pass with the
             // C API.
-            args.push("--unroll-runtime".to_string());
-            args.push("--unroll-runtime-multi-exit".to_string());
-            args.push(format!("--unroll-max-upperbound={}", std::u32::MAX));
-            args.push(format!("--unroll-threshold={}", std::u32::MAX));
+            args.extend([
+                "--unroll-runtime".into(),
+                "--unroll-runtime-multi-exit".into(),
+                format!("--unroll-max-upperbound={}", std::u32::MAX).into(),
+                format!("--unroll-threshold={}", std::u32::MAX).into(),
+            ]);
         }
         if !self.options.disable_expand_memcpy_in_order {
-            args.push("--bpf-expand-memcpy-in-order".to_string());
+            args.push("--bpf-expand-memcpy-in-order".into());
         }
-        args.extend_from_slice(&self.options.llvm_args);
+        args.extend(self.options.llvm_args.iter().map(Into::into));
         info!("LLVM command line: {:?}", args);
         unsafe {
             llvm::init(&args, "BPF linker");

src/llvm/mod.rs

@@ -2,6 +2,7 @@ mod iter;
 mod message;
 
 use std::{
+    borrow::Cow,
     collections::HashSet,
     ffi::{c_void, CStr, CString},
     os::raw::c_char,
@@ -172,7 +173,7 @@ pub unsafe fn optimize(
     module: LLVMModuleRef,
     opt_level: OptLevel,
     ignore_inline_never: bool,
-    export_symbols: &HashSet<String>,
+    export_symbols: &HashSet<Cow<'static, str>>,
 ) {
     if module_asm_is_probestack(module) {
         LLVMSetModuleInlineAsm2(module, ptr::null_mut(), 0);
@@ -213,10 +214,10 @@ pub unsafe fn optimize(
     LLVMPassManagerBuilderPopulateModulePassManager(pmb, mpm);
 
     for sym in module.globals_iter() {
-        internalize(sym, &symbol_name(sym), export_symbols);
+        internalize(sym, symbol_name(sym), export_symbols);
     }
     for sym in module.global_aliases_iter() {
-        internalize(sym, &symbol_name(sym), export_symbols);
+        internalize(sym, symbol_name(sym), export_symbols);
     }
 
     debug!("running function passes");
@@ -227,7 +228,7 @@ pub unsafe fn optimize(
             if ignore_inline_never {
                 remove_attribute(function, "noinline");
             }
-            internalize(function, &name, export_symbols);
+            internalize(function, name, export_symbols);
             if LLVMIsDeclaration(function) == 0 {
                 LLVMRunFunctionPassManager(fpm, function);
             }
@@ -254,11 +255,10 @@ unsafe fn module_asm_is_probestack(module: LLVMModuleRef) -> bool {
     asm.contains("__rust_probestack")
 }
 
-fn symbol_name(value: *mut llvm_sys::LLVMValue) -> String {
+fn symbol_name<'a>(value: *mut llvm_sys::LLVMValue) -> &'a str {
     let mut name_len = 0;
-    unsafe { CStr::from_ptr(LLVMGetValueName2(value, &mut name_len)) }
-        .to_string_lossy()
-        .to_string()
+    let ptr = unsafe { LLVMGetValueName2(value, &mut name_len) };
+    unsafe { CStr::from_ptr(ptr) }.to_str().unwrap()
 }
 
 unsafe fn remove_attribute(function: *mut llvm_sys::LLVMValue, name: &str) {
@@ -298,7 +298,11 @@ pub unsafe fn codegen(
     Ok(())
 }
 
-pub unsafe fn internalize(value: LLVMValueRef, name: &str, export_symbols: &HashSet<String>) {
+pub unsafe fn internalize(
+    value: LLVMValueRef,
+    name: &str,
+    export_symbols: &HashSet<Cow<'static, str>>,
+) {
     if !name.starts_with("llvm.") && !export_symbols.contains(name) {
         LLVMSetLinkage(value, LLVMLinkage::LLVMInternalLinkage);
         LLVMSetVisibility(value, LLVMVisibility::LLVMDefaultVisibility);

tests/tests.rs

@@ -6,23 +6,27 @@ use std::{
 use which::which;
 
 fn run_mode(target: &str, mode: &str, sysroot: Option<&Path>) {
-    let mut config = compiletest_rs::Config::default();
-    config.target = target.to_string();
     let mut target_rustcflags = format!("-C linker={}", env!("CARGO_BIN_EXE_bpf-linker"));
     if let Some(sysroot) = sysroot {
         let sysroot = sysroot.to_str().unwrap();
         target_rustcflags += &format!(" --sysroot {sysroot}");
     }
-    config.target_rustcflags = Some(target_rustcflags);
-    if let Ok(filecheck) = which("FileCheck") {
-        config.llvm_filecheck = Some(filecheck)
+    let llvm_filecheck = if let Ok(filecheck) = which("FileCheck") {
+        Some(filecheck)
     } else if let Ok(filecheck) = which("FileCheck-16") {
-        config.llvm_filecheck = Some(filecheck)
+        Some(filecheck)
     } else {
         panic!("no FileCheck binary found");
     };
-    config.mode = mode.parse().expect("Invalid mode");
-    config.src_base = PathBuf::from(format!("tests/{}", mode));
+    let mode = mode.parse().expect("Invalid mode");
+    let mut config = compiletest_rs::Config {
+        target: target.to_owned(),
+        target_rustcflags: Some(target_rustcflags),
+        llvm_filecheck,
+        mode,
+        src_base: PathBuf::from(format!("tests/{}", mode)),
+        ..Default::default()
+    };
     config.link_deps();
 
     compiletest_rs::run_tests(&config);
@@ -36,7 +40,7 @@ fn compile_test() {
     let rustc_src = rustc_build_sysroot::rustc_sysroot_src(rustc)
         .expect("could not determine sysroot source directory");
     let mut directory = env::current_dir().expect("could not determine current directory");
-    let () = directory.push("target/sysroot");
+    directory.push("target/sysroot");
     let () = rustc_build_sysroot::SysrootBuilder::new(&directory, target)
         .build_mode(rustc_build_sysroot::BuildMode::Build)
         .sysroot_config(rustc_build_sysroot::SysrootConfig::NoStd)