Merge pull request #1517 from ltratt/percolate_vaddr_error

Percolate dladdr() errors upwards.

hwtracer/src/lib.rs

@@ -17,6 +17,7 @@ pub use errors::{HWTracerError, TemporaryErrorKind};
 #[cfg(test)]
 use std::time::SystemTime;
 use std::{fmt::Debug, sync::Arc};
+use thiserror::Error;
 
 /// A builder for [Tracer]s. By default, will attempt to use the most appropriate [Tracer] for your
 /// platform/configuration. This can be overridden with [TracerBuilder::tracer_kind] and
@@ -93,7 +94,7 @@ pub trait Trace: Debug + Send {
     /// Iterate over the blocks of the trace.
     fn iter_blocks(
         self: Box<Self>,
-    ) -> Box<dyn Iterator<Item = Result<Block, HWTracerError>> + Send>;
+    ) -> Box<dyn Iterator<Item = Result<Block, BlockIteratorError>> + Send>;
 
     #[cfg(test)]
     fn bytes(&self) -> &[u8];
@@ -107,6 +108,15 @@ pub trait Trace: Debug + Send {
     fn len(&self) -> usize;
 }
 
+#[derive(Debug, Error)]
+pub enum BlockIteratorError {
+    #[cfg(ykpt)]
+    #[error("dladdr() cannot map vaddr")]
+    NoSuchVAddr,
+    #[error("HWTracerError: {0}")]
+    HWTracerError(HWTracerError),
+}
+
 /// A loop that does some work that we can use to build a trace.
 #[cfg(test)]
 #[inline(never)]

hwtracer/src/perf/collect.rs

@@ -7,7 +7,7 @@ use crate::pt::c_errors::PerfPTCError;
 use crate::pt::ykpt::YkPTBlockIterator;
 use crate::{
     errors::{HWTracerError, TemporaryErrorKind},
-    Block, ThreadTracer, Trace, Tracer,
+    Block, BlockIteratorError, ThreadTracer, Trace, Tracer,
 };
 use libc::{c_void, free, geteuid, malloc, size_t};
 use std::{fs::read_to_string, sync::Arc};
@@ -182,7 +182,7 @@ impl Trace for PerfTrace {
     #[cfg(ykpt)]
     fn iter_blocks(
         mut self: Box<Self>,
-    ) -> Box<dyn Iterator<Item = Result<Block, HWTracerError>> + Send> {
+    ) -> Box<dyn Iterator<Item = Result<Block, BlockIteratorError>> + Send> {
         // We hand ownership for self.buf over to `YkPTBlockIterator` so we need to make sure that
         // we don't try and free it.
         let buf = std::mem::replace(&mut self.buf, PerfTraceBuf(std::ptr::null_mut()));

hwtracer/src/pt/ykpt/mod.rs

@@ -44,7 +44,7 @@ use crate::{
     errors::{HWTracerError, TemporaryErrorKind},
     llvm_blockmap::{BlockMapEntry, SuccessorKind, LLVM_BLOCK_MAP},
     perf::collect::PerfTraceBuf,
-    Block,
+    Block, BlockIteratorError,
 };
 use intervaltree::IntervalTree;
 use std::{
@@ -233,12 +233,12 @@ impl YkPTBlockIterator<'_> {
 
     /// Convert a file offset to a virtual address.
     fn off_to_vaddr(&self, obj: &Path, off: u64) -> Result<usize, IteratorError> {
-        Ok(ykaddr::addr::off_to_vaddr(obj, off).unwrap())
+        ykaddr::addr::off_to_vaddr(obj, off).ok_or(IteratorError::NoSuchVAddr)
     }
 
     /// Convert a virtual address to a file offset.
     fn vaddr_to_off(&self, vaddr: usize) -> Result<(PathBuf, u64), IteratorError> {
-        Ok(ykaddr::addr::vaddr_to_obj_and_off(vaddr).unwrap())
+        ykaddr::addr::vaddr_to_obj_and_off(vaddr).ok_or(IteratorError::NoSuchVAddr)
     }
 
     /// Looks up the blockmap entry for the given offset in the "main object binary".
@@ -818,13 +818,14 @@ impl YkPTBlockIterator<'_> {
 }
 
 impl Iterator for YkPTBlockIterator<'_> {
-    type Item = Result<Block, HWTracerError>;
+    type Item = Result<Block, BlockIteratorError>;
 
     fn next(&mut self) -> Option<Self::Item> {
         match self.do_next() {
             Ok(b) => Some(Ok(b)),
             Err(IteratorError::NoMorePackets) => None,
-            Err(IteratorError::HWTracerError(e)) => Some(Err(e)),
+            Err(IteratorError::NoSuchVAddr) => Some(Err(BlockIteratorError::NoSuchVAddr)),
+            Err(IteratorError::HWTracerError(e)) => Some(Err(BlockIteratorError::HWTracerError(e))),
         }
     }
 }
@@ -844,6 +845,9 @@ impl Drop for YkPTBlockIterator<'_> {
 enum IteratorError {
     #[error("No more packets")]
     NoMorePackets,
+    #[cfg(ykpt)]
+    #[error("No such vaddr")]
+    NoSuchVAddr,
     #[error("HWTracerError: {0}")]
     HWTracerError(HWTracerError),
 }

tests/src/hwtracer_ykpt.rs

@@ -13,7 +13,7 @@
 //! To that end, the test files in `tests/hwtracer_ykpt` are compiled into test binaries (as a
 //! langtester suite) and then they call into this file to have assertions checked in Rust code.
 
-use hwtracer::{ThreadTracer, Trace, TracerBuilder};
+use hwtracer::{BlockIteratorError, ThreadTracer, Trace, TracerBuilder};
 use std::ffi::c_void;
 
 #[no_mangle]
@@ -52,7 +52,7 @@ pub extern "C" fn __hwykpt_decode_trace(trace: *mut Box<dyn Trace>) -> bool {
     for b in trace.iter_blocks() {
         match b {
             Ok(_) => (),
-            Err(HWTracerError::Temporary(_)) => return false,
+            Err(BlockIteratorError::HWTracerError(HWTracerError::Temporary(_))) => return false,
             Err(_) => panic!(),
         }
     }

ykrt/src/trace/hwt/mapper.rs

@@ -1,7 +1,10 @@
 //! The mapper translates a hwtracer trace into an IR trace.
 
 use crate::trace::{AOTTraceIterator, AOTTraceIteratorError, TraceAction, TraceRecorderError};
-use hwtracer::{llvm_blockmap::LLVM_BLOCK_MAP, Block, HWTracerError, TemporaryErrorKind, Trace};
+use hwtracer::{
+    llvm_blockmap::LLVM_BLOCK_MAP, Block, BlockIteratorError, HWTracerError, TemporaryErrorKind,
+    Trace,
+};
 use ykaddr::{
     addr::{vaddr_to_obj_and_off, vaddr_to_sym_and_obj},
     obj::SELF_BIN_PATH,
@@ -13,7 +16,7 @@ use ykaddr::{
 /// mapped LLVM IR block, or an unsuccessfully mapped "unmappable block" (an unknown region of
 /// code spanning at least one machine block).
 pub(crate) struct HWTTraceIterator {
-    hwt_iter: Box<dyn Iterator<Item = Result<Block, HWTracerError>> + Send>,
+    hwt_iter: Box<dyn Iterator<Item = Result<Block, BlockIteratorError>> + Send>,
     /// The next [TraceAction]`s we will produce when `next` is called. We need this intermediary
     /// to allow us to deduplicate mapped/unmapped basic blocks. This will be empty on the first
     /// iteration and from then on will always have at least one [TraceAction] in it at all times,
@@ -195,10 +198,12 @@ impl Iterator for HWTTraceIterator {
                         _ => panic!(),
                     }
                 }
-                Some(Err(HWTracerError::Temporary(TemporaryErrorKind::TraceBufferOverflow))) => {
+                Some(Err(BlockIteratorError::HWTracerError(HWTracerError::Temporary(
+                    TemporaryErrorKind::TraceBufferOverflow,
+                )))) => {
                     return Some(Err(AOTTraceIteratorError::TraceTooLong));
                 }
-                Some(Err(e)) => todo!("{e:?}"),
+                Some(Err(e)) => return Some(Err(AOTTraceIteratorError::Other(e.to_string()))),
                 None => return Some(Err(AOTTraceIteratorError::PrematureEnd)),
             }
             debug_assert!(self.tas_generated > 0);
@@ -212,15 +217,17 @@ impl Iterator for HWTTraceIterator {
                 Some(Ok(x)) => {
                     self.map_block(&x);
                 }
-                Some(Err(HWTracerError::Unrecoverable(x)))
+                Some(Err(BlockIteratorError::HWTracerError(HWTracerError::Unrecoverable(x))))
                     if x == "longjmp within traces currently unsupported" =>
                 {
                     return Some(Err(AOTTraceIteratorError::LongJmpEncountered));
                 }
-                Some(Err(HWTracerError::Temporary(TemporaryErrorKind::TraceBufferOverflow))) => {
+                Some(Err(BlockIteratorError::HWTracerError(HWTracerError::Temporary(
+                    TemporaryErrorKind::TraceBufferOverflow,
+                )))) => {
                     return Some(Err(AOTTraceIteratorError::TraceTooLong));
                 }
-                Some(Err(e)) => todo!("{e:?}"),
+                Some(Err(e)) => return Some(Err(AOTTraceIteratorError::Other(e.to_string()))),
                 None => {
                     // The last block should contains pointless unmappable code (the stop tracing call).
                     match self.upcoming.pop() {

ykrt/src/trace/mod.rs

@@ -96,6 +96,9 @@ pub(crate) enum AOTTraceIteratorError {
     #[error("longjmp encountered")]
     #[allow(dead_code)]
     LongJmpEncountered,
+    #[error("{0}")]
+    #[allow(dead_code)]
+    Other(String),
 }
 
 /// A processed item from a trace.