Relax the definition of memory safety in the documentation.

[ekpyron]

Came up in https://forum.soliditylang.org/t/non-memory-safe-assembly/2368.

De-facto this is already safe given the stack-to-memory logic. It's probably reasonable to strongly commit to this, guarantee this to be safe and consider changing this a breaking change.

[cameel]

Wouldn't messing with the zero slot be dangerous if the assembly accesses dynamic Solidity arrays that point at it due to not being explicitly initialized? Writing anything to it, even only for the duration of the block, not only changes their value, but potentially extends them to overlap the area reserved for variables moved to memory.

This seems pretty unsafe to me. And even if we'd rather just consider that a part of the internals that a user of the inline assembly should be aware of, I think that we should at least add a warning about it here.

[ekpyron]

Yeah, that should definitely be made clearer (even though it really doesn't have anything to do with memory-safety in the defined sense).

[ekpyron]

I added a paragraph about it - but yeah, we could also define the zero pointer as fully off limits.

[cameel]

True. I guess it's not unsafe by itself, unless the user actually reads or writes such an arrays in the block. Still, it can lead to accidental unsafe memory use very easily.

Also, now that I think of it, changing memory pointer could lead to a similar mistake - if it happens to point somewhere inside the reserved area and the user tries to "allocate" memory on their own. Though that one would be more obviously user error. But it could become less obvious and more likely in the future if we allow using global functions in inline assembly.

[ekpyron]

Yeah - I mean, this is why we didn't specifically declare this as safe so far. I'd still not necessarily say that we have to - but the fact is that people are doing it, so it might be better to be very clear about if and when this is valid. But yeah, we can also clarify this further and produce some dangerous examples and such.
But yeah, that's also why I already didn't add this to the main section, but to a separate "advanced" section :-).

[0xth0mas]

Could you please clarify how Solidity would use 0x60 inside of an assembly block to initialize a dynamic array? I have been under the impression that all memory management within an assembly block is explicitly performed by mload, mstore and mcopy (or calls to identity precompile) operations and that Solidity does not support dynamic memory arrays. Is there a current situation where Solidity reads or writes values from memory in an assembly block without an explicit command?

It also seems odd to use 0x60 to get a zero value which would require PUSH1 0x60 MLOAD (6 gas, 3 bytes of bytecode) versus PUSH1 0x00 (3 gas, 2 bytes of bytecode) or PUSH0 (2 gas, 1 byte of bytecode) depending on target EVM version.

[0xth0mas]

Michael Amadi sent me the explanation for 0x60 in a dev chat - 0x60 would be the stack value for the memory pointer location of a zero length array so that when the length is read it returns zero. That makes sense but the use of dynamic memory langauge is confusing since it's actually an uninitialized static length array.

[AmadiMichael]

On your last question, from my understanding, solidity uses 0x60 as zero slot because dynamic values have to hold a stack value that points to somewhere in memory (the offset to the length's memory offset) and if it points to somewhere that holds 0x00 e.g 0x60, then it assumes the len is stores 0x00 bytes away which is still 0x60 and when it reads this len it returns 0 since 0x60 still holds 0 and so returns a 0 len dynamic type.

[AmadiMichael]

I actually think it should be considered memory safe to overwrite 0x60 if all dynamic memory types within call scope are initialized I.e assigned a value (0 length or not). From my understanding, it's when the types are not assigned to that they point to 0x60.

By initialized I mean assigned a value even if it's a zero length value since once assigned solidity allocates memory for it

// this is allocated memory
string memory A = "";
bytes memory B = new bytes(0);
uint256[] memory C = new uint256[](len);

// this points to 0x60
string memory A;
bytes memory B;
uint256[] memory C;

What are your thoughts on it?
Is it too prone to foot guns?

[ekpyron]

To clarify the concern here: Solidity will, of course, not initialize any dynamic array within an assembly block. But an existing array in memory may have 0x60 as a value, which you need to take into account when overwriting the value.

So consider

function f(bool c) public {
  uint[] memory x;
  if (c) {
    x = ...;
  }

  assembly {
    function hashUintArray(ptr) -> y {
       let length := mload(ptr)
       let dataArea := add(ptr, 32)
       y := keccak256(dataArea, add(dataArea, mul(length, 32)))
    }
    mstore(0x60, ...)
    mstore(0x40, hashUintArray(x))
    revert(0x40, 0x40)
  }
}

Now, whenver c is false, whatever you've written to 0x60 will be interpreted as uint array in memory and hashed - which is not the contents of x.

This has technically nothing to do with "memory-safety" in the narrow sense defined in the docs, which is concerned with not overwriting areas the compiler may use to move stack variables to memory if it runs out of stack space (which can happen for variables in the assembly block).

But @cameel pointed out that it's worth mentioning when talking about overwriting the zero value at 0x60.
Even worse, if the assembly block doesn't terminate as in the example, any past and future accesses to any previously uninitialized memory array will read from whatever you wrote to 0x60 instead of being treated as empty array.

So, to cover all possible cases, you need to make sure that both:

• If the assembly block does not terminate, i.e. you move back to Solidity code, memory offset 0x60 needs to have a value of zero again at the end of the assembly block, otherwise behaviour is undefined.
• During the assembly block while you have memory offset 0x60 overwritten, you need to be aware that, if you manually read from Solidity memory arrays yourself in that assembly block, you can get unintended results (arrays that should be empty can have arbitrary content depending on what you wrote to 0x60).

[cameel]

Looks good to me.

I'd probably put the warning at the end in a RST warning block, but I don't want into nitpicking about details now. The content itself seems sound.

[github-actions]

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

[github-actions]

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.

[github-actions]

This pull request is stale because it has been open for 14 days with no activity.
It will be closed in 7 days unless the stale label is removed.