resolc: Report unsupported Yul object shapes instead of aborting - #600
Open
dimartiro wants to merge 1 commit into
Open
resolc: Report unsupported Yul object shapes instead of aborting#600dimartiro wants to merge 1 commit into
dimartiro wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refs #351
resolccompiles every Yul object as a contract of its own. Two shapessolcaccepts therefore have no representation, and both aborted the compiler rather than being reported. The two reproducers from the SRLabs finding, verbatim:Cause
An object referenced through
dataoffsetordatasizeis looked up among the compiled contracts. A sibling object that is not one has no entry, and neither does a dottedouter.innerpath, since the separator never appears in an identifier. Three sites asserted on the miss rather than reporting it:Project::compileresolving factory dependencies,.expect("Always exists").create.rstranslatingdataoffset,.expect("Always exists").create.rstranslatingdatasize,panic!("ICE: …").The issue reports two failures; the third is the same lookup duplicated between the
dataoffsetanddatasizetranslations, so it aborts on the same input by a different route.Fix
The two
create.rssites now share aresolve_object_pathhelper, and all three name the object and say which constraint it hit:Scope
This reports the limitations, it does not lift them. Supporting sibling objects or dotted paths changes what
resolcaccepts, which seemed like your call rather than something to decide inside a crash fix. HenceRefsrather thanCloses: 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
crates/resolc/src/tests/cli/yul_objects.rs, one per shape. They assert the output contains neitherpanickednorICE:, which is the property that regressed, rather than pinning an exact message.crates/integration/contracts/*.solfixtures compile to byte-identical bytecode against thev1.4.0release binary, and the five existing Yul fixtures still compile. Worth stating because the refactored lookup is on the path every contract with a_deployedsub-object takes, not only the unsupported shapes.cargo test -p resolc --lib70 passed, clippy clean.