Skip to content

[clang][analyzer] Add StoreToImmutable checker #150417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Aug 4, 2025
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
0630d81
[clang][analyzer] Add StoreToImmutable checker
gamesh411 Jul 24, 2025
fa3f84f
Apply review suggestiongs by @steakhal
gamesh411 Jul 25, 2025
b022182
[review-fix] fix test files
gamesh411 Jul 28, 2025
5190ee0
[review-fix] add test case for complex memory hierarchy
gamesh411 Jul 28, 2025
856a865
[review-fix] remove isInSystemMacro check
gamesh411 Jul 28, 2025
d8f3456
[review-fix] add example note on string literal limitation
gamesh411 Jul 28, 2025
01d0521
[review-fix] implement hierarchical memregion handling
gamesh411 Jul 28, 2025
2aacf92
[cornercase] Lambda initialization gives a false positive in C++14 an…
gamesh411 Jul 29, 2025
6e8a332
[format] fixed example file code formatting
gamesh411 Jul 29, 2025
d65aa88
[review-fix] don't repeat type names
gamesh411 Jul 29, 2025
7e94b10
[cornercase] fix false positive cornercase
gamesh411 Jul 29, 2025
4db2804
[review-fix] streamline example file
gamesh411 Jul 30, 2025
7e73177
[review-fix] add more C++ standard versions
gamesh411 Jul 30, 2025
cac94fe
[review-fix] streamline implementation
gamesh411 Jul 30, 2025
373679b
[review-fix] support SubRegions not just ElementRegions
gamesh411 Jul 30, 2025
7cbabf8
[review-fix] fix typo
gamesh411 Jul 30, 2025
e377b19
[review-fix] delete stray whitespace
gamesh411 Jul 30, 2025
cfedf88
[review-fix] more elaborate notes
gamesh411 Aug 1, 2025
fa0a379
[review-fix] remove redundant comments from example
gamesh411 Aug 1, 2025
4e6c988
[review-fix] document our options for the fixme
gamesh411 Aug 1, 2025
17a0e9c
Merge branch 'main' into store-to-immutable-checker
gamesh411 Aug 1, 2025
89389a5
[review-fix] clarify wording
gamesh411 Aug 2, 2025
4d883f1
fix formatting
gamesh411 Aug 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions clang/docs/analyzer/checkers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3077,6 +3077,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
^^^^^^^^^^^^^^^

Expand Down
23 changes: 23 additions & 0 deletions clang/docs/analyzer/checkers/storetoimmutable_example.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Global const variable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer removing this comment and "// Const struct member" because they just state the obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed redundant comments from the example.

const int global_const = 42;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't emphasise "global" in variable names, as globals and local variables are handled the same way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true. Globals live in the global memory space where we have a suitable sub-space for immutable globals, like constants.

In contrast to this, in the stack space (where locals live) has no sub-space for constants. So there is an important semantic difference even for your checker because you also check the mspaces.

Copy link
Contributor

@NagyDonat NagyDonat Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steakhal You are right that globals and locals are handled differently by the checker implementation, what we meant under "handled the same way" is the user experience. This is an example file that will be included in the documentation so we shouldn't highlight the global/local nature of variables if the user will see the same behavior (modifying const values or parts of them is reported) in either case.

Alternatively including a global and a local variable to show that both produce the same behavior is also OK (I feel that it's a bit too verbose, but I'm not opposed to it).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I misinterpreted the context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made the simplification, and it still has an example for a global and a local variable, demonstrating that misuses of both are detected.


// Const struct member
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 works in C++, but not in C, as the analyzer treats
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps replace "works" with "is reported" because "works" can be misunderstood as "is standard-compliant".

// 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
}
5 changes: 5 additions & 0 deletions clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,11 @@ def StackAddrAsyncEscapeChecker : Checker<"StackAddressAsyncEscape">,
Dependencies<[StackAddrEscapeBase]>,
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>,
Expand Down
1 change: 1 addition & 0 deletions clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ add_clang_library(clangStaticAnalyzerCheckers
SmartPtrChecker.cpp
SmartPtrModeling.cpp
StackAddrEscapeChecker.cpp
StoreToImmutableChecker.cpp
StdLibraryFunctionsChecker.cpp
StdVariantChecker.cpp
STLAlgorithmModeling.cpp
Expand Down
168 changes: 168 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
//=== 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;

private:
bool isInitializationContext(const Stmt *S, CheckerContext &C) const;
bool isEffectivelyConstRegion(const MemRegion *MR, CheckerContext &C) const;
};
} // end anonymous namespace

bool StoreToImmutableChecker::isInitializationContext(const Stmt *S,
CheckerContext &C) const {
// 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 once the API of checkBind would allow to
// distinguish between initialization and assignment, because this information
// is already available in the engine, it is just not passed to the checker
// API.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// FIXME: This should be eliminated once the API of checkBind would allow to
// distinguish between initialization and assignment, because this information
// is already available in the engine, it is just not passed to the checker
// API.
// 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.

As we discussed in person, the most probable solution to this corner case is slightly different from what you originally wrote here. I hope that this will become irrelevant soon (if you can restore consistency in the engine), but it is still slightly better to document this more accurately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this with a sidenote.

const auto *ConstructExp = dyn_cast<CXXConstructExpr>(S);
return ConstructExp && ConstructExp->isElidable();
}

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 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 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::isEffectivelyConstRegion(
const MemRegion *MR, CheckerContext &C) const {
while (true) {
if (isEffectivelyConstRegionAux(MR, C))
return true;
if (auto *SR = dyn_cast<SubRegion>(MR))
MR = SR->getSuperRegion();
else
return false;
}
}

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 corresponds to a const variable
if (!isEffectivelyConstRegion(MR, C))
return;

// Generate the bug report
ExplodedNode *N = C.generateNonFatalErrorNode();
if (!N)
return;

constexpr llvm::StringLiteral Msg = "Trying to write to immutable memory.";

auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
R->addRange(S->getSourceRange());

// If the location that is being written to has a declaration, place a note.
if (const DeclRegion *DR = dyn_cast<DeclRegion>(MR)) {
R->addNote(
"Memory region is declared as immutable here",
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;
}
120 changes: 120 additions & 0 deletions clang/test/Analysis/store-to-immutable-basic.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
// 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}}
}

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}}
}

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};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could give a note to the enclosing array declaration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm currently working on a better note placement logic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added more elaborate note implementation and tests.

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};

void test_write_to_const_global_array() {
int *ptr = (int*)global_array;
ptr[0] = 10; // expected-warning {{Trying to write to immutable memory}}
}

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}}
}

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}}
}
Loading
Loading