https://github.com/bozicrHT created https://github.com/llvm/llvm-project/pull/214140
`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 From ce00b3f42f3900ae0d047349ea8594d17df1b82c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Wed, 5 Aug 2026 08:30:22 +0200 Subject: [PATCH] [clang][analyzer] Avoid ArrayBound false positives for container-of expression ArrayBoundChecker treats an embedded field as the bounds owner when a pointer is adjusted back to its containing record. This produces a false negative-offset warnings even when the adjustment exactly matches the field's ABI offset. Recognize this MemRegion pattern, validate the record layout and backing storage, and continue bounds checking from the containing record. Add positive and negative regression tests. --- .../Checkers/ArrayBoundChecker.cpp | 175 ++++++++++ clang/test/Analysis/ArrayBound/container-of.c | 326 ++++++++++++++++++ 2 files changed, 501 insertions(+) create mode 100644 clang/test/Analysis/ArrayBound/container-of.c 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@-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}} +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
