[PATCH] D96215: [clang-tidy] Aliasing: Add support for lambda captures.

xazax.hun added a comment. > I'm arguing that it should scan for lambda captures by reference as well. What about immediately invoked lambdas? Would it make sense to exclude those? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96215/new/ https://reviews.llvm.org/D96215 ___

[PATCH] D101788: [AST] AnyCall: Implement arguments().

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/AnyCall.h:170 + return {ME->getArgs(), ME->getNumArgs()}; +} else if (const auto *CE = dyn_cast(E)) { + return {CE->getArgs(), CE->getNumArgs()}; As far as I remember `CXXMemb

[PATCH] D97605: [Lifetimes] Fix false positive warning from BUG 49342

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdd6738d93de1: [clang][Lifetimes] Fix false positive warning from BUG 49342 (authored by xazax.hun). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D97605: [Lifetimes] Fix false positive warning from BUG 49342

xazax.hun created this revision. xazax.hun added reviewers: rsmith, mgehre. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, Szelethus, dkrupp, rnkovacs. xazax.hun requested review of this revision. Herald added a subscriber: cfe-commits. After some interaction betw

[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

xazax.hun added a comment. In D93224#2497632 , @steakhal wrote: > Do you think it's acceptable? > @xazax.hun @NoQ It is fine by me. I believe, currently is Ericsson the biggest contributor and user of the CTU functionality. So in case they are onboard wi

[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

xazax.hun added a comment. In D93224#2495404 , @steakhal wrote: > Do we really promise ABI compatibility for AST dumps? If we support it to > some extent, then we would have a problem - which would mean that we can not > target clang-12 anymore with this

[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

xazax.hun added a comment. In D93224#2493434 , @steakhal wrote: > How should I continue to get this working with CTU? We have two CTU modes. One loading the dump and the other loading from AST source file. I assume that you refer to the former? Could you

[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h:38 + + std::map> + PartialPDs; A pointer pair might be small enough for a DenseMap? Comment at: clang/lib/Analysis/Path

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2897 + ASTContext &AC) { + PathDiagnosticLocation Location = BR.getLocation(); + What will this location return?

[PATCH] D93222: [RFC][analyzer] Introduce MacroExpansionContext to libAnalysis

xazax.hun added inline comments. Comment at: clang/lib/Analysis/MacroExpansionContext.cpp:22 +: PP(PP), SM(PP.getSourceManager()), LangOpts(LangOpts) { + class MacroExpansionRangeRecorder : public PPCallbacks { +const Preprocessor &PP; whisperity wrote:

[PATCH] D93222: [RFC][analyzer] Introduce MacroExpansionContext to libAnalysis

xazax.hun added inline comments. Comment at: clang/lib/Analysis/MacroExpansionContext.cpp:22 +: PP(PP), SM(PP.getSourceManager()), LangOpts(LangOpts) { + class MacroExpansionRangeRecorder : public PPCallbacks { +const Preprocessor &PP; steakhal wrote: >

[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Yay! Deleting code is always nice. Comment at: clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:148 nameSET_PTR_VAR_TO_NULL -

[PATCH] D93223: [RFC][analyzer] Create MacroExpansionContext member in AnalysisConsumer and pass down to the diagnostics consumers

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:129 +Plugins(plugins), Injector(injector), CTU(CI), +MacroExpansions(PP, CI.getLangOpts()) { DigestAnalyzerOptions(); This will always record

[PATCH] D93222: [RFC][analyzer] Introduce MacroExpansionContext to libAnalysis

xazax.hun added a comment. Overall looks good to me, I have some minor nits and questions inline. Comment at: clang/lib/Analysis/MacroExpansionContext.cpp:22 +: PP(PP), SM(PP.getSourceManager()), LangOpts(LangOpts) { + class MacroExpansionRangeRecorder : public PPCallbacks

[PATCH] D92432: [analyzer] Add a thin abstraction layer between libCrossTU and libAnalysis.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92432/new/ https://reviews.llvm.org/D92432 ___ cfe-commits mailing list cfe-comm

[PATCH] D90362: scan-build supprot relative 'file' in cdb.

xazax.hun added a comment. In D90362#2369394 , @phosek wrote: > What's the plan regarding updating scan-build-py to match the upstream? It > seems like this issue has already been address in the upstream version. As far as I understand, the author is no

[PATCH] D90362: scan-build supprot relative 'file' in cdb.

xazax.hun added a comment. I know that the current situation is a mess, but there is an alternative version of scan-build-py on Github, which is also distributed on pypi. Could you check if that version is also susceptible to this problem and open a pull request for the author in case it is? T

[PATCH] D87519: [analyzer][Liveness][NFC] Enqueue the CFGBlocks post-order

xazax.hun added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:522 - // FIXME: we should enqueue using post order. - for (const CFGBlock *B : cfg->nodes()) { + for (const CFGBlock *B : *AC.getAnalysis()) { worklist.enqueueBlock(B); Wi

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

xazax.hun added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:332 if (B->isAssignmentOp()) { if (!LV.killAtAssign) return; Nit: Maybe moving this to the front of the function would simplify the code a bit. Repository: rG LL

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Looks a lot cleaner this way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87518/new/ https://reviews.llvm.org/D87518

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

xazax.hun added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:326 void TransferFunctions::VisitBinaryOperator(BinaryOperator *B) { + if (LV.killAtAssign && B->getOpcode() == BO_Assign) { +if (const auto *DR = dyn_cast(B->getLHS()->IgnoreParens())) {

[PATCH] D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

xazax.hun added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:326 void TransferFunctions::VisitBinaryOperator(BinaryOperator *B) { + if (LV.killAtAssign && B->getOpcode() == BO_Assign) { +if (const auto *DR = dyn_cast(B->getLHS()->IgnoreParens())) {

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

xazax.hun accepted this revision. xazax.hun added a comment. So now all of the dependencies are landed and this should be ready for its prime time, right? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82598/new/ https://reviews.llvm.org/D82598 _

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86027/new/ https://reviews.llvm.org/D86027

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

xazax.hun added inline comments. Herald added a subscriber: danielkiss. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:556 +const LocationContext *LC = C.getLocationContext(); +InnerPointerVal = C.getSValBuilder().conjureSymbolVal( +CallExpr, L

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

xazax.hun added a comment. Is this related to https://bugs.llvm.org/show_bug.cgi?id=44493? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86135/new/ https://reviews.llvm.org/D86135 ___ cfe-commits mailing

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

xazax.hun added a comment. In D85528#2229309 , @steakhal wrote: > It would be nice to have this fix in clang11. > Do you think it's qualified for it? Unfortunately, this is not a fix only change. This is a fix for refutation, but also a behavioral change

[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351 +bool SmartPtrModeling::handleEqOp(const CallEvent &Call, + CheckerContext &C) const { I'd prefer to call this AssignOp to a

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

xazax.hun requested changes to this revision. xazax.hun added a comment. This revision now requires changes to proceed. Please add a test case, where the unique_ptr is initialized from a pointer parameter that has no assumptions. I think that case is not handled correctly. Com

[PATCH] D86223: [analyzer][z3] Use more elaborate z3 variable names in debug build

xazax.hun added inline comments. Comment at: clang/test/Analysis/z3/pretty-dump.c:15 +// CHECK: "constraints": [ +// CHECK-NEXT: { "symbol": "(reg_$[[#]]) == 3", "range": "(= reg_$[[#]] #x0003)" } + } Will this test case work with non-debug builds?

[PATCH] D86223: [analyzer][z3] Use more elaborate z3 variable names in debug build

xazax.hun accepted this revision. xazax.hun added a comment. This is what I had in mind, thanks! Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:335 +llvm::raw_svector_ostream OS(Str); +OS << PRETTY_SYMBOL_KIND << ID; +#undef PRETTY_SYMBOL_KI

[PATCH] D86334: [analyzer] Remove redundant output errs

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86334/new/ https://reviews.llvm.org/D86334 __

[PATCH] D86223: [analyzer][z3] Use more elaborate z3 variable names in debug build

xazax.hun added a comment. In D86223#2228855 , @steakhal wrote: > I wanted to conditionally, aka. in debug configuration override the base > implementation of the SymbolData::getKindStr I see. Yeah, that does not make much sense. I was thinking about alw

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

xazax.hun added a comment. MemRegion is a popular class to instantiate in the analyzer so it looks good to me. But unless you add a comment one will probably replace the offset with an optional as the part of a refactoring. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D86223: [analyzer][z3] Use more elaborate z3 variable names in debug build

xazax.hun added a comment. In D86223#2227353 , @steakhal wrote: > Eh, I'm not sure if it worth it to put these into virtual functions - as > conditionally overriding functions is not really a good idea. I am not sure I follow. What do you mean by conditi

[PATCH] D86223: [analyzer][z3] Use more elaborate z3 variable names in debug build

xazax.hun added a comment. Exactly, but you could return a StringRef to static storage. I am fine with both approach. Whichever you find cleaner. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86223/new/ https://reviews.llvm.org/D86223 ___

[PATCH] D86223: [analyzer][z3] Use more elaborate z3 variable names in debug build

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! I wonder whether having a virtual method for symbols to get the prefix would be cleaner (something like getKindCStr), but I do not insist. Repository: rG LLVM Github Monorepo

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:147 -if (!move::isMovedFrom(State, ThisR)) { - // TODO: Model this case as well. At least, avoid invalidation of - // globals. - return false; +if (ModelSm

[PATCH] D86029: [analyzer] Add modeling for unque_ptr::get()

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:362-363 + const auto *InnerPointVal = State->get(ThisRegion); + if (!InnerPointVal) +return; + NoQ wrote: > You'll have to

[PATCH] D85752: [Analyzer] Store the pointed/referenced type for dynamic casts

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicType.cpp:73 +Ty = STTPTy->getReplacementType(); + if (Ty->isPointerType()) +Ty = Ty->getPointeeType(); Is this doing what you intended? What about a reference to a pointer?

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

xazax.hun added a comment. In D85528#2205799 , @NoQ wrote: > I expect at least one LIT test //without// `-analyzer-config > crosscheck-with-z3=true` Agreed. I think the code snippet I proposed would be a great test to include with this revision. Repos

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

xazax.hun added a comment. Looks reasonable to me, but I am not very familiar with the impacts of the additional casts. Do we lose some modeling power when we are using the regular constraint solver? For example, when we have constraints both on the original and the cased symbol can the analyz

[PATCH] D85319: [analyzer][RFC] Get info from the LLVM IR for precision

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp:30 + // markers which are used by some LLVM analysis (e.g. AliasAnalysis). + CGO.OptimizationLevel = 2; // -O2 + martong wrote: > TODO overwrite ALL optimization

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, thanks! I think all the problems that left should be solved in a separate patch or even out of the scope for the GSoC. Comment at: clang/lib/StaticAnalyzer/Chec

[PATCH] D84600: [Analyzer] Support note tags for smart ptr checker

xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:312 + const NoteTag *getNoteTag( + std::function Cb, + bool IsPrunable = false) { The callback is taken is an rvalue reference in othe

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

xazax.hun added a comment. I think adding code snippets that are affected by this patch would make it much easier to evaluate the changes and whether this is a good idea or not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84736/new/ https://revi

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

xazax.hun added a comment. In D82598#2164363 , @Szelethus wrote: > Actually, what I said initially is true: > > > [...] the only non-expression statements it **queried** are > > ObjCForCollectionStmts [...] Btw, sorry. I somehow misunderstood the snippe

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

xazax.hun added inline comments. Comment at: clang/lib/Analysis/LiveVariables.cpp:317-320 for (Stmt *Child : S->children()) { -if (Child) - AddLiveStmt(val.liveStmts, LV.SSetFact, Child); +if (const auto *E = dyn_cast_or_null(Child)) + AddLiveExpr(val.liveExp

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

xazax.hun added a comment. In D82598#2162239 , @Szelethus wrote: > I chased my own tail for weeks before realizing that there is indeed another > instance when a live **statement** is stored, other then > `ObjCForCollectionStmt`... > > void clang_analy

[PATCH] D83877: [Analyzer] Changed in SmartPtrModeling to handle Swap

xazax.hun added inline comments. Comment at: clang/test/Analysis/Inputs/system-header-simulator-cxx.h:964-965 + + template + void swap(unique_ptr &x, unique_ptr &y) noexcept { +x.swap(y); NoQ wrote: > You seem to be relying on the fact that global `std::sw

[PATCH] D83836: [Analyzer] Implementing checkRegionChanges for SmartPtrModeling

xazax.hun added inline comments. Comment at: clang/test/Analysis/Inputs/system-header-simulator-cxx.h:962 + operator bool() const; + unique_ptr &operator=(unique_ptr &&p); +}; vrnithinkumar wrote: > added this to support use case like `Q = std::move(P)` This o

[PATCH] D83877: [Analyzer] Changed in SmartPtrModeling to handle Swap

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/test/Analysis/Inputs/system-header-simulator-cxx.h:965 + template + void swap(unique_ptr &x, unique_ptr &y) noexcept { +x.swap(y)

[PATCH] D83286: [analyzer][solver] Track symbol disequalities

xazax.hun added a comment. In D83286#2149912 , @vsavchenko wrote: > @xazax.hun You were interested in performance ⏫ > > These results here compare this patch together with D82445 > against **master**. This is really gre

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

xazax.hun accepted this revision. xazax.hun added a comment. Thanks, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82445/new/ https://reviews.llvm.org/D82445 ___ cfe-commits mailing list cfe-comm

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1254 +// sufficient. +return S1->get() == S2->get() && + S1->get() == S2->get(); xazax.hun wrote: > Sorry, but I am a bit confused here.

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:506 + APSIntType Type(Int); + return Int == Type.getZeroValue(); +} Does the semantics if this differ from ` llvm::APInt::isNullValue`? Comm

[PATCH] D82856: [analyzer][Z3-refutation] Add statistics for refutation visitor

xazax.hun added a comment. Yay! This looks good to me and I love statistics, so a huge +1. I have one question though. Isn't this counting all the reports in an equivalence class? I.e. if the analyzer finds something multiple times it will only be displayed once but here it will be counted mult

[PATCH] D81315: [analyzer] Warning for default constructed unique pointer dereferences

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:139 + + const auto *MethodDecl = dyn_cast_or_null(OC->getDecl()); + You should never get null here due to `isStdSmartPointerClass` guarding above. I think the nu

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

xazax.hun accepted this revision. xazax.hun added a comment. @szelethus The patch looks good to me and I find it a welcome change that should make things easier to understand and maybe even a bit more efficient, I hope this won't break ObjC :) My discussion with Artem is orthogonal to this chang

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

xazax.hun added a comment. In D82598#2115656 , @NoQ wrote: > > We could just kill all subexpr at the end of the full expression > > I suspect that it would be pretty bad if you, say, kill the condition of the > `if`-statement before picking the branch. Or

[PATCH] D82561: [analyzer][CrossTU] Lower CTUImportThreshold default value

xazax.hun added a comment. In D82561#2116194 , @gamesh411 wrote: > In D82561#2116091 , @balazske wrote: > > > That means perform a get CTU definition if the TU to be imported (where the > > function comes from) is

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

xazax.hun added a comment. I always wondered if we actually need to track the liveness of exprs at all. We could just kill all subexpr at the end of the full expression without precomputing anything. But I might miss something. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82561: [analyzer][CrossTU] Lower CTUImportThreshold default value

xazax.hun added a comment. The analyzer inlines small functions within a TU regardless of the thresholds. I think it would be sensible to do the same across TUs in the case we don't do this already. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82

[PATCH] D82445: [analyzer][solver] Track symbol equivalence

xazax.hun added a comment. I only checked the test cases and the comments so far and it looks very useful and promising. I really hope that the performance will be ok :) I'll look at the actual code changes later. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManag

[PATCH] D82381: [analyzer] Introduce small improvements to the solver infra

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:373 + ++Upper; + --Lower; + Sorry if my question is dumb, but I do not really have a mental model at this point about where do we actually handle types and ov

[PATCH] D81315: [analyzer] Warning for default constructed unique pointer dereferences

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:35 bool isNullAfterMoveMethod(const CallEvent &Call) const; + BugType NullDereferenceBugType{this, "Null-smartPtr-deref", + "C++ smart pointer"}

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun accepted this revision. xazax.hun added a comment. I would not call the results of the measurement within the margin of error but the results do not look bad. Unless there is some objection from someone else I am ok with committing this but please change the title of revision before c

[PATCH] D81564: [analyzer] SATest: Add posibility to download source from git and zip

xazax.hun added inline comments. Comment at: clang/utils/analyzer/ProjectMap.py:14 +class DownloadType(str, Enum): +GIT = "git" I was wondering what is the point of inheriting from `str`. I am not well-versed in Python so it is more of a curiosity. Repos

[PATCH] D81315: [Draft] [Prototype] warning for default constructed unique pointer dereferences

xazax.hun added a comment. Hi! Please add the [analyzer] tag in front of your patches as some folks have automated scripts based on that tag to add themselves as subscriber/reviewer. A small debugging/productivity tip, if you add a `printState` method to your checker like in https://github.co

[PATCH] D81315: [Draft] [Prototype] warning for default constructed unique pointer dereferences

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:46 +}; +} // end of anonymous namespace + You can merge the two anonymous namespaces, this and the one below. Comment at: clang/lib/StaticAnalyz

[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

xazax.hun added a comment. In D77866#2069144 , @NoQ wrote: > Like, i mean, the tree of packages that we currently have is a wrong > abstraction. I totally agree with this. > The most ad-hoc approach that's better than the current situation would be >

[PATCH] D80016: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

xazax.hun added a comment. A high level comment. Trying to match function signatures is not a unique problem! In fact, almost all of the checks the analyzer have is trying to solve the very some problem. One of the methods we have at this point is called CallDescription, see here: https://githu

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added a comment. In D78933#2054195 , @ASDenysPetrov wrote: > @xazax.hun, any thoughts? I think we should check it on some more projects. We saw vastly different analyzer behavior on other projects in the past. I think you should be able to r

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013 +addToFunctionSummaryMap( +"__buf_size_arg_constraint_mul", +Summary(ArgTypes{ConstVoidPtrTy, SizeTy, SizeTy}, RetType{IntTy},

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013 +addToFunctionSummaryMap( +"__buf_size_arg_constraint_mul", +Summary(ArgTypes{ConstVoidPtrTy, SizeTy, SizeTy}, RetType{IntTy},

[PATCH] D80117: [analyzer] Introduce reasoning about symbolic remainder operator

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:633 + + // Check if LHS is 0. It's a special case when the result is guaranteed + // to be 0 no matter what RHS is (we put to the side the case when RHS is I

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

xazax.hun added inline comments. Comment at: clang/test/Analysis/loop-unrolling.cpp:503 + +void arg_as_loop_counter(int i) { + for (i = 0; i < 10; ++i) { nit: we usually use `arg` for actual arguments at the call site, and use `param` for formal parameters. The

[PATCH] D80171: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter

xazax.hun added reviewers: NoQ, vsavchenko. xazax.hun added a comment. Thanks for finding this bug! Adding some reviewers. I think it would be perfectly fine to unroll loops where the loop counter is a pass-by-value parameter. That should not be considered escaped. I think in case of parameters

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:90 + + TriState GetCmpOpState(size_t CurrentOPIndex, size_t QueriedOPIndex) const { +assert(CurrentOPIndex < CmpOpCount && QueriedOPIndex <= CmpOpCount); I

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun marked an inline comment as done. xazax.hun added a comment. Thanks! I still have some nits inline, but overall the implementation looks good to me. I think, however, that we should not only focus on the implementation but on the high-level questions. Is this the way forward we want?

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

xazax.hun added a comment. Overall looks good to me some questions and nits inline. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:59 enum CallEventKind { + CE_CXXDeallocator, CE_Function, Szelethus wrote: > I need to move th

[PATCH] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

xazax.hun added a comment. In D79423#2025001 , @martong wrote: > 1. Some function types contain non-builtin types. E.g. `FILE*`. We cannot get > this type as easily as we do with builtin types (we can get builtins simply > from the ASTContext). In case o

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:588 + // AnyX2 means that two expressions marked as `Any` are met in code, + // and there is a special column for that, for example: + // if (x >= y) ASDenysP

[PATCH] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

xazax.hun added a comment. In D79423#2022812 , @martong wrote: > I don't think that could be a concern. > Actually, redefinition of a reserved name either in the C or in the C++ > standard library is undefined behavior: I disagree. As you mentioned in

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:588 + // AnyX2 means that two expressions marked as `Any` are met in code, + // and there is a special column for that, for example: + // if (x >= y) xazax.hu

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added a comment. In D78933#2022684 , @ASDenysPetrov wrote: > I have never run them before. How I can do this? What project to use as a > sample? Unfortunately, we do not really have a well-defined set of benchmarks. But as a first step, you

[PATCH] D79415: [analyzer][MallocChecker] When modeling realloc-like functions, don't early return if the argument is symbolic

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, thanks! I do agree that the warning message is not the best but it is not horrible either :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:588 + // AnyX2 means that two expressions marked as `Any` are met in code, + // and there is a special column for that, for example: + // if (x >= y) I have r

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added a comment. Thanks for working on this, I do believe the analyzer would greatly profit from better constraint solving capabilities. Unfortunately, we had some troubles in the past trying to improve upon the current status and we had to revert multiple patches. This is why the com

[PATCH] D78933: [analyzer] RangeConstraintManager optimizations in comparison expressions

xazax.hun added a comment. In D78933#2022288 , @ASDenysPetrov wrote: > Guys, > @xazax.hun, @Charusso, @steakhal, @vsavchenko, @NoQ, @baloghadamsoftware, > please, tell something. What do you think of this improvement? Sorry, I will definitely take a l

[PATCH] D79433: [analyzer] StdLibraryFunctionsChecker: Add summaries for POSIX

xazax.hun added a comment. This is cool! Some questions: 1. Have you reported those bugs to CppCheck devs? It might be useful for us as well, as they can also double-check who is right. 2. This is a really large number of summaries. At this point, I wonder if it would be worth have a separate

[PATCH] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

xazax.hun added a comment. I am a bit unsure about this change. C libraries might be consumed in C++ projects and C++ projects might be free to overload those functions. Does the effort needed to specify the signatures justify not supporting that (probably unintentional) name collisions in C++?

[PATCH] D79425: [analyzer] StdLibraryFunctionsChecker: Add overload for adding the same summary for different names

xazax.hun added a comment. I'd prefer to have this functionality committed together its first actual use with tests. I also agree with @balazske, we should diagnose the cases when we have multiple summaries for the same function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D79420: [analyzer] Make NonNullParamChecker as dependency for StdCLibraryFunctionsChecker

xazax.hun added a comment. > If a given parameter in a FunctionDecl has a nonull attribute then the > NonNull constraint in StdCLibraryFunctionsChecker has the same effect as > NonNullParamChecker. Wait, where the diagnostic is coming from? My point is, the user should be able to turn `apiMode

[PATCH] D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics

xazax.hun added a comment. Also, I see no tests :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D78118: [analyzer] StdLibraryFunctionsChecker: Add option to display loaded summaries

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78118/new/ https://reviews.llvm.org/D78118

[PATCH] D79430: [analyzer] StdLibraryFunctionsChecker: Add LazyRanges to support type dependent Max

xazax.hun added a comment. I am a bit unsure what the purpose of these `Max` summaries are? As far as I understand the `Max` represents the largest value for the type of the formal parameter. Do we really ever need to specify this in a summary? Isn't it always an error to pass a value that is

[PATCH] D79432: [analyzer] StdLibraryFunctionsChecker: Add summaries for libc

xazax.hun added a comment. I think testing summaries this way can be really hard to manage in the future. I see two possible ways forward to make this easier: a) Make something like https://reviews.llvm.org/D78118 that will also dump the actual summary in a textual form, not only the fact that a

[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

xazax.hun added a comment. Overall looks good to me, I have one question inline. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013 +addToFunctionSummaryMap( +"__buf_size_arg_constraint_mul", +Summary(ArgTypes{ConstVoidPtrTy,

[PATCH] D79336: [analyzer] Generalize bitwise OR rules for ranges

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:77 + assert(!isEmpty()); + // NOTE: It's a shame that we can't implement 'getMaxValue' without scanning + // the whole tree to get to the last element.

<    1   2   3   4   5   6   7   8   9   10   >