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

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:80 class ConstraintManager { + using ConstraintMap = std::map; + ConstraintMap Constraints; Traditionally LLVM projects use `llvm::DenseMap

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

2018-10-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:202 + void finalizeConstraints() { +Constraints.clear(

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

2018-10-23 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > because the guard that prevents it from working is useless and can be removed > as well Should we remove it then? Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:729 + SourceLocation L = FD->getLocation(); + return !L.isValid()

[PATCH] D53549: [analyzer] Trust summaries for OSObject::retain and OSObject::release

2018-10-23 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC345099: [analyzer] Trust summaries for OSObject::retain and OSObject::release (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://revi

[PATCH] D53615: [analyzer] [NFC] Change scanReachableSymbols to use ranges

2018-10-23 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC345101: [analyzer] [NFC] Change scanReachableSymbols to use ranges (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D53550: [analyzer] Do not stop tracking CXX methods touching OSObject.

2018-10-23 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC345100: [analyzer] Do not stop tracking CXX methods touching OSObject. (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

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

2018-10-24 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Good to go provided you will add an example. If we want to be serious about this page, it really has to be auto-generated (like clang-tidy one), but I understand t

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

2018-10-24 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:80 typedef llvm::ImmutableMap GenericDataMap;

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

2018-10-24 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I'll try to take a look this week. Repository: rC Clang https://reviews.llvm.org/D53206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-10-24 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: test/Analysis/self-assign.cpp:42 str = rhs.str; - rhs.str = nullptr; // FIXME: An improved leak checker should warn here + rhs.str =

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

2018-10-25 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Thanks a lot, this is almost ready! I think it should be good to go after this round of nitpicks. Comment at: include/clang/StaticAnalyzer/Core

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

2018-10-25 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor nitpicks, otherwise makes sense! Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:746 + /// exploring a top level function

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

2018-10-25 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Actually, apologies again: I have rushed through this too much. @NoQ has a good point: we need to preserve the distinction between the things analyzer "assumes" an

[PATCH] D53628: [analyzer] Remove custom rule for OSIterator in RetainCountChecker

2018-10-25 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC345339: [analyzer] Remove custom rule for OSIterator in RetainCountChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Patch context is missing. Comment at: Analysis/diagnostics/sarif-check.py:22 +passes = 0 +with open(testfile) as testfh: +lineno = 0 Wow, this is super neat! Since you are quite active in LLVM community, would you think it'

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor style notes + context missing. I think using `diff` would be better than a custom python tool. https://reviews.llvm.org/D53814 _

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-29 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. I don't think a new PathGenerationScheme is needed, unless you plan changes to BugReporter.cpp. The code is fine otherwise, but could we try to use `diff` for tes

[PATCH] D53814: Allow the analyzer to output to a SARIF file

2018-10-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. I much prefer this version. We've had the same problem with diffing plist output. One thing we have learned is using a FileCheck was definitely a bad idea, as it leads to un

[PATCH] D53628: [analyzer] Remove custom rule for OSIterator in RetainCountChecker

2018-10-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Actually, I'm reverting this: the rule seems fairly ubiquitous. Repository: rC Clang https://reviews.llvm.org/D53628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D55616: Emit ASM input in a constant context

2019-01-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @void @efriedma I can't build XNU anymore after this commit, with an error message: osfmk/i386/genassym.c:298:2: error: value '18446743523953737728' out of range for constraint 'n' DECLAREULL("KERNEL_BASE", KERNEL_BASE); ^~

[PATCH] D55616: Emit ASM input in a constant context

2019-01-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Apologies - the value seems to indeed overflow, but I'm still very confused how this was affected by this change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55616/new/ https://reviews.llvm.org/D55616 __

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

2019-01-14 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Whoops, sorry. There were holidays, and then I did forget about this patch. I'll commit this now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226 ___ cfe-commits ma

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

2019-01-14 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @Pierre-vh The patch does not compile due to unmatched braces. Please do test and compile before submitting! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55226/new/ https://reviews.llvm.org/D55226 ___ cfe

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

2019-01-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL351097: [analyzer] [PR39792] false positive on strcpy targeting struct members (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D56759: [analyzer] Another RetainCountChecker cleanup

2019-01-16 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC351394: [analyzer] Another RetainCountChecker cleanup (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D56759?vs=1

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

2019-01-17 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. LG, but it sounds like something which can skew results a lot (?) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56823/new/ https://reviews.llvm.org/D56823 ___ cfe-commits mailing lis

[PATCH] D56820: [analyzer] [RetainCountChecker] Produce a correct message when OSTypeAlloc is used

2019-01-17 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC351509: [analyzer] [RetainCountChecker] Produce a correct message when OSTypeAlloc is… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: htt

[PATCH] D56884: [analyzer] Extend BugType constructor to accept "SuppressOnSink" as a parameter

2019-01-17 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC351510: [analyzer] Extend BugType constructor to accept "SuppressOnSink" as a parameter (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: h

[PATCH] D56885: [analyzer] const-ify reference to bug type used in BugReporter

2019-01-17 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC351511: [analyzer] const-ify reference to bug type used in BugReporter (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

[PATCH] D56891: [analyzer] Introduce proper diagnostic for freeing unowned object

2019-01-17 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC351514: [analyzer] Introduce proper diagnostic for freeing unowned object (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.

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

2019-01-18 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. The code is fine, but I obviously would prefer a proper fix in a CFG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56899/new/ https://revie

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

2019-01-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > Deal with the consequences of this, and just correct all plist files to now > refer to the new base checker. What does it mean? > Each time a report is emitted from these checkers, create a ProgramPointTag, > and "manually" make sure the emitted checker name

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

2019-01-22 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > 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 set the first time it's created. The checker which was created first "wins" and gets to n

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

2019-01-23 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. We would start getting crashes from `TrustNonnullChecker` if we enable it. Shouldn't those be fixed first? An input which crashes with this assertion is `test/Ana

[PATCH] D57204: [AST] Add a method to get a call type from an ObjCMessageExpr

2019-01-24 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352147: [AST] Add a method to get a call type from an ObjCMessageExpr (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

2019-01-25 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > Can you elaborate a bit in what sense this decouples ARCMT from the analyzer? ARCMT now does not need to link against the analyzer > Can CLANG_ENABLE_STATIC_ANALYZER now be set independently of > CLANG_ENABLE_ARCMT? Yes Repository: rC Clang CHANGES SINC

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

2019-01-25 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > Also, isn't lib/Analysis a strange place for this? lib/Analysis is used by > the compiler proper (CodeGen, Sema, …), while RetainSummaryManager is only > used by tooling-like things (static analyzer, arcmigrate). Not only, it has utilities used by the static

[PATCH] D57127: [analyzer] Port RetainSummaryManager to the new GenericCall interface, decouple ARCMT from the analyzer

2019-01-28 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @thakis Sorry I don't understand your point. At the end of the day, ProgramPoint and BodyFarm are only used by the static analyzer. If you disagree, what is your proposed directory structure? What would be the benefits? Repository: rC Clang CHANGES SINCE LA

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

2019-01-28 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I'm in favor of this change, I never understood how invalidating a field invalidates entire structure. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57230/new/ https://reviews.llvm.org/D57230 ___ cfe-commi

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

2019-01-28 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I personally do use HTML output quite a lot (and we do have non-Xcode projects), and complex macros in HTML are currently totally unusable. I'm not sure whether this is a right approach to handling this, but it's definitely a problem for us. In general - thanks

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

2019-01-29 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. 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? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54438/new/ https

[PATCH] D57211: [analyzer] [RetainCountChecker] Support 'taggedRetain' and 'taggedRelease'

2019-01-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352530: [analyzer] [RetainCountChecker] Support 'taggedRetain' and 'taggedRelease' (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https:

[PATCH] D57201: [analyzer] [RetainSummaryManager] [NFC] Split one function into two, as it's really doing two things

2019-01-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352533: [analyzer] [RetainSummaryManager] [NFC] Split one function into two, as it's… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: http

[PATCH] D57344: Extend AnyCall to handle callable declarations without the call expressions

2019-01-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352531: Extend AnyCall to handle callable declarations without the call expressions (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https:

[PATCH] D57346: [analyzer] [ARCMT] [NFC] Unify entry point into RetainSummaryManager

2019-01-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352532: [analyzer] [ARCMT] [NFC] Unify entry point into RetainSummaryManager (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://revie

[PATCH] D57433: [analyzer] [RetainCountChecker] Bugfix for tracking top-level parameters of Objective-C methods

2019-01-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352588: [analyzer] [RetainCountChecker] Bugfix for tracking top-level parameters of… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https

[PATCH] D53902: RetainCountChecker: for now, do not trust the summaries of inlined code

2018-10-31 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC345746: [analyzer] RetainCountChecker: for now, do not trust the summaries of inlined… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: htt

[PATCH] D53982: Output "rule" information in SARIF

2018-11-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor nit: let's create a custom substitution for your diff command, like `diff_plist`. Otherwise LGTM. Comment at: test/Analysis/diagnostics/s

[PATCH] D53982: Output "rule" information in SARIF

2018-11-01 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif:10 }, - "length": 510, +

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

2018-11-02 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @Szelethus you can see the command required to get modules build in the mentioned log. I think -DLLVM_ENABLE_MODULES=On might be enough. Repository: rL LLVM https://reviews.llvm.org/D53277 ___ cfe-commits mailin

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

2018-11-02 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @Szelethus @NoQ OK it seems you need to annotate those headers in modulemap. E.g. take a look at: george1@/Volumes/Transcend/code/monorepo/llvm-project/clang (master)≻ rg SVals.def include/clang/module.modulemap 131: textual header "StaticAnalyzer/Core/P

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

2018-11-02 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @NoQ @Szelethus Actually pushed a fix. Repository: rL LLVM https://reviews.llvm.org/D53277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-11-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2369 ProgramStateRef state = C.getState(); - RegionStateTy RS = state->get(); + RegionStateTy OldRS = state->get(); RegionStateTy::Factory &F = state->get_context();

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

2018-11-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @lebedev.ri @erichkeane The test fails for me on macOS whenever asan and ubsan are both enabled. The failure is stack overflow at stack frame 943 (? maybe asan usage enforces lower stack size?) Repository: rC Clang https://reviews.llvm.org/D50050 ___

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

2018-11-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a subscriber: vsapsai. george.karpenkov added a comment. @lebedev.ri yeah ASAN is making stack frame size larger. It seems @vsapsai is working on disabling this test under ASAN. Repository: rC Clang https://reviews.llvm.org/D50050 ___

[PATCH] D54132: [CodeGenCXX] XFAIL test for ASAN on Darwin.

2018-11-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: clang/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:8 +// recursive template instantiation limit. +// XFAIL: darwin && asan + Do we actually want UNSUPPORTED here? We don't want to fail if ASAN stack usage

[PATCH] D54132: [CodeGenCXX] XFAIL test for ASAN on Darwin.

2018-11-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:8 +// recursive template instantiation limit. +// XFAIL: darwin && asan + --

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

2018-11-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added inline comments. Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:177 "no analyzer checkers are associated with '%0'">; -def note_suggest_disabling_all_checkers : Note< -"use -analyzer-disable-all-checks to disable all static analyzer ch

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

2018-11-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > Also, remove diags::note_suggest_disabling_all_checkers. Isn't that a separate revision? Given that removing existing options is often questionable, I'd much rather see this patch separated. Repository: rC Clang https://reviews.llvm.org/D54401

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

2018-11-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. From a first glance, it's missing screenshots and an introduction section. What do you propose we should do with other pages on the existing website? (OpenProjects / etc.) Repository: rC Clang https://reviews.llvm.org/D54429 _

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

2018-11-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I don't quite understand the need for this patch. If we are talking about a binary built from source, wouldn't it make more sense to build libcxx from source as well? If libcxx is built from source in the same repo, clang-based tools do find it. Repository: r

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

2018-11-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. I suspect it broke http://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/14090/console as well. Repository: rL LLVM https://reviews.llvm.org/D54310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

2018-11-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. In the future, for macOS-specific changes I think it would be better to wait for a sign-off from at least one maintainer who is an expert on Apple tools. Repository: rL LLVM https://reviews.llvm.org/D54310 ___ c

[PATCH] D54457: [AST] Generate unique identifiers for CXXCtorInitializer objects.

2018-11-12 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: lib/AST/DeclCXX.cpp:2249 +int64_t CXXCtorInitializer::getID(const ASTContext &Context) const { + Optional Out = Context.getAllocator().

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

2018-11-26 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. This looks more reasonable, thanks! What about just dropping the `Knowing` prefix? Just having `arr is null, taking true branch` seems considerably more readable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53076/new/ https://reviews.llvm.org/D53076

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

2018-11-26 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: test/Analysis/uninit-vals.m:222 testObj->origin = makeIntPoint(1, 2); - if (testObj->size > 0) { ; } // expected-note{{Takin

[PATCH] D55033: [analyzer] Add the type of the leaked object to the diagnostic message

2018-11-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC347943: [analyzer] Add the type of the leaked object to the diagnostic message (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://rev

[PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

2018-11-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC347949: [analyzer] RetainCountChecker: recognize that OSObject can be created directly… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: ht

[PATCH] D55041: [analyzer] Switch retain count checker for OSObject to use OS_* attributes

2018-11-29 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC347948: [analyzer] Switch retain count checker for OSObject to use OS_* attributes (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https:/

[PATCH] D55076: [analyzer] RetainCountChecker: recognize that OSObject can be created directly using an operator "new"

2018-11-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov marked an inline comment as done. george.karpenkov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp:483-497 case CE_Function: Summ = getFunctionSummary(cast(Call).getDecl()); break; case CE_CXXMember:

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

2018-12-03 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Thank you for the fix, but how far can the pattern matching go? Seems easy enough to think of cases not covered by the above. In any case, the fix looks good. Repository:

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

2018-12-05 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55321/new/ https://reviews.llvm.org/D55321 ___ cfe-commits mai

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-12-06 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added reviewers: dcoughlin, NoQ. george.karpenkov requested changes to this revision. george.karpenkov added a comment. Using `-Xclang` is the only way to pass options to the static analyzer, I don't think we should warn on it. CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-12-06 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. > Well,, that seems unfortunate if we have the only supported interface for the > static analyzer be an internal interface. Perhaps it can be given a different > option? Even discounting this change, I that seems like it would be > appropriate. Officially ther

[PATCH] D55155: [attributes] Add an attribute "os_consumes_this" with similar semantics to "ns_consumes_self"

2018-12-06 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348532: [attributes] Add an attribute os_consumes_this, with similar semantics to… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https:/

[PATCH] D55158: [analyzer] Rely on os_consumes_this attribute to signify that the method call consumes a reference for "this"

2018-12-06 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348533: [analyzer] Rely on os_consumes_this attribute to signify that the method call… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: htt

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

2018-12-07 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348638: [analyzer] Move out tracking retain count for OSObjects into a separate checker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: ht

[PATCH] D55385: [analyzer] RetainCountChecker: remove untested, unused, incorrect option IncludeAllocationLine

2018-12-07 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348637: [analyzer] RetainCountChecker: remove untested, unused, incorrect option… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://

[PATCH] D55465: Stop tracking retain count of OSObject after escape to void * / other primitive types

2018-12-07 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348675: Stop tracking retain count of OSObject after escape to void * / other primitive… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: h

[PATCH] D55528: [analyzer] Resolve another bug where the name of the leaked object was not printed properly

2018-12-10 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348819: [analyzer] Resolve another bug where the name of the leaked object was not… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https:

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

2018-12-10 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC348822: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.l

[PATCH] D54921: [analyzer] Remove memoization from RunLoopAutoreleaseLeakChecker

2018-12-11 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @NoQ thanks, interesting! But I thought the underlying map is not actually modified while the reference is alive? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54921/new/ https://reviews.llvm.org/D54921 __

[PATCH] D51821: [analyzer] Skip printing duplicate nodes, even if nodes have multiple predecessors/successors

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342308: [analyzer] Skip printing duplicate nodes, even if nodes have multiple… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://rev

[PATCH] D51822: Support generating unique identifiers for Stmt objects

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342309: Support generating unique identifiers for Stmt objects (authored by george.karpenkov, committed by ). Changed prior to commit: https://reviews.llvm.org/D51822?vs=164547&id=165625#toc Repository

[PATCH] D51824: StmtPrinter: allow customizing the end-of-line character

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL342311: StmtPrinter: allow customizing the end-of-line character (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org

[PATCH] D52113: Generate unique identifiers for Decl objects

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342315: Generate unique identifiers for Decl objects (authored by george.karpenkov, committed by ). Changed prior to commit: https://reviews.llvm.org/D52113?vs=165564&id=165631#toc Repository: rC Cla

[PATCH] D52114: [analyzer] Further printing improvements: use declarations

2018-09-14 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342316: [analyzer] Further printing improvements: use declarations, (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.o

[PATCH] D52133: [analyzer] A testing facility for testing relationships between symbols.

2018-09-17 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @NoQ Actually I agree with @baloghadamsoftware that it makes sense to have a separate test, as this functionality should be tested regardless of svalbuilder-rearrange-comparisons existence. https://reviews.llvm.org/D52133 ___

[PATCH] D52133: [analyzer] A testing facility for testing relationships between symbols.

2018-09-17 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:273 + + for (auto I : State->get()) { +SymbolRef Sym = I.first; ---

[PATCH] D52183: [analyzer] ExplodedGraph printing fixes

2018-09-17 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342413: [analyzer] ExplodedGraph printing fixes (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D52183?vs=165818&

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

2018-09-17 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. Thanks! The usual question: would it be easier to implement using AST matchers? Repository: rC Clang https://reviews.llvm.org/D51866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D51926: [scan-build-py] Prevent crashes of CTU analysis by suppressing warnings

2018-09-20 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. If that helps you, then sure. I'm not sure I understand why having warnings causes the collection process to fail, but I guess ultimately it's not important. Repository:

[PATCH] D52269: [analyzer] [NFC] Dead code removal

2018-09-21 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342765: [analyzer] [NFC] Dead code removal (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D52269?vs=166143&id=16

[PATCH] D52338: [analyzer] Process state in checkEndFunction in RetainCountChecker

2018-09-21 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342770: [analyzer] Process state in checkEndFunction in RetainCountChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews

[PATCH] D52337: [analyzer] Highlight sink nodes in red in exploded graph

2018-09-21 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL342769: [analyzer] Highlight sink nodes in red (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52337?vs=166400&

[PATCH] D52337: [analyzer] Highlight sink nodes in red in exploded graph

2018-09-21 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342769: [analyzer] Highlight sink nodes in red (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D52337?vs=166400&i

[PATCH] D52326: [analyzer] Associate diagnostics created in checkEndFunction with a return statement, if possible

2018-09-21 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342768: [analyzer] Associate diagnostics created in checkEndFunction with a return… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https:

[PATCH] D52338: [analyzer] Process state in checkEndFunction in RetainCountChecker

2018-09-21 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL342770: [analyzer] Process state in checkEndFunction in RetainCountChecker (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D52423: [analyzer] Make ConversionChecker load StdCLibraryFunctionsChecker

2018-09-24 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. The concept makes sense. @NoQ any comments? I don't recall seeing that pattern before. Comment at: test/Analysis/conversion.c:144 int isascii(int c); void falsePositive1() { char kb2[5]; Also the function name should be c

[PATCH] D52530: [analyzer] scan-build: if --status-bugs is passed, don't forget about the exit status of the actual build

2018-09-26 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment. @jroelofs Thanks, but in general code owners would need to take a look as well. @lebedev.ri The code looks great, thanks! Repository: rC Clang https://reviews.llvm.org/D52530 ___ cfe-commits mailing list cfe-com

<    1   2   3   4   5   6   >