[PATCH olang] fix: codegen: prevent stack overwrite

[PATCH olang] fix: codegen: prevent stack overwrite

[Carlos Maniero]

There was an issue in the stack allocation algorithm. Consider this
function:

  fn a(): u32 {
    var a: u32 = 0xAAAA
    var b: u64 = 0xBBBBBBBB

    ret
  }

There are three information the stack is required to store:

- 8 bytes: rip (from call instruction)
- 4 bytes: a
- 8 bytes: b

The 0x7FFFFF07 memory address was used to represent the RIP value at
call instant.

Our codegen was assuming the stack works that way:

0       -8  -C
^-------^---^-------
7FFFFF07AAAABBBBBBBB
^-------^---^-------
rip     a   b

So the code gen was:

- Adding the value at the stack;
- Increasing the offset.

But actually the stack was behaving as following:

8       0       -8  -C
^-------^-------^---^
7FFFFF070000BBBBBBBB.
^---------------^---^
rip             a   b

Once the instruction *mov %rax, -0xC(%rbp)* writes from -0xC(%rbp)
(exclusive) to -0x4(%rbp) (inclusive).

So after this change, this is the actual stack template:

        0   -4      -C
--------^---^-------^
7FFFFF07AAAABBBBBBBB.
--------^---^-------^
      rip   a       b

Signed-off-by: Carlos Maniero <carlos@maniero.me>
---
 src/codegen_linux_x86_64.c          | 13 ++++++-------
 tests/olc/0036_variable_overflow.ol | 30 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 7 deletions(-)
 create mode 100644 tests/olc/0036_variable_overflow.ol

diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
index fc8fcc4..83d1d2c 100644
--- a/src/codegen_linux_x86_64.c
+++ b/src/codegen_linux_x86_64.c
@@ -28,7 +28,6 @@
 
 // The call instruction pushes EIP into stack so the first 8 bytes from stack
 // must be preserved else the ret instruction will jump to nowere.
-#define X86_CALL_EIP_STACK_OFFSET (8)
 #define X86_CALL_ARG_SIZE 6
 
 #define bytes_max(a, b) ((a) > (b) ? (a) : (b))
@@ -795,6 +794,9 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
                 symbol_t *symbol = scope_lookup(scope, var_def.id);
                 assert(symbol);
 
+                size_t type_size = type_to_bytes(symbol->type);
+                codegen->base_offset += type_size;
+
                 codegen_linux_x86_64_put_stack_offset(
                     codegen, symbol, codegen->base_offset);
 
@@ -803,13 +805,10 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
                                                          var_def.value);
                 }
 
-                size_t type_size = type_to_bytes(symbol->type);
-
                 fprintf(codegen->out,
                         "    mov %s, -%ld(%%rbp)\n",
                         get_reg_for(REG_ACCUMULATOR, type_size),
                         codegen->base_offset);
-                codegen->base_offset += type_size;
 
                 break;
             }
@@ -957,7 +956,7 @@ static void
 codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen,
                                    ast_fn_definition_t *fn_def)
 {
-    codegen->base_offset = X86_CALL_EIP_STACK_OFFSET;
+    codegen->base_offset = 0;
 
     ast_node_t *block_node = fn_def->block;
     fprintf(codegen->out, "" SV_FMT ":\n", SV_ARG(fn_def->id));
@@ -975,6 +974,8 @@ codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen,
         symbol_t *symbol = scope_lookup(fn_def->scope, param->id);
         assert(symbol);
 
+        // FIXME: add offset according to the param size
+        codegen->base_offset += 8;
         size_t offset = codegen->base_offset;
 
         codegen_linux_x86_64_put_stack_offset(
@@ -986,8 +987,6 @@ codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen,
                 get_reg_for(x86_call_args[i], symbol->type->as_primitive.size),
                 offset);
 
-        // FIXME: add offset according to the param size
-        codegen->base_offset += 8;
         ++i;
     }
 
diff --git a/tests/olc/0036_variable_overflow.ol b/tests/olc/0036_variable_overflow.ol
new file mode 100644
index 0000000..edb3c7e
--- /dev/null
+++ b/tests/olc/0036_variable_overflow.ol
@@ -0,0 +1,30 @@
+# Copyright (C) 2024 olang mantainers
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+fn main(): u32 {
+  var a: u32 = 0
+  var b: u64 = 0
+  var c: u32 = 0
+
+  # This operation will fill all bits in b location.
+  # If there is an overflow, both a or c would be impacted
+  b = ~b
+
+  return a + c
+}
+
+# TEST test_compile(exit_code=0)
+
+# TEST test_run_binary(exit_code=0)

base-commit: cf5e4abf07a38f0ddf3ac6979b01b942ab99a691
-- 
2.46.1

[olang/patches/.build.yml] build failed

[builds.sr.ht]

olang/patches/.build.yml: FAILED in 28s

[fix: codegen: prevent stack overwrite][0] from [Carlos Maniero][1]

[0]: https://lists.sr.ht/~johnnyrichard/olang-devel/patches/55483
[1]: mailto:carlos@maniero.me

✗ #1350731 FAILED olang/patches/.build.yml https://builds.sr.ht/~johnnyrichard/job/1350731

Re: [olang/patches/.build.yml] build failed

[Carlos Maniero]

There is an issue unrelated to my changes. The spec that does not allows
comments inside.

I sent another patch [1] that will fix the spec.

[1]: Message-ID: <20241015225750.211129-2-carlos@maniero.me>.

Re: [PATCH olang] fix: codegen: prevent stack overwrite

[Johnny Richard]

Thanks. applied!

Lovely patch description by the way <3

Build started: https://builds.sr.ht/~johnnyrichard/job/1352096

To git.sr.ht:~johnnyrichard/olang
   f0c9d1c..df8420f  main -> main