[PATCH] D42457: [analyzer] Don't communicate evaluation failures through memregion hierarchy.

NoQ added a comment. > Functional change here is accidental - by communicating array destructor > situation properly, we're able to fix an old FIXME. Minor temporary insanity. This test was "fixed" because in `mayInlineCall()` for destructors i started to look for the flag that i never set for

[PATCH] D42991: [analyzer] NFC: Use EvalCallOptions for destructors as well.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. In https://reviews.llvm.org/D42457 we added the `EvalCallOptions` structure to notify `evalCall()` when some other code believes that there's

[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.

NoQ updated this revision to Diff 133106. NoQ added a comment. Remove a couple of accidental blank lines. https://reviews.llvm.org/D42991 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

[PATCH] D42645: New simple Checker for mmap calls

NoQ added a comment. > What I propose is "two" separated checkers one using same but the value of > PROT_EXEC will depend on the checker name e.g. security.MmapWriteExecGen vs > security.MmapWriteExecGlibc ... or by default we keep PROT_EXEC as is and we > would allow to override the value via

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ updated this revision to Diff 133139. NoQ added a comment. Make CFGConstructor a sub-class of CFGStmt. I failed to notice any compile time regressions for now; if any, they must be super bottlenecked on CFG construction or analysis-based warnings. After poking @rsmith in the chat a little b

[PATCH] D42719: [CFG] [analyzer] Add construction context when constructor is wrapped into ExprWithCleanups.

NoQ updated this revision to Diff 133142. NoQ added a comment. Rebase. https://reviews.llvm.org/D42719 Files: lib/Analysis/CFG.cpp test/Analysis/cfg-rich-constructors.cpp test/Analysis/temp-obj-dtors-cfg-output.cpp Index: test/Analysis/temp-obj-dtors-cfg-output.cpp ==

[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.

NoQ updated this revision to Diff 133299. NoQ added a comment. Remove the check in `shouldInlineCall()` as well. It's quite covered by the `IsConstructorWithImproperlyModeledTargetRegion` check. https://reviews.llvm.org/D42991 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine

[PATCH] D43056: [CFG] [analyzer] Add construction context for CXXBindTemporaryExpr.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. This covers temporary constructors for which a destructor is necessary. It is necessary to support `CXXTemporaryObjectExpr` here, which is a

[PATCH] D16403: Add scope information to CFG

NoQ added a comment. I poked Devin offline and we agreed that the overall approach is good to go. Maxim, thank you for picking it up! We still don't have scopes for segments of code that don't have any variables in them, so i guess it's not yet in the shape where it is super useful for loops,

[PATCH] D16403: Add scope information to CFG

NoQ added a reviewer: rsmith. NoQ added a subscriber: rsmith. NoQ added a comment. Added @rsmith because we're trying to give him a heads up every time large CFG changes are coming, because if we mess up it would affect not only the analyzer but the whole compiler through analysis-based compiler

[PATCH] D43062: [analyzer] Support CXXTemporaryObjectExpr constructors that have destructors.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. NoQ added a dependency: D43056: [CFG] [analyzer] Add construction context for CXXBindTemporaryExpr.. Patch https://reviews.llvm.org/D43056 ad

[PATCH] D43062: [analyzer] Support CXXTemporaryObjectExpr constructors that have destructors.

NoQ updated this revision to Diff 133475. NoQ added a comment. Add a simple test for a class with no destructor. https://reviews.llvm.org/D43062 Files: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp test/Analysis/temporaries.cpp Index: test/Analysis/temporaries.cpp =

[PATCH] D42300: [Analyzer] Add PreStmt and PostStmt callbacks for OffsetOfExpr

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. All right, i guess we already do have a pair of callbacks for `IntegerLiteral` and it doesn't hurt, especially because here they'd eventually be actually useful. I think it should land, just to make

[PATCH] D42645: New simple Checker for mmap calls

NoQ added a comment. Hmm, maybe it'd also be a good idea to disable the check completely when a likely-correct value for the macro cannot be determined. Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:74-75 + mgr.registerChecker(); + Mwec->ProtExecOv = +

[PATCH] D42745: [analyzer] Add support for __builtin_constant_p to BuiltinFunctionChecker

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Tests look good. You can't test deadcode :) https://reviews.llvm.org/D42745 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.o

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

NoQ added a comment. All right, so this change is indeed safe, i.e. no crashes or changes in analyzer behavior so far on my large codebase run. Will commit now. https://reviews.llvm.org/D42672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43074: [Analyzer] Fix a typo about `categories::MemoryError` in `MallocChecker.cpp`

NoQ accepted this revision. NoQ added a comment. Woohoo thanks nice. Repository: rC Clang https://reviews.llvm.org/D43074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43074: [Analyzer] Fix a typo about `categories::MemoryError` in `MallocChecker.cpp`

NoQ added a comment. I guess this happened due to a race with https://reviews.llvm.org/rL301913. My bad anyways^^ Repository: rC Clang https://reviews.llvm.org/D43074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D42745: [analyzer] Add support for __builtin_constant_p to BuiltinFunctionChecker

NoQ added a comment. Do you have commit access, or do you want me to commit this for you? https://reviews.llvm.org/D42745 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43104: [analyzer] Find correct region for simple temporary destructor calls and inline them if possible.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:68-71 typedef llvm::ImmutableMap, SVal> -

[PATCH] D43104: [analyzer] Find correct region for simple temporary destructor calls and inline them if possible.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:68-71 typedef llvm::ImmutableMap, SVal> -CXXNewAllocatorValuesMap; + const LocationContext *>, + SVal> +CXXNewAllocatorValuesMa

[PATCH] D43104: [analyzer] Find correct region for simple temporary destructor calls and inline them if possible.

NoQ updated this revision to Diff 133672. NoQ marked an inline comment as done. NoQ added a comment. Assert that the destructors are cleaned up. This assertion is pretty important because it says that we didn't miss any destructors for initialized temporaries during analysis. Disable tracking i

[PATCH] D42779: [analyzer] NFC: Make sure we don't ever inline the constructor for which the temporary destructor is noreturn and missing.

NoQ updated this revision to Diff 133675. NoQ marked an inline comment as done. NoQ added a comment. Add the comment. https://reviews.llvm.org/D42779 Files: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp ===

[PATCH] D43144: [analyzer] Implement path notes for temporary destructors.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. Temporaries are destroyed at the end of their `CXXBindTemporaryExpr`, which can be picked up from their `CFGTemporaryDtor`. Note that lifetim

[PATCH] D43144: [analyzer] Implement path notes for temporary destructors.

NoQ updated this revision to Diff 133687. NoQ added a comment. Minor indent fix. https://reviews.llvm.org/D43144 Files: lib/StaticAnalyzer/Core/PathDiagnostic.cpp test/Analysis/inlining/temp-dtors-path-notes.cpp Index: test/Analysis/inlining/temp-dtors-path-notes.cpp =

[PATCH] D43144: [analyzer] Implement path notes for temporary destructors.

NoQ added inline comments. Comment at: test/Analysis/inlining/temp-dtors-path-notes.cpp:17 +// expected-note@-2{{Returning from constructor for 'C'}} +// expected-note@-3{{Calling '~C'}} +} george.karpenkov wrote: > Should we have "returning from

[PATCH] D43144: [analyzer] Implement path notes for temporary destructors.

NoQ added a comment. Another question is why do we have such inconsistency between `Calling constructor for 'C'` and `Calling '~C'`, i.e. why not `Calling destructor for 'C'`. Seems accidental. https://reviews.llvm.org/D43144 ___ cfe-commits maili

[PATCH] D43144: [analyzer] Implement path notes for temporary destructors.

NoQ updated this revision to Diff 133692. NoQ added a comment. Add a test for returning from destructor. https://reviews.llvm.org/D43144 Files: lib/StaticAnalyzer/Core/PathDiagnostic.cpp test/Analysis/inlining/temp-dtors-path-notes.cpp Index: test/Analysis/inlining/temp-dtors-path-notes.

[PATCH] D43149: [analyzer] Fix a crash on destroying a temporary array.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. Tried to actually run the analyzer with temporary destructor support on some real code, found two crashes for now - https://reviews.llvm.org/

[PATCH] D43149: [analyzer] Fix a crash on destroying a temporary array.

NoQ updated this revision to Diff 133718. NoQ added a comment. > And even then, calling a destructor of a single array element does not > invalidate the whole array for us, because destructors are `const` (unless > there are mutable members). So we'd have to do this manually later as well. Hmm,

[PATCH] D42721: [analyzer] NFC: Use construction contexts for finding the target region for the construction.

NoQ updated this revision to Diff 133732. NoQ added a comment. Rebase. https://reviews.llvm.org/D42721 Files: include/clang/Analysis/CFG.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Index: lib/StaticAnalyzer/Core/ExprEngineCXX.c

[PATCH] D42876: [analyzer] Support returning objects by value.

NoQ updated this revision to Diff 133933. NoQ added a comment. Well, it still seems to be a reasonable improvement given how all temporary materialization works now, and it's under the flag (`-analyzer-config cfg-temporary-dtors=true`), so i guess i'll mark it as TODO and commit, and i'll keep

[PATCH] D43104: [analyzer] Find correct region for simple temporary destructor calls and inline them if possible.

NoQ updated this revision to Diff 133956. NoQ added a comment. - Test `const C &c = coin ? C(x, y) : C(z, w);`. - Fix comments surrounding the assertion. https://reviews.llvm.org/D43104 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/CallEvent.cpp

[PATCH] D43104: [analyzer] Find correct region for simple temporary destructor calls and inline them if possible.

NoQ updated this revision to Diff 134341. NoQ added a comment. All right, so the assertion that we actually destroy all temporaries in our `InitializedTemporaries` map is still violated quite often - every time we do any sort of lifetime extension, we're actually calling the destructor on a di

[PATCH] D42876: [analyzer] Support returning objects by value.

NoQ updated this revision to Diff 134346. NoQ added a comment. Update the comment with some thoughts from http://lists.llvm.org/pipermail/cfe-dev/2018-February/056898.html https://reviews.llvm.org/D42876 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer

[PATCH] D43104: [analyzer] Find correct region for simple temporary destructor calls and inline them if possible.

NoQ updated this revision to Diff 134351. NoQ added a comment. Whoops - recall that we still need to cleanup the temporaries map even, now that we know that we cannot assert that it's already empty. Clean up the map and enable the assertion that becomes tautological until the respective FIXME i

[PATCH] D42876: [analyzer] Support returning objects by value.

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Committed this but put a wrong phabricator link in the commit message, sorry >_< https://reviews.llvm.org/rC325202 https://reviews.llvm.org/D42876 ___

[PATCH] D42875: [CFG] [analyzer] Add construction context for ReturnStmt.

NoQ added a comment. https://reviews.llvm.org/rC325202 is attached to this revision accidentally; it should have been attached to https://reviews.llvm.org/D42876. Repository: rL LLVM https://reviews.llvm.org/D42875 ___ cfe-commits mailing list c

[PATCH] D43428: [CFG] NFC: Allow more complicated construction contexts.

NoQ created this revision. NoQ added reviewers: rsmith, dcoughlin, xazax.hun, a.sidorin. Herald added subscribers: cfe-commits, rnkovacs, kristof.beyls, mehdi_amini. `ConstructionContext`s introduced in https://reviews.llvm.org/D42672 are an additional piece of information included with `CFGConst

[PATCH] D43428: [CFG] NFC: Allow more complicated construction contexts.

NoQ updated this revision to Diff 134785. NoQ added a comment. Fix comment. https://reviews.llvm.org/D43428 Files: include/clang/Analysis/CFG.h lib/Analysis/CFG.cpp Index: lib/Analysis/CFG.cpp === --- lib/Analysis/CFG.cpp +++

[PATCH] D42266: [analyzer] Prevent AnalyzerStatsChecker from crash

NoQ accepted this revision. NoQ added a subscriber: george.karpenkov. NoQ added a comment. This revision is now accepted and ready to land. Herald added a reviewer: george.karpenkov. LGTM! @george.karpenkov has also tested that when he was gathering statistics about his traversal order improvemen

[PATCH] D16403: Add scope information to CFG

NoQ added a comment. In https://reviews.llvm.org/D16403#1010096, @szepet wrote: > In https://reviews.llvm.org/D16403#992454, @NoQ wrote: > > > @szepet: so i see that `LoopExit` goes in the beginning of the cleanup > > block after the loop, while various `ScopeEnd`s go after the `LoopExit`. > >

[PATCH] D43477: [CFG] [analyzer] Add MaterializeTemporaryExpr into the construction context.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. `MaterializeTemporaryExpr` captures lifetime extension information. In the analyzer it is important to have this information at construction

[PATCH] D43477: [CFG] [analyzer] Add MaterializeTemporaryExpr into the construction context.

NoQ added a comment. > eg. `const C &c(123);` or the actual (not the elidable copy) constructor in > `C foo() { return C(123); }` Emm, sry, never mind, forget it, i was trying to say that the reason why we don't have a `CXXBindTemporary` is because we don't have a destructor in class `C`, not

[PATCH] D43480: [CFG] [analyzer] Add construction context when the constructor is treated like a functional cast.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. When a constructor with a single argument is treated as a functional cast expression, skip the functional cast while finding the construction

[PATCH] D43481: [CFG] [analyzer] Add construction context when the constructor is being no-op-casted to a const value type.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. Because lifetime-extended temporaries are treated as const objects, an implicit `NoOp` cast to `const` usually surrounds them in the AST, som

[PATCH] D43483: [CFG] [analyzer] Add construction context when the constructor is on a branch of a ternary operator

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. Similarly to https://reviews.llvm.org/D43480 and https://reviews.llvm.org/D43481, we need to skip the ternary conditional operator `... ? ..

[PATCH] D43483: [CFG] [analyzer] Add construction context when the constructor is on a branch of a ternary operator

NoQ updated this revision to Diff 134979. NoQ added a comment. Fix the test. https://reviews.llvm.org/D43483 Files: lib/Analysis/CFG.cpp test/Analysis/cfg-rich-constructors.cpp test/Analysis/temp-obj-dtors-cfg-output.cpp Index: test/Analysis/temp-obj-dtors-cfg-output.cpp

[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. This patch uses the reference to `MaterializeTemporaryExpr` stored in the construction context since https://reviews.llvm.org/D43477 in order

[PATCH] D42645: New simple Checker for mmap calls

NoQ added a comment. I believe we should relocate this checker into `alpha.security` in order to indicate that this is still in development, so that you (or anyone else) could provide auto-detection for the macro values later as an incremental improvement, and then it will be back in `security`

[PATCH] D16403: Add scope information to CFG

NoQ added a comment. In https://reviews.llvm.org/D16403#1013346, @m.ostapenko wrote: > In https://reviews.llvm.org/D16403#1011218, @NoQ wrote: > > > Yeah, i mean, like, if we change the scope markers to also appear even when > > no variables are present in the scope, then it would be possible to

[PATCH] D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs, mgorny. NoQ added a dependency: D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.. `ConstructionContext` is m

[PATCH] D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases.

NoQ updated this revision to Diff 135154. NoQ added a comment. Remove unused methods. https://reviews.llvm.org/D43533 Files: include/clang/Analysis/CFG.h include/clang/Analysis/ConstructionContext.h lib/Analysis/CFG.cpp lib/Analysis/CMakeLists.txt lib/Analysis/ConstructionContext.cpp

[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.

NoQ updated this revision to Diff 135186. NoQ added a comment. Add one more FIXME test (`dont_forget_destructor_around_logical_op` in `temporaries.cpp`) which demonstrates a situation where we fail to call the temporary destructor after calling the temporary constructor. It should be fixed once

[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.

NoQ updated this revision to Diff 135312. NoQ marked 4 inline comments as done. NoQ added a comment. - Address comments. - Add a FIXME test case that demonstrates that automatic destructors don't fire after lifetime extension through a POD field, even though lifetime extension itself seems to wo

[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:396 +ProgramStateRef State, const LocationContext *FromLC, +const LocationContext *ToLC) { + const LocationContext *LC = FromLC; a.sidorin wrote: > EndLC? (similar to iterators

[PATCH] D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases.

NoQ updated this revision to Diff 135480. NoQ added a comment. Address comments. https://reviews.llvm.org/D43533 Files: include/clang/Analysis/CFG.h include/clang/Analysis/ConstructionContext.h lib/Analysis/CFG.cpp lib/Analysis/CMakeLists.txt lib/Analysis/ConstructionContext.cpp lib

[PATCH] D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases.

NoQ added inline comments. Comment at: include/clang/Analysis/ConstructionContext.h:119 + static const ConstructionContext * + finalize(BumpVectorContext &C, const ConstructionContextLayer *TopLayer); + a.sidorin wrote: > Maybe just `build()`? For me, `finalize

[PATCH] D43657: [analyzer] dump() dynamic type info and taint into state dumps.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. More dumps! Eg.: Dynamic types of regions: x : class PR13569_virtual::Child Taint dumps were already implemented, so i added them becau

[PATCH] D43659: [analyzer] Don't crash when dynamic type of a concrete region is hard-set with placement new.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. This is assertion removal that i find valid. With placement new (which isn't even need to be inlined, we used to model it conservatively well

[PATCH] D43659: [analyzer] Don't crash when dynamic type of a concrete region is hard-set with placement new.

NoQ updated this revision to Diff 135572. NoQ added a comment. - Add a definitely-not-UB example (`char` buffers are special). - Bring back an accidentally deleted blank line. https://reviews.llvm.org/D43659 Files: lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/new-dynamic-types.cpp

[PATCH] D43659: [analyzer] Don't crash when dynamic type of a concrete region is hard-set with placement new.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:593 +// Fall back to a generic pointer cast for this-value. +const CXXMethodDecl *StaticMD = cast(getDecl()); +const CXXRecordDecl *StaticClass = StaticMD->getParent(); --

[PATCH] D43659: [analyzer] Don't crash when dynamic type of a concrete region is hard-set with placement new.

NoQ added a comment. > In this case, could we emit a warning? If not from CallEvent, then from where? (1) This might result in a buffer overflow, so i home that `alpha.ArrayBound` may eventually catch it. (2) It might be a good idea to make a fairly generic checker for the strict aliasing rule.

[PATCH] D43666: [analyzer] When constructing a temporary without construction context, track it for destruction anyway.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. This brings back code that was doing so earlier (when we had no constructed contexts at all) but was removed too early in https://reviews.llv

[PATCH] D42645: New simple Checker for mmap calls

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thank you! I'll try to commit again. https://reviews.llvm.org/D42645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D43497: [analyzer] Introduce correct lifetime extension behavior in simple cases.

NoQ added a comment. > Add a FIXME test case that demonstrates that automatic destructors don't fire > after lifetime extension through a POD field, even though lifetime extension > itself seems to work correctly. This has always been broken - the CFG element for the automatic destructor is si

[PATCH] D43533: [CFG] [analyzer] NFC: Refactor ConstructionContext into a finite set of cases.

NoQ updated this revision to Diff 135667. NoQ added a comment. Missing break!~~ https://reviews.llvm.org/D43533 Files: include/clang/Analysis/CFG.h include/clang/Analysis/ConstructionContext.h lib/Analysis/CFG.cpp lib/Analysis/CMakeLists.txt lib/Analysis/ConstructionContext.cpp lib/

[PATCH] D43689: [analyzer] Disable constructor inlining when lifetime extension through fields occurs.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. As i mentioned in https://reviews.llvm.org/D43497, automatic destructors are missing in the CFG in situations like const int &x = C().x;

[PATCH] D43714: [analyzer] Don't do anything when trivial-copying an empty class object.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added a subscriber: rnkovacs. When modeling implicit copy/move-constructor or copy/move-assignment operator of an empty class, don't do anything. The previous behavior was to take the

[PATCH] D43689: [analyzer] Disable constructor inlining when lifetime extension through fields occurs.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:177 + assert(VD->getType()->isReferenceType()); + if (VD->getType()->getPointeeType().getCanonicalType() != + MTE->GetTemporaryExpr()->getType().getCanonicalType()) { -

[PATCH] D43666: [analyzer] When constructing a temporary without construction context, track it for destruction anyway.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2262 + assert(DidCacheOutOnCleanup || + areInitializedTemporariesClear(Pred->getState(), Pred->getLocationContext(), dcoughlin wrote: >

[PATCH] D43666: [analyzer] When constructing a temporary without construction context, track it for destruction anyway.

NoQ updated this revision to Diff 135994. NoQ marked 3 inline comments as done. NoQ added a comment. Fix cleanup node generation logic. https://reviews.llvm.org/D43666 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/

[PATCH] D43791: [analyzer] Suppress MallocChecker positives in destructors with atomics.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. This patch is a targeted suppression heuristic for false positives `MallocChecker` produces when a shared / reference-counting pointer is cop

[PATCH] D43798: [analyzer] UndefinedAssignment: Fix warning message on implicit copy/move constructors.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs. When a class forgets to initialize a field in the constructor, and then gets copied around, a warning is emitted that the value assigned to a

[PATCH] D43791: [analyzer] Suppress MallocChecker positives in destructors with atomics.

NoQ updated this revision to Diff 136020. NoQ added a comment. Address comments. https://reviews.llvm.org/D43791 Files: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/NewDelete-atomics.cpp Index: test/Analysis/NewDelete-atomics.cpp =

[PATCH] D43791: [analyzer] Suppress MallocChecker positives in destructors with atomics.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2847 + + AtomicExpr::AtomicOp Op = AE->getOp(); + if (Op != AtomicExpr::AO__c11_atomic_fetch_add && george.karpenkov wrote: > IMO would be slightly easier to read with logic re

[PATCH] D43791: [analyzer] Suppress MallocChecker positives in destructors with atomics.

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2899 + // to find out if a likely-false-positive suppression should kick in. + for (const LocationContext *LC = CurrentLC; LC; LC = LC->getParent()) { +if (isa(LC->getDecl())) {

[PATCH] D32592: [Analyzer] Iterator Checker - Part 1: Minimal Checker for a Simple Test Case

NoQ added a comment. Sorry, got carried away by GSoC and critical stuff... In https://reviews.llvm.org/D32592#749613, @baloghadamsoftware wrote: > In https://reviews.llvm.org/D32592#747132, @NoQ wrote: > > > Then, in methods that deal with iterator `SVal`s directly, i wish we had > > hints expl

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I'd commit your patch without the .gitignore change, as it deserves a separate commit and more attention; will have a look at it myself - llvm's and clang's .gitignores have diverged quite a bit. T

[PATCH] D32592: [Analyzer] Iterator Checker - Part 1: Minimal Checker for a Simple Test Case

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I believe we can move on to the next one :) Just hope we didn't screw up the rebase too much here. Thanks again! https://reviews.llvm.org/D32592 ___ c

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. I'll land this. Thanks again for working on all that stuff! https://reviews.llvm.org/D30909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

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

NoQ added a comment. Uhm, messed up the phabricator link in https://reviews.llvm.org/rL304162, which should have been pointing to https://reviews.llvm.org/D30909 but points here instead. Repository: rL LLVM https://reviews.llvm.org/D28445 ___ c

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

NoQ closed this revision. NoQ added a comment. Uhm, messed up the phabricator link in https://reviews.llvm.org/rL304162, which should have been pointing here but points to https://reviews.llvm.org/D28445 instead. https://reviews.llvm.org/D30909 __

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

NoQ added a comment. We've broken something: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5288/steps/check-clang%20asan/logs/stdio I hope i fixed it in https://reviews.llvm.org/rL304170. https://reviews.llvm.org/D30909 ___

[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions

NoQ added a comment. Yep, fixed indeed. https://reviews.llvm.org/D30909 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

NoQ updated this revision to Diff 100673. NoQ marked 3 inline comments as done. NoQ added a comment. Fix comments. https://reviews.llvm.org/D32437 Files: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp lib/StaticAnalyzer/Co

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

NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:704 +const LocationContext *ParentLC = LC->getParent(); +while (ParentLC->getAnalysisDeclContext()->isBodyAutosynthesized()) { + LC = ParentLC; zaks.anna wrote: > Is Pa

[PATCH] D33671: [analyzer] In plist alternate mode, don't add weird control flow pieces from ObjC property declarations to uses.

NoQ created this revision. Herald added a subscriber: xazax.hun. These result in really weird arrows that have little to do with control flow. Other path diagnostic modes behave differently and seem to be unaffected. Patching on top of https://reviews.llvm.org/D32437, where this gets reproduced,

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

NoQ added a comment. I have just one comment and i think it'd be good to land. Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:104 + ProgramStateManager &Mgr = State->getStateManager(); + if (!LHSVal.getAs() && LHSVal.getAs()) { +LHSVal = Mgr.getStoreManager().getB

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

NoQ added a comment. An idea. I believe the safest way to find the bugs you mentioned would be to replace extent-as-a-symbol with extent-as-a-trait. I.e., currently we construct `extent_$3{SymRegion{conj_$1{char *}}}`, assume that it is equal to `reg_$0` (which produces a `SymSymExpr`) and then

[PATCH] D31868: [analyzer] Check NULL pointer dereference issue for memset function

NoQ added a comment. The code looks good now! A few minor comments and we can commit this :) Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2010 + const Expr *Mem = CE->getArg(0); + const Expr *Const = CE->getArg(1); + const Expr *Size = CE->getArg(2); --

[PATCH] D33828: [analyzer] Don't crash when the code tries to construct an Objective-C object in AllocaRegion.

NoQ created this revision. Herald added a subscriber: xazax.hun. The analyzer crashes when the user tries to allocate stack memory through `alloca()` and then construct an Objective-C object in it. The `alloca()` function is handled in the analyzer by its own concrete untyped memory region, `Al

[PATCH] D33828: [analyzer] Don't crash when the code tries to construct an Objective-C object in AllocaRegion.

NoQ updated this revision to Diff 101401. NoQ added a comment. Turn the comment into an assertion. https://reviews.llvm.org/D33828 Files: lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/DynamicTypePropagation.m Index: test/Analysis/DynamicTypePropagation.m

[PATCH] D34102: [analyzer] Add portability package for the checkers.

NoQ created this revision. Checkers that find implementation-defined behavior seem to better be off by default - or, at least, there should be a way to turn them off - because we're not sure if our users are developing cross-platform code or target a specific platform. If the behavior is well-d

[PATCH] D34102: [analyzer] Add portability package for the checkers.

NoQ added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:425 -def UnixAPIChecker : Checker<"API">, +def UnixAPIMisuseChecker : Checker<"API">, HelpText<"Check calls to various UNIX/Posix functions">, This rename is user-invisi

[PATCH] D34102: [analyzer] Add portability package for the checkers.

NoQ added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:454 +def UnixAPIPortabilityChecker : Checker<"API">, + HelpText<"Finds implementation-defined behavior in UNIX/Posix functions">, + DescFile<"UnixAPIChecker.cpp">; zaks.ann

[PATCH] D34277: [analyzer] Bump default performance thresholds?

NoQ created this revision. Because we now have faster CPUs and more RAM and stuff, should we now skew the balance to finding more bugs? We could probably make a few rounds of such changes, observing any delayed feedback from users who use default settings and aren't watching phabricator, and r

[PATCH] D34102: [analyzer] Add portability package for the checkers.

NoQ added a comment. Here's a version of the patch without `.unix`. I'd still hate it to re-add the subpackages if we decide to turn some portability checkers on and off depending on language/platform (eg. checkers for portability across linux/bsd should be off on windows by default, checkers f

[PATCH] D34102: [analyzer] Add portability package for the checkers.

NoQ updated this revision to Diff 102829. NoQ added a comment. Whoops, forgot to actually attach the patch. Here. https://reviews.llvm.org/D34102 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp test/Analysis/malloc-overflow2.c test

[PATCH] D34277: [analyzer] Bump default performance thresholds?

NoQ added a comment. This was an mixture of internal apple projects (user apps, drivers, deamons, whatever) with a relatively balanced selection of languages and levels of analyzer adoption. They amounted to ~16 hours of analysis CPU time (i.e. 4 hours on a quad-core machine per run). I've also

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