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});
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.