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

NoQ accepted this revision. NoQ added a comment. Looks good! One minor comment. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:488-491 // TODO: Dereferencing should be done according to the dynamic type. while (Optional L = DerefdV.getAs()) {

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

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:392 -if (T->isMemberPointerType()) { - if (isMemberPointerUninit(FR, LocalChain)) +if (T->isPointerType() || T->isReferenceType()) { + if (isPointerOrReferenceUni

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

NoQ added a comment. I think we need to finish our dialog on who's responsible for initialization and why do we need to filter constructors at all, cause it's kinda hanging (i.e. https://reviews.llvm.org/D45532#inline-422673). https://reviews.llvm.org/D48436

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

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:392 -if (T->isMemberPointerType()) { - if (isMemberPointerUninit(FR, LocalChain)) +if (T->isPointerType() || T->isReferenceType()) { + if (isPointerOrReferenceUni

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thank you!! Please commit. Comment at: test/Analysis/constraint_manager_negate_difference.c:95 +void negate_mixed(int m, int n) { + if (m -n > INT_MIN && m - n <= 0) +return;

[PATCH] D48427: [Analyzer] Fix for D47417 to make the tests pass

NoQ added a comment. Aha, ok, yeah, we should have seen this coming. Whenever a checker tracks pairs of objects, like containers and iterators, or strings objects and their internal buffers (https://reviews.llvm.org/D48522), or return values and out-parameters of the same function call (https:/

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

NoQ added a comment. I think this looks good. There's a problem with missing construction contexts, but i guess that's not the checker's fault, so let's add a FIXME and commit. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:454-455 + return; +const auto O

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

NoQ updated this revision to Diff 153155. NoQ added a comment. Code re-use! https://reviews.llvm.org/D48249 Files: lib/Analysis/CFG.cpp lib/Analysis/ConstructionContext.cpp test/Analysis/cfg-rich-constructors.cpp test/Analysis/cfg-rich-constructors.mm test/Analysis/temporaries.cpp t

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

NoQ updated this revision to Diff 153157. NoQ added a comment. Actually, yeah, add the comment. https://reviews.llvm.org/D48249 Files: lib/Analysis/CFG.cpp lib/Analysis/ConstructionContext.cpp test/Analysis/cfg-rich-constructors.cpp test/Analysis/cfg-rich-constructors.mm test/Analysis

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

NoQ updated this revision to Diff 153165. NoQ added a comment. Code re-use! And the comment. https://reviews.llvm.org/D48608 Files: include/clang/Analysis/CFG.h lib/Analysis/CFG.cpp test/Analysis/cfg-rich-constructors.mm test/Analysis/lifetime-extension.mm Index: test/Analysis/lifetim

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

NoQ marked 2 inline comments as done. NoQ added inline comments. Comment at: lib/Analysis/CFG.cpp:799 +ConstructionContextMap.lookup(ME)) { + cleanupConstructionContext(ME); + if (const auto *CC = ConstructionContext::createFromLayers( --

[PATCH] D48681: [CFG] [analyzer] Add construction contexts for function arguments.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware. Replace stubs we had to discriminate between temporaries and function arguments with actual constructio

[PATCH] D48681: [CFG] [analyzer] Add construction contexts for function arguments.

NoQ added inline comments. Comment at: lib/Analysis/CFG.cpp:5011 print_initializer(OS, Helper, SICC->getCXXCtorInitializer()); -break; +return; } For consistency. Comment at: lib/Analysis/CFG.cpp:5074-5078 for (auto I: Stmts)

[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

NoQ accepted this revision. NoQ edited reviewers, added: NoQ; removed: dergachev.a. NoQ added a comment. Yep, this definitely looks safe and sound in the current shape. I'm also very sorry for the lack of attention. https://reviews.llvm.org/D46944

[PATCH] D48764: [Analyzer] Hotfix for iterator checkers: Mark left-hand side of `SymIntExpr` objects as live in the program state maps.

NoQ added a comment. That's right. You only need to mark "atomic" symbols (`SymbolData`) as live, and expressions that contain them would automatically become live. So i think you should just iterate through a `symbol_iterator` and mark all `SymbolData` symbols you encounter as live. Is this a

[PATCH] D48427: [Analyzer] Fix for D47417 to make the tests pass

NoQ added a comment. That'd be a hell for you because when the container is updated you won't be able to easily find iterators all that iterate over it. Normally what you want to do is keep mapping iterators to container regions, and when the region dies, "freeze" the data (make sure it can no

[PATCH] D48681: [CFG] [analyzer] Add construction contexts for function arguments.

NoQ added inline comments. Comment at: include/clang/Analysis/ConstructionContext.h:90 assert(Other); return (Trigger == Other->Trigger); } Uhm. Will fix. Repository: rC Clang https://reviews.llvm.org/D48681 __

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68 : IILockGuard(nullptr), IIUniqueLock(nullptr), - LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"), - FgetsFn("fgets"), ReadFn("read"), RecvFn

[PATCH] D48831: alpha.unix.cstring.OutOfBounds checker enable/disable fix

NoQ added a comment. Uhm, so we had an alpha checker enabled all along? Thanks for patching this up! Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:308 // These checks are either enabled by the CString out-of-bounds checker -// explicitly or the "basic" CSt

[PATCH] D49057: [analyzer] Track multiple raw pointer symbols in DanglingInternalBufferChecker

NoQ added a comment. Much symbols! Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:126 + NewSet = F.add(NewSet, RawPtr.getAsSymbol()); + if (!NewSet.isEmpty()) { +State = State->set(ObjRegion, NewSet); rnkovacs wrote:

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

NoQ added inline comments. Comment at: test/Analysis/constraint_manager_scale.c:78 + assert(x * 2 < 8); + clang_analyzer_eval(x < 4); // expected-warning{{TRUE}} + clang_analyzer_eval(x < 2); // expected-warning{{UNKNOWN}} If `int` is 32-bit and `x` equal to 2

[PATCH] D49057: [analyzer] Track multiple raw pointer symbols in DanglingInternalBufferChecker

NoQ accepted this revision. NoQ added a comment. Looks great, thanks! https://reviews.llvm.org/D49057 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

NoQ edited subscribers, added: NoQ; removed: dergachev.a. NoQ added inline comments. Comment at: test/Analysis/nullability_nullonly.mm:103 void testObjCARCExplicitZeroInitialization() { - TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned

[PATCH] D48681: [CFG] [analyzer] Add construction contexts for function arguments.

NoQ added inline comments. Comment at: lib/Analysis/ConstructionContext.cpp:186 +isa(TopLayer->getTriggerStmt())) { + return create( + C, cast(TopLayer->getTriggerStmt()), TopLayer->getIndex(), Should assert that we only have one layer. Re

[PATCH] D48764: [Analyzer] Hotfix for iterator checkers: Mark left-hand side of `SymIntExpr` objects as live in the program state maps.

NoQ added a comment. Looks good with minor comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:493 +for (auto i = Offset->symbol_begin(); i != Offset->symbol_end(); ++i) + SR.markLive(*i); } Let's only mark `SymbolData`-type symbol

[PATCH] D48681: [CFG] [analyzer] Add construction contexts for function arguments.

NoQ updated this revision to Diff 155091. NoQ added a comment. Address my own comments. https://reviews.llvm.org/D48681 Files: include/clang/Analysis/CFG.h include/clang/Analysis/ConstructionContext.h lib/Analysis/CFG.cpp lib/Analysis/ConstructionContext.cpp lib/StaticAnalyzer/Core/Ex

[PATCH] D49210: [CFG] [analyzer] NFC: Enumerate construction context layer kinds and re-use their code for ExprEngine keys.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware. This is a refactoring patch; no functional change intended. `ConstructionContext` is composed of `Const

[PATCH] D22391: [Sema] Add warning for implicitly casting a null constant to a non null pointer type

NoQ added inline comments. Comment at: test/Analysis/nullability_nullonly.mm:103 void testObjCARCExplicitZeroInitialization() { - TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}} +

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

NoQ abandoned this revision. NoQ added a comment. Herald added subscribers: mikhail.ramalho, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. Outdated by https://reviews.llvm.org/D44131. https://reviews.llvm.org/D27202 ___ cfe-commits ma

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

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware. @alexfh provided an example that demonstrates that elided copy constructors may get agglutinated due to

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

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:432-433 ConstructedObjectKey Key(Item, LC->getCurrentStackFrame()); // FIXME: Currently the state might already contain the marker due to // incorrect handling of temporaries bound to default p

[PATCH] D49215: [analyzer] Admit that some copy/move constructors have more than one argument.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs. Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware. Copy/move constructors may have additional default arguments and still be treated as normal copy/move c

[PATCH] D49215: [analyzer] Admit that some copy/move constructors have more than one argument.

NoQ updated this revision to Diff 155105. NoQ added a comment. Actually verify the CFG in the test. https://reviews.llvm.org/D49215 Files: lib/Analysis/CFG.cpp test/Analysis/cfg-rich-constructors.cpp Index: test/Analysis/cfg-rich-constructors.cpp ==

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

NoQ accepted this revision. NoQ added a comment. Looks good! 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 -

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

NoQ accepted this revision. NoQ added a comment. Yay less code. https://reviews.llvm.org/D48325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

NoQ added a comment. > A call to `Derived::Derived()` previously emitted no warnings. However, with > these changes, a warning is emitted for `Base::a`. Yep, a heuristic to skip implicit constructor declarations during analysis and make the first explicit constructor responsible for initializin

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

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674 + const LocationContext *LC = Context.getLocationContext(); + while ((LC = LC->getParent())) { + george.karpenkov wrote: > nit: could we have `while (LC)` foll

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

NoQ added a comment. I'd also rather stick to integer arithmetic and avoid using floats even in intermediate calculations. It'd be hard to make sure that no rounding errors kick in if we use floats. https://reviews.llvm.org/D49074 ___ cfe-commits

[PATCH] D48764: [Analyzer] Hotfix for iterator checkers: Mark left-hand side of `SymIntExpr` objects as live in the program state maps.

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks good otherwise, please commit. https://reviews.llvm.org/D48764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D48831: alpha.unix.cstring.OutOfBounds checker enable/disable fix

NoQ accepted this revision. NoQ added a comment. Whoops, sry, yeah, looks good, please commit! https://reviews.llvm.org/D48831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49361: [analyzer][WIP] Detect pointers escaped after return statement execution in MallocChecker

NoQ added a comment. Aha, so the destructors are called //after// the return statement! Makes sense. The return statement is available in `ExprEngine::processEndOfFunction()`, so i guess we could improve the `checkEndFunction()` callback to provide it, and then we wouldn't need any of those ext

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Cool! I don't have a strong preference with respect to whitelist vs. blacklist; your approach is safer but listing functions that don't immediately invalidate the buffer would allow us to avoid hard

[PATCH] D49387: [analyzer] Make checkEndFunction() give access to the return statement

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great, thanks! Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:501 - void _registerForBeginFunction(CheckEndFunctionFunc checkfn); + void _registerForBegin

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

NoQ added a comment. Herald added a subscriber: mikhail.ramalho. Just noticed: `getRuntimeDefinition()` has a lot of overrides in `CallEvent` sub-classes, and there paths that don't defer to `AnyFunctionCall::getRuntimeDefinition()`, eg., ` CXXInstanceCall::getRuntimeDefinition()` => `if (MD->i

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

NoQ added a comment. Hmm, the destructor-specific message was pretty good, can we keep it? It should be possible to print a different message depending on the program point within `N`. https://reviews.llvm.org/D49360 ___ cfe-commits mailing list c

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

NoQ added a comment. I showed the bug mentioned in https://reviews.llvm.org/D49058 to a friend who didn't do much C++ recently, for a fresh look, and he provided a bunch of interesting feedback by explaining the way he didn't understand what the analyzer was trying to say. 1. When we call `c_s

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

NoQ added a comment. Aha, cool, so it's probably just virtual functions. Repository: rC Clang https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

NoQ accepted this revision. NoQ added a comment. Let's commit this patch and make another round later, as we gather ideas for better names and messages. https://reviews.llvm.org/D49360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

NoQ added inline comments. Comment at: test/Analysis/dangling-internal-buffer.cpp:175 std::string s; - { -c = s.c_str(); - } - consume(c); // no-warning + c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} + s.clear(); // expected-no

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

NoQ added a comment. Herald added a subscriber: mikhail.ramalho. There are still performance regressions coming in, and this time it doesn't look like it's my fault: https://bugs.llvm.org/show_bug.cgi?id=38208 I suspect that this might be because we aren't enforcing complexity thresholds over a

[PATCH] D68591: [analyzer] PR43551: Do not dereferce void* in UndefOrNullArgVisitor

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks!! Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2030-2033 // FIXME: this is a hack for fixing a later crash when attempting to // dereference a

[PATCH] D68591: [analyzer] PR43551: Do not dereferce void* in UndefOrNullArgVisitor

NoQ added a comment. I think it's worth it to commit the patch as-is, because the crash seems to be fairly popular. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68591/new/ https://reviews.llvm.org/D68591 ___

[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. I need this for the demo at the dev me

[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

NoQ updated this revision to Diff 225540. NoQ marked an inline comment as done. NoQ added a comment. Fxd. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69015/new/ https://reviews.llvm.org/D69015 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h clang/lib/

[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

NoQ added inline comments. Comment at: clang/test/Analysis/exploded-graph-rewriter/node_labels.dot:46 +// COLOR-SAME: Sink Node +// GREY-SAME: Sink Node +// CHECK-SAME: Charusso wrote: > `GREY` typo. Whoops!! CHANGES SINCE LAST ACTION https://revi

[PATCH] D69015: [analyzer] Make ExplodedNode identifiers truly stable.

NoQ closed this revision. NoQ added a comment. Closed by rC375186 but i forgot the phabricator link :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69015/new/ https://reviews.llvm.org/D69015 ___ cfe-com

[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/test/Analysis/dump_egraph.c:32 +// CHECK-SAME:\"pretty\": \"0\", \"location\": \{ +// CHECK-SAME:\"line\": 15, \"column\": 12, \"file\": \" +// CHECK-SAME:\}, \"stmt_point_kind\": \

[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. NoQ marked an inline comment as done. N

[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. This is the bug that I'll be using in

[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:325 if (getKind() != CE_CXXAllocator) if (isArgumentConstructedDirectly(Idx)) if (auto AdjIdx = getAdjustedParameterIndex(Idx)) --

[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/test/Analysis/dump_egraph.c:46 // CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 18, \"column\": 10, \"file\": \"{{(.+)}}dump_egraph.c\" \} Charusso wrote: > `\"file\"

[PATCH] D69150: [analyzer] Fix hidden node traversal in exploded graph dumps.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7a17f197093a: [analyzer] Fix hidden node traversal in exploded graph dumps. (authored by dergachev.a). Changed prior to commit: https://reviews.llvm.org/D69150?vs=225547&id=225685#toc Repository: rG

[PATCH] D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized.

NoQ edited reviewers, added: NoQ; removed: dergachev.a. NoQ added inline comments. Comment at: clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist:191 -10 14 It looks like the body farm for the property accessor has stopped working

[PATCH] D68591: [analyzer] PR43551: Do not dereferce void* in UndefOrNullArgVisitor

This revision was automatically updated to reflect the committed changes. Closed by commit rG4a5df7312ec2: [analyzer] PR43551: Do not dereferce void* in UndefOrNullArgVisitor. (authored by dergachev.a). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6

[PATCH] D68591: [analyzer] PR43551: Do not dereferce void* in UndefOrNullArgVisitor

NoQ added a comment. Decided to commit myself in order to hurry things up^^ Also rC375328 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68591/new/ https://reviews.llvm.org/D68591 _

[PATCH] D69155: [analyzer] Fix off-by-one in operator call parameter binding.

This revision was automatically updated to reflect the committed changes. Closed by commit rGbe86fdb86e1e: [analyzer] Fix off-by-one in operator call parameter binding. (authored by dergachev.a). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69155/ne

[PATCH] D68725: [analyzer] MemoryBlockRegion: Generalize AllocaRegion

NoQ added a comment. > This patch generalizes the `AllocaRegion` to store metadata about the > expression of the allocation of a memory block. Mmm, but you can easily obtain that expression from the existing `AllocaRegion`. I.e., you can obtain the expression on which the memory was allocated,

[PATCH] D69308: [analyzer] Test cases for the unsupported features for Clang Static Analyzer

NoQ added a comment. Thanks for the tests! Both of these features are relatively hard. Operator `new[]` requires invoking multiple (potentially unknown) amount of constructors with the same construct-expression. Apart from the technical difficulties of juggling program points around correctly

[PATCH] D78122: [analyzer][Nullability] Don't emit under the checker name NullabilityBase

NoQ added a comment. We're observing a surprising disappearance of nullability reports today - like, they didn't just change the name, they disappeared completely. Investigating. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78122/new/ https://rev

[PATCH] D77880: get scan-view executable from environment

This revision was automatically updated to reflect the committed changes. Closed by commit rG3f333e0af7a8: [analyzer] Get scan-view executable from environment. (authored by dergachev.a). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77880/new/ http

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great, thanks! This is probably not the precise solution (i.e., we might be able to see where exactly is the reference taken and proceed from there) but neither is the original code - it's al

[PATCH] D77802: [analyzer] Improved RangeSet::Negate support of unsigned ranges

NoQ accepted this revision. NoQ added a subscriber: vsavchenko. NoQ added a comment. This revision is now accepted and ready to land. Looks fantastic, thanks! I guess let's race with @vsavchenko on whoever commits first :p CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77802/new/ https:

[PATCH] D77177: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker & shared utils

NoQ added a comment. > Reverted in 1108f5c737dbdab0277874a7e5b237491839c43a > for now. Uh-oh. Thank you for reverting! > Project-specific checks like this usually go in clang-tidy, not in the static > analyzer (which ships w

[PATCH] D80423: [analyzer] SATestBuild.py: Refactor and add type annotations

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Nice! I guess next time it'd be easier to review if you split up style changes from structural changes. Comment at: clang/utils/analyzer/SATestBuild.py:276 -AllCheckers = C

[PATCH] D80424: [analyzer] SATestUtils.py: Refactor and add type annotations

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Sure thanks! Comment at: clang/utils/analyzer/SATestUtils.py:47 +root, ext = os.path.splitext(file_name) +return ext in (".i", ".ii", ".c", ".cpp", ".m", "")

[PATCH] D80427: [analyzer] SumTimerInfo.py: Partially modernize

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. This one's straightforward too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80427/new/ https://reviews.llvm.org/D80427 __

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112 +Optional ExprEngine::retrieveFromConstructionContext( +ProgramStateRef State, const LocationContext *LCtx, Please instead re-use the code that computes the object un

[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. This looks straightforward, thanks! Comment at: clang/test/Analysis/explain-svals.cpp:97 void test_6() { - clang_analyzer_explain(conjure_S()); // expected-warning-re^lazily

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG99b94f29ac5d: [analyzer] LoopUnrolling: fix crash when a parameter is a loop counter. (authored by dergachev.a). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D80443: [analyzer] SATestBuild.py: Fix hang when one of the tasks fails

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Not unreasonable! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80443/new/ https://reviews.llvm.org/D80443

[PATCH] D80548: [Analyzer][NFC] Remove the SubEngine interface

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I absolutely buy this. If somebody is to ever implement another `ExprEngine`, dealing with the lack of a convenient virtual interface will definitely be a negligible problem for them. ===

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

NoQ added a comment. Nice, looks like you managed to reuse most of the code. I still feel like we should ditch `DeclRegion` entirely (forcing its ~5 current users consider its specific variants separately) but i don't insist. Comment at: clang/include/clang/StaticAnalyzer/Che

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112 +Optional ExprEngine::retrieveFromConstructionContext( +ProgramStateRef State, const LocationContext *LCtx, baloghadamsoftware wrote: > NoQ wrote: > > Please instead

[PATCH] D80626: [analyzer] SATestBuild.py: Make verbosity level a cmd option

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Sounds great! I'm still in favor of at least some lit tests for these scripts. It should be as simple as adding a tiny lit substitution

[PATCH] D80669: [analyzer] LoopWidening: fix crash by avoiding aliased references invalidation

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Fair! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80669/new/ https://reviews.llvm.org/D80669 ___ cfe-

[PATCH] D78990: [analyzer] Allow bindings of the CompoundLiteralRegion

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Great, thanks! Comment at: clang/unittests/StaticAnalyzer/StoreTest.cpp:66 -Store StInit = StMgr.getInitialStore(SFC).getStore(); -SVal Zero = SVB.makeZeroVal(ACtx.IntTy)

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Aha, so performance regressions on real code weren't real, that's a relief :) > I believe that as of now we can submit these modifications as is and explore > performance optimizations later if need

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112 +Optional ExprEngine::retrieveFromConstructionContext( +ProgramStateRef State, const LocationContext *LCtx, balazske wrote: > baloghadamsoftware wrote: > > baloghadam

[PATCH] D79434: [analyzer] Generalize bitwise AND rules for ranges

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

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

NoQ added a comment. In D80117#2059567 , @NoQ wrote: > > I believe that as of now we can submit these modifications as is and > > explore performance optimizations later if needed. > > I still encourage you to explore the tests we have from our previous a

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:112 +Optional ExprEngine::retrieveFromConstructionContext( +ProgramStateRef State, const LocationContext *LCtx, baloghadamsoftware wrote: > martong wrote: > > baloghadams

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

NoQ added a comment. https://bugs.llvm.org/show_bug.cgi?id=46128 looks like a crash caused by this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https://reviews.llvm.org/D79330 ___ c

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:114-115 // Convert the array length to size_t. NonLoc IndexLength = SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs(); // Multiply the array length by the elem

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

NoQ added a comment. I think this patch is a fairly historic moment to celebratte. > checker callback evaluation order is ensured And it will be the same for all callbacks, right? Like, we're not doing this thing yet where it intentionally goes like chk1->checkPreCall(); chk2->checkPreCall

[PATCH] D80522: [Analyzer] [NFC] Parameter Regions -- Alternative Approach

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great, thanks! I like how it turned out, i guess let's prefer this to D79704 . Comment at: clang/include/clang/StaticAnalyzer/Core/PathSens

[PATCH] D77178: [Analyzer][WebKit] NoUncountedMembersChecker

NoQ added a comment. Looks similar to https://bugs.llvm.org/show_bug.cgi?id=46142. It sounds pretty important to address quickly because it crashes on a fairly large portion of C++ code with default settings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:114-115 // Convert the array length to size_t. NonLoc IndexLength = SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs(); // Multiply the array length by the elem

[PATCH] D80905: [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

NoQ added a reviewer: vrnithinkumar. NoQ added a comment. +Nithin because it may be relevant to the smart pointer checker if inter-checker communication turns out to be necessary (eg., with the move checker). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D80517: [analyzer] CmpRuns.py: Refactor and add type annotations

NoQ added a comment. I guess this one's stuck on the discussion about whether we need to retain python2 support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80517/new/ https://reviews.llvm.org/D80517 __

[PATCH] D80366: [Analyzer] Add `getReturnValueUnderConstruction()` to `CallEvent`

NoQ added reviewers: vrnithinkumar, vsavchenko, xazax.hun. NoQ added a comment. +GSoC gang because it'll ultimately help Nithin's future checker track smart pointers returned from functions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80366/new/ https://reviews.llvm.org/D80366 ___

<    12   13   14   15   16   17   18   19   20   21   >