Functions with assembly containing mcli result in MemoryOverflow when passed a literal

[simonr0204]

For example, in order to call :

    fn from(bits: b256) -> EthAddress{
        asm(r1: bits) {
            mcli r1 i12;
        };
        ...
        ...
    }

the parameter must first be assigned to a variable. :

    let temp= 0x... ; 
    let address = ~EthAddress::from(temp);

otherwise a MemoryOverflow error occurs.

[otrho]

Here's a full repro showing that yep, the passed value must be in a local variable and not passed as a literal.

script;

fn f(bits: b256) {
  asm(r1: bits) {
    mcli r1 i12;
  };
}

fn main() {
    // This works.
    let b = 0xABABABABABABABABABABABABABABABABABABABABABABABABABABABABABABABAB;
    f(b);

    // This fails.
    f(0xCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCD);
}

The second call is trying to use a literal which is referenced directly in the data section which is not stack memory and therefore causes the MemoryOverflow.

[adlerjohn]

I wouldn't call this a bug, since it's expected behavior than you can't clear memory outside your owned stack or heap range.

[otrho]

It's a bit tricky for Sway devs with the implicit by-reference semantics, versus having explicit references in the language.

We tend to say anything that doesn't fit in a register is passed around by reference, but the Sway dev doesn't want to think about whether it was allocated somewhere on the stack by the compiler or not.

So I think the above should work, and the inliner needs to put all args onto the stack by default, and later if we find that copying it is unnecessary we can optimise it away.

Once we're back to non-critical work I'm very keen to update the memory model we currently use in the IR, and this fix fits into that scope of work.

[mohammadfawaz]

So I think the above should work, and the inliner needs to put all args onto the stack by default, and later if we find that copying it is unnecessary we can optimise it away.

This is reasonable I think. If we do this, then only devs writing asm blocks would potentially face the issue above and that's probably okay (unless an asm block follows the same behavior as functions and put its args onto the stack?).

E.g.:

script;

fn main() {
    asm(r1: 0xCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCDCD) {
        mcli r1 i12;
    };
}

[otrho]

Ugh, even then I feel like r1 should be a reference to something... it's not unreasonable to want this to work. At the very least it should be a compile error rather than a panic.

[mohammadfawaz]

Yes absolutely... I wanted to say that legal code should never panic (unless intentional, e.g.: assert), but we can't ever make that promise to someone writing asm blocks.

[mohammadfawaz]

Some updates from: #2217 (comment)

I'm starting to think that this behavior should be disallowed by implementing mutability checking in asm blocks AND disallow passing literals (which are basically constants) as mut to function arguments and asm blocks (once mut is supported for both).

[otrho]

That would work too. 🙂