Re: [PATCH 04/11] target/hexagon: guard writes to unimplemented guest registers
Brian Cain <[email protected]>
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
On 8/20/2026 1:44 PM, Pierrick Bouvier wrote: > On 8/18/2026 6:31 PM, Brian Cain wrote: >> Gate guest-register writes on greg_writable() in the generated code so >> writes to gregs above G3 are dropped instead of dereferencing an >> unallocated TCG temp. >> >> Signed-off-by: Brian Cain <[email protected]> >> --- >> target/hexagon/translate.h | 7 ++++--- >> target/hexagon/hex_common.py | 18 +++++++++++++++++- >> 2 files changed, 21 insertions(+), 4 deletions(-) >> >> diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h >> index 00de2b0d2ec..05425d92b29 100644 >> --- a/target/hexagon/translate.h >> +++ b/target/hexagon/translate.h >> @@ -97,9 +97,10 @@ bool is_gather_store_insn(DisasContext *ctx); >> #ifndef CONFIG_USER_ONLY >> static inline void ctx_log_greg_write(DisasContext *ctx, int rnum) >> { >> - assert(rnum <= HEX_GREG_G3); >> - ctx->greg_log[ctx->greg_log_idx] = rnum; >> - ctx->greg_log_idx++; >> + if (rnum <= HEX_GREG_G3) { >> + ctx->greg_log[ctx->greg_log_idx] = rnum; >> + ctx->greg_log_idx++; >> + } >> } >> > I'm not sure how this change is related to what is given in written > message. The functional change here is that we'll ignore write, instead > of asserting. Is that still expected to have a write on a wrong register > anyway? > > If not, maybe we should at least log a guest_error instead of silently > ignoring it. Indeed there is an unimp logged, by greg_writable(). The fact that we have an analysis phase, reviewing the instructions in a packet and a subsequent generation phase obscures things a bit and the commit message can be improved. I'll work on making things clearer/more explicit here in v2. >> static inline void ctx_log_greg_write_pair(DisasContext *ctx, int rnum) >> diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py >> index e33d43e3ce0..c180c19b092 100755 >> --- a/target/hexagon/hex_common.py >> +++ b/target/hexagon/hex_common.py >> @@ -1097,11 +1097,24 @@ def analyze_write(self, f, tag, regno): >> """)) >> >> class GuestRegister(Register): >> - pass >> + def gen_check_impl(self, f, regno): >> + if self.is_written(): >> + f.write(code_fmt(f"""\ >> + if (!greg_writable(insn->regno[{regno}], >> + {str(self.is_pair()).lower()})) {{ >> + return; >> + }} >> + """)) >> + else: >> + f.write(code_fmt(f"""\ >> + check_greg_impl(insn->regno[{regno}], >> + {str(self.is_pair()).lower()}); >> + """)) >> >> class GuestDest(GuestRegister, Single, Dest): >> def decl_tcg(self, f, tag, regno): >> self.decl_reg_num(f, regno) >> + self.gen_check_impl(f, regno) >> f.write(code_fmt(f"""\ >> TCGv_i32 {self.reg_tcg()} = tcg_temp_new_i32(); >> """)) >> @@ -1121,6 +1134,7 @@ def decl_reg_num(self, f, regno): >> """)) >> def decl_tcg(self, f, tag, regno): >> self.decl_reg_num(f, regno) >> + self.gen_check_impl(f, regno) >> f.write(code_fmt(f"""\ >> TCGv_i32 {self.reg_tcg()} = tcg_temp_new_i32(); >> gen_read_greg({self.reg_tcg()}, {self.reg_num}); >> @@ -1131,6 +1145,7 @@ def analyze_read(self, f, regno): >> class GuestPairDest(GuestRegister, Pair, Dest): >> def decl_tcg(self, f, tag, regno): >> self.decl_reg_num(f, regno) >> + self.gen_check_impl(f, regno) >> f.write(code_fmt(f"""\ >> TCGv_i64 {self.reg_tcg()} = tcg_temp_new_i64(); >> """)) >> @@ -1150,6 +1165,7 @@ def decl_reg_num(self, f, regno): >> """)) >> def decl_tcg(self, f, tag, regno): >> self.decl_reg_num(f, regno) >> + self.gen_check_impl(f, regno) >> f.write(code_fmt(f"""\ >> TCGv_i64 {self.reg_tcg()} = tcg_temp_new_i64(); >> gen_read_greg_pair({self.reg_tcg()}, {self.reg_num});