Merge pull request #12584 from nishant-sachdeva/indexed_log_topic_differs_between_legacy_and_ir_if_explicitly_downcast

Code generators needed fixing of the cleanup process during typecasting

Author: nishant-sachdeva

Changelog.md

@@ -16,6 +16,8 @@ Bugfixes:
  * Code Generator: Fix ICE when doing an explicit conversion from ``string calldata`` to ``bytes``.
  * Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis.
  * Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the parent contract contains immutable variables.
+ * IR Generator: Add missing cleanup during the conversion of fixed bytes types to smaller fixed bytes types.
+ * IR Generator: Add missing cleanup for indexed event arguments of value type.
  * IR Generator: Fix IR syntax error when copying storage arrays of structs containing functions.
  * Natspec: Fix ICE when overriding a struct getter with a Natspec-documented return value and the name in the struct is different.
  * TypeChecker: Fix ICE when a constant variable declaration forward references a struct.

libsolidity/codegen/YulUtilFunctions.cpp

@@ -3400,11 +3400,11 @@ string YulUtilFunctions::conversionFunction(Type const& _from, Type const& _to)
 						.render();
 			else
 			{
-				// clear for conversion to longer bytes
 				solAssert(toCategory == Type::Category::FixedBytes, "Invalid type conversion requested.");
+				FixedBytesType const& to = dynamic_cast<FixedBytesType const&>(_to);
 				body =
 					Whiskers("converted := <clean>(value)")
-					("clean", cleanupFunction(from))
+					("clean", cleanupFunction((to.numBytes() <= from.numBytes()) ? to : from))
 					.render();
 			}
 			break;

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -1035,7 +1035,10 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall)
 						")\n";
 				}
 				else
-					indexedArgs.emplace_back(convert(arg, *paramTypes[i]));
+				{
+					solAssert(parameterTypes[i]->sizeOnStack() == 1, "");
+					indexedArgs.emplace_back(convert(arg, *paramTypes[i], true));
+				}
 			}
 			else
 			{
@@ -2724,14 +2727,14 @@ void IRGeneratorForStatements::assignInternalFunctionIDIfNotCalledDirectly(
 	m_context.addToInternalDispatch(_referencedFunction);
 }
 
-IRVariable IRGeneratorForStatements::convert(IRVariable const& _from, Type const& _to)
+IRVariable IRGeneratorForStatements::convert(IRVariable const& _from, Type const& _to, bool _forceCleanup)
 {
-	if (_from.type() == _to)
+	if (_from.type() == _to && !_forceCleanup)
 		return _from;
 	else
 	{
 		IRVariable converted(m_context.newYulVariable(), _to);
-		define(converted, _from);
+		define(converted, _from, _forceCleanup);
 		return converted;
 	}
 }
@@ -2763,10 +2766,10 @@ void IRGeneratorForStatements::declare(IRVariable const& _var)
 		appendCode() << "let " << _var.commaSeparatedList() << "\n";
 }
 
-void IRGeneratorForStatements::declareAssign(IRVariable const& _lhs, IRVariable const& _rhs, bool _declare)
+void IRGeneratorForStatements::declareAssign(IRVariable const& _lhs, IRVariable const& _rhs, bool _declare, bool _forceCleanup)
 {
 	string output;
-	if (_lhs.type() == _rhs.type())
+	if (_lhs.type() == _rhs.type() && !_forceCleanup)
 		for (auto const& [stackItemName, stackItemType]: _lhs.type().stackItems())
 			if (stackItemType)
 				declareAssign(_lhs.part(stackItemName), _rhs.part(stackItemName), _declare);

libsolidity/codegen/ir/IRGeneratorForStatements.h

@@ -86,7 +86,11 @@ class IRGeneratorForStatements: public IRGeneratorForStatementsBase
 	IRVariable evaluateExpression(Expression const& _expression, Type const& _to);
 
 	/// Defines @a _var using the value of @a _value while performing type conversions, if required.
-	void define(IRVariable const& _var, IRVariable const& _value) { declareAssign(_var, _value, true); }
+	/// If @a _forceCleanup is set to true, it also cleans the value of the variable after the conversion.
+	void define(IRVariable const& _var, IRVariable const& _value, bool _forceCleanup = false)
+	{
+		declareAssign(_var, _value, true, _forceCleanup);
+	}
 
 	/// @returns the name of a function that computes the value of the given constant
 	/// and also generates the function.
@@ -162,8 +166,8 @@ class IRGeneratorForStatements: public IRGeneratorForStatementsBase
 	);
 
 	/// Generates the required conversion code and @returns an IRVariable referring to the value of @a _variable
-	/// converted to type @a _to.
-	IRVariable convert(IRVariable const& _variable, Type const& _to);
+	/// If @a _forceCleanup is set to true, it also cleans the value of the variable after the conversion.
+	IRVariable convert(IRVariable const& _variable, Type const& _to, bool _forceCleanup = false);
 
 	/// @returns a Yul expression representing the current value of @a _expression,
 	/// converted to type @a _to if it does not yet have that type.
@@ -179,7 +183,7 @@ class IRGeneratorForStatements: public IRGeneratorForStatementsBase
 	/// Declares variable @a _var.
 	void declare(IRVariable const& _var);
 
-	void declareAssign(IRVariable const& _var, IRVariable const& _value, bool _define);
+	void declareAssign(IRVariable const& _var, IRVariable const& _value, bool _define, bool _forceCleanup = false);
 
 	/// @returns an IRVariable with the zero
 	/// value of @a _type.

test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening.sol renamed to test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_OldCodeGen.sol

@@ -11,7 +11,6 @@ contract C {
     }
 }
 // ====
-// compileToEwasm: also
-// compileViaYul: also
+// compileViaYul: false
 // ----
-// f() -> "\xff\xff\xff\xff"
+// f() -> 0xffffffff00000000000000000000000000000000000000000000000000000000

test/libsolidity/semanticTests/cleanup/cleanup_bytes_types_shortening_newCodeGen.sol

@@ -0,0 +1,14 @@
+contract C {
+    function f() public pure returns (bytes32 r) {
+        bytes4 x = 0xffffffff;
+        bytes2 y = bytes2(x);
+        assembly {
+            r := y
+        }
+    }
+}
+// ====
+// compileToEwasm: also
+// compileViaYul: true
+// ----
+// f() -> 0xffff000000000000000000000000000000000000000000000000000000000000

test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast.sol

@@ -0,0 +1,23 @@
+contract C {
+    function f() public pure returns (uint32 y) {
+        uint8 x = uint8(uint256(0x31313131313131313131313131313131));
+        assembly { y := x }
+    }
+
+    function g() public pure returns (bytes32 y) {
+        bytes1 x = bytes1(bytes16(0x31313131313131313131313131313131));
+        assembly { y := x }
+    }
+
+    function h() external returns (bytes32 y) {
+        bytes1 x;
+        assembly { x := sub(0,1) }
+        y = x;
+    }
+}
+// ====
+// compileViaYul: true
+// ----
+// f() -> 0x31
+// g() -> 0x3100000000000000000000000000000000000000000000000000000000000000
+// h() -> 0xff00000000000000000000000000000000000000000000000000000000000000

test/libsolidity/semanticTests/cleanup/indexed_log_topic_during_explicit_downcast_during_emissions.sol

@@ -0,0 +1,19 @@
+contract C {
+    event ev0(bytes1 indexed);
+    constructor() {
+        emit ev0(bytes1(bytes16(0x31313131313131313131313131313131)));
+    }
+    function j() external {
+        bytes1 x;
+        assembly { x := 0x3131313131313131313131313131313131313131313131313131313131313131 }
+        emit ev0(x);
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// constructor() ->
+// ~ emit ev0(bytes1): #"1"
+// gas legacy: 168735
+// j() ->
+// ~ emit ev0(bytes1): #"1"

test/libsolidity/semanticTests/userDefinedValueType/erc20.sol

@@ -115,7 +115,7 @@ contract ERC20 {
 // ----
 // constructor()
 // ~ emit Transfer(address,address,uint256): #0x00, #0x1212121212121212121212121212120000000012, 0x14
-// gas irOptimized: 442239
+// gas irOptimized: 447831
 // gas legacy: 861559
 // gas legacyOptimized: 420959
 // totalSupply() -> 20

test/libsolidity/semanticTests/various/erc20.sol

@@ -98,7 +98,7 @@ contract ERC20 {
 // ----
 // constructor()
 // ~ emit Transfer(address,address,uint256): #0x00, #0x1212121212121212121212121212120000000012, 0x14
-// gas irOptimized: 437697
+// gas irOptimized: 443295
 // gas legacy: 833310
 // gas legacyOptimized: 416135
 // totalSupply() -> 20