Rework executable id generation (#143)

The executable is now based on the first 8 bytes of the build id, which
reduces the io and cpu needed to hash the code section from every
binary. This is now only done for executables without GNU or Go build
id. Besides the performance improvement, this will standardise how build
ids are handle once kernel code sections (main executable but also
modules) are processed.

Note: this is a backwards-incompatible change as the executable id will
change for GNU and Go binaries.

Test Plan
=========

Unit tests + ran the profiler for a while w/o issues

Author: javierhonduco

Cargo.lock

@@ -51,7 +51,7 @@ itertools = "0.14.0"
 lightswitch-metadata = { path = "lightswitch-metadata", version = "0.1.0" }
 lightswitch-proto = { path = "lightswitch-proto", version = "0.1.0" }
 lightswitch-capabilities = { path = "lightswitch-capabilities", version = "0.1.0" }
-lightswitch-object = { path = "lightswitch-object", version = "0.1.0" }
+lightswitch-object = { path = "lightswitch-object", version = "0.2.0" }
 memmap2 = { workspace = true }
 anyhow = { workspace = true }
 object = { workspace = true }

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "lightswitch-object"
-version = "0.1.1"
+version = "0.2.0"
 edition = "2021"
 description = "Deals with object files"
 license = "MIT"

lightswitch-object/Cargo.toml

@@ -1,55 +1,97 @@
 use std::fmt;
+use std::fmt::Debug;
 use std::fmt::Display;
 use std::fmt::Formatter;
 use std::str;
 
-use anyhow::Result;
 use data_encoding::HEXLOWER;
 use ring::digest::Digest;
 
+#[derive(Hash, Eq, PartialEq, Clone)]
+pub enum BuildIdFlavour {
+    Gnu,
+    Go,
+    Sha256,
+}
+
 /// Represents a build id, which could be either a GNU build ID, the build
 /// ID from Go, or a Sha256 hash of the code in the .text section.
-#[derive(Hash, Eq, PartialEq, Clone, Debug)]
-pub enum BuildId {
-    Gnu(String),
-    Go(String),
-    Sha256(String),
+#[derive(Hash, Eq, PartialEq, Clone)]
+pub struct BuildId {
+    pub flavour: BuildIdFlavour,
+    pub data: Vec<u8>,
 }
 
 impl BuildId {
     pub fn gnu_from_bytes(bytes: &[u8]) -> Self {
-        BuildId::Gnu(
-            bytes
-                .iter()
-                .map(|b| format!("{:02x}", b))
-                .collect::<Vec<_>>()
-                .join(""),
-        )
+        BuildId {
+            flavour: BuildIdFlavour::Gnu,
+            data: bytes.to_vec(),
+        }
     }
 
-    pub fn go_from_bytes(bytes: &[u8]) -> Result<Self> {
-        Ok(BuildId::Go(str::from_utf8(bytes)?.to_string()))
+    pub fn go_from_bytes(bytes: &[u8]) -> Self {
+        BuildId {
+            flavour: BuildIdFlavour::Go,
+            data: bytes.to_vec(),
+        }
     }
 
     pub fn sha256_from_digest(digest: &Digest) -> Self {
-        BuildId::Sha256(HEXLOWER.encode(digest.as_ref()))
+        BuildId {
+            flavour: BuildIdFlavour::Sha256,
+            data: digest.as_ref().to_vec(),
+        }
     }
-}
 
-impl Display for BuildId {
-    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        match self {
-            BuildId::Gnu(build_id) => {
-                write!(f, "gnu-{}", build_id)
-            }
-            BuildId::Go(build_id) => {
-                write!(f, "go-{}", build_id)
+    pub fn build_id_formatted(&self) -> String {
+        match self.flavour {
+            BuildIdFlavour::Gnu => {
+                self.data
+                    .iter()
+                    .fold(String::with_capacity(self.data.len() * 2), |mut res, el| {
+                        res.push_str(&format!("{:02x}", el));
+                        res
+                    })
             }
-            BuildId::Sha256(build_id) => {
-                write!(f, "sha256-{}", build_id)
+            BuildIdFlavour::Go => {
+                match str::from_utf8(&self.data) {
+                    Ok(res) => res.to_string(),
+                    // This should never happen in practice.
+                    Err(e) => format!("error converting go build id: {}", e),
+                }
             }
+            BuildIdFlavour::Sha256 => HEXLOWER.encode(self.data.as_ref()),
         }
     }
+
+    pub fn formatted(&self) -> String {
+        format!("{}-{}", self.flavour, self.build_id_formatted())
+    }
+}
+
+impl Display for BuildIdFlavour {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        let name = match self {
+            BuildIdFlavour::Gnu => "gnu",
+            BuildIdFlavour::Go => "go",
+            BuildIdFlavour::Sha256 => "sha256",
+        };
+
+        write!(f, "{}", name)
+    }
+}
+
+impl Display for BuildId {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        write!(f, "{}", self.formatted())
+    }
+}
+
+impl Debug for BuildId {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        write!(f, "BuildId({})", self.formatted())
+    }
 }
 
 #[cfg(test)]
@@ -58,15 +100,13 @@ mod tests {
     use ring::digest::{Context, SHA256};
 
     #[test]
-    fn test_buildid_constructors() {
+    fn test_buildid() {
         assert_eq!(
             BuildId::gnu_from_bytes(&[0xbe, 0xef, 0xca, 0xfe]).to_string(),
             "gnu-beefcafe"
         );
         assert_eq!(
-            BuildId::go_from_bytes("fake".as_bytes())
-                .unwrap()
-                .to_string(),
+            BuildId::go_from_bytes("fake".as_bytes()).to_string(),
             "go-fake"
         );
 
@@ -78,11 +118,4 @@ mod tests {
             "sha256-b80ad5b1508835ca2191ac800f4bb1a5ae1c3e47f13a8f5ed1b1593337ae5af5"
         );
     }
-
-    #[test]
-    fn test_buildid_display() {
-        assert_eq!(BuildId::Gnu("fake".into()).to_string(), "gnu-fake");
-        assert_eq!(BuildId::Go("fake".into()).to_string(), "go-fake");
-        assert_eq!(BuildId::Sha256("fake".into()).to_string(), "sha256-fake");
-    }
 }

lightswitch-object/src/buildid.rs

@@ -39,7 +39,7 @@ pub struct ObjectFile {
     /// before. Rust guarantees that fields are dropped in the order they are defined.
     object: object::File<'static>, // Its lifetime is tied to the `mmap` below.
     mmap: Box<Mmap>,
-    code_hash: Digest,
+    build_id: BuildId,
 }
 
 impl ObjectFile {
@@ -54,27 +54,28 @@ impl ObjectFile {
         // `object` by defining `object` before.
         let object =
             unsafe { std::mem::transmute::<object::File<'_>, object::File<'static>>(object) };
-        let Some(code_hash) = code_hash(&object) else {
-            return Err(anyhow!("code hash is None"));
-        };
+        let build_id = Self::read_build_id(&object)?;
+
         Ok(ObjectFile {
             object,
             mmap,
-            code_hash,
+            build_id,
         })
     }
 
-    /// Returns an identifier for the executable using the first 8 bytes of the Sha256 of the code section.
+    /// Returns an identifier for the executable using the first 8 bytes of the build id.
     pub fn id(&self) -> Result<ExecutableId> {
-        let mut buffer = [0; 8];
-        let _ = self.code_hash.as_ref().read(&mut buffer)?;
-        Ok(u64::from_ne_bytes(buffer))
+        Ok(u64::from_ne_bytes(self.build_id.data[..8].try_into()?))
+    }
+
+    /// Returns the executable build ID.
+    pub fn build_id(&self) -> &BuildId {
+        &self.build_id
     }
 
     /// Returns the executable build ID if present. If no GNU build ID and no Go build ID
     /// are found it returns the hash of the text section.
-    pub fn build_id(&self) -> Result<BuildId> {
-        let object = &self.object;
+    pub fn read_build_id(object: &object::File<'static>) -> Result<BuildId> {
         let gnu_build_id = object.build_id()?;
 
         if let Some(data) = gnu_build_id {
@@ -85,13 +86,16 @@ impl ObjectFile {
         for section in object.sections() {
             if section.name()? == ".note.go.buildid" {
                 if let Ok(data) = section.data() {
-                    return BuildId::go_from_bytes(data);
+                    return Ok(BuildId::go_from_bytes(data));
                 }
             }
         }
 
         // No build id (Rust, some compilers and Linux distributions).
-        Ok(BuildId::sha256_from_digest(&self.code_hash))
+        let Some(code_hash) = code_hash(object) else {
+            return Err(anyhow!("code hash is None"));
+        };
+        Ok(BuildId::sha256_from_digest(&code_hash))
     }
 
     /// Returns whether the object has debug symbols.

lightswitch-object/src/object.rs

@@ -1591,12 +1591,9 @@ impl Profiler {
                         return Err(anyhow!("Go applications are not supported yet"));
                     }
 
-                    let Ok(build_id) = object_file.build_id() else {
-                        continue;
-                    };
-
+                    let build_id = object_file.build_id();
                     let Ok(executable_id) = object_file.id() else {
-                        debug!("could not get id for object file: {}", abs_path);
+                        info!("could not get id for object file: {}", abs_path);
                         continue;
                     };
 
@@ -1637,7 +1634,7 @@ impl Profiler {
                         };
                         let res = self.debug_info_manager.add_if_not_present(
                             &name,
-                            &build_id,
+                            build_id,
                             executable_id,
                             &abs_path,
                         );
@@ -1701,10 +1698,6 @@ impl Profiler {
                             debug!("vDSO object file id failed");
                             continue;
                         };
-                        let Ok(build_id) = object_file.build_id() else {
-                            debug!("vDSO object file build_id failed");
-                            continue;
-                        };
                         let Ok(file) = std::fs::File::open(&vdso_path) else {
                             debug!("vDSO object file open failed");
                             continue;
@@ -1713,6 +1706,7 @@ impl Profiler {
                             debug!("vDSO elf_load_segments failed");
                             continue;
                         };
+                        let build_id = object_file.build_id().clone();
 
                         object_files.insert(
                             executable_id,