Merge pull request #10194 from ethereum/abicoderv1-empty-string

Change ABIEncoderV1 to not pad empty strings

Changelog.md

@@ -12,6 +12,7 @@ Compiler Features:
  * SMTChecker: Support struct constructor.
 
 Bugfixes:
+ * Code generator: Do not pad empty string literals with a single 32-byte zero field in the ABI coder v1.
  * SMTChecker: Fix internal compiler error when doing bitwise compound assignment with string literals.
 
 ### 0.7.5 (2020-11-18)

libsolidity/codegen/CompilerUtils.cpp

@@ -531,10 +531,15 @@ void CompilerUtils::encodeToMemory(
 			if (_givenTypes[i]->category() == Type::Category::StringLiteral)
 			{
 				auto const& strType = dynamic_cast<StringLiteralType const&>(*_givenTypes[i]);
-				m_context << u256(strType.value().size());
+				auto const size = strType.value().size();
+				m_context << u256(size);
 				storeInMemoryDynamic(*TypeProvider::uint256(), true);
 				// stack: ... <end_of_mem'>
-				storeInMemoryDynamic(strType, _padToWordBoundaries);
+
+				// Do not output empty padding for zero-length strings.
+				// TODO: handle this in storeInMemoryDynamic
+				if (size != 0)
+					storeInMemoryDynamic(strType, _padToWordBoundaries);
 			}
 			else
 			{

libsolidity/codegen/CompilerUtils.h

@@ -104,12 +104,13 @@ public:
 	/// Stores a 256 bit integer from stack in memory.
 	/// @param _offset offset in memory
 	void storeInMemory(unsigned _offset);
+
 	/// Dynamic version of @see storeInMemory, expects the memory offset below the value on the stack
 	/// and also updates that. For reference types, only copies the data pointer. Fails for
-	/// non-memory-references.
+	/// non-memory-references. For string literals no value is available on the stack.
 	/// @param _padToWords if true, adds zeros to pad to multiple of 32 bytes. Array elements
+	///                    are always padded (except for byte arrays), regardless of this parameter.
 	/// @param _cleanup if true, adds code to cleanup the value before storing it.
-	/// are always padded (except for byte arrays), regardless of this parameter.
 	/// Stack pre: memory_offset value...
 	/// Stack post: (memory_offset+length)
 	void storeInMemoryDynamic(Type const& _type, bool _padToWords = true, bool _cleanup = true);

test/libsolidity/semanticTests/abiEncoderV1/abi_encode_empty_string.sol

@@ -23,12 +23,12 @@ contract C {
 // ABIEncoderV1Only: true
 // compileViaYul: false
 // ----
-// f1() -> 0x20, 0x60, 0x20, 0, 0
+// f1() -> 0x20, 0x40, 0x20, 0
 // f2(string): 0x20, 0 -> 0x20, 0x40, 0x20, 0
 // f2(string): 0x20, 0, 0 -> 0x20, 0x40, 0x20, 0
 // g1() -> 32, 0
 // g2(string): 0x20, 0 -> 0x20, 0
 // g2(string): 0x20, 0, 0 -> 0x20, 0
-// h1() -> 0x20, 0x64, 26959946667150639794667015087019630673637144422540572481103610249216, 862718293348820473429344482784628181556388621521298319395315527974912, 0, 0
+// h1() -> 0x20, 0x44, 26959946667150639794667015087019630673637144422540572481103610249216, 862718293348820473429344482784628181556388621521298319395315527974912, 0
 // h2(string): 0x20, 0 -> 0x20, 0x44, 26959946667150639794667015087019630673637144422540572481103610249216, 862718293348820473429344482784628181556388621521298319395315527974912, 0
 // h2(string): 0x20, 0, 0 -> 0x20, 0x44, 26959946667150639794667015087019630673637144422540572481103610249216, 862718293348820473429344482784628181556388621521298319395315527974912, 0

test/libsolidity/semanticTests/abiencodedecode/abi_encode_empty_string.sol

@@ -6,6 +6,7 @@ contract C {
     }
 }
 // ====
+// compileViaYul: also
 // ABIEncoderV1Only: true
 // ----
-// f() -> 0x40, 0xc0, 0x60, 0x20, 0x0, 0x0, 0x0
+// f() -> 0x40, 0xa0, 0x40, 0x20, 0x0, 0x0

test/libsolidity/semanticTests/revertStrings/empty_v1.sol

@@ -13,6 +13,6 @@ contract C {
 // compileViaYul: false
 // revertStrings: debug
 // ----
-// f() -> FAILURE, hex"08c379a0", 0x20, 0, ""
+// f() -> FAILURE, hex"08c379a0", 0x20, 0
 // g(string): 0x20, 0, "" -> FAILURE, hex"08c379a0", 0x20, 0
 // g(string): 0x20, 0 -> FAILURE, hex"08c379a0", 0x20, 0