Rework the error hierarchy

Add BpfError::IO and BpfError::BtfError and remove the nested IO and
BtfError variants inside ParseError and RelocationError

src/bpf.rs

@@ -2,13 +2,15 @@ use std::{
     cell::{Ref, RefCell, RefMut},
     collections::HashMap,
     convert::TryFrom,
+    io,
 };
 
 use thiserror::Error;
 
 use crate::{
     maps::{Map, MapError},
-    obj::{BtfRelocationError, Object, ParseError, RelocationError},
+    obj::btf::RelocationError as BtfRelocationError,
+    obj::{btf::BtfError, Object, ParseError, RelocationError},
     programs::{KProbe, Program, ProgramData, ProgramError, SocketFilter, TracePoint, UProbe, Xdp},
     syscalls::bpf_map_update_elem_ptr,
 };
@@ -141,17 +143,24 @@ impl Bpf {
 
 #[derive(Debug, Error)]
 pub enum BpfError {
+    #[error("IO error: {0}")]
+    IO(#[from] io::Error),
+
     #[error("error parsing BPF object: {0}")]
     ParseError(#[from] ParseError),
+
+    #[error("BTF error: {0}")]
+    BtfError(#[from] BtfError),
+
     #[error("error relocating BPF object: {0}")]
     RelocationError(#[from] RelocationError),
-    #[error("BTF error: {error}")]
-    BtfRelocationError {
-        #[from]
-        error: BtfRelocationError,
-    },
+
+    #[error(transparent)]
+    BtfRelocationError(#[from] BtfRelocationError),
+
     #[error("map error: {0}")]
     MapError(#[from] MapError),
+
     #[error("program error: {0}")]
     ProgramError(#[from] ProgramError),
 }

src/maps/perf_map.rs

@@ -261,17 +261,11 @@ pub enum PerfMapError {
     #[error("invalid cpu {cpu_id}")]
     InvalidCpu { cpu_id: u32 },
 
-    #[error("map error: {map_error}")]
-    MapError {
-        #[from]
-        map_error: MapError,
-    },
+    #[error("map error: {0}")]
+    MapError(#[from] MapError),
 
-    #[error("perf buffer error: {buf_error}")]
-    PerfBufferError {
-        #[from]
-        buf_error: PerfBufferError,
-    },
+    #[error("perf buffer error: {0}")]
+    PerfBufferError(#[from] PerfBufferError),
 
     #[error("bpf_map_update_elem failed: {io_error}")]
     UpdateElementFailed {

src/obj/btf/relocation.rs

@@ -19,26 +19,18 @@ use crate::{
         },
         Btf, BtfError, Object, Program,
     },
+    BpfError,
 };
 
 #[derive(Error, Debug)]
 pub enum RelocationError {
-    #[error("{error}")]
-    BtfError {
-        #[from]
-        error: BtfError,
-    },
-
-    #[error("{error}")]
-    IOError {
-        #[from]
-        error: io::Error,
-    },
+    #[error(transparent)]
+    IOError(#[from] io::Error),
 
     #[error("section `{name}` not found")]
     SectionNotFound { name: String },
 
-    #[error("invalid BTF relocation access string {access_str}")]
+    #[error("invalid relocation access string {access_str}")]
     InvalidAccessString { access_str: String },
 
     #[error("invalid instruction index #{index} referenced by relocation #{relocation_number} in section `{section_name}`")]
@@ -161,7 +153,7 @@ impl Relocation {
 }
 
 impl Object {
-    pub fn relocate_btf(&mut self) -> Result<(), RelocationError> {
+    pub fn relocate_btf(&mut self) -> Result<(), BpfError> {
         let (local_btf, btf_ext) = match (&self.btf, &self.btf_ext) {
             (Some(btf), Some(btf_ext)) => (btf, btf_ext),
             _ => return Ok(()),
@@ -203,7 +195,7 @@ impl Object {
                         num_instructions: instructions.len(),
                         section_name: section_name.to_string(),
                         relocation_number: rel.number,
-                    });
+                    })?;
                 }
 
                 let local_ty = local_btf.type_by_id(rel.type_id)?;
@@ -262,7 +254,7 @@ impl Object {
                         return Err(RelocationError::ConflictingCandidates {
                             type_name: local_name.to_string(),
                             candidates: conflicts,
-                        });
+                        })?;
                     }
                     target_comp_rel
                 } else {
@@ -314,7 +306,7 @@ fn find_candidates<'target>(
 fn match_candidate<'target>(
     local_spec: &AccessSpec,
     candidate: &'target Candidate,
-) -> Result<Option<AccessSpec<'target>>, RelocationError> {
+) -> Result<Option<AccessSpec<'target>>, BpfError> {
     let mut target_spec = AccessSpec {
         btf: candidate.btf,
         root_type_id: candidate.type_id,
@@ -416,7 +408,7 @@ fn match_candidate<'target>(
                     if target_spec.parts.len() == MAX_SPEC_LEN {
                         return Err(RelocationError::MaximumNestingLevelReached {
                             type_name: Some(candidate.name.clone()),
-                        });
+                        })?;
                     }
 
                     target_spec.parts.push(accessor.index);
@@ -442,7 +434,7 @@ fn match_member<'local, 'target>(
     target_btf: &'target Btf,
     target_id: u32,
     target_spec: &mut AccessSpec<'target>,
-) -> Result<Option<u32>, RelocationError> {
+) -> Result<Option<u32>, BpfError> {
     let local_ty = local_btf.type_by_id(local_accessor.type_id)?;
     let local_member = match local_ty {
         BtfType::Struct(_, members) | BtfType::Union(_, members) => {
@@ -467,7 +459,7 @@ fn match_member<'local, 'target>(
             let root_ty = target_spec.btf.type_by_id(target_spec.root_type_id)?;
             return Err(RelocationError::MaximumNestingLevelReached {
                 type_name: target_spec.btf.err_type_name(root_ty),
-            });
+            })?;
         }
 
         let bit_offset = member_bit_offset(target_ty.info().unwrap(), target_member);
@@ -527,7 +519,7 @@ impl<'a> AccessSpec<'a> {
         root_type_id: u32,
         spec: &str,
         relocation: Relocation,
-    ) -> Result<AccessSpec<'a>, RelocationError> {
+    ) -> Result<AccessSpec<'a>, BpfError> {
         let parts = spec
             .split(":")
             .map(|s| s.parse::<usize>())
@@ -547,7 +539,7 @@ impl<'a> AccessSpec<'a> {
                 if parts != [0] {
                     return Err(RelocationError::InvalidAccessString {
                         access_str: spec.to_string(),
-                    });
+                    })?;
                 }
                 AccessSpec {
                     btf,
@@ -563,7 +555,7 @@ impl<'a> AccessSpec<'a> {
                     if parts.len() != 1 {
                         return Err(RelocationError::InvalidAccessString {
                             access_str: spec.to_string(),
-                        });
+                        })?;
                     }
                     let index = parts[0];
                     if index >= members.len() {
@@ -573,7 +565,7 @@ impl<'a> AccessSpec<'a> {
                             index: index,
                             max_index: members.len(),
                             error: "tried to access nonexistant enum variant".to_string(),
-                        });
+                        })?;
                     }
                     let accessors = vec![Accessor {
                         type_id,
@@ -596,7 +588,7 @@ impl<'a> AccessSpec<'a> {
                         relocation_kind: format!("{:?}", relocation.kind),
                         type_kind: format!("{:?}", ty.kind()?.unwrap()),
                         error: "enum relocation on non-enum type".to_string(),
-                    })
+                    })?
                 }
             },
 
@@ -626,7 +618,7 @@ impl<'a> AccessSpec<'a> {
                                     index: index,
                                     max_index: members.len(),
                                     error: "out of bounds struct or union access".to_string(),
-                                });
+                                })?;
                             }
 
                             let member = members[index];
@@ -662,7 +654,7 @@ impl<'a> AccessSpec<'a> {
                                     index,
                                     max_index: array.nelems as usize,
                                     error: "array index out of bounds".to_string(),
-                                });
+                                })?;
                             }
                             accessors.push(Accessor {
                                 type_id,
@@ -679,7 +671,7 @@ impl<'a> AccessSpec<'a> {
                                 type_kind: format!("{:?}", ty.kind()),
                                 error: "field relocation on a type that doesn't have fields"
                                     .to_string(),
-                            });
+                            })?;
                         }
                     };
                 }
@@ -732,7 +724,7 @@ impl ComputedRelocation {
         rel: &Relocation,
         local_spec: &AccessSpec,
         target_spec: Option<&AccessSpec>,
-    ) -> Result<ComputedRelocation, RelocationError> {
+    ) -> Result<ComputedRelocation, BpfError> {
         use RelocationKind::*;
         let ret = match rel.kind {
             FieldByteOffset | FieldByteSize | FieldExists | FieldSigned | FieldLShift64
@@ -760,7 +752,7 @@ impl ComputedRelocation {
         section_name: &str,
         local_btf: &Btf,
         target_btf: &Btf,
-    ) -> Result<(), RelocationError> {
+    ) -> Result<(), BpfError> {
         let instructions = &mut program.instructions;
         let num_instructions = instructions.len();
         let ins_index = rel.ins_offset as usize / std::mem::size_of::<bpf_insn>();
@@ -786,7 +778,7 @@ impl ComputedRelocation {
                         relocation_number: rel.number,
                         index: ins_index,
                         error: format!("invalid src_reg={:x} expected {:x}", src_reg, BPF_K),
-                    });
+                    })?;
                 }
 
                 ins.imm = target_value as i32;
@@ -797,7 +789,7 @@ impl ComputedRelocation {
                         relocation_number: rel.number,
                         index: ins_index,
                         error: format!("value `{}` overflows 16 bits offset field", target_value),
-                    });
+                    })?;
                 }
 
                 ins.off = target_value as i16;
@@ -822,7 +814,7 @@ impl ComputedRelocation {
                                     err_type_name(&target_btf.err_type_name(target_ty)),
                                     self.target.size,
                                 ),
-                            })
+                            })?
                         }
                     }
 
@@ -836,7 +828,7 @@ impl ComputedRelocation {
                                 relocation_number: rel.number,
                                 index: ins_index,
                                 error: format!("invalid target size {}", size),
-                            })
+                            })?
                         }
                     } as u8;
                     ins.code = ins.code & 0xE0 | size | ins.code & 0x07;
@@ -860,7 +852,7 @@ impl ComputedRelocation {
                     relocation_number: rel.number,
                     index: ins_index,
                     error: format!("invalid instruction class {:x}", class),
-                })
+                })?
             }
         };
 
@@ -870,7 +862,7 @@ impl ComputedRelocation {
     fn compute_enum_relocation(
         rel: &Relocation,
         spec: Option<&AccessSpec>,
-    ) -> Result<ComputedRelocationValue, RelocationError> {
+    ) -> Result<ComputedRelocationValue, BpfError> {
         use RelocationKind::*;
         let value = match rel.kind {
             EnumVariantExists => spec.is_some() as u32,
@@ -896,7 +888,7 @@ impl ComputedRelocation {
     fn compute_field_relocation(
         rel: &Relocation,
         spec: Option<&AccessSpec>,
-    ) -> Result<ComputedRelocationValue, RelocationError> {
+    ) -> Result<ComputedRelocationValue, BpfError> {
         use RelocationKind::*;
 
         if let FieldExists = rel.kind {
@@ -931,7 +923,7 @@ impl ComputedRelocation {
                         relocation_kind: format!("{:?}", rel_kind),
                         type_kind: format!("{:?}", ty.kind()),
                         error: "invalid relocation kind for array type".to_string(),
-                    });
+                    })?;
                 }
             };
         }
@@ -947,7 +939,7 @@ impl ComputedRelocation {
                     relocation_kind: format!("{:?}", rel.kind),
                     type_kind: format!("{:?}", ty.kind()),
                     error: "field relocation on a type that doesn't have fields".to_string(),
-                });
+                })?;
             }
         };
 
@@ -1021,7 +1013,7 @@ impl ComputedRelocation {
         rel: &Relocation,
         local_spec: &AccessSpec,
         target_spec: Option<&AccessSpec>,
-    ) -> Result<ComputedRelocationValue, RelocationError> {
+    ) -> Result<ComputedRelocationValue, BpfError> {
         use RelocationKind::*;
         let value = match rel.kind {
             TypeIdLocal => local_spec.root_type_id,

src/obj/mod.rs

@@ -1,4 +1,4 @@
-mod btf;
+pub(crate) mod btf;
 mod relocation;
 
 use object::{
@@ -14,13 +14,13 @@ use std::{
 };
 use thiserror::Error;
 
-pub use btf::RelocationError as BtfRelocationError;
 pub use relocation::*;
 
 use crate::{
     bpf_map_def,
     generated::{bpf_insn, bpf_map_type::BPF_MAP_TYPE_ARRAY},
     obj::btf::{Btf, BtfError, BtfExt},
+    BpfError,
 };
 
 const KERNEL_VERSION_ANY: u32 = 0xFFFF_FFFE;
@@ -85,8 +85,8 @@ impl FromStr for ProgramKind {
 }
 
 impl Object {
-    pub(crate) fn parse(data: &[u8]) -> Result<Object, ParseError> {
-        let obj = object::read::File::parse(data).map_err(|source| ParseError::Error { source })?;
+    pub(crate) fn parse(data: &[u8]) -> Result<Object, BpfError> {
+        let obj = object::read::File::parse(data).map_err(|e| ParseError::ElfError(e))?;
         let endianness = obj.endianness();
 
         let section = obj
@@ -138,9 +138,7 @@ impl Object {
     fn parse_program(&self, section: &Section, ty: &str) -> Result<Program, ParseError> {
         let num_instructions = section.data.len() / mem::size_of::<bpf_insn>();
         if section.data.len() % mem::size_of::<bpf_insn>() > 0 {
-            return Err(ParseError::InvalidProgramCode {
-                name: section.name.to_owned(),
-            });
+            return Err(ParseError::InvalidProgramCode);
         }
         let instructions = (0..num_instructions)
             .map(|i| unsafe {
@@ -171,7 +169,7 @@ impl Object {
         Ok(())
     }
 
-    fn parse_section(&mut self, section: Section) -> Result<(), ParseError> {
+    fn parse_section(&mut self, section: Section) -> Result<(), BpfError> {
         let parts = section.name.split("/").collect::<Vec<_>>();
 
         match parts.as_slice() {
@@ -209,13 +207,7 @@ impl Object {
 #[derive(Debug, Clone, Error)]
 pub enum ParseError {
     #[error("error parsing ELF data")]
-    Error {
-        #[source]
-        source: object::read::Error,
-    },
-
-    #[error("Error parsing BTF: {0}")]
-    BTF(#[from] BtfError),
+    ElfError(#[from] object::read::Error),
 
     #[error("no license specified")]
     MissingLicense,
@@ -245,8 +237,8 @@ pub enum ParseError {
     #[error("invalid program kind `{kind}`")]
     InvalidProgramKind { kind: String },
 
-    #[error("error parsing program `{name}`")]
-    InvalidProgramCode { name: String },
+    #[error("invalid program code")]
+    InvalidProgramCode,
 
     #[error("error parsing map `{name}`")]
     InvalidMapDefinition { name: String },
@@ -410,7 +402,7 @@ mod tests {
     fn test_parse_generic_error() {
         assert!(matches!(
             Object::parse(&b"foo"[..]),
-            Err(ParseError::Error { .. })
+            Err(BpfError::ParseError(ParseError::ElfError(_)))
         ))
     }
 
@@ -570,14 +562,8 @@ mod tests {
         let obj = fake_obj();
 
         assert_matches!(
-            obj.parse_program(
-                &fake_section(
-                    "kprobe/foo",
-                    &42u32.to_ne_bytes(),
-                ),
-                "kprobe"
-            ),
-            Err(ParseError::InvalidProgramCode { name }) if name == "kprobe/foo"
+            obj.parse_program(&fake_section("kprobe/foo", &42u32.to_ne_bytes(),), "kprobe"),
+            Err(ParseError::InvalidProgramCode)
         );
     }
 

src/obj/relocation.rs

@@ -34,12 +34,6 @@ pub enum RelocationError {
         num_instructions: usize,
         relocation_number: usize,
     },
-
-    #[error("IO error: {io_error}")]
-    IO {
-        #[from]
-        io_error: io::Error,
-    },
 }
 
 #[derive(Debug, Copy, Clone)]