diff --git a/Cargo.lock b/Cargo.lock index a1bdf328b73e982150cd43e2c9993245c0f786de..8a138b88e4b8b06cf7aeda230aedb77908943f42 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -507,7 +507,6 @@ dependencies = [ "parity-wasm 0.42.2", "platforms", "pretty_assertions", - "pwasm-utils", "regex", "rustc_version", "semver", @@ -2332,17 +2331,6 @@ dependencies = [ "unicode-xid", ] -[[package]] -name = "pwasm-utils" -version = "0.18.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0c1a2f10b47d446372a4f397c58b329aaea72b2daf9395a623a411cb8ccb54f" -dependencies = [ - "byteorder", - "log", - "parity-wasm 0.42.2", -] - [[package]] name = "quote" version = "1.0.9" diff --git a/Cargo.toml b/Cargo.toml index bfb9a42e608013e8792b16cf7a6370a392cb8e20..d819a8f5c06944c8273992be2fdeb9b469fd3215 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,6 @@ structopt = "0.3.22" log = "0.4.14" heck = "0.3.3" zip = { version = "0.5.13", default-features = false } -pwasm-utils = "0.18.1" parity-wasm = "0.42.2" cargo_metadata = "0.14.0" codec = { package = "parity-scale-codec", version = "2.1", features = ["derive"] } diff --git a/src/cmd/build.rs b/src/cmd/build.rs index fbbc6514fc7a6a575d9b9d463dc8e74b4e7ba106..416eaa280e4155d56b4b2caaf1472b94a40aec26 100644 --- a/src/cmd/build.rs +++ b/src/cmd/build.rs @@ -23,7 +23,7 @@ use crate::{ }; use anyhow::{Context, Result}; use colored::Colorize; -use parity_wasm::elements::{External, MemoryType, Module, Section}; +use parity_wasm::elements::{External, Internal, MemoryType, Module, Section}; use regex::Regex; use std::{ convert::TryFrom, @@ -86,8 +86,13 @@ pub struct BuildCommand { /// - It is possible to define the number of optimization passes in the /// `[package.metadata.contract]` of your `Cargo.toml` as e.g. `optimization-passes = "3"`. /// The CLI argument always takes precedence over the profile value. - #[structopt(long = "optimization-passes")] + #[structopt(long)] optimization_passes: Option, + /// Do not remove symbols (Wasm name section) when optimizing. + /// + /// This is useful if one wants to analyze or debug the optimized binary. + #[structopt(long)] + keep_debug_symbols: bool, } impl BuildCommand { @@ -118,6 +123,7 @@ impl BuildCommand { self.build_artifact, unstable_flags, optimization_passes, + self.keep_debug_symbols, ) } } @@ -146,6 +152,7 @@ impl CheckCommand { BuildArtifacts::CheckOnly, unstable_flags, OptimizationPasses::Zero, + false, ) } } @@ -260,31 +267,43 @@ fn ensure_maximum_memory_pages(module: &mut Module, maximum_allowed_pages: u32) /// Strips all custom sections. /// /// Presently all custom sections are not required so they can be stripped safely. +/// The name section is already stripped by `wasm-opt`. fn strip_custom_sections(module: &mut Module) { - module.sections_mut().retain(|section| { - !matches!( - section, - Section::Custom(_) | Section::Name(_) | Section::Reloc(_) - ) - }); + module.sections_mut().retain(|section| match section { + Section::Reloc(_) => false, + Section::Custom(custom) if custom.name() != "name" => false, + _ => true, + }) +} + +/// A contract should export nothing but the "call" and "deploy" functions. +/// +/// Any elements not referenced by these exports become orphaned and are removed by `wasm-opt`. +fn strip_exports(module: &mut Module) { + if let Some(section) = module.export_section_mut() { + section.entries_mut().retain(|entry| { + matches!(entry.internal(), Internal::Function(_)) + && (entry.field() == "call" || entry.field() == "deploy") + }) + } +} + +/// Load and parse a wasm file from disk. +fn load_module>(path: P) -> Result { + let path = path.as_ref(); + parity_wasm::deserialize_file(path).context(format!( + "Loading of wasm module at '{}' failed", + path.display(), + )) } /// Performs required post-processing steps on the wasm artifact. fn post_process_wasm(crate_metadata: &CrateMetadata) -> Result<()> { // Deserialize wasm module from a file. let mut module = - parity_wasm::deserialize_file(&crate_metadata.original_wasm).context(format!( - "Loading original wasm file '{}'", - crate_metadata.original_wasm.display() - ))?; - - // Perform optimization. - // - // In practice only tree-shaking is performed, i.e transitively removing all symbols that are - // NOT used by the specified entrypoints. - if pwasm_utils::optimize(&mut module, ["call", "deploy"].to_vec()).is_err() { - anyhow::bail!("Optimizer failed"); - } + load_module(&crate_metadata.original_wasm).context("Loading of original wasm failed")?; + + strip_exports(&mut module); ensure_maximum_memory_pages(&mut module, MAX_MEMORY_PAGES)?; strip_custom_sections(&mut module); @@ -306,6 +325,7 @@ fn post_process_wasm(crate_metadata: &CrateMetadata) -> Result<()> { fn optimize_wasm( crate_metadata: &CrateMetadata, optimization_passes: OptimizationPasses, + keep_debug_symbols: bool, ) -> Result { let mut dest_optimized = crate_metadata.dest_wasm.clone(); dest_optimized.set_file_name(format!( @@ -316,6 +336,7 @@ fn optimize_wasm( crate_metadata.dest_wasm.as_os_str(), dest_optimized.as_os_str(), optimization_passes, + keep_debug_symbols, )?; if !dest_optimized.exists() { @@ -348,6 +369,7 @@ fn do_optimization( dest_wasm: &OsStr, dest_optimized: &OsStr, optimization_level: OptimizationPasses, + keep_debug_symbols: bool, ) -> Result<()> { // check `wasm-opt` is installed let which = which::which("wasm-opt"); @@ -379,7 +401,8 @@ fn do_optimization( "Optimization level passed to wasm-opt: {}", optimization_level ); - let output = Command::new(wasm_opt_path) + let mut command = Command::new(wasm_opt_path); + command .arg(dest_wasm) .arg(format!("-O{}", optimization_level)) .arg("-o") @@ -387,15 +410,17 @@ fn do_optimization( // the memory in our module is imported, `wasm-opt` needs to be told that // the memory is initialized to zeroes, otherwise it won't run the // memory-packing pre-pass. - .arg("--zero-filled-memory") - .output() - .map_err(|err| { - anyhow::anyhow!( - "Executing {} failed with {:?}", - wasm_opt_path.display(), - err - ) - })?; + .arg("--zero-filled-memory"); + if keep_debug_symbols { + command.arg("-g"); + } + let output = command.output().map_err(|err| { + anyhow::anyhow!( + "Executing {} failed with {:?}", + wasm_opt_path.display(), + err + ) + })?; if !output.status.success() { let err = str::from_utf8(&output.stderr) @@ -529,6 +554,7 @@ pub(crate) fn execute( build_artifact: BuildArtifacts, unstable_flags: UnstableFlags, optimization_passes: OptimizationPasses, + keep_debug_symbols: bool, ) -> Result { let crate_metadata = CrateMetadata::collect(manifest_path)?; @@ -557,7 +583,8 @@ pub(crate) fn execute( format!("[3/{}]", build_artifact.steps()).bold(), "Optimizing wasm file".bright_green().bold() ); - let optimization_result = optimize_wasm(&crate_metadata, optimization_passes)?; + let optimization_result = + optimize_wasm(&crate_metadata, optimization_passes, keep_debug_symbols)?; Ok(optimization_result) }; @@ -600,7 +627,7 @@ pub(crate) fn execute( mod tests_ci_only { use super::{assert_compatible_ink_dependencies, check_wasm_opt_version_compatibility}; use crate::{ - cmd::BuildCommand, + cmd::{build::load_module, BuildCommand}, util::tests::{with_new_contract_project, with_tmp_dir}, workspace::Manifest, BuildArtifacts, ManifestPath, OptimizationPasses, UnstableFlags, UnstableOptions, @@ -631,6 +658,13 @@ mod tests_ci_only { .expect("writing manifest failed"); } + fn has_debug_symbols>(p: P) -> bool { + load_module(p) + .unwrap() + .custom_sections() + .any(|e| e.name() == "name") + } + /// Creates an executable `wasm-opt-mocked` file which outputs /// "wasm-opt version `version`". /// @@ -660,6 +694,7 @@ mod tests_ci_only { BuildArtifacts::CodeOnly, UnstableFlags::default(), OptimizationPasses::default(), + false, ) .expect("build failed"); @@ -682,6 +717,10 @@ mod tests_ci_only { // our optimized contract template should always be below 3k. assert!(optimized_size < 3.0); + // we specified that debug symbols should be removed + // original code should have some but the optimized version should have them removed + assert!(!has_debug_symbols(&res.dest_wasm.unwrap())); + Ok(()) }) } @@ -699,6 +738,7 @@ mod tests_ci_only { BuildArtifacts::CheckOnly, UnstableFlags::default(), OptimizationPasses::default(), + false, ) .expect("build failed"); @@ -731,6 +771,7 @@ mod tests_ci_only { // we choose zero optimization passes as the "cli" parameter optimization_passes: Some(OptimizationPasses::Zero), + keep_debug_symbols: false, }; // when @@ -740,17 +781,14 @@ mod tests_ci_only { .expect("no optimization result available"); // then - // we have to truncate here to account for a possible small delta - // in the floating point numbers - let optimized_size = optimization.optimized_size.trunc(); - let original_size = optimization.original_size.trunc(); + // The size does not exactly match the original size even without optimization + // passed because there is still some post processing happening. + let size_diff = optimization.original_size - optimization.optimized_size; assert!( - optimized_size == original_size, - "The optimized size {:?} differs from the original size {:?}", - optimized_size, - original_size + 0.0 < size_diff && size_diff < 10.0, + "The optimized size savings are larger than allowed or negative: {}", + size_diff, ); - Ok(()) }) } @@ -771,6 +809,7 @@ mod tests_ci_only { // we choose no optimization passes as the "cli" parameter optimization_passes: None, + keep_debug_symbols: false, }; // when @@ -780,15 +819,13 @@ mod tests_ci_only { .expect("no optimization result available"); // then - // we have to truncate here to account for a possible small delta - // in the floating point numbers - let optimized_size = optimization.optimized_size.trunc(); - let original_size = optimization.original_size.trunc(); + // The size does not exactly match the original size even without optimization + // passed because there is still some post processing happening. + let size_diff = optimization.original_size - optimization.optimized_size; assert!( - optimized_size < original_size, - "The optimized size DOES NOT {:?} differ from the original size {:?}", - optimized_size, - original_size + size_diff > (optimization.original_size / 2.0), + "The optimized size savings are too small: {}", + size_diff, ); Ok(()) @@ -930,6 +967,7 @@ mod tests_ci_only { verbosity: VerbosityFlags::default(), unstable_options: UnstableOptions::default(), optimization_passes: None, + keep_debug_symbols: false, }; let res = cmd.exec().expect("build failed"); @@ -944,4 +982,24 @@ mod tests_ci_only { Ok(()) }) } + + #[test] + fn keep_debug_symbols() { + with_new_contract_project(|manifest_path| { + let res = super::execute( + &manifest_path, + Verbosity::Default, + BuildArtifacts::CodeOnly, + UnstableFlags::default(), + OptimizationPasses::default(), + true, + ) + .expect("build failed"); + + // we specified that debug symbols should be kept + assert!(has_debug_symbols(&res.dest_wasm.unwrap())); + + Ok(()) + }) + } } diff --git a/src/cmd/metadata.rs b/src/cmd/metadata.rs index 2a0d2e2b5ddd0f00f6f79de4e26239106c86ffdd..8cd8184e4f52f32ec11a1f93b32de563c0a05487 100644 --- a/src/cmd/metadata.rs +++ b/src/cmd/metadata.rs @@ -328,6 +328,7 @@ mod tests { BuildArtifacts::All, UnstableFlags::default(), OptimizationPasses::default(), + false, )?; let dest_bundle = build_result .metadata_result