From 02471f053fe1dc6f5c870821cf74a3d124acb4f4 Mon Sep 17 00:00:00 2001 From: cwshugg Date: Mon, 7 Oct 2024 14:51:55 -0400 Subject: [PATCH 1/8] implemented the exposing of environment variables during build-time ('exec_build()') for dependencies to detect when cargo-fuzz is running --- src/project.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/src/project.rs b/src/project.rs index c62bfe9..3ee8295 100644 --- a/src/project.rs +++ b/src/project.rs @@ -15,6 +15,14 @@ use std::{ const DEFAULT_FUZZ_DIR: &str = "fuzz"; +/// The name of the environment variable that is exposed to indicate a +/// cargo-fuzz build is occurring. +const BUILD_ENV_CARGO_FUZZ: &str = "CARGO_FUZZ"; + +/// The name of the environment variable that exposes the cargo fuzz manifest +/// path during builds. +const BUILD_ENV_MANIFEST_DIR: &str = "CARGO_FUZZ_MANIFEST"; + pub struct FuzzProject { /// The project with fuzz targets fuzz_dir: PathBuf, @@ -234,8 +242,27 @@ impl FuzzProject { rustflags.push_str(" -Cdebug-assertions"); } if build.triple.contains("-msvc") { - // The entrypoint is in the bundled libfuzzer rlib, this gets the linker to find it. + // The entrypoint is in the bundled libfuzzer rlib, this gets the + // linker to find it. rustflags.push_str(" -Clink-arg=/include:main"); + + // NOTE: On Windows, if the user's fuzzing targets have a dependency + // on a local Rust DLL (with `crate-type` containing `["cdylib"]), + // the MSVC Linker will be unable to resolve the `main` symbol when + // linking the DLL. It will fail with this error: + // + // LINK : error LNK2001: unresolved external symbol main + // C:\....\depedency.dll : fatal error LNK1120: 1 unresolved externals + // + // We cannot remove the above argument from the rustc args *only* + // for the cdylib dependencies, so this fix will have to be on the + // user's side. To fix this, the user should add the following MSVC + // linker argument to their DLL's build script (`build.rs`): + // + // println!("cargo:rustc-link-arg=/force:unresolved"); + // + // See here for information on the `/force` MSVC linker argument: + // https://learn.microsoft.com/en-us/cpp/build/reference/force-force-file-output } // If release mode is enabled then we force 1 CGU to be used in rustc. @@ -319,6 +346,39 @@ impl FuzzProject { Ok(None) } } + + // Helper function for `exec_build()` used to expose cargo-fuzz information + // via environment variables. Such environment variables can be used by fuzz + // target dependencies' build scripts to detect whether or not cargo-fuzz is + // responsible for the build. + // + // This is called directly before the `cargo build ...` command is executed. + fn build_env_expose(&self, + _mode: options::BuildMode, + _build: &options::BuildOptions, + _fuzz_target: Option<&str> + ) -> Result<()> { + // expose a boolean-like environment variable to allow the detection of + // cargo-fuzz + env::set_var(BUILD_ENV_CARGO_FUZZ, "1"); + + // expose the path to the cargo-fuzz manifest: + let manifest_path = self.manifest_path(); + env::set_var(BUILD_ENV_MANIFEST_DIR, manifest_path.as_os_str()); + + Ok(()) + } + + // Helper function for `exe_build()` used to un-expose cargo-fuzz + // information that was previously exposed in environment variables during + // `build_env_expose()`. + // + // This is called directly after the `cargo build ...` command is executed. + fn build_env_unexpose(&self) -> Result<()> { + env::remove_var(BUILD_ENV_CARGO_FUZZ); + env::remove_var(BUILD_ENV_MANIFEST_DIR); + Ok(()) + } pub fn exec_build( &self, @@ -341,6 +401,12 @@ impl FuzzProject { if let Some(target_dir) = self.target_dir(&build)? { cmd.arg("--target-dir").arg(target_dir); } + + // expose build information via environment variables, before executing + // the build command + self.build_env_expose(mode, build, fuzz_target).expect( + "Failed to set cargo-fuzz build environment variables." + ); let status = cmd .status() @@ -348,6 +414,11 @@ impl FuzzProject { if !status.success() { bail!("failed to build fuzz script: {:?}", cmd); } + + // un-expose build information, after the command has finished + self.build_env_unexpose().expect( + "Failed to un-set cargo-fuzz build environment variables." + ); Ok(()) } From 7385662f589d5e44c5f52f70ff4b2a99bc4cdb1e Mon Sep 17 00:00:00 2001 From: cwshugg Date: Mon, 7 Oct 2024 14:57:52 -0400 Subject: [PATCH 2/8] minor updates to env var name and a typo --- src/project.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/project.rs b/src/project.rs index 3ee8295..19cc8df 100644 --- a/src/project.rs +++ b/src/project.rs @@ -17,7 +17,7 @@ const DEFAULT_FUZZ_DIR: &str = "fuzz"; /// The name of the environment variable that is exposed to indicate a /// cargo-fuzz build is occurring. -const BUILD_ENV_CARGO_FUZZ: &str = "CARGO_FUZZ"; +const BUILD_ENV_FLAG: &str = "CARGO_FUZZ"; /// The name of the environment variable that exposes the cargo fuzz manifest /// path during builds. @@ -358,24 +358,23 @@ impl FuzzProject { _build: &options::BuildOptions, _fuzz_target: Option<&str> ) -> Result<()> { - // expose a boolean-like environment variable to allow the detection of - // cargo-fuzz - env::set_var(BUILD_ENV_CARGO_FUZZ, "1"); + // expose a flag environment variable to allow the detection of cargo-fuzz + env::set_var(BUILD_ENV_FLAG, "1"); - // expose the path to the cargo-fuzz manifest: + // expose the path to the cargo-fuzz manifest let manifest_path = self.manifest_path(); env::set_var(BUILD_ENV_MANIFEST_DIR, manifest_path.as_os_str()); Ok(()) } - // Helper function for `exe_build()` used to un-expose cargo-fuzz + // Helper function for `exec_build()` used to un-expose cargo-fuzz // information that was previously exposed in environment variables during // `build_env_expose()`. // // This is called directly after the `cargo build ...` command is executed. fn build_env_unexpose(&self) -> Result<()> { - env::remove_var(BUILD_ENV_CARGO_FUZZ); + env::remove_var(BUILD_ENV_FLAG); env::remove_var(BUILD_ENV_MANIFEST_DIR); Ok(()) } From e2b684bbc1ba54c807941a8f307da62729477ed1 Mon Sep 17 00:00:00 2001 From: cwshugg Date: Mon, 7 Oct 2024 15:53:52 -0400 Subject: [PATCH 3/8] softened the language of my note on the MSVC linker error --- src/project.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/project.rs b/src/project.rs index 19cc8df..80b3eb0 100644 --- a/src/project.rs +++ b/src/project.rs @@ -248,8 +248,8 @@ impl FuzzProject { // NOTE: On Windows, if the user's fuzzing targets have a dependency // on a local Rust DLL (with `crate-type` containing `["cdylib"]), - // the MSVC Linker will be unable to resolve the `main` symbol when - // linking the DLL. It will fail with this error: + // the MSVC Linker may be unable to resolve the `main` symbol when + // linking the DLL. It may fail with this error: // // LINK : error LNK2001: unresolved external symbol main // C:\....\depedency.dll : fatal error LNK1120: 1 unresolved externals From 30bac16b9eb64592db2e9c349191e4006f27f8ff Mon Sep 17 00:00:00 2001 From: cwshugg Date: Tue, 15 Oct 2024 09:24:22 -0400 Subject: [PATCH 4/8] added new '--no-include-main-msvc' command-line build option, which allows for Windows DLLs to be built *without* the '/include:main' linker argument. This, plus a few other tricks, allows for Windows DLLs to be built for fuzzing. --- src/options.rs | 14 ++++++++++++++ src/project.rs | 36 ++++++++++++++++-------------------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/options.rs b/src/options.rs index 677b0e7..aa2d287 100644 --- a/src/options.rs +++ b/src/options.rs @@ -165,6 +165,20 @@ pub struct BuildOptions { /// and the fuzzer can store an input to the corpus at each condition that it passes; /// giving it a better chance of producing an input that reaches `res = 2;`. pub disable_branch_folding: Option, + + #[arg(long)] + /// Disable the inclusion of the `/include:main` MSVC linker argument + /// + /// The purpose of `/include:main` is to force the MSVC linker to include an + /// external reference to the symbol `main`, such that fuzzing targets built + /// on Windows are able to find LibFuzzer's `main` function. + /// + /// In certain corner cases, users may prefer to *not* build with this + /// argument. One such example: if a user is intending to build and fuzz a + /// Windows DLL, they would likely choose to enable this flag, to prevent + /// the DLL from having an extern `main` reference added to it. (DLLs/shared + /// libraries should not have any reference to `main`.) + pub no_include_main_msvc: bool, } impl stdfmt::Display for BuildOptions { diff --git a/src/project.rs b/src/project.rs index 80b3eb0..936de97 100644 --- a/src/project.rs +++ b/src/project.rs @@ -241,28 +241,24 @@ impl FuzzProject { if !build.release || build.debug_assertions || build.careful_mode { rustflags.push_str(" -Cdebug-assertions"); } - if build.triple.contains("-msvc") { - // The entrypoint is in the bundled libfuzzer rlib, this gets the - // linker to find it. - rustflags.push_str(" -Clink-arg=/include:main"); - - // NOTE: On Windows, if the user's fuzzing targets have a dependency - // on a local Rust DLL (with `crate-type` containing `["cdylib"]), - // the MSVC Linker may be unable to resolve the `main` symbol when - // linking the DLL. It may fail with this error: - // - // LINK : error LNK2001: unresolved external symbol main - // C:\....\depedency.dll : fatal error LNK1120: 1 unresolved externals + if build.triple.contains("-msvc") && !build.no_include_main_msvc { + // This forces the MSVC linker (which runs on Windows systems) to + // find the entry point (i.e. the `main` function) within the + // LibFuzzer `.rlib` file produced during the build. // - // We cannot remove the above argument from the rustc args *only* - // for the cdylib dependencies, so this fix will have to be on the - // user's side. To fix this, the user should add the following MSVC - // linker argument to their DLL's build script (`build.rs`): + // The `--no-include-main-msvc` argument disables the addition of + // this linker argument. In certain situations, a user may not want + // this argument included as part of the MSVC invocation. // - // println!("cargo:rustc-link-arg=/force:unresolved"); - // - // See here for information on the `/force` MSVC linker argument: - // https://learn.microsoft.com/en-us/cpp/build/reference/force-force-file-output + // For example, if the user is attempting to build and fuzz a + // Windows DLL (shared library), adding `/include:main` will force + // the DLL to compile with an external reference to `main`. + // DLLs/shared libraries are designed to be built as a separate + // object file, intentionally left *without* knowledge of the entry + // point. So, by forcing a DLL to include `main` will cause linking + // to fail. Using `--no-include-main-msvc` will allow the DLL to be + // built without issue. + rustflags.push_str(" -Clink-arg=/include:main"); } // If release mode is enabled then we force 1 CGU to be used in rustc. From 2e88c5fb376fcec4979a139f8cf829f7bad52a53 Mon Sep 17 00:00:00 2001 From: cwshugg Date: Tue, 15 Oct 2024 11:41:49 -0400 Subject: [PATCH 5/8] fixed up formatting of new code --- src/project.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/project.rs b/src/project.rs index 936de97..e903a5c 100644 --- a/src/project.rs +++ b/src/project.rs @@ -342,17 +342,18 @@ impl FuzzProject { Ok(None) } } - + // Helper function for `exec_build()` used to expose cargo-fuzz information // via environment variables. Such environment variables can be used by fuzz // target dependencies' build scripts to detect whether or not cargo-fuzz is // responsible for the build. // // This is called directly before the `cargo build ...` command is executed. - fn build_env_expose(&self, - _mode: options::BuildMode, - _build: &options::BuildOptions, - _fuzz_target: Option<&str> + fn build_env_expose( + &self, + _mode: options::BuildMode, + _build: &options::BuildOptions, + _fuzz_target: Option<&str>, ) -> Result<()> { // expose a flag environment variable to allow the detection of cargo-fuzz env::set_var(BUILD_ENV_FLAG, "1"); @@ -363,7 +364,7 @@ impl FuzzProject { Ok(()) } - + // Helper function for `exec_build()` used to un-expose cargo-fuzz // information that was previously exposed in environment variables during // `build_env_expose()`. @@ -396,12 +397,11 @@ impl FuzzProject { if let Some(target_dir) = self.target_dir(&build)? { cmd.arg("--target-dir").arg(target_dir); } - + // expose build information via environment variables, before executing // the build command - self.build_env_expose(mode, build, fuzz_target).expect( - "Failed to set cargo-fuzz build environment variables." - ); + self.build_env_expose(mode, build, fuzz_target) + .expect("Failed to set cargo-fuzz build environment variables."); let status = cmd .status() @@ -409,11 +409,10 @@ impl FuzzProject { if !status.success() { bail!("failed to build fuzz script: {:?}", cmd); } - + // un-expose build information, after the command has finished - self.build_env_unexpose().expect( - "Failed to un-set cargo-fuzz build environment variables." - ); + self.build_env_unexpose() + .expect("Failed to un-set cargo-fuzz build environment variables."); Ok(()) } From 1fe865769ffb7af03efef61447f5f196b348451c Mon Sep 17 00:00:00 2001 From: cwshugg Date: Wed, 16 Oct 2024 13:57:06 -0400 Subject: [PATCH 6/8] removed no_include_main_msvc argument; separated this feature into a separate PR --- src/options.rs | 14 -------------- src/project.rs | 19 ++----------------- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/src/options.rs b/src/options.rs index aa2d287..677b0e7 100644 --- a/src/options.rs +++ b/src/options.rs @@ -165,20 +165,6 @@ pub struct BuildOptions { /// and the fuzzer can store an input to the corpus at each condition that it passes; /// giving it a better chance of producing an input that reaches `res = 2;`. pub disable_branch_folding: Option, - - #[arg(long)] - /// Disable the inclusion of the `/include:main` MSVC linker argument - /// - /// The purpose of `/include:main` is to force the MSVC linker to include an - /// external reference to the symbol `main`, such that fuzzing targets built - /// on Windows are able to find LibFuzzer's `main` function. - /// - /// In certain corner cases, users may prefer to *not* build with this - /// argument. One such example: if a user is intending to build and fuzz a - /// Windows DLL, they would likely choose to enable this flag, to prevent - /// the DLL from having an extern `main` reference added to it. (DLLs/shared - /// libraries should not have any reference to `main`.) - pub no_include_main_msvc: bool, } impl stdfmt::Display for BuildOptions { diff --git a/src/project.rs b/src/project.rs index e903a5c..51d4cea 100644 --- a/src/project.rs +++ b/src/project.rs @@ -241,23 +241,8 @@ impl FuzzProject { if !build.release || build.debug_assertions || build.careful_mode { rustflags.push_str(" -Cdebug-assertions"); } - if build.triple.contains("-msvc") && !build.no_include_main_msvc { - // This forces the MSVC linker (which runs on Windows systems) to - // find the entry point (i.e. the `main` function) within the - // LibFuzzer `.rlib` file produced during the build. - // - // The `--no-include-main-msvc` argument disables the addition of - // this linker argument. In certain situations, a user may not want - // this argument included as part of the MSVC invocation. - // - // For example, if the user is attempting to build and fuzz a - // Windows DLL (shared library), adding `/include:main` will force - // the DLL to compile with an external reference to `main`. - // DLLs/shared libraries are designed to be built as a separate - // object file, intentionally left *without* knowledge of the entry - // point. So, by forcing a DLL to include `main` will cause linking - // to fail. Using `--no-include-main-msvc` will allow the DLL to be - // built without issue. + if build.triple.contains("-msvc") { + // The entrypoint is in the bundled libfuzzer rlib, this gets the linker to find it. rustflags.push_str(" -Clink-arg=/include:main"); } From df47fffd48002df342388939dd4b5ad447ce602c Mon Sep 17 00:00:00 2001 From: cwshugg Date: Wed, 16 Oct 2024 13:58:33 -0400 Subject: [PATCH 7/8] minor comment typo fix --- src/project.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project.rs b/src/project.rs index 51d4cea..5698463 100644 --- a/src/project.rs +++ b/src/project.rs @@ -242,7 +242,7 @@ impl FuzzProject { rustflags.push_str(" -Cdebug-assertions"); } if build.triple.contains("-msvc") { - // The entrypoint is in the bundled libfuzzer rlib, this gets the linker to find it. + // The entrypoint is in the bundled libfuzzer rlib, this gets the linker to find it. rustflags.push_str(" -Clink-arg=/include:main"); } From 5a009301b8b475e8a0ab74dab662791f012a1b14 Mon Sep 17 00:00:00 2001 From: cwshugg Date: Wed, 23 Oct 2024 11:09:03 -0400 Subject: [PATCH 8/8] removed 'CARGO_FUZZ' env var and switched to use Command::env() to set up subprocess environment. Also, changed function name to better indicate it's role as a helper function --- src/project.rs | 38 ++++++++++---------------------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/src/project.rs b/src/project.rs index 5698463..4f09cb4 100644 --- a/src/project.rs +++ b/src/project.rs @@ -15,10 +15,6 @@ use std::{ const DEFAULT_FUZZ_DIR: &str = "fuzz"; -/// The name of the environment variable that is exposed to indicate a -/// cargo-fuzz build is occurring. -const BUILD_ENV_FLAG: &str = "CARGO_FUZZ"; - /// The name of the environment variable that exposes the cargo fuzz manifest /// path during builds. const BUILD_ENV_MANIFEST_DIR: &str = "CARGO_FUZZ_MANIFEST"; @@ -330,37 +326,27 @@ impl FuzzProject { // Helper function for `exec_build()` used to expose cargo-fuzz information // via environment variables. Such environment variables can be used by fuzz - // target dependencies' build scripts to detect whether or not cargo-fuzz is - // responsible for the build. + // target dependencies' build scripts to adjust its build settings based on + // these variables' values. // // This is called directly before the `cargo build ...` command is executed. - fn build_env_expose( + // The command is passed in as a mutable reference such that the subprocess' + // environment (not *this* process' environment) will have the environment + // variables set. + fn exec_build_expose_env( &self, + cmd: &mut Command, _mode: options::BuildMode, _build: &options::BuildOptions, _fuzz_target: Option<&str>, ) -> Result<()> { - // expose a flag environment variable to allow the detection of cargo-fuzz - env::set_var(BUILD_ENV_FLAG, "1"); - - // expose the path to the cargo-fuzz manifest + // expose the path to the cargo-fuzz manifest to the subprocess let manifest_path = self.manifest_path(); - env::set_var(BUILD_ENV_MANIFEST_DIR, manifest_path.as_os_str()); + cmd.env(BUILD_ENV_MANIFEST_DIR, manifest_path.as_os_str()); Ok(()) } - // Helper function for `exec_build()` used to un-expose cargo-fuzz - // information that was previously exposed in environment variables during - // `build_env_expose()`. - // - // This is called directly after the `cargo build ...` command is executed. - fn build_env_unexpose(&self) -> Result<()> { - env::remove_var(BUILD_ENV_FLAG); - env::remove_var(BUILD_ENV_MANIFEST_DIR); - Ok(()) - } - pub fn exec_build( &self, mode: options::BuildMode, @@ -385,7 +371,7 @@ impl FuzzProject { // expose build information via environment variables, before executing // the build command - self.build_env_expose(mode, build, fuzz_target) + self.exec_build_expose_env(&mut cmd, mode, build, fuzz_target) .expect("Failed to set cargo-fuzz build environment variables."); let status = cmd @@ -395,10 +381,6 @@ impl FuzzProject { bail!("failed to build fuzz script: {:?}", cmd); } - // un-expose build information, after the command has finished - self.build_env_unexpose() - .expect("Failed to un-set cargo-fuzz build environment variables."); - Ok(()) }