[PATCH] D152197: [NFC][CLANG] Fix static analyzer bugs with null pointer dereferences in CheckSizelessVectorOperands()

2023-06-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane requested changes to this revision.
erichkeane added a comment.
This revision now requires changes to proceed.

The SA tool is incorrect here.




Comment at: clang/lib/Sema/SemaExpr.cpp:9
   if ((OperationKind == ACK_Arithmetic) &&
   ((LHSBuiltinTy && LHSBuiltinTy->isSVEBool()) ||
(RHSBuiltinTy && RHSBuiltinTy->isSVEBool( {

sdesmalen wrote:
> This doesn't seem like a non-functional change. Here it checks that 
> LHSBuiltinTy is not nullptr before using it, which suggests that it is valid 
> for LHSType/RHSType not to be a builtin type. There is also nothing else 
> guarding that the types are all VLSTBuiltinTypes. Do none of the tests fail?
In fact they DO!  Pre-commit CI found them.  



Comment at: clang/lib/Sema/SemaExpr.cpp:11148
   if (LHSType->isVLSTBuiltinType() && RHSType->isVLSTBuiltinType() &&
   Context.getBuiltinVectorTypeInfo(LHSBuiltinTy).EC !=
   Context.getBuiltinVectorTypeInfo(RHSBuiltinTy).EC) {

This is the only other use of htem, but the 1st part of the 'if' checks if 
these are builtin types.  


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152197/new/

https://reviews.llvm.org/D152197

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D152197: [NFC][CLANG] Fix static analyzer bugs with null pointer dereferences in CheckSizelessVectorOperands()

2023-06-06 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:9
   if ((OperationKind == ACK_Arithmetic) &&
   ((LHSBuiltinTy && LHSBuiltinTy->isSVEBool()) ||
(RHSBuiltinTy && RHSBuiltinTy->isSVEBool( {

This doesn't seem like a non-functional change. Here it checks that 
LHSBuiltinTy is not nullptr before using it, which suggests that it is valid 
for LHSType/RHSType not to be a builtin type. There is also nothing else 
guarding that the types are all VLSTBuiltinTypes. Do none of the tests fail?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152197/new/

https://reviews.llvm.org/D152197

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits