bug-mes
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] riscv64-gen: Fix `load` and `store` type_size usage


From: Ekaitz Zarraga
Subject: [PATCH] riscv64-gen: Fix `load` and `store` type_size usage
Date: Mon, 29 Jan 2024 13:09:38 +0100

In `load` and `store` RISC-V gen used `type_size` to retrieve the size
of the types being moved around, the problem with that function is it
tries to obtain internal information of the type. When using `type_size`
with a `char *` it returns 1, and emits a `lb` instruction when using a
conditional expression like so (but probably also in other cases):

    char *a, *b;
    b = "hello";
    a = x ? b : "" ;                // this emits an `lb`

That `lb` clobbers the pointer, only loading the latest byte of it:

    // if `b` was, say: 0x1f822e
    a = b;
    // now `a` is 0x00002e

NOTE: We spotted this when building make-3.82, in `init_switches`
      function in the `main.c` file. This error made `make` unable to
      run long options like `make --help` (segfault when doing the
      `strlen` later)

This happens because a `char *` is internally stored as a `char[1]` or
something similar, it's result is the size of a `char` (1) times the
size of the array `1`. This is not what we want, we are copying the
pointer, not the array itself. Using `type_size` for this is not
appropriate even if it works for some cases.

If the conditional expression is rewritten to imperative style using an
`if` it works properly, so it might be related with the fact the pointer
is `load`ed to a register.

`load` and `store` should only work with integral types, so reading the
size of the array is not useful. This commit creates a simpler version
of this function that only reads the integral type of what's working
with: char * is considered just a pointer. So it makes an `ld` instead,
and the code above works.
---
 riscv64-gen.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/riscv64-gen.c b/riscv64-gen.c
index 33bf36d9..7914262b 100644
--- a/riscv64-gen.c
+++ b/riscv64-gen.c
@@ -351,6 +351,48 @@ static void load_large_constant(int rr, int fc, uint32_t 
pi)
     EI(0x13, 1, rr, rr, 8); // slli RR, RR, 8
 }
 
+ST_FUNC int type_size_integral(CType *type, int *a){
+    int bt;
+    bt = type->t & VT_BTYPE;
+    switch (bt){
+        case VT_VOID:
+        case VT_BOOL:
+        case VT_BYTE:
+            *a = 1;
+            return 1;
+        case VT_SHORT:
+            *a = 2;
+            return 2;
+        case VT_LLONG:
+        case VT_DOUBLE:
+            *a = 8;
+            return 8;
+        case VT_PTR:
+        case VT_FUNC:
+            *a = PTR_SIZE;
+            return PTR_SIZE;
+        case VT_FLOAT:
+        case VT_INT:
+            *a = 4;
+            return 4;
+        case VT_LDOUBLE:
+            *a = LDOUBLE_ALIGN;
+            return LDOUBLE_SIZE;
+        case VT_QLONG:
+        case VT_QFLOAT:
+            *a = 8;
+            return 16;
+        case VT_ENUM:
+            *a = 4;
+            /* Enums might be incomplete, so don't just return '4' here.  */
+            return type->ref->c;
+        default:
+            /* VT_STRUCT and VT_UNION */
+            tcc_error("load/store with a non integral");
+            return type_size(type, a);
+    }
+}
+
 ST_FUNC void load(int r, SValue *sv)
 {
     int fr = sv->r;
@@ -361,7 +403,7 @@ ST_FUNC void load(int r, SValue *sv)
     int align, size;
     if (fr & VT_LVAL) {
         int func3, opcode = is_freg(r) ? 0x07 : 0x03, br;
-        size = type_size(&sv->type, &align);
+        size = type_size_integral(&sv->type, &align);
         assert (!is_freg(r) || bt == VT_FLOAT || bt == VT_DOUBLE);
         if (bt == VT_FUNC) /* XXX should be done in generic code */
           size = PTR_SIZE;
@@ -442,7 +484,7 @@ ST_FUNC void load(int r, SValue *sv)
           EI(0x13, 0, rr, ireg(v), 0); // addi RR, V, 0 == mv RR, V
         else {
             int func7 = is_ireg(r) ? 0x70 : 0x78;
-            size = type_size(&sv->type, &align);
+            size = type_size_integral(&sv->type, &align);
             if (size == 8)
               func7 |= 1;
             assert(size == 4 || size == 8);
@@ -503,7 +545,7 @@ ST_FUNC void store(int r, SValue *sv)
     int rr = is_ireg(r) ? ireg(r) : freg(r), ptrreg;
     int fc = sv->c.i;
     int bt = sv->type.t & VT_BTYPE;
-    int align, size = type_size(&sv->type, &align);
+    int align, size = type_size_integral(&sv->type, &align);
     assert(!is_float(bt) || is_freg(r) || bt == VT_LDOUBLE);
     /* long doubles are in two integer registers, but the load/store
        primitives only deal with one, so do as if it's one reg.  */
-- 
2.41.0




reply via email to

[Prev in Thread] Current Thread [Next in Thread]