diff --git a/Changelog.md b/Changelog.md index 441ff22dcc89..6c4ec7de819e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ Compiler Features: * General: Remove support for the experimental EOF (EVM Object Format) backend. Bugfixes: +* Codegen: Fix uninitialized internal function pointers being read from a packed storage slot with the wrong value when a subsequent variable in the slot holds a non-zero value. * NatSpec: Disallow `@return` tag in event documentation. * SMTChecker: Fix incorrect handling of constant operands of unary operations. diff --git a/libsolidity/codegen/LValue.cpp b/libsolidity/codegen/LValue.cpp index e8f6ae61c65b..21cb3e58c99c 100644 --- a/libsolidity/codegen/LValue.cpp +++ b/libsolidity/codegen/LValue.cpp @@ -242,6 +242,7 @@ void GenericStorageItem::retrieveValue(langutil::SourceLocation con if (type->category() == Type::Category::UserDefinedValueType) type = type->encodingType(); bool cleaned = false; + // shift bytes to the right positioning the function pointer at the start of the slot m_context << Instruction::SWAP1 << s_loadInstruction << Instruction::SWAP1 << u256(0x100) << Instruction::EXP << Instruction::SWAP1 << Instruction::DIV; @@ -257,9 +258,12 @@ void GenericStorageItem::retrieveValue(langutil::SourceLocation con } else if (fun->kind() == FunctionType::Kind::Internal) { + // internal function pointers occupy 8 bytes, so we mask the remaining bytes in the slot + m_context << ((u256(0x1) << (8 * type->storageBytes())) - 1) << Instruction::AND; m_context << Instruction::DUP1 << Instruction::ISZERO; CompilerUtils(m_context).pushZeroValue(*fun); m_context << Instruction::MUL << Instruction::OR; + cleaned = true; } } else if (type->leftAligned()) diff --git a/test/cmdlineTests/optimizer_inliner_dynamic_reference/output b/test/cmdlineTests/optimizer_inliner_dynamic_reference/output index 5313893fc735..35877fd1270e 100644 --- a/test/cmdlineTests/optimizer_inliner_dynamic_reference/output +++ b/test/cmdlineTests/optimizer_inliner_dynamic_reference/output @@ -112,15 +112,20 @@ sub_0: assembly { /* "input.sol":295:298 x() */ tag_19 swap1 + 0xffffffff /* "input.sol":295:296 x */ - dup1 - iszero tag_20 + 0xffffffffffffffff + dup4 + and + iszero mul - or /* "input.sol":295:298 x() */ - 0xffffffff + dup2 and + swap2 + and + or jump // in tag_19: /* "input.sol":295:302 x() + 1 */ diff --git a/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output b/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output index 5997bd1c7b60..1cf48a0d6962 100644 --- a/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output +++ b/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output @@ -117,15 +117,20 @@ sub_0: assembly { /* "input.sol":289:292 x() */ tag_16 swap1 + 0xffffffff /* "input.sol":289:290 x */ - dup1 - iszero tag_17 + 0xffffffffffffffff + dup4 + and + iszero mul - or /* "input.sol":289:292 x() */ - 0xffffffff + dup2 and + swap2 + and + or jump // in tag_16: /* "input.sol":289:296 x() + 1 */ diff --git a/test/libsolidity/semanticTests/functionTypes/call_to_zero_initialized_function_type_legacy.sol b/test/libsolidity/semanticTests/functionTypes/call_to_zero_initialized_function_type_legacy.sol index feca44e0c10f..f11ad4228403 100644 --- a/test/libsolidity/semanticTests/functionTypes/call_to_zero_initialized_function_type_legacy.sol +++ b/test/libsolidity/semanticTests/functionTypes/call_to_zero_initialized_function_type_legacy.sol @@ -24,4 +24,8 @@ contract C { // ==== // compileViaYul: false // ---- -// t() -> FAILURE +// t() -> FAILURE, hex"4e487b71", 0x51 +// gas legacy: 77534 +// gas legacy code: 69600 +// gas legacyOptimized: 76677 +// gas legacyOptimized code: 28600 diff --git a/test/libsolidity/semanticTests/functionTypes/store_function.sol b/test/libsolidity/semanticTests/functionTypes/store_function.sol index 904bc705f725..d95912ef5989 100644 --- a/test/libsolidity/semanticTests/functionTypes/store_function.sol +++ b/test/libsolidity/semanticTests/functionTypes/store_function.sol @@ -29,5 +29,5 @@ contract C { // gas irOptimized code: 19000 // gas legacy: 79492 // gas legacy code: 69600 -// gas legacyOptimized: 77587 +// gas legacyOptimized: 77611 // gas legacyOptimized code: 28600 diff --git a/test/libsolidity/semanticTests/functionTypes/uninitialized_internal_storage_function_pointer_comparison.sol b/test/libsolidity/semanticTests/functionTypes/uninitialized_internal_storage_function_pointer_comparison.sol new file mode 100644 index 000000000000..dc19d9e624c0 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/uninitialized_internal_storage_function_pointer_comparison.sol @@ -0,0 +1,22 @@ +contract C { + uint32 public x = 1; + function() internal internalFunctionPointer; + uint32 private y = 2; + + function testComparison() public view returns (bool) { + function() internal uninitializedPointer; + if (internalFunctionPointer == uninitializedPointer) + return true; + return false; + } + function testLocalAssignment() public view returns (bool) { + function() internal uninitializedPointer; + function() internal localPointer = internalFunctionPointer; + if (localPointer == uninitializedPointer) + return true; + return false; + } +} +// ---- +// testComparison() -> true +// testLocalAssignment() -> true diff --git a/test/libsolidity/semanticTests/functionTypes/uninitialized_internal_transient_storage_function_pointer_comparison.sol b/test/libsolidity/semanticTests/functionTypes/uninitialized_internal_transient_storage_function_pointer_comparison.sol new file mode 100644 index 000000000000..23cf90ef97d2 --- /dev/null +++ b/test/libsolidity/semanticTests/functionTypes/uninitialized_internal_transient_storage_function_pointer_comparison.sol @@ -0,0 +1,28 @@ +contract C { + uint32 public transient x; + function() internal transient internalFunctionPointer; + uint32 private transient y; + + function testComparison() public returns (bool) { + x = 1; + y = 2; + function() internal uninitializedPointer; + if (internalFunctionPointer == uninitializedPointer) + return true; + return false; + } + function testLocalAssignment() public returns (bool) { + x = 1; + y = 2; + function() internal uninitializedPointer; + function() internal localPointer = internalFunctionPointer; + if (localPointer == uninitializedPointer) + return true; + return false; + } +} +// ==== +// EVMVersion: >=cancun +// ---- +// testComparison() -> true +// testLocalAssignment() -> true