Skip to content

Commit

Permalink
revive: Bump PolkaVM and add static code validation (#5939)
Browse files Browse the repository at this point in the history
This PR adds **static** validation that prevents upload of code that:

1) Contains basic blocks larger than the specified limit (currently
`200`)
2) Contains invalid instructions
3) Uses the `sbrk` instruction

Doing that statically at upload time (instead of at runtime) allows us
to change the basic block limit or add instructions later without
worrying about breaking old code. This is well worth the linear scan of
the whole blob on deployment in my opinion. Please note that those
checks are not applied when existing code is just run (hot path).

Also some drive by fixes:
- Remove superflous `publish = true`
- Abort fixture build on warning and fix existing warnings
- Re-enable optimizations in fixture builds (should be fixed now in
PolkaVM)
- Disable stripping for fixture builds (maybe we can get some line
information on trap via `RUST_LOG`)

---------

Co-authored-by: command-bot <>
Co-authored-by: PG Herveou <pgherveou@gmail.com>
  • Loading branch information
athei and pgherveou authored Oct 7, 2024
1 parent 3846691 commit 5f55185
Show file tree
Hide file tree
Showing 16 changed files with 664 additions and 474 deletions.
77 changes: 40 additions & 37 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions prdoc/pr_5939.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
title: "[pallet-revive] Bump PolkaVM and add static code validation"

doc:
- audience: Runtime Dev
description: |
Statically validate basic block sizes and instructions.

crates:
- name: pallet-revive
bump: major
- name: pallet-revive-fixtures
bump: minor
- name: pallet-revive-uapi
bump: patch
4 changes: 3 additions & 1 deletion substrate/frame/revive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
environmental = { workspace = true }
paste = { workspace = true }
polkavm = { version = "0.11.0", default-features = false }
polkavm = { version = "0.12.0", default-features = false }
polkavm-common = { version = "0.12.0", default-features = false }
bitflags = { workspace = true }
codec = { features = [
"derive",
Expand Down Expand Up @@ -83,6 +84,7 @@ std = [
"pallet-revive-fixtures/std",
"pallet-timestamp/std",
"pallet-utility/std",
"polkavm-common/std",
"polkavm/std",
"rlp/std",
"scale-info/std",
Expand Down
3 changes: 1 addition & 2 deletions substrate/frame/revive/fixtures/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[package]
name = "pallet-revive-fixtures"
publish = true
version = "0.1.0"
authors.workspace = true
edition.workspace = true
Expand All @@ -22,7 +21,7 @@ log = { workspace = true }
parity-wasm = { workspace = true }
tempfile = { workspace = true }
toml = { workspace = true }
polkavm-linker = { version = "0.11.0" }
polkavm-linker = { version = "0.12.0" }
anyhow = { workspace = true, default-features = true }

[features]
Expand Down
5 changes: 3 additions & 2 deletions substrate/frame/revive/fixtures/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ mod build {

fn invoke_build(current_dir: &Path) -> Result<()> {
let encoded_rustflags = [
"-Dwarnings",
"-Crelocation-model=pie",
"-Clink-arg=--emit-relocs",
"-Clink-arg=--export-dynamic-symbol=__polkavm_symbol_export_hack__*",
Expand Down Expand Up @@ -158,8 +159,8 @@ mod build {
/// Post-process the compiled code.
fn post_process(input_path: &Path, output_path: &Path) -> Result<()> {
let mut config = polkavm_linker::Config::default();
config.set_strip(true);
config.set_optimize(false);
config.set_strip(false);
config.set_optimize(true);
let orig =
fs::read(input_path).with_context(|| format!("Failed to read {:?}", input_path))?;
let linked = polkavm_linker::program_from_elf(config, orig.as_ref())
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/revive/fixtures/build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ edition = "2021"
[dependencies]
uapi = { package = 'pallet-revive-uapi', path = "", default-features = false }
common = { package = 'pallet-revive-fixtures-common', path = "" }
polkavm-derive = { version = "0.11.0" }
polkavm-derive = { version = "0.12.0" }

[profile.release]
opt-level = 3
Expand Down
47 changes: 47 additions & 0 deletions substrate/frame/revive/fixtures/contracts/basic_block.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// This file is part of Substrate.

// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//! Create a basic block that is larger than we allow.

#![no_std]
#![no_main]

extern crate common;

use core::arch::asm;

// Export that is never called. We can put code here that should be in the binary
// but is never supposed to be run.
#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call_never() {
// Stores cannot be optimized away because the optimizer cannot
// know whether they have side effects.
let value: u32 = 42;
unsafe {
// Repeat 1001 times to intentionally exceed the allowed basic block limit (1000)
asm!(".rept 1001", "sw {x}, 0(sp)", ".endr", x = in(reg) value);
}
}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn deploy() {}

#[no_mangle]
#[polkavm_derive::polkavm_export]
pub extern "C" fn call() {}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
#![no_std]
#![no_main]

use common::input;
extern crate common;

use uapi::{HostFn, HostFnImpl as api};

const BUF_SIZE: usize = 8;
Expand All @@ -36,7 +37,7 @@ static DATA: [u8; BUF_SIZE] = [1, 2, 3, 4, 5, 6, 7, 8];
/// and expect the call output to match `expected_output`.
fn assert_call<const N: usize>(callee_address: &[u8; 20], expected_output: [u8; BUF_SIZE]) {
let mut output_buf = [0u8; BUF_SIZE];
let mut output_buf_capped = &mut &mut output_buf[..N];
let output_buf_capped = &mut &mut output_buf[..N];

api::call(
uapi::CallFlags::ALLOW_REENTRY,
Expand All @@ -62,7 +63,7 @@ fn assert_instantiate<const N: usize>(expected_output: [u8; BUF_SIZE]) {
api::own_code_hash(&mut code_hash);

let mut output_buf = [0u8; BUF_SIZE];
let mut output_buf_capped = &mut &mut output_buf[..N];
let output_buf_capped = &mut &mut output_buf[..N];

api::instantiate(
&code_hash,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#![no_std]
#![no_main]

use common::{input, u256_bytes};
use common::input;
use uapi::{HostFn, HostFnImpl as api};

#[no_mangle]
Expand Down
Loading

0 comments on commit 5f55185

Please sign in to comment.