llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: [ Taha. Dostifam ] (tahadostifam) <details> <summary>Changes</summary> # Added Abstract VAList region normalization into MemRegionManager Component: `clang` `static analyzer` The Clang Static Analyzer's `VAListChecker` currently contains **checker-local logic** to normalize va_list memory regions (handling ElementRegion wrappers). This logic is marked with: > TODO: In the future this should be abstracted away by the analyzer. At [VAListChecker.cpp](https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp#L179) I implemented a small abstraction in MemRegionManager and adjusted VAListChecker to use it. The change compiles and basic analyzer runs with --analyze work as expected. ## Changes - New helper in MemRegionManager In `clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h` ```cpp /// Strips ElementRegion wrappers from VA lists /// Returning canonical base region. MemRegion *getVAListRegion(const MemRegion *Reg); ``` In `clang/lib/StaticAnalyzer/Core/MemRegion.cpp`: ```cpp MemRegion *MemRegionManager::getVAListRegion(const MemRegion *Reg) { if (!Reg) return nullptr; if (const ElementRegion *EReg = dyn_cast<ElementRegion>(Reg)) { return getVAListRegion(EReg->getSuperRegion()); } if (const DeclRegion *DeclReg = dyn_cast<DeclRegion>(Reg)) { if (isa<ParmVarDecl>(DeclReg->getDecl())) { return const_cast<MemRegion*>(Reg); } } return const_cast<MemRegion*>(Reg); } ``` - And now VAListChecker uses engine helper method In clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp: ```cpp const MemRegion *VAListChecker::getVAListAsRegion(SVal SV, const Expr *E, CheckerContext &C) const { const MemRegion *Reg = SV.getAsRegion(); if (!Reg) return nullptr; MemRegionManager &MRMgr = Reg->getMemRegionManager(); return MRMgr.getVAListRegion(Reg); } ``` The previous checker-local logic that inspected `CastExpr`, `ParmVarDecl`, and `ElementRegion` has been removed. ## Small test cases I used a small `va_list`-based example to test the new getVAListRegion logic: ```cpp #include <stdarg.h> void test_array_model(va_list ap) { char *p = (char *)ap; *p = 0; } void foo(va_list args) { char *p = (char *)args; *p = 0; } void test_nested(va_list ap) { char (*arr)[1] = (char (*)[1])ap; (*arr)[0] = 0; } int main() { return 0; } ``` Built and analyzed with my patched Clang: ```bash ./bin/clang --analyze va_list_test.c ``` The analyzer runs successfully on this test, and the VAList-related code paths work without crashes or unexpected diagnostics. --- Full diff: https://github.com/llvm/llvm-project/pull/181948.diff 3 Files Affected: - (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (+4) - (modified) clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp (+3-14) - (modified) clang/lib/StaticAnalyzer/Core/MemRegion.cpp (+16) ``````````diff diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h index c59413abc352d..925f75d65dac7 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -1604,6 +1604,10 @@ class MemRegionManager { getCXXDerivedObjectRegion(const CXXRecordDecl *BaseClass, const SubRegion *Super); + /// Strips ElementRegion wrappers from VA lists + /// Returning canonical base region. + MemRegion *getVAListRegion(const MemRegion *Reg); + const FunctionCodeRegion *getFunctionCodeRegion(const NamedDecl *FD); const BlockCodeRegion *getBlockCodeRegion(const BlockDecl *BD, CanQualType locTy, diff --git a/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp index 503fa5de868f2..321ba2d21059e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp @@ -176,20 +176,9 @@ const MemRegion *VAListChecker::getVAListAsRegion(SVal SV, const Expr *E, const MemRegion *Reg = SV.getAsRegion(); if (!Reg) return nullptr; - // TODO: In the future this should be abstracted away by the analyzer. - bool VAListModelledAsArray = false; - if (const auto *Cast = dyn_cast<CastExpr>(E)) { - QualType Ty = Cast->getType(); - VAListModelledAsArray = - Ty->isPointerType() && Ty->getPointeeType()->isRecordType(); - } - if (const auto *DeclReg = Reg->getAs<DeclRegion>()) { - if (isa<ParmVarDecl>(DeclReg->getDecl())) - Reg = C.getState()->getSVal(SV.castAs<Loc>()).getAsRegion(); - } - // Some VarRegion based VA lists reach here as ElementRegions. - const auto *EReg = dyn_cast_or_null<ElementRegion>(Reg); - return (EReg && VAListModelledAsArray) ? EReg->getSuperRegion() : Reg; + + MemRegionManager &MRMgr = Reg->getMemRegionManager(); + return MRMgr.getVAListRegion(Reg); } void VAListChecker::checkPreStmt(const VAArgExpr *VAA, diff --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp index f20e79ae675a4..7c1dcccd9fec7 100644 --- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -1888,3 +1888,19 @@ bool RegionAndSymbolInvalidationTraits::hasTrait(const MemRegion *MR, return false; } + +MemRegion *MemRegionManager::getVAListRegion(const MemRegion *Reg) { + if (!Reg) return nullptr; + + if (const ElementRegion *EReg = dyn_cast<ElementRegion>(Reg)) { + return getVAListRegion(EReg->getSuperRegion()); + } + + if (const DeclRegion *DeclReg = dyn_cast<DeclRegion>(Reg)) { + if (isa<ParmVarDecl>(DeclReg->getDecl())) { + return const_cast<MemRegion*>(Reg); + } + } + + return const_cast<MemRegion*>(Reg); +} \ No newline at end of file `````````` </details> https://github.com/llvm/llvm-project/pull/181948 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
