Propagate VCD data parse errors

src/cat.rs

@@ -6,7 +6,7 @@
 // SPDX-License-Identifier: MIT
 //------------------------------------------------------------------------------
 
-use std::io::{BufRead, Seek, Write};
+use std::io::{self, BufRead, Seek, Write};
 
 use crossbeam_channel as channel;
 use fst_reader::{FstFilter, FstSignalHandle, FstSignalValue};
@@ -175,15 +175,15 @@ fn send_cat_changes(
     start: u64,
     end: Option<u64>,
     tx: channel::Sender<CatChange>,
-) {
+) -> io::Result<()> {
     match &mut reader {
         WaveReader::Fst(fst_reader) => {
             let filter = FstFilter {
                 start,
                 end,
                 include: None,
             };
-            let _ = fst_reader.read_signals(&filter, |time, handle, value| {
+            let read_result = fst_reader.read_signals(&filter, |time, handle, value| {
                 if time < filter.start {
                     return;
                 }
@@ -204,9 +204,10 @@ fn send_cat_changes(
                     value: value_str,
                 });
             });
+            read_result.map_err(|e| io::Error::other(format!("Failed to read signals: {}", e)))?;
         }
         WaveReader::Vcd(vcd_data) => {
-            while let Some((time, handle_idx, value_str)) = next_vcd_change(vcd_data) {
+            while let Some((time, handle_idx, value_str)) = next_vcd_change(vcd_data)? {
                 if time < start {
                     continue;
                 }
@@ -223,6 +224,7 @@ fn send_cat_changes(
             }
         }
     }
+    Ok(())
 }
 
 /// Write signal values from multiple WaveReaders, merging their outputs in time order.
@@ -258,15 +260,15 @@ pub fn write_signals_wave_multi<W: Write>(
     for (reader, &offset) in readers.into_iter().zip(offsets.iter()) {
         let (tx, rx) = channel::bounded(CHANNEL_BOUND);
         threads.push(std::thread::spawn(move || {
-            send_cat_changes(reader, offset, start, end, tx);
+            send_cat_changes(reader, offset, start, end, tx)
         }));
         rxs.push(rx);
     }
 
     let mut current_time: Option<u64> = None;
     let mut batch: Vec<(String, String)> = Vec::new();
 
-    crate::kway_merge_channels(&rxs, |c| c.time, |change| {
+    let merge_result = crate::kway_merge_channels(&rxs, |c| c.time, |change| {
         if options.sort {
             if let Some(prev_time) = current_time {
                 if prev_time != change.time {
@@ -291,18 +293,38 @@ pub fn write_signals_wave_multi<W: Write>(
             }
         }
         Ok(())
-    })?;
+    });
+
+    if merge_result.is_err() {
+        drop(rxs);
+    }
+
+    let mut thread_result = Ok(());
+    for t in threads {
+        match t.join() {
+            Ok(Ok(())) => {}
+            Ok(Err(e)) => {
+                if thread_result.is_ok() {
+                    thread_result = Err(e);
+                }
+            }
+            Err(_) => {
+                if thread_result.is_ok() {
+                    thread_result = Err(io::Error::other("wavecat reader thread panicked"));
+                }
+            }
+        }
+    }
+
+    merge_result?;
+    thread_result?;
 
     if options.sort {
         if let Some(time) = current_time {
             flush_signal_batch(writer, time, &mut batch, options.time_pound)?;
         }
     }
 
-    for t in threads {
-        t.join().unwrap();
-    }
-
     Ok(())
 }
 
@@ -317,7 +339,7 @@ fn write_vcd_signals<W: Write>(
     let mut current_batch_time: Option<u64> = None;
     let mut batch: Vec<(String, String)> = Vec::new();
 
-    while let Some((time, handle_idx, value_str)) = next_vcd_change(vcd_data) {
+    while let Some((time, handle_idx, value_str)) = next_vcd_change(vcd_data)? {
         if time < start {
             continue;
         }

src/diff.rs

@@ -8,7 +8,7 @@
 
 use std::collections::{HashMap, HashSet};
 use std::fmt;
-use std::io::{BufRead, Seek, Write};
+use std::io::{self, BufRead, Seek, Write};
 use std::path::Path;
 
 use crossbeam_channel as channel;
@@ -200,10 +200,10 @@ fn read_and_send_signals<R: BufRead + Seek>(
     filter: fst_reader::FstFilter,
     handle_offset: usize,
     tx: channel::Sender<TimeBatch>,
-) {
+) -> io::Result<()> {
     let mut batch = Vec::with_capacity(BATCH_SIZE);
     let mut batch_time: u64 = 0;
-    let _ = fst_reader.read_signals(&filter, |time, handle, value| {
+    let read_result = fst_reader.read_signals(&filter, |time, handle, value| {
         if time < filter.start {
             return;
         }
@@ -221,9 +221,11 @@ fn read_and_send_signals<R: BufRead + Seek>(
             value: OwnedSignalValue::from_fst_value(value),
         });
     });
+    read_result.map_err(|e| io::Error::other(format!("Failed to read signals: {}", e)))?;
     if !batch.is_empty() {
         let _ = tx.send(TimeBatch { time: batch_time, changes: batch });
     }
+    Ok(())
 }
 
 /// Format signal names for a handle on-demand from SignalMap + NameTree.
@@ -375,20 +377,20 @@ fn send_wave_changes(
     start: u64,
     end: Option<u64>,
     tx: channel::Sender<TimeBatch>,
-) {
+) -> io::Result<()> {
     match reader {
         WaveReader::Fst(fst_reader) => {
             let filter = FstFilter {
                 start,
                 end,
                 include: None,
             };
-            read_and_send_signals(*fst_reader, filter, handle_offset, tx);
+            read_and_send_signals(*fst_reader, filter, handle_offset, tx)?;
         }
         WaveReader::Vcd(mut vcd_data) => {
             let mut batch = Vec::with_capacity(BATCH_SIZE);
             let mut batch_time: u64 = 0;
-            while let Some((time, handle, value_str)) = next_vcd_change(&mut vcd_data) {
+            while let Some((time, handle, value_str)) = next_vcd_change(&mut vcd_data)? {
                 if time < start {
                     continue;
                 }
@@ -411,6 +413,7 @@ fn send_wave_changes(
             }
         }
     }
+    Ok(())
 }
 
 /// Send changes from multiple WaveReaders through a single channel, merging in time order.
@@ -423,16 +426,15 @@ fn send_merged_wave_changes(
     start: u64,
     end: Option<u64>,
     tx: channel::Sender<TimeBatch>,
-) {
+) -> io::Result<()> {
     if readers.len() == 1 {
-        send_wave_changes(
+        return send_wave_changes(
             readers.into_iter().next().unwrap(),
             offsets[0],
             start,
             end,
             tx,
         );
-        return;
     }
 
     // Spawn a thread per reader, each sending TimeBatches to its own channel.
@@ -442,7 +444,7 @@ fn send_merged_wave_changes(
     for (reader, &offset) in readers.into_iter().zip(offsets.iter()) {
         let (inner_tx, inner_rx) = channel::bounded(CHANNEL_BOUND);
         threads.push(std::thread::spawn(move || {
-            send_wave_changes(reader, offset, start, end, inner_tx);
+            send_wave_changes(reader, offset, start, end, inner_tx)
         }));
         inner_rxs.push(inner_rx);
     }
@@ -466,8 +468,14 @@ fn send_merged_wave_changes(
     }
 
     for t in threads {
-        t.join().unwrap();
+        match t.join() {
+            Ok(Ok(())) => {}
+            Ok(Err(e)) => return Err(e),
+            Err(_) => return Err(io::Error::other("wavediff reader thread panicked")),
+        }
     }
+
+    Ok(())
 }
 
 /// Compare metadata and attributes for signals that share the same name across two files.
@@ -589,10 +597,10 @@ pub fn diff_wave_sets<W: Write>(
     } = sets;
 
     let thread1 = std::thread::spawn(move || {
-        send_merged_wave_changes(readers1, &offsets1, start, end, tx1);
+        send_merged_wave_changes(readers1, &offsets1, start, end, tx1)
     });
     let thread2 = std::thread::spawn(move || {
-        send_merged_wave_changes(readers2, &offsets2, start, end, tx2);
+        send_merged_wave_changes(readers2, &offsets2, start, end, tx2)
     });
 
     let result = compare_signal_channels(
@@ -605,10 +613,20 @@ pub fn diff_wave_sets<W: Write>(
         options.real_epsilon,
     );
 
-    thread1.join().unwrap();
-    thread2.join().unwrap();
+    let thread1_result = match thread1.join() {
+        Ok(r) => r,
+        Err(_) => Err(io::Error::other("wavediff set1 reader thread panicked")),
+    };
+    let thread2_result = match thread2.join() {
+        Ok(r) => r,
+        Err(_) => Err(io::Error::other("wavediff set2 reader thread panicked")),
+    };
 
-    result
+    let has_differences = result?;
+    thread1_result?;
+    thread2_result?;
+
+    Ok(has_differences)
 }
 
 /// Open two sets of waveform files and return readers and merged hierarchies

src/lib.rs

@@ -8,7 +8,7 @@
 
 mod cat;
 mod diff;
-#[allow(dead_code, unused_imports, clippy::manual_repeat_n, mismatched_lifetime_syntaxes)]
+#[allow(dead_code, unused_imports, clippy::manual_repeat_n)]
 mod vcd;
 
 pub use cat::{write_signals_wave, write_signals_wave_multi, SignalOutputOptions};
@@ -527,37 +527,40 @@ fn vcd_value_char(v: vcd::Value) -> char {
 }
 
 /// Read the next signal change from a VCD parser, skipping non-change commands
-pub(crate) fn next_vcd_change(vcd_data: &mut VcdData) -> Option<(u64, usize, String)> {
-    while let Some(Ok(cmd)) = vcd_data.parser.next() {
+pub(crate) fn next_vcd_change(
+    vcd_data: &mut VcdData,
+) -> std::io::Result<Option<(u64, usize, String)>> {
+    for cmd in vcd_data.parser.by_ref() {
+        let cmd = cmd?;
         match cmd {
             vcd::Command::Timestamp(t) => {
                 vcd_data.current_time = t;
             }
             vcd::Command::ChangeScalar(id, val) => {
                 if let Some(&idx) = vcd_data.id_to_idx.get(&id) {
-                    return Some((vcd_data.current_time, idx, vcd_value_char(val).to_string()));
+                    return Ok(Some((vcd_data.current_time, idx, vcd_value_char(val).to_string())));
                 }
             }
             vcd::Command::ChangeVector(id, ref vec) => {
                 if let Some(&idx) = vcd_data.id_to_idx.get(&id) {
                     let s: String = vec.iter().map(vcd_value_char).collect();
-                    return Some((vcd_data.current_time, idx, s));
+                    return Ok(Some((vcd_data.current_time, idx, s)));
                 }
             }
             vcd::Command::ChangeReal(id, val) => {
                 if let Some(&idx) = vcd_data.id_to_idx.get(&id) {
-                    return Some((vcd_data.current_time, idx, val.to_string()));
+                    return Ok(Some((vcd_data.current_time, idx, val.to_string())));
                 }
             }
             vcd::Command::ChangeString(id, ref s) => {
                 if let Some(&idx) = vcd_data.id_to_idx.get(&id) {
-                    return Some((vcd_data.current_time, idx, s.clone()));
+                    return Ok(Some((vcd_data.current_time, idx, s.clone())));
                 }
             }
             _ => {}
         }
     }
-    None
+    Ok(None)
 }
 
 /// Registry of enum table definitions, keyed by handle ID.

src/vcd/value.rs

@@ -89,7 +89,7 @@ impl Vector {
     }
 
     /// Returns an iterator over the values in the vector.
-    pub fn iter(&self) -> VectorIter {
+    pub fn iter(&self) -> VectorIter<'_> {
         VectorIter(self.0.iter())
     }
 

tests/data/error/malformed_data.vcd

@@ -0,0 +1,13 @@
+$date now $end
+$version test $end
+$timescale 1 ns $end
+$scope module top $end
+$var wire 1 ! clk $end
+$upscope $end
+$enddefinitions $end
+#0
+0!
+#10
+q!
+#20
+1!

tests/diff_test.rs

@@ -739,6 +739,41 @@ fn test_cli_no_attrs_still_detects_value_diffs() {
     );
 }
 
+#[test]
+fn test_cli_wavecat_malformed_vcd_data_exits_with_error() {
+    let output = run_wavecat_cli(&["tests/data/error/malformed_data.vcd"]);
+    assert_eq!(
+        output.status.code(),
+        Some(1),
+        "wavecat should exit 1 on malformed VCD data"
+    );
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("unexpected character"),
+        "wavecat stderr should report the parser error: {}",
+        stderr
+    );
+}
+
+#[test]
+fn test_cli_wavediff_malformed_vcd_data_exits_with_error() {
+    let output = run_wavediff_cli(&[
+        "tests/data/error/malformed_data.vcd",
+        "tests/data/error/malformed_data.vcd",
+    ]);
+    assert_eq!(
+        output.status.code(),
+        Some(2),
+        "wavediff should exit 2 on malformed VCD data"
+    );
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("unexpected character"),
+        "wavediff stderr should report the parser error: {}",
+        stderr
+    );
+}
+
 // -- Enum conflict detection tests --------------------------------------------
 
 #[test]