Corresponding code in the .cpp file has been commented instead of begin removed pending preliminary reviews

Code generators needed fixing of the cleanup process during typecasting of bytes and integers

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