Re: [PATCH] tcg/riscv: Fix potential bug in clobbered call register set

[Philippe Mathieu-Daudé]

On 9/27/21 01:06, Richard Henderson wrote:
> On 9/26/21 5:39 PM, Philippe Mathieu-Daudé wrote:
>> The tcg_target_call_clobber_regs variable is of type TCGRegSet,
>> which is unsigned and might be 64-bit wide. By initializing it
>> as unsigned type, only 32-bit are set. Currently the RISCV TCG
>> backend only uses 32 registers, so this is not a problem.
>> However if more register were to be implemented (such vectors)
>> then it would become problematic. Since we are better safe than
>> sorry, properly initialize the value as 64-bit.
>>
>> Fixes: 7a5549f2aea ("tcg/riscv: Add the target init code")
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   tcg/riscv/tcg-target.c.inc | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
>> index dc8d8f1de23..5bd95633b0d 100644
>> --- a/tcg/riscv/tcg-target.c.inc
>> +++ b/tcg/riscv/tcg-target.c.inc
>> @@ -1734,7 +1734,7 @@ static void tcg_target_init(TCGContext *s)
>>           tcg_target_available_regs[TCG_TYPE_I64] = 0xffffffff;
>>       }
>>   -    tcg_target_call_clobber_regs = -1u;
>> +    tcg_target_call_clobber_regs = -1ull;
> 
> There are not 64 registers, so this is incorrect.

Currently there are 32 registers, but I was looking at this draft:
https://five-embeddev.com/riscv-v-spec/draft/v-spec.html#_vector_registers
"The vector extension adds 32 architectural vector registers, v0-v31
to the base scalar RISC-V ISA."
If this were to be implemented (and available on the host), wouldn't
we have 64 registers?

> I don't think your logic is correct.

Eventually this line would be easier to review as:

  tcg_target_call_clobber_regs = MAKE_64BIT_MASK(0, TCG_TARGET_NB_REGS);