Skip to content

yul: Compile objects that carry deploy code only - #597

Open
dimartiro wants to merge 3 commits into
paritytech:mainfrom
dimartiro:fix/490-deploy-only-yul-object
Open

yul: Compile objects that carry deploy code only#597
dimartiro wants to merge 3 commits into
paritytech:mainfrom
dimartiro:fix/490-deploy-only-yul-object

Conversation

@dimartiro

Copy link
Copy Markdown

Description

Closes #490

resolc --yul rejected any Yul object that carries deploy code only, that is one without a _deployed runtime sub-object, while solc compiles it:

object "Test" {
    code {
        { mstore(0, 42) return(0, 32) }
    }
}
Error: The contract `deploy_only.yul:Test` unoptimized LLVM IR verification error:
Basic Block in function '__runtime' does not have terminator!
label %entry

Cause

Object::declare declares PolkaVMRuntimeCodeFunction unconditionally, so __runtime always exists with an empty entry block. Object::into_llvm only ever defines it from the _deployed branch, which is reached through if let Some(object) = self.inner_object. With no sub-object the function stays declared and undefined, and the module fails verification.

Not declaring it is not an option: Entry::leave_entry looks __runtime up and bails with Contract runtime code not found, since __entry dispatches to either __deploy or __runtime on the call flag.

Fix

Object::parse now materializes the runtime sub-object when the source omits it, so the AST that reaches code generation always has one.

Doing it in the parser rather than special casing the absence in code generation keeps every consumer on the path it already handles. That matters here because both IR pipelines read this field: newyork's YulTranslator::translate_object has the same if let Some(inner_object) shape, and --newyork reproduces the identical error. One parser-level change fixes both.

The synthesized body is an explicit stop(). Code::into_llvm terminates every code block with an implicit stop anyway ("The EVM lets the code return implicitly"), so this is exactly what an empty runtime block lowers to, and it matches the EVM behaviour of calling an account with no code: success with empty return data.

I did not use an empty block, for two reasons. It would leave __runtime as entry: br %return / return: unreachable, and that unreachable is now reachable rather than a dead tail, which is UB the optimizer is free to exploit — __runtime is Private and called only from __entry, so folding it away could turn a call into a constructor run. And separately, an empty runtime block currently trips an assertion in polkavm-linker under --newyork (see below), which the stop() avoids.

Tests added:

  • revive-yul: the deploy-only object gets the implicit runtime object with a stop() body, and an explicit _deployed object is not clobbered by it.
  • resolc: compiles_object_without_a_runtime_sub_object compiles the new deploy_only_object.yul fixture end to end and cross-checks that solc agrees on the exit code, which is the premise of the issue.

@xermicus xermicus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm thanks

"An object carrying deploy code only, without a `_deployed` sub-object",
);

// `solc` accepts this too, which is the whole point: the two front-ends should agree.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// `solc` accepts this too, which is the whole point: the two front-ends should agree.

This is the default behavior so no comment needed.

Comment on lines +555 to +556
// An explicit `stop`, so that `__runtime` is emitted with a terminator.
assert_eq!(inner.code.block.statements.len(), 1);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// An explicit `stop`, so that `__runtime` is emitted with a terminator.
assert_eq!(inner.code.block.statements.len(), 1);
assert_eq!(inner.code.block.statements.len(), 1, "expected one statement");

Comment on lines +181 to +195
/// Builds the runtime code object for a deploy-only object, that is one the source omitted
/// the `_deployed` sub-object for.
///
/// Such an object carries deploy code only, so the deployed contract has no runtime code to
/// run. `solc` accepts this, and the EVM behaviour of "no runtime code" is a successful
/// return with empty data, which is exactly what an empty Yul code block lowers to (see
/// `Code::into_llvm`, which terminates every code block with an implicit `stop`).
///
/// Materializing the sub-object here instead of special casing its absence in code
/// generation keeps every consumer of the AST -- both IR pipelines, the visitors and the
/// analyses -- on the one path they already handle.
///
/// The body is an explicit `stop` rather than an empty block. The two are equivalent, but an
/// empty runtime block currently trips an assertion in `polkavm-linker` when lowered through
/// the `--newyork` pipeline.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/// Builds the runtime code object for a deploy-only object, that is one the source omitted
/// the `_deployed` sub-object for.
///
/// Such an object carries deploy code only, so the deployed contract has no runtime code to
/// run. `solc` accepts this, and the EVM behaviour of "no runtime code" is a successful
/// return with empty data, which is exactly what an empty Yul code block lowers to (see
/// `Code::into_llvm`, which terminates every code block with an implicit `stop`).
///
/// Materializing the sub-object here instead of special casing its absence in code
/// generation keeps every consumer of the AST -- both IR pipelines, the visitors and the
/// analyses -- on the one path they already handle.
///
/// The body is an explicit `stop` rather than an empty block. The two are equivalent, but an
/// empty runtime block currently trips an assertion in `polkavm-linker` when lowered through
/// the `--newyork` pipeline.
/// A short-hand constructor returning an object with a call to `stop`.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

resolc fails on deploy-only Yul objects

2 participants