Re: [PATCH] D13126: New static analyzer checker for loss of sign/precision

zaks.anna added a comment. Could you add the reduced false positives to the tests file? > As far as I see the diagnostics are showing the proper path now.. What do you mean? Does this refer to supplying more information the the path about why the error occurs? Comment at: li

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

zaks.anna added inline comments. Comment at: test/Analysis/MPIChecker.cpp:98 @@ +97,3 @@ + + MPI_Request req; + MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &req); // expected-note{{Request is previously used by nonblocking call here.}} ---

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:95 @@ +94,3 @@ + if (Optional SP = N->getLocation().getAs()) { +if (const CallExpr *CE = clang::dyn_cast(SP->getStmt())) { + Would it be possible to identi

Re: [PATCH] D14629: [analyzer] Configuration file for scan-build.

zaks.anna added a comment. Anton, Have you investigated if we can reuse code from clang-tidy? Also, the hope is that the python rewrite of scan-build will replace the current scan-build in the near future. Of cause, it still needs to be tested on Windows. I would really appreciate if you could

Re: [PATCH] D15888: [Analyzer] Change the default SA checkers for PS4

zaks.anna accepted this revision. This revision is now accepted and ready to land. Comment at: lib/Driver/Tools.cpp:3609 @@ -3602,2 +3608,3 @@ // Enable the following experimental checkers for testing. + if (!IsPS4CPU) { These are no longer experimen

Re: [PATCH] D15448: [analyzer] SVal Visitor.

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def:31 @@ +30,3 @@ +// is both instantiated and derived from. +// Additionally, its kind is not its name with "Kind" suffix, +// unlike all other regions. I'd rath

r256886 - [analyzer] Suppress reports coming from std::__independent_bits_engine

Author: zaks Date: Tue Jan 5 18:32:52 2016 New Revision: 256886 URL: http://llvm.org/viewvc/llvm-project?rev=256886&view=rev Log: [analyzer] Suppress reports coming from std::__independent_bits_engine The analyzer reports a shift by a negative value in the constructor. The bug can be easily trig

r256885 - [analyzer] Don't report null dereferences on address_space annotated memory

Author: zaks Date: Tue Jan 5 18:32:49 2016 New Revision: 256885 URL: http://llvm.org/viewvc/llvm-project?rev=256885&view=rev Log: [analyzer] Don't report null dereferences on address_space annotated memory Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp cfe/trunk/t

r256887 - [analyzer] Fix false warning about memory leak for QApplication::postEvent

Author: zaks Date: Tue Jan 5 18:32:56 2016 New Revision: 256887 URL: http://llvm.org/viewvc/llvm-project?rev=256887&view=rev Log: [analyzer] Fix false warning about memory leak for QApplication::postEvent According to Qt documentation Qt takes care of memory allocated for QEvent: http://doc.qt.i

Re: [PATCH] D15227: [analyzer] Valist checkers.

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:31 @@ +30,3 @@ +struct VAListAcceptingFunc { + mutable IdentifierInfo *II; + StringRef FuncName; It does not support ObjC methods. I think this is most useful to checker

Re: [PATCH] D15921: [analyzer] Utility to match function calls.

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:309 @@ -286,1 +308,3 @@ + /// \brief Returns true if the CallEvent is call to a function that matches + /// the CallDescription. "is call" -> "is a ca

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

zaks.anna added a comment. > This patch also fixes a bug in 'RangeSet::pin' causing single value ranges to > not be considered conventionally ordered. Can that fix be submitted as a separate patch? Is there a test for it? Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:35

Re: [PATCH] D15448: [analyzer] SVal Visitor.

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def:31 @@ +30,3 @@ +// is both instantiated and derived from. +// Additionally, its kind is not its name with "Kind" suffix, +// unlike all other regions. NoQ wrot

Re: [PATCH] D15611: [Patch 2/3]: Rebasing Ryan Govostes' STP patch for Clang SA

zaks.anna added a comment. What was the last outstanding issue with this patch (I believe it was related to either make or cmake changes)? Has it been addressed? Repository: rL LLVM http://reviews.llvm.org/D15611 ___ cfe-commits mailing list cf

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Sounds good. Please, split this into 2 patches (each fixing the separate problem) and commit. Thanks! Anna. http://reviews.llvm.org/D12901 __

Re: [PATCH] D16062: [analyzer] Rename kind-enumeration values of SVal, SymExpr, MemRegion classes, for consistency.

zaks.anna added a comment. > MemSpaceRegion is now an abstract base What prevents it from being instantiated? http://reviews.llvm.org/D16062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

Re: [PATCH] D15921: [analyzer] Utility to match function calls.

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312 @@ +311,3 @@ + /// calls. + bool isCalled(const CallEvent &Call, const CallDescription &CD); + The API is a bit awkward. Maybe it would be better if

Re: [PATCH] D9600: Add scan-build python implementation

zaks.anna accepted this revision. zaks.anna added a comment. Laszlo, I am very excited about having the new and much improved scan-build in tree! It will serve as a solid foundation for moving forward. Thank you for all your hard work! Anna. http://reviews.llvm.org/D9600 __

Re: [PATCH] D15448: [analyzer] SVal Visitor.

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM. Thank you! http://reviews.llvm.org/D15448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

Re: [PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

zaks.anna added a comment. > I see, so essentially you want to use a different approach for determining > sanitizer availability (on OS X for now): if the library is present, then we > support > sanitizer, otherwise we don't: i.e. the binary distribution is the source of > truth, not the lis

Re: [PATCH] D15624: Add iOS/watchOS/tvOS support for ASan (clang part)

zaks.anna updated this revision to Diff 45437. zaks.anna added a comment. Refactor the code to address Samsonov's review. http://reviews.llvm.org/D15624 Files: lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h test/Driver/fsanitize.c Index: test/Driver/fsanitize.c ===

Re: [PATCH] D15624: Add iOS/watchOS/tvOS support for ASan (clang part)

zaks.anna marked an inline comment as done. zaks.anna added a comment. I've introduced the helper function. Looks like addProfileRTLibs might be able to use it in the future (after support for autoconf is dropped) as well. http://reviews.llvm.org/D15624 __

Re: [PATCH] D15921: [analyzer] Utility to match function calls.

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM http://reviews.llvm.org/D15921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

Re: [PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

zaks.anna added a comment. > I don't know, is there a way to install runtime components for ASan if your > distribution doesn't happen to have one (that must be tricky, as the version > of ASan should match the version of the compiler). Correct, there is no recommended way of installing the

Re: [PATCH] D15624: Add iOS/watchOS/tvOS support for ASan (clang part)

zaks.anna updated this revision to Diff 45720. zaks.anna added a comment. Thanks for spotting the bug! The bug is fixed and the tests are added in this revision. http://reviews.llvm.org/D15624 Files: lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h test/Driver/darwin-sanitizer-ld.c te

r258591 - [analyzer] Fixup r258572 Utility to match function calls.

Author: zaks Date: Fri Jan 22 18:45:37 2016 New Revision: 258591 URL: http://llvm.org/viewvc/llvm-project?rev=258591&view=rev Log: [analyzer] Fixup r258572 Utility to match function calls. Initialize the IdentifierInfo pointer. Hope this fixes the buildbot breakage. Modified: cfe/trunk/inclu

Re: [PATCH] D15921: [analyzer] Utility to match function calls.

zaks.anna added a comment. Looks like the 'II' pointer wasn't initialized. Should be fixed by r258591. Repository: rL LLVM http://reviews.llvm.org/D15921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

r259221 - [analyzer] Improve Nullability checker diagnostics

Author: zaks Date: Fri Jan 29 12:43:15 2016 New Revision: 259221 URL: http://llvm.org/viewvc/llvm-project?rev=259221&view=rev Log: [analyzer] Improve Nullability checker diagnostics - Include the position of the argument on which the nullability is violated - Differentiate between a 'method' and

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

zaks.anna added a comment. Hi Alexander, Sorry for the delay! The patch should be rebased from the clang repo; for example, you could run "svn diff" from the clang directory. More comments inline. Comment at: tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td:75 @@ -74,1 +74

Re: [PATCH] D16063: [Analyzer] Use a wider integer type for an array index

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Looks like all of Gabor's comments were addressed. LGTM. Thank you! http://reviews.llvm.org/D16063 ___ cfe-commits mailing list cfe-commits

Re: [PATCH] D13488: [analyzer] Assume escape is possible through system functions taking void*

zaks.anna closed this revision. zaks.anna added a comment. Committed in r251449. http://reviews.llvm.org/D13488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14652: [analyzer] Improve modeling of static initializers.

zaks.anna added inline comments. Comment at: test/Analysis/inline.cpp:308 @@ +307,3 @@ +clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}} +clang_analyzer_eval(0 != ((char *)void_string)[1]); // expected-warning{{TRUE}} + } Why are we che

Re: [PATCH] D14779: Adding checker to detect excess padding in records

zaks.anna added a comment. > I may be mistaken, but this check looks more appropriate for Clang-tidy. This is a syntactic check. Both clang-tidy as well as the clang static analyzer contain this type of checks. If we move all syntactic checks to clang-tidy, the users that use the analyzer but

r253532 - [analyzer] Improve modeling of static initializers.

Author: zaks Date: Wed Nov 18 19:25:28 2015 New Revision: 253532 URL: http://llvm.org/viewvc/llvm-project?rev=253532&view=rev Log: [analyzer] Improve modeling of static initializers. Conversions between unrelated pointer types (e.g. char * and void *) involve bitcasts which were not properly mode

Re: [PATCH] D14652: [analyzer] Improve modeling of static initializers.

zaks.anna added a comment. Fixed and committed. Comment at: test/Analysis/inline.cpp:308 @@ +307,3 @@ +clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}} +clang_analyzer_eval(0 != ((char *)void_string)[1]); // expected-warning{{TRUE}} + } --

Re: [PATCH] D14629: [analyzer] Configuration file for scan-build.

zaks.anna added a comment. > > Also, how is this different from -analyzer-config? > > > -analyzer-config is used to transfers a number of options to the analyzer, > while configuration file is > used to customize scan-build, ccc-analyzer and c++-analyzer scripts. Could you elaborate on

Re: [PATCH] D14170: Fix false positive warning about memory leak for QApplication::postEvent

zaks.anna added inline comments. Comment at: test/Analysis/qt_malloc.cpp:11 @@ +10,3 @@ + QCoreApplication::postEvent(obj, event); + QApplication::postEvent(obj, event); +} Should the leak be reported when the object is passed to QApplication::postEvent? In

Re: [PATCH] D14170: Fix false positive warning about memory leak for QApplication::postEvent

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Thanks! LGTM. I'll commit. http://reviews.llvm.org/D14170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

Re: [PATCH] D14779: Adding checker to detect excess padding in records

zaks.anna added a comment. This is a partial review. I did not look at the padding calculations closely. Have you run this over codebases other than clang? Are there any false positives? > Even with the default of 8, this checker is too noisy to justify turning on > by default. Clang+LLVM has

Re: [PATCH] D14779: Adding checker to detect excess padding in records

zaks.anna added a comment. > I have seen plenty of structures where the specific layout was important and > couldn't be changed. Can you give specific examples of these? Can we develop heuristics for them? > These generally felt like noisy reports unless I had more specific > justification fo

Re: [PATCH] D14779: Adding checker to detect excess padding in records

zaks.anna added a comment. > can be suppressed without breaking ABI by adding explicit padding members. We should convey the suppression mechanism as part of the diagnostic. http://reviews.llvm.org/D14779 ___ cfe-commits mailing list cfe-commits@l

Re: [PATCH] D14629: [analyzer] Configuration file for scan-build.

zaks.anna added a comment. > And I really like how the clang-tidy guys were doing it. It would be great if the clang static analyzer config file would be similar to the clang-tidy one. We would avoid user confusion if they are consistent. Is some reuse possible? Currently, when clang-tidy call

Re: [PATCH] D14919: Fix IssueHash generation

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM. http://reviews.llvm.org/D14919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

Re: [PATCH] D14779: Adding checker to detect excess padding in records

zaks.anna added a comment. > An analysis of llvm+clang+compiler-rt now only generates 16 excessive padding > reports in index.html. Should those be fixed or are they all false positives? Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:48 @@ +47,3 @@ +// The cal

Re: [PATCH] D13731: [Analyzer] Supporting function attributes in .model files.

zaks.anna added a comment. Pierre, Have you seen the post about API Notes? http://llvm.cc/t/cfe-dev-clang-and-swift/331 I believe using API notes would be a better approach for adding annotations. By the time the static analyzer sees the AST, the annotations would already be there. The API Not

Re: [PATCH] D12726: [analyzer] A fix for symbolic element region index lifetime.

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM. (Feel free to add comments to the existing code!) > So the real question is whether (or rather how) the Store should maintain > correct region liveness information > after comp

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

zaks.anna added a comment. I've been mostly looking at the path sensitive checks. Maybe clang-tidy team would be interested in reviewing the syntactic checks. Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Container.h:31 @@ +30,2 @@ + +#endif A

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

zaks.anna added a comment. This patch contains a mix of checks, some are path sensitive and some are AST checks. See MPICheckerAST.cpp for the latter. http://reviews.llvm.org/D12761 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

Re: [PATCH] D9600: Add scan-build python implementation

zaks.anna added a comment. Hi Laszlo, Here are some comments from me. Should we be worried about the name conflicts (between old scan-build and this tool) during rollout? I think it would be beneficial to rename the tools, but let's discuss the names later. (If we integrate Codecheck, that wil

Re: [PATCH] D9600: Add scan-build python implementation

zaks.anna added a comment. Hi Laszlo, Comment at: tools/scan-build-py/libear/ear.c:281 @@ +280,3 @@ + +DLSYM(func, fp, "execve"); + This is not the recommended way of interposing on Darwin. All you need to do is provide your function, which can call the fun

Re: [PATCH] D9600: Add scan-build python implementation

zaks.anna added inline comments. Comment at: tools/scan-build-py/bin/analyze-c++:2 @@ +1,3 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# The LLVM Compiler Infrastructure Where/How is analyze-c++ used? Comment at: tools

Re: [PATCH] D9600: Add scan-build python implementation

zaks.anna added inline comments. Comment at: tools/scan-build-py/README.md:86 @@ +85,3 @@ +The 2. mode is available only on FreeBSD, Linux and OSX. Where library preload +is available from the dynamic loader. On OSX System Integrity Protection security +feature enabled prevents l

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

zaks.anna added inline comments. Comment at: test/Analysis/inlining/analysis-order.c:13 @@ +12,3 @@ + +// CHECK: analysis-order.c test2 +// CHECK: analysis-order.c test1 Can you use CHECK-NEXT instead? Repository: rL LLVM http://reviews.llvm.org/D15410 __

Re: [PATCH] D15370: [scan-view] replace deprecated optparse with argparse

zaks.anna added a comment. Overall, looks good! Comment at: tools/scan-view/bin/scan-view:94 @@ +93,3 @@ +import argparse +parser = argparse.ArgumentParser() +parser.add_argument("root", metavar="", type=str) Please, add the tool description, like "T

Re: [PATCH] D14779: Adding checker to detect excess padding in records

zaks.anna added a comment. With respect to the issues this checker found, I suggest submitting patches for the clang issues that can be fixed. Can the x-macro case be suppressed with the recommended suppression? I'd also submit a patch to gtest. Submitting patches with the fixes provides a good

Re: [PATCH] D14779: Adding checker to detect excess padding in records

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:49 @@ -48,1 +48,3 @@ +def Performance : Package<"performance">; + zaks.anna wrote: > I think Performance should be in the OptIn package. What do you think about this one? http

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

zaks.anna added a comment. I wonder if we can refactor the code so that it is less error prone.. shouldSkipFunction(D, Visited, VisitedAsTopLevel) works with two sets. I assume that you have not updated Decls coming from VisitedAsTopLevel because they come from the CFG and should already be can

Re: [PATCH] D15448: [analyzer] SVal Visitor.

zaks.anna added a comment. Can/Should something like this be used when dumping SVals (during debugging)? (Possibly in addition to the debug checker.) What are the advantages of implementing this using visitors? Can this be implemented similarly to SVal::dumpToStream? Do you envision other use ca

Re: [PATCH] D15448: [analyzer] SVal Visitor.

zaks.anna added a comment. Sorry, I forgot to read the description before commenting; I see it is intended to be used not only for debugging purposes:) http://reviews.llvm.org/D15448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

Re: [PATCH] D15227: [analyzer] Valist checkers.

zaks.anna added a comment. Looks good overall; comments below. Please, provide more information on real world code evaluation. Which codebases this has been tested on? What was the false positive rate? How many real bugs were found/fixed? What is the criteria for taking it out of alpha? Pleas

Re: [PATCH] D15448: [analyzer] SVal Visitor.

zaks.anna added a comment. Are you saying that we need to rename "SymbolValKind" to "SymbolKind"? That would probably be a tiny change. http://reviews.llvm.org/D15448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D15624: Add iOS/watchOS/tvOS support for ASan (clang part)

zaks.anna created this revision. zaks.anna added reviewers: kcc, cfe-commits. Change the clang driver to accept ASan on iOS/watchOS/tvOS. This change along with the corresponding changes in llvm and compiler-rt complete ASan support for iOS/watchOS/tvOS. http://reviews.llvm.org/D15624 Files:

Re: [PATCH] D15624: Add iOS/watchOS/tvOS support for ASan (clang part)

zaks.anna added inline comments. Comment at: lib/Driver/ToolChains.cpp:368 @@ +367,3 @@ + StringRef OS = ""; + if (isTargetMacOS()) +OS = "osx"; samsonov wrote: > Wait, this looks horrible. Can we teach toolchain to give us OS name? These are not OS names; t

<    1   2   3   4