Skip to content

resolc: Report unsupported Yul object shapes instead of aborting - #600

Open
dimartiro wants to merge 1 commit into
paritytech:mainfrom
dimartiro:fix/yul-object-diagnostics
Open

resolc: Report unsupported Yul object shapes instead of aborting#600
dimartiro wants to merge 1 commit into
paritytech:mainfrom
dimartiro:fix/yul-object-diagnostics

Conversation

@dimartiro

Copy link
Copy Markdown

Description

Refs #351

resolc compiles every Yul object as a contract of its own. Two shapes solc accepts therefore have no representation, and both aborted the compiler rather than being reported. The two reproducers from the SRLabs finding, verbatim:

object "Test" {
    code { stop() }
    object "Test_deployed" { code {} }
    object "Error" { code {} }
}
thread 'main' panicked at crates/resolc/src/project/mod.rs:97:30: Always exists
object "Test" {
    code {
        let size := datasize("Test_deployed.Test")
        datacopy(0, dataoffset("Test_deployed.Test"), size)
        return(0, size)
    }
    object "Test_deployed" {
        code { stop() }
        object "Test" { code { revert(0, 0) } }
    }
}
thread '<unnamed>' panicked at crates/llvm-context/src/polkavm/evm/create.rs:156:32: ICE: Test_deployed.Test not found YulData { identifier_paths: {"Test": "…:Test"} }

Cause

An object referenced through dataoffset or datasize is looked up among the compiled contracts. A sibling object that is not one has no entry, and neither does a dotted outer.inner path, since the separator never appears in an identifier. Three sites asserted on the miss rather than reporting it:

  • Project::compile resolving factory dependencies, .expect("Always exists").
  • create.rs translating dataoffset, .expect("Always exists").
  • create.rs translating datasize, panic!("ICE: …").

The issue reports two failures; the third is the same lookup duplicated between the dataoffset and datasize translations, so it aborts on the same input by a different route.

Fix

The two create.rs sites now share a resolve_object_path helper, and all three name the object and say which constraint it hit:

Error: sibling_objects.yul:Test: the object(s) `Error` referenced by `dataoffset` or `datasize` are not compiled contracts. `resolc` expects every Yul object to be a contract of its own; sibling objects that are not, and the dotted notation for addressing a nested object, are not supported.
Error: LLVM IR generator: the object `Test_deployed.Test` can not be resolved: `resolc` does not support the dotted notation for addressing a nested Yul object

Scope

This reports the limitations, it does not lift them. Supporting sibling objects or dotted paths changes what resolc accepts, which seemed like your call rather than something to decide inside a crash fix. Hence Refs rather than Closes: close the issue if diagnostics are the wanted resolution, and I am happy to take on either shape as a follow-up if not.

The finding also notes that both discrepancies are undocumented. A compiler message that explains the constraint covers the half a developer hits in practice; adding them to that page is the other half and is not in this PR.

Verification

  • Two tests in crates/resolc/src/tests/cli/yul_objects.rs, one per shape. They assert the output contains neither panicked nor ICE:, which is the property that regressed, rather than pinning an exact message.
  • All 83 crates/integration/contracts/*.sol fixtures compile to byte-identical bytecode against the v1.4.0 release binary, and the five existing Yul fixtures still compile. Worth stating because the refactored lookup is on the path every contract with a _deployed sub-object takes, not only the unsupported shapes.
  • cargo test -p resolc --lib 70 passed, clippy clean.

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.

1 participant