[clang] cb1eeb4 - [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Thu May 14 05:29:42 PDT 2020


Author: Balázs Kéri
Date: 2020-05-14T14:30:05+02:00
New Revision: cb1eeb42c03c31d4dadd00dbaec693e6d7516099

URL: https://github.com/llvm/llvm-project/commit/cb1eeb42c03c31d4dadd00dbaec693e6d7516099
DIFF: https://github.com/llvm/llvm-project/commit/cb1eeb42c03c31d4dadd00dbaec693e6d7516099.diff

LOG: [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

Summary:
The check of VLA size was done previously for variable declarations
(of VLA type) only. Now it is done for typedef (and type-alias)
and sizeof expressions with VLA too.

Reviewers: Szelethus, martong

Reviewed By: Szelethus, martong

Subscribers: rnkovacs, xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, gamesh411, Charusso, martong, ASDenysPetrov, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D79072

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
    clang/test/Analysis/vla.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
index 0c7961a2a28b..3bd2520f013a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -30,10 +30,26 @@ using namespace ento;
 using namespace taint;
 
 namespace {
-class VLASizeChecker : public Checker< check::PreStmt<DeclStmt> > {
+class VLASizeChecker
+    : public Checker<check::PreStmt<DeclStmt>,
+                     check::PreStmt<UnaryExprOrTypeTraitExpr>> {
   mutable std::unique_ptr<BugType> BT;
   enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted, VLA_Negative };
 
+  /// Check a VLA for validity.
+  /// Every dimension of the array is checked for validity, and
+  /// dimension sizes are collected into 'VLASizes'. 'VLALast' is set to the
+  /// innermost VLA that was encountered.
+  /// In "int vla[x][2][y][3]" this will be the array for index "y" (with type
+  /// int[3]). 'VLASizes' contains 'x', '2', and 'y'. Returns null or a new
+  /// state where the size is validated for every dimension.
+  ProgramStateRef checkVLA(CheckerContext &C, ProgramStateRef State,
+                           const VariableArrayType *VLA,
+                           const VariableArrayType *&VLALast,
+                           llvm::SmallVector<const Expr *, 2> &VLASizes) const;
+
+  /// Check one VLA dimension for validity.
+  /// Returns null or a new state where the size is validated.
   ProgramStateRef checkVLASize(CheckerContext &C, ProgramStateRef State,
                                const Expr *SizeE) const;
 
@@ -43,9 +59,37 @@ class VLASizeChecker : public Checker< check::PreStmt<DeclStmt> > {
 
 public:
   void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
+  void checkPreStmt(const UnaryExprOrTypeTraitExpr *UETTE,
+                    CheckerContext &C) const;
 };
 } // end anonymous namespace
 
+ProgramStateRef
+VLASizeChecker::checkVLA(CheckerContext &C, ProgramStateRef State,
+                         const VariableArrayType *VLA,
+                         const VariableArrayType *&VLALast,
+                         llvm::SmallVector<const Expr *, 2> &VLASizes) const {
+  assert(VLA && "Function should be called with non-null VLA argument.");
+
+  VLALast = nullptr;
+  // Walk over the VLAs for every dimension until a non-VLA is found.
+  // There is a VariableArrayType for every dimension (fixed or variable) until
+  // the most inner array that is variably modified.
+  while (VLA) {
+    const Expr *SizeE = VLA->getSizeExpr();
+    State = checkVLASize(C, State, SizeE);
+    if (!State)
+      return nullptr;
+    VLASizes.push_back(SizeE);
+    VLALast = VLA;
+    VLA = C.getASTContext().getAsVariableArrayType(VLA->getElementType());
+  };
+  assert(VLALast &&
+         "Array should have at least one variably-modified dimension.");
+
+  return State;
+}
+
 ProgramStateRef VLASizeChecker::checkVLASize(CheckerContext &C,
                                              ProgramStateRef State,
                                              const Expr *SizeE) const {
@@ -146,36 +190,34 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   if (!DS->isSingleDecl())
     return;
 
-  const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl());
-  if (!VD)
-    return;
-
   ASTContext &Ctx = C.getASTContext();
   SValBuilder &SVB = C.getSValBuilder();
   ProgramStateRef State = C.getState();
+  QualType TypeToCheck;
+
+  const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl());
+
+  if (VD)
+    TypeToCheck = VD->getType().getCanonicalType();
+  else if (const auto *TND = dyn_cast<TypedefNameDecl>(DS->getSingleDecl()))
+    TypeToCheck = TND->getUnderlyingType().getCanonicalType();
+  else
+    return;
 
-  const VariableArrayType *VLA = Ctx.getAsVariableArrayType(VD->getType());
+  const VariableArrayType *VLA = Ctx.getAsVariableArrayType(TypeToCheck);
   if (!VLA)
     return;
 
+  // Check the VLA sizes for validity.
   llvm::SmallVector<const Expr *, 2> VLASizes;
   const VariableArrayType *VLALast = nullptr;
-  // Walk over the VLAs for every dimension until a non-VLA is found.
-  // Collect the sizes in VLASizes, put the most inner VLA to `VLALast`.
-  // In "vla[x][2][y][3]" this will be the array for index "y".
-  // There is a VariableArrayType for every dimension (here "x", "2", "y")
-  // until a non-vla is found.
-  while (VLA) {
-    const Expr *SizeE = VLA->getSizeExpr();
-    State = checkVLASize(C, State, SizeE);
-    if (!State)
-      return;
-    VLASizes.push_back(SizeE);
-    VLALast = VLA;
-    VLA = Ctx.getAsVariableArrayType(VLA->getElementType());
-  };
-  assert(VLALast &&
-         "Array should have at least one variably-modified dimension.");
+
+  State = checkVLA(C, State, VLA, VLALast, VLASizes);
+  if (!State)
+    return;
+
+  if (!VD)
+    return;
 
   // VLASizeChecker is responsible for defining the extent of the array being
   // declared. We do this by multiplying the array length by the element size,
@@ -219,6 +261,33 @@ void VLASizeChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
   C.addTransition(State);
 }
 
+void VLASizeChecker::checkPreStmt(const UnaryExprOrTypeTraitExpr *UETTE,
+                                  CheckerContext &C) const {
+  // Want to check for sizeof.
+  if (UETTE->getKind() != UETT_SizeOf)
+    return;
+
+  // Ensure a type argument.
+  if (!UETTE->isArgumentType())
+    return;
+
+  const VariableArrayType *VLA =
+      C.getASTContext().getAsVariableArrayType(UETTE->getTypeOfArgument());
+  // Ensure that the type is a VLA.
+  if (!VLA)
+    return;
+
+  ProgramStateRef State = C.getState();
+
+  llvm::SmallVector<const Expr *, 2> VLASizes;
+  const VariableArrayType *VLALast = nullptr;
+  State = checkVLA(C, State, VLA, VLALast, VLASizes);
+  if (!State)
+    return;
+
+  C.addTransition(State);
+}
+
 void ento::registerVLASizeChecker(CheckerManager &mgr) {
   mgr.registerChecker<VLASizeChecker>();
 }

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index b17f26aa9c53..2fea21bc1a70 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -573,6 +573,18 @@ void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL,
 
 void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
                                ExplodedNodeSet &Dst) {
+  if (isa<TypedefNameDecl>(*DS->decl_begin())) {
+    // C99 6.7.7 "Any array size expressions associated with variable length
+    // array declarators are evaluated each time the declaration of the typedef
+    // name is reached in the order of execution."
+    // The checkers should know about typedef to be able to handle VLA size
+    // expressions.
+    ExplodedNodeSet DstPre;
+    getCheckerManager().runCheckersForPreStmt(DstPre, Pred, DS, *this);
+    getCheckerManager().runCheckersForPostStmt(Dst, DstPre, DS, *this);
+    return;
+  }
+
   // Assumption: The CFG has one DeclStmt per Decl.
   const VarDecl *VD = dyn_cast_or_null<VarDecl>(*DS->decl_begin());
 

diff  --git a/clang/test/Analysis/vla.c b/clang/test/Analysis/vla.c
index f163e4952dc9..062bb0828e23 100644
--- a/clang/test/Analysis/vla.c
+++ b/clang/test/Analysis/vla.c
@@ -89,6 +89,17 @@ void check_negative_sized_VLA_11(int x) {
     check_negative_sized_VLA_11_sub(x);
 }
 
+void check_VLA_typedef() {
+  int x = -1;
+  typedef int VLA[x]; // expected-warning{{Declared variable-length array (VLA) has negative size}}
+}
+
+size_t check_VLA_sizeof() {
+  int x = -1;
+  size_t s = sizeof(int[x]); // expected-warning{{Declared variable-length array (VLA) has negative size}}
+  return s;
+}
+
 // Multi-dimensional arrays.
 
 void check_zero_sized_VLA_multi1(int x) {


        


More information about the cfe-commits mailing list