1 files changed, 81 insertions, 57 deletions

diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 4bcc1fa9..4cec69c8 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -349,6 +349,10 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation)
 	case Token::Inc: // ++ (pre- or postfix)
 	case Token::Dec: // -- (pre- or postfix)
 		solAssert(!!m_currentLValue, "LValue not retrieved.");
+		solUnimplementedAssert(
+			_unaryOperation.annotation().type->category() != Type::Category::FixedPoint,
+			"Not yet implemented - FixedPointType."
+		);
 		m_currentLValue->retrieveValue(_unaryOperation.location());
 		if (!_unaryOperation.isPrefixOperation())
 		{
@@ -699,16 +703,24 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 		}
 		case FunctionType::Kind::SHA3:
 		{
-			TypePointers argumentTypes;
-			for (auto const& arg: arguments)
+			solAssert(arguments.size() == 1, "");
+			solAssert(!function.padArguments(), "");
+			TypePointer const& argType = arguments.front()->annotation().type;
+			solAssert(argType, "");
+			arguments.front()->accept(*this);
+			// Optimization: If type is bytes or string, then do not encode,
+			// but directly compute keccak256 on memory.
+			if (*argType == ArrayType(DataLocation::Memory) || *argType == ArrayType(DataLocation::Memory, true))
 			{
-				arg->accept(*this);
-				argumentTypes.push_back(arg->annotation().type);
+				ArrayUtils(m_context).retrieveLength(ArrayType(DataLocation::Memory));
+				m_context << Instruction::SWAP1 << u256(0x20) << Instruction::ADD;
+			}
+			else
+			{
+				utils().fetchFreeMemoryPointer();
+				utils().packedEncode({argType}, TypePointers());
+				utils().toSizeAfterFreeMemoryPointer();
 			}
-			utils().fetchFreeMemoryPointer();
-			solAssert(!function.padArguments(), "");
-			utils().packedEncode(argumentTypes, TypePointers());
-			utils().toSizeAfterFreeMemoryPointer();
 			m_context << Instruction::KECCAK256;
 			break;
 		}
@@ -866,6 +878,19 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				StorageByteArrayElement(m_context).storeValue(*type, _functionCall.location(), true);
 			break;
 		}
+		case FunctionType::Kind::ArrayPop:
+		{
+			_functionCall.expression().accept(*this);
+			solAssert(function.parameterTypes().empty(), "");
+
+			ArrayType const& arrayType = dynamic_cast<ArrayType const&>(
+				*dynamic_cast<MemberAccess const&>(_functionCall.expression()).expression().annotation().type
+			);
+			solAssert(arrayType.dataStoredIn(DataLocation::Storage), "");
+
+			ArrayUtils(m_context).popStorageArrayElement(arrayType);
+			break;
+		}
 		case FunctionType::Kind::ObjectCreation:
 		{
 			ArrayType const& arrayType = dynamic_cast<ArrayType const&>(*_functionCall.annotation().type);
@@ -1345,11 +1370,13 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 					break;
 				}
 		}
-		else if (member == "push")
+		else if (member == "push" || member == "pop")
 		{
 			solAssert(
-				type.isDynamicallySized() && type.location() == DataLocation::Storage,
-				"Tried to use .push() on a non-dynamically sized array"
+				type.isDynamicallySized() &&
+				type.location() == DataLocation::Storage &&
+				type.category() == Type::Category::Array,
+				"Tried to use ." + member + "() on a non-dynamically sized array"
 			);
 		}
 		else
@@ -1624,12 +1651,12 @@ void ExpressionCompiler::appendOrdinaryBinaryOperatorCode(Token::Value _operator
 
 void ExpressionCompiler::appendArithmeticOperatorCode(Token::Value _operator, Type const& _type)
 {
-	IntegerType const& type = dynamic_cast<IntegerType const&>(_type);
-	bool const c_isSigned = type.isSigned();
-
 	if (_type.category() == Type::Category::FixedPoint)
 		solUnimplemented("Not yet implemented - FixedPointType.");
 
+	IntegerType const& type = dynamic_cast<IntegerType const&>(_type);
+	bool const c_isSigned = type.isSigned();
+
 	switch (_operator)
 	{
 	case Token::Add:
@@ -1722,11 +1749,36 @@ void ExpressionCompiler::appendShiftOperatorCode(Token::Value _operator, Type co
 			m_context << u256(2) << Instruction::EXP << Instruction::MUL;
 		break;
 	case Token::SAR:
-		// NOTE: SAR rounds differently than SDIV
-		if (m_context.evmVersion().hasBitwiseShifting() && !c_valueSigned)
-			m_context << Instruction::SHR;
+		if (m_context.evmVersion().hasBitwiseShifting())
+			m_context << (c_valueSigned ? Instruction::SAR : Instruction::SHR);
 		else
-			m_context << u256(2) << Instruction::EXP << Instruction::SWAP1 << (c_valueSigned ? Instruction::SDIV : Instruction::DIV);
+		{
+			if (c_valueSigned)
+				// In the following assembly snippet, xor_mask will be zero, if value_to_shift is positive.
+				// Therefore xor'ing with xor_mask is the identity and the computation reduces to
+				// div(value_to_shift, exp(2, shift_amount)), which is correct, since for positive values
+				// arithmetic right shift is dividing by a power of two (which, as a bitwise operation, results
+				// in discarding bits on the right and filling with zeros from the left).
+				// For negative values arithmetic right shift, viewed as a bitwise operation, discards bits to the
+				// right and fills in ones from the left. This is achieved as follows:
+				// If value_to_shift is negative, then xor_mask will have all bits set, so xor'ing with xor_mask
+				// will flip all bits. First all bits in value_to_shift are flipped. As for the positive case,
+				// dividing by a power of two using integer arithmetic results in discarding bits to the right
+				// and filling with zeros from the left. Flipping all bits in the result again, turns all zeros
+				// on the left to ones and restores the non-discarded, shifted bits to their original value (they
+				// have now been flipped twice). In summary we now have discarded bits to the right and filled with
+				// ones from the left, i.e. we have performed an arithmetic right shift.
+				m_context.appendInlineAssembly(R"({
+					let xor_mask := sub(0, slt(value_to_shift, 0))
+					value_to_shift := xor(div(xor(value_to_shift, xor_mask), exp(2, shift_amount)), xor_mask)
+				})", {"value_to_shift", "shift_amount"});
+			else
+				m_context.appendInlineAssembly(R"({
+					value_to_shift := div(value_to_shift, exp(2, shift_amount))
+				})", {"value_to_shift", "shift_amount"});
+			m_context << Instruction::POP;
+
+		}
 		break;
 	case Token::SHR:
 	default:
@@ -1766,10 +1818,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	bool returnSuccessCondition = funKind == FunctionType::Kind::BareCall || funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::BareDelegateCall;
 	bool isCallCode = funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::CallCode;
 	bool isDelegateCall = funKind == FunctionType::Kind::BareDelegateCall || funKind == FunctionType::Kind::DelegateCall;
-	bool useStaticCall =
-		_functionType.stateMutability() <= StateMutability::View &&
-		m_context.experimentalFeatureActive(ExperimentalFeature::V050) &&
-		m_context.evmVersion().hasStaticCall();
+	bool useStaticCall = _functionType.stateMutability() <= StateMutability::View && m_context.evmVersion().hasStaticCall();
 
 	bool haveReturndatacopy = m_context.evmVersion().supportsReturndata();
 	unsigned retSize = 0;
@@ -1796,38 +1845,12 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	// Evaluate arguments.
 	TypePointers argumentTypes;
 	TypePointers parameterTypes = _functionType.parameterTypes();
-	bool manualFunctionId = false;
-	if (
-		(funKind == FunctionType::Kind::BareCall || funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::BareDelegateCall) &&
-		!_arguments.empty()
-	)
-	{
-		solAssert(_arguments.front()->annotation().type->mobileType(), "");
-		manualFunctionId =
-			_arguments.front()->annotation().type->mobileType()->calldataEncodedSize(false) ==
-			CompilerUtils::dataStartOffset;
-	}
-	if (manualFunctionId)
-	{
-		// If we have a Bare* and the first type has exactly 4 bytes, use it as
-		// function identifier.
-		_arguments.front()->accept(*this);
-		utils().convertType(
-			*_arguments.front()->annotation().type,
-			IntegerType(8 * CompilerUtils::dataStartOffset),
-			true
-		);
-		for (unsigned i = 0; i < gasValueSize; ++i)
-			m_context << swapInstruction(gasValueSize - i);
-		gasStackPos++;
-		valueStackPos++;
-	}
 	if (_functionType.bound())
 	{
 		argumentTypes.push_back(_functionType.selfType());
 		parameterTypes.insert(parameterTypes.begin(), _functionType.selfType());
 	}
-	for (size_t i = manualFunctionId ? 1 : 0; i < _arguments.size(); ++i)
+	for (size_t i = 0; i < _arguments.size(); ++i)
 	{
 		_arguments[i]->accept(*this);
 		argumentTypes.push_back(_arguments[i]->annotation().type);
@@ -1856,26 +1879,27 @@ void ExpressionCompiler::appendExternalFunctionCall(
 		{
 			m_context << u256(0);
 			utils().fetchFreeMemoryPointer();
-			// This touches too much, but that way we save some rounding arithmetics
+			// This touches too much, but that way we save some rounding arithmetic
 			m_context << u256(retSize) << Instruction::ADD << Instruction::MSTORE;
 		}
 	}
 
 	// Copy function identifier to memory.
 	utils().fetchFreeMemoryPointer();
-	if (!_functionType.isBareCall() || manualFunctionId)
+	if (!_functionType.isBareCall())
 	{
 		m_context << dupInstruction(2 + gasValueSize + CompilerUtils::sizeOnStack(argumentTypes));
 		utils().storeInMemoryDynamic(IntegerType(8 * CompilerUtils::dataStartOffset), false);
 	}
-	// If the function takes arbitrary parameters, copy dynamic length data in place.
+
+	// If the function takes arbitrary parameters or is a bare call, copy dynamic length data in place.
 	// Move arguments to memory, will not update the free memory pointer (but will update the memory
 	// pointer on the stack).
 	utils().encodeToMemory(
 		argumentTypes,
 		parameterTypes,
 		_functionType.padArguments(),
-		_functionType.takesArbitraryParameters(),
+		_functionType.takesArbitraryParameters() || _functionType.isBareCall(),
 		isCallCode || isDelegateCall
 	);
 
@@ -1956,9 +1980,9 @@ void ExpressionCompiler::appendExternalFunctionCall(
 
 	unsigned remainsSize =
 		2 + // contract address, input_memory_end
-		_functionType.valueSet() +
-		_functionType.gasSet() +
-		(!_functionType.isBareCall() || manualFunctionId);
+		(_functionType.valueSet() ? 1 : 0) +
+		(_functionType.gasSet() ? 1 : 0) +
+		(!_functionType.isBareCall() ? 1 : 0);
 
 	if (returnSuccessCondition)
 		m_context << swapInstruction(remainsSize);
@@ -2025,7 +2049,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 				mstore(0x40, newMem)
 			})", {"start", "size"});
 
-		utils().abiDecode(returnTypes, true, true);
+		utils().abiDecode(returnTypes, true);
 	}
 }