diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d380ad2..9f18369a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +Unreleased +---------- +- Adjusted normalization logic to not fail overall operation on build ID + read failure + + 0.2.0-rc.0 ---------- - Added support for transparently following debug links in ELF binaries diff --git a/capi/include/blazesym.h b/capi/include/blazesym.h index b925d164..f377b890 100644 --- a/capi/include/blazesym.h +++ b/capi/include/blazesym.h @@ -305,6 +305,9 @@ typedef struct blaze_normalizer_opts { /** * Whether to read and report build IDs as part of the normalization * process. + * + * Note that build ID read failures will be swallowed without + * failing the normalization operation. */ bool build_ids; /** @@ -347,7 +350,7 @@ typedef struct blaze_user_meta_elf { */ size_t build_id_len; /** - * The optional build ID of the ELF file, if found. + * The optional build ID of the ELF file, if found and readable. */ uint8_t *build_id; /** diff --git a/capi/src/normalize.rs b/capi/src/normalize.rs index edf6fcac..06406f64 100644 --- a/capi/src/normalize.rs +++ b/capi/src/normalize.rs @@ -51,6 +51,9 @@ pub struct blaze_normalizer_opts { pub cache_maps: bool, /// Whether to read and report build IDs as part of the normalization /// process. + /// + /// Note that build ID read failures will be swallowed without + /// failing the normalization operation. pub build_ids: bool, /// Whether or not to cache build IDs. This flag only has an effect /// if build ID reading is enabled in the first place. @@ -292,7 +295,7 @@ pub struct blaze_user_meta_elf { pub path: *mut c_char, /// The length of the build ID, in bytes. pub build_id_len: usize, - /// The optional build ID of the ELF file, if found. + /// The optional build ID of the ELF file, if found and readable. pub build_id: *mut u8, /// Unused member available for future expansion. pub reserved: [u8; 8], diff --git a/src/normalize/buildid.rs b/src/normalize/buildid.rs index f55bb9ba..9e6a9b19 100644 --- a/src/normalize/buildid.rs +++ b/src/normalize/buildid.rs @@ -99,7 +99,22 @@ fn read_build_id_impl(parser: &ElfParser) -> Result> { } pub(super) trait BuildIdReader<'src> { - fn read_build_id(&self, path: &Path) -> Result>>; + fn read_build_id(&self, path: &Path) -> Option> { + #[cfg_attr( + feature = "tracing", + crate::log::instrument(err, skip_all, fields(path = ?path)) + )] + fn read_build_id<'src, B>(slf: &B, path: &Path) -> Result>> + where + B: BuildIdReader<'src> + ?Sized, + { + slf.read_build_id_fallible(path) + } + + read_build_id(self, path).ok().flatten() + } + + fn read_build_id_fallible(&self, path: &Path) -> Result>>; } @@ -107,8 +122,7 @@ pub(super) struct DefaultBuildIdReader; impl BuildIdReader<'_> for DefaultBuildIdReader { /// Attempt to read an ELF binary's build ID from a file. - #[cfg_attr(feature = "tracing", crate::log::instrument(skip(self)))] - fn read_build_id(&self, path: &Path) -> Result>> { + fn read_build_id_fallible(&self, path: &Path) -> Result>> { let parser = ElfParser::open(path)?; let buildid = read_build_id_impl(&parser)?.map(|buildid| Cow::Owned(buildid.to_vec())); Ok(buildid) @@ -130,8 +144,7 @@ impl<'cache> CachingBuildIdReader<'cache> { impl<'src> BuildIdReader<'src> for CachingBuildIdReader<'src> { /// Attempt to read an ELF binary's build ID from a file. - #[cfg_attr(feature = "tracing", crate::log::instrument(skip(self)))] - fn read_build_id(&self, path: &Path) -> Result>> { + fn read_build_id_fallible(&self, path: &Path) -> Result>> { let (file, cell) = self.cache.entry(path)?; let build_id = cell .get_or_try_init(|| { @@ -151,7 +164,7 @@ pub(super) struct NoBuildIdReader; impl BuildIdReader<'_> for NoBuildIdReader { #[inline] - fn read_build_id(&self, _path: &Path) -> Result>> { + fn read_build_id_fallible(&self, _path: &Path) -> Result>> { Ok(None) } } diff --git a/src/normalize/meta.rs b/src/normalize/meta.rs index f6183bdc..1e196e33 100644 --- a/src/normalize/meta.rs +++ b/src/normalize/meta.rs @@ -54,7 +54,7 @@ pub struct Apk { pub struct Elf<'src> { /// The canonical absolute path to the ELF file, including its name. pub path: PathBuf, - /// The ELF file's build ID, if available. + /// The ELF file's build ID, if available and readable. pub build_id: Option>, /// The struct is non-exhaustive and open to extension. #[doc(hidden)] diff --git a/src/normalize/normalizer.rs b/src/normalize/normalizer.rs index a614da0e..94f816f4 100644 --- a/src/normalize/normalizer.rs +++ b/src/normalize/normalizer.rs @@ -59,6 +59,9 @@ pub struct Builder { cache_maps: bool, /// Whether to read and report build IDs as part of the normalization /// process. + /// + /// Note that build ID read failures will be swallowed without + /// failing the normalization operation. build_ids: bool, /// Whether or not to cache build IDs. This flag only has an effect /// if build ID reading is enabled in the first place. @@ -142,6 +145,9 @@ pub struct Normalizer { cache_maps: bool, /// Flag indicating whether or not to read build IDs as part of the /// normalization process. + /// + /// Note that build ID read failures will be swallowed without + /// failing the normalization operation. build_ids: bool, /// Whether or not to cache build IDs. This flag only has an effect /// if build ID reading is enabled in the first place. diff --git a/src/normalize/user.rs b/src/normalize/user.rs index 50a2b817..d3f59ead 100644 --- a/src/normalize/user.rs +++ b/src/normalize/user.rs @@ -29,7 +29,7 @@ fn make_elf_meta<'src>( ) -> Result> { let elf = Elf { path: path.to_path_buf(), - build_id: build_id_reader.read_build_id(maps_file)?, + build_id: build_id_reader.read_build_id(maps_file), _non_exhaustive: (), }; let meta = UserMeta::Elf(elf); @@ -272,6 +272,8 @@ mod tests { use test_log::test; use crate::normalize::buildid::NoBuildIdReader; + use crate::BuildId; + use crate::Error; use crate::Pid; @@ -337,4 +339,39 @@ mod tests { test(0x7fffffff1001, Reason::Unmapped); test(0x7fffffffffff, Reason::Unmapped); } + + struct FailingBuildIdReader; + + impl BuildIdReader<'_> for FailingBuildIdReader { + #[inline] + fn read_build_id_fallible(&self, _path: &Path) -> Result>> { + Err(Error::with_unsupported("induced failure")) + } + } + + /// Check that errors when reading build IDs are handled gracefully. + #[test] + fn build_id_read_failures() { + let addrs = [build_id_read_failures as Addr]; + let map_files = false; + + let entries = maps::parse_filtered(Pid::Slf).unwrap(); + let reader = FailingBuildIdReader; + let mut handler = NormalizationHandler::new(&reader, addrs.len(), map_files); + let () = normalize_sorted_user_addrs_with_entries( + addrs.as_slice().iter().copied(), + entries, + &mut handler, + ) + .unwrap(); + + let normalized = handler.normalized; + assert_eq!(normalized.outputs.len(), 1); + assert_eq!(normalized.meta.len(), 1); + assert!( + normalized.meta[0].elf().is_some(), + "{:?}", + normalized.meta[0] + ); + } }