Merge pull request #11930 from ethereum/calldataStructInlineAssembly

Fix inline assembly assignments to calldata structs and statically-sized calldata arrays.

Changelog.md

@@ -15,6 +15,7 @@ Compiler Features:
 
 
 Bugfixes:
+ * Code Generator: Fix ICE on assigning to calldata structs and statically-sized calldata arrays in inline assembly.
  * Code Generator: Use stable source order for ABI functions.
  * Commandline Interface: Report optimizer options as invalid in Standard JSON and linker modes instead of ignoring them.
  * Commandline Interface: Disallow the ``--experimental-via-ir`` option in Standard JSON, Assembler and Linker modes.

docs/assembly.rst

@@ -124,9 +124,22 @@ Access to External Variables, Functions and Libraries
 You can access Solidity variables and other identifiers by using their name.
 
 Local variables of value type are directly usable in inline assembly.
+They can both be read and assigned to.
 
-Local variables that refer to memory or calldata evaluate to the
-address of the variable in memory, resp. calldata, not the value itself.
+Local variables that refer to memory evaluate to the address of the variable in memory not the value itself.
+Such variables can also be assigned to, but note that an assignment will only change the pointer and not the data
+and that it is your responsibility to respect Solidity's memory management.
+See :ref:`Conventions in Solidity <conventions-in-solidity>`.
+
+Similarly, local variables that refer to statically-sized calldata arrays or calldata structs
+evaluate to the address of the variable in calldata, not the value itself.
+The variable can also be assigned a new offset, but note that no validation to ensure that
+the variable will not point beyond ``calldatasize()`` is performed.
+
+For dynamic calldata arrays, you can access
+their calldata offset (in bytes) and length (number of elements) using ``x.offset`` and ``x.length``.
+Both expressions can also be assigned to, but as for the static case, no validation will be performed
+to ensure that the resulting data area is within the bounds of ``calldatasize()``.
 
 For local storage variables or state variables, a single Yul identifier
 is not sufficient, since they do not necessarily occupy a single full storage slot.
@@ -135,9 +148,10 @@ inside that slot. To retrieve the slot pointed to by the variable ``x``, you
 use ``x.slot``, and to retrieve the byte-offset you use ``x.offset``.
 Using ``x`` itself will result in an error.
 
-For dynamic calldata arrays, you can access
-their calldata offset (in bytes) and length (number of elements) using ``x.offset`` and ``x.length``.
-Both expressions can also be assigned to.
+You can also assign to the ``.slot`` part of a local storage variable pointer.
+For these (structs, arrays or mappings), the ``.offset`` part is always zero.
+It is not possible to assign to the ``.slot`` or ``.offset`` part of a state variable,
+though.
 
 Local Solidity variables are available for assignments, for example:
 
@@ -178,17 +192,6 @@ Since Solidity 0.7.0, variables and functions declared inside the
 inline assembly block may not contain ``.``, but using ``.`` is
 valid to access Solidity variables from outside the inline assembly block.
 
-Assignments are possible to assembly-local variables and to function-local
-variables. Take care that when you assign to variables that point to
-memory or storage, you will only change the pointer and not the data.
-
-You can assign to the ``.slot`` part of a local storage variable pointer.
-For these (structs, arrays or mappings), the ``.offset`` part is always zero.
-It is not possible to assign to the ``.slot`` or ``.offset`` part of a state variable,
-though.
-
-
-
 Things to Avoid
 ---------------
 
@@ -199,6 +202,8 @@ functional-style opcodes, counting stack height for
 variable access and removing stack slots for assembly-local variables when the end
 of their block is reached.
 
+.. _conventions-in-solidity:
+
 Conventions in Solidity
 -----------------------
 

libsolidity/codegen/ContractCompiler.cpp

@@ -860,16 +860,29 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly)
 			}
 			else if (variable->type()->dataStoredIn(DataLocation::CallData))
 			{
-				auto const* arrayType = dynamic_cast<ArrayType const*>(variable->type());
-				solAssert(
-					arrayType && arrayType->isDynamicallySized() && arrayType->dataStoredIn(DataLocation::CallData),
-					""
-				);
+				if (auto const* arrayType = dynamic_cast<ArrayType const*>(variable->type()))
+				{
+					if (arrayType->isDynamicallySized())
+					{
 						solAssert(suffix == "offset" || suffix == "length", "");
 						solAssert(variable->type()->sizeOnStack() == 2, "");
 						if (suffix == "length")
 							stackDiff--;
 					}
+					else
+					{
+						solAssert(variable->type()->sizeOnStack() == 1, "");
+						solAssert(suffix.empty(), "");
+					}
+				}
+				else
+				{
+					auto const* structType = dynamic_cast<StructType const*>(variable->type());
+					solAssert(structType, "");
+					solAssert(variable->type()->sizeOnStack() == 1, "");
+					solAssert(suffix.empty(), "");
+				}
+			}
 			else
 				solAssert(suffix.empty(), "");
 

test/libsolidity/semanticTests/inlineAssembly/calldata_array_assign_static.sol

@@ -0,0 +1,10 @@
+contract C {
+    function f(uint[2][2] calldata x) public returns (uint[2][2] memory r) {
+        assembly { x := 0x24 }
+        r = x;
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// f(uint256[2][2]): 0x0, 8, 7, 6, 5 -> 8, 7, 6, 5

test/libsolidity/semanticTests/inlineAssembly/calldata_struct_assign.sol

@@ -0,0 +1,18 @@
+pragma abicoder v2;
+
+contract C {
+	struct S { uint256 x; }
+	struct S2 { uint256 x; uint256 y; }
+	function f(S calldata s, S2 calldata s2) public pure returns (uint256 r, uint256 r2) {
+		assembly {
+			s := s2
+			s2 := 4
+		}
+		r = s.x;
+		r2 = s2.x;
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// f((uint256),(uint256,uint256)): 0x42, 0x07, 0x77 -> 0x07, 0x42

test/libsolidity/semanticTests/inlineAssembly/calldata_struct_assign_and_return.sol

@@ -0,0 +1,23 @@
+pragma abicoder v2;
+contract C {
+  struct S { int8 x; int8 y; }
+  function f() internal pure returns(S calldata s) {
+    assembly {
+      s := 0x24
+    }
+  }
+  function g() public pure returns(int8, int8) {
+    S calldata s = f();
+    return (s.x, s.y);
+  }
+  function h() public pure returns(uint256) { f(); return 0x42; }
+  function i() public pure returns(uint256) { abi.decode(msg.data[4:], (S)); return 0x42; }
+}
+// ====
+// compileViaYul: also
+// ----
+// g(): 0xCAFFEE, 0x42, 0x21 -> 0x42, 0x21
+// g(): 0xCAFFEE, 0x4242, 0x2121 -> FAILURE
+// g(): 0xCAFFEE, 0x42 -> 0x42, 0
+// h() -> 0x42
+// i() -> FAILURE

test/libsolidity/syntaxTests/controlFlow/calldataReturn/calldata_return_dynamic_array.sol

@@ -0,0 +1,5 @@
+contract C {
+    function f() internal returns (uint256[] calldata) {}
+}
+// ----
+// TypeError 3464: (48-66): This variable is of calldata pointer type and can be returned without prior assignment, which would lead to undefined behaviour.

test/libsolidity/syntaxTests/controlFlow/calldataReturn/calldata_return_static_array.sol

@@ -0,0 +1,5 @@
+contract C {
+    function f() internal returns (uint256[1] calldata) {}
+}
+// ----
+// TypeError 3464: (48-67): This variable is of calldata pointer type and can be returned without prior assignment, which would lead to undefined behaviour.

test/libsolidity/syntaxTests/controlFlow/calldataReturn/calldata_return_struct.sol

@@ -0,0 +1,6 @@
+contract C {
+    struct S { uint256 x; }
+    function f() internal returns (S calldata) {}
+}
+// ----
+// TypeError 3464: (76-86): This variable is of calldata pointer type and can be returned without prior assignment, which would lead to undefined behaviour.