[clang][analyzer] Add StoreToImmutable checker

clang/docs/analyzer/checkers/storetoimmutable_example.cpp

@@ -8,6 +8,7 @@ void test_global_const() {
 }
 
 // String literal
+// NOTE: This only works in C++, not in C, as the analyzer treats string literals as non-const char arrays in C mode.
 void test_string_literal() {
   char *str = (char *)"hello";
   str[0] = 'H'; // warn: Writing to immutable memory

clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp

@@ -29,29 +29,24 @@ class StoreToImmutableChecker : public Checker<check::Bind> {
   void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const;
 
 private:
-  bool isConstVariable(const MemRegion *MR, CheckerContext &C) const;
+  bool isEffectivelyConstRegion(const MemRegion *MR, CheckerContext &C) const;
   bool isConstQualifiedType(const MemRegion *MR, CheckerContext &C) const;
 };
 } // end anonymous namespace
 
-bool StoreToImmutableChecker::isConstVariable(const MemRegion *MR,
-                                              CheckerContext &C) const {
+static bool isEffectivelyConstRegionAux(const MemRegion *MR,
+                                        CheckerContext &C) {
   // Check if the region is in the global immutable space
   const MemSpaceRegion *MS = MR->getMemorySpace(C.getState());
   if (isa<GlobalImmutableSpaceRegion>(MS))
     return true;
 
-  // Check if this is a VarRegion with a const-qualified type
-  if (const VarRegion *VR = dyn_cast<VarRegion>(MR)) {
-    const VarDecl *VD = VR->getDecl();
-    if (VD && VD->getType().isConstQualified())
-      return true;
-  }
-
-  // Check if this is a FieldRegion with a const-qualified type
-  if (const FieldRegion *FR = dyn_cast<FieldRegion>(MR)) {
-    const FieldDecl *FD = FR->getDecl();
-    if (FD && FD->getType().isConstQualified())
+  // Check if this is a TypedRegion with a const-qualified type
+  if (const TypedRegion *TR = dyn_cast<TypedRegion>(MR)) {
+    QualType LocationType = TR->getDesugaredLocationType(C.getASTContext());
+    if (LocationType->isPointerOrReferenceType())
+      LocationType = LocationType->getPointeeType();
+    if (LocationType.isConstQualified())
       return true;
   }
 
@@ -62,22 +57,33 @@ bool StoreToImmutableChecker::isConstVariable(const MemRegion *MR,
       return true;
   }
 
-  // Check if this is an ElementRegion accessing a const array
-  if (const ElementRegion *ER = dyn_cast<ElementRegion>(MR)) {
-    return isConstQualifiedType(ER->getSuperRegion(), C);
-  }
+  // NOTE: The only kind of region that is not checked by the above branches is
+  // AllocaRegion. We do not need to check AllocaRegion, as it models untyped
+  // memory, that is allocated on the stack.
 
   return false;
 }
 
-bool StoreToImmutableChecker::isConstQualifiedType(const MemRegion *MR,
-                                                   CheckerContext &C) const {
-  // Check if the region has a const-qualified type
-  if (const TypedValueRegion *TVR = dyn_cast<TypedValueRegion>(MR)) {
-    QualType Ty = TVR->getValueType();
-    return Ty.isConstQualified();
+bool StoreToImmutableChecker::isEffectivelyConstRegion(
+    const MemRegion *MR, CheckerContext &C) const {
+  // If the region is an ElementRegion, we need to check if any of the super
+  // regions have const-qualified type.
+  if (const ElementRegion *ER = dyn_cast<ElementRegion>(MR)) {
+    SmallVector<const MemRegion *, 8> SuperRegions;
+    const MemRegion *Current = MR;
+    const MemRegion *Base = ER->getBaseRegion();
+    while (Current != Base) {
+      SuperRegions.push_back(Current);
+      assert(isa<SubRegion>(Current));
+      Current = cast<SubRegion>(Current)->getSuperRegion();
+    }
+    SuperRegions.push_back(Base);
+    return llvm::any_of(SuperRegions, [&C](const MemRegion *MR) {
+      return isEffectivelyConstRegionAux(MR, C);
+    });
   }
-  return false;
+
+  return isEffectivelyConstRegionAux(MR, C);
 }
 
 void StoreToImmutableChecker::checkBind(SVal Loc, SVal Val, const Stmt *S,
@@ -93,7 +99,7 @@ void StoreToImmutableChecker::checkBind(SVal Loc, SVal Val, const Stmt *S,
     return;
 
   // Check if the region corresponds to a const variable
-  if (!isConstVariable(MR, C))
+  if (!isEffectivelyConstRegion(MR, C))
     return;
 
   // Generate the bug report

clang/test/Analysis/store-to-immutable-basic.cpp

@@ -1,5 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -verify %s
-
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++17 -verify %s
 
 void test_write_to_const_ref_param(const int &param) {
   *(int*)&param = 100; // expected-warning {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}}

clang/test/Analysis/store-to-immutable-lambda-init.cpp

@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++14 -verify %s
+
+// expected-no-diagnostics
+
+// In C++14 and before, when initializing a lambda, the statement given in the checkBind callback is not the whole DeclExpr, but the CXXConstructExpr of the lambda object.
+// FIXME: Once the API of checkBind provides more information about the statement, the checker should be simplified, and this this test case will no longer be a cornercase in the checker.
+
+void test_const_lambda_initialization_pre_cpp17() {
+  const auto lambda = [](){}; // No warning expected
+}