[PATCH] D30341: [analyzer] clarify error messages about uninitialized function arguments

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:211 // Generate a report for this bug. - StringRef Desc = - describeUninitializedArgumentInCall(Call, IsFirstArgument); + std::string Desc = + des

[PATCH] D30157: [analyzer] Improve valist check

zaks.anna added inline comments. Comment at: test/Analysis/valist-uninitialized-no-undef.c:25 + va_list va; + vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is ca

[PATCH] D28348: [analyzer] Taught the analyzer about Glib API to check Memory-leak

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. > I am not clear why need to calculate the precise allocated size? The information generated by this checker is used for array bounds checking. For example, see https://reviews.llvm.org/

[PATCH] D30406: [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

zaks.anna added a comment. > I’ve added the single file output option but I would like to keep the > multi-file option default This sounds good to me! I agree that this is a very useful addition. https://reviews.llvm.org/D30406 ___ cfe-commits m

[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Thanks. looks good. https://reviews.llvm.org/D28297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D27092: Add RecursionChecker for finding infinite recursion

zaks.anna added a comment. This is waiting for a resolution on a CallEvent API patch as described in https://reviews.llvm.org/D27091. This is blocked on https://reviews.llvm.org/D27091. Comment at: lib/StaticAnalyzer/Checkers/RecursionChecker.cpp:29 +// this patch. +REGISTER_

[PATCH] D30489: [analyzer] catch out of bounds for VLA

zaks.anna requested changes to this revision. zaks.anna added a comment. This revision now requires changes to proceed. Following Gabor's suggestion, we should investigate if ArrayBoundCheckerV2 supports this. If not it's possible that we are hitting the Constraint Solver limitations. Reposito

[PATCH] D28445: [Analyzer] Extend taint propagation and checking

zaks.anna added a comment. @vlad.tsyrklevich, Do you have commit access or should we commit on your behalf? https://reviews.llvm.org/D28445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D22862: [analyzer] Fix for PR15623: eliminate unwanted ProgramState checker data propagation.

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Thi has been committed in r290505. https://reviews.llvm.org/D22862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D27753: [analyzer] alpha.security.DirtyScalar Checker

zaks.anna added a comment. > All this leads to the need of several types of taintednesses (string > taintedness, array taintedness, general bound check taintedness) because the > cleansing can only take down one type of taintedness at a time. That would be > the only way for a checker to be abl

[PATCH] D30762: [ubsan] Add a nullability sanitizer

zaks.anna added inline comments. Comment at: docs/UndefinedBehaviorSanitizer.rst:101 + ``-fsanitize=nullability-assign``, and the argument check with + ``-fsanitize=nullability-arg``. While violating nullability rules does + not result in undefined behavior, it is oft

[PATCH] D30798: [analyzer] Turn suppress-c++-stdlib on by default

zaks.anna created this revision. We have several reports of false positives coming from libc++. For example, there are reports of false positives in std::regex, std::wcout, and also a bunch of issues are reported in https://reviews.llvm.org/D30593. In many cases, the analyzer trips over the com

[PATCH] D30593: Add correct "-isystem"/"-isysroot" warning handling to static analysis' BugReporter.

zaks.anna added a comment. @kmarshall, We are going to turn this off by default, see https://reviews.llvm.org/D30798. Please, do file a bug that lists all the issues you are seeing and desirably instructions on how to reproduce. (Please, list even the cases you are not 100% sure are false posi

[PATCH] D30798: [analyzer] Turn suppress-c++-stdlib on by default

zaks.anna added a comment. I've committed the change, but would very much appreciate community feedback here if if there is any! Repository: rL LLVM https://reviews.llvm.org/D30798 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D30762: [ubsan] Add a nullability sanitizer

zaks.anna added inline comments. Comment at: docs/UndefinedBehaviorSanitizer.rst:101 + ``-fsanitize=nullability-assign``, and the argument check with + ``-fsanitize=nullability-arg``. While violating nullability rules does + not result in undefined behavior, it is oft

[PATCH] D30593: Add correct "-isystem"/"-isysroot" warning handling to static analysis' BugReporter.

zaks.anna added a comment. Thank you! Repository: rL LLVM https://reviews.llvm.org/D30593 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30798: [analyzer] Turn suppress-c++-stdlib on by default

zaks.anna added a comment. Correct, this will suppress some valid warnings that occur due to user errors and valid warnings coming from the standard library. However, I believe this is the right choice right now since we know that the analyzer is not currently effective in understanding invaria

[PATCH] D26768: [analyzer] Improve VirtualCallChecker diagnostics and move out of alpha

zaks.anna added a comment. Not sure if we should make pure vs not an option so that users could turn the checking off. Is there a way to suppress the warning? Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:210 + if (isPure) +os << "pure "; + --

[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx

zaks.anna added a comment. Hi! Looks like this this is used by the Infinite recursion checker. Specifically, the checker not only needs to get Smalls for arguments of the current CallEvent, but it also looks for arguments of other calls on the stack. The checker walks the LocationContext and u

[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx

zaks.anna added a comment. > Hmm, i'm thinking of just the opposite - refactor the CallEvent methods to > use the respective new ProgramState methods. > > This way we'd avoid touching the CallEvent allocator for a simple Environment > lookup, while still avoiding code duplication. I see what

[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx

zaks.anna added a comment. Artem just pointed out that I have "Smalls" instead of "SVals" in my first comment. https://reviews.llvm.org/D27091 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D27090: Add LocationContext as a parameter to checkRegionChanges

zaks.anna added a comment. Thank you for doing this!!! Artem, do you want to disallow the creation of successors from checkRegionChanges callback or can this go in as is? Anna. Comment at: lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp:240 - /// #checkRegionChanges wh

[PATCH] D27043: Remove assertion on analysis of rvalue vector

zaks.anna added a comment. Thank you! The assert definitely needs to be relaxed. Would be great to add test cases that check if the analyzer models the vector types correctly, not just that we do not crash. (Not a blocker, but would be very useful.) https://reviews.llvm.org/D27043

[PATCH] D26837: [analyzer] Litter the SVal/SymExpr/MemRegion class hierarchy with asserts.

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:23 #include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" It's a bit sad to

[PATCH] D26837: [analyzer] Litter the SVal/SymExpr/MemRegion class hierarchy with asserts.

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:737 /// either a real region, a NULL pointer, etc. It essentially is used to /// map the concept of symbolic values into the domain of regions. Symbolic /// regions do

[PATCH] D26762: Add a method to obtain this SVal of a method that created given StackFrameCtx

zaks.anna added a comment. I see. You are handling more types of calls with this API! However, Similarly to the comment in the related patch, I think we should reuse the logic from CallEvent. https://reviews.llvm.org/D26762 ___ cfe-commits mailing

[PATCH] D26694: [analyzer] Drop explicit mention of range constraint solver

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Awesome!!! Thanks for the cleanup! https://reviews.llvm.org/D26694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D26691: [analyzer] Run clang-format and fix style

zaks.anna added a comment. I am not a big fan of loosing svn blame only to fix formatting, but since you are modifying this code anyway, it's fine by me. Artem and Devin, what is your opinion on this? Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:459 // No

[PATCH] D26768: [analyzer] Improve VirtualCallChecker diagnostics and move out of alpha

zaks.anna accepted this revision. zaks.anna added a comment. Looks great! Thank you. https://reviews.llvm.org/D26768 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26691: [analyzer] Run clang-format and fix style

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:459 // Notice that the lower bound is greater than the upper bound. - RangeSet New = GetRange(St, Sym).Intersect(getBasicVals(), F, Upper, Lower); + RangeSet New = getRange(St, Sy

[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

zaks.anna added a comment. It's awesome to see that all the major issues have been addressed. Thank you for working on this and diligently working through the code review!!! I have a few minor comments below. Could you add this example yours as a "no-warning" test case: const auto start = v.beg

[PATCH] D27408: [analyzer] RetainCountChecker: remove unused enum value; NFC.

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. I suspect allocated owned means that the analyzer saw the allocation site. Removing dead code is great! Thanks. This looks good to me other than the name of the method that I commented a

[PATCH] D27409: [analyzer] RetainCountChecker: The callback in dispatch_data_create() doesn't free the return symbol.

zaks.anna added inline comments. Comment at: test/Analysis/dispatch-data-leak.m:50 + + malloc_buf = malloc(1024); + data = dispatch_data_create(buf, 1024, dispatch_get_main_queue(), ^{}); // expected-warning{{Potential leak of memory pointed to by 'malloc_buf'}} --

[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

zaks.anna added a comment. Also, have you evaluated this on real codebases? What results do you see? Are there any false positives found? Are there any true positives found? https://reviews.llvm.org/D25660 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D27408: [analyzer] RetainCountChecker: remove unused enum value; NFC.

zaks.anna added a comment. LGTM. https://reviews.llvm.org/D27408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp:721 + +static bool inTopLevelNamespace(const Decl *D, IdentifierInfo *II) { + const auto *ND = dyn_cast(D->getDeclContext()); zaks.anna wrote: > Would isInStdName

[PATCH] D27599: [analyzer] Teach the analyzer that pointers can escape into __cxa_demangle

zaks.anna created this revision. zaks.anna added a reviewer: dergachev.a. zaks.anna added subscribers: dcoughlin, cfe-commits. This fixes a reported false positive in the malloc checker. https://reviews.llvm.org/D27599 Files: lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/Inputs/system

[PATCH] D27600: [analyzer] Refine the diagnostics in the nullability checker to differentiate between nil and null

zaks.anna created this revision. zaks.anna added a reviewer: dcoughlin. zaks.anna added subscribers: dergachev.a, cfe-commits. This is a big deal for ObjC, where nullability annotations are extensively used. I've also changed "Null" -> "null" and removed "is" as this is the pattern that Sema is

[PATCH] D27600: [analyzer] Refine the diagnostics in the nullability checker to differentiate between nil and null

zaks.anna added a comment. Looks like Sema uses "null" not only when referring to literals (see below). Also, if we were referring to literals, we would use single quotes, no? I suggest keeping as is for consistency with the wording that uses nil. I do not see much difference between the two ca

[PATCH] D25660: [Analyzer] Checker for iterators dereferenced beyond their range.

zaks.anna added a comment. Thanks Artem! Just to be clear, I think this patch should be committed once "inTopLevelNamespace" issue is addressed. That is the only issue pending as far as I can see. The visitor should be a separate patch. https://reviews.llvm.org/D25660

[PATCH] D27607: [ubsan] Treat ObjC's BOOL as if its range is always {0, 1}

zaks.anna added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:1222 +/// modes, it's safe to treat such a type as 'the builtin bool'. +static bool isObjCBool(QualType Ty, const SourceManager &SM, + const LangOptions &LO) { Could you use

[PATCH] D27600: [analyzer] Refine the diagnostics in the nullability checker to differentiate between nil and null

zaks.anna updated this revision to Diff 80925. zaks.anna added a comment. Updated "null"-> "Null" as per Devin's suggestion. https://reviews.llvm.org/D27600 Files: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp test/Analysis/nullability-no-arc.mm test/Analysis/nullability.mm test/An

[PATCH] D27607: [ubsan] Treat ObjC's BOOL as if its range is always {0, 1}

zaks.anna accepted this revision. zaks.anna added a reviewer: zaks.anna. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D27607 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D27535: [analyzer] Add ObjCPropertyChecker - check for autosynthesized copy-properties of mutable types.

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:494 +def ObjCPropertyChecker : Checker<"ObjCProperty">, + HelpText<"Find various issues with Objective-C properties">, + DescFile<"ObjCPropertyChecker.cpp">; Nit: may

[PATCH] D27202: [analyzer] Do not conjure a symbol for return value of a conservatively evaluated function

zaks.anna added a comment. > Could you please remove the IteratorPastEndChecker file differences from this > patch and make https://reviews.llvm.org/D25660 > dependent on this one? Since this has not been reviewed yet while the IteratorPastEndChecker has been, I do not think we should block t

[PATCH] D27091: Add the way to extract SVals of arguments used in a call for a given StackFrameCtx

zaks.anna added a comment. Update: Adding support for top frame to the CallEvent is difficult, so let's just use CallEvent API in the checkers for all frames but the top one and have custom top frame handling in the Recursion checker itself. The top frame handling does not need to be complete (

[PATCH] D26691: [analyzer] Run clang-format and fix style

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Thank you! Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:459 // Notice that the lower bound is greater than the upper bound. - RangeSet New = GetRa

[PATCH] D27726: [analyzer] Refer to macro names in diagnostics for macros representing a literal

zaks.anna created this revision. zaks.anna added reviewers: dergachev.a, dcoughlin. zaks.anna added a subscriber: cfe-commits. When a macro expending to a literal is used in a comparison, use the macro name in the diagnostic rather than the literal. This improves readability of path notes. Adde

[PATCH] D27740: [analyzer] Include type name in Retain Count Checker diagnostics

zaks.anna created this revision. zaks.anna added a reviewer: dcoughlin. zaks.anna added subscribers: cfe-commits, dcoughlin. The more detailed diagnostic will make identifying which object the diagnostics refer to easier. https://reviews.llvm.org/D27740 Files: lib/StaticAnalyzer/Checkers/Ret

[PATCH] D31975: [Analyzer] Iterator Checkers

zaks.anna added a comment. @baloghadamsoftware Thanks for working on this! However, this patch is getting too big. Could you, please, split it into incremental patches so that it would be easier to review? More motivation of why this is very important is here http://llvm.org/docs/DeveloperPol

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

zaks.anna added a comment. I agree that scan-build or scan-build-py integration is an important issue to resolve here. What I envision is that users will just call scan-build and pass -whole-project as an option to it. Everything else will happen automagically:) Another concern is dumping the A

[PATCH] D32328: [analyzer] Fix assert in ExprEngine::processSwitch

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Thank you! Repository: rL LLVM https://reviews.llvm.org/D32328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D32328: [analyzer] Fix assert in ExprEngine::processSwitch

zaks.anna added a comment. Do you have commit access? Repository: rL LLVM https://reviews.llvm.org/D32328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32291: [analyzer] Implement handling array subscript into null pointer, improve null dereference checks for array subscripts

zaks.anna added inline comments. Comment at: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:80 } -else if (isDeclRefExprToReference(E)) { +else if (isa(E)) { return E; Not sure what this does, but looks like we are stricter here now

[PATCH] D32702: [analyzer] Fix 'Memory Error' bugtype capitalization.

zaks.anna accepted this revision. zaks.anna added a comment. Thanks! https://reviews.llvm.org/D32702 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32437: [analyzer] Nullability: fix a crash when adding notes inside a synthesized property accessor.

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. > These new "extra notes" of mine might be useful (we could put them at > property declaration), i could add them if everybody likes this idea. What are these? Is there a PR? It would b

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

zaks.anna added a comment. Sorry for the delay!!! Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:180 + inline const llvm::APSInt &getZeroWithTypeSize(QualType T, + bool isUnsigned = true) { +

[PATCH] D32437: [analyzer] Nullability: fix a crash when adding notes inside a synthesized property accessor.

zaks.anna added a comment. > That wouldn't work this way because we'd have the completely redundant > "calling property accessor" piece before that, and "returning..." after that. I think we should not print "calling" and "returning" for calling into and returning from autogenerated code, If we

[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.

zaks.anna added a comment. Thank you for the patch! Could you please re-submit the patch with context? Instructions on how to do that can be found here: http://llvm.org/docs/Phabricator.html Repository: rL LLVM https://reviews.llvm.org/D32449 __

[PATCH] D33263: [scan-build] Patch to scan-build tool to support "--target=" flag

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Looks good. Thank you! Do you have commit access or should we commit? https://reviews.llvm.org/D33263 ___ cfe-commits mailing list cfe-com

[PATCH] D33191: [analyzer] ObjCGenerics: account for __kindof specifiers met along a chain of casts.

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:464 ASTContext &C) { + if (hasKindofArgs(StaticUpperBound)) { +// If the upper bound type has more __kindof specs, we drop all the info,

[PATCH] D32437: [analyzer] Nullability: fix a crash when adding notes inside a synthesized property accessor.

zaks.anna added a comment. Thanks! A couple of minor comments below. Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:699 + // We cannot place diagnostics on autosynthesized code. + // Put them onto the call site through which we jumped into autosynthesized + // code f

<    1   2