[PATCH] D76996: [analyzer] Improve PlacementNewChecker

NoQ added a comment. https://bugs.llvm.org/show_bug.cgi?id=46266! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG7c3768495e8c: [analyzer] Improve PlacementNewChecker (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.or

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong added a comment. Thanks, just committed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat added a comment. In D76996#2017572 , @martong wrote: > > ... This draws a pattern that we should recursively descend down to the top > > most base region. I.e. the different check*RegionAlign methods should call > > into each other until we reach

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong accepted this revision. martong added a comment. > ... This draws a pattern that we should recursively descend down to the top > most base region. I.e. the different check*RegionAlign methods should call > into each other until we reach the top level base region. > The observation her

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 259667. f00kat added a comment. 1. Build fix CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 Files: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp clang/test/Analysis/placement-new.cpp Index: cl

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 259636. f00kat marked 4 inline comments as done. f00kat added a comment. 1. Refactoring 2. Build fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 Files: clang/lib/S

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:261 + + if (const VarRegion *TheVarRegion = BaseRegion->getAs()) { +const VarDecl *TheVarDecl = TheVarRegion->getDecl(); Perhaps you could call instead `checkV

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 259499. f00kat added a comment. 1. Rewroted ElementRegion processing and fixed tests for this cases. 2. Simplified the code a bit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.or

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:125 +Msg = std::string(llvm::formatv( +"Possibly not enough {0} bytes for array allocation which " +"requires " f00kat wrote: > martong

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189 + if (const MemRegion *MRegion = PlaceVal.getAsRegion()) { +if (const ElementRegion *TheElementRegion = +MRegion->getAs()) { f00kat wrote: > f0

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat marked 4 inline comments as done. f00kat added a comment. Ping? :) Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189 + if (const MemRegion *MRegion = PlaceVal.getAsRegion()) { +if (const ElementRegion *TheElementRegion = +MRegion->g

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189 + if (const MemRegion *MRegion = PlaceVal.getAsRegion()) { +if (const ElementRegion *TheElementRegion = +MRegion->getAs()) { NoQ wrote: > The seque

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:189 + if (const MemRegion *MRegion = PlaceVal.getAsRegion()) { +if (const ElementRegion *TheElementRegion = +MRegion->getAs()) { The sequence of `Field

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 255039. f00kat added a comment. 1. Maybe build fix 2. Added tests for nested arrays of structures 3. Fixed bugs in implementation for ElementRegion cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 Files:

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Thanks! But I am not that confident with the element regions and field regions, so @NoQ could you please take another look? Comment at: clang/lib/StaticAnalyzer/Chec

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 253803. f00kat marked an inline comment as done. f00kat added a comment. Fixed comments in tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 Files: clang/lib/Stati

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong added inline comments. Comment at: clang/test/Analysis/placement-new.cpp:265 + + // bad 2(custom align) + 1(index '2' offset) + ::new (&Xi.b[1]) long; // expected-warning{{Storage type is aligned to 3 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 253642. f00kat marked an inline comment as done. f00kat added a comment. test fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 Files: clang/lib/StaticAnalyzer/Check

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 253628. f00kat marked 3 inline comments as done. f00kat added a comment. 1. Removed code and tests for ConcreteInt cases 2. Fixed FieldRegion check 3. Added handling for ElementRegion cases such as void f7() { short b[10]; // ok. 2(short align) +

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

martong added a comment. Wohoow! I am impressed, this is really nice work, I like it! :) Could not find any glitch, looks good from my side once you address NoQ's concerns. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:125 +Msg = std::string(llvm:

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

NoQ added a comment. That was fast! Looks alright. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:25 public: void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const; Before i forget: Ideally @martong should have subscribed to

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat updated this revision to Diff 253408. f00kat added a comment. lint fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76996/new/ https://reviews.llvm.org/D76996 Files: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp clang/test/

[PATCH] D76996: [analyzer] Improve PlacementNewChecker

f00kat created this revision. f00kat added reviewers: aaron.ballman, lebedev.ri, NoQ, martong. f00kat added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.