[PATCH] D48521: [analyzer] Highlight STL object destruction in MallocChecker

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Only minor nits, as usual :) Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:483 // Did not track -> released. Other state (allocated) -> released. - return (

[PATCH] D52905: [analyzer] fix accessing GDM data from shared libraries

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Cool, thanks! I'll commit. Repository: rC Clang https://reviews.llvm.org/D52905 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D52905: [analyzer] fix accessing GDM data from shared libraries

NoQ added a comment. > where `isStaticLocal` is defined as: You can send this one as well if you like! It's weird that we don't already have it. Repository: rC Clang https://reviews.llvm.org/D52905 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D52906: [analyzer] allow plugins built as shared libraries to receive events

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yup, this one looks great as well. I'll commit. Thanks! Repository: rC Clang https://reviews.llvm.org/D52906 ___ cfe-commits mailing list cfe-commit

[PATCH] D52905: [analyzer] fix accessing GDM data from shared libraries

NoQ added a comment. In https://reviews.llvm.org/D52905#1259249, @NoQ wrote: > > where `isStaticLocal` is defined as: > > You can send this one as well if you like! It's weird that we don't already > have it. Or actually maybe it can be implemented as `allOf(hasStaticStorageDuration(), unless

[PATCH] D52983: [analyzer] Support Reinitializes attribute in MisusedMovedObject check

NoQ added a comment. In https://reviews.llvm.org/D52983#1258499, @xazax.hun wrote: > In https://reviews.llvm.org/D52983#1258466, @NoQ wrote: > > > Yay, these look useful. Is there also an attribute for methods that should > > never be called on a 'moved-from' object? > > > I do not know about su

[PATCH] D53024: [analyzer][www] Add more open projects

NoQ added a comment. Whoa thanks! Will have a closer look again. Comment at: www/analyzer/open_projects.html:33 + +Handle aggregate construction. +Aggregates are object that can be brace-initialized without calling a I'll try to list other constructor k

[PATCH] D53076: [analyzer] WIP: Enhance ConditionBRVisitor to write out more information

NoQ added a comment. In https://reviews.llvm.org/D53076#1260663, @george.karpenkov wrote: > 1. Note that "Assuming X" directives are useful for the analyzer developers, > since they know that's where the checker makes arbitrary assumptions, but to > end users that mostly feels like noise ("Taki

[PATCH] D53024: [analyzer][www] Add more open projects

NoQ added a comment. > In addition 2018 Bay Area LLVM Developers' Meetings may bring some new open > projects :) Actually, let's commit this before the conference, even if it's not perfect, so that people who suddenly get inspired to work on Static Analyzer already had an updated list :) ==

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ added a comment. In https://reviews.llvm.org/D52984#1260761, @george.karpenkov wrote: > @Szelethus @xazax.hun Since you guys are looking into the website, do you > know why the top bar has disappeared from all pages? (E.g. > http://clang-analyzer.llvm.org/available_checks.html ) The relev

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ added a comment. > we probably also should be moving towards an auto-generated model anyway, if > we want to e.g. automatically build sections of a website from the code. This too will most likely require server maintainers to intervene, so i think fixing SSI is more realistic. https://re

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ added a comment. > is it though? If we simply add a new menu point, it'll require us to modify a huge number of files. We might get away with good comments that explain the situation and making sure that all changes are done with sed, but i guess we'll still be stuck forever. > HTML files

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

NoQ added a reviewer: george.karpenkov. NoQ added a comment. I'll take care of this review as an [analyzer]-responsible person, maybe not immediately because i'm a bit overwhelmed right now - sorry! - and George may want to have a look (also or instead). Repository: rC Clang https://reviews

[PATCH] D53239: [python] [tests] Disable python binding tests when building with LLVM_USE_SANITIZER=Address

NoQ created this revision. NoQ added reviewers: mgorny, delcypher. Herald added a subscriber: cfe-commits. @mgorny: These fail for me on macOS too, if LLVM is configured as cmake ../llvm/ -DCMAKE_BUILD_TYPE=Release -DLLVM_USE_SANITIZER=Address I guess we might have to outright disable them und

[PATCH] D32905: [Analyzer] Iterator Checker - Part 9: Evaluation of std::find-like calls

NoQ added inline comments. Herald added a subscriber: donat.nagy. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1756-1760 + auto stateFound = state->BindExpr(CE, LCtx, RetVal); + auto stateNotFound = state->BindExpr(CE, LCtx, SecondParam); + + C.addTransition(sta

[PATCH] D32905: [Analyzer] Iterator Checker - Part 9: Evaluation of std::find-like calls

NoQ added a comment. In https://reviews.llvm.org/D32906#1264452, @baloghadamsoftware wrote: > Unfortunately, we are at the beginning of a long road. I will post several > new patches that we already test internally. However the only checker with > acceptable false-positive rate is the `invalida

[PATCH] D52957: [analyzer] Teach CallEvent about C++17 aligned new.

NoQ added a comment. Whoops, almost forgot to doxygen-ize comments. Landed in https://reviews.llvm.org/rC344540. Repository: rL LLVM https://reviews.llvm.org/D52957 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D53024: [analyzer][www] Add more open projects

NoQ added inline comments. Comment at: clang/www/analyzer/open_projects.html:27-32 +New checkers which were contributed to the analyzer, +but have not passed a rigorous evaluation process, +are committed as "alpha checkers" (from "alpha version"), +and are not ena

[PATCH] D53024: [analyzer][www] Add more open projects

NoQ added a comment. The current text looks great. https://reviews.llvm.org/D53024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51762: First part of the calendar stuff

NoQ added a comment. There seem to be more buildbots failing even after https://reviews.llvm.org/rL344535, eg. http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/50318/console Failing Tests (3): libc++ :: std/utilities/time/time.cal/time.cal.day/time.cal.day.nonmembers/liter

[PATCH] D51762: First part of the calendar stuff

NoQ added a comment. Had to revert. Sorry! https://reviews.llvm.org/rL344580. This failure was masked by another error, so i guess it was missed. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D51762: First part of the calendar stuff

NoQ added a comment. Well, i guess something went wrong, because the job behind the link is in fact the first job on this buildbot that included r344535. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D51762: First part of the calendar stuff

NoQ added a comment. Also i should not have reverted r344546, it was completely unrelated >_< https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51762: First part of the calendar stuff

NoQ added a comment. Re-applied https://reviews.llvm.org/rL344546 as https://reviews.llvm.org/rL344582. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D53024: [analyzer][www] Add more open projects

NoQ added inline comments. Comment at: clang/www/analyzer/open_projects.html:27-32 +New checkers which were contributed to the analyzer, +but have not passed a rigorous evaluation process, +are committed as "alpha checkers" (from "alpha version"), +and are not ena

[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:469 +DisplayMacroExpansions = +getBooleanOption("expand-macros", /*Default=*/false); + return DisplayMacroExpansions.getValue(); Should we say something about plists i

[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:727 + +static std::string getExpandedMacroImpl(TokenPrinter &Printer, +SourceLocation MacroLoc, Whoa, that's so easy! Co

[PATCH] D53024: [analyzer][www] Add more open projects

NoQ accepted this revision. NoQ added a comment. Ok, let's see if this actually works :) https://reviews.llvm.org/D53024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Code looks good to me, but I'd wait for @george.karpenkov because it was his idea. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:502

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

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:515 + continue; +const auto *FirstAccess = Accesses[0].getNodeAs("access"); + I feel that it's a good practice to `assert(FirstAccess)`

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163 + +switch (FloatingSize) { + case 64: Continuing the float semantics discussion on the new revision - Did you consider `llvm::APFloat`? (http://llvm.org/doxygen

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ added a comment. My take on the out-of-alpha checklist: - The checker should be evaluated on a large codebase in order to discover crashes and false positives specific to the checker. It should demonstrate the expected reasonably good results on it, with the stress on having as little fals

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ added a comment. Hmm, i guess i sent this out too early. As per our long-lasting tradition, i forgot the point about the web page :\ Bullets about RUN-lines, Checkers.td descriptions and CMakeLists are also great for everyday use :) https://reviews.llvm.org/D52984 _

[PATCH] D52906: [analyzer] allow plugins built as shared libraries to receive events

NoQ closed this revision. NoQ added a comment. Herald added subscribers: dkrupp, donat.nagy. I committed both patches as https://reviews.llvm.org/rC344823 but it seems that Phabricator doesn't pick up closing multiple revisions with the same commit, so closing manually. Also thanks!~ Reposito

[PATCH] D53069: [analyzer][www] Update avaible_checks.html

NoQ accepted this revision. NoQ added a comment. I guess maybe let's skip stuff without examples and leave Objective-C descriptions waiting on us? Comment at: www/analyzer/available_checks.html:483 + +LLVM Checkers Wow, i never noticed this one. It seems to

[PATCH] D53276: [analyzer][NFC] Fix some incorrect uses of -analyzer-config options

NoQ added a reviewer: dcoughlin. NoQ added inline comments. Herald added a subscriber: dkrupp. Comment at: lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:1401-1402 checker->IsAggressive = - mgr.getAnalyzerOptions().getBooleanOption("AggressiveReport", false); +

[PATCH] D53274: [analyzer][NFC] Fix inconsistencies in AnalyzerOptions

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dkrupp. Thanks! Let's commit. https://reviews.llvm.org/D53274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D53277: [analyzer][NFC] Collect all -analyzer-config options in a .def file

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I think this is awesome o_o Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:70-71 + +/// Controls the high-level analyzer mode, which influences the default +///

[PATCH] D53280: [analyzer] Emit a warning for unknown -analyzer-config options

NoQ added a comment. Herald added a subscriber: dkrupp. I guess we should consider the idea in http://lists.llvm.org/pipermail/cfe-dev/2018-October/059864.html Comment at: lib/Frontend/CompilerInvocation.cpp:343 } + // Check whether this really is a valid -analyzer

[PATCH] D53296: [analyzer] New flag to print all -analyzer-config options

NoQ accepted this revision. NoQ added a comment. I'll use this. And i think it's unfair to give users -analyzer-config and not give them a list of options. Users should be able to discover either both or none of the two. If a user goes far enough to start digging into non-driver options, such u

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

NoQ added a comment. Ugh, i was sure i scanned through all of them, sorry! Pls let me recall what's going on>< https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

NoQ added a comment. `MemRegion`s have lifetime of `ExprEngine`, i.e. a single analysis of top-level function. You'll need to clean them up in `checkEndAnalysis()`. https://reviews.llvm.org/D51531 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D51531: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once

NoQ added a comment. I think `RetainCountChecker` is the only checker that maintains such maps and does such cleanup: https://github.com/llvm-mirror/clang/blob/efe41bf98/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h#L288 I didn't know it does that until today. https://re

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, baloghadamsoftware. Fixes https://bugs.llvm.org/show_bug.cgi?id=39348 Under `-fsized-deallo

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

NoQ updated this revision to Diff 170541. NoQ added a comment. Nono, that was an old patch, sry. Here's the correct patch. https://reviews.llvm.org/D53543 Files: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/NewDelete-custom.cpp test/Analysis/NewDelete-sized-deallocation.cpp

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

NoQ updated this revision to Diff 170543. NoQ added a comment. Remove weird flags that i used for debugging from the test run-lines. https://reviews.llvm.org/D53543 Files: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/NewDelete-custom.cpp test/Analysis/NewDelete-sized-deallo

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

NoQ updated this revision to Diff 170704. NoQ marked an inline comment as done. NoQ added a comment. Add tests for different versions of the standard and for different ways to declare operator delete. > Should we remove it then? > why it's fine to remove this branch? Because these two questi

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:727 - // Skip all operator new/delete methods. - if (isa(FD)) -return false; - - // Return true if tested operator is a standard placement nothrow operator. - if (FD->getNumParams() ==

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

NoQ updated this revision to Diff 170946. NoQ added a comment. Address comments! https://reviews.llvm.org/D53543 Files: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/NewDelete-custom.cpp test/Analysis/NewDelete-sized-deallocation.cpp Index: test/Analysis/NewDelete-sized-dea

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/Environment.cpp:180 - for (; SI != SE; ++SI) -SymReaper.maybeDead(*SI); } zaks.anna wrote: > We are removing this because the maybeDead is no longer used, correct? Yup. ===

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

NoQ updated this revision to Diff 171006. NoQ marked 2 inline comments as done. NoQ added a comment. Herald added subscribers: dkrupp, donat.nagy, Szelethus, mikhail.ramalho. Rebase! Changes on large codebase runs still look mild and reasonable, with much less slowdown than before. Even if there

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

NoQ added inline comments. Comment at: test/Analysis/simple-stream-checks.c:96 + fp = 0; +} // expected-warning {{Opened file is never closed; potential resource leak}} george.karpenkov wrote: > Woo-hoo, were we losing this case before? Yes, this is the whole po

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

NoQ added a comment. I mean, the idea of checking constraints instead of matching program points is generally good, but the example i gave above suggests that there's a bug somewhere. https://reviews.llvm.org/D53076 ___ cfe-commits mailing list cf

[PATCH] D53076: [analyzer] Enhance ConditionBRVisitor to write out more information

NoQ added a comment. In https://reviews.llvm.org/D53076#1276315, @Charusso wrote: > In https://reviews.llvm.org/D53076#1276277, @NoQ wrote: > > > I mean, the idea of checking constraints instead of matching program points > > is generally good, but the example i gave above suggests that there's

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163 + +switch (FloatingSize) { + case 64: donat.nagy wrote: > NoQ wrote: > > Continuing the float semantics discussion on the new revision - Did you > > consider `l

[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:471 +DisplayMacroExpansions = +getBooleanOption("expand-macros", /*Default=*/false); + return DisplayMacroExpansions.getValue(); Yup, np, let's wait until it lands the

[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag

NoQ accepted this revision. NoQ added inline comments. This revision is now accepted and ready to land. Comment at: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:17-18 + + name + expansion + NoQ wrote: > They look em

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Patch looks great, thanks! Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:78-81 +// We need to be looking at a definition, not just any pointer to the +// decla

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

NoQ accepted this revision. NoQ added a comment. Thanks! Also do you have commit access or do you need someone to commit this for you? Repository: rC Clang https://reviews.llvm.org/D53206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D53206: Allow padding checker to traverse simple class hierarchies

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:78-81 +// We need to be looking at a definition, not just any pointer to the +// declaration. +if (!(RD = RD->getDefinition())) + return; tekknolagi wrote: > te

[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Wow thanks! Great point. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82 -static void EmitRanges(raw_ostream &o, - const ArrayRef Ranges, -

[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82 -static void EmitRanges(raw_ostream &o, - const ArrayRef Ranges, - const FIDMap& FM, - const Preprocessor &PP, -

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

NoQ accepted this revision. NoQ added a comment. Thanks! I like where this is going. Let's land the patch and continue developing it incrementally in trunk. The next steps for this checker are, in my opinion: - Do the visitor thingy that i requested in inline-311373

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I've actually never noticed this checker until you started updating the website :) This might be also covered by @rnkovacs's string buffer escape checker - either already or eventually, it'll becom

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ added a comment. Yes. Not just for checkers, not just for beginners :) https://reviews.llvm.org/D52984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56441: [analyzer][CrossTU][NFC] Generalize to external definitions instead of external functions

NoQ added a comment. Hmm, it disappeared after a clean rebuild. I think the problem was that i did `make clang` and then ran tests manually, instead of `make`, so the tool was not rebuilt and was only present under its old name. Because the tool does get rebuilt under `make check-clang-analysis

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

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet, mgorny. This is a follow-up to D56042

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

NoQ updated this revision to Diff 181419. NoQ marked an inline comment as done. NoQ added a comment. Prettify the unittest a bit, especially the `ASTMatcher` part of it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56632/new/ https://reviews.llvm.org/D56632 Files: include/clang/Stat

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

NoQ updated this revision to Diff 181961. NoQ marked 12 inline comments as done. NoQ added a comment. Fix stuff. In D56632#1356163 , @baloghadamsoftware wrote: > Did you measure decrease in the false-positive rate or an increase in the > true-positive r

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

NoQ added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:390 - // FIXME: Remove when we migrate over to just using ValueManager. SymbolManager &getSymbolManager() { return SymMgr; } baloghadamsoftware wrote: > Is t

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

NoQ added a comment. In D56632#1359576 , @xazax.hun wrote: > If you find out the reason why we need `markElementIndicesLive`, documenting > that would also be useful. But it is also independent of this change. > Maybe something like we could learn new i

[PATCH] D56823: [analyzer] Do not try to body-farm bodies for Objective-C properties with custom accessors.

NoQ created this revision. NoQ added reviewers: dcoughlin, george.karpenkov. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, kristof.beyls, xazax.hun, javed.absar. If a property is defined with a custom getter, we shou

[PATCH] D56824: [analyzer] MoveChecker: add ".assign" to the list of common reinitializing methods.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet. Eg., `std::vector::assign(std::initializer_list)`. I wonder h

[PATCH] D56823: [analyzer] Do not try to body-farm bodies for Objective-C properties with custom accessors.

NoQ added a comment. Yeah, i'll be testing this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56823/new/ https://reviews.llvm.org/D56823 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

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

NoQ added a comment. In D56632#1359215 , @NoQ wrote: > In D56632#1356163 , > @baloghadamsoftware wrote: > > > Did you measure decrease in the false-positive rate or an increase in the > > true-positive rate on rea

[PATCH] D56899: [analyzer] pr37688: Fix a crash on trying to evaluate a deleted destructor of a union.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, szepet. This is a slow quick fix for https://bugs.llvm.org/show_bug.cgi?id=37688

[PATCH] D56823: [analyzer] Do not try to body-farm bodies for Objective-C properties with custom accessors.

NoQ added a comment. Nope, it doesn't seem to skew results at all. I hope my testing machinery is actually working :/ Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56823/new/ https://reviews.llvm.org/D56823 ___ cfe-

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I have no objections. George, this was your idea, does it look good to you? Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:577-578 + +

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

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet. This assertion was only enabled in Debug+Asserts, not on Relea

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

NoQ added a subscriber: alexfh. NoQ added a comment. I think there are much more problems that we will find this way, starting from long foo(long x) { unsigned y = (unsigned)x; if (y <= -1) return 0; return (y + 2) < 2; // crash :( } I don't mind delaying this patch unti

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

NoQ added a comment. Thanks! A surprise, to be sure :) I'll try to test this on my set of projects as well :) > Most of the extra results are not false positive due to the less invalidation > but other reasons, so we could focus on those problems instead of them being > hidden. Could you shar

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yup, totally. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56988/new/ https://reviews.llvm.org/D56988 ___ cfe-commits

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Cool! I guess we still need to think how exactly do we want to resolve conflicts between dependencies. Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:152 +

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

NoQ accepted this revision. NoQ added a comment. *gets hyped for the upcoming patchlanding party* In D54438#1329425 , @Szelethus wrote: > Hmmm, I left plenty of room for improvement in the tblgen generation, we > could generate compile-time errors on cyc

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

NoQ added a comment. In D57230#1372488 , @xazax.hun wrote: > In D57230#1372275 , @NoQ wrote: > > > > > > Do you have success reducing false positives using creduce? My problem > usually is that we cannot tell if a

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

NoQ accepted this revision. NoQ added a comment. In D57230#1373721 , @xazax.hun wrote: > Do you think I should try to reduce additional files? Aha, ok, it reduced an interesting positive into a non-interesting positive. So i guess my method only works w

[PATCH] D55734: [analyzer] Revise GenericTaintChecker's internal representation

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. In D55734#1372972 , @boga95 wrote: > Is it ready to land? Whops. Yes, right, it's totally ready to land! Sorry. Should i commit? CHANGES SINCE LAST AC

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

NoQ accepted this revision. NoQ added a comment. Ok! I hope that the C11 check would do the trick, let's see how it goes :) In D35068#1364947 , @xazax.hun wrote: > What do we want to validate here? The lack of crashes? Or evaluate false > positive ratio?

[PATCH] D52984: [analyzer] Checker reviewer's checklist

NoQ accepted this revision. NoQ added a comment. Thanks!!~ I'm slow, but at least i admit it... Just made myself a fancier phabricator query so that not to forget about patches, so hopefully i won't forget about patches that often anymore :/ I mean, seriously, i'm very happy that we're all doi

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

NoQ added a comment. In D46421#1354188 , @r.stahl wrote: > In my old version I seemed to get away with the tests, but they failed after > rebasing. It seems like earlier there was only a single VarDecl for the > imported ones with InitExpr. Now after imp

[PATCH] D55734: [analyzer] Revise GenericTaintChecker's internal representation

NoQ added a comment. Hmm, the diff is a mixture of D54918 and this patch. I'll apply it by reverting D54918 locally and then applying this diff, but please let us know if you accidentally uploaded a wrong diff! CHANGES SINCE

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I think let's get this up and running and slowly redirect links from the front page to the new docs until only the front page is left? Comment at: docs/analyzer/checkers.rst:36-37

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

NoQ updated this revision to Diff 172263. NoQ added a comment. Ha! Apart from Zombie symbols that are neither dead nor alive and therefore not buried properly, there are also Schrödinger symbols that are dead and alive at the same time. Moreover, asking whether a Schrödinger symbol is `isLive()`

[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

NoQ added a comment. Just to be clear - this newly found problem is also automatically fixed by that patch. https://reviews.llvm.org/D18860 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D54013: [analyzer] MallocChecker: Avoid redundant transitions.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, rnkovacs. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, szepet, baloghadamsoftware. In its `checkDeadSymbols` callback, when no pointers are tracked and the

[PATCH] D54017: [analyzer] NullabilityChecker: Invariant violation should only be triggered for symbols.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, george.karpenkov. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. This false negative bug was exposed by https://reviews.llvm.org/D18860. T

[PATCH] D54017: [analyzer] NullabilityChecker: Invariant violation should only be triggered for symbols.

NoQ added a comment. It also seems that we still have a false negative under ARC, but i didn't bother investigating it. Just in case, i added `-fobjc-arc` run-lines to older tests to see if there's any difference, and it turned out that there's no difference. Repository: rC Clang https://r

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

NoQ accepted this revision. NoQ added a comment. The new addition makes perfect sense, please feel free to commit :) Eventually i guess we'll need to declare where is this checker going and what specific lint rule is this checker designed to check, document this rule, and move it to `optin` onc

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

NoQ accepted this revision. NoQ added a comment. Looks great, let's land? Not sure if i already asked - am i understanding correctly that this is a "poor-man's" support for macro expansions for tools that read plists but do not support those new plist macro dictionaries yet? Also i wanted to ha

[PATCH] D52794: [analyzer][PlistMacroExpansion] Part 2.: Retrieving the macro name and primitive expansion

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Because i don't know much about macros, i shouldn't be blocking this review on waiting for myself to understand the code :) I guess we'll eventually figure out if something is wrong with it. https

<    1   2   3   4   5   6   7   8   9   10   >