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

Szelethus accepted this revision. Szelethus added a comment. Herald added a subscriber: nullptr.cpp. This is amazing. We longed for a sensible implementation for this for a long time. Really liking the unit tests as well! There are a number of inlines you didn't mark as done but seem to have ad

[PATCH] D96163: [analyzer] Add 12.0.0. release notes

Szelethus updated this revision to Diff 322550. Szelethus added a comment. Fix'd! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96163/new/ https://reviews.llvm.org/D96163 Files: clang/docs/ReleaseNotes.rst clang/docs/analyzer/checkers.rst Index: clang/docs/analyzer/checkers.rst =

[PATCH] D96163: [analyzer] Add 12.0.0. release notes

Szelethus created this revision. Szelethus added reviewers: tstellar, NoQ, vsavchenko, dcoughlin, vrnithinkumar, xazax.hun, baloghadamsoftware, martong, balazske, steakhal, jkorous, dkrupp, gamesh411, ASDenysPetrov, aabbaabb, isthismyaccount. Szelethus added a project: clang. Herald added subscri

[PATCH] D91948: [WIP][analyzer][doc] Add Container- and IteratorModeling developer docs

Szelethus added a comment. I don't have the time to comb through this doc, unfortunately, but I want to applaud this effort to make the iterator checker family more accessible. Its certainly a forerunner in modeling tricky C++ constructs, and I can't wait to be a more valuable reviewer after be

[PATCH] D83678: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG22e7182002b5: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator (authored by Szelethus). Changed p

[PATCH] D89987: [analyzer] [NFC] Rename SymbolRef to SymExprRef

Szelethus added a comment. In D89987#2350649 , @NoQ wrote: > Honestly i'd rather eliminate `SymExpr` and go with `Symbol` everywhere. It's > an overloaded term but appending "Expr" to it doesn't really make it > significantly less overloaded. Why? I thi

[PATCH] D89414: clang/StaticAnalyzer: Stop using SourceManager::getBuffer

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Sounds good! Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89414/new/ https://reviews.llvm.org/D89414 ___ cfe-commits mailin

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

Szelethus added a subscriber: aaron.ballman. Szelethus added inline comments. Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307 .Case(FULLNAME, HELPTEXT) #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER NoQ wrote: > Szele

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. In D72705#2274568 , @balazske wrote: > I am not sure if this checker is worth to further development. A part of the > found bugs can be detected with other checkers too, specially if the > preconditions of many standard functio

[PATCH] D88312: "ErrorReturn checker" WIP review

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:505 + Report->markInteresting(CallSym); Report->addRange(CallLocation); C.emitReport(std::move(Report)); This still causes more harm then good

[PATCH] D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer

Szelethus added a comment. Congrats on your GSoC! Unless I missed it, it seems like you haven't posted your final evaluation on cfe-dev, even though its an amazing looking document with a lot of examples and explanations. I think it would be great to spread the message, its a work to be proud o

[PATCH] D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Yup, we've talked about this before. This is indeed a better interface design. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88100/n

[PATCH] D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. A joy of reviewing C++ code is that you get to marvel in all the great things the language has, without having to pull fistfuls of hair out to get get to that point. These patches are al

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

Szelethus accepted this revision. Szelethus 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/D87942/new/ https://reviews.llvm.org/D87942 __

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

Szelethus added a comment. The fix is great, thank you so much! The test seems to be annoying, it might be worth looking into it some other time, but that is definitely orthogonal to this patch. Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1211-1212 -

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

Szelethus added a comment. I figured you're still working on this, sorry! I'd really like to chat about my earlier comment D71524#1917251 , as it kind of challenges the high level idea. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71524/new/ h

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

Szelethus added a comment. Post-commit LGTM on the post-commit changes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87081/new/ https://reviews.llvm.org/D87081 ___ cfe-commits mailing list cfe-commits@l

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGdd1d5488e47d: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing… (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

This revision was automatically updated to reflect the committed changes. Closed by commit rG7c6f5b7fbf5a: [analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus added a comment. Sorry for the slack (which is kind of ironic with my attention grabbing title :) ). Landed in rG791b7e9f73e0064153a7c3db8045a7333a8c390c . Thanks everyone! CHANGES SINCE LAST ACTION https://revie

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. @balazske may have some closing words. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1262 Getline(LongLongTy

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

Szelethus created this revision. Szelethus added reviewers: xazax.hun, NoQ, vsavchenko, balazske, martong, baloghadamsoftware, steakhal. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnk

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

Szelethus created this revision. Szelethus added reviewers: xazax.hun, NoQ, vsavchenko, martong, balazske, baloghadamsoftware, steakhal. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnk

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

Szelethus updated this revision to Diff 291231. Szelethus added a comment. Rename the live statements checker to live expressions checker. The test file added in a revert commit changed rather heavily, but it makes sense that these entries are removed IMO. Unless anyone objects, I'll intend to l

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

This revision was automatically updated to reflect the committed changes. Closed by commit rGb9bca883c970: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a… (authored by Szelethus). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Szelethus updated this revision to Diff 291199. Szelethus marked 4 inline comments as done. Szelethus added a comment. Reinstate the assert removed in rG032b78a0762bee129f33e4255ada6d374aa70c71 , add a test. Enforce that `Envir

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Szelethus added a comment. In D86736#2257880 , @NoQ wrote: > i'm very glad that we now consistently use expressions in our Environment. *except for `ReturnStmt`, but I haven't dug myself into that pit just yet :) Thanks! Repository: rG LLVM Github Mo

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

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D79431#2263693 , @martong wrote: > In D79431#2263690 , @martong wrote: > >> What if we'd add a `toStri

[PATCH] D83678: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

Szelethus accepted this revision. Szelethus added a comment. LGTM on my end! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83678/new/ https://reviews.llvm.org/D83678 ___ cfe-commits mailing list cfe-comm

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

Szelethus added a comment. In D86135#2259325 , @steakhal wrote: > Perfectly clear, thank you. However, I would still rely on the others to > accept this :| > > BTW why does the `plist-macros-with-expansion.cpp.plist` change? It makes the > diff somewhat

[PATCH] D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. This is exactly how I imagines weak dependencies to work. LGTM on my end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87240/new/ https:/

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus updated this revision to Diff 290280. Szelethus marked 4 inline comments as done. Szelethus added a comment. Added a line about D78933 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86533/new/ https://reviews.llvm.org/D86533 Files: clang/do

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus added inline comments. Comment at: clang/docs/ReleaseNotes.rst:453 + equal or known to be non-equal. + +- Added :ref:`on-demand parsing ` capability to Cross Translation ASDenysPetrov wrote: > I've added the patch "Reasoning about comparison expressio

[PATCH] D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer

Szelethus added a comment. I can only imagine how long it took for you to write all this, because reading it wasn't that short either. Thank you so much! It really gave me a perspective on how you see this problem, as well as what is actually happening (and should happen) in the checker. In D8

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

Szelethus added a reviewer: NoQ. Szelethus added a comment. Herald added a subscriber: Charusso. NoQ in particular has been working hard on the common infrastructure in between the static analyzer and clang-tidy, I'll add him :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87118/new/

[PATCH] D76590: [Analyzer] Model `empty()` member function of containers

Szelethus added a comment. I'm in favor of most, if not all of the changes, though I will admit that this patch seems pretty cluttered, you are doing a lot of refactoring under the same hood. You're moving, adding, removing and changing helper functions and their invocations. Would be possible

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. @balazske seems to be very involved, he might have some closing words -- from my end, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

Szelethus added a comment. The tests look great, thanks! I still lack the confidence to accept, unfortunately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85351/new/ https://reviews.llvm.org/D85351 ___ cfe-commits mailing list cfe-commits@

[PATCH] D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite

Szelethus added a comment. The patch looks great, in fact, it demonstrates how well thought out your summary crafting machinery is. In D87081#2258579 , @martong wrote: > However, in a similar case with the CallAndMessage Checker, we decided to > list th

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM! Nice! Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:1 +// RUN: %clang_analyze_cc1 --std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. In D72705#2241542 , @balazske wrote: > The summary of this last discussion is that it is not acceptable to have only > the simple check for the explicit comparison with a fixed constant. At least > not for return types where th

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Szelethus added a comment. In D86736#2249920 , @martong wrote: > I don't have anymore immediate concerns, but I will need more time to comb > through the rest of the patch in more details... hopefully I can do that in > the following days. Thank you so

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:327 +using ObjCForLctxPair = +std::pair; + martong wrote: > Why it is not enough to simply have ObjCForCollectionStmt* as a key? `Environment` is the data structure we

[PATCH] D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2

Szelethus added a comment. Before we dive into this too much, if you can point to discussion that explains why we have a 2 versions of the same checker, that would be nice. Why did you chose to work on this one, and not the other? I recall us talking about this in a meeting, but its always grea

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus added a comment. I'd prefer to have a couple more green lights. In particular, another look on the constraint manager and Objective C stuff would be great. Comment at: clang/docs/ReleaseNotes.rst:487 +- While still in alpha, ``alpha.unix.PthreadLock``, the iterator a

[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

Szelethus added a comment. In D85351#2247037 , @baloghadamsoftware wrote: > In D85351#2215547 , @Szelethus wrote: > >> Shouldn't we create a new test care for this, instead of expanding an >> existing one? Btw, th

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Szelethus added a comment. Herald added a subscriber: danielkiss. In D86736#2244365 , @martong wrote: >> For any other loops, in order to know whether we should analyze another >> iteration, among other things, we evaluate it's condition. Which is a >> p

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

Szelethus added a comment. Herald added a subscriber: danielkiss. I'm not thrilled by this solution. As I understand it, the assertion was put there to enforce our ability to create a new assumption, and its great that we had it, because we managed to catch a fault in that. Seems like this patch

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:896-898 +RawLexer.reset(new Lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts, + MB->getBufferStart(), MacroNameTokenPos, +

[PATCH] D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

Szelethus created this revision. Szelethus added reviewers: NoQ, vsavchenko, dcoughlin, baloghadamsoftware, martong, balazske, xazax.hun, steakhal. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.s

[PATCH] D86691: [analyzer] Fix wrong parameter name in printFormattedEntry

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Nice, thank you! Did you stumble across this, or found it with a tool? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86691/new/ https://re

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus added inline comments. Comment at: clang/docs/ReleaseNotes.rst:471 + +- Improve the pre- and post condition modeling of several hundred more standard + C functions. whisperity wrote: > martong wrote: > > Umm, this is still an alpha command line option,

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus updated this revision to Diff 287930. Szelethus marked 14 inline comments as done. Szelethus added a comment. Fixes according to reviewer comments! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86533/new/ https://reviews.llvm.org/D86533 Files: clang/docs/ReleaseNotes.rst

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus requested changes to this revision. Szelethus added a subscriber: bruntib. Szelethus added a comment. This revision now requires changes to proceed. I debated this

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

Szelethus marked an inline comment as done and an inline comment as not done. Szelethus added a comment. I'll get the nits I didn't object to fixed, thats the status you're looking for :) Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1338-1339 void TokenPrint

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus added a comment. @vsavchenko I just realized your work for this release is the following list: git log llvmorg-11-init..llvmorg-11.0.0-rc2 --oneline -- clang/utils/analyzer/ Is it a correct guess that while your primary audience are the analyzer developers, but it wouldn't hurt to m

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

Szelethus added a reviewer: hans. Szelethus added a comment. We will need to push this to the 11.0.0. release branch as well, sorry for the trouble. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86532/new/ https://reviews.llvm.org/D86532

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

Szelethus created this revision. Szelethus added reviewers: hans, NoQ, vsavchenko, dcoughlin, xazax.hun, baloghadamsoftware, martong, balazske, steakhal, Charusso, jkorous, dkrupp, gamesh411. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, phosek, donat.nag

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

Szelethus created this revision. Szelethus added reviewers: xazax.hun, haowei, NoQ, martong, balazske, steakhal. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, phosek, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, b

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

Szelethus planned changes to this revision. Szelethus marked an inline comment as done. Szelethus added a comment. Thanks! I'll get these fixed. Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:901 + + void next(Token &Result) { +if (CurrTokenIt == TokenRange

[PATCH] D73376: [analyzer] Add FuchsiaLockChecker and C11LockChecker

Szelethus added a comment. Herald added subscribers: ASDenysPetrov, martong. Documentation under `clang/docs/analyzer/checkers.rst` seems to be missing. Could we get that fixed before 11.0.0 releases? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

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

Szelethus added a comment. In D86135#2233611 , @martong wrote: >> The fundamental problem is, we simply can't ask Preprocessor what a macro >> expands into without hacking really hard. > > Can you summarize what is the exact problem (or give a link to a d

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

Szelethus added a comment. Do I sense correctly that the only information `CSrtingLengthModeling.cpp` requires from the actual `CStringChecker` is a checker tag? Because if so, I think we should just separate them even more cleanly -- we could just make a `CStringLengthModeling` checker impleme

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

Szelethus added a comment. I really like the patch, but have nothing to add to what other reviewers already said. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86295/new/ https://reviews.llvm.org/D86295 _

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

Szelethus added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1226 +TStream.injextRange( +const_cast(PrevParamMap)[__VA_ARGS__II]); +TStream.next(TheTok); Oh, this has to be fixed as well.

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

Szelethus created this revision. Szelethus added reviewers: NoQ, vsavchenko, xazax.hun, martong, balazske, baloghadamsoftware, gamesh411. Szelethus added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnk

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

Szelethus added a comment. In D67422#2216137 , @NoQ wrote: > I agree that there's something here and also that it's not that > urgent/critical to rename things at this point. It's not that people suffer > horribly from having to link to only some of thes

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

Szelethus accepted this revision. Szelethus added a comment. Unless anyone else objects, I'm happy to see this landed! Nice work! ^-^ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67422/new/ https://reviews.llvm.org/D67422 ___ cfe-commits mai

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

Szelethus added inline comments. Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307 .Case(FULLNAME, HELPTEXT) #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER NoQ wrote: > This thing still worries me but this definitely is

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

Szelethus added a comment. Ah, okay, silly me. Didn't catch that. Then the message is OK for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79431/new/ https://reviews.llvm.org/D79431 ___ cfe-commits

[PATCH] D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

Szelethus added a comment. Shouldn't we create a new test care for this, instead of expanding an existing one? Btw, this looks great, but I lack the confidence to accept. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85351/new/ https://reviews.llv

[PATCH] D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions

Szelethus added reviewers: NoQ, vsavchenko. Szelethus accepted this revision. Szelethus added a comment. Lets make sure we invite the wider community to see whats going on. Otherwise, LGTM! It seems like this patch adds two new additions at once -- infrastructure for when the arg constraint siz

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

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Tests c: I'm still not a huge fan of the warning message. Now it describes //what kind// of constraint was violated, but not //how// (too large? too small?). Also, while I res

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

Szelethus added a comment. The patch looks great. I guess the only remaining discussion remains as to whether this should be in `libAnalysis`, or somewhere else. Here is my take: Since clang-tidy, CSA, and some other parts of the compiler (like uninitialized variable warnings) are static analyz

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

Szelethus added a comment. Layering violations are a running theme in the analyzer -- CheckerRegistry and the entire MallocChecker fiasco are two glaring examples. Luckily, this isn't a severe case so I wouldn't worry about it much. I've been follo

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. In D72705#2199333 , @balazske wrote: > Test results for tmux are available here >

[PATCH] D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange

Szelethus added a comment. Yeah, this looks straightforward, but how come you didn't manage to get a small test case? creduce gave up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85424/new/ https://reviews.llvm.org/D85424 __

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

Szelethus accepted this revision. Szelethus added a reviewer: gamesh411. Szelethus added a comment. LGTM on my end, but please wait for approval from either @gamesh411 or @NoQ. In D77150#2191049 , @baloghadamsoftware wrote: > However, I think we should c

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. Would it be possible to publish these results on a public CodeChecker server? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72705/new/ https://reviews.llvm.org/D72705 ___ cfe-c

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

Szelethus added a comment. We definitely need more tests. The idea overall however is amazing, thanks! Comment at: clang/test/Analysis/misc-ps-region-store.m:1160 struct list_pr8141 *items; - for (;; items = ({ do { } while (0); items->tail; })) // expected-warning{{Derefe

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

Szelethus added a comment. In D67422#2192407 , @NoQ wrote: > One bit that's not directly related but i decided to keep it from the > original patch was moving the plist-html diagnostic builder function into its > own file. That is a great call indeed.

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Let me double down -- just because we let some select people know about an option, I still don't think it should be user facing. I'm strongly against the philosophy that a core

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

Szelethus added reviewers: NoQ, vsavchenko. Szelethus accepted this revision. Szelethus added a comment. Lets make sure that we invite others from the community to participate, here and on other patches as well. Otherwise, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

Szelethus added a comment. In D82598#2172416 , @NoQ wrote: > I still wonder what made this statement live in my example. There must have > been some non-trivial liveness analysis going on that caused a statement to > be live; probably something to do wit

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

Szelethus added a comment. Wow, I never realized I accidentally landed that assert (D82122#2172360 ), but I guess its great to have that covered. Would you prefer to have that reverted as I'm looking to fix this for good? Repository: rG LLVM Github M

[PATCH] D82122: [analyzer][Liveness][RFC][NFC] Propose to turn statement liveness into expression liveness

Szelethus added a comment. This was accidentally committed as a part of rGb6cbe6cb0399d4671e5384dcc326af56bc6bd122 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82122/new/ htt

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

Szelethus added a comment. Thank you so much, you really went out of your way to dig that out! I'll try to patch this somehow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82598/new/ https://reviews.llvm.org/D82598

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

Szelethus added a comment. In D84316#2171270 , @NoQ wrote: > Imagine something like re-using the state trait implementation between > `MallocChecker` and `StreamChecker` because they both model "resources that > can be deallocated twice or leaked" - rega

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

Szelethus added a comment. I personally preferred the previous diff, the reporting part of the checker and the string length modeling was separated far more cleanly. In D84316#2171267 , @NoQ wrote: > Mmm, none of these benefits sound like they outweigh c

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

Szelethus added a comment. You could do it in the code, but if the modeling wouldn't be present from CStringModeling, CStringChecker wouldn't work properly. So you should make it a strong dependency, just as you did in this patch. My comment was mainly a response to @NoQ :)

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

Szelethus marked an inline comment as done. Szelethus added a comment. Yay! This checker has become a major headache as the analyzer grew. Not on a RetainCount scale, but on a MallocChecker one. Cheap shots, I know :) The patch looks great! I have some miscellaneous comments, but nothing blockin

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

Szelethus added a comment. Ouch. Let me know how severe this is, because this is a big milestone in my project. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82598/new/ https://reviews.llvm.org/D82598 ___

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

Szelethus marked an inline comment as done. Szelethus 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

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

Szelethus marked an inline comment as done. Szelethus added a comment. In D82598#2162496 , @xazax.hun wrote: > In D82598#2162239 , @Szelethus wrote: > > > I chased my own tail for weeks before realizing that there i

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

Szelethus added a comment. 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_analyzer_eval(bool); void test_lambda_refcapture() { int a = 6; [&](int &a) { a =

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

Szelethus added a comment. I find the summary here a bit lacking. We detected this issue in D83120 , where a lot of discussion can be found, so its worth linking in. On its own, I'm not immediately sold on whether this is the correct solution, and even if it is,

[PATCH] D83407: [analyzer][StdLibraryFunctionsChecker] Add POSIX networking functions

Szelethus accepted this revision. Szelethus 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/D83407/new/ https://reviews.llvm.org/D83407

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

Szelethus added a comment. I hope you don't mind if I bring up a point you mentioned during a daily meeting. > The error checking shouldn't be present on one of execution, but rather on > all of them. I think your recent comment highlights why that wouldn't be sufficient, unfortunately. Also,

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

Szelethus added a comment. I suppose LLVM could be a HUGE project? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83942/new/ https://reviews.llvm.org/D83942 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D83942: [analyzer][tests] Add a notion of project sizes

Szelethus added a comment. I don't speak snek, but I approve this message! Its also great that we have dedicated tiny/small projects to analyze locally. Comment at: clang/utils/analyzer/ProjectMap.py:31 +SMALL: 1min-10min +BIG: 10min-1h +HUGE: >1h --

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