From aab2c2a55bca35cd7e6d3a8369d3404edbfd1311 Mon Sep 17 00:00:00 2001
From: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
Date: Mon, 19 Oct 2020 15:59:15 +0200
Subject: [PATCH 1/2] check defmt version before demangling symbols

otherwise `probe-run` prints a cryptic "Error: expected value at line 1 column 1" when it's not
compatible with `defmt` used by the application
---
 decoder/src/lib.rs   | 23 ++++++++++++++++-------
 elf2table/src/lib.rs | 31 +++++++++++++++++++++----------
 2 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs
index 52212f8e..952ab7e8 100644
--- a/decoder/src/lib.rs
+++ b/decoder/src/lib.rs
@@ -87,15 +87,24 @@ pub struct Table {
     entries: BTreeMap<usize, TableEntry>,
 }
 
+/// Checks if the version encoded in the symbol table is compatible with this version of the
+/// `decoder` crate
+pub fn check_version(version: &str) -> Result<(), String> {
+    if version != DEFMT_VERSION {
+        return Err(format!(
+            "defmt version mismatch (firmware is using {}, host is using {}); \
+             are you using the same git version of defmt and related tools?
+Try `cargo install`-ing the latest git version of `probe-run` AND updating your project's `Cargo.lock` with `cargo update`",
+            version, DEFMT_VERSION,
+        ));
+    }
+
+    Ok(())
+}
+
 impl Table {
     pub fn new(entries: BTreeMap<usize, TableEntry>, version: &str) -> Result<Self, String> {
-        if version != DEFMT_VERSION {
-            return Err(format!(
-                "defmt version mismatch (firmware is using {}, host is using {}); \
-                 are you using the same git version of defmt and related tools?",
-                version, DEFMT_VERSION,
-            ));
-        }
+        check_version(version)?;
 
         Ok(Self { entries })
     }
diff --git a/elf2table/src/lib.rs b/elf2table/src/lib.rs
index bdd67ff8..6a48c4c9 100644
--- a/elf2table/src/lib.rs
+++ b/elf2table/src/lib.rs
@@ -19,14 +19,7 @@ use object::{Object, ObjectSection};
 /// This function returns `None` if the ELF file contains no `.defmt` section
 pub fn parse(elf: &[u8]) -> Result<Option<Table>, anyhow::Error> {
     let elf = object::File::parse(elf)?;
-    // find the index of the `.defmt` section
-    let defmt_shndx = if let Some(section) = elf.section_by_name(".defmt") {
-        section.index()
-    } else {
-        return Ok(None);
-    };
-
-    let mut map = BTreeMap::new();
+    // first pass to extract the `_defmt_version`
     let mut version = None;
     for (_, entry) in elf.symbols() {
         let name = match entry.name() {
@@ -52,6 +45,26 @@ pub fn parse(elf: &[u8]) -> Result<Option<Table>, anyhow::Error> {
             }
             version = Some(new_version);
         }
+    }
+
+    let version = version.ok_or_else(|| anyhow!("defmt version symbol not found"))?;
+
+    defmt_decoder::check_version(version).map_err(anyhow::Error::msg)?;
+
+    // find the index of the `.defmt` section
+    let defmt_shndx = if let Some(section) = elf.section_by_name(".defmt") {
+        section.index()
+    } else {
+        return Ok(None);
+    };
+
+    // second pass to demangle symbols
+    let mut map = BTreeMap::new();
+    for (_, entry) in elf.symbols() {
+        let name = match entry.name() {
+            Some(name) => name,
+            None => continue,
+        };
 
         if entry.section_index() == Some(defmt_shndx) {
             let sym = symbol::Symbol::demangle(name)?;
@@ -67,8 +80,6 @@ pub fn parse(elf: &[u8]) -> Result<Option<Table>, anyhow::Error> {
         }
     }
 
-    let version = version.ok_or_else(|| anyhow!("defmt version symbol not found"))?;
-
     Table::new(map, version)
         .map_err(anyhow::Error::msg)
         .map(Some)

From 6805111a7682fa3c5b4150386c99fc850d268f4f Mon Sep 17 00:00:00 2001
From: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
Date: Mon, 19 Oct 2020 16:09:56 +0200
Subject: [PATCH 2/2] Table::new: remove internal version check

---
 decoder/src/lib.rs   | 8 ++++----
 elf2table/src/lib.rs | 4 +---
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs
index 952ab7e8..ca80ad26 100644
--- a/decoder/src/lib.rs
+++ b/decoder/src/lib.rs
@@ -103,10 +103,10 @@ Try `cargo install`-ing the latest git version of `probe-run` AND updating your
 }
 
 impl Table {
-    pub fn new(entries: BTreeMap<usize, TableEntry>, version: &str) -> Result<Self, String> {
-        check_version(version)?;
-
-        Ok(Self { entries })
+    /// NOTE caller must verify that defmt symbols are compatible with this version of the `decoder`
+    /// crate using the `check_version` function
+    pub fn new(entries: BTreeMap<usize, TableEntry>) -> Self {
+        Self { entries }
     }
 
     fn _get(&self, index: usize) -> Result<(Option<Level>, &str), ()> {
diff --git a/elf2table/src/lib.rs b/elf2table/src/lib.rs
index 6a48c4c9..7e48ea26 100644
--- a/elf2table/src/lib.rs
+++ b/elf2table/src/lib.rs
@@ -80,9 +80,7 @@ pub fn parse(elf: &[u8]) -> Result<Option<Table>, anyhow::Error> {
         }
     }
 
-    Table::new(map, version)
-        .map_err(anyhow::Error::msg)
-        .map(Some)
+    Ok(Some(Table::new(map)))
 }
 
 #[derive(Clone)]