Skip to content
This repository was archived by the owner on Jan 17, 2022. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 31 additions & 88 deletions cli/build/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,52 +9,30 @@ extern crate pwasm_utils_cli as logger;
mod source;

use std::{fs, io};
use std::io::Write;
use std::path::PathBuf;

use clap::{App, Arg};
use parity_wasm::elements;

use utils::{CREATE_SYMBOL, CALL_SYMBOL, ununderscore_funcs, externalize_mem, shrink_unknown_stack};
use utils::{build, BuildError, Target};

#[derive(Debug)]
pub enum Error {
Io(io::Error),
FailedToCopy(String),
Decoding(elements::Error, String),
Encoding(elements::Error),
Packing(utils::PackingError),
Optimizer,
}

impl From<io::Error> for Error {
fn from(err: io::Error) -> Self {
Error::Io(err)
}
}

impl From<utils::OptimizerError> for Error {
fn from(_err: utils::OptimizerError) -> Self {
Error::Optimizer
}
}

impl From<utils::PackingError> for Error {
fn from(err: utils::PackingError) -> Self {
Error::Packing(err)
}
Build(BuildError)
Copy link
Contributor

Choose a reason for hiding this comment

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

comma

}

impl std::fmt::Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
use Error::*;
use self::Error::*;
match *self {
Io(ref io) => write!(f, "Generic i/o error: {}", io),
FailedToCopy(ref msg) => write!(f, "{}. Have you tried to run \"cargo build\"?", msg),
Decoding(ref err, ref file) => write!(f, "Decoding error ({}). Must be a valid wasm file {}. Pointed wrong file?", err, file),
Encoding(ref err) => write!(f, "Encoding error ({}). Almost impossible to happen, no free disk space?", err),
Optimizer => write!(f, "Optimization error due to missing export section. Pointed wrong file?"),
Packing(ref e) => write!(f, "Packing failed due to module structure error: {}. Sure used correct libraries for building contracts?", e),
Build(ref err) => write!(f, "Build error: {}", err)
}
}
}
Expand All @@ -70,8 +48,8 @@ pub fn process_output(input: &source::SourceInput) -> Result<(), Error> {
let wasm_name = input.bin_name().to_string().replace("-", "_");
cargo_path.push(
match input.target() {
source::SourceTarget::Emscripten => source::EMSCRIPTEN_TRIPLET,
source::SourceTarget::Unknown => source::UNKNOWN_TRIPLET,
Target::Emscripten => source::EMSCRIPTEN_TRIPLET,
Target::Unknown => source::UNKNOWN_TRIPLET,
}
);
cargo_path.push("release");
Expand All @@ -87,14 +65,6 @@ pub fn process_output(input: &source::SourceInput) -> Result<(), Error> {
Ok(())
}

fn has_ctor(module: &elements::Module) -> bool {
if let Some(ref section) = module.export_section() {
section.entries().iter().any(|e| CREATE_SYMBOL == e.field())
} else {
false
}
}

fn do_main() -> Result<(), Error> {
logger::init_log();

Expand Down Expand Up @@ -166,71 +136,44 @@ fn do_main() -> Result<(), Error> {

let path = wasm_path(&source_input);

let mut module = parity_wasm::deserialize_file(&path)
let module = parity_wasm::deserialize_file(&path)
.map_err(|e| Error::Decoding(e, path.to_string()))?;

if let source::SourceTarget::Emscripten = source_input.target() {
module = ununderscore_funcs(module);
}

if let source::SourceTarget::Unknown = source_input.target() {
// 49152 is 48kb!
if matches.is_present("enforce_stack_adjustment") {
let stack_size: u32 = matches.value_of("shrink_stack").unwrap_or_else(|| "49152").parse().expect("New stack size is not valid u32");
assert!(stack_size <= 1024*1024);
let (new_module, new_stack_top) = shrink_unknown_stack(module, 1024 * 1024 - stack_size);
module = new_module;
let mut stack_top_page = new_stack_top / 65536;
if new_stack_top % 65536 > 0 { stack_top_page += 1 };
module = externalize_mem(module, Some(stack_top_page), 16);
} else {
module = externalize_mem(module, None, 16);
}
}

if let Some(runtime_type) = matches.value_of("runtime_type") {
let runtime_type: &[u8] = runtime_type.as_bytes();
if runtime_type.len() != 4 {
let (runtime_type, runtime_version) = if let (Some(runtime_type), Some(runtime_version)) = (matches.value_of("runtime_type"), matches.value_of("runtime_version")) {
let ty: &[u8] = runtime_type.as_bytes();
if ty.len() != 4 {
panic!("--runtime-type should be equal to 4 bytes");
}
let runtime_version: u32 = matches.value_of("runtime_version").unwrap_or("1").parse()
let version: u32 = runtime_version.parse()
.expect("--runtime-version should be a positive integer");
module = utils::inject_runtime_type(module, &runtime_type, runtime_version);
}

let mut ctor_module = module.clone();
(Some(ty), Some(version))
} else {
(None, None)
};

let mut public_api_entries = matches.value_of("public_api")
let public_api_entries = matches.value_of("public_api")
.map(|val| val.split(",").collect())
.unwrap_or(Vec::new());
public_api_entries.push(CALL_SYMBOL);
if !matches.is_present("skip_optimization") {
utils::optimize(
&mut module,
public_api_entries,
)?;
}

let (module, ctor_module) = build(
module,
true,
source_input.target(),
runtime_type, runtime_version,
public_api_entries,
matches.is_present("enforce_stack_adjustment"),
matches.value_of("shrink_stack").unwrap_or_else(|| "49152").parse()
.expect("New stack size is not valid u32"),
matches.is_present("skip_optimization")).map_err(Error::Build)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

).map_err should be on the next line


if let Some(save_raw_path) = matches.value_of("save_raw") {
parity_wasm::serialize_to_file(save_raw_path, module.clone()).map_err(Error::Encoding)?;
}

let raw_module = parity_wasm::serialize(module).map_err(Error::Encoding)?;

// If module has an exported function with name=CREATE_SYMBOL
// build will pack the module (raw_module) into this funciton and export as CALL_SYMBOL.
// Otherwise it will just save an optimised raw_module
if has_ctor(&ctor_module) {
if !matches.is_present("skip_optimization") {
utils::optimize(&mut ctor_module, vec![CREATE_SYMBOL])?;
}
let ctor_module = utils::pack_instance(raw_module, ctor_module)?;
parity_wasm::serialize_to_file(&path, ctor_module).map_err(Error::Encoding)?;
} else {
let mut file = fs::File::create(&path)?;
file.write_all(&raw_module)?;
}

parity_wasm::serialize_to_file(
&path,
ctor_module.expect("ctor_module shouldn't be None, b/c 'constructor' argument is set to true in build")
Copy link
Contributor

Choose a reason for hiding this comment

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

b/c ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because

Copy link
Contributor

Choose a reason for hiding this comment

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

can we avoid non-obvious abbreviations?

).map_err(Error::Encoding)?;
Ok(())
}

Expand Down
19 changes: 7 additions & 12 deletions cli/build/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,15 @@
pub const UNKNOWN_TRIPLET: &str = "wasm32-unknown-unknown";
pub const EMSCRIPTEN_TRIPLET: &str = "wasm32-unknown-emscripten";

/// Target configiration of previous build step
#[derive(Debug, Clone, Copy)]
pub enum SourceTarget {
Emscripten,
Unknown,
}
use utils::Target;

/// Configuration of previous build step (cargo compilation)
#[derive(Debug)]
pub struct SourceInput<'a> {
target_dir: &'a str,
bin_name: &'a str,
final_name: &'a str,
target: SourceTarget,
target: Target,
}

impl<'a> SourceInput<'a> {
Expand All @@ -25,17 +20,17 @@ impl<'a> SourceInput<'a> {
target_dir: target_dir,
bin_name: bin_name,
final_name: bin_name,
target: SourceTarget::Emscripten,
target: Target::Emscripten,
}
}

pub fn unknown(mut self) -> Self {
self.target = SourceTarget::Unknown;
self.target = Target::Unknown;
self
}

pub fn emscripten(mut self) -> Self {
self.target = SourceTarget::Emscripten;
self.target = Target::Emscripten;
self
}

Expand All @@ -56,7 +51,7 @@ impl<'a> SourceInput<'a> {
self.final_name
}

pub fn target(&self) -> SourceTarget {
pub fn target(&self) -> Target {
self.target
}
}
}
121 changes: 121 additions & 0 deletions src/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
use std;
use super::{
CREATE_SYMBOL,
CALL_SYMBOL,
optimize,
pack_instance,
ununderscore_funcs,
externalize_mem,
shrink_unknown_stack,
inject_runtime_type,
PackingError,
OptimizerError
Copy link
Contributor

Choose a reason for hiding this comment

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

comma

};
Copy link
Contributor

Choose a reason for hiding this comment

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

should be at the beginning of the line

use parity_wasm;
use parity_wasm::elements;


Copy link
Contributor

Choose a reason for hiding this comment

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

redundant empty line

#[derive(Debug)]
pub enum Error {
Encoding(elements::Error),
Packing(PackingError),
NoCreateSymbolFound,
Optimizer,
}

impl From<OptimizerError> for Error {
fn from(_err: OptimizerError) -> Self {
Error::Optimizer
}
}

impl From<PackingError> for Error {
fn from(err: PackingError) -> Self {
Error::Packing(err)
}
}

#[derive(Debug, Clone, Copy)]
pub enum Target {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think Target is better name that SourceTarget, since SourceTarget outlines that it is the target which we take as a source to process further

Copy link
Contributor Author

@lexfrl lexfrl Aug 1, 2018

Choose a reason for hiding this comment

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

sure.
But because now it lives in build.rs, which don't care know about "files" and now it doesn't require such a context clarification, IMO

Copy link
Contributor

Choose a reason for hiding this comment

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

But the logic retained
Why not call it "source" then?

Emscripten,
Unknown,
}

impl std::fmt::Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
use self::Error::*;
match *self {
Encoding(ref err) => write!(f, "Encoding error ({})", err),
Optimizer => write!(f, "Optimization error due to missing export section. Pointed wrong file?"),
Packing(ref e) => write!(f, "Packing failed due to module structure error: {}. Sure used correct libraries for building contracts?", e),
NoCreateSymbolFound => write!(f, "Packing failed: no \"{}\" symbol found?", CREATE_SYMBOL),
}
}
}

fn has_ctor(module: &elements::Module) -> bool {
if let Some(ref section) = module.export_section() {
section.entries().iter().any(|e| CREATE_SYMBOL == e.field())
} else {
false
}
}

pub fn build(
mut module: elements::Module,
Copy link
Contributor

Choose a reason for hiding this comment

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

bad identations everywhere

Copy link
Contributor Author

@lexfrl lexfrl Aug 1, 2018

Choose a reason for hiding this comment

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

I don't understand, why you think so?

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like tabs everywhere, sorry

constructor: bool,
target: Target,
runtime_type: Option<&[u8]>,
runtime_version: Option<u32>,
mut public_api_entries: Vec<&str>,
Copy link
Contributor

@NikVolf NikVolf Aug 1, 2018

Choose a reason for hiding this comment

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

this is a bad choice for this argument in signature for public api

&[&str] would be ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you explain? I'd better to use [u8; 4] or u32, not &str

Copy link
Contributor

Choose a reason for hiding this comment

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

because it allows wider use of types when invoking the function

&vec!["a", "b"][..], &["a", "b"], &["a", "b"][1..], etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to use the type system soundness here. So I'd better use [u8; 4] here

Copy link
Contributor

Choose a reason for hiding this comment

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

what is [u8; 4]? how it is related to Vec<&str>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so for user it will be more descriptive API which could help to avoid the runtime error

Copy link
Contributor

Choose a reason for hiding this comment

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

how can you put a string in [u8; 4] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cause for runtime_type we use exactly 4 bytes. Would be nice to declare it as it is 🤗

Copy link
Contributor

@NikVolf NikVolf Aug 1, 2018

Choose a reason for hiding this comment

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

the conversation is about strings for public_api_entries argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, ok 🙈

enforce_stack_adjustment: bool,
stack_size: u32,
skip_optimization: bool) -> Result<(elements::Module, Option<elements::Module>), Error> {
Copy link
Contributor

@NikVolf NikVolf Aug 1, 2018

Choose a reason for hiding this comment

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

but this is bad should be

	skip_optimization: bool,
) -> Result<(elements::Module, Option<elements::Module>), Error> {


if let Target::Emscripten = target {
module = ununderscore_funcs(module);
}

if let Target::Unknown = target {
// 49152 is 48kb!
if enforce_stack_adjustment {
assert!(stack_size <= 1024*1024);
let (new_module, new_stack_top) = shrink_unknown_stack(module, 1024 * 1024 - stack_size);
module = new_module;
let mut stack_top_page = new_stack_top / 65536;
if new_stack_top % 65536 > 0 { stack_top_page += 1 };
module = externalize_mem(module, Some(stack_top_page), 16);
} else {
module = externalize_mem(module, None, 16);
}
}

if let (Some(ref runtime_type), Some(runtime_version)) = (runtime_type, runtime_version) {
module = inject_runtime_type(module, runtime_type, runtime_version);
}

let mut ctor_module = module.clone();

public_api_entries.push(CALL_SYMBOL);
if !skip_optimization {
optimize(
&mut module,
public_api_entries,
)?;
}

if constructor {
if !has_ctor(&ctor_module) {
Err(Error::NoCreateSymbolFound)?
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what are the identation here? some spaces + tab?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cant find any spaces in indentation in this snippet

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm.. probably github changed the way how it displays the diff

Copy link
Contributor

Choose a reason for hiding this comment

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

looks ok, sorry

if !skip_optimization {
optimize(&mut ctor_module, vec![CREATE_SYMBOL])?;
}
let ctor_module =
pack_instance(
parity_wasm::serialize(module.clone()).map_err(Error::Encoding)?, ctor_module.clone())?;
Copy link
Contributor

Choose a reason for hiding this comment

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

bad indentations

should use smth like

pack_instance(
   ... arguments
)?;

Ok((module, Some(ctor_module)))
} else {
Ok((module, None))
}
}
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub static RET_SYMBOL: &'static str = "ret";

pub mod rules;

mod build;
mod optimizer;
mod gas;
mod symbols;
Expand All @@ -24,6 +25,7 @@ mod runtime_type;

pub mod stack_height;

pub use build::{build, Target, Error as BuildError};
pub use optimizer::{optimize, Error as OptimizerError};
pub use gas::inject_gas_counter;
pub use ext::{externalize, externalize_mem, underscore_funcs, ununderscore_funcs, shrink_unknown_stack};
Expand Down