Features flags do not work as expected

[kardeiz]

If I put some unsafe usage behind a (not default) feature flag:

#[cfg(feature = "foo")]
pub mod foo {
    pub fn bar() -> &'static [i32] {
        let address = 0x01234usize;
        let r = address as *mut i32;
        unsafe { std::slice::from_raw_parts_mut(r, 10000) }
    }
}

And then run cargo geiger, it is showing use of an unsafe expression. Due to the presence of all-features and features flags for cargo-geiger, I would have expected this usage to be not be reported, unless I ran cargo-geiger with --all-features or --features foo. Am I missing something?

[pinkforest]

Interesting.

Thanks for reporting - I can reproduce that on latest release 0.11.4, 0.11.3 and it seems this has been there for a while

feature-sets seem to work only when they are both declared as such in Cargo.toml and not in-lined

I would have expected worst case only in-lined not to work but also requires declaration..

Will need to investigate and fix.

.... or would you like to work on a fix ? :)

[kardeiz]

@pinkforest Thanks for the response! Unfortunately I don't have the the time or familiarity with this project to work on a fix.

Also, I fear the "damage" is already done, since this is a binary that users may not update frequently. The context for my issue here is that I have the opportunity to make a meaningful (but not essential) optimization using unsafe for one of my libraries. I was interested in putting this optimization behind a feature flag, so that users could opt into it, but it would not necessarily be the default, and should not be flagged by tools such as this. I will probably still do so, but am now worried that users may be deterred unnecessarily.

[pinkforest]

I think the expectations needs to be clarified -

To me it sounds like cargo-geiger is not really the tool for the intended hypothetical target audience -

Users misunderstanding the intended use of tools is a big problem itself and we should not give wrong impression to them -

Geiger is not meant for end-users to make a direct Yes / No decision whether to use something or not.

This is why it's mentioned within very first lines in the readme that it should not be used this way -

The tool is intended to aid analysis - not replace it -

From geiger PoV - it is better to flag it as false positive rather than false negatives as geiger is solely intended to be used for pointing directions to investigate rather than giving plain yes / no decision wether something is insecure or not -

Cargo-geiger is solely intended to flag where somebody should look at with informed mind and not something end-user is supposed to look at - we've clarified this in the readme what this tool is for and what it's not for.

There is plenty of history around performance vs unsafe and it is not one tool is going to give direct answer once you drop the guarantees: https://github.com/rust-secure-code/safety-dance

You can also look at the advisory-db https://github.com/rustsec/advisory-db around "unsound" issues on how complicated this whole topic is - there is even working group around it.

There was some discussion around earlier that there could be a standardised feature flag to switch between speed optimisations and safety but that went nowhere as nobody I've seen really builds rust that way - that should have been where this problem should have been addressed - an analysis tool cannot solve this problem and replace human eye when it comes dealing with when safety guarantees are given away.

TL;DR So no - there is not going to be a tool that will tell users plain Yes / No whether they should use something.