Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare for stabilization of sanitizers #376

Merged
merged 11 commits into from
Aug 6, 2024
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use clap::Parser;
mod templates;
mod options;
mod project;
mod rustc_version;
mod utils;

static FUZZ_TARGETS_DIR_OLD: &str = "fuzzers";
Expand Down
44 changes: 34 additions & 10 deletions src/project.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::options::{self, BuildMode, BuildOptions, Sanitizer};
use crate::rustc_version::{is_nightly, rust_version_string, RustVersion};
use crate::utils::default_target;
use anyhow::{anyhow, bail, Context, Result};
use cargo_metadata::MetadataCommand;
use std::collections::HashSet;
use std::io::Read;
use std::io::Write;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::{
env, ffi, fs,
process::{Command, Stdio},
Expand Down Expand Up @@ -187,17 +189,39 @@ impl FuzzProject {
rustflags.push_str(" -Cinstrument-coverage");
}

match build.sanitizer {
Sanitizer::None => {}
Sanitizer::Memory => {
// Memory sanitizer requires more flags to function than others:
// https://doc.rust-lang.org/unstable-book/compiler-flags/sanitizer.html#memorysanitizer
rustflags.push_str(" -Zsanitizer=memory -Zsanitizer-memory-track-origins")
if !matches!(build.sanitizer, Sanitizer::None) {
// Select the appropriate sanitizer flag for the given rustc version:
// either -Zsanitizer or -Csanitizer.
let rust_version_string = rust_version_string()?;
let rust_version =
RustVersion::from_str(&rust_version_string).map_err(|e| anyhow::anyhow!(e))?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: seems like these two steps could be collapsed into a single, fallible RustVersion constructor:

impl RustVersion {
    pub fn discover() -> anyhow::Result<Self> {
        let version_string = rust_version_string()?;
        let me = Self::from_str(&version_string)?;
        Ok(me)
    }
}

No need to make API-consumers do two things when they could do one; just an opportunity for them to mess things up in between or not connect the two interfaces properly.

Copy link
Member Author

@Shnatsel Shnatsel Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason these are two separate steps is that we need the version string too, to detect whether the compiler is nightly or not.

I considered adding the version string to the RustVersion struct, but that would make it no longer Copy, and neither API looked obviously better than the other.

If you want me to encapsulate everything, I guess I could turn RustVersion into this:

pub struct RustVersion {
    pub major: u32,
    pub minor: u32,
    // we don't care about the patch version and it's a bit of a pain to parse
    pub nightly: bool,
}

and with the nightly flag being resolved in RustVersion we don't need to expose the string outside the module at all.

let sanitizer_flag = match rust_version.has_sanitizers_on_stable() {
true => "-Csanitizer",
false => "-Zsanitizer",
};

// Set rustc CLI arguments for the chosen sanitizer
match build.sanitizer {
Sanitizer::None => {} // needs no flags
Sanitizer::Memory => {
// Memory sanitizer requires more flags to function than others:
// https://doc.rust-lang.org/unstable-book/compiler-flags/sanitizer.html#memorysanitizer
rustflags.push_str(&format!(
" {sanitizer_flag}=memory -Zsanitizer-memory-track-origins"
))
}
_ => rustflags.push_str(&format!(" {sanitizer_flag}={}", build.sanitizer)),
}

// Not all sanitizers are stabilized on all platforms.
// It is infeasible to keep up this code to date with the list.
// So we just set `-Zunstable-options`` required for some sanitizers
// whenever we're on nightly on a recent enough compiler,
// and let the compiler show an error message
// if the user tries to enable a sanitizer not supported on their stable compiler.
if is_nightly(&rust_version_string) && rust_version.has_sanitizers_on_stable() {
rustflags.push_str(" -Zunstable-options")
}
_ => rustflags.push_str(&format!(
" -Zsanitizer={sanitizer}",
sanitizer = build.sanitizer
)),
}

if build.careful_mode {
Expand Down
129 changes: 129 additions & 0 deletions src/rustc_version.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
//! Rust compiler version detection

use std::{cmp::Ordering, process::Command, str::FromStr};

use anyhow::Context;

/// Checks if the compiler currently in use is nightly, or `RUSTC_BOOTSTRAP` is set to get nightly features on stable
pub fn is_nightly(version_string: &str) -> bool {
version_string.contains("-nightly ") || std::env::var_os("RUSTC_BOOTSTRAP").is_some()
}

/// Returns the output of `rustc --version`
pub fn rust_version_string() -> anyhow::Result<String> {
// The path to rustc can be specified via an environment variable:
// https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-reads
let rustc_path = std::env::var_os("RUSTC").unwrap_or("rustc".into());
let raw_output = Command::new(rustc_path)
.arg("--version")
.output()
.context("Failed to invoke rustc! Is it in your $PATH?")?
.stdout;
String::from_utf8(raw_output).context("`rustc --version` returned non-text output somehow")
}

#[derive(Debug, Copy, Clone, Eq, PartialEq, PartialOrd)]
pub struct RustVersion {
pub major: u32,
pub minor: u32,
// we don't care about the patch version and it's a bit of a pain to parse
}

impl Ord for RustVersion {
fn cmp(&self, other: &Self) -> Ordering {
match self.major.cmp(&other.major) {
Ordering::Equal => self.minor.cmp(&other.minor),
other => other,
}
}
}

impl FromStr for RustVersion {
type Err = &'static str;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s
.strip_prefix("rustc ")
.ok_or("Rust version string does not start with 'rustc'!")?;
let mut iter = s.split('.');
let major: u32 = iter
.next()
.ok_or("No major version found in `rustc --version` output!")?
.parse()
.map_err(|_| {
"Failed to parse major version in `rustc --version` output as a number!"
})?;
let minor: u32 = iter
.next()
.ok_or("No minor version found in `rustc --version` output!")?
.parse()
.map_err(|_| {
"Failed to parse minor version in `rustc --version` output as a number!"
})?;
Ok(RustVersion { major, minor })
}
}

/// Checks whether the compiler supports sanitizers on stable channel.
/// Such compilers (even nightly) do not support `-Zsanitizer` flag,
/// and require a different combination of flags even on nightly.
///
/// Stabilization PR with more info: <https://github.com/rust-lang/rust/pull/123617>
impl RustVersion {
pub fn has_sanitizers_on_stable(&self) -> bool {
// TODO: the release that stabilizes sanitizers is not currently known.
// This value is a PLACEHOLDER.
let release_that_stabilized_sanitizers = RustVersion {
major: 1,
minor: 85,
};
Shnatsel marked this conversation as resolved.
Show resolved Hide resolved
self >= &release_that_stabilized_sanitizers
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_parsing_stable() {
let version_string = "rustc 1.78.0 (9b00956e5 2024-04-29)";
let result = RustVersion::from_str(version_string).unwrap();
assert_eq!(
result,
RustVersion {
major: 1,
minor: 78
}
);
assert!(!is_nightly(version_string))
}

#[test]
fn test_parsing_nightly() {
let version_string = "rustc 1.81.0-nightly (d7f6ebace 2024-06-16)";
let result = RustVersion::from_str(version_string).unwrap();
assert_eq!(
result,
RustVersion {
major: 1,
minor: 81
}
);
assert!(is_nightly(version_string))
}

#[test]
fn test_parsing_future_stable() {
let version_string = "rustc 2.356.1 (deadfaced 2029-04-01)";
let result = RustVersion::from_str(version_string).unwrap();
assert_eq!(
result,
RustVersion {
major: 2,
minor: 356
}
);
assert!(!is_nightly(version_string))
}
}
Loading