[PATCH] D52585: [analyzer] [testing] Pass through an extra argument for specifying extra analyzer options

This revision was automatically updated to reflect the committed changes. Closed by commit rC343158: [analyzer] [testing] Pass through an extra argument for specifying extra… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://

[PATCH] D52519: [analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue

This revision was automatically updated to reflect the committed changes. Closed by commit rC343159: [analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.o

[PATCH] D52519: [analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue

This revision was automatically updated to reflect the committed changes. Closed by commit rL343159: [analyzer] [NFC] Heavy refactoring of trackNullOrUndefValue (authored by george.karpenkov, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.

[PATCH] D52583: [analyzer] [NFC] Move the code for dumping the program point to ProgramPoint

This revision was automatically updated to reflect the committed changes. Closed by commit rC343160: [analyzer] [NFC] Move the code for dumping the program point to ProgramPoint (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https

[PATCH] D52584: [analyzer] Highlight nodes which have error reports in them in red in exploded graph

This revision was automatically updated to reflect the committed changes. Closed by commit rC343239: [analyzer] Highlight nodes which have error reports in them in red in exploded… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: ht

[PATCH] D52637: [analyzer] Provide an option to dump generated exploded graphs to a given file.

This revision was automatically updated to reflect the committed changes. Closed by commit rC343352: [analyzer] Provide an option to dump generated exploded graphs to a given file. (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: ht

[PATCH] D52640: [analyzer] [NFC] Remove unused parameters, as found by -Wunused-parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rC343353: [analyzer] [NFC] Remove unused parameters, as found by -Wunused-parameter (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiDiagnostics to take Preproc as parameter

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Please reupload with context (-U999) Repository: rC Clang https://reviews.llvm.org/D52735 ___ cfe-commits mailin

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter

george.karpenkov accepted this revision. george.karpenkov added inline comments. This revision is now accepted and ready to land. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:98 + + const SourceManager &SM = PP.getSourceManager(); + const LangOptions &LangOpts = PP.

[PATCH] D52735: [analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter

george.karpenkov added a comment. Sorry no --- going through those while doing archaeology sucks. You could reformat your changed lines though. https://reviews.llvm.org/D52735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D52756: [analyzer] Fix crash in exploded graph dumping

This revision was automatically updated to reflect the committed changes. Closed by commit rC343635: [analyzer] Fix crash in exploded graph dumping (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D52756?vs=

[PATCH] D52801: [analyzer] [tests] Allow specifying entire -analyze-config on the command line, make sure it's always propagated

This revision was automatically updated to reflect the committed changes. Closed by commit rC343636: [analyzer] [tests] Allow specifying entire -analyze-config on the command line… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: h

[PATCH] D52804: [analyzer] NFC: RetainCountChecker: Avoid dumping symbols during normal operation.

george.karpenkov added a comment. This should also prevent leaks. Repository: rC Clang https://reviews.llvm.org/D52804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52848: [analyzer] Do not crash if the assumption added in TrustNonNullChecker is enough to make the state unfeasible

This revision was automatically updated to reflect the committed changes. Closed by commit rC343735: [analyzer] Do not crash if the assumption added in TrustNonNullChecker is… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https:/

[PATCH] D52853: [Analyzer] Try fixing ConstraintManager::assumeDual

george.karpenkov added a comment. Tracked in rdar://44992170. Marking the test as unsupported for now. Repository: rC Clang https://reviews.llvm.org/D52853 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

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

george.karpenkov added a comment. Checking for asserts makes sense, but as a rough estimate will suppress roughly zero false positives I have seen. In general, LLVM codebase is exceptional in its use of asserts, and most projects, unfortunately, don't really know how to use them.

[PATCH] D57782: [analyzer] [RetainCountChecker] Bugfix: in non-OSObject-mode, do not track CXX method calls

This revision was automatically updated to reflect the committed changes. george.karpenkov marked an inline comment as done. Closed by commit rC353227: [analyzer] [RetainCountChecker] Bugfix: in non-OSObject-mode, do not track CXX… (authored by george.karpenkov, committed by ). Herald added a pro

[PATCH] D54978: Move the SMT API to LLVM

george.karpenkov added a comment. @mikhail.ramalho could you revert then? In general, we should not use Z3 unless it's explicitly requested. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54978/new/ https://reviews.llvm.org/D54978 __

[PATCH] D54978: Move the SMT API to LLVM

george.karpenkov added a comment. > There is at least one other conflicting commit rL353465 > on top of this code already. Sorry about that. I think it would be fine to revert both or to replay the other commit on top of reverted version of this one. @mikhai

[PATCH] D57261: [analyzer] [WIP] Opt-in C Style Cast Checker for OSObject pointers

This revision was automatically updated to reflect the committed changes. Closed by commit rC353566: [analyzer] Opt-in C Style Cast Checker for OSObject pointers (authored by george.karpenkov, committed by ). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to

[PATCH] D58065: [analyzer] Document the frontend library

george.karpenkov added a comment. High-level feedback: mixing of abstraction levels is wrong for the "bundled" documentation. This might also work better as a blogpost, if you want to jump from topic to topic. Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:11-13

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

george.karpenkov added a comment. I don't, but I can check whether tests pass. https://reviews.llvm.org/D37156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer

george.karpenkov added a comment. @kcc I've disabled the relevant test on Mac in r311916, please revert my change once this CR goes through. https://reviews.llvm.org/D37156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

george.karpenkov added a comment. This is amazing, huge thanks for doing this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55792/new/ https://reviews.llvm.org/D55792 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D55823: [analyzer] Fix backward compatibility issue after D53280 'Emit an error for invalid -analyzer-config inputs'

george.karpenkov added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:3700 + // through them. + for (size_t Index = 0; Index < Args.size(); ++Index) { +if (StringRef(Args.getArgString(Index)).contains("-analyzer-config")) { NoQ wrote: > Needs

[PATCH] D55907: [analyzer] RetainCount: Bluntly suppress the CFRetain detection heuristic on a couple of CM functions.

george.karpenkov added a comment. In general, I'm against hardcoding, and I think we should use the annotations indicating that the function does retain. (or use a naming convention, and use an annotation to indicate that the function does not retain). However, if this is prohibitive for some r

[PATCH] D55976: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for inlined functions

This revision was automatically updated to reflect the committed changes. Closed by commit rC349876: [analyzer] Perform escaping in RetainCountChecker on type mismatch even for… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https

[PATCH] D55978: [gn build] Add build files for clang/lib/{ASTMatchers, CrossTU}, clang/lib/StaticAnalyzer/{Checkers, Core, Frontend}

george.karpenkov added a comment. Looks reasonable, what about linking with Z3? Or is your goal just to get a minimally working functionality? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55978/new/ https://reviews.llvm.org/D55978 ___ cfe

[PATCH] D55978: [gn build] Add build files for clang/lib/{ASTMatchers, CrossTU}, clang/lib/StaticAnalyzer/{Checkers, Core, Frontend}

george.karpenkov added a comment. > I might want to default to clang_enable_static_analyzer=false when I add the > clang/lib/FrontendTool build file. I don't think that quite makes sense, since by default clang does have the analyzer built in. CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D55875: [analyzer] pr38668: RegionStore: Do not attempt to cast loaded values of non-scalar types.

george.karpenkov accepted this revision. george.karpenkov added a comment. Seems good, but a comment explaining why this is necessary and why the crash follows otherwise would be great. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55875/new/ https://reviews.llvm.org/D55875

[PATCH] D56402: [analyzer] [NFC] [RetainCountChecker] Remove dead unused map

This revision was automatically updated to reflect the committed changes. Closed by commit rC350868: [analyzer] [NFC] [RetainCountChecker] Remove dead unused map (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.

[PATCH] D45109: Remove -cc1 option "-backend-option"

george.karpenkov added subscribers: efriedma, george.karpenkov. george.karpenkov added a comment. Hi Eli, The commit makes sense, but I’m not sure about your change to the `test/Driver/apple-kext-mkernel.c ` file: ins

[PATCH] D45839: [analyzer] Add support for WebKit "unified sources".

george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:144 +// includes the full path. +if (SM.getFilename(IL).contains("UnifiedSource")) { + StringRef Name = SM.getFilename(SL); Is this

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1720 +// Either the record variable or the field has to be const qualified. +if (RecordVarTy.isConstQualified() || Ty.isConstQualified()) { + if (const Expr *Init = VD->getIni

[PATCH] D45718: Allow registering custom statically-linked analyzer checkers

george.karpenkov added a comment. Tests would be nice, but on the other hand AFAIK we don't have unit tests for the analyzer. Maybe it's time we add those. Repository: rC Clang https://reviews.llvm.org/D45718 ___ cfe-commits mailing list cfe-com

[PATCH] D44557: [analyzer] CStringChecker.cpp - Code refactoring on bug report.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang https://reviews.llvm.org/D44557 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45839: [analyzer] Add support for WebKit "unified sources".

george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h:144 +// includes the full path. +if (SM.getFilename(IL).contains("UnifiedSource")) { + StringRef Name = SM.getFilename(SL); NoQ wro

[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

george.karpenkov added a comment. I'm new to the taint visitor, but I am quite confused by your change description. > and many checkers rely on it How can other checkers rely on it if it's private to the taint checker? Also, it's probably to explicitly include BugReporterVisitors.h in the chec

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

george.karpenkov added inline comments. Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:284 + /// \sa shouldPostProcessBugReports + Optional PostProcessBugReports; + The option name should be more self-explanatory, post-processing in general can

[PATCH] D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.

george.karpenkov added a comment. > I could also move RangedConstraintManager.h under include We probably don't want to do that: currently it can only be imported by files in `Core`, and we probably should keep it that way Comment at: lib/StaticAnalyzer/Core/RangedConstraintM

[PATCH] D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.

george.karpenkov added a comment. Another approach would be to instead teach `RangedConstraintManager` to convert it's constraints to Z3. That would be an unwanted dependency, but the change would be much smaller, and the internals of the solver would not have to be exposed. @NoQ thoughts? Re

[PATCH] D45718: [analyzer] Allow registering custom statically-linked analyzer checkers

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Actually, we do have unittests in `tools/clang/unittest/Analysis`. @alexfh would you be able to write a unittest in there? It should be fairly easy following the s

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

george.karpenkov added a comment. The code looks good apart from a few minor nits. I think I would prefer a new category created for this checker instead of using `alpha`; let's see what @NoQ has to say. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:305 +

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

george.karpenkov added a comment. `antipatterns` or `security` could be another potential category name. https://reviews.llvm.org/D45532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. > can add extra notes like Ah right, than it's "can rely on" rather than "rely on". > it is necessary to explicitly include I've meant adding the include to `GenericTaintC

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

george.karpenkov added a comment. Looks reasonable. Have you tried on any large codebases? You relax two guards, so I would be wary of explosion in false positives. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:77 if (Opc == BO_Assign) { - LossOfSign

[PATCH] D45611: [analyzer] Fix filename in cross-file HTML report

george.karpenkov accepted this revision. george.karpenkov added a comment. LGTM Repository: rC Clang https://reviews.llvm.org/D45611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D48474: [analyzer][ctu] fix unsortable diagnostics

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. > My fear of using file IDs is that I don't know whether they are stable IIRC they are currently stable, but that's relying on implementation details. Still better than not

[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name

george.karpenkov added a comment. Few more nits. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:691 +StringRef getVariableName(const FieldDecl *Field) { + // If \p Field is a captured lambda variable, Field->getName() will return Is t

[PATCH] D48291: [analyzer][UninitializedObjectChecker] Fixed captured lambda variable name

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Looks like @NoQ had some valid comments as well. https://reviews.llvm.org/D48291 ___ cfe-commits mailing list cfe-c

[PATCH] D47616: [CFG] [analyzer] Explain copy elision through construction contexts.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D47616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[PATCH] D47554: [analyzer] Check for dead/impossible status checks

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. @NoQ had some valid comments. I think this could be still useful without dataflow if we negate matches containing escaping and assignments. Though of course this c

[PATCH] D48608: [CFG] [analyzer] Add construction contexts for C++ objects returned from Objective-C messages.

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor nits: would like some more comments. Comment at: include/clang/Analysis/CFG.h:185 +assert(isa(E) || isa(E)); +return !E->isGLValue

[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM with a nit. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674 + const LocationContext *LC = Context.getLocationContext(); +

[PATCH] D48325: [analyzer][UninitializedObjectChecker] Support for MemberPointerTypes

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Requesting changes or clarification on uninitialized references. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:392 -

[PATCH] D48249: [analyzer] Add stubs for argument construction contexts for arguments of C++ constructors and Objective-C messages.

george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: lib/Analysis/CFG.cpp:2397 + for (auto Arg: C->arguments()) +if (Arg->getType()->getAsCXXRecordDecl() && !Arg->isGLValue())

[PATCH] D44756: [analyzer] [NFC] A convenient getter for getting a current stack frame

This revision was automatically updated to reflect the committed changes. Closed by commit rC335701: [analyzer] [NFC] A convenient getter for getting a current stack frame (authored by george.karpenkov, committed by ). Herald added subscribers: cfe-commits, mikhail.ramalho. Changed prior to commi

[PATCH] D48514: [analyzer] [NFC] Add -verify to malloc checker test

This revision was automatically updated to reflect the committed changes. Closed by commit rC335700: [analyzer] [NFC] Add -verify to malloc checker test (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D4851

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

george.karpenkov added a comment. @sylvestre.ledru Have you found any actual bugs using this checker? @Szelethus Interesting, it seems that the pointer itself is initialized, but not what it's pointing to. I think we should just check the fields directly, and do not attempt to traverse the point

[PATCH] D48561: [Analyzer] Moved RangeConstraintManager to header. NFC.

george.karpenkov reopened this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. After thinking about this change a bit longer, I think it does not make sense. Albeit poorly named, the previous design had a purpose: `RangedConstraintManager` is a public

[PATCH] D48249: [analyzer] Add stubs for argument construction contexts for arguments of C++ constructors and Objective-C messages.

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Minor nit: request for code reuse. Comment at: lib/Analysis/CFG.cpp:694 +std::is_same::value || +std::is_same::v

[PATCH] D48910: [ASTMatchers] A matcher for Objective-C @autoreleasepool

This revision was automatically updated to reflect the committed changes. Closed by commit rC336468: [ASTMatchers] A matcher for Objective-C @autoreleasepool (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

george.karpenkov added a comment. @baloghadamsoftware @dkrupp @xazax.hun Interesting. What do you think about instead using Z3 cross-check functionality recently added, to solve this and all other similar problems instead? https://reviews.llvm.org/D49074

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Let's discuss alternatives first. https://reviews.llvm.org/D49074 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

george.karpenkov added a comment. The overall point is that writing this kind of code is *extremely* error-prone. We are actually considering going in a different direction and doing a rollback for the previous rearrangement patches due to some issues. Could you see whether Z3 visitor would meet

[PATCH] D49050: [analyzer] Pass through all arguments from the registerChecker() to the checker constructor

This revision was automatically updated to reflect the committed changes. Closed by commit rC336753: [analyzer] Pass through all arguments from the registerChecker() to the checker… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang http

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

george.karpenkov added a comment. > What issues could it cause since it is guarded by an option? I've meant the rearrangement. Actually, I would like to apologize for that point, I thought it was causing some issues, but it wasn't, we've just checked it yesterday. > As far as I know the purpos

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

george.karpenkov added a comment. > The imprecision in the built in solver might result in failure to constrain a > value to zero while the Z3 might be able to do that. Ah right, that's a good point, we only throw a null-pointer deref/division by zero when the pointer/value is definitely zero.

[PATCH] D49058: [analyzer] Move DanglingInternalBufferChecker out of alpha

george.karpenkov added a comment. @rnkovacs Do you have evaluation statistics handy for this checker? How many bugs it finds, on which projects? How many of those are real bugs? https://reviews.llvm.org/D49058 ___ cfe-commits mailing list cfe-commi

[PATCH] D49333: [ASTMatchers] Introduce Objective-C matchers `hasReceiver` and `isInstanceMessage` for ObjCMessageExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rC337209: [ASTMatchers] Introduce Objective-C matchers `hasReceiver` and… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.ll

[PATCH] D49166: [analyzer] Provide a symmetric method for generating a PathDiagnosticLocation from Decl

This revision was automatically updated to reflect the committed changes. Closed by commit rC337211: [analyzer] Provide a symmetric method for generating a PathDiagnosticLocation… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: htt

[PATCH] D48911: [analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is initialized to zero

This revision was automatically updated to reflect the committed changes. Closed by commit rC337212: [analyzer] Fix GCDAntipatternChecker to only fire when the semaphore is… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https://revi

[PATCH] D48856: [analyzer] Fix overly eager suppression of NPE when the value used is returned from a macro

This revision was automatically updated to reflect the committed changes. Closed by commit rC337213: [analyzer] Bugfix for an overly eager suppression for null pointer return from… (authored by george.karpenkov, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: ht

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

george.karpenkov updated this revision to Diff 155785. george.karpenkov edited the summary of this revision. george.karpenkov edited reviewers, added: delcypher; removed: bob.wilson, glider, t.p.northover, samsonov, beanz. george.karpenkov added a comment. Attempt #2: reduced version of this patc

[PATCH] D49228: [analyzer][UninitializedObjectChecker] Void pointer objects are casted back to their dynmic type in note message

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Cf. my comments to https://reviews.llvm.org/D49437: while this change looks great, is it possible to separate the pointer chasing from the rest of the checker?

[PATCH] D49199: [analyzer][UninitializedObjectChecker] Pointer/reference objects are dereferenced according to dynamic type

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Cf. my comments to https://reviews.llvm.org/D49437: is it possible to separate pointer-chasing from the rest of the checker? https://reviews.llvm.org/D49199 _

[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing

george.karpenkov added a comment. @Szelethus false positives are a single biggest problem of the analyzer. By a *huge* margin, most projects would prefer to err on the side of less, more precise, warnings. Given that currently in my understanding no actual bugs we are sure about were found by th

[PATCH] D49438: [analyzer][UninitializedObjectChecker] New flag to turn off dereferencing

george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:715 void ento::registerUninitializedObjectChecker(CheckerManager &Mg

[PATCH] D49213: [analyzer] pr38072: Suppress an assertion failure for eliding the same destructor twice due to the default argument problem.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM with a nit. Comment at: test/Analysis/temporaries.cpp:472 + for (int i = 0;;) +F j(i ? j : h); +} // no-crash? Repository:

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

george.karpenkov added a comment. @delcypher Could you take a look? @kcc Any objections? https://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

george.karpenkov requested changes to this revision. george.karpenkov added inline comments. This revision now requires changes to proceed. Comment at: tools/scan-build-py/libscanbuild/analyze.py:44 +CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt' +CTU_TEMP_FNMAP_FOLDER = 'tmpE

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

george.karpenkov added a comment. > If the type extension approach is proven to be sound I lack the full context here, but in my experience Z3 is really great for proving whether certain operations may or may not overflow, using the built-in bitvector type. (I'm not sure though if that is what

[PATCH] D39709: [analyzer] [NFC] remove duplicated function

george.karpenkov added a comment. @dcoughlin @NoQ OK to commit? https://reviews.llvm.org/D39709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39709: [analyzer] [NFC] remove duplicated function

This revision was automatically updated to reflect the committed changes. Closed by commit rC319697: [analyzer] [NFC] remove duplicated function (authored by george.karpenkov). Repository: rC Clang https://reviews.llvm.org/D39709 Files: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp I

[PATCH] D40809: [WIP] [analyzer] Dump counterexample traces as C programs

george.karpenkov created this revision. Herald added subscribers: a.sidorin, szepet, xazax.hun. This patch is work in progress, but has already shown itself to be useful in a few cases. For large traces understanding the analyzer output can be painful, especially when arrows indicating the flow

[PATCH] D40809: [WIP] [analyzer] Dump counterexample traces as C programs

george.karpenkov updated this revision to Diff 125429. george.karpenkov edited the summary of this revision. https://reviews.llvm.org/D40809 Files: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp lib/StaticAnalyz

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

george.karpenkov added inline comments. Comment at: tools/scan-build-py/libscanbuild/analyze.py:44 +CTU_FUNCTION_MAP_FILENAME = 'externalFnMap.txt' +CTU_TEMP_FNMAP_FOLDER = 'tmpExternalFnMaps' gerazo wrote: > george.karpenkov wrote: > > What would happen when m

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

george.karpenkov added a comment. Python part looks good to me. I don't know whether @dcoughlin or @NoQ would want to insert additional comments on C++ parts. https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

george.karpenkov added inline comments. Comment at: tools/scan-build-py/libscanbuild/report.py:257 def read_bugs(output_dir, html): +# type: (str, bool) -> Generator[Dict[str, Any], None, None] """ Generate a unique sequence of bugs from given output directory.

[PATCH] D40809: [WIP] [analyzer] Dump counterexample traces as C programs

george.karpenkov updated this revision to Diff 126853. george.karpenkov edited the summary of this revision. https://reviews.llvm.org/D40809 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h lib/StaticAnalyzer/Core

[PATCH] D44597: [CFG] [analyzer] Add C++17-specific variable and return value construction contexts.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM, subject to minor nits Comment at: include/clang/Analysis/CFG.h:172 /// by value. This, like constructor, requires a construction context, which -//

[PATCH] D44755: [analyzer] Suppress more C++17-related crashes.

george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:480 } + CC = dyn_cast( + RTCElem->getConstructionContext()); I'm a bit confused as to what is going on here; maybe a short comment would be helpful? W

[PATCH] D44725: [analyzer] NFC: Move construction context allocation into a helper method.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM, but would actually make even more sense as a static function for the allocator. E.g. we have `llvm::make_shared<>()`, so we could also have `Allocator::make_obj<...>(

[PATCH] D44804: [StaticAnalyzer] Silence an unused variable warning. NFC.

george.karpenkov added a comment. Actually, a better change would be to simply change `cast(MR)` to `VR` Repository: rC Clang https://reviews.llvm.org/D44804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[PATCH] D44804: [StaticAnalyzer] Silence an unused variable warning. NFC.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM, provided it compiles and tests run (`ninja check-clang`) Repository: rC Clang https://reviews.llvm.org/D44804 ___ cf

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

george.karpenkov added a comment. @koldaniel Have you evaluated this checker? On which codebases? Were the warnings real security issues, or were they mostly spurious? The code seems fine, but I'm not sure whether it should be in `security` or in `alpha`. https://reviews.llvm.org/D35068 ___

[PATCH] D45086: [analyzer] Unroll the loop when it has a unsigned counter.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. Looks reasonable Repository: rC Clang https://reviews.llvm.org/D45086 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D45071: [analyzer] Track null or undef values through pointer arithmetic.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM with a nit. Also I don't quite understand why being additive is important? Isn't pointer subtraction basically the same? Comment at: lib/StaticAnaly

[PATCH] D44955: [CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM with a nit, provided a proper fix can not be landed in a timely enough matter. Comment at: lib/Analysis/CFG.cpp:740 cleanupConstructionCont

[PATCH] D44854: [analyzer] Be more careful about C++17 copy elision.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. LGTM provided comments are answered. Field rename would be appreciated, if possible. Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:215 +

<    1   2   3   4   5   6   >