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

Add support for sign extension instructions. #400

Merged
merged 5 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
Binary file not shown.
8 changes: 8 additions & 0 deletions smart-contracts/testdata/contracts/v1/i32.extend16_s.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
;; The only purpose of this contract is to check
;; whether i32.extend16_s instruction is allowed in the module.
(module
;; Init contract
(func $init_contract (export "init_contract") (param i64) (result i32)
(i32.extend16_s (i32.const 3))
(return (i32.const 0))) ;; Successful init
)
Binary file not shown.
8 changes: 8 additions & 0 deletions smart-contracts/testdata/contracts/v1/i32.extend8_s.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
;; The only purpose of this contract is to check
;; whether i32.extend8_s instruction is allowed in the module.
(module
;; Init contract
(func $init_contract (export "init_contract") (param i64) (result i32)
(i32.extend8_s (i32.const 3))
(return (i32.const 0))) ;; Successful init
)
Binary file not shown.
8 changes: 8 additions & 0 deletions smart-contracts/testdata/contracts/v1/i64.extend16_s.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
;; The only purpose of this contract is to check
;; whether i64.extend16_s instruction is allowed in the module.
(module
;; Init contract
(func $init_contract (export "init_contract") (param i64) (result i32)
(i64.extend16_s (i64.const 3))
(return (i32.const 0))) ;; Successful init
)
Binary file not shown.
8 changes: 8 additions & 0 deletions smart-contracts/testdata/contracts/v1/i64.extend32_s.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
;; The only purpose of this contract is to check
;; whether i64.extend32_s instruction is allowed in the module.
(module
;; Init contract
(func $init_contract (export "init_contract") (param i64) (result i32)
(i64.extend32_s (i64.const 3))
(return (i32.const 0))) ;; Successful init
)
Binary file not shown.
8 changes: 8 additions & 0 deletions smart-contracts/testdata/contracts/v1/i64.extend8_s.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
;; The only purpose of this contract is to check
;; whether i64.extend8_s instruction is allowed in the module.
(module
;; Init contract
(func $init_contract (export "init_contract") (param i64) (result i32)
(i64.extend8_s (i64.const 3))
(return (i32.const 0))) ;; Successful init
)
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,40 @@
)
)

(func (export "i32.extend8_s")
(loop $loop
(i32.extend8_s (i32.const 312312))
(br $loop)
)
)

(func (export "i32.extend16_s")
(loop $loop
(i32.extend16_s (i32.const 312312))
(br $loop)
)
)

(func (export "i64.extend8_s")
(loop $loop
(i64.extend8_s (i64.const 312312))
(br $loop)
)
)

(func (export "i64.extend16_s")
(loop $loop
(i64.extend16_s (i64.const 312312))
(br $loop)
)
)

(func (export "i64.extend32_s")
(loop $loop
(i64.extend32_s (i64.const 312312))
(br $loop)
)
)

;; indirectly call an empty function with 100 arguments
(func (export "call_empty_function_100")
Expand Down
5 changes: 5 additions & 0 deletions smart-contracts/wasm-chain-integration/benches/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,11 @@ pub fn criterion_benchmark(c: &mut Criterion) {
exec("i64.rem_s", &[]);
exec("i64.rem_u", &[]);
exec("i32.wrap_i64", &[]);
exec("i32.extend8_s", &[]);
exec("i32.extend16_s", &[]);
exec("i64.extend8_s", &[]);
exec("i64.extend16_s", &[]);
exec("i64.extend32_s", &[]);
group.finish();
}

Expand Down
5 changes: 4 additions & 1 deletion smart-contracts/wasm-chain-integration/src/v1/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,8 @@ unsafe extern "C" fn validate_and_process_v1(
support_upgrade: u8,
// Allow globals in initialization expressions for data and element sections.
allow_globals_in_init: u8,
// Allow sign extension instructions.
allow_sign_extension_instr: u8,
wasm_bytes_ptr: *const u8,
wasm_bytes_len: size_t,
// this is the total length of the output byte array
Expand All @@ -392,7 +394,8 @@ unsafe extern "C" fn validate_and_process_v1(
let wasm_bytes = slice_from_c_bytes!(wasm_bytes_ptr, wasm_bytes_len);
match utils::instantiate_with_metering::<ProcessedImports, _>(
ValidationConfig {
allow_globals_in_init: allow_globals_in_init != 0,
allow_globals_in_init: allow_globals_in_init != 0,
allow_sign_extension_instr: allow_sign_extension_instr != 0,
},
&ConcordiumAllowedImports {
support_upgrade: support_upgrade == 1,
Expand Down
6 changes: 5 additions & 1 deletion smart-contracts/wasm-transform/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
## Unreleased changes

- `validate_module` is now parameterized by `ValidationConfig` which determines
which Wasm features should be allowed.
which Wasm features should be allowed. The currently supported configurable features are
- allow access to globals (defined in the current module) in data and element
initialization sections.
- allow instructions defined in the [sign extension operators](https://github.com/WebAssembly/sign-extension-ops/blob/master/proposals/sign-extension-ops/Overview.md)
Wasm proposal.

## concordium-wasm 2.0.0 (2023-06-16)

Expand Down
22 changes: 22 additions & 0 deletions smart-contracts/wasm-transform/src/artifact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,13 @@ pub enum InternalOpcode {
I32WrapI64,
I64ExtendI32S,
I64ExtendI32U,

// Sign extension instructions, optionally supported depending on the protocol version.
I32Extend8S,
I32Extend16S,
I64Extend8S,
I64Extend16S,
I64Extend32S,
}

/// Result of compilation. Either Ok(_) or an error indicating the reason.
Expand Down Expand Up @@ -1147,6 +1154,21 @@ impl Handler<&OpCode> for BackPatch {
OpCode::I64ExtendI32U => {
self.out.push(I64ExtendI32U);
}
OpCode::I32Extend8S => {
self.out.push(I32Extend8S);
}
OpCode::I32Extend16S => {
self.out.push(I32Extend16S);
}
OpCode::I64Extend8S => {
self.out.push(I64Extend8S);
}
OpCode::I64Extend16S => {
self.out.push(I64Extend16S);
}
OpCode::I64Extend32S => {
self.out.push(I64Extend32S);
}
}
Ok(())
}
Expand Down
2 changes: 2 additions & 0 deletions smart-contracts/wasm-transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ pub mod validate;

#[cfg(test)]
mod metering_transformation_test;
#[cfg(test)]
mod tests;
5 changes: 5 additions & 0 deletions smart-contracts/wasm-transform/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,11 @@ impl<I: TryFromImport, R: RunnableCode> Artifact<I, R> {
// and then extend, making it so that it is extended with 0's.
top.long = unsafe { top.short } as u32 as i64;
}
InternalOpcode::I32Extend8S => unary_i32(&mut stack, |x| x as i8 as i32),
InternalOpcode::I32Extend16S => unary_i32(&mut stack, |x| x as i16 as i32),
InternalOpcode::I64Extend8S => unary_i64(&mut stack, |x| x as i8 as i64),
InternalOpcode::I64Extend16S => unary_i64(&mut stack, |x| x as i16 as i64),
InternalOpcode::I64Extend32S => unary_i64(&mut stack, |x| x as i32 as i64),
}
}

Expand Down
5 changes: 5 additions & 0 deletions smart-contracts/wasm-transform/src/metering_transformation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,11 @@ pub(crate) mod cost {
I32WrapI64 => SIMPLE_UNOP,
I64ExtendI32S => SIMPLE_UNOP,
I64ExtendI32U => SIMPLE_UNOP,
I32Extend8S => SIMPLE_UNOP,
I32Extend16S => SIMPLE_UNOP,
I64Extend8S => SIMPLE_UNOP,
I64Extend16S => SIMPLE_UNOP,
I64Extend32S => SIMPLE_UNOP,
};
Ok(res)
}
Expand Down
87 changes: 67 additions & 20 deletions smart-contracts/wasm-transform/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
//! such as pruning and embedding additional metadata into the module.
//!
//! In the second stage each section can be parsed into a proper structure.
use crate::{constants::*, types::*};
use crate::{constants::*, types::*, validate::ValidationConfig};
use anyhow::{bail, ensure};
use std::{
convert::TryFrom,
Expand Down Expand Up @@ -555,6 +555,24 @@ impl<'a, Ctx: Copy> Parseable<'a, Ctx> for FunctionSection {
}
}

#[derive(Copy, Clone, Debug)]
/// Internal (to the library) helper to provide as context
/// when parsing instructions and sections where we need to distinguish
/// between the different versions of the Wasm spec we support.
///
/// Note that this is analogous to
/// [`ValidationConfig`](crate::validate::ValidationConfig), but instead of just
/// listing which things are allowed, it provides additional context needed
/// during parsing.
/// Currently this context is just a list of globals.
pub(crate) struct InstructionValidationContext<'a> {
/// If globals are allowed, then this is a parsed global section, otherwise
/// it is [`None`].
pub(crate) globals_allowed: Option<&'a GlobalSection>,
/// Whether to allow the instructions listed in the sign extension proposal.
pub(crate) allow_sign_extension_instr: bool,
}

/// Attempt to read a constant expression of given type (see section 3.3.7.2).
/// The `ty` argument specifies the expected type of the expression.
/// For the version of the standard we use this is always a single value type,
Expand All @@ -574,9 +592,13 @@ impl<'a, Ctx: Copy> Parseable<'a, Ctx> for FunctionSection {
fn read_constant_expr(
cursor: &mut Cursor<&'_ [u8]>,
ty: ValueType,
globals_allowed: Option<&GlobalSection>,
ctx: InstructionValidationContext,
) -> ParseResult<GlobalInit> {
let instr = decode_opcode(cursor)?;
// In principle it does not matter whether we allow or do not allow sign
// extension instructions since they are not constant. However the failure/error
// will be slightly different. It is more consistent to parse the instruction as
// allowed, and then reject as non-constant, as opposed to not-allow at all.
let instr = decode_opcode(ctx.allow_sign_extension_instr, cursor)?;
let res = match instr {
OpCode::I32Const(n) => {
ensure!(ty == ValueType::I32, "Constant instruction of type I64, but I32 expected.");
Expand All @@ -586,7 +608,7 @@ fn read_constant_expr(
ensure!(ty == ValueType::I64, "Constant instruction of type I32, but I64 expected.");
GlobalInit::I64(n)
}
OpCode::GlobalGet(idx) => match globals_allowed {
OpCode::GlobalGet(idx) => match ctx.globals_allowed {
None => bail!("GlobalGet not allowed in this constant expression."),
Some(globals) => {
let global = globals.get(idx).ok_or_else(|| {
Expand Down Expand Up @@ -702,8 +724,11 @@ impl<'a, Ctx> Parseable<'a, Ctx> for StartSection {
}
}

impl<'a> Parseable<'a, Option<&GlobalSection>> for Element {
fn parse(ctx: Option<&GlobalSection>, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
impl<'a> Parseable<'a, InstructionValidationContext<'_>> for Element {
fn parse(
ctx: InstructionValidationContext,
cursor: &mut Cursor<&'a [u8]>,
) -> ParseResult<Self> {
let table_index = TableIndex::parse(ctx, cursor)?;
ensure!(table_index == 0, "Only table index 0 is supported.");
let offset = read_constant_expr(cursor, ValueType::I32, ctx)?;
Expand All @@ -719,34 +744,40 @@ impl<'a> Parseable<'a, Option<&GlobalSection>> for Element {
}
}

impl<'a> Parseable<'a, Option<&GlobalSection>> for ElementSection {
fn parse(ctx: Option<&GlobalSection>, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
impl<'a> Parseable<'a, InstructionValidationContext<'_>> for ElementSection {
fn parse(
ctx: InstructionValidationContext<'_>,
cursor: &mut Cursor<&'a [u8]>,
) -> ParseResult<Self> {
let elements = cursor.next(ctx)?;
Ok(ElementSection {
elements,
})
}
}

impl<'a, Ctx: Copy> Parseable<'a, Ctx> for Global {
fn parse(ctx: Ctx, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
impl<'a> Parseable<'a, ValidationConfig> for Global {
fn parse(ctx: ValidationConfig, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
let ty = cursor.next(ctx)?;
let mutable = match Byte::parse(ctx, cursor)? {
0x00 => false,
0x01 => true,
flag => bail!("Unsupported mutability flag {:#04x}", flag),
};
// Globals initialization expressions cannot refer to other (in-module) globals.
let init = read_constant_expr(cursor, ty, None)?;
let init = read_constant_expr(cursor, ty, InstructionValidationContext {
globals_allowed: None,
allow_sign_extension_instr: ctx.allow_sign_extension_instr,
})?;
Ok(Global {
init,
mutable,
})
}
}

impl<'a, Ctx: Copy> Parseable<'a, Ctx> for GlobalSection {
fn parse(ctx: Ctx, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
impl<'a> Parseable<'a, ValidationConfig> for GlobalSection {
fn parse(ctx: ValidationConfig, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
let globals = cursor.next(ctx)?;
Ok(GlobalSection {
globals,
Expand Down Expand Up @@ -798,8 +829,11 @@ impl<'a, Ctx: Copy> Parseable<'a, Ctx> for Local {
/// defined globals in data and element sections. Since imported globals are
/// disallowed already in our execution environment, this means we disallow
/// globals in total.
impl<'a> Parseable<'a, Option<&GlobalSection>> for Data {
fn parse(ctx: Option<&GlobalSection>, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
impl<'a> Parseable<'a, InstructionValidationContext<'_>> for Data {
fn parse(
ctx: InstructionValidationContext,
cursor: &mut Cursor<&'a [u8]>,
) -> ParseResult<Self> {
let index = u32::parse(ctx, cursor)?;
ensure!(index == 0, "Only memory index 0 is supported.");
let offset = read_constant_expr(cursor, ValueType::I32, ctx)?;
Expand All @@ -815,8 +849,11 @@ impl<'a> Parseable<'a, Option<&GlobalSection>> for Data {
}
}

impl<'a> Parseable<'a, Option<&GlobalSection>> for DataSection {
fn parse(ctx: Option<&GlobalSection>, cursor: &mut Cursor<&'a [u8]>) -> ParseResult<Self> {
impl<'a> Parseable<'a, InstructionValidationContext<'_>> for DataSection {
limemloh marked this conversation as resolved.
Show resolved Hide resolved
fn parse(
ctx: InstructionValidationContext,
cursor: &mut Cursor<&'a [u8]>,
) -> ParseResult<Self> {
let sections = cursor.next(ctx)?;
Ok(DataSection {
sections,
Expand Down Expand Up @@ -881,7 +918,10 @@ impl std::fmt::Display for ParseError {
}

/// Decode the next opcode directly from the cursor.
pub(crate) fn decode_opcode(cursor: &mut Cursor<&[u8]>) -> ParseResult<OpCode> {
pub(crate) fn decode_opcode(
allow_sign_extension_instr: bool,
cursor: &mut Cursor<&[u8]>,
) -> ParseResult<OpCode> {
match Byte::parse(EMPTY_CTX, cursor)? {
END => Ok(OpCode::End),
0x00 => Ok(OpCode::Unreachable),
Expand Down Expand Up @@ -1112,20 +1152,27 @@ pub(crate) fn decode_opcode(cursor: &mut Cursor<&[u8]>) -> ParseResult<OpCode> {

0xAC => Ok(OpCode::I64ExtendI32S),
0xAD => Ok(OpCode::I64ExtendI32U),
0xC0 if allow_sign_extension_instr => Ok(OpCode::I32Extend8S),
0xC1 if allow_sign_extension_instr => Ok(OpCode::I32Extend16S),
0xC2 if allow_sign_extension_instr => Ok(OpCode::I64Extend8S),
0xC3 if allow_sign_extension_instr => Ok(OpCode::I64Extend16S),
0xC4 if allow_sign_extension_instr => Ok(OpCode::I64Extend32S),
byte => bail!(ParseError::UnsupportedInstruction {
opcode: byte,
}),
}
}

pub(crate) struct OpCodeIterator<'a> {
allow_sign_extension_instr: bool,
state: Cursor<&'a [u8]>,
}

impl<'a> OpCodeIterator<'a> {
pub fn new(bytes: &'a [u8]) -> Self {
pub fn new(allow_sign_extension_instr: bool, bytes: &'a [u8]) -> Self {
Self {
state: Cursor::new(bytes),
allow_sign_extension_instr,
}
}
}
Expand All @@ -1137,7 +1184,7 @@ impl<'a> Iterator for OpCodeIterator<'a> {
if self.state.position() == self.state.get_ref().len() as u64 {
None
} else {
Some(decode_opcode(&mut self.state))
Some(decode_opcode(self.allow_sign_extension_instr, &mut self.state))
}
}
}
Expand Down
Loading