[clang][analyzer] Add StoreToImmutable checker (#150417)

This adds alpha.core.StoreToImmutable, a new alpha checker that detects
writes
to immutable memory regions, implementing part of SEI CERT Rule ENV30-C.
The
original proposal only handled global const variables, but this
implementation
extends it to also detect writes to:
- Local const variables
- String literals
- Const parameters and struct members
- Const arrays and pointers to const data

This checker is the continuation of the work started by zukatsinadze.
Discussion: https://reviews.llvm.org/D124244

clang/docs/analyzer/checkers.rst

@@ -3086,6 +3086,23 @@ Either the comparison is useless or there is division by zero.
    if (x == 0) { } // warn
  }
 
+.. _alpha-core-StoreToImmutable:
+
+alpha.core.StoreToImmutable (C, C++)
+""""""""""""""""""""""""""""""""""""
+Check for writes to immutable memory regions. This implements part of SEI CERT Rule ENV30-C.
+
+This checker detects attempts to write to memory regions that are marked as immutable,
+including const variables, string literals, and other const-qualified memory.
+
+.. literalinclude:: checkers/storetoimmutable_example.cpp
+    :language: cpp
+
+**Solution**
+
+Avoid writing to const-qualified memory regions. If you need to modify the data,
+remove the const qualifier from the original declaration or use a mutable copy.
+
 alpha.cplusplus
 ^^^^^^^^^^^^^^^
 

clang/docs/analyzer/checkers/storetoimmutable_example.cpp

@@ -0,0 +1,21 @@
+const int global_const = 42;
+
+struct TestStruct {
+  const int x;
+  int y;
+};
+
+void immutable_violation_examples() {
+  *(int *)&global_const = 100; // warn: Trying to write to immutable memory
+
+  const int local_const = 42;
+  *(int *)&local_const = 43; // warn: Trying to write to immutable memory
+
+  // NOTE: The following is reported in C++, but not in C, as the analyzer
+  // treats string literals as non-const char arrays in C mode.
+  char *ptr_to_str_literal = (char *)"hello";
+  ptr_to_str_literal[0] = 'H'; // warn: Trying to write to immutable memory
+
+  TestStruct s = {1, 2};
+  *(int *)&s.x = 10; // warn: Trying to write to immutable memory
+}

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

@@ -297,6 +297,11 @@ def StackAddrAsyncEscapeChecker
           "Check that addresses to stack memory do not escape the function">,
       Documentation<HasDocumentation>;
 
+def StoreToImmutableChecker : Checker<"StoreToImmutable">,
+  HelpText<"Check for writes to immutable memory regions. "
+           "This implements part of SEI CERT Rule ENV30-C.">,
+  Documentation<HasDocumentation>;
+
 def PthreadLockBase : Checker<"PthreadLockBase">,
   HelpText<"Helper registering multiple checks.">,
   Documentation<NotDocumented>,

clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

@@ -104,6 +104,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   SmartPtrChecker.cpp
   SmartPtrModeling.cpp
   StackAddrEscapeChecker.cpp
+  StoreToImmutableChecker.cpp
   StdLibraryFunctionsChecker.cpp
   StdVariantChecker.cpp
   STLAlgorithmModeling.cpp

clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp

@@ -0,0 +1,188 @@
+//=== StoreToImmutableChecker.cpp - Store to immutable memory ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines StoreToImmutableChecker, a checker that detects writes
+// to immutable memory regions. This implements part of SEI CERT Rule ENV30-C.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class StoreToImmutableChecker : public Checker<check::Bind> {
+  const BugType BT{this, "Write to immutable memory", "CERT Environment (ENV)"};
+
+public:
+  void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const;
+};
+} // end anonymous namespace
+
+static bool isInitializationContext(const Stmt *S, CheckerContext &C) {
+  // Check if this is a DeclStmt (variable declaration)
+  if (isa<DeclStmt>(S))
+    return true;
+
+  // This part is specific for initialization of const lambdas pre-C++17.
+  // Lets look at the AST of the statement:
+  // ```
+  // const auto lambda = [](){};
+  // ```
+  //
+  // The relevant part of the AST for this case prior to C++17 is:
+  // ...
+  // `-DeclStmt
+  //   `-VarDecl
+  //     `-ExprWithCleanups
+  //       `-CXXConstructExpr
+  // ...
+  // In C++17 and later, the AST is different:
+  // ...
+  // `-DeclStmt
+  //   `-VarDecl
+  //     `-ImplicitCastExpr
+  //       `-LambdaExpr
+  //         |-CXXRecordDecl
+  //         `-CXXConstructExpr
+  // ...
+  // And even beside this, the statement `S` that is given to the checkBind
+  // callback is the VarDecl in C++17 and later, and the CXXConstructExpr in
+  // C++14 and before. So in order to support the C++14 we need the following
+  // ugly hack to detect whether this construction is used to initialize a
+  // variable.
+  //
+  // FIXME: This should be eliminated by improving the API of checkBind to
+  // ensure that it consistently passes the `VarDecl` (instead of the
+  // `CXXConstructExpr`) when the constructor call denotes the initialization
+  // of a variable with a lambda, or maybe less preferably, try the more
+  // invasive approach of passing the information forward to the checkers
+  // whether the current bind is an initialization or an assignment.
+  const auto *ConstructExp = dyn_cast<CXXConstructExpr>(S);
+  return ConstructExp && ConstructExp->isElidable();
+}
+
+static bool isEffectivelyConstRegion(const MemRegion *MR, CheckerContext &C) {
+  if (isa<GlobalImmutableSpaceRegion>(MR))
+    return true;
+
+  // Check if this is a TypedRegion with a const-qualified type
+  if (const auto *TR = dyn_cast<TypedRegion>(MR)) {
+    QualType LocationType = TR->getDesugaredLocationType(C.getASTContext());
+    if (LocationType->isPointerOrReferenceType())
+      LocationType = LocationType->getPointeeType();
+    if (LocationType.isConstQualified())
+      return true;
+  }
+
+  // Check if this is a SymbolicRegion with a const-qualified pointee type
+  if (const auto *SR = dyn_cast<SymbolicRegion>(MR)) {
+    QualType PointeeType = SR->getPointeeStaticType();
+    if (PointeeType.isConstQualified())
+      return true;
+  }
+
+  // NOTE: The above branches do not cover AllocaRegion. We do not need to check
+  // AllocaRegion, as it models untyped memory, that is allocated on the stack.
+
+  return false;
+}
+
+static const MemRegion *getInnermostConstRegion(const MemRegion *MR,
+                                                CheckerContext &C) {
+  while (true) {
+    if (isEffectivelyConstRegion(MR, C))
+      return MR;
+    if (auto *SR = dyn_cast<SubRegion>(MR))
+      MR = SR->getSuperRegion();
+    else
+      return nullptr;
+  }
+}
+
+static const DeclRegion *
+getInnermostEnclosingConstDeclRegion(const MemRegion *MR, CheckerContext &C) {
+  while (true) {
+    if (const auto *DR = dyn_cast<DeclRegion>(MR)) {
+      const ValueDecl *D = DR->getDecl();
+      QualType DeclaredType = D->getType();
+      if (DeclaredType.isConstQualified())
+        return DR;
+    }
+    if (auto *SR = dyn_cast<SubRegion>(MR))
+      MR = SR->getSuperRegion();
+    else
+      return nullptr;
+  }
+}
+
+void StoreToImmutableChecker::checkBind(SVal Loc, SVal Val, const Stmt *S,
+                                        CheckerContext &C) const {
+  // We are only interested in stores to memory regions
+  const MemRegion *MR = Loc.getAsRegion();
+  if (!MR)
+    return;
+
+  // Skip variable declarations and initializations - we only want to catch
+  // actual writes
+  // FIXME: If the API of checkBind would allow to distinguish between
+  // initialization and assignment, we could use that instead.
+  if (isInitializationContext(S, C))
+    return;
+
+  // Check if the region is in the global immutable space
+  const MemSpaceRegion *MS = MR->getMemorySpace(C.getState());
+  const bool IsGlobalImmutableSpace = isa<GlobalImmutableSpaceRegion>(MS);
+  // Check if the region corresponds to a const variable
+  const MemRegion *InnermostConstRegion = getInnermostConstRegion(MR, C);
+  if (!IsGlobalImmutableSpace && !InnermostConstRegion)
+    return;
+
+  SmallString<64> WarningMessage{"Trying to write to immutable memory"};
+  if (IsGlobalImmutableSpace)
+    WarningMessage += " in global read-only storage";
+
+  // Generate the bug report
+  ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+    return;
+
+  auto R = std::make_unique<PathSensitiveBugReport>(BT, WarningMessage, N);
+  R->addRange(S->getSourceRange());
+
+  // Generate a note if the location that is being written to has a
+  // declaration or if it is a subregion of a const region with a declaration.
+  const DeclRegion *DR =
+      getInnermostEnclosingConstDeclRegion(InnermostConstRegion, C);
+  if (DR) {
+    const char *NoteMessage =
+        (DR != MR) ? "Enclosing memory region is declared as immutable here"
+                   : "Memory region is declared as immutable here";
+    R->addNote(NoteMessage, PathDiagnosticLocation::create(
+                                DR->getDecl(), C.getSourceManager()));
+  }
+
+  // For this checker, we are only interested in the value being written, no
+  // need to mark the value being assigned interesting.
+
+  C.emitReport(std::move(R));
+}
+
+void ento::registerStoreToImmutableChecker(CheckerManager &mgr) {
+  mgr.registerChecker<StoreToImmutableChecker>();
+}
+
+bool ento::shouldRegisterStoreToImmutableChecker(const CheckerManager &mgr) {
+  return true;
+}

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

@@ -0,0 +1,121 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -verify %s
+
+// Test basic functionality of StoreToImmutable checker for the C programming language.
+
+const int tentative_global_const; // expected-note {{Memory region is declared as immutable here}}
+
+void test_direct_write_to_tentative_const_global() {
+  *(int*)&tentative_global_const = 100; // expected-warning {{Trying to write to immutable memory in global read-only storage}}
+}
+
+const int global_const = 42; // expected-note {{Memory region is declared as immutable here}}
+
+void test_direct_write_to_const_global() {
+  // This should trigger a warning about writing to immutable memory
+  *(int*)&global_const = 100; // expected-warning {{Trying to write to immutable memory in global read-only storage}}
+}
+
+void test_write_through_const_pointer() {
+  const int local_const = 10; // expected-note {{Memory region is declared as immutable here}}
+  int *ptr = (int*)&local_const;
+  *ptr = 20; // expected-warning {{Trying to write to immutable memory}}
+}
+
+void test_write_to_const_array() {
+  const int arr[5] = {1, 2, 3, 4, 5}; // expected-note {{Enclosing memory region is declared as immutable here}}
+  int *ptr = (int*)arr;
+  ptr[0] = 10; // expected-warning {{Trying to write to immutable memory}}
+}
+
+struct TestStruct {
+  const int x; // expected-note 2 {{Memory region is declared as immutable here}}
+  int y;
+};
+
+void test_write_to_const_struct_member() {
+  struct TestStruct s = {1, 2};
+  int *ptr = (int*)&s.x;
+  *ptr = 10; // expected-warning {{Trying to write to immutable memory}}
+}
+
+const int global_array[3] = {1, 2, 3}; // expected-note {{Enclosing memory region is declared as immutable here}}
+
+void test_write_to_const_global_array() {
+  int *ptr = (int*)global_array;
+  ptr[0] = 10; // expected-warning {{Trying to write to immutable memory in global read-only storage}}
+}
+
+const struct TestStruct global_struct = {1, 2};
+
+void test_write_to_const_global_struct() {
+  int *ptr = (int*)&global_struct.x;
+  *ptr = 10; // expected-warning {{Trying to write to immutable memory in global read-only storage}}
+}
+
+
+void test_write_to_const_param(const int param) { // expected-note {{Memory region is declared as immutable here}}
+  *(int*)&param = 100; // expected-warning {{Trying to write to immutable memory}}
+}
+
+void test_write_to_const_ptr_param(const int *param) {
+  *(int*)param = 100; // expected-warning {{Trying to write to immutable memory}}
+}
+
+void test_write_to_const_array_param(const int arr[5]) {
+  *(int*)arr = 100; // expected-warning {{Trying to write to immutable memory}}
+}
+
+struct ParamStruct {
+  const int z; // expected-note 2 {{Memory region is declared as immutable here}}
+  int w;
+};
+
+void test_write_to_const_struct_param(const struct ParamStruct s) {
+  *(int*)&s.z = 100; // expected-warning {{Trying to write to immutable memory}}
+}
+
+void test_write_to_const_struct_ptr_param(const struct ParamStruct *s) {
+  *(int*)&s->z = 100; // expected-warning {{Trying to write to immutable memory}}
+}
+
+void test_write_to_nonconst() {
+  int non_const = 42;
+  *(int*)&non_const = 100; // No warning expected
+}
+
+int global_non_const = 42;
+
+void test_write_to_nonconst_global() {
+  *(int*)&global_non_const = 100; // No warning expected
+}
+
+struct NonConstStruct {
+  int x;
+  int y;
+};
+
+void test_write_to_nonconst_struct_member() {
+  struct NonConstStruct s = {1, 2};
+  *(int*)&s.x = 100; // No warning expected
+}
+
+void test_write_to_nonconst_param(int param) {
+  *(int*)&param = 100; // No warning expected
+}
+
+void test_normal_assignment() {
+  int x = 42;
+  x = 100; // No warning expected
+}
+
+void test_const_ptr_to_nonconst_data() {
+  int data = 42;
+  const int *ptr = &data;
+  *(int*)ptr = 100; // No warning expected
+}
+
+void test_const_ptr_to_const_data() {
+  const int data = 42; // expected-note {{Memory region is declared as immutable here}}
+  const int *ptr = &data;
+  *(int*)ptr = 100; // expected-warning {{Trying to write to immutable memory}}
+} 

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

@@ -0,0 +1,72 @@
+// 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 {{Trying to write to immutable memory}}
+}
+
+// FIXME: This should warn in C mode too.
+void test_write_to_string_literal() {
+  char *str = (char*)"hello";
+  str[0] = 'H'; // expected-warning {{Trying to write to immutable memory}}
+}
+
+struct ParamStruct {
+  const int z; // expected-note {{Memory region is declared as immutable here}}
+  int w;
+};
+
+void test_write_to_const_struct_ref_param(const ParamStruct &s) {
+  *(int*)&s.z = 100; // expected-warning {{Trying to write to immutable memory}}
+}
+
+void test_const_ref_to_nonconst_data() {
+  int data = 42;
+  const int &ref = data;
+  *(int*)&ref = 100; // No warning expected
+} 
+
+void test_const_ref_to_const_data() {
+  const int data = 42; // expected-note {{Memory region is declared as immutable here}}
+  const int &ref = data;
+  *(int*)&ref = 100; // expected-warning {{Trying to write to immutable memory}}
+} 
+
+void test_ref_to_nonconst_data() {
+  int data = 42;
+  int &ref = data;
+  ref = 100; // No warning expected
+}
+
+void test_ref_to_const_data() {
+  const int data = 42; // expected-note {{Memory region is declared as immutable here}}
+  int &ref = *(int*)&data;
+  ref = 100; // expected-warning {{Trying to write to immutable memory}}
+}
+
+struct MultipleLayerStruct {
+  MultipleLayerStruct();
+  const int data; // expected-note {{Memory region is declared as immutable here}}
+  const int buf[10]; // expected-note {{Enclosing memory region is declared as immutable here}}
+};
+
+MultipleLayerStruct MLS[10];
+
+void test_multiple_layer_struct_array_member() {
+  int *p = (int*)&MLS[2].data;
+  *p = 4; // expected-warning {{Trying to write to immutable memory}}
+}
+
+void test_multiple_layer_struct_array_array_member() {
+  int *p = (int*)&MLS[2].buf[3];
+  *p = 4; // expected-warning {{Trying to write to immutable memory}}
+}
+
+struct StructWithNonConstMember {
+  int x;
+};
+
+const StructWithNonConstMember SWNCM{0}; // expected-note {{Enclosing memory region is declared as immutable here}}
+
+void test_write_to_non_const_member_of_const_struct() {
+  *(int*)&SWNCM.x = 100; // expected-warning {{Trying to write to immutable memory in global read-only storage}}
+}

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

@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++14 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++17 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -std=c++20 -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 test case will no longer be a cornercase in the checker.
+
+void test_const_lambda_initialization_pre_cpp17() {
+  const auto lambda = [](){}; // No warning expected
+}