[PATCH] D69642: [analyzer] DynamicSize: Simplify the assumption creating of sizes

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a parent revision: D69599: [analyzer] DynamicSize

[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1548 - state = checkNonNull(C, state, Dst, DstVal); + state = checkNonNull(C, state, Dst, DstVal, 1); if (!state) bruntib wrote: > NoQ wrote: > > You could als

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso added a comment. The [1] patch which introduced such static element-count data has only one test case in `outofbound.c`: void f2() { int *p = malloc(12); p[3] = 4; // expected-warning{{Access out-of-bound array element (buffer overflow)}} } which probably wanted to be `(in

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This patch uses the new `DynamicSize.cpp` to serve dynamic info

[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:158 // symbols to use, only content metadata. - return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR)); + return FTR->getMemRegionManager().getS

[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

Charusso updated this revision to Diff 227013. Charusso marked 2 inline comments as done. Charusso added a comment. - Unpack the issue-solving about code regions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69540/new/ https://reviews.llvm.org/D69540 Files: clang/include/clang/Stati

[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

Charusso added a comment. Thanks for the review! I am not sure why but after your review I always see the most appropriate design immediately. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1253-1255 + MemRegionManager(ASTContext &c, llvm::Bump

[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

Charusso updated this revision to Diff 227006. Charusso marked 12 inline comments as done. Charusso added a comment. - Fix. - Added a `FIXME` about the missing symbol of `BlockDataRegion`, `BlockCodeRegion` and `FunctionCodeRegion`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69540/ne

[PATCH] D68725: [analyzer] MemoryBlockRegion: Generalize AllocaRegion

Charusso added a comment. In D68725#1726332 , @NoQ wrote: > I don't particularly love `SymbolMetadata` because i believe it should be > associated with symbols [...] That is why I secretly wanted to get rid of that concept, but I am fine with using a g

[PATCH] D68725: [analyzer] MemoryBlockRegion: Generalize AllocaRegion

Charusso abandoned this revision. Charusso added a comment. In D68725#1726332 , @NoQ wrote: > But i believe that we already have enough concepts and tools here, there's no > need to introduce more of them. F10581160: image.png

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

Charusso added a reviewer: Charusso. Charusso requested changes to this revision. Charusso added a comment. This revision now requires changes to proceed. Could you please mark resolved issues as resolved? I would like to see what we miss, and what has been done. Comment at: c

[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

Charusso marked 3 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:880 uint64_t Length = UINT64_MAX; - SVal Extent = Top->getExtent(SVB); + SVal Extent = Top->getMemRegionManager().getStaticSize(Top); if (O

[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. This patch introduces a placeholder for representing th

[PATCH] D68725: [analyzer] MemoryBlockRegion: Generalize AllocaRegion

Charusso added a comment. Thanks for the notes! In D68725#1722136 , @NoQ wrote: > Generally, there's no need to add more information to `MemRegion` and > `SymExpr` objects [...] Well, that was my first idea, then I saw we allow helper-methods inside r

[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

Charusso accepted this revision. Charusso marked an inline comment as done. Charusso added a comment. This revision is now accepted and ready to land. I think it will be a great educational material how to touch the core. Good luck! Comment at: clang/lib/StaticAnalyzer/Core/Ca

[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. When you see static bool traverseHiddenNodes( const ExplodedNode *N, llvm::function_ref PreCallback, llvm::function_ref PostCallback, llvm::function_ref Stop) { t

[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:325 if (getKind() != CE_CXXAllocator) if (isArgumentConstructedDirectly(Idx)) if (auto AdjIdx = getAdjustedParameterIndex(Idx)) What about this one? It sm

[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

Charusso added a comment. > i'd much rather enable graph dumping in noassert builds. Totally! When I am not an advanced developer I definitely would like to print the graph with the release pre-built binaries to measure my stuff when I analyze. Like after someone watch your tutorial and starts

[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I was not sure why this is not opaque in case of LLDB usage, but now it is perfect, thanks! Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:3079 +

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

Charusso added a comment. I think everything is working fine on the build-bot side. One tiny issue on Windows left where the `getLength()` could not obtain the length properly: rL374713 , I will look into that. In D45050#1071898

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

Charusso added a comment. In D45050#1707489 , @xbolva00 wrote: > `typedef __SIZE_TYPE__ size_t;` Hm, yes, I have overestimated the uploaded solution: $ clang/test grep -rn 'typedef __SIZE_TYPE__ size_t;' | wc -l 60 $ clang/test grep -rn 'typedef _

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

Charusso added a comment. In D45050#1707483 , @thakis wrote: > This fails on Windows: http://45.33.8.238/win/386/step_7.txt > > Please take a look, and if it's not immediately clear how to fix, b please > revert while you investigate. Oh, thanks! We def

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

This revision was automatically updated to reflect the committed changes. Closed by commit rG82f8f8b44cd0: [clang-tidy] New checker for not null-terminated result caused by strlen()… (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D45050?vs=221096&id=224773#toc Reposi

[PATCH] D68725: [analyzer] MemoryBlockRegion: Generalize AllocaRegion

Charusso created this revision. Charusso added reviewers: NoQ, xazax.hun. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. This patch generalizes the `AllocaRegion` to store met

[PATCH] D67932: [analyzer] Fix accidentally skipping the call during inlined defensive check suppression.

Charusso accepted this revision. Charusso added a comment. > [...] one reason why we can't simply use the error node may be that the > binding to that variable might have already disappeared from the state by the > time the bug is found. Yes, that is pretty interesting. I have found error nodes

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added a comment. Well, the rebase is a little-bit weird, sorry for the inconvenience. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222932. Charusso added a comment. - Rebase properly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h clang/include/clang/

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222931. Charusso added a comment. - Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h clang/include/clang/StaticAna

[PATCH] D68199: [analyzer] DynamicTypeInfo: Simplify the API

Charusso updated this revision to Diff 222927. Charusso edited the summary of this revision. Charusso added a comment. - No comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68199/new/ https://reviews.llvm.org/D68199 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/D

[PATCH] D68199: [analyzer] DynamicTypeInfo: Simplify the API

Charusso marked 3 inline comments as done. Charusso added a comment. Sorry, I was sure it is working as expected. Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:53-57 if (const auto *TR = dyn_cast(MR)) -return DynamicTypeInfo(TR->getLocationType(), /*CanBeSub=

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222685. Charusso marked an inline comment as done. Charusso added a comment. - Use the TU's Decls instead of the gathered casts' Decls. - The math is still missing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 4 inline comments as done. Charusso added inline comments. Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27 + if (isa(a)) +if (isa(a)) + clang_analyzer_warnIfReached(); // no-warning NoQ wrote: > NoQ wrote: >

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 3 inline comments as done. Charusso added inline comments. Comment at: clang/test/Analysis/cast-value-hierarchy-fp-suppression.cpp:25-27 + if (isa(a)) +if (isa(a)) + clang_analyzer_warnIfReached(); // no-warning NoQ wrote: > Charusso wro

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 2 inline comments as done. Charusso added a comment. In D67079#1688648 , @NoQ wrote: > In D67079#1687577 , @Charusso wrote: > > > - `CastVisitor` is the new facility which decides whether the assumpt

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 222370. Charusso marked 2 inline comments as done. Charusso added a comment. - When we have no information whether the cast succeeds or not we assume both. - `CastVisitor` is the new facility which decides whether the assumptions are appropriate. - The math

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:44 + +bool isDerivedFrom(QualType X, QualType Y) { + const CXXRecordDecl *XRD = X->getPointeeCXXRecordDecl(); Szelethus wrote: > Hmm, I think this function answers the ques

[PATCH] D68199: [analyzer] DynamicTypeInfo: Simplify the API

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso retitled this revision from "[analyzer] DynamicTypeInfo

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

Charusso marked 3 inline comments as done. Charusso added a comment. Thanks! Comment at: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:22-33 +constexpr llvm::StringLiteral FuncExprName = "entire-called-function-expr"; +constexpr llvm::StringLiteral Cas

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

Charusso updated this revision to Diff 221096. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45050/new/ https://reviews.llvm.org/D45050 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLi

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126 + + // If the casts have a common anchestor it could not be a succeeded downcast. + for (const auto &PreviousBase : PreviousRD->bases()) NoQ wrote: > Charusso

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 221070. Charusso marked 9 inline comments as done. Charusso added a comment. - Try to do the math. - Create a consistent dynamic type API. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/incl

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

Charusso added a comment. Hm, I wanted to upload the new patch here to see the changes with the diff-mode, but it does not work, sorry. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45050/new/ https://reviews.llvm.org/D45050

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

Charusso updated this revision to Diff 220215. Charusso edited the summary of this revision. Charusso removed reviewers: hokein, ilya-biryukov, xbolva00, dyung. Charusso set the repository for this revision to rCTE Clang Tools Extra. Charusso added a comment. Herald added a project: clang. After a

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeInfo.h:24 + DynamicTypeInfo(QualType ty, bool CanBeSub = true) + : Ty(ty), CanBeASubClass(CanBeSub) {} NoQ wrote: > `Ty(Ty)` is the idiom here. G

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso updated this revision to Diff 219391. Charusso marked 5 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67079/new/ https://reviews.llvm.org/D67079 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCast

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:211 }, - /*IsPrunable=*/true); + /*IsPrunable=*/!CastCtx.CastSucceeds); } That could be helpful, but I

[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. The key is `CXXRecordDecl::isDerivedFrom()`. Repository: rC

[PATCH] D67019: [analyzer] pr43179: CallDescription: Check number of parameters as well as number of arguments.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I like the isolation of this kind of stuff. Thanks you! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67019/new/ https://reviews.llvm.org/D67019 ___

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso added a comment. In D66721#1644531 , @NoQ wrote: > Now you're silencing the *whole* checker. This is equivalent to passing an > `-analyzer-silence-checker` flag. I am silencing the *whole* checker if and only if a bitwise operation or a shift

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso updated this revision to Diff 217062. Charusso marked 4 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66721/new/ https://reviews.llvm.org/D66721 Files: clang/include/clang

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:305 +"Whether the bitwise (and shift) operations should be checked.", +false) + NoQ wrote: > We didn't do enough debugging to

[PATCH] D66721: [analyzer] Analysis: Prevent bitwise operation false positives

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This patch introduces a new `analyzer-config` configuration: `-

[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

Charusso added a comment. In D66593#1642253 , @Charusso wrote: > Wow, it is great you could address every of the edge cases. Thanks you so > much! I believe only one problem left: we need to `return false` instead of > plain `return` in order to let the

[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

Charusso abandoned this revision. Charusso marked 4 inline comments as done. Charusso added a comment. Wow, it is great you could address every of the edge cases. Thanks you so much! I believe only one problem left: we need to `return false` instead of plain `return` in order to let the Analyzer

[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:743-750 + QualType RegionType = DynType.getType(); + if (RegionType->isPointerType()) +RegionType = RegionType->getPointeeType(); + else +RegionType = RegionType.getNonReferenceType

[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

Charusso updated this revision to Diff 216737. Charusso marked 2 inline comments as done. Charusso added a comment. - The null `MemRegion` is a `0 (Loc)`, try to avoid to store it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66593/new/ https://reviews.llvm.org/D66593 Files: clang/l

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso added a comment. In overall I wanted to keep the [A, B] shape of the tests, but now they are more precise, thanks! Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:503 + + if (!BinaryOperator::isBitwiseOrShiftOp(SIE->getOpcode())) +return Stat

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

Charusso updated this revision to Diff 216731. Charusso marked 7 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65239/new/ https://reviews.llvm.org/D65239 Files: clang/include/clang

[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

Charusso marked an inline comment as done. Charusso added a comment. I am not sure how would I fix them, so I just commented them out. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:102 + else if (Ty->isReferenceType()) Ty = Ty.getNonReferenceType();

[PATCH] D66593: [analyzer] CastValueChecker: Fix some assertions

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. - Repository: rC Clang https://reviews.llvm.org/D66593 Fi

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

This revision was automatically updated to reflect the committed changes. Closed by commit rL369615: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull() (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

Charusso added a comment. Thanks for the review! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66423/new/ https://reviews.llvm.org/D66423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

Charusso updated this revision to Diff 216541. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66423/new/ https://reviews.llvm.org/D66423 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp clang/test/Analysis/Inputs/llvm.h clang/test/Anal

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso added a comment. return C.getNoteTag( - [=] { + [=]() -> std::string { SmallString<128> Msg; That was the fix by rL369609 . Somehow it converted to a temporary object therefore that was an issue: [175/176] Running the C

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso added a comment. In D66325#1640483 , @thakis wrote: > Thanks! Looks like it builds fine now, but the tests are failing: > http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19297/steps/test-check-all/logs/stdio > Fai

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso added a comment. In D66325#1640426 , @thakis wrote: > It looks like this renamed DynamicTypeMap.h but didn't update all users, see > all the files in Checkers at > http://llvm-cs.pcc.me.uk/?q=include.*dynamictypemap.h for example. > > Which targ

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

This revision was automatically updated to reflect the committed changes. Charusso marked an inline comment as done. Closed by commit rL369605: [analyzer] CastValueChecker: Store the dynamic types and casts (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscri

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso marked 6 inline comments as done. Charusso added a comment. Thanks for the review! The build-bots will fire with that `QualType` fix (1028 TU on its own). I will look into the exploded-graph-rewriter.py after GSoC to fix every stuff like that patch and also invoke my HTML simplification

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

This revision was automatically updated to reflect the committed changes. Closed by commit rL369604: [analyzer] TrackConstraintBRVisitor: Do not track unknown values (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to comm

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

Charusso added a comment. Thanks for the reviews! I hope that mentioned error will be visible by the `BugReporter` revisions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66267/new/ https://reviews.llvm.org/D66267 ___ cfe-commits mailing

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:83-93 +static QualType getRecordType(QualType Ty) { + Ty = Ty.getCanonicalType(); + + if (Ty->isPointerType()) +return getRecordType

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216513. Charusso added a comment. - Fix printing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 Files: clang/include/clang/AST/Type.h clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerConte

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216509. Charusso marked 4 inline comments as done and an inline comment as not done. Charusso added a comment. - Fix CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 Files: clang/include/clang/AST/Type.h

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

Charusso added a comment. Thanks for the reviews so far! I had a contradiction in my mind about regions, but now everything is okay and the notes are not misleading. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:183 + if (Body) +DRE = dyn_cast(Body);

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

Charusso updated this revision to Diff 216322. Charusso marked 7 inline comments as done. Charusso added a comment. - Simplify the template argument obtaining. - Added a tiny new test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66423/new/ https://reviews.llvm.org/D66423 Files:

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216317. Charusso added a comment. - Make the check-clang pass and simplify a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 Files: clang/include/clang/AST/Type.h clang/include/clang/StaticAn

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216314. Charusso added a comment. - Remove `CastFromTy` finally from the `getNoteTag()` API as it was uninformative. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 Files: clang/include/clang/AST/Type.h

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/test/Analysis/cast-value-notes.cpp:34 + if (dyn_cast_or_null(C)) { +// no-note: 'Assuming 'C' is a 'Circle', not a 'Circle'' +return; NoQ wrote: > A circle is al

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicCastInfo.h:19 +public: + enum CastKind { Success, Fail }; + NoQ wrote: > I suggest `enum CastResult { Success, Failure }` ("a fail" is slang, also > "resul

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216312. Charusso marked 3 inline comments as done. Charusso added a comment. - Fix more and publish the previously forgotten comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 Files: clang/include/

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216250. Charusso added a comment. - Added a new test case and refactored that test file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 Files: clang/include/clang/AST/Type.h clang/include/clang/StaticA

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216246. Charusso marked 15 inline comments as done. Charusso added a comment. Herald added a subscriber: mgorny. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 Files: clang/include/clang/AST/Type.

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

Charusso marked 9 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:216 + const NoteTag *Tag = C.getNoteTag( + [=](BugReport &) -> std::string { +SmallString<128> Msg; NoQ wro

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso added a comment. In D66325#1636091 , @NoQ wrote: > Yay! I understand the rough idea and it looks perfectly reasonable to start > with. Please add FIXMEs/TODOs on how we eventually want to support more > complicated inheritance hierarchies. I h

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 216023. Charusso edited the summary of this revision. Charusso added a comment. - Use a set factory to store a dynamic cast information set per memory region. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32 +/// A set of dynamic cast informations. +REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo) NoQ wrote: > Charusso w

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h:32 +/// A set of dynamic cast informations. +REGISTER_SET_WITH_PROGRAMSTATE(DynamicCastSet, clang::ento::DynamicCastInfo) NoQ wrote: > Emm, so yo

[PATCH] D66423: [analyzer] CastValueChecker: Model isa(), isa_and_nonnull()

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a parent revision: D66325: [analyzer] CastValueCh

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types and casts

Charusso updated this revision to Diff 215893. Charusso retitled this revision from "[analyzer] CastValueChecker: Store the dynamic types in DynamicTypeMap" to "[analyzer] CastValueChecker: Store the dynamic types and casts". Charusso added a comment. This patch introduces `DynamicCastInfo` simi

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

Charusso updated this revision to Diff 215525. Charusso marked an inline comment as done. Charusso added a comment. - Rebased. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66267/new/ https://reviews.llvm.org/D66267 Files: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp clang

[PATCH] D66042: [analyzer] Analysis: Silence checkers

This revision was automatically updated to reflect the committed changes. Closed by commit rL369078: [analyzer] Analysis: Silence checkers (authored by Charusso, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso added a comment. Thanks for the reviews! Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::ve

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso updated this revision to Diff 215518. Charusso marked 10 inline comments as done. Charusso added a comment. - Rebased. - Added the remaining FIXME. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66042/new/ https://reviews.llvm.org/D66042 Files: clang-tools-extra/clang-tidy/Cl

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types in DynamicTypeMap

Charusso added a comment. @xazax.hun It is somehow performance critical code as we have too many casts in the LLVM. I would really appreciate it if you could review it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66325/new/ https://reviews.llvm.org/D66325 _

[PATCH] D66325: [analyzer] CastValueChecker: Store the dynamic types in DynamicTypeMap

Charusso created this revision. Charusso added reviewers: NoQ, xazax.hun. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. Charusso added a comment. Charusso added a parent revis

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

Charusso marked an inline comment as done. Charusso added a comment. @Szelethus pointed out well. My patch is about nullability, and it is perfect. The bug report you made is about deduplication rather than nullability. The fact is we fail to call the deduplication function: `eventsDescribeSame

[PATCH] D66267: [analyzer] TrackConstraintBRVisitor: Do not track unknown values

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a parent revision: D65889: [analyzer] CastValueCh

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimental=*/true); +std::vector Packages = +

[PATCH] D66042: [analyzer] Analysis: Silence checkers

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:483-504 + if (!AnOpts.RawSilencedCheckersAndPackages.empty()) { +std::vector Checkers = +AnOpts.getRegisteredCheckers(/*IncludeExperimenta

<    1   2   3   4   5   6   7   >