Defer the index access to elements of bytes arrays in storage to avoid dangling references.

This commit is contained in:
Daniel Kirchner 2022-06-08 19:21:57 +02:00
parent efcbc79b1c
commit fd83c278cd
16 changed files with 136 additions and 49 deletions

View File

@ -1046,12 +1046,14 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
// stack: ArrayReference 1 newLength
m_context << Instruction::SUB;
// stack: ArrayReference (newLength-1)
ArrayUtils(m_context).accessIndex(*arrayType, false);
if (arrayType->isByteArrayOrString())
setLValue<StorageByteArrayElement>(_functionCall);
else
{
ArrayUtils(m_context).accessIndex(*arrayType, false);
setLValueToStorageItem(_functionCall);
}
}
else
{
@ -1072,7 +1074,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
// stack: argValue ArrayReference newLength
m_context << u256(1) << Instruction::SWAP1 << Instruction::SUB;
// stack: argValue ArrayReference (newLength-1)
ArrayUtils(m_context).accessIndex(*arrayType, false);
if (!arrayType->isByteArrayOrString())
ArrayUtils(m_context).accessIndex(*arrayType, false);
// stack: argValue storageSlot slotOffset
utils().moveToStackTop(2, argType->sizeOnStack());
// stack: storageSlot slotOffset argValue
@ -2108,14 +2111,16 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
switch (arrayType.location())
{
case DataLocation::Storage:
ArrayUtils(m_context).accessIndex(arrayType);
if (arrayType.isByteArrayOrString())
{
solAssert(!arrayType.isString(), "Index access to string is not allowed.");
setLValue<StorageByteArrayElement>(_indexAccess);
}
else
{
ArrayUtils(m_context).accessIndex(arrayType);
setLValueToStorageItem(_indexAccess);
}
break;
case DataLocation::Memory:
ArrayUtils(m_context).accessIndex(arrayType);

View File

@ -508,6 +508,8 @@ StorageByteArrayElement::StorageByteArrayElement(CompilerContext& _compilerConte
void StorageByteArrayElement::retrieveValue(SourceLocation const&, bool _remove) const
{
ArrayUtils(m_context).accessIndex(*TypeProvider::bytesStorage());
// stack: ref byte_number
if (_remove)
m_context << Instruction::SWAP1 << Instruction::SLOAD
@ -520,6 +522,8 @@ void StorageByteArrayElement::retrieveValue(SourceLocation const&, bool _remove)
void StorageByteArrayElement::storeValue(Type const&, SourceLocation const&, bool _move) const
{
ArrayUtils(m_context).accessIndex(*TypeProvider::bytesStorage());
// stack: value ref byte_number
m_context << u256(31) << Instruction::SUB << u256(0x100) << Instruction::EXP;
// stack: value ref (1<<(8*(31-byte_number)))
@ -540,6 +544,8 @@ void StorageByteArrayElement::storeValue(Type const&, SourceLocation const&, boo
void StorageByteArrayElement::setToZero(SourceLocation const&, bool _removeReference) const
{
ArrayUtils(m_context).accessIndex(*TypeProvider::bytesStorage());
// stack: ref byte_number
solAssert(_removeReference, "");
m_context << u256(31) << Instruction::SUB << u256(0x100) << Instruction::EXP;

View File

@ -1634,12 +1634,14 @@ string YulUtilFunctions::storageArrayPushZeroFunction(ArrayType const& _type)
let data := sload(array)
let oldLen := <extractLength>(data)
<increaseBytesSize>(array, data, oldLen, add(oldLen, 1))
slot := array
offset := oldLen
<!isBytes>
let oldLen := <fetchLength>(array)
if iszero(lt(oldLen, <maxArrayLength>)) { <panic>() }
sstore(array, add(oldLen, 1))
slot, offset := <indexAccess>(array, oldLen)
</isBytes>
slot, offset := <indexAccess>(array, oldLen)
})")
("functionName", functionName)
("isBytes", _type.isByteArrayOrString())

View File

@ -1377,13 +1377,22 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall)
appendCode() << "let " << slotName << ", " << offsetName << " := " <<
m_utils.storageArrayPushZeroFunction(*arrayType) <<
"(" << IRVariable(_functionCall.expression()).commaSeparatedList() << ")\n";
setLValue(_functionCall, IRLValue{
*arrayType->baseType(),
IRLValue::Storage{
slotName,
offsetName,
}
});
if (arrayType->isByteArrayOrString())
setLValue(_functionCall, IRLValue{
*arrayType->baseType(),
IRLValue::StorageBytesElement{
slotName,
offsetName
}
});
else
setLValue(_functionCall, IRLValue{
*arrayType->baseType(),
IRLValue::Storage{
slotName,
offsetName,
}
});
}
else
{
@ -2220,24 +2229,33 @@ void IRGeneratorForStatements::endVisit(IndexAccess const& _indexAccess)
{
case DataLocation::Storage:
{
string slot = m_context.newYulVariable();
string offset = m_context.newYulVariable();
appendCode() << Whiskers(R"(
let <slot>, <offset> := <indexFunc>(<array>, <index>)
)")
("slot", slot)
("offset", offset)
("indexFunc", m_utils.storageArrayIndexAccessFunction(arrayType))
("array", IRVariable(_indexAccess.baseExpression()).part("slot").name())
("index", IRVariable(*_indexAccess.indexExpression()).name())
.render();
setLValue(_indexAccess, IRLValue{
*_indexAccess.annotation().type,
IRLValue::Storage{slot, offset}
});
if (arrayType.isByteArrayOrString())
setLValue(_indexAccess, IRLValue{
*_indexAccess.annotation().type,
IRLValue::StorageBytesElement{
IRVariable(_indexAccess.baseExpression()).part("slot").name(),
IRVariable(*_indexAccess.indexExpression()).name()
}
});
else
{
string slot = m_context.newYulVariable();
string offset = m_context.newYulVariable();
appendCode() << Whiskers(R"(
let <slot>, <offset> := <indexFunc>(<array>, <index>)
)")
("slot", slot)
("offset", offset)
("indexFunc", m_utils.storageArrayIndexAccessFunction(arrayType))
("array", IRVariable(_indexAccess.baseExpression()).part("slot").name())
("index", IRVariable(*_indexAccess.indexExpression()).name())
.render();
setLValue(_indexAccess, IRLValue{
*_indexAccess.annotation().type,
IRLValue::Storage{slot, offset}
});
}
break;
}
case DataLocation::Memory:
@ -2994,6 +3012,27 @@ void IRGeneratorForStatements::writeToLValue(IRLValue const& _lvalue, IRVariable
")\n";
},
[&](IRLValue::StorageBytesElement const& _storage) {
string offset = m_context.newYulVariable();
string slot = m_context.newYulVariable();
appendCode() << Whiskers(R"(
let <slot>, <offset> := <indexFunc>(<array>, <index>)
)")
("slot", slot)
("offset", offset)
("indexFunc", m_utils.storageArrayIndexAccessFunction(*TypeProvider::bytesStorage()))
("array", _storage.baseref)
("index", _storage.index)
.render();
appendCode() <<
m_utils.updateStorageValueFunction(_value.type(), _lvalue.type) <<
"(" <<
slot <<
", " <<
offset <<
_value.commaSeparatedListPrefixed() <<
")\n";
},
[&](IRLValue::Memory const& _memory) {
if (_lvalue.type.isValueType())
{
@ -3084,6 +3123,26 @@ IRVariable IRGeneratorForStatements::readFromLValue(IRLValue const& _lvalue)
_storage.slot <<
")\n";
},
[&](IRLValue::StorageBytesElement const& _storage) {
string offset = m_context.newYulVariable();
string slot = m_context.newYulVariable();
appendCode() << Whiskers(R"(
let <slot>, <offset> := <indexFunc>(<array>, <index>)
)")
("slot", slot)
("offset", offset)
("indexFunc", m_utils.storageArrayIndexAccessFunction(*TypeProvider::bytesStorage()))
("array", _storage.baseref)
("index", _storage.index)
.render();
define(result) <<
m_utils.readFromStorageDynamic(_lvalue.type, true) <<
"(" <<
slot <<
", " <<
offset <<
")\n";
},
[&](IRLValue::Memory const& _memory) {
if (_lvalue.type.isValueType())
define(result) <<

View File

@ -40,6 +40,11 @@ struct IRLValue
{
VariableDeclaration const* variable = nullptr;
};
struct StorageBytesElement
{
std::string const baseref;
std::string const index;
};
struct Storage
{
std::string const slot;
@ -64,7 +69,7 @@ struct IRLValue
{
std::vector<std::optional<IRLValue>> components;
};
std::variant<Stack, Immutable, Storage, Memory, Tuple> kind;
std::variant<Stack, Immutable, Storage, StorageBytesElement, Memory, Tuple> kind;
};
}

View File

@ -0,0 +1,10 @@
contract C {
bytes x = "012345678901234567890123456789A";
function test() external returns(uint) {
(x[0], x.push()) = (0x80,0x42);
return x.length;
// used to return 0x4000000000000000000000000000000000000000000000000000000000000020
}
}
// ----
// test() -> 0x20

View File

@ -46,8 +46,8 @@ contract c {
// storageEmpty -> 0
// test_long() -> 67
// gas irOptimized: 89148
// gas legacy: 103039
// gas legacyOptimized: 100493
// gas legacy: 111195
// gas legacyOptimized: 107824
// storageEmpty -> 0
// test_pop() -> 1780731860627700044960722568376592200742329637303199754547598369979433020
// gas legacy: 61930

View File

@ -18,5 +18,5 @@ contract c {
// ----
// test() -> 0
// gas irOptimized: 157200
// gas legacy: 188576
// gas legacyOptimized: 183333
// gas legacy: 196732
// gas legacyOptimized: 190664

View File

@ -16,6 +16,6 @@ contract c {
}
// ----
// test1() -> true
// gas irOptimized: 207928
// gas irOptimized: 207933
// gas legacy: 254650
// gas legacyOptimized: 247384

View File

@ -11,5 +11,5 @@ contract c {
// ----
// test() -> 0x20, 29, 0x0303030303030303030303030303030303030303030303030303030303000000
// gas irOptimized: 109341
// gas legacy: 126728
// gas legacyOptimized: 123444
// gas legacy: 134884
// gas legacyOptimized: 131171

View File

@ -17,6 +17,6 @@ contract c {
// ----
// test() -> true
// gas irOptimized: 180539
// gas legacy: 228685
// gas legacyOptimized: 209662
// gas legacy: 238713
// gas legacyOptimized: 218665
// storageEmpty -> 1

View File

@ -16,6 +16,6 @@ contract c {
// ----
// test() ->
// gas irOptimized: 142639
// gas legacy: 164430
// gas legacyOptimized: 158513
// gas legacy: 174458
// gas legacyOptimized: 168008
// storageEmpty -> 1

View File

@ -11,5 +11,5 @@ contract c {
// ----
// test() -> 0x20, 33, 0x303030303030303030303030303030303030303030303030303030303030303, 0x0300000000000000000000000000000000000000000000000000000000000000
// gas irOptimized: 108146
// gas legacy: 125610
// gas legacyOptimized: 122582
// gas legacy: 134000
// gas legacyOptimized: 130530

View File

@ -16,5 +16,5 @@ contract c {
// ----
// test() -> 0
// gas irOptimized: 173650
// gas legacy: 216790
// gas legacyOptimized: 203886
// gas legacy: 226350
// gas legacyOptimized: 212471

View File

@ -21,9 +21,9 @@ contract C {
// ----
// l() -> 0
// g(uint256): 70 ->
// gas irOptimized: 183587
// gas legacy: 183811
// gas legacyOptimized: 179218
// gas irOptimized: 187549
// gas legacy: 200625
// gas legacyOptimized: 195542
// l() -> 70
// a(uint256): 69 -> left(69)
// f() ->

View File

@ -18,5 +18,5 @@ contract C {
// deposit() ->
// ~ emit E(string,uint256[4]): #0xa7fb06bb999a5eb9aff9e0779953f4e1e4ce58044936c2f51c7fb879b85c08bd, #0xe755d8cc1a8cde16a2a31160dcd8017ac32d7e2f13215b29a23cdae40a78aa81
// gas irOptimized: 333479
// gas legacy: 388679
// gas legacyOptimized: 374441
// gas legacy: 410173
// gas legacyOptimized: 394765