[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-14 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Awesome detective work! I glanced over the code, it looks great. I'd love to dedicate more time to your liveness-related patches, but university is a thing, so finding typos and the like

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2019-01-15 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Ping^2 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51866/new/ https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56632: [analyzer] Track region liveness only through base regions.

2019-01-16 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/symbol-reaper.cpp:31 + clang_analyzer_eval(glob); // expected-warning{{TRUE}} + // expected-warning@-1{{SYMBOL DEAD}} +} NoQ wrote: > Szelethus wrote: > > N00b question: What

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2019-01-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. Overall I think this looks great, thanks! I left some inlines that would be nice to fix before commiting, but all of them are minor nits. Would it be possible for you to commit the clang-formatting and the actual logic separately?

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2019-01-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/security-syntax-checks.m:253 + FILE *file; + sprintf(buf, "a"); // expected-warning{{Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2019-01-17 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. In D35068#811436 , @NoQ wrote: > I wonder how noisy this check is - did you test it on large codebases? > Because these functions are po

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Ping^3. @NoQ @george.karpenkov Could you please take a look? @xazax.hun Any other objections? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51866/new/ https://reviews.llvm.org/D51866 ___ cfe-commits mailing list

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I'm working on rebasing my dependency branches on top of trunk, and I'm somewhat stuck on this patch, could you lend a hand please? So, I'm seeing in essence 3 checkers: the class `RetainCountChecker`, that does the modeling, and two checkers that are exposed to the u

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D55424#1326397 , @NoQ wrote: > Should we also pass `CheckerManager` into `shouldRegister...`? Or is it > entirely useless? I wouldn't say useless, but I'm struggling to come up with an example where registering would depen

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 182721. Szelethus added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55424/new/ https://reviews.llvm.org/D55424 Files: include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h include/clang/StaticAnalyzer/Core/Ch

[PATCH] D56988: [analyzer][NFC] Supply CheckerRegistry with AnalyzerOptions

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, baloghadamsoftware, xazax.hun, rnkovacs. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Title says it all. Repository: rC Clang https://r

[PATCH] D56989: [analyzer][NFC] Fully initialize CheckerRegistry in by the end of construction

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, baloghadamsoftware. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity. Szelethus retitled this revision from "[analyze

[PATCH] D56989: [analyzer][NFC] Fully initialize CheckerRegistry in by the end of construction

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 182724. Szelethus added a comment. Remove `mutable` specifiers from all fields of `CheckerRegistry`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56989/new/ https://reviews.llvm.org/D56989 Files: include/clang/StaticAnalyzer/Frontend/CheckerRe

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 182726. Szelethus added a comment. - Rebase - Resolve the issue mentioned above by not enabling checkers that has any of their dependencies explicitly disabled - Introduce `osx.RetainCountBase` to "solve" the issue mentioned in D55400#1364683

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

2019-01-20 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 182727. Szelethus added a comment. Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55429/new/ https://reviews.llvm.org/D55429 Files: include/clang/StaticAnalyzer/Core/CheckerManager.h lib/StaticAnalyzer/Checkers/CStringChecker.cpp lib

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-01-21 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Cheers, this is as good as it gets! IRL we also mentioned making a unipage for each checker which would be awesome (and would require a lot of tedious work), but as a start, I'd much prefer browsing through this doc than the current one. Very much appreciated! I won'

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

2019-01-21 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Yup, I'm sold on that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068 ___ cfe-commits ma

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

2019-01-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Poor wording on my end, sorry about that. Let me clarify. In D55400#1366684 , @george.karpenkov wrote: > > Deal with the consequences of this, and just correct all plist files to now > > refer to the new base checker. > > What

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2019-01-22 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. If you don't mind, I'd prefer to finally get over this patch. I'll commit on the 31st, but will wait for any potential feedback 'til then. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51866/new/ https://reviews.llvm.org/D51866

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

2019-01-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D55400#1367046 , @george.karpenkov wrote: > > Hmmm, does this mess with options that bad? Could you please clarify? > > `registerChecker` gets-or-creates a checker object. A checker name (used for > getting the options) is s

[PATCH] D57062: [analyzer] Re-enable the "System is over constrained" assertion on optimized builds.

2019-01-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Hmmm, came across this one in the not too distant future, and always wondered how painful that performance hit would be that even Release+Asserts should be spared from it. I think 5% per

[PATCH] D57062: [analyzer] Re-enable the "System is over constrained" assertion on optimized builds.

2019-01-23 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. And if we find out that this is far too painful, we can always put it back anyways. Doubt that'll happen though. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57062/new/ https://reviews.llvm.org/D57062 __

[PATCH] D57230: [analyzer] Toning down invalidation a bit

2019-01-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus edited reviewers, added: NoQ; removed: dergachev.a. Szelethus added a subscriber: NoQ. Szelethus added a comment. This revision is now accepted and ready to land. Let's also have a link to your cfe-dev mail in this patch: http://lists.llvm.org/pipermail

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D55424#1364696 , @Szelethus wrote: > In D55424#1326397 , @NoQ wrote: > > > Should we also pass `CheckerManager` into `shouldRegister...`? Or is it > > entirely useless? > > > I wouldn'

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352277: [analyzer] Supply all checkers with a shouldRegister function (authored by Szelethus, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55424/new/ http

[PATCH] D55425: [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352278: [analyzer] Split unix.API up to UnixAPIMisuseChecker and… (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D5542

[PATCH] D56988: [analyzer][NFC] Supply CheckerRegistry with AnalyzerOptions

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352279: [analyzer][NFC] Supply CheckerRegistry with AnalyzerOptions (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D56

[PATCH] D56989: [analyzer][NFC] Keep track of whether enabling a checker was explictly specified in command line arguments

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:143 + // Would it be better to name it '~experimental' or something else + // that's ASCIIbetically last? + llvm::sort(Checkers, checkerName

[PATCH] D56989: [analyzer][NFC] Keep track of whether enabling a checker was explictly specified in command line arguments

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352282: [analyzer][NFC] Keep track of whether enabling a checker was explictly… (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.

[PATCH] D56989: [analyzer][NFC] Keep track of whether enabling a checker was explictly specified in command line arguments

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Oops, I ran into the issue of `check-clang-analysis` not actually running out unit tests. I decided not to revert this patch and just commit the fix: rC352284 . Any objections? Repository: rL LLVM CHANGES SINCE LAST ACTION http

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D54438#1372324 , @NoQ wrote: > *gets hyped for the upcoming patchlanding party* Oh yeah, super excited about this! It was a blast! > In D54438#1329425 , @Szelethus wrote: > >> Hmmm,

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352292: [analyzer] Add CheckerManager::getChecker, make sure that a registry function… (authored by Szelethus, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://r

[PATCH] D52790: [analyzer][PlistMacroExpansion] New flag to convert macro expansions to events

2019-01-26 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus abandoned this revision. Szelethus added a comment. As we currently support the actual `macro_expansions` format in CodeChecker, and AFAIK you guys at Apple prefer relying on Xcode (http://lists.llvm.org/pipermail/cfe-dev/2018-September/059231.html), I'd like to focus on more pressing

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2019-01-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td:230 def NullReturnedFromNonnullChecker : Checker<"NullReturnedFromNonnull">, HelpText<"Warns when a null pointer is returned

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2019-01-31 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D54438#1375858 , @george.karpenkov wrote: > After this landed, options for RetainCountChecker stopped working - e.g. I > can't use `osx.cocoa.RetainCount:blah=X`. > Do you know why is this the case / how to fix it? That's

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2019-01-31 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D54438#1378425 , @Szelethus wrote: > In D54438#1375858 , > @george.karpenkov wrote: > > > After this landed, options for RetainCountChecker stopped working - e.g. I > > can't use `osx

[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-01-31 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. @NoQ @dcoughlin @george.karpenkov Any objections to this? :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54429/new/ https://reviews.llvm.org/D54429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. The code not directly related to adding the actual flag looks great, but this still should be an `-analyzer-config` option. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Thanks! LGTM! :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. We should probably prefer `%clang_analyze_cc1` to `%clang_cc1 -analyze` here too. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 ___ cfe-commits m

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Thanks for the fix! :) In D55226#1318853 , @Pierre-vh wrote: > Hello again! I updated the diff and completely removed the outer if. Please > let me know what you think! If we're sure that `Target` isn't a `nullptr`, it's fin

[PATCH] D54592: [analyzer][CStringChecker] evaluate explicit_bzero

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I hope you don't mind me changing the revision title -- many of us are automatically subscribed to revisions with `analyzer` in the title, that also helps with getting feedback sooner :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D53280#1319446 , @NoQ wrote: > Ugh. We broke backwards compatibility when an invalid `-analyzer-config` is > specified but `-analyze` isn't specified. Yes, people are accidentally > relying on that :/ :/ :/ > > In this case

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3307-3309 void Clang::ConstructJob(Compilation &C, const JobAction &JA, const InputInfo &Output, const InputInfoList &Inputs,

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added a comment. In D53280#1319503 , @NoQ wrote: > It's a flag that was removed but old soft doesn't know that it was removed, > and it won't be fixed because it's old. We could just add the flag bac

[PATCH] D55280: [CTU] Remove redundant error check

2018-12-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. From what I can see, this patch LGTM, but I lack the experience in the CTU department just yet to give any meaningful feedback. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:147 llvm::Expected CrossTranslationUnitContext::getCrossTUDefinition(

[PATCH] D55131: [CTU] Add more lit tests and better error handling

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/ctu-main.c:4 +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c +// RUN: cp %S/Inputs/externalFnMap2.txt %t/ctudir2/externalFnMap.txt +// RUN: %clang_cc1 -triple

[PATCH] D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies.

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Cool! Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:528 ArrayRef ExplicitRegions, ArrayRef Regions, const LocationContext *LCtx, const CallEvent *Call) const { This isn't specific to this revision, but I find

[PATCH] D55307: [analyzer] MoveChecker Pt.6: Suppress the warning for the few move-safe STL classes.

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added a comment. This looks great! > Had to pass the checker into the visitor and make some methods non-static > because globals with constructors are discouraged. How about making the set `constexpr`? Comment at: lib/Sta

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I like what I'm seeing. In D55226#1320281 , @Pierre-vh wrote: > Hi again! > > As I'm quite new to this, I don't know what the next step is. Do we need to > wait for more people to review this diff? > What happens when it's con

[PATCH] D55307: [analyzer] MoveChecker Pt.6: Suppress the warning for the few move-safe STL classes.

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. AFAIK `constexpr` arrays can be `std::sort`-ed, but it probably isn't worth the effort, I tried it myself when I was working with non-checker configs, and it's a big hassle for ultimatel

[PATCH] D52790: [analyzer][PlistMacroExpansion] New flag to convert macro expansions to events

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus planned changes to this revision. Szelethus added a comment. Herald added subscribers: gamesh411, baloghadamsoftware. Since the way how these macro events are created as a whole changed in part 1, this path will need a big overhaul. CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D55307: [analyzer] MoveChecker Pt.6: Suppress the warning for the few move-safe STL classes.

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: test/Analysis/diagnostics/explicit-suppression.cpp:22 #ifndef SUPPRESSED - // expected-warning@../Inputs/system-header-simulator-cxx.h:670 {{Called C++ object pointer is null}} + // expected-warning@../Inputs/system-header-simulato

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

2018-12-05 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D54401#1318282 , @alexfh wrote: > In D54401#1315354 , @NoQ wrote: > > > The code looks good, but i vaguely remember that we might accidentally > > break clang-tidy integration that use

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus planned changes to this revision. Szelethus added a comment. Let's try that again then! :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51866/new/ https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies.

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:528 ArrayRef ExplicitRegions, ArrayRef Regions, const LocationContext *LCtx, const CallEvent *Call) const { NoQ wro

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 177002. Szelethus marked 32 inline comments as done. Szelethus added a reviewer: MTC. Szelethus added a comment. Addressing inline comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54823/new/ https://reviews.llvm.org/D54823 Files: lib/Sta

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1087 if (FD->getKind() == Decl::Function) { -initIdentifierInfo(C.getASTContext()); +MemFunctionInfo.initIdentifierInfo(C.getASTContext()); IdentifierInfo *FunI = FD->getIde

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1087 if (FD->getKind() == Decl::Function) { -initIdentifierInfo(C.getASTContext()); +MemFunctionInfo.initIdentifierInfo(C.getASTContex

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Thanks! I'll commit in a couple days to give some breathing room for anyone who'd object. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54401/new/ https://reviews.llvm.org/D54401 ___ cfe-commits mailing list cfe-

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 177057. Szelethus added a comment. Woops, forgot to fix the doc of deallocation functions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54823/new/ https://reviews.llvm.org/D54823 Files: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Anal

[PATCH] D54438: [analyzer][WIP] Reimplement dependencies between checkers

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D54438#1322266 , @NoQ wrote: > In D54438#1315953 , @Szelethus wrote: > > > - ✔️ There are in fact a variety of checkers that contain subcheckers like > > `MallocChecker`, which I think

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-12-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 177080. Szelethus added a comment. This revision is now accepted and ready to land. - Rebased to master. - Adding functions calls with the `noreturn` attribute to the guards. - Shamelessly stole all sorts of assert-like function names from `NoReturnFunction

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC, martong. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, jfb, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a reviewer: teemperor. S

[PATCH] D55425: [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, jfb, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. The actual implementation of `unix.API` featu

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 177182. Szelethus retitled this revision from "[analyzer][WIP] Reimplement dependencies between checkers" to "[analyzer] Reimplement dependencies between checkers". Szelethus edited the summary of this revision. Szelethus added reviewers: MTC, baloghadamsoft

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 177189. Szelethus added a comment. - Register the checker after it's dependencies (accidentally left this change in the next branch). This implies that `unix.MallocChecker:Optimistic` is no longer a thing, and is now called ųnix.DynamicMemoryModeling:Optim

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested review of this revision. Szelethus added a comment. Please take a second look, this is a pretty important to get right. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54438/new/ https://reviews.llvm.org/D54438 ___ cfe-com

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested review of this revision. Szelethus added a comment. Marking this patch as non-accepted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51866/new/ https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D55424: [analyzer] Supply all checkers with a shouldRegister function

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. The checker file changes in this patch are pretty boring, here are those that don't always return true in `shouldRegister`: - `CastSizeChecker` - `GTestChecker` - `NSAutoreleasePoolChecker` - `ObjCAtSyncChecker` Repository: rC Clang CHANGES SINCE LAST ACTION htt

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker that asserts on non-registered checkers, assert on registering already registered checkers

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC, baloghadamsoftware. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity. Title says it all, this is pretty important. Whil

[PATCH] D55387: [analyzer] MoveChecker Pt.7: NFC: Misc refactoring.

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Woohoo! Thanks! Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:515-516 // class in which the encountered method defined. while (const auto *BR = dyn_cast

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 177245. Szelethus retitled this revision from "[analyzer] Add CheckerManager::getChecker that asserts on non-registered checkers, assert on registering already registered checkers" to "[analyzer] Add CheckerManager::getChecker, make sure that a registry fu

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D54438#1315953 , @Szelethus wrote: > - ❌ Move `CheckerManager::registerChecker` out of the registry functions. > - ❌ Since `shouldRegister##CHECKERNAME` is a thing, we can move everything > to the checker's constructor, sup

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I recently had trouble with ignoring the return type, I would certainly appreciate a tool like this. Can you test it on LLVM+Clang? Maybe some methods in the AST library could also benefit from `LLVM_NODISCARD` (containers with factories is my no1 thought), and it's a

[PATCH] D55425: [analyzer] Split unix.API up to UnixAPIMisuseChecker and UnixAPIPortabilityChecker

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: test/Analysis/Inputs/expected-plists/unix-fns.c.plist:3018 - /clang/test/Analysis/unix-fns.c + /home/szelethus/Documents/analyzer_opts/clang/test/Analysis/unix-fns.c

[PATCH] D55429: [analyzer] Add CheckerManager::getChecker, make sure that a registry function registers no more than 1 checker

2018-12-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. In D55429#1324035 , @NoQ wrote: > Aha, ok, so what's the final procedure In D55429#1324035 , @NoQ wrote: > Aha, ok, so what's the final procedure now to register a checker that's > a

[PATCH] D53280: [analyzer] Emit an error for invalid -analyzer-config inputs

2018-12-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I guess the solution would be to check whether there are any user supplied flags with "analyze" substring, and add the compatibility flag then. It is possible if not probable that a non-static-analyzer related flag with a name like that will eventually be added, but I

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2018-12-10 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. I can't fix these right away, but I don't want myself to forget it before commiting. Pay no attention. Comment at: include/clang/StaticAnalyzer/Checkers/CheckerBase.td:51-56 +/// relies on information MallocBase gathers. +/// Example: +/// def Inne

[PATCH] D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure.

2018-12-11 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Makes sense! I like the summary a lot, and the fact that you added a new debug checker. I feel like I'm not yet qualified to give meaningful feedback though, but if you are not in a hurry, I'll happily play around with this patch next week, and both learn a bit and po

[PATCH] D58777: [analyzer] Fix an assertation failurure for invalid sourcelocation, add a new debug checker

2019-02-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, baloghadamsoftware, Charusso. Szelethus added a project: clang. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. For a rather short code snip

[PATCH] D58777: [analyzer] Fix an assertation failurure for invalid sourcelocation, add a new debug checker

2019-02-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 188713. Szelethus added a comment. Added context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58777/new/ https://reviews.llvm.org/D58777 Files: docs/analyzer/developer-docs/DebugChecks.rst include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D58828: [analyzer] Fix taint propagation in GenericTaintChecker

2019-03-01 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58828/new/ https://reviews.llvm.org/D58828 ___ cfe-comm

[PATCH] D57855: [analyzer] Reimplement checker options

2019-03-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 189088. Szelethus added a comment. Herald added a subscriber: Charusso. - Capitalized some variable names - Added more comments - Preferring binary searches to linear searches wherever possible CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/ne

[PATCH] D57855: [analyzer] Reimplement checker options

2019-03-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 189090. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new/ https://reviews.llvm.org/D57855 Files: include/clang/Basic/DiagnosticCommonKinds.td include/clang/StaticAnalyzer/Checkers/CheckerBase.td include/clang/StaticAnalyzer/Checkers/Ch

[PATCH] D58573: [analyzer] Move UninitializedObject out of alpha

2019-03-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Herald added a subscriber: Charusso. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58573/new/ https://reviews.llvm.org/D58573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D57579: [analyzer][WIP] Enable subcheckers to possess checker options

2019-03-03 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC355297: [analyzer] Enable subcheckers to possess checker options (authored by Szelethus, committed by ). Herald added a su

[PATCH] D57855: [analyzer] Reimplement checker options

2019-03-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 189105. Szelethus added a comment. Remembered that we can't use use `llvm::binary_search`, as we'll need the iterators in a later patch. Should've rebased all my branches before updating :^) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

2019-03-03 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 189107. Szelethus added a comment. Herald added a reviewer: teemperor. Herald added a subscriber: Charusso. This revision now requires review to proceed. Remove the default value parameters from `getChecker*Option`, as they are no longer necessary. Note the

[PATCH] D58828: [analyzer] Fix taint propagation in GenericTaintChecker

2019-03-05 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC355396: [analyzer] Fix taint propagation in GenericTaintChecker (authored by Szelethus, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58828/new/ https://re

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

2019-03-06 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus marked an inline comment as done. Szelethus added inline comments. Herald added a subscriber: Charusso. Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:352-355 + if (Checker->AllowedPad < 0) + Mgr.getDiagnostics().Report(diag::err_analyzer_checker_option

[PATCH] D57891: [analyzer] Fix infinite recursion in printing macros

2019-03-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Well, yea, getting rid of a crash is great, it's kind of bummer that can't expand the macro properly in the testfile. I really fear that we need a far greater overhaul of this entire eff

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

2019-03-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. At first glance this looks OK, but I'll take the time to understand the infrastructure behind it and give a proper review. Comment at: clang/test/Analysis/array-struct-region.cpp:3 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core,debug.

[PATCH] D59054: [analyzer] C++17: PR40022: Support aggregate initialization with compound values in presence of base classes.

2019-03-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Ooooh you explained that very well, I'm clear on what's happening then :D Still, I'll poke the code here and there to learn about this part of the analyzer, I might even come up with some meaningful feedback (or a meaningful patch acceptance). CHANGES SINCE LAST ACT

[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker

2019-03-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: JonasToth. Szelethus added a comment. Is this patch good to go? :) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48866/new/ https://reviews.llvm.org/D48866 ___ cfe-commits ma

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

2019-03-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus planned changes to this revision. Szelethus added a comment. Uhh, we still need to restore the default value -- I might end up splitting this patch a little further. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57922/new/ https://reviews.llvm.org/D57922 __

[PATCH] D59055: [analyzer] Prepare generic taint checker for new sources

2019-03-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. Cheers! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59055/new/ https://reviews.llvm.org/D59055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D59121: [analyzer] Fix macro names in diagnostics within bigger macros.

2019-03-08 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Uhh, it's such a chore to work on these things -- You can really feel that the preprocessor must've been the first thing implemented in clang. Unfortunately, you really have to

[PATCH] D59055: [analyzer] Prepare generic taint checker for new sources

2019-03-08 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC355703: [analyzer] Use the new infrastructure of expressing taint propagation, NFC (authored by Szelethus, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D590

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

2019-03-08 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC355704: [analyzer] Emit an error rather than assert on invalid checker option input (authored by Szelethus, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57

  1   2   3   4   5   6   7   8   9   10   >