[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
- [PATCH] riscv64-gen: Fix `load` and `store` type_size usage,
Ekaitz Zarraga <=