[clang] [clang][analyzer] Avoid ArrayBound false positives for container-of expression (PR #214140)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 5 00:02:59 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Radovan Božić (bozicrHT)
<details>
<summary>Changes</summary>
`ArrayBoundChecker` reports false positives for `container-of`-style pointer arithmetic. The pointer starts at an embedded field and subtracts the field offset to recover the containing structure. `computeOffset()` follows consecutive `ElementRegion`s. Once it reaches the `FieldRegion`, traversal ends and the embedded field becomes the bounds owner. The negative adjustment is therefore checked relative to the field instead of the containing structure.
The discussion around issue #<!-- -->104771 considers broader solutions. This patch takes a narrower, checker-local approach. It does not change expression tracking or the analyzer's value model. When the region hierarchy proves that:
- the pointer originated from a direct field;
- the result is viewed as the field's containing record;
- the negative character offset exactly matches the field's ABI offset;
- the backing storage can contain the record;
the checker continues bounds calculation from the field's parent region.
Fixes #<!-- -->104771
---
Full diff: https://github.com/llvm/llvm-project/pull/214140.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp (+175)
- (added) clang/test/Analysis/ArrayBound/container-of.c (+326)
``````````diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
index 67110f021bc56..f2e1b0b008bc4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
@@ -192,6 +192,171 @@ class ArrayBoundChecker : public Checker<check::PostStmt<ArraySubscriptExpr>,
} // anonymous namespace
+static QualType getRegionObjectType(const MemRegion *Region) {
+ if (const auto *TVR = dyn_cast<TypedValueRegion>(Region))
+ return TVR->getValueType();
+ if (const auto *SR = dyn_cast<SymbolicRegion>(Region))
+ return SR->getPointeeStaticType();
+ return {};
+}
+
+/// Return true when the region containing \p ContainerRegion has type
+/// \p ContainerType. ElementRegion represents both array elements and casts,
+/// so the type of ContainerRegion itself is not sufficient evidence.
+static bool hasContainerTypeProvenance(const SubRegion *ContainerRegion,
+ QualType ContainerType,
+ ASTContext &Ctx) {
+ const MemRegion *StorageRegion = ContainerRegion;
+ if (const auto *ER = dyn_cast<ElementRegion>(ContainerRegion)) {
+ if (!ASTContext::hasSameUnqualifiedType(ER->getElementType(),
+ ContainerType))
+ return false;
+ StorageRegion = ER->getSuperRegion();
+ }
+
+ QualType StorageType = getRegionObjectType(StorageRegion);
+ if (StorageType.isNull())
+ return false;
+
+ if (const ArrayType *AT = Ctx.getAsArrayType(StorageType))
+ StorageType = AT->getElementType();
+
+ return ASTContext::hasSameUnqualifiedType(StorageType, ContainerType);
+}
+
+/// Return whether the concrete storage containing \p ContainerRegion is large
+/// enough to contain an object of \p ContainerType at that region's offset.
+/// Return std::nullopt when either the offset or the extent is symbolic.
+static std::optional<bool>
+hasSufficientContainerExtent(ProgramStateRef State,
+ const SubRegion *ContainerRegion,
+ QualType ContainerType, SValBuilder &SVB) {
+ ASTContext &Ctx = SVB.getContext();
+ RegionOffset Offset = ContainerRegion->getAsOffset();
+ if (!Offset.isValid() || Offset.hasSymbolicOffset())
+ return std::nullopt;
+
+ const int64_t OffsetBits = Offset.getOffset();
+ const uint64_t CharWidth = Ctx.getCharWidth();
+ if (OffsetBits < 0 || static_cast<uint64_t>(OffsetBits) % CharWidth != 0)
+ return false;
+
+ const MemRegion *BaseRegion = Offset.getRegion();
+ const auto BaseExtent =
+ getDynamicExtent(State, BaseRegion, SVB).getAs<nonloc::ConcreteInt>();
+ if (!BaseExtent)
+ return std::nullopt;
+
+ const int64_t ContainerSize =
+ Ctx.getTypeSizeInChars(ContainerType).getQuantity();
+ if (ContainerSize < 0)
+ return false;
+
+ const uint64_t OffsetChars = static_cast<uint64_t>(OffsetBits) / CharWidth;
+ const uint64_t ContainerSizeChars = static_cast<uint64_t>(ContainerSize);
+ if (OffsetChars > std::numeric_limits<uint64_t>::max() - ContainerSizeChars)
+ return false;
+
+ const uint64_t RequiredExtent = OffsetChars + ContainerSizeChars;
+ const llvm::APSInt RequiredExtentValue =
+ llvm::APSInt::getUnsigned(RequiredExtent);
+ return llvm::APSInt::compareValues(*BaseExtent->getValue(),
+ RequiredExtentValue) >= 0;
+}
+
+/// Recognize the region shape produced when a pointer to a direct field is
+/// adjusted back to the beginning of its containing record. For example,
+///
+/// (struct Parent *)((char *)&P.Field - offsetof(struct Parent, Field))
+///
+/// is represented as:
+///
+/// ElementRegion<Parent, 0>
+/// ElementRegion<char, -offsetof(Parent, Field)>
+/// FieldRegion<Parent::Field>
+/// <region for P>
+///
+/// The character ElementRegion is absent when the field offset is zero. Return
+/// the region for P only when the record type, field declaration, target ABI
+/// layout, and underlying storage prove that the adjustment lands exactly at
+/// the beginning of P.
+static const SubRegion *
+getContainerOfParentRegion(const ElementRegion *ContainerER,
+ ProgramStateRef State, SValBuilder &SVB) {
+ ASTContext &Ctx = SVB.getContext();
+ const MemRegion *SuperRegion = ContainerER->getSuperRegion();
+ const FieldRegion *FieldR = nullptr;
+ int64_t CharacterIndex = 0;
+
+ if (const auto *CharacterER = dyn_cast<ElementRegion>(SuperRegion)) {
+ QualType CharacterType = CharacterER->getElementType();
+ if (!CharacterType->isCharType() ||
+ Ctx.getTypeSizeInChars(CharacterType).getQuantity() != 1)
+ return nullptr;
+
+ const auto ConcreteIndex =
+ CharacterER->getIndex().getAs<nonloc::ConcreteInt>();
+ if (!ConcreteIndex)
+ return nullptr;
+
+ std::optional<int64_t> Index = ConcreteIndex->getValue()->tryExtValue();
+ if (!Index)
+ return nullptr;
+ CharacterIndex = *Index;
+
+ FieldR = dyn_cast<FieldRegion>(CharacterER->getSuperRegion());
+ } else {
+ // SValBuilder folds an adjustment of zero, so a first field is represented
+ // without an intermediate character ElementRegion.
+ FieldR = dyn_cast<FieldRegion>(SuperRegion);
+ }
+
+ if (!FieldR)
+ return nullptr;
+
+ const FieldDecl *Field = FieldR->getDecl();
+ if (Field->isBitField())
+ return nullptr;
+
+ QualType ContainerType =
+ ContainerER->getElementType().getCanonicalType().getUnqualifiedType();
+ const auto *ContainerRT = ContainerType->getAs<RecordType>();
+ if (!ContainerRT)
+ return nullptr;
+
+ const RecordDecl *FieldParent = Field->getParent();
+ if (!FieldParent || !FieldParent->isCompleteDefinition() ||
+ ContainerRT->getDecl()->getCanonicalDecl() !=
+ FieldParent->getCanonicalDecl())
+ return nullptr;
+
+ const uint64_t FieldOffsetBits = Ctx.getFieldOffset(Field);
+ const uint64_t CharWidth = Ctx.getCharWidth();
+ if (FieldOffsetBits % CharWidth != 0 || CharacterIndex > 0)
+ return nullptr;
+
+ // Avoid negating INT64_MIN while comparing the signed character index with
+ // the unsigned ABI field offset.
+ const uint64_t BackwardOffset =
+ static_cast<uint64_t>(-(CharacterIndex + 1)) + 1;
+ if (BackwardOffset != FieldOffsetBits / CharWidth)
+ return nullptr;
+
+ const auto *ParentRegion = dyn_cast<SubRegion>(FieldR->getSuperRegion());
+ if (!ParentRegion)
+ return nullptr;
+
+ std::optional<bool> HasSufficientExtent =
+ hasSufficientContainerExtent(State, ParentRegion, ContainerType, SVB);
+ if (HasSufficientExtent && !*HasSufficientExtent)
+ return nullptr;
+ if (!HasSufficientExtent &&
+ !hasContainerTypeProvenance(ParentRegion, ContainerType, Ctx))
+ return nullptr;
+
+ return ParentRegion;
+}
+
/// For a given Location that can be represented as a symbolic expression
/// Arr[Idx] (or perhaps Arr[Idx1][Idx2] etc.), return the parent memory block
/// Arr and the distance of Location from the beginning of Arr (expressed in a
@@ -236,6 +401,16 @@ computeOffset(ProgramStateRef State, SValBuilder &SVB, SVal Location) {
if (!Offset)
return std::nullopt;
+ if (const SubRegion *ParentRegion =
+ getContainerOfParentRegion(CurRegion, State, SVB)) {
+ // The negative character offset exactly cancels the field's offset in
+ // its parent record. Continue from the parent so that an enclosing array
+ // (if any) remains the bounds owner.
+ OwnerRegion = ParentRegion;
+ CurRegion = dyn_cast<ElementRegion>(OwnerRegion);
+ continue;
+ }
+
OwnerRegion = CurRegion->getSuperRegion()->getAs<SubRegion>();
// When this is just another ElementRegion layer, we need to continue the
// offset calculations:
diff --git a/clang/test/Analysis/ArrayBound/container-of.c b/clang/test/Analysis/ArrayBound/container-of.c
new file mode 100644
index 0000000000000..a1a997f0f1cf7
--- /dev/null
+++ b/clang/test/Analysis/ArrayBound/container-of.c
@@ -0,0 +1,326 @@
+// RUN: %clang_analyze_cc1 -Wno-array-bounds -Wno-address-of-packed-member \
+// RUN: -analyzer-checker=core,security.ArrayBound,unix.Malloc \
+// RUN: -verify %s
+//
+
+#define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER)
+#define container_of(PTR, TYPE, MEMBER) \
+ ((TYPE *)((char *)(PTR) - offsetof(TYPE, MEMBER)))
+#define container_of_uchar(PTR, TYPE, MEMBER) \
+ ((TYPE *)((unsigned char *)(PTR) - offsetof(TYPE, MEMBER)))
+#define container_of_typed(PTR, TYPE, MEMBER) ({ \
+ const __typeof__(((TYPE *)0)->MEMBER) *__member_ptr = (PTR); \
+ (TYPE *)((char *)__member_ptr - offsetof(TYPE, MEMBER)); \
+})
+
+void *malloc(__SIZE_TYPE__);
+void free(void *);
+
+struct Test {
+ int a;
+ int b;
+};
+
+static void update_a(int *b) {
+ struct Test *head = container_of_typed(b, struct Test, b);
+ head->a = 10; // no-warning
+}
+
+void scalar_member(void) {
+ struct Test object = {0};
+ update_a(&object.b);
+}
+
+struct Child {
+ int value;
+};
+
+struct Parent {
+ int id;
+ struct Child child;
+ int tail;
+};
+
+static void set_id(struct Child *child) {
+ struct Parent *parent =
+ container_of_typed(child, struct Parent, child);
+ parent->id = 1; // no-warning
+}
+
+void direct_member(void) {
+ struct Parent object = {0};
+ set_id(&object.child);
+}
+
+static int read_tail(struct Child *child) {
+ struct Parent *parent = container_of(child, struct Parent, child);
+ return parent->tail; // no-warning
+}
+
+struct Holder {
+ struct Parent *parent;
+};
+
+int symbolic_parent(struct Holder *holder) {
+ return read_tail(&holder->parent->child); // no-warning
+}
+
+struct PathList {
+ int flags;
+};
+
+struct Route {
+ char pad[56];
+ struct PathList pathlist;
+ void *head;
+};
+
+struct QueuedRoute {
+ struct Route *route;
+};
+
+static void bind_pathlist(struct PathList *pathlist) {
+ struct Route *route = container_of(pathlist, struct Route, pathlist);
+ if (route->head) // no-warning
+ (void)0;
+}
+
+void symbolic_field_parent(struct QueuedRoute *queued) {
+ bind_pathlist(&queued->route->pathlist);
+}
+
+struct GrandParent {
+ int prefix;
+ struct Parent parent;
+};
+
+int nested_parent(void) {
+ struct GrandParent object = {0};
+ struct Parent *parent =
+ container_of(&object.parent.child, struct Parent, child);
+ return parent->tail; // no-warning
+}
+
+void containing_array(void) {
+ struct Parent objects[2] = {0};
+ struct Parent *parent =
+ container_of(&objects[0].child, struct Parent, child);
+ parent[1].tail = 1; // no-warning
+}
+
+void containing_array_from_second_element(void) {
+ struct Parent objects[2] = {0};
+ struct Parent *parent =
+ container_of(&objects[1].child, struct Parent, child);
+ (parent - 1)->id = 1; // no-warning
+}
+
+struct FirstMember {
+ struct Child child;
+ int tail;
+};
+
+int zero_offset_field(void) {
+ struct FirstMember object = {0};
+ struct FirstMember *parent =
+ container_of(&object.child, struct FirstMember, child);
+ return parent->tail; // no-warning
+}
+
+void zero_offset_containing_array(void) {
+ struct FirstMember objects[2] = {0};
+ struct FirstMember *parent =
+ container_of(&objects[0].child, struct FirstMember, child);
+ parent[1].tail = 1; // no-warning
+}
+
+union ParentUnion {
+ struct Child child;
+ int value;
+};
+
+void union_containing_array(void) {
+ union ParentUnion objects[2] = {0};
+ union ParentUnion *parent =
+ container_of(&objects[0].child, union ParentUnion, child);
+ parent[1].value = 1; // no-warning
+}
+
+struct PackedParent {
+ char tag;
+ struct Child child;
+ int tail;
+} __attribute__((packed));
+
+int packed_parent(void) {
+ struct PackedParent object = {0};
+ struct PackedParent *parent =
+ container_of(&object.child, struct PackedParent, child);
+ return parent->tail; // no-warning
+}
+
+int unsigned_character_arithmetic(void) {
+ struct Parent object = {0};
+ struct Parent *parent =
+ container_of_uchar(&object.child, struct Parent, child);
+ return parent->tail; // no-warning
+}
+
+int sufficient_raw_storage(void) {
+ unsigned char storage[sizeof(struct Parent)] = {0};
+ struct Parent *object = (struct Parent *)storage;
+ struct Parent *parent =
+ container_of(&object->child, struct Parent, child);
+ parent->tail = 1; // no-warning
+ return parent->tail; // no-warning
+}
+
+int sufficient_heap_storage(void) {
+ struct Parent *object = (struct Parent *)malloc(sizeof(*object));
+ if (!object)
+ return 0;
+
+ struct Parent *parent =
+ container_of(&object->child, struct Parent, child);
+ parent->tail = 1; // no-warning
+ int result = parent->tail; // no-warning
+ free(object);
+ return result;
+}
+
+struct ForwardParent;
+struct ForwardParent {
+ int id;
+ struct Child child;
+};
+
+int forward_declared_parent(void) {
+ struct ForwardParent object = {0};
+ struct ForwardParent *parent =
+ container_of(&object.child, struct ForwardParent, child);
+ return parent->id; // no-warning
+}
+
+int split_adjustment(void) {
+ struct Parent object = {0};
+ char *address = (char *)&object.child;
+ address -= offsetof(struct Parent, child);
+ struct Parent *parent = (struct Parent *)address;
+ return parent->tail; // no-warning
+}
+
+// The matcher relies on region provenance and the ABI field offset, not on an
+// OffsetOfExpr surviving in the subtraction expression.
+enum { ParentChildOffset = offsetof(struct Parent, child) };
+
+int saved_offset_constant(void) {
+ struct Parent object = {0};
+ struct Parent *parent =
+ (struct Parent *)((char *)&object.child - ParentChildOffset);
+ return parent->tail; // no-warning
+}
+
+int off_by_one_before_parent(void) {
+ struct Parent object = {0};
+ struct Parent *parent =
+ (struct Parent *)((char *)&object.child -
+ offsetof(struct Parent, child) - 1);
+ return parent->id; // expected-warning{{Out of bound access to memory}}
+}
+
+struct OtherParent {
+ int prefix[2];
+ struct Child child;
+ int tail;
+};
+
+int wrong_parent_type(void) {
+ struct Parent object = {0};
+ struct OtherParent *parent =
+ container_of(&object.child, struct OtherParent, child);
+ return parent->prefix[0]; // expected-warning{{Out of bound access to memory}}
+}
+
+int raw_storage_with_sufficient_extent(void) {
+ unsigned char storage[sizeof(struct Parent)] = {0};
+ struct Parent *fake_parent = (struct Parent *)storage;
+ struct Parent *parent =
+ container_of(&fake_parent->child, struct Parent, child);
+ return parent->tail; // no-warning
+}
+
+int unrelated_storage(void) {
+ int storage = 0;
+ struct Parent *fake_parent = (struct Parent *)&storage;
+ struct Parent *parent =
+ container_of(&fake_parent->child, struct Parent, child);
+ return parent->tail; // expected-warning{{Out of bound access to memory}}
+}
+
+int insufficient_raw_storage(void) {
+ unsigned char storage[sizeof(struct Parent) - 1] = {0};
+ struct Parent *fake_parent = (struct Parent *)storage;
+ struct Parent *parent =
+ container_of(&fake_parent->child, struct Parent, child);
+ return parent->tail; // expected-warning{{Out of bound access to memory}}
+}
+
+int insufficient_heap_storage(void) {
+ struct Parent *object = (struct Parent *)malloc(sizeof(*object) - 1);
+ // expected-warning at -1{{allocation of insufficient size}}
+ if (!object)
+ return 0;
+
+ struct Parent *parent =
+ container_of(&object->child, struct Parent, child);
+ parent->tail = 1; // expected-warning{{Out of bound access to memory}}
+ free(object);
+ return 0;
+}
+
+int standalone_child(void) {
+ struct Child child = {0};
+ struct Parent *parent = container_of(&child, struct Parent, child);
+ parent->id = 1; // expected-warning{{Out of bound access to memory}}
+ return 0;
+}
+
+int unrelated_storage_zero_offset(void) {
+ int storage = 0;
+ struct FirstMember *fake_parent = (struct FirstMember *)&storage;
+ struct FirstMember *parent =
+ container_of(&fake_parent->child, struct FirstMember, child);
+ return parent[1].tail; // expected-warning{{Out of bound access to memory}}
+}
+
+struct TwoChildren {
+ int id;
+ struct Child first;
+ struct Child second;
+};
+
+int wrong_member_offset(void) {
+ struct TwoChildren object = {0};
+ struct TwoChildren *parent =
+ container_of(&object.first, struct TwoChildren, second);
+ return parent->id; // expected-warning{{Out of bound access to memory}}
+}
+
+int before_reconstructed_parent(void) {
+ struct Parent object = {0};
+ struct Parent *parent = container_of(&object.child, struct Parent, child);
+ return (parent - 1)->id; // expected-warning{{Out of bound access to memory}}
+}
+
+int after_reconstructed_parent(void) {
+ struct Parent object = {0};
+ struct Parent *parent = container_of(&object.child, struct Parent, child);
+ return (parent + 1)->id; // expected-warning{{Out of bound access to memory}}
+}
+
+int after_containing_array(void) {
+ struct Parent objects[2] = {0};
+ struct Parent *parent =
+ container_of(&objects[0].child, struct Parent, child);
+ return parent[2].id; // expected-warning{{Out of bound access to memory}}
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/214140
More information about the cfe-commits
mailing list