Panic in cargo crev verify --show-all

[cehteh]

cargo crev verify --show-all panics in presence of raw C string literals c"like this". Cargo crev will then hang when a tread paniced. The panic is not a direct bug in crev itself, it rather stems from the fact that some of its dependencies (geiger) still use syn 1.x which does not support the new (rust 1.77) C string literal syntax.

Question is how can this handled more graceful?

[jayvdb]

The error I see is

thread '<unnamed>' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/syn-1.0.109/src/lit.rs:1020:13:
Unrecognized literal: `c""`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[jayvdb]

confirming that geiger is using syn 1.0.109
https://github.com/geiger-rs/cargo-geiger/blob/master/geiger/Cargo.toml#L18

Upstream bug geiger-rs/cargo-geiger#517

Upstream PR geiger-rs/cargo-geiger#518 seems to be stalled

Development of geiger in general seems to be stalled.

[jayvdb]

It might be easier to replace geiger with an alternative.

https://github.com/cackle-rs/cackle/blob/main/src/unsafe_checker.rs does similar and doesnt look very complicated. ping @davidlattimore

https://github.com/daminals/unsafe-parser also looks interesting, but isnt published yet. ping @daminals

Another approach would be to make "geiger-count" an optional feature until they have fixed their problems.

[davidlattimore]

Cackle uses two forms of checking for unsafe. It passes -Funsafe-code to rustc when compiling the crate and it also tokenises the source of the crate looking for the unsafe keyword. Neither is 100% and even with the two being used in conjunction, I think it's still possible to use unsafe from code emitted by a proc-macro. I haven't looked at how cargo-geiger` is implemented, but presumably it has a similar limitation.

One downside to Cackle as a replacement is that it's currently ELF-only, so doesn't replace geiger for use on Mac and Windows. Although presumably most use is via CI, which is presumably mostly on Linux, so maybe that doesn't matter too much.

edit: But if you're just looking to copy the code that checks for unsafe from cackle and use it in cargo-crev, then the ELF-specific side of Cackle would be sidestepped.

[cehteh]

I think checking for 'unsafe' shouldn't even a be a part of crev. crev is about a trust infrastructure, it manages a harness for reviewing code but it shouldn't be the linting tool. More proper would be to add lints to clippy which check for unsafe code or require it to have a '// SAFETY:' comment and so on. Then crev may eventually have some structured comments like "has unsafe", "all unsafe code is audited and fine", "doubtful use of unsafe", ...

This comes down to that crev gives higher guarantees than that the code is just well written and (memory-)safe. Eventually it should assure that a crate does what it promises. This covers certain correctness assertions as well, like memory leaks, deadlocks, correctly validating input data and more.