[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

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

Charusso added a comment. Thank you guys for investigating it! In D69726#2671734 , @vsavchenko wrote: > 2. The analyzer doesn't explain why it thinks that `guc_malloc` returns null > pointer. I find it alarming that it might assume it for all the wrong

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

Charusso added a comment. In D69726#2671526 , @vsavchenko wrote: > @Charusso > It looks like this patch introduced a some weird false positive on PostgreSQL > F16161734: report-guc.c-ParseLongOption-13-1.html > >

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

Charusso added a comment. Cool, thanks! Debug facility NFC: https://reviews.llvm.org/rG89d210fe1a7a1c6cbf926df0595b6f107bc491d5 `size` -> `extent` conversion: https://reviews.llvm.org/rG9b3df78b4c2ab7a7063e532165492e1ffa38d401 Comment at:

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

This revision was automatically updated to reflect the committed changes. Charusso marked an inline comment as done. Closed by commit rGdf64f471d1e2: [analyzer] DynamicSize: Store the dynamic size (authored by Charusso). Changed prior to commit:

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

Charusso added a comment. In D97699#2601804 , @NoQ wrote: > Why are you even tracking `reg_$0`? It's obvious from the structure of > the symbol that it's an environment pointer, there's no need to keep it in > maps. `envp` is confusable with `argv`:

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Great idea! Comment at: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist:11413 - categoryDead store + categoryUnused code typeDead initialization

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

Charusso added a comment. @NoQ, could you upstream it and move this idea forward please? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/ https://reviews.llvm.org/D69726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[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

[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

[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 +

[PATCH] D70411: [analyzer] CERT STR rule checkers: STR31-C

Charusso added a comment. Thanks for the reviews! In D70411#2153562 , @Szelethus wrote: > Please do not bypass the previous comments that hadn't reached a conclusion > -- littering inlines about miscellaneous stuff at this stage does more harm > then

[PATCH] D70411: [analyzer] CERT STR rule checkers: STR31-C

Charusso updated this revision to Diff 278368. Charusso marked 18 inline comments as done. Charusso edited the summary of this revision. Charusso added a comment. - Resolve most of the review comments. - We really need to specify the design of future checkers. CHANGES SINCE LAST ACTION

[PATCH] D71155: [analyzer] CERT STR rule checkers: STR30-C

Charusso added a comment. In D71155#1854908 , @NoQ wrote: > Let's separate `CStringChecker` improvements into a separate patch and have a > separate set of tests for it. I was thinking about creating tests, but I cannot figure out any better testing

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso abandoned this revision. Charusso added a comment. Herald added subscribers: ASDenysPetrov, martong, steakhal. Way more sophisticated matching: https://reviews.llvm.org/D77745 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70805/new/ https://reviews.llvm.org/D70805

[PATCH] D71155: [analyzer] CERT STR rule checkers: STR30-C

Charusso updated this revision to Diff 265963. Charusso retitled this revision from "[analyzer] CERT: STR30-C" to "[analyzer] CERT STR rule checkers: STR30-C". Charusso added a comment. Herald added subscribers: ASDenysPetrov, martong, steakhal. - Refactor. CHANGES SINCE LAST ACTION

[PATCH] D71033: [analyzer] CERT STR rule checkers: STR32-C

Charusso updated this revision to Diff 265962. Charusso retitled this revision from "[analyzer] CERT: STR32-C" to "[analyzer] CERT STR rule checkers: STR32-C". Charusso added a comment. Herald added subscribers: ASDenysPetrov, martong, steakhal. - Refactor. - State out explicitly whether the

[PATCH] D70411: [analyzer] CERT STR rule checkers: STR31-C

Charusso updated this revision to Diff 265959. Charusso retitled this revision from "[analyzer] CERT: STR31-C" to "[analyzer] CERT STR rule checkers: STR31-C". Charusso added a comment. - Refactor. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/

[PATCH] D76768: [analyzer] Added support of scan-build and exploded-graph-rewriter regression tests for Windows

Charusso added a comment. I believe it is very strange on a Windows system to have multiple dots in a file. The other issue could be the wildcard `/*/` in a path full of `\`s. The LLVM `lit` (https://llvm.org/docs/CommandGuide/lit.html) has tons of Windows-related shortcuts, which I have never

[PATCH] D76768: [analyzer] Added support of scan-build and exploded-graph-rewriter regression tests for Windows

Charusso added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76768/new/ https://reviews.llvm.org/D76768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

Charusso added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:33-50 +void SignalInMultithreadedProgramCheck::registerMatchers(MatchFinder *Finder) { + auto signalCall = + callExpr( +

[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/

[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

[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 + //

[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

[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

[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] 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

[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] 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

[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] 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#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,

[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] 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

[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

[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

[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.

[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] 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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

Charusso added a comment. In D76229#1925363 , @f00kat wrote: > In D76229#1925268 , @aaron.ballman > wrote: > > > Have you considered making this a static analyzer check as opposed to a > > clang-tidy check? I

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

Charusso added a comment. In D75529#1920796 , @vabridgers wrote: > I believe all comments have been addressed. Please let me know if there's > anything else required. Thanks I think you have solved everything. Do you have commit access? May you would

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

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:24 def CoreBuiltin : Package<"builtin">, ParentPackage, Hidden; -def CoreUninitialized : Package<"uninitialized">, ParentPackage; +def

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

Charusso updated this revision to Diff 250020. Charusso marked 3 inline comments as done. Charusso retitled this revision from "[analyzer][WIP] add-new-checker.py: Introduction" to "[analyzer] add-new-checker.py: Introduction". Charusso added a comment. Herald added a subscriber: mgorny. - Try

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

Charusso accepted this revision. Charusso added a comment. In D75529#1912849 , @vabridgers wrote: > @Charusso, I cannot find a way to create a test for this change, since this > was found using an architecture that supports 16-bit chars. I really felt

[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

[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

[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/

[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] 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

[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'

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=248106#toc Repository: rG LLVM Github

[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-Tidys script (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D69746?vs=246209=248103#toc Repository:

[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

[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

[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

[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

[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

[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 ) -> std::string { + SmallString<256> Msg; baloghadamsoftware wrote: > Szelethus wrote: > >

[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

[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

[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/

[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 +

[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 , const

[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 >

[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`

[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

[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

[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

[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

[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; }

[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:

[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 accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Thanks! I would mention in the Summary the necessary flags to perform index-only output. (May write some release notes?) That option sounds very strange, but I like it. For example to run

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

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

[PATCH] D73966: [analyzer][WIP] Add 10.0.0 release notes.

Charusso added inline comments. Comment at: clang/docs/ReleaseNotes.rst:408 + +- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage + provided for default placement new is sufficiently large. balazske wrote: > Szelethus wrote: > > NoQ

[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Cool, thanks you! Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:612 + return !(Range.getBegin().isMacroID() || Range.getEnd().isMacroID()); +} +

[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:925 HighlightRange(R, LPosInfo.first, Range); - } } Here the gray highlighting goes, so the `PopUpRanges` store whether we have already highlighted the range

[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

Charusso added a comment. Thanks for the fix! The `PopUpRanges` is very important, please revert it back in its original shape. Sorry for the inconvenience. I have ran a quick scan-build with this patch on LLVM because I wanted to give you a real world example (which you cannot visibly see at

[PATCH] D71155: [analyzer] CERT: STR30-C

Charusso added a comment. Thanks! I have felt that may I create a too complex symbolic value. Sorry for stealing your time. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2097-2098 +void CStringChecker::evalCharPtrCommon(CheckerContext , +

[PATCH] D71155: [analyzer] CERT: STR30-C

Charusso updated this revision to Diff 242159. Charusso marked 8 inline comments as done. Charusso added a comment. - Remove the modeling from CStringChecker. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71155/new/ https://reviews.llvm.org/D71155 Files:

[PATCH] D71033: [analyzer] CERT: STR32-C

Charusso updated this revision to Diff 241761. Charusso added a comment. - Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71033/new/ https://reviews.llvm.org/D71033 Files: clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso updated this revision to Diff 241758. Charusso marked an inline comment as done. Charusso added a comment. - Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70805/new/ https://reviews.llvm.org/D70805 Files:

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h:55 + bool VisitSymbolRegionValue(const SymbolRegionValue *S) { +return Visit(S->getRegion()); + }

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

Charusso updated this revision to Diff 241752. Charusso edited the summary of this revision. Charusso added a comment. - 2020-ify the checker writing - Remove extra bullet-points of CERT checker documentation: we only need the checker's documentation, not the packages. CHANGES SINCE LAST

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

Charusso updated this revision to Diff 241749. Charusso edited the summary of this revision. Charusso added a comment. - Let us reuse this patch. - Remove the expression storing feature. - Only store known sizes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/

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

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: D69746: [analyzer] FixItHint:

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

Charusso updated this revision to Diff 241492. Charusso marked 8 inline comments as done. Charusso added a comment. - Change to 4-column space standard. - Simplify obtaining the Clang include directory. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69746/new/

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

Charusso updated this revision to Diff 241493. Charusso edited the summary of this revision. Charusso added a comment. - Rename the script from `check_analyzer_fixit.py` to `check-analyzer-fixit.py` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69746/new/

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

Charusso added a comment. Thanks for the reviews! Sorry for the delay. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69746/new/ https://reviews.llvm.org/D69746 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:92 + + bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false, + ApplyFixIts = false; NoQ wrote: > I'll be perfectly happy with

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG38ab3b876baa: [analyzer] CheckerContext: Make the Preprocessor available (authored by Charusso). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69731/new/

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

Charusso abandoned this revision. Charusso marked an inline comment as done. Charusso added a comment. Let us say, we do not need this patch. In case we would need it, I will reopen. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/ https://reviews.llvm.org/D69726

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGaf3d0d16286a: [analyzer] DynamicSize: Remove getSizeInElements() from store (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D69599?vs=227546=241460#toc Repository: rG LLVM

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG601687bf731a: [analyzer] DynamicSize: Remove getExtent() from regions (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D69540?vs=227013=241447#toc Repository: rG LLVM Github

[PATCH] D73629: [analyzer] vfork checker: allow execve after vfork

Charusso added a reviewer: NoQ. Charusso added a comment. Hey, thanks! The patch looks great, but please note that we do the reviews with context using `git diff -U99` or uploading with `arc` (https://secure.phabricator.com/book/phabricator/article/arcanist/). Repository: rG LLVM Github

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

Charusso marked an inline comment as done. Charusso added a comment. This script dumps out a dummy checker which is the continuation of the Clang Tidy's https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/add_new_check.py However, we could stick to the

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

Charusso updated this revision to Diff 240759. Charusso added a comment. - Make it runable. Whoops. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73521/new/ https://reviews.llvm.org/D73521 Files: clang/test/Analysis/add-new-checker/add-main-package.rst

  1   2   3   4   5   6   7   >