target/loongarch: Avoid shifting with bool type variable

[PATCH] target/loongarch: Avoid shifting with bool type variable

Posted by Bibo Mao 2 months, 1 week ago

Variable env->cf[i] is defined as bool type, it cannot be used with left
shifting operation. There is existing api read_fcc(), it can be used when
dumping fp registers into coredump note segment.

Resolves: Coverity CID 1561133
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 target/loongarch/arch_dump.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/loongarch/arch_dump.c b/target/loongarch/arch_dump.c
index 4986db970e..d9e1120333 100644
--- a/target/loongarch/arch_dump.c
+++ b/target/loongarch/arch_dump.c
@@ -97,11 +97,7 @@ static int loongarch_write_elf64_fprpreg(WriteCoreDumpFunction f,
 
     loongarch_note_init(&note, s, "CORE", 5, NT_PRFPREG, sizeof(note.fpu));
     note.fpu.fcsr = cpu_to_dump64(s, env->fcsr0);
-
-    for (i = 0; i < 8; i++) {
-        note.fpu.fcc |= env->cf[i] << (8 * i);
-    }
-    note.fpu.fcc = cpu_to_dump64(s, note.fpu.fcc);
+    note.fpu.fcc = cpu_to_dump64(s, read_fcc(env));
 
     for (i = 0; i < 32; ++i) {
         note.fpu.fpr[i] = cpu_to_dump64(s, env->fpr[i].vreg.UD[0]);

base-commit: 28ae3179fc52d2e4d870b635c4a412aab99759e7
-- 
2.39.3

Re: [PATCH] target/loongarch: Avoid shifting with bool type variable

Posted by Richard Henderson 2 months, 1 week ago

On 9/13/24 18:43, Bibo Mao wrote:
> Variable env->cf[i] is defined as bool type, it cannot be used with left
> shifting operation. 

Not true; it promotes to 'int'.  But that isn't wide enough for the value that you're 
trying to construct.

There is existing api read_fcc(), it can be used when
> dumping fp registers into coredump note segment.
> 
> Resolves: Coverity CID 1561133
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> ---
>   target/loongarch/arch_dump.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/target/loongarch/arch_dump.c b/target/loongarch/arch_dump.c
> index 4986db970e..d9e1120333 100644
> --- a/target/loongarch/arch_dump.c
> +++ b/target/loongarch/arch_dump.c
> @@ -97,11 +97,7 @@ static int loongarch_write_elf64_fprpreg(WriteCoreDumpFunction f,
>   
>       loongarch_note_init(&note, s, "CORE", 5, NT_PRFPREG, sizeof(note.fpu));
>       note.fpu.fcsr = cpu_to_dump64(s, env->fcsr0);
> -
> -    for (i = 0; i < 8; i++) {
> -        note.fpu.fcc |= env->cf[i] << (8 * i);
> -    }
> -    note.fpu.fcc = cpu_to_dump64(s, note.fpu.fcc);
> +    note.fpu.fcc = cpu_to_dump64(s, read_fcc(env));
>   
>       for (i = 0; i < 32; ++i) {
>           note.fpu.fpr[i] = cpu_to_dump64(s, env->fpr[i].vreg.UD[0]);
> 
> base-commit: 28ae3179fc52d2e4d870b635c4a412aab99759e7

Re: [PATCH] target/loongarch: Avoid shifting with bool type variable

Posted by maobibo 2 months, 1 week ago


On 2024/9/14 上午10:58, Richard Henderson wrote:
> On 9/13/24 18:43, Bibo Mao wrote:
>> Variable env->cf[i] is defined as bool type, it cannot be used with left
>> shifting operation. 
> 
> Not true; it promotes to 'int'.  But that isn't wide enough for the 
> value that you're trying to construct.
yeap, you are right. Variable with 'bool' type can be used with left 
shifting and treated as 'int' type.

I will refresh patch in the next version.

Regards
Bibo Mao
> 
> There is existing api read_fcc(), it can be used when
>> dumping fp registers into coredump note segment.
>>
>> Resolves: Coverity CID 1561133
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> r~
> 
>> ---
>>   target/loongarch/arch_dump.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/target/loongarch/arch_dump.c b/target/loongarch/arch_dump.c
>> index 4986db970e..d9e1120333 100644
>> --- a/target/loongarch/arch_dump.c
>> +++ b/target/loongarch/arch_dump.c
>> @@ -97,11 +97,7 @@ static int 
>> loongarch_write_elf64_fprpreg(WriteCoreDumpFunction f,
>>       loongarch_note_init(&note, s, "CORE", 5, NT_PRFPREG, 
>> sizeof(note.fpu));
>>       note.fpu.fcsr = cpu_to_dump64(s, env->fcsr0);
>> -
>> -    for (i = 0; i < 8; i++) {
>> -        note.fpu.fcc |= env->cf[i] << (8 * i);
>> -    }
>> -    note.fpu.fcc = cpu_to_dump64(s, note.fpu.fcc);
>> +    note.fpu.fcc = cpu_to_dump64(s, read_fcc(env));
>>       for (i = 0; i < 32; ++i) {
>>           note.fpu.fpr[i] = cpu_to_dump64(s, env->fpr[i].vreg.UD[0]);
>>
>> base-commit: 28ae3179fc52d2e4d870b635c4a412aab99759e7
>