https://github.com/bozicrHT updated https://github.com/llvm/llvm-project/pull/232824
From 840d34fad1f563b238cfb0c3c888d2d1332bd184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Fri, 16 Oct 2026 18:13:50 +0200 Subject: [PATCH 1/7] [clang][analyzer] Diagnose whole-object copies of uninitialized structs Inspect named struct fields when memory functions read a complete struct or struct array. Preserve the first/last-element array heuristic. Fixes #232652 --- .../Checkers/CStringChecker.cpp | 98 +++++++++++++++++-- clang/test/Analysis/bstring_UninitRead.c | 5 +- 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 3749c262e8bba9..88a73b0bd4d032 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -326,7 +326,8 @@ class CStringChecker // Check whether the origin region behind \p Element (like the actual array // region \p Element is from) is initialized. ProgramStateRef checkInit(CheckerContext &C, ProgramStateRef state, - AnyArgExpr Buffer, SVal Element, SVal Size) const; + AnyArgExpr Buffer, SVal Element, SVal Size, + CharKind CK) const; ProgramStateRef CheckLocation(CheckerContext &C, ProgramStateRef state, AnyArgExpr Buffer, SVal Element, AccessKind Access, @@ -447,16 +448,104 @@ static void printIdxWithOrdinalSuffix(llvm::raw_ostream &Os, unsigned Idx) { Os << Idx << llvm::getOrdinalSuffix(Idx); } +static const FieldRegion *findUninitializedField(ProgramStateRef State, + const TypedValueRegion *R) { + QualType Ty = R->getValueType(); + SValBuilder &SVB = State->getStateManager().getSValBuilder(); + ASTContext &Ctx = SVB.getContext(); + MemRegionManager &MRMgr = SVB.getRegionManager(); + + if (R->getValueType()->isUnionType()) + return nullptr; + + if (Ty->isArrayType()) { + const auto *First = R; + const auto *Last = R; + + while (const auto *AT = Ctx.getAsConstantArrayType(Ty)) { + if (AT->getSize().isZero()) + return nullptr; + + Ty = AT->getElementType(); + First = MRMgr.getElementRegion(Ty, SVB.makeZeroArrayIndex(), First, Ctx); + Last = MRMgr.getElementRegion( + Ty, SVB.makeArrayIndex(AT->getSize().getZExtValue() - 1), Last, Ctx); + } + + if (!Ty->isStructureType()) + return nullptr; + + if (const auto *FR = findUninitializedField(State, First)) + return FR; + + return First == Last ? nullptr : findUninitializedField(State, Last); + } + + const auto *RT = Ty->getAsStructureType(); + if (!RT) + return nullptr; + const auto *RD = RT->getDecl()->getDefinition(); + if (!RD) + return nullptr; + + for (const auto *FD : RD->fields()) { + if (!FD->getIdentifier()) + continue; + + const auto *FR = MRMgr.getFieldRegion(FD, R); + if (FD->getType()->isStructureType()) { + if (const auto *Uninit = findUninitializedField(State, FR)) + return Uninit; + } else if (!FD->getType()->isAggregateType() && State->getSVal(FR).isUndef()) + return FR; + } + return nullptr; +} + ProgramStateRef CStringChecker::checkInit(CheckerContext &C, ProgramStateRef State, AnyArgExpr Buffer, SVal Element, - SVal Size) const { + SVal Size, CharKind CK) const { // If a previous check has failed, propagate the failure. if (!State) return nullptr; SVal BufVal = C.getSVal(Buffer.Expression); + SValBuilder &SVB = C.getSValBuilder(); + ASTContext &Ctx = SVB.getContext(); + + // Field initialization is meaningful only when the entire source object is + // read. Do not widen partial copies or nonzero offsets to their base object + if (UninitializedRead.isEnabled()) { + const auto *BufR = BufVal.getAsRegion(); + const auto *Object = llvm::dyn_cast_or_null<TypedValueRegion>( + BufR ? BufR->StripCasts(false) : nullptr); + + if (Object && !isa<ElementRegion>(Object)) { + QualType Ty = Object->getValueType(); + const llvm::APSInt *Length = SVB.getKnownValue(State, Size); + CharUnits CharSize = Ctx.getTypeSizeInChars( + CK == CharKind::Regular ? Ctx.CharTy : Ctx.WideCharTy); + + if (Ctx.getBaseElementType(Ty)->isStructureType() && + !Ty->isIncompleteType() && !Ty->isVariablyModifiedType() && Length && + Ctx.getTypeSizeInChars(Ty).isMultipleOf(CharSize) && + *Length == Ctx.getTypeSizeInChars(Ty) / CharSize) { + if (const auto *FR = findUninitializedField(State, Object)) { + SmallString<128> Buf; + llvm::raw_svector_ostream OS(Buf); + OS << "The "; + printIdxWithOrdinalSuffix(OS, Buffer.ArgumentIndex + 1); + OS << " argument contains uninitialized data (e.g., field: '" + << FR->getDecl()->getName() << "')"; + emitUninitializedReadBug(C, State, Buffer.Expression, FR, OS.str()); + return nullptr; + } + } + } + } + const auto *ER = dyn_cast_or_null<ElementRegion>(BufVal.getAsRegion()); if (!ER) return State; @@ -470,9 +559,6 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C, if (!SuperR->getValueType()->isArrayType()) return State; - SValBuilder &SVB = C.getSValBuilder(); - ASTContext &Ctx = SVB.getContext(); - const QualType ElemTy = Ctx.getBaseElementType(SuperR->getValueType()); std::optional<Loc> FirstElementVal = @@ -670,7 +756,7 @@ CStringChecker::CheckBufferAccess(CheckerContext &C, ProgramStateRef State, svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy); State = CheckLocation(C, State, Buffer, BufEnd, Access, CK); if (Access == AccessKind::read) - State = checkInit(C, State, Buffer, BufStart, *Length); + State = checkInit(C, State, Buffer, BufStart, *Length, CK); // If the buffer isn't large enough, abort. if (!State) diff --git a/clang/test/Analysis/bstring_UninitRead.c b/clang/test/Analysis/bstring_UninitRead.c index 6c6fefdf9ff3ed..a330c6b8050c5b 100644 --- a/clang/test/Analysis/bstring_UninitRead.c +++ b/clang/test/Analysis/bstring_UninitRead.c @@ -128,9 +128,8 @@ void mempcpy_struct_fully_uninit() { struct st s1; struct st s2; - // FIXME: Maybe ask UninitializedObjectChecker whether s1 is fully - // initialized? - mempcpy(&s2, &s1, sizeof(struct st)); + mempcpy(&s2, &s1, sizeof(struct st)); // expected-warning{{The 2nd argument contains uninitialized data (e.g., field: 'i')}} + // expected-note@-1{{Other elements might also be undefined}} } // Creduced crash. In this case, an symbolicregion is wrapped in an From 7d0e697825f6a5cf72865e63417cab9734841717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Fri, 16 Oct 2026 18:25:48 +0200 Subject: [PATCH 2/7] Fix formatting --- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 88a73b0bd4d032..eeab88c2face8a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -456,7 +456,7 @@ static const FieldRegion *findUninitializedField(ProgramStateRef State, MemRegionManager &MRMgr = SVB.getRegionManager(); if (R->getValueType()->isUnionType()) - return nullptr; + return nullptr; if (Ty->isArrayType()) { const auto *First = R; @@ -496,7 +496,8 @@ static const FieldRegion *findUninitializedField(ProgramStateRef State, if (FD->getType()->isStructureType()) { if (const auto *Uninit = findUninitializedField(State, FR)) return Uninit; - } else if (!FD->getType()->isAggregateType() && State->getSVal(FR).isUndef()) + } else if (!FD->getType()->isAggregateType() && + State->getSVal(FR).isUndef()) return FR; } return nullptr; From 35b0142323f19b33609859516fc277764cc974c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Wed, 21 Oct 2026 14:14:31 +0200 Subject: [PATCH 3/7] Reuse `FindUninitializedFields` in CStringChecker --- .../Checkers/CStringChecker.cpp | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index eeab88c2face8a..8458877495f215 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "InterCheckerAPI.h" +#include "UninitializedObject/UninitializedObject.h" #include "clang/AST/OperationKinds.h" #include "clang/Basic/CharInfo.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" @@ -481,26 +482,15 @@ static const FieldRegion *findUninitializedField(ProgramStateRef State, return First == Last ? nullptr : findUninitializedField(State, Last); } - const auto *RT = Ty->getAsStructureType(); - if (!RT) - return nullptr; - const auto *RD = RT->getDecl()->getDefinition(); - if (!RD) + if (!Ty->isStructureType()) return nullptr; - for (const auto *FD : RD->fields()) { - if (!FD->getIdentifier()) - continue; + UninitObjCheckerOptions Opts; + Opts.IsPedantic = true; - const auto *FR = MRMgr.getFieldRegion(FD, R); - if (FD->getType()->isStructureType()) { - if (const auto *Uninit = findUninitializedField(State, FR)) - return Uninit; - } else if (!FD->getType()->isAggregateType() && - State->getSVal(FR).isUndef()) - return FR; - } - return nullptr; + FindUninitializedFields Finder(State, R, Opts); + const auto &UninitFields = Finder.getResults().second; + return UninitFields.empty() ? nullptr : UninitFields.begin()->first; } ProgramStateRef CStringChecker::checkInit(CheckerContext &C, From 0557c82b43787d9512c004d0073f94ac840d26ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Wed, 21 Oct 2026 14:17:07 +0200 Subject: [PATCH 4/7] Loosen the test so it doesn't depend on the internal pointer ordering --- clang/test/Analysis/bstring_UninitRead.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Analysis/bstring_UninitRead.c b/clang/test/Analysis/bstring_UninitRead.c index a330c6b8050c5b..5ba14e1885e3ad 100644 --- a/clang/test/Analysis/bstring_UninitRead.c +++ b/clang/test/Analysis/bstring_UninitRead.c @@ -128,7 +128,7 @@ void mempcpy_struct_fully_uninit() { struct st s1; struct st s2; - mempcpy(&s2, &s1, sizeof(struct st)); // expected-warning{{The 2nd argument contains uninitialized data (e.g., field: 'i')}} + mempcpy(&s2, &s1, sizeof(struct st)); // expected-warning{{The 2nd argument contains uninitialized data}} // expected-note@-1{{Other elements might also be undefined}} } From 9ee5b0e6538075cc461faf18f38e16933b6d18af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Thu, 22 Oct 2026 12:42:19 +0200 Subject: [PATCH 5/7] Handle struct arrays in uninitialized copy checks --- .../Checkers/CStringChecker.cpp | 55 +++++++++++-------- clang/test/Analysis/bstring_UninitRead.c | 37 +++++++++++++ 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 8458877495f215..e31fc6df0aed19 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -507,32 +507,39 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C, ASTContext &Ctx = SVB.getContext(); // Field initialization is meaningful only when the entire source object is - // read. Do not widen partial copies or nonzero offsets to their base object + // read. if (UninitializedRead.isEnabled()) { const auto *BufR = BufVal.getAsRegion(); - const auto *Object = llvm::dyn_cast_or_null<TypedValueRegion>( - BufR ? BufR->StripCasts(false) : nullptr); - - if (Object && !isa<ElementRegion>(Object)) { - QualType Ty = Object->getValueType(); - const llvm::APSInt *Length = SVB.getKnownValue(State, Size); - CharUnits CharSize = Ctx.getTypeSizeInChars( - CK == CharKind::Regular ? Ctx.CharTy : Ctx.WideCharTy); - - if (Ctx.getBaseElementType(Ty)->isStructureType() && - !Ty->isIncompleteType() && !Ty->isVariablyModifiedType() && Length && - Ctx.getTypeSizeInChars(Ty).isMultipleOf(CharSize) && - *Length == Ctx.getTypeSizeInChars(Ty) / CharSize) { - if (const auto *FR = findUninitializedField(State, Object)) { - SmallString<128> Buf; - llvm::raw_svector_ostream OS(Buf); - OS << "The "; - printIdxWithOrdinalSuffix(OS, Buffer.ArgumentIndex + 1); - OS << " argument contains uninitialized data (e.g., field: '" - << FR->getDecl()->getName() << "')"; - emitUninitializedReadBug(C, State, Buffer.Expression, FR, OS.str()); - return nullptr; - } + const auto *Length = SVB.getKnownValue(State, Size); + CharUnits CharSize = Ctx.getTypeSizeInChars( + CK == CharKind::Regular ? Ctx.CharTy : Ctx.WideCharTy); + + const auto IsWholeStructObject = [&](const TypedValueRegion *R) { + if (!R || !Length) + return false; + + QualType Ty = R->getValueType(); + return Ctx.getBaseElementType(Ty)->isStructureType() && + !Ty->isIncompleteType() && !Ty->isVariablyModifiedType() && + Ctx.getTypeSizeInChars(Ty).isMultipleOf(CharSize) && + *Length == Ctx.getTypeSizeInChars(Ty) / CharSize; + }; + + const auto *Object = llvm::dyn_cast_or_null<TypedValueRegion>(BufR); + if (!IsWholeStructObject(Object)) + Object = llvm::dyn_cast_or_null<TypedValueRegion>( + BufR ? BufR->StripCasts(false) : nullptr); + + if (IsWholeStructObject(Object)) { + if (const auto *FR = findUninitializedField(State, Object)) { + SmallString<128> Buf; + llvm::raw_svector_ostream OS(Buf); + OS << "The "; + printIdxWithOrdinalSuffix(OS, Buffer.ArgumentIndex + 1); + OS << " argument contains uninitialized data (e.g., field: '" + << FR->getDecl()->getName() << "')"; + emitUninitializedReadBug(C, State, Buffer.Expression, FR, OS.str()); + return nullptr; } } } diff --git a/clang/test/Analysis/bstring_UninitRead.c b/clang/test/Analysis/bstring_UninitRead.c index 5ba14e1885e3ad..e1533d0b9d0dd1 100644 --- a/clang/test/Analysis/bstring_UninitRead.c +++ b/clang/test/Analysis/bstring_UninitRead.c @@ -132,6 +132,43 @@ void mempcpy_struct_fully_uninit() { // expected-note@-1{{Other elements might also be undefined}} } +void mempcpy_struct_array_first_element_fully_uninit() { + struct st s1[2]; + struct st s2; + + memcpy(&s2, &s1[0], sizeof(s1[0])); // expected-warning{{The 2nd argument contains uninitialized data}} + // expected-note@-1{{Other elements might also be undefined}} +} + +void mempcpy_struct_array_first_element_fully_init() { + struct st s1[2]; + struct st s2; + + s1[0].i = 0; + s1[0].j = 0; + mempcpy(&s2, &s1[0], sizeof(s1[0])); // no-warning +} + +void memcpy_struct_array_second_element_fully_uninit() { + struct st s1[2]; + struct st s2; + + s1[0].i = 0; + s1[0].j = 0; + memcpy(&s2, &s1[1], sizeof(s1[1])); // expected-warning{{The 2nd argument contains uninitialized data}} + // expected-note@-1{{Other elements might also be undefined}} +} + +void mempcpy_struct_array_last_element_fully_uninit() { + struct st s1[2]; + struct st s2[2]; + + s1[0].i = 0; + s1[0].j = 0; + mempcpy(&s2, &s1, sizeof(s1)); // expected-warning{{The 2nd argument contains uninitialized data}} + // expected-note@-1{{Other elements might also be undefined}} +} + // Creduced crash. In this case, an symbolicregion is wrapped in an // elementregion for the src argument. void *ga_copy_strings_from_0; From 57ac0aed37f5a3aa959efc6f3dc9b4575b302649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Thu, 22 Oct 2026 12:52:16 +0200 Subject: [PATCH 6/7] Fix and remove comments --- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index e31fc6df0aed19..1e449dcc292caf 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -324,8 +324,8 @@ class CStringChecker // Re-usable checks ProgramStateRef checkNonNull(CheckerContext &C, ProgramStateRef State, AnyArgExpr Arg, SVal l) const; - // Check whether the origin region behind \p Element (like the actual array - // region \p Element is from) is initialized. + // Check whether the source buffer or its backing array contains + // uninitialized data. ProgramStateRef checkInit(CheckerContext &C, ProgramStateRef state, AnyArgExpr Buffer, SVal Element, SVal Size, CharKind CK) const; @@ -552,8 +552,6 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C, if (!SuperR) return State; - // FIXME: We ought to able to check objects as well. Maybe - // UninitializedObjectChecker could help? if (!SuperR->getValueType()->isArrayType()) return State; From e1f5752b335a1bf3537b25ac3189d6ac385df504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radovan=20Bo=C5=BEi=C4=87?= <[email protected]> Date: Thu, 22 Oct 2026 12:58:20 +0200 Subject: [PATCH 7/7] Use checked arithmetic for CString copy sizes --- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 1e449dcc292caf..57407cf692f640 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -31,6 +31,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/CheckedArithmetic.h" #include "llvm/Support/raw_ostream.h" #include <functional> #include <optional> @@ -513,16 +514,20 @@ ProgramStateRef CStringChecker::checkInit(CheckerContext &C, const auto *Length = SVB.getKnownValue(State, Size); CharUnits CharSize = Ctx.getTypeSizeInChars( CK == CharKind::Regular ? Ctx.CharTy : Ctx.WideCharTy); + const std::optional<int64_t> LengthValue = + Length ? Length->tryExtValue() : std::nullopt; + const std::optional<int64_t> CopySize = + LengthValue ? llvm::checkedMul(*LengthValue, CharSize.getQuantity()) + : std::nullopt; const auto IsWholeStructObject = [&](const TypedValueRegion *R) { - if (!R || !Length) + if (!R || !CopySize) return false; QualType Ty = R->getValueType(); return Ctx.getBaseElementType(Ty)->isStructureType() && !Ty->isIncompleteType() && !Ty->isVariablyModifiedType() && - Ctx.getTypeSizeInChars(Ty).isMultipleOf(CharSize) && - *Length == Ctx.getTypeSizeInChars(Ty) / CharSize; + *CopySize == Ctx.getTypeSizeInChars(Ty).getQuantity(); }; const auto *Object = llvm::dyn_cast_or_null<TypedValueRegion>(BufR); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
