[PATCH] D37806: [analyzer] PthreadLock: Fix return values of XNU lock functions.

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:271 } -assert(lockFail && lockSucc); -C.addTransition(lockFail); - +// We might want to handle the case when the mutex lock function was inlined +// and returned

[PATCH] D40668: [Blocks] Inherit sanitizer options from parent decl

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. LGTM. Thanks! I was wondering if there are other places where this propagation needs to be added, for example, other calls to GenerateBlockFunction. https://reviews.llvm.org/D40668

[PATCH] D36067: [analyzer] Create infrastructure for organizing and declaring analyzer configs.

zaks.anna added a comment. >> I tried to keep this as a minimal starting example because this currently >> blocks @yamaguchi 's GSoC project for bash completion. There we want to >> complete the values for -analyzer-config and we currently don't have a good >> way to get a complete list of av

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

zaks.anna added a comment. > What do you suggest? Should we widen the type of the difference, or abandon > this patch and revert back to the local solution I originally used in the > iterator checker? Does the local solution you used in the iterator checker not have the same problem? https:/

[PATCH] D27918: [analyzer] OStreamChecker

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/OStreamFormatChecker.cpp:513 + +bool OStreamFormatChecker::evalCall(const CallExpr *CE, +CheckerContext &C) const { gamesh411 wrote: > NoQ wrote: > > One

[PATCH] D158071: [clang] Remove rdar links; NFC

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Looks like @NoQ wetted the remaining code changes. The rest LGTM. Thank you for preparing the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message

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

[PATCH] D37437: [analyzer] Fix some checker's output plist not containing the checker name

zaks.anna added a comment. Just to be clear, since this leads to regression to the checker API, I am asking to look into other ways of solving this problem. For example, is there a way to ensure that the checker names are set at construction? https://reviews.llvm.org/D37437

[PATCH] D38844: [analyzer] Make issue hash related tests more concise

zaks.anna added a comment. Please, change the commit description to be more comprehensive. Comment at: lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:68 // (globals should not be invalidated, etc), hence the use of evalCall. - FnCheck Handler = llvm::StringSwitch(C.g

[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation

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

[PATCH] D39269: [Analyzer] [Tests] Do not discard output from CmpRuns.py when running integration tests

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. What kind of output will this start displaying? (I believe currently the script does print the summary of the objects that are added or deleted.) https://reviews.llvm.org/D39269 ___

[PATCH] D38844: [analyzer] Make issue hash related tests more concise

zaks.anna added a comment. I'd also include some info on how it's now possible to dump the issue hash. You introduce a new debugging function here "clang_analyzer_hashDump" but it's not mentioned in the commit message. Thanks! Comment at: lib/StaticAnalyzer/Checkers/ExprInsp

[PATCH] D39428: [Analyzer] As suggested, use value storage for BodyFarm

zaks.anna added a comment. > Also if you check the code snippets in the coding standard: > https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto > you can see that where we officially put the references. Correct! The reference should not go with the type name. George, p

[PATCH] D39964: Change code owner for Clang Static Analyzer to Devin Coughlin.

ndows codegen, ARM EABI -N: Anna Zaks -E: ga...@apple.com -D: Clang Static Analyzer - N: John McCall E: rjmcc...@apple.com D: Clang LLVM IR generation Index: CODE_OWNERS.TXT === --- CODE_OWNERS.TXT +++ CODE_OWNERS.TXT @@ -25,6 +

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

zaks.anna updated this revision to Diff 81429. zaks.anna added a comment. Devin did not like the '*' in the diagnostic for ObjC objects, so remove the '*'. https://reviews.llvm.org/D27740 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp test/Analysis/edges-new.mm test/Analysis/i

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

zaks.anna updated this revision to Diff 81482. zaks.anna added a comment. Address Devin's comment regarding 'id'. https://reviews.llvm.org/D27740 Files: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp test/Analysis/edges-new.mm test/Analysis/inlining/path-notes.m test/Analysis/objc-a

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

zaks.anna added a comment. > Did you checked if same warnings may be emitted by another checkers? For > example, > ArrayBoundChecker may warn if index is tainted. I second that. The GenericTaintChecker also reports uses of tainted values. It is not clear that we should add a new separate chec

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

zaks.anna added a comment. > I am doing it right now. Unfortunately I found a crash which I fixed, Is it fixed in this patch? > but then it turned out that overwrites of the iterator variable are not > handled. I am working on this > problem. My suggestion is to commit this patch and address

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

zaks.anna added a comment. And thank you for the awesome work and addressing the review comments!!! https://reviews.llvm.org/D25660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27710: [analyzer] Prohibit ExplodedGraph's edges duplicating

zaks.anna added a comment. Are there any negative effects from having the duplicates in edges? When does this occur? It's a bit suspicious since we have a FromN, and a path is split at that node, resulting in successors that are the same. Is it possible that whoever split the state did not enco

[PATCH] D27773: [analyzer] Add checker modeling gtest APIs.

zaks.anna added a comment. Looks great overall. I have minor comments below. Thanks for the awesome comments!!! Comment at: lib/StaticAnalyzer/Checkers/GTestChecker.cpp:152 + + ProgramStateRef State = C.getState(); + This could be moved up as you are using th

[PATCH] D27710: [analyzer] Prohibit ExplodedGraph's edges duplicating

zaks.anna added a comment. To deal with non-determinism, you can sort the results by non-pointer values, such as identifiers, before producing the warnings. It is not clear if you want to print all warnings or only the first one. Is it an option to list all dead symbols in one warning message?

[PATCH] D27710: [analyzer] Prohibit ExplodedGraph's edges duplicating

zaks.anna added a comment. If this is a common mistake for checker writers, we could consider adding assertions that check for this situation. We should make sure that we do not to add any release builds overhead. Maybe we could put this check behind NDEBUG or ensure that whatever code is added

[PATCH] D28330: [analyzer] Fix false positives in Keychain API checker

zaks.anna created this revision. zaks.anna added a reviewer: dcoughlin. zaks.anna added subscribers: cfe-commits, dergachev.a. The checker has several false positives that this patch addresses: 1. Do not check if the return status has been compared to error (or no error) at the time when leaks

[PATCH] D28330: [analyzer] Fix false positives in Keychain API checker

zaks.anna updated this revision to Diff 83160. zaks.anna added a comment. Addressed all comments https://reviews.llvm.org/D28330 Files: lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp test/Analysis/keychainAPI.m Index: test/Analysis/keychainAPI.m ===

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

zaks.anna added a comment. Thanks for the patch! Could you, please, resubmit the patch with context as described here http://llvm.org/docs/Phabricator.html Also, please, add tests. See test/Analysis/ for examples. Repository: rL LLVM https://reviews.llvm.org/D28348 __

[PATCH] D28330: [analyzer] Fix false positives in Keychain API checker

zaks.anna added a comment. I did not think of solution #1! It's definitely better than the pattern matching I've added here. However, this checker fires so infrequently, that I do not think it's worth investing more time into perfecting it. I suspect the solution #2 is what this checker was try

[PATCH] D28387: [tsan] Do not report errors in __destroy_helper_block_

zaks.anna created this revision. zaks.anna added a reviewer: kubabrecka. zaks.anna added a subscriber: cfe-commits. There is a synchronization point between the reference count of a block dropping to zero and it's destruction, which TSan does not observe. Do not report errors in the compiler-emi

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

zaks.anna added a comment. Phabricator still says that context is not available. Please, pass -U when generating the patch. Thanks! Anna Comment at: test/Analysis/gmalloc.c:1 +// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.Ca

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

zaks.anna added a comment. Thanks for working on this! Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:443 + if (auto LCV = Val.getAs()) +return C.getSymbolManager().getRegionValueSymbol(LCV->getRegion()); + This might create a new symbol.

[PATCH] D28495: [analyzer] Support inlining of '[self classMethod]' and '[[self class] classMethod]'

zaks.anna created this revision. zaks.anna added reviewers: dergachev.a, dcoughlin. zaks.anna added a subscriber: cfe-commits. https://reviews.llvm.org/D28495 Files: lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/inlining/InlineObjCClassMethod.m Index: test/Analysis/inlining/InlineObjCC

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

zaks.anna added a comment. Please, subscribe cfe-commits list on the patches as described in http://llvm.org/docs/Phabricator.html. Thanks! Anna Repository: rL LLVM https://reviews.llvm.org/D28297 ___ cfe-commits mailing list cfe-commits@lists.

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

zaks.anna added a comment. Are there other cases where makeNull would need to be replaced? Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width

zaks.anna added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46 -} // end GR namespace +bool isExprResultConformsComparisonRule(CheckerContext &C, +BinaryOperatorKind CompRule,

[PATCH] D31982: [analyzer] Improve suppression for inlined defensive checks when operator& is involved.

zaks.anna added a comment. Thanks!!! Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:965 + +// Performing operator `&' on an lvalue expression is essentially a no-op. +// Then, if we are taking addresses of fields or elements, these are also

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

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. This needs to be committed. https://reviews.llvm.org/D27090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D27773: [analyzer] Add checker modeling gtest APIs.

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

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

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:810 +if (CE->getNumArgs() == 2) + State = ProcessZeroAllocation(C, CE, 1, State); } else if (CE->getNumArgs() == 3) { Why did you remove the old beh

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

zaks.anna added a comment. I think it's more valuable to report a warning here even if the error message is not very precise. Marking something as in bounds when we know it's not does not feel right and could lead to inconsistent states down the road. Repository: rL LLVM https://reviews.llv

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

zaks.anna added a comment. From what I recall, it is not clear that this patch is the step in the right direction. At least, it need more investigation. https://reviews.llvm.org/D27202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D28765: CStringChecker can crash when uninitialized checks are disabled

zaks.anna added a comment. > It is not supported to run the analyzer with some of the core checkers turned > off. Correct. > Maybe we should change the behavior such that turning off core checkers turn > off the warnings from those checkers but not the checkers themselves? Having this as the

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

zaks.anna added a comment. Thanks for working on this Dominic!!! Can you talk a bit about your motivation for working on this project and what your goals are? Have you compared the performance when using Z3 vs the current builtin solver? I saw that you mention performance issues on large codeb

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

zaks.anna added a comment. The static analyzer is definitely the place to go for bug detection that requires path sensitivity. It's also reasonably good for anything that needs flow-sensitivity. Although, most flow-sensitive analysis (such as liveness) now live in lib/Analysis/, which is used b

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

zaks.anna added a comment. Before clang-tidy came into existence the guidelines were very clear. One should write a clang warning if the diagnostic: - can be implemented without path-insensitive analysis (including flow-sensitive) - is checking for language rules (not specific API misuse) In m

[PATCH] D29151: [clang-tidy] Add misc-invalidated-iterators check.

zaks.anna added a comment. > I tried to come up with some advice on where checks should go in > http://clang.llvm.org/extra/clang-tidy/#choosing-the-right-place-for-your-check The guidelines stated on the clang-tidy page seem reasonable to me. > For example, IMO, AST-based analyses make more se

[PATCH] D16403: Add scope information to CFG

zaks.anna added a comment. > @dcoughlin As a reviewer of both patches - could you tell us what's the > difference between them? And how are we going to resolve this issue? Unfortunately, @dcoughlin is on vacation this week; should be back next week. Repository: rL LLVM https://reviews.llvm.

[PATCH] D34508: [Analyzer] Bug Reporter Visitor to Display Values of Variables - PRELIMINARY!

zaks.anna added a comment. I agree that we should not print the values of all variables. The users will be overwhelmed with the huge amount of information. It is very valuable to highlight just the right information. (I believe even the current diagnostics often produce too much info and highli

[PATCH] D15227: [analyzer] Valist checkers.

zaks.anna added a comment. @xazax.hun, Can we move this out of alpha? Have this checkers been tested on a large codebase? What are false positive rates? Thanks! Anna Repository: rL LLVM https://reviews.llvm.org/D15227 ___ cfe-commits mailing

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

zaks.anna added a comment. Please, make sure future reviews go through cfg-dev list. See http://llvm.org/docs/Phabricator.html. Repository: rL LLVM https://reviews.llvm.org/D28297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

zaks.anna added a comment. Does the code you added detects array out of bounds cases without false positives? Is it an option to just have this checkers produce a more precise error message in the specific case. A lot of work will probably need to be done to implement a proper array out of bou

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

zaks.anna added a comment. > In this checker, I give warnings for values which are both tainted and were > also not checked by the programmer. So unlike GenericTaintChecker, I do > implement the boundedness check here for certain, interesting constructs > (which is controlled by the critical op

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

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:885 +return; + State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State); + State = ProcessZeroAllocation(C, CE, 0, State); I am not sure this is

[PATCH] D29643: [analyzer] Do not duplicate call graph nodes for function that has definition and forward declaration.

zaks.anna accepted this revision. zaks.anna added a comment. This revision is now accepted and ready to land. Looks good. Thank you for catching this! Do you have commit access or should I commit on your behalf? https://reviews.llvm.org/D29643 ___

[PATCH] D29303: In VirtualCallChecker, handle indirect calls

zaks.anna added a comment. Has this been cherry-picked into the clang 4.0 release branch? If not, we should definitely do that! Repository: rL LLVM https://reviews.llvm.org/D29303 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D15227: [analyzer] Valist checkers.

zaks.anna added a comment. > But as far as I remember, this produced false negatives in the tests not > false positives. Could you double check that? Maybe you still have some notes in your mail box or just by looking at the code. Did none of the checks work or just some of them? Also, whic

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

zaks.anna added a comment. Could you please split the patch into two - one with the previously reviewed support for functions that take a single size value and another patch that models the two size arguments (num and size). It's easier to review patches if they do not grow new functionality. S

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

zaks.anna added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:76 if (Ex) { + bool ArrayIndexOutOfBounds = false; + if (isa(Ex)) { Please, pull this out into a sub-rutine. Thanks! https://reviews.llvm.org/D28278

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

zaks.anna added a comment. I agree this can clarify the error message quite a bit! Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:160 if (ParamDecl->getType()->isPointerType()) { -Message = "Function call argument is a pointer to uninitialized value";

[PATCH] D30289: [Analyzer] Add bug visitor for taint checker

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

[PATCH] D29419: [Analyzer] Checker for mismatched iterators

zaks.anna added a comment. > So it would be a wast of resources to duplicate these data. So now I am > also working on the merged version. Would it make sense to just resume the review on the merged patch? https://reviews.llvm.org/D29419 ___ cfe

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

zaks.anna added a comment. > Firstly I uploaded Glib-MallocChecker-single-size-value.patch for code > review, if submitted to UPSTREAM, then upload another one, correct? Yes. By the way, you can model XXX_n variants similarly to how calloc is modeled (see CallocMem). Comment

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

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/D28278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

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

zaks.anna added a comment. No multi-file support is a long outstanding limitation of scan-build html output. Great to see the patch!! Thank you for working on it! > It's not as immediately clear this is a multi-file output. In addition to Artem's suggestions, you might want to insert multiple

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

zaks.anna added a comment. Gábor's suggestion sounds good to me. I think ArrayBoundCheckerV2 checker has a higher chance to be productized / moved out of alpha in the future. Should we just remove ArrayBoundChecker.cpp or is there a value in keeping it around? Repository: rL LLVM https://r

[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

<    1   2   3   4   5   6   >