[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] 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] 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 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] 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] 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 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 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] 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] 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] 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 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 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] D72097: [LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner annotations

Charusso added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4619 This attribute may be used by analysis tools and has no effect on code -generation. +generation. A ``void`` argument means that the pointer cna point to any type. `cna` typo

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I have created a `notes.cpp` test-file to test the notes in my checkers, but I think this checker is fine without that test file. @NoQ, what do you think? CHANGES SINCE LAST ACTION http

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso added a comment. In D71433#1784238 , @NoQ wrote: > Currently the check may warn on non-bugs of the following kind: > > void foo() { > char env[] = "NAME=value"; > putenv(env); > doStuff(); > putenv("NAME=anothervalue"); > } >

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

Charusso added a reviewer: vsavchenko. Charusso added a comment. Hey! We are somewhat slow in reviews, please understand that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92634/new/ https://reviews.llvm.org/D92634 ___

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

Charusso added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:60 - If copy to the destination array can overflow [1] and - ``AreSafeFunctionsAvailable`` is set to ``Yes``, ``y`` or non-zero and it is + ``AreSafeFu

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso updated this revision to Diff 319940. Charusso marked 6 inline comments as done. Charusso added a comment. - Fix everything. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/ https://reviews.llvm.org/D69726 Files: clang/docs/analyzer/developer-docs/DebugChecks.rst c

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso added a reviewer: vsavchenko. Charusso added a comment. Hey, I am back. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:41-44 +/// Set the dynamic size of a CXXNewExpr \p NE by its region \p MR. +ProgramStateRef setDynamicSize(ProgramSt

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

Charusso resigned from this revision. Charusso added a comment. @NoQ, what do you think? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97699/new/ https://reviews.llvm.org/D97699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D76622: [analyzer] ConstraintManager - use EXPENSIVE_CHECKS instead of (gcc specific) __OPTIMIZE__ guard

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Nice catch, thanks! We have some FIXMEs about MSVC sadly and I was thinking about the same change back in the days. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso marked 5 inline comments as done. Charusso added a comment. Thanks for the feedback! Given that it will remain an `alpha` checker for a long time (~1 year), no one really should use it. Comment at: clang/docs/analyzer/checkers.rst:1935 + +alpha.security.cert.str.31c +

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso marked 4 inline comments as done. Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:803 + +} // end "cert.str" + balazske wrote: > NoQ wrote: > > `alpha.cert.str`. > This text may be hard to understand. T

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso marked 6 inline comments as done. Charusso added a comment. "To prevent such errors, either limit copies through truncation or, preferably, ensure that the destination is of sufficient size to hold the character data" - from the rule's page. Most of the projects are fine truncating by h

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:25 -/// Get the stored dynamic size for the region \p MR. +/// \returns The stored dynamic size for the region \p MR. DefinedOrUnknownSVal getDynamicSize(ProgramSta

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso updated this revision to Diff 253394. Charusso marked 19 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. Herald added a subscriber: ASDenysPetrov. - Fix `VLASizeChecker`'s multi-dimensional array early return. - So that fix the regression i

[PATCH] D77022: [analyzer] Use IgnoreImpCasts() instead of reimplementing it.

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:512 + return E->IgnoreImpCasts(); } I think it would make sense to remove the helper-function completely. (Being used 2 times.) CHANGES SINCE LAST ACTION

[PATCH] D73521: [analyzer] add-new-checker.py: Introduction

Charusso added a comment. In D73521#1923693 , @NoQ wrote: > Or is each test run updating the repo? Can we simply make the script do `cat > DummyChecker.cpp | sed s/DummyChecker/$NAME/g > $NAME.cpp` and store the > checker only once? It was updating, ye

[PATCH] D73521: [analyzer] add-new-checker.py: Introduction

Charusso updated this revision to Diff 253777. Charusso marked 3 inline comments as done. Charusso added a comment. Herald added a subscriber: ASDenysPetrov. - Remove the test of creating a live checker, instead copy over the live checker when the script runs. - Simplify the script by adding the

[PATCH] D73521: [analyzer] add-new-checker.py: Introduction

Charusso added a comment. In D73521#1951776 , @ASDenysPetrov wrote: > Will this utility affect Visual Studio builds? The community owns like 50 build-bots, so it should affect all of them by default. I am hoping it will just work. CHANGES SINCE LAST

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

Charusso added a comment. Please avoid to stuff in `CheckerContext` because this facility should be used by ExprEngine/Store as well. Let us reword your API: `getDynamicSizeWithOffset(ProgramStateRef, SVal, SValBuilder &)`. Of course we are trying to obtain some buffer-ish size, that is the pur

[PATCH] D77022: [analyzer] Use IgnoreImpCasts() instead of reimplementing it.

Charusso accepted this revision. Charusso marked an inline comment as done. Charusso added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:512 + return E->IgnoreImpCasts(); } --

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso updated this revision to Diff 254051. Charusso added a comment. - Remove the last gymnastic. - Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/ https://reviews.llvm.org/D69726 Files: clang/docs/analyzer/developer-docs/DebugChecks.rst clang/include/clang/Sta

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 254061. Charusso marked 6 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. - Get rid of the secondary behavior for now. - Fix review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso added a comment. Given that the secondary behavior confuse people I have removed it for now. May if someone introduce a NullTerminationChecker then we introduce such option to warn on insecure calls instant. Thanks @balazske for influencing that change. @NoQ this project had a deadline

[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

Charusso added a comment. In D77066#1953280 , @Charusso wrote: > getDynamicSizeWithOffset(State, MR, SVB) { > Offset = State->getStoreManager().getStaticOffset(MR, SVB); > ... > } > Hm, the MemRegion's offset should be great. I was thinking a

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso added a comment. Thanks for the review, hopefully if I ping @NoQ in every round, it will be green-marked soon. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:55 + // they can cause a not null-terminated string. In this case we store the + // strin

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 254423. Charusso added a comment. - Remove the last dead comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/ https://reviews.llvm.org/D70411 Files: clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Chec

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 254420. Charusso marked 4 inline comments as done. Charusso added a comment. - Simplify tests. - Remove dead code, they are far away to being used. - Add an extra test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/ https://reviews.l

[PATCH] D74735: [analyzer] Add support for CXXInheritedCtorInitExpr.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I think on a long term our knowledge increases so we could generalize upon what we have learnt. For now as long as it is working and have tests, it is cool. Comment at:

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

Charusso added a comment. That is very unfortunate, but may if you could introduce a bullet-proof `ls` we could see if the `scan-build` sub-directory removal is non-alphabetical. I think the latter smells more badly. Comment at: clang/test/Analysis/scan-build/rebuild_index/re

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script

Charusso added a comment. Thanks for the reviews! Are we good to go? Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:112 void enableFixitsAsRemarks() { FixitsAsRemarks = true; } + void enableFixItApplication() { ApplyFixIts = true; } al

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script

Charusso updated this revision to Diff 246209. Charusso marked 4 inline comments as done. Charusso retitled this revision from "[analyzer] FixItHint: Apply and test hints with the Clang Tidy's script" to "[analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script". Charusso added a c

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:92 + + bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false, + ApplyFixIts = false; NoQ wr

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Charusso added a comment. I am so sorry to mention, but we need the `AnalysisManager` to pass around which manages the analysis, therefore it knows both the `LangOptions` and `AnalyzerOptions`. Also this entire callback should be removed ideally: it has to be a virtual function defaulting to `r

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Charusso added a comment. PS: The `CheckerManager` also could serve this behavior as `registerXXX()` already passing around that manager, but I believe the `AnalysisManager` supposed to manage the analysis. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Charusso added a comment. In D75271#1895949 , @baloghadamsoftware wrote: > It is impossible to use `CheckerManager` as parameter for > `shouldRegisterXXX()` because `CheckerRegistry` does not have it. If I add > `CheckerManager` to `CheckerRegistry` the

[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

Charusso accepted this revision. Charusso added a comment. > ! In D75271#1896223 , @Szelethus > wrote: > Thinking back, I did have a number of failed attempts to make something a > bit less ugly, but the sharp divide between the 2 libraries makes is > r

[PATCH] D68165: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

Charusso added a comment. I wish for a third map, something like `ReallocationMap`. Other than that it is a great direction, I love it. Thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:355 + template + void checkRealloc(CheckerContext &C, const CallEx

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

Charusso added a comment. Cool! May it worth to mention the corresponding mail from the mailing list in the Summary: http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:941 + CXXDeall

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. > Also... as to why I added so much `LLVM_UNREACHABLE` annotations I believe it is not necessary to add `LLVM_NODISCARD`, but as it perfectly works here, I like it. I would mention the ma

[PATCH] D75431: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

Charusso added a comment. > look for a solution better then demonstrated in this patch. I believe this solution exactly what Artem suggested, so there is nothing to feel bad about. Cool. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75431/new/ ht

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Charusso added a comment. I have added green markers to all of your patches as well. I really appreciate the simplification of the `MallocChecker`. May you would commit it as soon as possible, given that you have nailed what Artem has suggested. Cool^2. Repository: rG LLVM Github Monorepo C

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731 + } + return C.getNoteTag([Text, Name](BugReport &BR) -> std::string { + SmallString<256> Msg; baloghadamsoftware wrote: > Szelethus wrote: > > baloghad

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of te affected container

Charusso added a comment. I believe our path and context sensitive engine is more extensible and precise than checking the source file. Are you sure it scales? I would prefer to tie this information for MemRegions, rather than arbitrary places in the source code. My knowledge is very weak in th

[PATCH] D75514: [Analyzer] Only add container note tags to the operations of te affected container

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Cool, that one a lucky one! I think the SymbolRef based world also working, just at some point it could not scale because other systems are region based... For now, it is a much better sol

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

Charusso added a comment. Could you add a test please? We really need tests for every patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75529/new/ https://reviews.llvm.org/D75529 ___ cfe-commits mai

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso updated this revision to Diff 248097. Charusso marked 4 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. - Set the size properly. - Add new debug.ExprInspection patterns: region, size, element count. - `clang-format -i ExprInspectionChecker.

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:85 +if (CI->getValue().isUnsigned()) + Size = SVB.makeIntVal(CI->getValue(), /*IsUnsigned=*/false); + That one is interes

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script

Charusso marked an inline comment as done. Charusso added a comment. Thanks everyone! I hope the Analyzer developers start to use the wonderful features from Clang-Tidy. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69746/new/ https://reviews.llvm.org/D69746

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script

This revision was automatically updated to reflect the committed changes. Closed by commit rGf69c74db34f4: [analyzer] FixItHint: Apply and test hints with the Clang-Tidy's script (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D69746?vs=246209&id=248103#toc Repository

[PATCH] D73729: [analyzer] AnalyzerOptions: Remove 'fixits-as-remarks'

This revision was automatically updated to reflect the committed changes. Closed by commit rGabdd33c86a34: [analyzer] AnalyzerOptions: Remove 'fixits-as-remarks' (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D73729?vs=241507&id=248106#toc Repository: rG LLVM Githu

[PATCH] D73519: [analyzer] AnalysisDeclContext: Refactor and documentation

This revision was automatically updated to reflect the committed changes. Closed by commit rG7e1a6ca9e89c: [analyzer] AnalysisDeclContext: Refactor and documentation (authored by Charusso). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D73729: [analyzer] AnalyzerOptions: Remove 'fixits-as-remarks'

Charusso added a comment. [Achievement unlocked] 3 green marks. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73729/new/ https://reviews.llvm.org/D73729 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:44 -/// BugReporterVisitors are used to add custom diagnostics along a path. +/// BugReporterVisitors are used to add custom diagnostics along a \emoji bug +//

[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation

Charusso updated this revision to Diff 248110. Charusso marked 4 inline comments as done. Charusso added a comment. Herald added subscribers: martong, steakhal. - Make the tags robust and more unique. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73520/new/ https://reviews.llvm.org/D735

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

Charusso added a comment. Could you mention how to use this feature in the Summary please? cd reports scan-build --generate-index-only . --- And something is not right, I have tried it: Use of uninitialized value $Clang in concatenation (.) or string at /llvm-project/clang/tools/scan-bu

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

Charusso added a comment. In D74467#1905416 , @NoQ wrote: > In D74467#1905011 , @Charusso wrote: > > > Could you mention how to use this feature in the Summary please? > > And something is not right, I have tried i

[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] 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] 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] 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 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 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] 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] 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] 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] 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] D69642: [analyzer] DynamicSize: Simplify the assumption creating of sizes

Charusso added a comment. @NoQ, I hope this is what you have suggested. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69642/new/ https://reviews.llvm.org/D69642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

Charusso accepted this revision. Charusso marked an inline comment as done. Charusso added a comment. This revision is now accepted and ready to land. My concern was the too heavy `Optional` and `bool` usage. Cool patch! Comment at: clang/lib/StaticAnalyzer/Checkers/StreamCheck

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

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 way to store the symbolic size and its

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

Charusso abandoned this revision. Charusso added a comment. I think we do not need to create assumptions. However, there is a tiny difference which continues in D69726 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69642/new/ htt

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso marked an inline comment as done. Charusso added a comment. I do not want to reverse engineer the `MallocChecker` to obtain the size on call basis. The current model without D68725 makes it enough difficult to obtain the size even with this generic map,

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

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: D69726: [analyzer] DynamicSize

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso marked an inline comment as done. Charusso added a comment. It is needed for my work, and also I have seen other checkers in need of that. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:195 ASTContext *Ctx; - const Preprocessor &PP; const std

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

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

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

Charusso added a comment. In D69599#1730707 , @NoQ wrote: > > This is the first step to mitigate that issue. > > What's the issue? Well, after I mentioned an issue I have realized the somewhat path-insensitive `getSizeInElements()` does not touch the (v

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

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:40-48 + if (const llvm::APSInt *SizeInt = SVB.getKnownValue(State, Size)) { +CharUnits RegionSize = CharUnits::fromQuantity(SizeInt->getSExtValue()); + +// If a variable is reinterp

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:29-31 +/// \returns The stored dynamic size expression for the region \p MR. +const Expr *getDynamicSizeExpr(ProgramStateRef State, const MemRegion *MR); + ---

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

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

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso added a comment. In D69731#1730784 , @NoQ wrote: > I'm not sure though - because we somehow survived without this for like 10 > years. Eg. `BugReporterVisitors.cpp`: [...] > I'd love to see some actual use before committing. "Teaser": const

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

Charusso added a comment. Thanks, now it is cool! Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:40-48 + if (const llvm::APSInt *SizeInt = SVB.getKnownValue(State, Size)) { +CharUnits RegionSize = CharUnits::fromQuantity(SizeInt->getSExtValue()); + +// If a

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

Charusso updated this revision to Diff 227543. Charusso marked 4 inline comments as done. Charusso added a comment. - Old division swapped by `evalBinOp`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69599/new/ https://reviews.llvm.org/D69599 Files: clang/include/clang/StaticAnalyze

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso added a comment. In D69731#1730899 , @NoQ wrote: > Clang-Tidy's `PPCallbacks` subsystem looks much more realistic. I wanted to make it available from the `AnalysisManager` so that I can obtain the macro definitions once before the analysis star

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

Charusso updated this revision to Diff 227546. Charusso marked 3 inline comments as done. Charusso added a comment. - Done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69599/new/ https://reviews.llvm.org/D69599 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSiz

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

Charusso added a comment. Thanks for the review! Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:40-48 + if (const llvm::APSInt *SizeInt = SVB.getKnownValue(State, Size)) { +CharUnits RegionSize = CharUnits::fromQuantity(SizeInt->getSExtValue()); + +// If a v

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

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

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso added a comment. I am thinking of a callback which is something like: void checkBeginAnalysis(const Decl *D, BugReporter &BR) const; so it would be easy and meaningful to have a place for the `Preprocessor` logic. Do you think it would worth it? Repository: rC Clang CHANGES SINC

<    1   2   3   4   5   6   7   >