[v1] Hexagon (target/hexagon) Fix shift amount check in fASHIFTL/fLSHIFTR

[PATCH] Hexagon (target/hexagon) Fix shift amount check in fASHIFTL/fLSHIFTR

Posted by Taylor Simpson 3 years, 1 month ago

Address Coverity warnings

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/macros.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 78c4efb..cfcb817 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -459,7 +459,7 @@ static inline void gen_logical_not(TCGv dest, TCGv src)
                    : (fCAST##REGSTYPE##s(SRC) >> (SHAMT)))
 #define fASHIFTR(SRC, SHAMT, REGSTYPE) (fCAST##REGSTYPE##s(SRC) >> (SHAMT))
 #define fLSHIFTR(SRC, SHAMT, REGSTYPE) \
-    (((SHAMT) >= 64) ? 0 : (fCAST##REGSTYPE##u(SRC) >> (SHAMT)))
+    (((SHAMT) >= (sizeof(SRC) * 8)) ? 0 : (fCAST##REGSTYPE##u(SRC) >> (SHAMT)))
 #define fROTL(SRC, SHAMT, REGSTYPE) \
     (((SHAMT) == 0) ? (SRC) : ((fCAST##REGSTYPE##u(SRC) << (SHAMT)) | \
                               ((fCAST##REGSTYPE##u(SRC) >> \
@@ -469,7 +469,7 @@ static inline void gen_logical_not(TCGv dest, TCGv src)
                               ((fCAST##REGSTYPE##u(SRC) << \
                                  ((sizeof(SRC) * 8) - (SHAMT))))))
 #define fASHIFTL(SRC, SHAMT, REGSTYPE) \
-    (((SHAMT) >= 64) ? 0 : (fCAST##REGSTYPE##s(SRC) << (SHAMT)))
+    (((SHAMT) >= (sizeof(SRC) * 8)) ? 0 : (fCAST##REGSTYPE##s(SRC) << (SHAMT)))
 
 #ifdef QEMU_GENERATE
 #define fLOAD(NUM, SIZE, SIGN, EA, DST) MEM_LOAD##SIZE##SIGN(DST, EA)
-- 
2.7.4

Re: [PATCH] Hexagon (target/hexagon) Fix shift amount check in fASHIFTL/fLSHIFTR

Posted by Philippe Mathieu-Daudé 3 years, 1 month ago

Hi Taylor,

On 3/4/21 5:56 PM, Taylor Simpson wrote:
> Address Coverity warnings
> 

We usually include here the Coverity IDs, eventually the
warning reported, and refer to the culprit commit. See
for example commit 2132cfe52bd. This is also documented
at the end of this paragraph:

https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message

So here:
Fixes: a646e99cb90 ("Hexagon (target/hexagon) macros")

> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
>  target/hexagon/macros.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
> index 78c4efb..cfcb817 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -459,7 +459,7 @@ static inline void gen_logical_not(TCGv dest, TCGv src)
>                     : (fCAST##REGSTYPE##s(SRC) >> (SHAMT)))
>  #define fASHIFTR(SRC, SHAMT, REGSTYPE) (fCAST##REGSTYPE##s(SRC) >> (SHAMT))
>  #define fLSHIFTR(SRC, SHAMT, REGSTYPE) \
> -    (((SHAMT) >= 64) ? 0 : (fCAST##REGSTYPE##u(SRC) >> (SHAMT)))
> +    (((SHAMT) >= (sizeof(SRC) * 8)) ? 0 : (fCAST##REGSTYPE##u(SRC) >> (SHAMT)))
>  #define fROTL(SRC, SHAMT, REGSTYPE) \
>      (((SHAMT) == 0) ? (SRC) : ((fCAST##REGSTYPE##u(SRC) << (SHAMT)) | \
>                                ((fCAST##REGSTYPE##u(SRC) >> \
> @@ -469,7 +469,7 @@ static inline void gen_logical_not(TCGv dest, TCGv src)
>                                ((fCAST##REGSTYPE##u(SRC) << \
>                                   ((sizeof(SRC) * 8) - (SHAMT))))))
>  #define fASHIFTL(SRC, SHAMT, REGSTYPE) \
> -    (((SHAMT) >= 64) ? 0 : (fCAST##REGSTYPE##s(SRC) << (SHAMT)))
> +    (((SHAMT) >= (sizeof(SRC) * 8)) ? 0 : (fCAST##REGSTYPE##s(SRC) << (SHAMT)))
>  
>  #ifdef QEMU_GENERATE
>  #define fLOAD(NUM, SIZE, SIGN, EA, DST) MEM_LOAD##SIZE##SIGN(DST, EA)
>