fix[codegen]: fix non-memory reason strings (vyperlang#3877)

fix code generation for non-memory reason strings. previously, the byte
array copier would fail trying to produce code for the buf, which was an
int rather than an IRnode. this commit cleans up the revert generation
code, allocates a fresh buffer to avoid memory cleanliness issues, and
refactors the code to use abi_encoder code instead of handrolling the
ABI encoding routine.

---------

Co-authored-by: Charles Cooper <[email protected]>

.github/workflows/pull-request.yaml

@@ -34,6 +34,7 @@ jobs:
           # ux: language changes (UX)
           # tool: integration
           # ir: (old) IR/codegen changes
+          # codegen: lowering from vyper AST to codegen
           # venom: venom changes
           scopes: |
             ci
@@ -43,6 +44,7 @@ jobs:
             ux
             tool
             ir
+            codegen
             venom
           requireScope: true
           subjectPattern: '^(?![A-Z]).+$'

tests/functional/codegen/features/test_assert.py

@@ -28,6 +28,8 @@ def foo():
 
 def test_assert_reason(w3, get_contract_with_gas_estimation, tx_failed, memory_mocker):
     code = """
+err: String[32]
+
 @external
 def test(a: int128) -> int128:
     assert a > 1, "larger than one please"
@@ -43,6 +45,17 @@ def test2(a: int128, b: int128, extra_reason: String[32]) -> int128:
 @external
 def test3(reason_str: String[32]):
     raise reason_str
+
+@external
+def test4(a: int128, reason_str: String[32]) -> int128:
+    self.err = reason_str
+    assert a > 1, self.err
+    return 1 + a
+
+@external
+def test5(reason_str: String[32]):
+    self.err = reason_str
+    raise self.err
     """
     c = get_contract_with_gas_estimation(code)
 
@@ -66,6 +79,15 @@ def test3(reason_str: String[32]):
         c.test3("An exception")
     assert _fixup_err_str(e_info.value.args[0]) == "An exception"
 
+    assert c.test4(2, "msg") == 3
+    with pytest.raises(TransactionFailed) as e_info:
+        c.test4(0, "larger than one again please")
+    assert _fixup_err_str(e_info.value.args[0]) == "larger than one again please"
+
+    with pytest.raises(TransactionFailed) as e_info:
+        c.test5("A storage exception")
+    assert _fixup_err_str(e_info.value.args[0]) == "A storage exception"
+
 
 invalid_code = [
     """

vyper/codegen/stmt.py

@@ -1,6 +1,7 @@
 import vyper.codegen.events as events
 import vyper.utils as util
 from vyper import ast as vy_ast
+from vyper.codegen.abi_encoder import abi_encode
 from vyper.codegen.context import Constancy, Context
 from vyper.codegen.core import (
     LOAD,
@@ -9,20 +10,14 @@
     clamp_le,
     get_dyn_array_count,
     get_element_ptr,
-    make_byte_array_copier,
+    get_type_for_exact_size,
     make_setter,
-    zero_pad,
+    wrap_value_for_external_return,
 )
 from vyper.codegen.expr import Expr
 from vyper.codegen.return_ import make_return_stmt
 from vyper.evm.address_space import MEMORY, STORAGE
-from vyper.exceptions import (
-    CodegenPanic,
-    CompilerPanic,
-    StructureException,
-    TypeCheckFailure,
-    tag_exceptions,
-)
+from vyper.exceptions import CodegenPanic, StructureException, TypeCheckFailure, tag_exceptions
 from vyper.semantics.types import DArrayT
 from vyper.semantics.types.shortcuts import UINT256_T
 
@@ -132,39 +127,26 @@ def _assert_reason(self, test_expr, msg):
         finally:
             self.context.constancy = tmp
 
-        # TODO this is probably useful in codegen.core
-        # compare with eval_seq.
-        def _get_last(ir):
-            if len(ir.args) == 0:
-                return ir.value
-            return _get_last(ir.args[-1])
-
-        # TODO maybe use ensure_in_memory
-        if msg_ir.location != MEMORY:
-            buf = self.context.new_internal_variable(msg_ir.typ)
-            instantiate_msg = make_byte_array_copier(buf, msg_ir)
-        else:
-            buf = _get_last(msg_ir)
-            if not isinstance(buf, int):  # pragma: nocover
-                raise CompilerPanic(f"invalid bytestring {buf}\n{self}")
-            instantiate_msg = msg_ir
+        msg_ir = wrap_value_for_external_return(msg_ir)
+        bufsz = 64 + msg_ir.typ.memory_bytes_required
+        buf = self.context.new_internal_variable(get_type_for_exact_size(bufsz))
 
         # offset of bytes in (bytes,)
         method_id = util.method_id_int("Error(string)")
 
-        # abi encode method_id + bytestring
-        assert buf >= 36, "invalid buffer"
-        # we don't mind overwriting other memory because we are
-        # getting out of here anyway.
-        _runtime_length = ["mload", buf]
-        revert_seq = [
-            "seq",
-            instantiate_msg,
-            zero_pad(buf),
-            ["mstore", buf - 64, method_id],
-            ["mstore", buf - 32, 0x20],
-            ["revert", buf - 36, ["add", 4 + 32 + 32, ["ceil32", _runtime_length]]],
-        ]
+        # abi encode method_id + bytestring to `buf+32`, then
+        # write method_id to `buf` and get out of here
+        payload_buf = buf + 32
+        bufsz -= 32  # reduce buffer by size of `method_id` slot
+        encoded_length = abi_encode(payload_buf, msg_ir, self.context, bufsz, returns_len=True)
+        with encoded_length.cache_when_complex("encoded_len") as (b1, encoded_length):
+            revert_seq = [
+                "seq",
+                ["mstore", buf, method_id],
+                ["revert", buf + 28, ["add", 4, encoded_length]],
+            ]
+            revert_seq = b1.resolve(revert_seq)
+
         if is_raise:
             ir_node = revert_seq
         else: