Hex-encode defmt symbols to avoid compatibility issues.

[Dirbaio]

Hex-encode defmt symbols to avoid compatibility issues.

defmt currently puts json as-is in symbol names, which can contain special
characters not normally found in symbol names like quotes ", braces {}
and spaces .

This can cause compatibility issues with language features or tools that don't
expect this. For example it breaks sym in asm!.

This is a breaking change of the wire format, so this PR increases the
format numbre. defmt-decoder is updated to be able to decode the new format,
while keeping ability to decode older formats.

• decoder: improve version mismatch error message, don't mention probe-run.
• Hex-encode defmt symbols to avoid compatibility issues.

[Dirbaio]

• the cargo-semver-check ci error seems bogus? it's from a previous already-merged PR
• How to fix backcompat CI tests? it's expected that they fail, this is a wire format version bump.

[jonathanpallant]

I've pinged the team internally so we can work out how to fix those tests. We're on version 4 of the wire format already, so we must have a process for bumping it.

[jonathanpallant]

This magic string exists twice (once here, once in the macro), and it doesn't say a lot about the encoding used.

• Perhaps it should be __defmt_hex_, allowing for some future __defmt_base64_ or similar.
• Is there somewhere it can be defined once and imported in the other place? If not, we should document the canonical form in a third place (like the book) and have the other two places refer to it.

[Dirbaio]

Perhaps it should be _defmt_hex, allowing for some future _defmt_base64 or similar.

changed!

Is there somewhere it can be defined once and imported in the other place? If not, we should document the canonical form in a third place (like the book) and have the other two places refer to it.

there's no crate that's a shared dep of the macro and the decoder. I've added a section to the book.

[Urhengulas]

there's no crate that's a shared dep of the macro and the decoder. I've added a section to the book.

There is! It's the defmt-parser. I think it makes sense to put the magic string there.

[jonathanpallant]

Huh.

➜  defmt git:(main) cargo tree -p defmt-macros | grep parser
├── defmt-parser v0.3.4 (/Users/jonathan/Documents/knurling-rs/defmt/parser)

➜  defmt git:(main) cargo tree -p defmt-decoder | grep parser
├── defmt-parser v0.3.4 (/Users/jonathan/Documents/knurling-rs/defmt/parser)

[Urhengulas]

• the cargo-semver-check ci error seems bogus? it's from a previous already-merged PR

Yes, that's a known problem. I marked the PR as breaking change, which is not true, but skips the check.

* How to fix backcompat CI tests? it's expected that they fail, this is a wire format version bump.

Just bump the commit hash

defmt/xtask/src/backcompat.rs

Line 47 in 7d74e8b

const REVISION_UNDER_TEST: &str = "0e92d3a88aa472377b964979f522829d961d8986";

.

[jonathanpallant]

One nit, and one thought.

By hex encoding the symbols, you stop people from scanning the output of objdump -t and seeing the log strings, modules, etc.

Is there some other encoding which would solve the problem of invalid characters (", {, @, etc), but which retains the ability for a human to read the text? I'm thinking of something like C trigraphs perhaps. Or (shudder) C++/Rust symbol mangling.

[jonathanpallant]

Suggested change

	The symbol is prefixed with `__defmt_hex_` to denote it's is hex-encoded, and to allow for future expansion.
	The symbol is prefixed with `__defmt_hex_` to denote that it's hex-encoded, and to allow for future expansion.

[jonathanpallant]

We should probably fix the input section name at the same time (i.e. the link_section = "...."). Currently this is JSON, unless you are on macOS when we hex encode it. We should probably always hex encode it, or hash it, or use the disambiguator. I think we do still need to put somewhere there (i.e. don't throw them all into .defmt.error et al) so that the garbage collection can throw out symbols that aren't referenced (which happens at the input section level).

[jonathanpallant]

@Dirbaio did you have any thoughts on whether there's a more readable encoding than plain hex encoding here?

[jonathanpallant]

I gave it some thought. What about mangling::mangle

main.rs:

use std::io::Read;

fn main() {
    let mut stdin = std::io::stdin();
    let mut buffer = Vec::new();
    stdin.read_to_end(&mut buffer).expect("reading stdin");
    let result = mangling::mangle(buffer);
    println!("{}", result);
}

➜  mangler git:(master) ✗ echo '{ name: "Smith" }' | cargo run
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/debug/mangler`
_02_7b204name03_3a20225Smith04_22207d0a

[chrysn]

I just tried out this branch exploring how well defmt can be Just Another Dependency that libraries can impl trait for without too much feature gating ado. My go-to test for "will anything break" is having Python library crates compile for various platforms, and so far, adding defmt populates the content of -Wl,--version-script=/tmp/…/list with data that trips up a cc based linker. With this branch, it still trips at the same stage, but later in the file, where it says _defmt_encoding_ = rzcobs (or any of the other equals signs). So, would it make sense to apply the same encoding there? (It's a new wire format anyway, I'd guess that the absence of a _defmt_version_ = %d encoded wire format indicator works just as well to trip up old tools).

---

Unrelatedly: If we want something else than hex (IMO, why not hex), going the shuddering route of Rust mangling has the advantage that users who pipe their symbol names through a demangler will also see fully demangled defmt data.

[jonathanpallant]

Yes, good point, we'll need to Mangle All The Things.

pipe their symbol names through a demangler will also see fully demangled defmt data

I'm mentally uprating my idea from "bizarre" to "huh, actually..."