* [PATCH 0/2] tcg/riscv64: Fix AUIPC pair range validation @ 2026-09-03 17:04 Max Chou 2026-09-03 17:04 ` [PATCH 1/2] tcg/riscv64: Validate AUIPC relocation range Max Chou 2026-09-03 17:04 ` [PATCH 2/2] tcg/riscv64: Fall back when AUIPC pairs are out of range Max Chou 0 siblings, 2 replies; 5+ messages in thread From: Max Chou @ 2026-09-03 17:04 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: Palmer Dabbelt, Alistair Francis, Daniel Henrique Barboza, Richard Henderson, Max Chou This patchset tries to fix the AUIPC-pair range checking issue in current TCG riscv64 backend. The issue will be triggered in the following example. --- Assumptions - AUIPC at 0x00007fff77fa1258 - target call at 0x00007ffff7fa12d6 The direct pc-relative displacement from the AUIPC at 0x00007fff77fa1258 is 0x000000008000007e, so it is already outside the signed 32-bit range. tcg_out_call_int therefore takes its far-call path: it separates the JALR immediate (0x2d6) and asks tcg_out_movi to materialize this page-aligned base: target call = 0x00007ffff7fa12d6 JALR lo = 0x2d6 base = 0x00007ffff7fa1000 base - AUIPC = 0x000000007ffffda8 The final value, 0x7ffffda8, is less than INT32_MAX. The current range therefore accepts it, but that is not sufficient: the signed low 12-bit immediate must be removed before the value can be encoded in AUIPC. The current reloc_call performs that split as follows: int32_t lo = sextreg(offset, 0, 12); int32_t hi = offset - lo; For the captured placement, the values required by the split are: offset = 0x000000007ffffda8 lo = 0xfffffffffffffda8 hi = 0x0000000080000000 The lo is representable by the ADDI immediate. But the hi is not representable by int32_t: narrowing it produces the bit pattern 0x80000000, which is -0x80000000 as a signed 32-bit value. Thus the int32_t split can accept a wrapped upper value instead of proving that the positive upper contribution required by AUIPC is representable. AUIPC has a 20-bit immediate which it shifts left by 12 and sign-extends. Consequently, an encoded immediate of 0x80000 means -0x80000000, not the required +0x80000000. The resulting generated code will be: 0x00007fff77fa1258: auipc t6,-524288 0x00007fff77fa125c: addi t6,t6,-600 0x00007fff77fa1260: jalr ra,t6,726 It computes the base as 0x00007ffef7fa1000 and transfers to 0x00007ffef7fa12d6, exactly 4 GiB below the requested callback which is 0x00007ffff7fa12d6. --- This patchset addresses the issue of AUIPC split checking and applies the corrected split. It ensures that the split is checked before emitting AUIPC/ADDI and AUIPC/JALR pairs in tcg_out_[movi|call_int]. rnax Max Chou (2): tcg/riscv64: Validate AUIPC relocation range tcg/riscv64: Fall back when AUIPC pairs are out of range tcg/riscv64/tcg-target.c.inc | 68 ++++++++++++++++++++++++++---------- 1 file changed, 49 insertions(+), 19 deletions(-) -- 2.43.7 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] tcg/riscv64: Validate AUIPC relocation range 2026-09-03 17:04 [PATCH 0/2] tcg/riscv64: Fix AUIPC pair range validation Max Chou @ 2026-09-03 17:04 ` Max Chou 2026-09-03 17:31 ` Philippe Mathieu-Daudé 2026-09-03 17:04 ` [PATCH 2/2] tcg/riscv64: Fall back when AUIPC pairs are out of range Max Chou 1 sibling, 1 reply; 5+ messages in thread From: Max Chou @ 2026-09-03 17:04 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: Palmer Dabbelt, Alistair Francis, Daniel Henrique Barboza, Richard Henderson, Max Chou reloc_call splits a PC-relative offset into a signed 12-bit low immediate and an AUIPC contribution. When the low immediate carries into bit 31, the existing 32-bit arithmetic can accept an unencodable positive 0x80000000 AUIPC contribution. Keep the split in pointer-width arithmetic and reject it unless the rounded upper contribution is representable as signed 32-bit. Factor this validation into split_auipc_offset so reloc_call retains its existing failure contract. Fixes: dfa8e74f9463 ("tcg/riscv: Add the relocation functions") Signed-off-by: Max Chou <max.chou@sifive.com> --- tcg/riscv64/tcg-target.c.inc | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc index a439ba5c20e..1c41f8ffade 100644 --- a/tcg/riscv64/tcg-target.c.inc +++ b/tcg/riscv64/tcg-target.c.inc @@ -634,14 +634,31 @@ static bool reloc_jimm20(tcg_insn_unit *src_rw, const tcg_insn_unit *target) return false; } -static bool reloc_call(tcg_insn_unit *src_rw, const tcg_insn_unit *target) +static bool split_auipc_offset(tcg_insn_unit *src_rw, + const tcg_insn_unit *target, + intptr_t *hi, intptr_t *lo) { const tcg_insn_unit *src_rx = tcg_splitwx_to_rx(src_rw); intptr_t offset = (intptr_t)target - (intptr_t)src_rx; - int32_t lo = sextreg(offset, 0, 12); - int32_t hi = offset - lo; + intptr_t low = sextreg(offset, 0, 12); + intptr_t high = offset - low; + + /* AUIPC sign-extends its 20-bit immediate after shifting by 12. */ + if (high != sextreg(high, 0, 32)) { + return false; + } + + *hi = high; + *lo = low; + + return true; +} + +static bool reloc_call(tcg_insn_unit *src_rw, const tcg_insn_unit *target) +{ + intptr_t hi, lo; - if (offset == hi + lo) { + if (split_auipc_offset(src_rw, target, &hi, &lo)) { src_rw[0] |= encode_uimm20(hi); src_rw[1] |= encode_imm12(lo); return true; -- 2.43.7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] tcg/riscv64: Validate AUIPC relocation range 2026-09-03 17:04 ` [PATCH 1/2] tcg/riscv64: Validate AUIPC relocation range Max Chou @ 2026-09-03 17:31 ` Philippe Mathieu-Daudé 2026-09-04 7:56 ` Max Chou 0 siblings, 1 reply; 5+ messages in thread From: Philippe Mathieu-Daudé @ 2026-09-03 17:31 UTC (permalink / raw) To: Max Chou, qemu-devel, qemu-riscv Cc: Palmer Dabbelt, Alistair Francis, Daniel Henrique Barboza, Richard Henderson On 3/9/26 19:04, Max Chou wrote: > reloc_call splits a PC-relative offset into a signed 12-bit low > immediate and an AUIPC contribution. When the low immediate carries into > bit 31, the existing 32-bit arithmetic can accept an unencodable > positive 0x80000000 AUIPC contribution. > > Keep the split in pointer-width arithmetic and reject it unless the > rounded upper contribution is representable as signed 32-bit. Factor > this validation into split_auipc_offset so reloc_call retains its > existing failure contract. > > Fixes: dfa8e74f9463 ("tcg/riscv: Add the relocation functions") > Signed-off-by: Max Chou <max.chou@sifive.com> > --- > tcg/riscv64/tcg-target.c.inc | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc > index a439ba5c20e..1c41f8ffade 100644 > --- a/tcg/riscv64/tcg-target.c.inc > +++ b/tcg/riscv64/tcg-target.c.inc > @@ -634,14 +634,31 @@ static bool reloc_jimm20(tcg_insn_unit *src_rw, const tcg_insn_unit *target) > return false; > } > > -static bool reloc_call(tcg_insn_unit *src_rw, const tcg_insn_unit *target) > +static bool split_auipc_offset(tcg_insn_unit *src_rw, > + const tcg_insn_unit *target, > + intptr_t *hi, intptr_t *lo) > { > const tcg_insn_unit *src_rx = tcg_splitwx_to_rx(src_rw); > intptr_t offset = (intptr_t)target - (intptr_t)src_rx; > - int32_t lo = sextreg(offset, 0, 12); > - int32_t hi = offset - lo; > + intptr_t low = sextreg(offset, 0, 12); > + intptr_t high = offset - low; > + > + /* AUIPC sign-extends its 20-bit immediate after shifting by 12. */ > + if (high != sextreg(high, 0, 32)) { > + return false; > + } > + > + *hi = high; > + *lo = low; > + > + return true; > +} > + > +static bool reloc_call(tcg_insn_unit *src_rw, const tcg_insn_unit *target) > +{ > + intptr_t hi, lo; > > - if (offset == hi + lo) { > + if (split_auipc_offset(src_rw, target, &hi, &lo)) { > src_rw[0] |= encode_uimm20(hi); > src_rw[1] |= encode_imm12(lo); I'm a bit confused by these encode_*imm() taking uint32_t arguments, otherwise your change LGTM: Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] tcg/riscv64: Validate AUIPC relocation range 2026-09-03 17:31 ` Philippe Mathieu-Daudé @ 2026-09-04 7:56 ` Max Chou 0 siblings, 0 replies; 5+ messages in thread From: Max Chou @ 2026-09-04 7:56 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-devel, qemu-riscv, Palmer Dabbelt, Alistair Francis, Daniel Henrique Barboza, Richard Henderson On 2026-09-03 19:31, Philippe Mathieu-Daudé wrote: > > +static bool reloc_call(tcg_insn_unit *src_rw, const tcg_insn_unit *target) > > +{ > > + intptr_t hi, lo; > > - if (offset == hi + lo) { > > + if (split_auipc_offset(src_rw, target, &hi, &lo)) { > > src_rw[0] |= encode_uimm20(hi); > > src_rw[1] |= encode_imm12(lo); > > I'm a bit confused by these encode_*imm() taking uint32_t arguments, > otherwise your change LGTM: > Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> > Hi Philippe, Thanks for the feedback. I’ll address that data type mismatch and send out a v2. rnax ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] tcg/riscv64: Fall back when AUIPC pairs are out of range 2026-09-03 17:04 [PATCH 0/2] tcg/riscv64: Fix AUIPC pair range validation Max Chou 2026-09-03 17:04 ` [PATCH 1/2] tcg/riscv64: Validate AUIPC relocation range Max Chou @ 2026-09-03 17:04 ` Max Chou 1 sibling, 0 replies; 5+ messages in thread From: Max Chou @ 2026-09-03 17:04 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: Palmer Dabbelt, Alistair Francis, Daniel Henrique Barboza, Richard Henderson, Max Chou tcg_out_movi and tcg_out_call_int only check whether a PC-relative displacement is signed 32-bit before emitting an AUIPC pair. This does not account for rounding the signed 12-bit immediate, which can require an unencodable positive 0x80000000 AUIPC contribution. Validate the split and emit the pair only when it fits. Otherwise, tcg_out_movi keeps its existing full-address materialization and tcg_out_call_int uses its existing indirect far-call sequence. Fixes: dfa8e74f9463 ("tcg/riscv: Add the relocation functions") Signed-off-by: Max Chou <max.chou@sifive.com> --- tcg/riscv64/tcg-target.c.inc | 43 +++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc index 1c41f8ffade..12efb44c3d7 100644 --- a/tcg/riscv64/tcg-target.c.inc +++ b/tcg/riscv64/tcg-target.c.inc @@ -667,6 +667,21 @@ static bool reloc_call(tcg_insn_unit *src_rw, const tcg_insn_unit *target) return false; } +static bool tcg_out_auipc_pair(TCGContext *s, const tcg_insn_unit *target, + RISCVInsn opc, TCGReg base, + TCGReg rd, TCGReg rs1) +{ + intptr_t hi, lo; + + if (!split_auipc_offset(s->code_ptr, target, &hi, &lo)) { + return false; + } + tcg_out_opc_upper(s, OPC_AUIPC, base, hi); + tcg_out_opc_imm(s, opc, rd, rs1, lo); + + return true; +} + static bool patch_reloc(tcg_insn_unit *code_ptr, int type, intptr_t value, intptr_t addend) { @@ -803,7 +818,8 @@ static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd, tcg_target_long val) { tcg_target_long lo, hi, tmp; - int shift, ret; + int shift; + bool ret; if (type == TCG_TYPE_I32) { val = (int32_t)val; @@ -824,12 +840,9 @@ static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd, return; } - tmp = tcg_pcrel_diff(s, (void *)val); - if (tmp == (int32_t)tmp) { - tcg_out_opc_upper(s, OPC_AUIPC, rd, 0); - tcg_out_opc_imm(s, OPC_ADDI, rd, rd, 0); - ret = reloc_call(s->code_ptr - 2, (const tcg_insn_unit *)val); - tcg_debug_assert(ret == true); + ret = tcg_out_auipc_pair(s, (const tcg_insn_unit *)val, OPC_ADDI, rd, rd, + rd); + if (ret) { return; } @@ -1583,7 +1596,7 @@ static void tcg_out_call_int(TCGContext *s, const tcg_insn_unit *arg, bool tail) { TCGReg link = tail ? TCG_REG_ZERO : TCG_REG_RA; ptrdiff_t offset = tcg_pcrel_diff(s, arg); - int ret; + bool ret; init_setting_vtype(s); @@ -1591,14 +1604,14 @@ static void tcg_out_call_int(TCGContext *s, const tcg_insn_unit *arg, bool tail) if (offset == sextreg(offset, 0, 20)) { /* short jump: -2097150 to 2097152 */ tcg_out_opc_jump(s, OPC_JAL, link, offset); - } else if (offset == (int32_t)offset) { - /* long jump: -2147483646 to 2147483648 */ - tcg_out_opc_upper(s, OPC_AUIPC, TCG_REG_TMP0, 0); - tcg_out_opc_imm(s, OPC_JALR, link, TCG_REG_TMP0, 0); - ret = reloc_call(s->code_ptr - 2, arg); - tcg_debug_assert(ret == true); } else { - /* far jump: 64-bit */ + ret = tcg_out_auipc_pair(s, arg, OPC_JALR, TCG_REG_TMP0, link, + TCG_REG_TMP0); + if (ret) { + return; + } + + /* Far jump: 64-bit. */ tcg_target_long imm = sextreg((tcg_target_long)arg, 0, 12); tcg_target_long base = (tcg_target_long)arg - imm; tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_TMP0, base); -- 2.43.7 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-04 7:57 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-03 17:04 [PATCH 0/2] tcg/riscv64: Fix AUIPC pair range validation Max Chou 2026-09-03 17:04 ` [PATCH 1/2] tcg/riscv64: Validate AUIPC relocation range Max Chou 2026-09-03 17:31 ` Philippe Mathieu-Daudé 2026-09-04 7:56 ` Max Chou 2026-09-03 17:04 ` [PATCH 2/2] tcg/riscv64: Fall back when AUIPC pairs are out of range Max Chou
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.