Fix static analyzer bugs with null pointer dereferences in CheckSizelessVectorOperands()

[Manna]

Summary

This patch adds nullptr checks for LHSBuiltinTy and RHSBuiltinTy before using them to getBuiltinVectorTypeInfo() in clang::​Sema::​CheckSizelessVectorOperands(clang::​ActionResult<clang::​Expr *, true> &, clang::​ActionResult<clang::​Expr *, true> &, clang::​SourceLocation, bool, clang::​Sema::​ArithConvKind).

[sdesmalen]

clang/lib/Sema/SemaExpr.cpp

11119

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?

[erichkeane]

The SA tool is incorrect here.

clang/lib/Sema/SemaExpr.cpp

11119

In fact they DO! Pre-commit CI found them.

11149

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

[Manna]

Thank you @erichkeane and @sdesmalen for reviews and feedbacks.

[Manna]

clang/lib/Sema/SemaExpr.cpp

11149

Thanks for the comments. I have added nullptr checks for LHSBuiltinTy and RHSBuiltinTy before using them to getBuiltinVectorTypeInfo() which dereferences.

[erichkeane]

clang/lib/Sema/SemaExpr.cpp

11148

I think this is unnecessary. isVLSTBuiltinType only returns true if LHSType is a BuiltinType already (or at least, a subset-of). See : https://clang.llvm.org/doxygen/Type_8cpp_source.html#l02409

[Manna]

clang/lib/Sema/SemaExpr.cpp

11148

This makes sense to me. Thank you for the pointer and explanation!

clang/lib/Sema/SemaExpr.cpp

QualType Sema::CheckSizelessVectorOperands(ExprResult &LHS, ExprResult &RHS,
   QualType LHSType = LHS.get()->getType().getUnqualifiedType();
   QualType RHSType = RHS.get()->getType().getUnqualifiedType();
 
   const BuiltinType *LHSBuiltinTy = LHSType->getAs<BuiltinType>();
   const BuiltinType *RHSBuiltinTy = RHSType->getAs<BuiltinType>();
 
   unsigned DiagID = diag::err_typecheck_invalid_operands;
   if ((OperationKind == ACK_Arithmetic) &&
       ((LHSBuiltinTy && LHSBuiltinTy->isSVEBool()) ||
        (RHSBuiltinTy && RHSBuiltinTy->isSVEBool()))) {
     Diag(Loc, DiagID) << LHSType << RHSType << LHS.get()->getSourceRange()
                       << RHS.get()->getSourceRange();
     return QualType();
   }
 
   if (Context.hasSameType(LHSType, RHSType))
     return LHSType;
if ((!LHSType->isVLSTBuiltinType() && !LHSType->isRealType()) ||
       (!RHSType->isVLSTBuiltinType() && !RHSType->isRealType())) {
     Diag(Loc, diag::err_typecheck_vector_not_convertable_non_scalar)
         << LHSType << RHSType << LHS.get()->getSourceRange()
         << RHS.get()->getSourceRange();
     return QualType();
   }
 
   if (LHSType->isVLSTBuiltinType() && RHSType->isVLSTBuiltinType() &&
+      LHSBuiltinTy && RHSBuiltinTy &&
       Context.getBuiltinVectorTypeInfo(LHSBuiltinTy).EC !=
           Context.getBuiltinVectorTypeInfo(RHSBuiltinTy).EC) {
     Diag(Loc, diag::err_typecheck_vector_lengths_not_equal)
         << LHSType << RHSType << LHS.get()->getSourceRange()
         << RHS.get()->getSourceRange();
     return QualType();
   }
 
   if (LHSType->isVLSTBuiltinType() || RHSType->isVLSTBuiltinType()) {