summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEkaitz Zarraga <ekaitz@elenq.tech>2024-01-20 23:36:45 +0100
committerEkaitz Zarraga <ekaitz@elenq.tech>2024-01-29 11:09:59 +0100
commitc7263571d217e0863d3e62629207659b6aaf2b8b (patch)
tree10d712de5a7e81e1a6e23141449020e0c855e27a
parent7f0a28f6ca026a0b2b50282a2168346eef668651 (diff)
riscv64-gen: Fix `load` and `store` type_size usage
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.
-rw-r--r--riscv64-gen.c48
1 files changed, 45 insertions, 3 deletions
diff --git a/riscv64-gen.c b/riscv64-gen.c
index 8a17b70..7da8714 100644
--- a/riscv64-gen.c
+++ b/riscv64-gen.c
@@ -222,6 +222,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;
@@ -232,7 +274,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;
@@ -313,7 +355,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);
@@ -373,7 +415,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. */