Merge pull request #80 from jfrimmel/fix-panic

Accept more issue types (such as invalid reads)

src/main.rs

@@ -26,29 +26,51 @@ use std::env;
 use std::process;
 
 /// Nicely format the errors in the valgrind output, if there are any.
-fn display_error(errors: &[valgrind::xml::Error]) {
+fn display_errors(errors: &[valgrind::xml::Error]) {
     // format the output in a helpful manner
     for error in errors {
-        eprintln!(
-            "{:>12} leaked {} in {} block{}",
-            "Error".red().bold(),
-            bytesize::to_string(error.resources.bytes as _, true),
-            error.resources.blocks,
-            if error.resources.blocks == 1 { "" } else { "s" }
-        );
+        if error.kind.is_leak() {
+            display_leak(error);
+        } else {
+            display_generic_error(error);
+        }
         let mut info = Some("Info".cyan().bold());
-        error
-            .stack_trace
+        error.stack_trace[0]
             .frames
             .iter()
             .for_each(|frame| eprintln!("{:>12} at {}", info.take().unwrap_or_default(), frame));
     }
 
     let total: usize = errors.iter().map(|error| error.resources.bytes).sum();
     eprintln!(
-        "{:>12} Leaked {} total",
+        "{:>12} Leaked {} total ({} other errors)",
         "Summary".red().bold(),
-        bytesize::to_string(total as _, true)
+        bytesize::to_string(total as _, true),
+        errors.iter().filter(|e| !e.kind.is_leak()).count()
+    );
+}
+
+/// Nicely format a single memory leak error.
+fn display_leak(error: &valgrind::xml::Error) {
+    eprintln!(
+        "{:>12} leaked {} in {} block{}",
+        "Error".red().bold(),
+        bytesize::to_string(error.resources.bytes as _, true),
+        error.resources.blocks,
+        if error.resources.blocks == 1 { "" } else { "s" }
+    );
+}
+
+/// Nicely format a non-memory-leak error.
+fn display_generic_error(error: &valgrind::xml::Error) {
+    eprintln!(
+        "{:>12} {}",
+        "Error".red().bold(),
+        error
+            .main_info
+            .as_ref()
+            .map(String::as_str)
+            .unwrap_or("unknown"),
     );
 }
 
@@ -94,7 +116,7 @@ fn main() {
                 errors: Some(errors),
                 ..
             }) => {
-                display_error(&errors);
+                display_errors(&errors);
                 127
             }
             Ok(_) => 0,

src/valgrind/mod.rs

@@ -94,7 +94,9 @@ where
             if let Some(err) = output.errors {
                 let new_err: Vec<xml::Error> = err
                     .into_iter()
-                    .filter(|e| e.resources.bytes > 0 || e.resources.blocks > 0)
+                    .filter(|e| {
+                        !e.kind.is_leak() || e.resources.bytes > 0 || e.resources.blocks > 0
+                    })
                     .collect();
                 if new_err.is_empty() {
                     output.errors = None;

src/valgrind/xml/mod.rs

@@ -58,8 +58,14 @@ pub struct Error {
     #[serde(default)]
     #[serde(rename = "xwhat")]
     pub resources: Resources,
+    #[serde(default)]
+    #[serde(rename = "what")]
+    pub main_info: Option<String>,
+    #[serde(default)]
+    #[serde(rename = "auxwhat")]
+    pub auxiliary_info: Vec<String>,
     #[serde(rename = "stack")]
-    pub stack_trace: Stack,
+    pub stack_trace: Vec<Stack>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize)]
@@ -84,6 +90,28 @@ pub enum Kind {
     SyscallParam,
     ClientCheck,
 }
+impl Kind {
+    /// Query, if the current error kind is a memory leak
+    pub(crate) fn is_leak(&self) -> bool {
+        match self {
+            Kind::LeakDefinitelyLost
+            | Kind::LeakStillReachable
+            | Kind::LeakIndirectlyLost
+            | Kind::LeakPossiblyLost => true,
+            Kind::InvalidFree
+            | Kind::MismatchedFree
+            | Kind::InvalidRead
+            | Kind::InvalidWrite
+            | Kind::InvalidJump
+            | Kind::Overlap
+            | Kind::InvalidMemPool
+            | Kind::UninitCondition
+            | Kind::UninitValue
+            | Kind::SyscallParam
+            | Kind::ClientCheck => false,
+        }
+    }
+}
 impl Display for Kind {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         match self {

src/valgrind/xml/tests.rs

@@ -22,7 +22,7 @@ fn sample_output() {
         }
     );
     assert_eq!(
-        &errors[0].stack_trace.frames[..2],
+        &errors[0].stack_trace[0].frames[..2],
         &[
             Frame {
                 instruction_pointer: 0x483_AD7B,

tests/corpus/.gitignore

@@ -0,0 +1 @@
+target/

tests/corpus/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "corpus"
+version = "0.0.0"
+publish = false
+
+[[bin]]
+name = "issue-74"
+path = "issue-74.rs"

tests/corpus/issue-74.rs

@@ -0,0 +1,69 @@
+#![allow(dead_code)]
+#![allow(unused_variables)]
+
+fn ex1() {
+  let mut num = 6;
+
+  let r1 = &num as *const i32;
+  let r2 = &mut num as *mut i32;
+  unsafe {
+    println!("r1: {:?}", *r1);
+    println!("r2: {:?}", *r2);
+  }
+}
+fn ex2() {
+  let address = 0x_012345_usize;
+  let r = address as *const i32;
+  unsafe {
+    // error?
+    println!("r={}", *r);
+  }
+}
+fn ex3() {
+  unsafe fn dangerous() {}
+  unsafe { dangerous() }
+}
+fn ex4() {
+  let mut v: Vec<_> = (1..7).collect();
+  let r = &mut v[..];
+  let (a, b)
+      = split_at_mut(r, 4);
+
+  println!("a={a:?} (should be [1, 2, 3, 4]");
+  drop(a);
+  println!("b={b:?} (should be [5, 6])");
+  // assert_eq!(a, &mut[1, 2, 3, 4]);
+  // assert_eq!(b, &mut[5, 6]);
+}
+fn split_at_mut(values: &mut[i32], ind: usize) 
+-> (&mut[i32], &mut[i32]) {
+
+  use std::slice::from_raw_parts_mut as part;
+
+  assert!(ind < values.len());
+
+  let len = values.len();
+  let ptr = values.as_mut_ptr();
+
+  unsafe {(
+    part(ptr, ind), 
+    // bug!
+    part(ptr.add(ind + 1), len - ind)
+  )}
+}
+extern "C" {
+  fn abs(input: i32) -> i32;
+}
+static mut COUNTER: u32 = 0;
+fn add_to_cound(inc: u32) {
+  unsafe { COUNTER += inc }
+}
+fn ex5() {
+  add_to_cound(3);
+  unsafe { println!("{}", COUNTER) }
+}
+fn main() {
+  ex4()
+}
+unsafe trait Doo {}
+unsafe impl Doo for i32 {}

tests/regression.rs

@@ -0,0 +1,23 @@
+use assert_cmd::Command;
+
+fn cargo_valgrind() -> Command {
+    let mut cmd = Command::cargo_bin("cargo-valgrind").unwrap();
+    cmd.arg("valgrind");
+    cmd
+}
+
+const TARGET_CRATE: &[&str] = &["--manifest-path", "tests/corpus/Cargo.toml"];
+
+#[test]
+fn issue74() {
+    cargo_valgrind()
+        .arg("run")
+        .args(TARGET_CRATE)
+        .arg("--bin=issue-74")
+        .assert()
+        .failure()
+        .stderr(predicates::str::contains("Error Invalid read of size 4"))
+        .stderr(predicates::str::contains(
+            "Summary Leaked 0 B total (1 other errors)",
+        ));
+}