Correct hashtable parsing (#67)

* Correct hashtable parsing

After version 10 bitmap data was added into the hash table values, the
length of this wasn't being parsed leading to values parsed missing 64
bits at the end. This PR aims to fix this

* Changelog

* Fmt

* Add regression test

Author: xd009642

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.9.2]
+### Fixed
+- Parsing of hash tables with the bitmap data field
+
 ## [0.9.1]
 ### Fixed
 - Fix expression evaluation for split regions.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "llvm_profparser"
-version = "0.9.1"
+version = "0.9.2"
 authors = ["xd009642 <danielmckenna93@gmail.com>"]
 description = "Parsing and interpretation of llvm coverage profiles and generated data"
 repository = "https://github.com/xd009642/llvm-profparser"

src/hash_table.rs

@@ -93,34 +93,45 @@ fn read_value(
             break;
         }
 
-        // If the version is > v2 then there can also be value profiling data so lets try and parse
-        // that now
-        let (bytes, total_size) = le_u32(input)?;
-        if bytes.len() <= end_len {
-            break;
-        }
-        let (bytes, num_value_kinds) = le_u32(bytes)?;
-        // Here it's just less than because we don't need to read anything else so if it's equal to
-        // we're good
-        if bytes.len() < end_len {
-            break;
+        if version > 10 {
+            let (bytes, bitmap_bytes) = le_u64(input)?;
+            input = bytes;
+            for _i in 0..bitmap_bytes {
+                let (bytes, _) = le_u64(input)?;
+                input = bytes;
+            }
         }
-        input = bytes;
-        let value_prof_data = ValueProfData {
-            total_size,
-            num_value_kinds,
-        };
-        if value_prof_data.num_value_kinds > 0 && version > 2 {
-            // If we actually want to change data in future get result.last_mut() and change it
-            // there
-            break;
+        // This should always be true
+        if version > 2 {
+            // If the version is > v2 then there can also be value profiling data so lets try and parse
+            // that now
+            let (bytes, total_size) = le_u32(input)?;
+            if bytes.len() <= end_len {
+                break;
+            }
+            let (bytes, num_value_kinds) = le_u32(bytes)?;
+            // Here it's just less than because we don't need to read anything else so if it's equal to
+            // we're good
+            if bytes.len() < end_len {
+                break;
+            }
+            input = bytes;
+            let value_prof_data = ValueProfData {
+                total_size,
+                num_value_kinds,
+            };
+            if value_prof_data.num_value_kinds > 0 && version > 2 {
+                // If we actually want to change data in future get result.last_mut() and change it
+                // there
+                break;
+            }
         }
     }
     if result.is_empty() {
         result.push((last_hash, InstrProfRecord::default()));
     }
     input = expected_end;
-    assert_eq!(result.len(), 1);
+    assert!(!result.is_empty());
     Ok((input, result.remove(0)))
 }
 
@@ -142,6 +153,7 @@ impl HashTable {
         debug!("Number of hashtable buckets: {}", num_buckets);
         let (_bytes, mut num_entries) = le_u64(bytes)?;
         debug!("Number of entries: {}", num_entries);
+
         let mut payload = input;
         let mut result = Self::new();
         //TODO is this change right?

src/instrumentation_profile/mod.rs

@@ -44,7 +44,7 @@ pub fn parse_bytes(data: &[u8]) -> io::Result<InstrumentationProfile> {
         ));
     };
     nom_res.map(|(_bytes, res)| res).map_err(|e| {
-        // trace!("{}", e);
+        trace!("{}", e);
         let verbose_error_message = |err: VerboseError<&[u8]>| {
             err.errors
                 .iter()

tests/data/profdata/misc/ferrocene-library-aarch64-apple-darwin.profdata

@@ -491,3 +491,16 @@ fn check_raw_data_consistency() {
         std::assert_eq!(data_record, raw_record);
     }
 }
+
+// https://github.com/xd009642/llvm-profparser/issues/65 prevent regression on hash table parsing
+// changing
+#[test]
+fn hash_table_regression_check() {
+    let ferrocene = data_root_dir()
+        .join("misc")
+        .join("ferrocene-library-aarch64-apple-darwin.profdata");
+
+    // We've not locked this to version, but it's enough to parse it and making sure it parses
+    // correctly to prevent a regression.
+    parse(&ferrocene).unwrap();
+}