diff options
author | Ekaitz Zarraga <ekaitz@elenq.tech> | 2024-01-20 23:36:45 +0100 |
---|---|---|
committer | Ekaitz Zarraga <ekaitz@elenq.tech> | 2024-01-29 11:09:59 +0100 |
commit | c7263571d217e0863d3e62629207659b6aaf2b8b (patch) | |
tree | 10d712de5a7e81e1a6e23141449020e0c855e27a | |
parent | 7f0a28f6ca026a0b2b50282a2168346eef668651 (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.c | 48 |
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. */ |