[Tinycc-devel] Fix invalid loads generated by gfunc_return()

[Ziyao]

Hi list,

On riscv64, code below could trigger a SIGSEGV:

struct s {
        long int a;
        long int b;
};

struct s
foo(void)
{
        struct s d;
        struct s *p = &d;
        long int x = 0;
        return *p;
}

int
main(void)
{
        foo();
}

Generated code:
   1151c:       fca43823                sd      a0,-48(s0)
   11520:       fd843503                ld      a0,-40(s0)
   11524:       fca43423                sd      a0,-56(s0)
   11528:       fc843503                ld      a0,-56(s0)
   1152c:       00053503                ld      a0,0(a0)
   11530:       fd043583                ld      a1,-48(s0)
=> 11534:       0005b583                ld      a1,0(a1)

-48(s0) is x and -40(s0) is pointer *p, and the last line leads to a
fault.

When returning the struct to which *p points, generated code actually
loads address from the area contains x and try to dereference it,
which leads to a segfault. (The value x holds, 0x0, is obviously an
invalid memory address)

This could be fixed with a little patch to gfunc_return(). It has been
tested on aarch64 Debian, x86-64 Alpine and riscv64 Debian and
everything worked (but AFAIK TinyCC on aarch64 and x86-64 do not rely
gfunc_return() to handle small structures returned in paired registers)

Many thanks if you could help test this on other architectures/OS.

--
Ziyao

----

From a82aff333774f3bd38841d3c167f1add8c473c1c Mon Sep 17 00:00:00 2001
From: Yao Zi <ziyao@disroot.org>
Date: Sun, 2 Jul 2023 03:58:02 +0800
Subject: [PATCH] Fix invalid load generated by gfunc_return()

  On backends that rely on gfunc_return() to handle structures
  returned in registers (like RISC-V), gfunc_return() may generate
  invalid loads for structures without VT_LOCAL and VT_LVAL. This
  commit fixes it and adds a regression test
  (131_return_struct_in_reg)
---
 tccgen.c                                     |  2 +-
 tests/tests2/131_return_struct_in_reg.c      | 49 ++++++++++++++++++++
 tests/tests2/131_return_struct_in_reg.expect |  3 ++
 3 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 tests/tests2/131_return_struct_in_reg.c
 create mode 100644 tests/tests2/131_return_struct_in_reg.expect

diff --git a/tccgen.c b/tccgen.c
index c582c4a..f91a43d 100644
--- a/tccgen.c
+++ b/tccgen.c
@@ -6622,7 +6622,7 @@ static void gfunc_return(CType *func_type)
             size = type_size(func_type,&align);
             if ((vtop->r != (VT_LOCAL | VT_LVAL) ||
                  (vtop->c.i & (ret_align-1)))
-                && (align & (ret_align-1))) {
+                || (align & (ret_align-1))) {
                 loc = (loc - size) & -ret_align;
                 addr = loc;
                 type = *func_type;
diff --git a/tests/tests2/131_return_struct_in_reg.c 
b/tests/tests2/131_return_struct_in_reg.c
new file mode 100644
index 0000000..911b660
--- /dev/null
+++ b/tests/tests2/131_return_struct_in_reg.c
@@ -0,0 +1,49 @@
+#include<stdio.h>
+
+#define DATA 0x1234567890abcd, 0x5a5aa5a5f0f00f0f
+
+struct s {
+       long int a;
+       long int b;
+};
+
+struct {
+       struct s d;
+} g = { { DATA } }, *gp = &g;
+
+struct s
+foo1(void)
+{
+       struct s d = { DATA };
+       struct s *p = &d;
+       long int x = 0;
+       return *p;
+}
+
+struct s
+foo2(void)
+{
+       long int unused = 0;
+       return gp->d;
+}
+
+struct s
+foo3(void)
+{
+       struct s d = { DATA };
+       long int unused = 0;
+       return d;
+}
+
+int
+main(void)
+{
+       struct s d;
+       d = foo1();
+       printf("%lx %lx\n", d.a, d.b);
+       d = foo2();
+       printf("%lx %lx\n", d.a, d.b);
+       d = foo3();
+       printf("%lx %lx\n", d.a, d.b);
+       return 0;
+}
diff --git a/tests/tests2/131_return_struct_in_reg.expect 
b/tests/tests2/131_return_struct_in_reg.expect
new file mode 100644
index 0000000..301e3b0
--- /dev/null
+++ b/tests/tests2/131_return_struct_in_reg.expect
@@ -0,0 +1,3 @@
+1234567890abcd 5a5aa5a5f0f00f0f
+1234567890abcd 5a5aa5a5f0f00f0f
+1234567890abcd 5a5aa5a5f0f00f0f
--
2.41.0