llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Endre Fülöp (gamesh411) <details> <summary>Changes</summary> 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 detect writes to: - Global const variables - 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 --- Full diff: https://github.com/llvm/llvm-project/pull/150417.diff 7 Files Affected: - (modified) clang/docs/analyzer/checkers.rst (+17) - (added) clang/docs/analyzer/checkers/storetoimmutable_example.cpp (+30) - (modified) clang/include/clang/StaticAnalyzer/Checkers/Checkers.td (+5) - (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (+1-1) - (modified) clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt (+1) - (added) clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp (+146) - (added) clang/test/Analysis/store-to-immutable-basic.cpp (+166) ``````````diff diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 26c5028e04955..a00e9d81ba208 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -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 ^^^^^^^^^^^^^^^ diff --git a/clang/docs/analyzer/checkers/storetoimmutable_example.cpp b/clang/docs/analyzer/checkers/storetoimmutable_example.cpp new file mode 100644 index 0000000000000..e1a0683ff91e4 --- /dev/null +++ b/clang/docs/analyzer/checkers/storetoimmutable_example.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable %s + +// Global const variable +const int global_const = 42; + +void test_global_const() { + *(int *)&global_const = 100; // warn: Writing to immutable memory +} + +// String literal +void test_string_literal() { + char *str = (char *)"hello"; + str[0] = 'H'; // warn: Writing to immutable memory +} + +// Const parameter +void test_const_param(const int param) { + *(int *)¶m = 100; // warn: Writing to immutable memory +} + +// Const struct member +struct TestStruct { + const int x; + int y; +}; + +void test_const_member() { + TestStruct s = {1, 2}; + *(int *)&s.x = 10; // warn: Writing to immutable memory +} \ No newline at end of file diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 2234143004b6f..8799f1d15b0b6 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -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>, diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h index 89d306fb94046..0f7bab6839703 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -819,7 +819,7 @@ class SymbolicRegion : public SubRegion { s->getType()->isReferenceType() || s->getType()->isBlockPointerType()); assert(isa<UnknownSpaceRegion>(sreg) || isa<HeapSpaceRegion>(sreg) || - isa<GlobalSystemSpaceRegion>(sreg)); + isa<GlobalSystemSpaceRegion>(sreg) || isa<GlobalImmutableSpaceRegion>(sreg)); } public: diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 22dd3f0374849..78360418a8b81 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/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 diff --git a/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp new file mode 100644 index 0000000000000..a853e2e3f9c4a --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp @@ -0,0 +1,146 @@ +//=== StoreToImmutableChecker.cpp - Store to immutable memory checker -*- 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 isConstVariable(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 { + // 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 ParamVarRegion with a const-qualified type + if (const ParamVarRegion *PVR = dyn_cast<ParamVarRegion>(MR)) { + const ParmVarDecl *PVD = PVR->getDecl(); + if (PVD && PVD->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()) + return true; + } + + // Check if this is a StringRegion (string literals are const) + if (isa<StringRegion>(MR)) + return true; + + // Check if this is a SymbolicRegion with a const-qualified pointee type + if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR)) { + QualType PointeeType = SR->getPointeeStaticType(); + if (PointeeType.isConstQualified()) + 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); + } + + 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(); + } + 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 + if (isa<DeclStmt>(S) || isa<DeclRefExpr>(S)) + return; + + // Check if the region corresponds to a const variable + if (!isConstVariable(MR, C)) + return; + + const SourceManager &SM = C.getSourceManager(); + // Skip if this is a system macro (likely from system headers) + if (SM.isInSystemMacro(S->getBeginLoc())) + return; + + // Generate the bug report + ExplodedNode *N = C.generateNonFatalErrorNode(); + if (!N) + return; + + constexpr llvm::StringLiteral Msg = + "Writing to immutable memory is undefined behavior. " + "This memory region is marked as immutable and should not be modified."; + + 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 in immutable space", + PathDiagnosticLocation::create(DR->getDecl(), SM)); + } + + // 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; +} \ No newline at end of file diff --git a/clang/test/Analysis/store-to-immutable-basic.cpp b/clang/test/Analysis/store-to-immutable-basic.cpp new file mode 100644 index 0000000000000..2b28b933e97b4 --- /dev/null +++ b/clang/test/Analysis/store-to-immutable-basic.cpp @@ -0,0 +1,166 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.StoreToImmutable -verify %s + +// Test basic functionality of StoreToImmutable checker +// This tests direct writes to immutable regions without function modeling + +// Direct write to a const global variable +const int global_const = 42; // expected-note {{Memory region is in immutable space}} + +void test_direct_write_to_const_global() { + // This should trigger a warning about writing to immutable memory + *(int*)&global_const = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write through a pointer to const memory +void test_write_through_const_pointer() { + const int local_const = 10; // expected-note {{Memory region is in immutable space}} + int *ptr = (int*)&local_const; + *ptr = 20; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to string literal (should be in immutable space) +void test_write_to_string_literal() { + char *str = (char*)"hello"; + str[0] = 'H'; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const array +void test_write_to_const_array() { + const int arr[5] = {1, 2, 3, 4, 5}; + int *ptr = (int*)arr; + ptr[0] = 10; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const struct member +struct TestStruct { + const int x; // expected-note 2{{Memory region is in immutable space}} + int y; +}; + +void test_write_to_const_struct_member() { + TestStruct s = {1, 2}; + int *ptr = (int*)&s.x; + *ptr = 10; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const global array +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 {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const global struct +const TestStruct global_struct = {1, 2}; + +void test_write_to_const_global_struct() { + int *ptr = (int*)&global_struct.x; + *ptr = 10; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const parameter +void test_write_to_const_param(const int param) { // expected-note {{Memory region is in immutable space}} + *(int*)¶m = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const reference parameter +void test_write_to_const_ref_param(const int ¶m) { + *(int*)¶m = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const pointer parameter +void test_write_to_const_ptr_param(const int *param) { + *(int*)param = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const array parameter +void test_write_to_const_array_param(const int arr[5]) { + *(int*)arr = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const struct parameter +struct ParamStruct { + const int z; // expected-note 3{{Memory region is in immutable space}} + int w; +}; + +void test_write_to_const_struct_param(const ParamStruct s) { + *(int*)&s.z = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const struct reference parameter +void test_write_to_const_struct_ref_param(const ParamStruct &s) { + *(int*)&s.z = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +// Write to const struct pointer parameter +void test_write_to_const_struct_ptr_param(const ParamStruct *s) { + *(int*)&s->z = 100; // expected-warning {{Writing to immutable memory is undefined behavior}} + // expected-note@-1 {{Writing to immutable memory is undefined behavior. This memory region is marked as immutable and should not be modified}} +} + +//===--- NEGATIVE TEST CASES ---===// +// These tests should NOT trigger warnings + +// Write to non-const variable (should not warn) +void test_write_to_nonconst() { + int non_const = 42; + *(int*)&non_const = 100; // No warning expected +} + +// Write to non-const global variable (should not warn) +int global_non_const = 42; + +void test_write_to_nonconst_global() { + *(int*)&global_non_const = 100; // No warning expected +} + +// Write to non-const struct member (should not warn) +struct NonConstStruct { + int x; + int y; +}; + +void test_write_to_nonconst_struct_member() { + NonConstStruct s = {1, 2}; + *(int*)&s.x = 100; // No warning expected +} + +// Write to non-const parameter (should not warn) +void test_write_to_nonconst_param(int param) { + *(int*)¶m = 100; // No warning expected +} + +// Normal assignment to non-const variable (should not warn) +void test_normal_assignment() { + int x = 42; + x = 100; // No warning expected +} + +// Write to non-const data through const pointer (should not warn - underlying memory is non-const) +void test_const_ptr_to_nonconst_data() { + int data = 42; + const int *ptr = &data; + *(int*)ptr = 100; // No warning expected +} + +// Write to non-const data through const reference (should not warn - underlying memory is non-const) +void test_const_ref_to_nonconst_data() { + int data = 42; + const int &ref = data; + *(int*)&ref = 100; // No warning expected +} \ No newline at end of file `````````` </details> https://github.com/llvm/llvm-project/pull/150417 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
