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

Yul object names with dots are accepted but ambiguous #15540

Open
cameel opened this issue Oct 23, 2024 · 2 comments
Open

Yul object names with dots are accepted but ambiguous #15540

cameel opened this issue Oct 23, 2024 · 2 comments
Labels
bug 🐛 low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.

Comments

@cameel
Copy link
Member

cameel commented Oct 23, 2024

Description

An identifier like A.B.C is a valid name for a Yul object. However, the compiler also uses dots as separators in qualified object/data names. This creates ambiguity when names with dots are passed to builtins like dataoffset() or datasize().

Such names are interpreted as paths by some parts of the compiler and as names by other parts. This results in errors or ICEs unless both possibilities are defined. And when both are defined, it's possible that some checks are getting bypassed.

Environment

  • Compiler version: 0.8.28

Steps to Reproduce

Unreferenced object

unreferenced.yul

object "A.B.C" {
    code {}
}
solc --strict-assembly unreferenced.yul --asm --debug-info none
======= unreferenced.yul (EVM) =======

Text representation:
  stop

Dots in names are accepted and apparently cause no issue on their own.

Top-level subobject

top-level.yul:

object "X" {
    code {
        sstore(0, datasize("A.B"))
    }

    data "A.B" hex"11223344556677"
}
solc --strict-assembly top-level.yul --asm
Error: Unknown data object "A.B".
 --> top-level.yul:3:28:
  |
3 |         sstore(0, datasize("A.B"))
  |                            ^^^^^

The error is coming from Yul analysis, which means that it treats it as a path and expects a nested subobject.

Nested subobject

nested.yul:

object "X" {
    code {
        sstore(0, datasize("A.B"))
    }

    object "A" {
        code {}

        data "B" hex"112233"
    }
}
solc --strict-assembly nested.yul --asm
======= nested.yul (EVM) =======
Uncaught exception:
/solidity/libyul/Object.cpp(155): Throw in function std::vector<long unsigned int> solidity::yul::Object::pathToSubObject(solidity::yul::YulString) const
Dynamic exception type: boost::wrapexcept<solidity::yul::YulAssertion>
std::exception::what: Assembly object <A.B> not found or does not contain code.
[solidity::util::tag_comment*] = Assembly object <A.B> not found or does not contain code.

This one probably passes Yul analysis but then fails later at a place that interprets the path differently.

Ambiguous subobject

ambiguous.yul:

object "X" {
    code {
        sstore(0, datasize("A.B"))
    }

    data "A.B" hex"11223344556677"

    object "A" {
        code {}

        data "B" hex"112233"
    }
}
solc --strict-assembly ambiguous.yul --asm --debug-info none
======= ambiguous.yul (EVM) =======

Text representation:
  0x07
  0x00
  sstore
  stop
stop
data_c304e60a0d3af7489b64bc5b186db2aed0df052c89a05935e0731015cd85d049 11223344556677

sub_0: assembly {
      stop
    stop
    data_50fceab2fe7ed15023d21b343e098d8a822f44ed61ba7e988e708db9c68c2535 112233
}

Note the 0x07 - when assembling we're apparently choosing the top-level data, again different from analysis.

@cameel
Copy link
Member Author

cameel commented Oct 23, 2024

Decision from the call today: we're going to disallow dots in names and we don't consider it breaking. Such names don't seem to work properly outside of weird corner cases anyway and any code broken by it can be trivially converted into working code by just removing the dots. It also only affects Yul - the Solidity compiler never generates object names with dots by itself.

@cameel cameel added low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. labels Oct 23, 2024
@salvadormartin3z
Copy link

@cameel Hello, I hope you're doing well. I noticed that this issue has a "low effort" label, and I would be interested in contributing. Is there a possibility that I could help with something?

I'm just starting to study Solidity, and I created a repo to track my progress: https://github.com/salvadormartin3z/Learn-Solidity any recommendations are welcome.

I hope we can connect.

@ethereum ethereum deleted a comment Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.
Projects
None yet
Development

No branches or pull requests

2 participants