[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:195 + + static const char *getTag() { return "FindLastStore"; } + I have made every tag a

[PATCH] D73521: [analyzer][WIP] add-new-checker.py: Introduction

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This script could be used to generate a dummy Static Analyzer

[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. - Repository: rC Clang https://reviews.llvm.org/D73520

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso added a comment. In D71433#1784238 , @NoQ wrote: > Currently the check may warn on non-bugs of the following kind: > > void foo() { > char env[] = "NAME=value"; > putenv(env); > doStuff(); > putenv("NAME=anothervalue"); > } >

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I have created a `notes.cpp` test-file to test the notes in my checkers, but I think this checker is fine without that test file. @NoQ, what do you think? CHANGES SINCE LAST ACTION

[PATCH] D72097: [LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner annotations

Charusso added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4619 This attribute may be used by analysis tools and has no effect on code -generation. +generation. A ``void`` argument means that the pointer cna point to any type. `cna`

[PATCH] D71791: [CFG] Fix an assertion failure with static initializers

Charusso added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:5923 + if (isa(Cond) || isa(Cond)) return nullptr; What about the following?: ```lang=c if (const auto *E = dyn_cast(StmtElem->getStmt())) return E->IgnoreParens(); return nullptr;

[PATCH] D71155: [analyzer] CERT: STR30-C

Charusso marked 3 inline comments as done. Charusso added a comment. In order to bypass the `CK_LValueToRValue` `evalCast()` we have to create en `ElementRegion` as a return-value of the problematic function call. In that case for a mythical reason we miss the fact the pointer is nullable. I

[PATCH] D71033: [analyzer] CERT: STR32-C

Charusso added a comment. We need to add the interestingness to the `NoteTags` so that we only emit notes on initialization/function calls which leads to an error. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71033/new/ https://reviews.llvm.org/D71033

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

Charusso updated this revision to Diff 233905. Charusso marked 6 inline comments as done. Charusso added a comment. - Try to conjure the index of the 'ElementRegion'. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71155/new/ https://reviews.llvm.org/D71155 Files:

[PATCH] D71033: [analyzer] CERT: STR32-C

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:479 +if (VD->getType()->getAsArrayTypeUnsafe()) { + VD->dumpColor(); + ProgramStateRef State = C.getState();

[PATCH] D71033: [analyzer] CERT: STR32-C

Charusso updated this revision to Diff 233904. Charusso retitled this revision from "[analyzer] CERT: StrChecker: 32.c" to "[analyzer] CERT: STR32-C". Charusso added a comment. - Add notes to the initialization of char-arrays. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71033/new/

[PATCH] D70411: [analyzer] CERT: STR31-C

Charusso updated this revision to Diff 233903. Charusso retitled this revision from "[analyzer] CERT: StrChecker: Implementing most of the STR31-C" to "[analyzer] CERT: STR31-C". Charusso added a comment. - Iterate over parameters rather arguments for searching string-reading. - Better notes of

[PATCH] D71433: [analyzer] CERT: POS34-C

Charusso added a reviewer: aaron.ballman. Charusso added a subscriber: aaron.ballman. Charusso added inline comments. Comment at: clang/docs/analyzer/checkers.rst:1881 + + #include + I would remove that line. Comment at:

[PATCH] D70411: [analyzer] CERT: StrChecker: Implementing most of the STR31-C

Charusso added a comment. In D70411#1776460 , @NoQ wrote: > Ok, so, what should the checker warn on? What should be the intended state > machine for this checker? > > We basically have two possible categories of answers to this question: > > - "The

[PATCH] D70411: [analyzer] CERT: StrChecker: Implementing most of the STR31-C

Charusso updated this revision to Diff 233252. Charusso marked 7 inline comments as done. Charusso retitled this revision from "[analyzer] CERT: StrChecker: 31.c" to "[analyzer] CERT: StrChecker: Implementing most of the STR31-C". Charusso edited the summary of this revision. Charusso added a

[PATCH] D71322: [analyzer] CStringChecker: Fix overly eager assumption that memcmp arguments overlap.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Could you write a test for `strcmp` please? I have found out the `evalMemcmp` behaves differently than `evalStrcmp`. However when the `memcmp` is going to be used like `strcmp`/`strncmp`

[PATCH] D71321: [analyzer] CStringChecker: Warning text fixes.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Cool, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71321/new/ https://reviews.llvm.org/D71321 ___

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso added a comment. In D70805#1776527 , @NoQ wrote: > Usually this kind of code is hard to re-use because all use cases require > different definitions of "has descendant". We already have at least 3 such > definitions floating around and we can't

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2109 + if (const MemRegion *MR = SrcV.getAsRegion()) { +if (const auto *SR = MR->getBaseRegion()->getAs()) { + State =

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

Charusso closed this revision. Charusso added a comment. Forgotten `amend` of commit message. Commited as: https://github.com/llvm/llvm-project/commit/8d288a0668a574863d52784084ff565c89f7366e Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69181/new/

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I think it is fine, but please let us wait with the final thoughts from @aaron.ballman. Comment at: clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp:21 +static

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a comment. Charusso added a parent revision:

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

Charusso added a comment. Examples generated by CodeChecker from the `curl` project: F10986729: str30-c.tar.gz Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71155/new/ https://reviews.llvm.org/D71155

[PATCH] D71033: [analyzer] CERT: StrChecker: 32.c

Charusso updated this revision to Diff 232672. Charusso edited the summary of this revision. Charusso added a comment. - Add docs. - Move to alpha. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71033/new/ https://reviews.llvm.org/D71033 Files: clang/docs/analyzer/checkers.rst

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso added a comment. Please let us measure your examples at first to have a consensus what we would like to see from this checker. In D70411#1769803 , @NoQ wrote: > In D70411#1769628 , @Charusso wrote: > > >

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso updated this revision to Diff 232671. Charusso marked 10 inline comments as done. Charusso added a comment. - Make the checker alpha. - Add docs. - Copy-paste @NoQ's test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/ https://reviews.llvm.org/D70411 Files:

[PATCH] D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools

Charusso added a comment. I would change the order of CCh and scan-build because we usually list stuff in alphabetical order. Also the chronological order is that, the newest is the first. Comment at: clang/www/analyzer/command-line.html:54 +Can run clang-tidy checkers

[PATCH] D71033: [analyzer] CERT: StrChecker: 32.c

Charusso added a comment. Examples generated by CodeChecker from the `curl` project: F10966937: str32-c.tar.gz Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71033/new/ https://reviews.llvm.org/D71033

[PATCH] D71033: [analyzer] CERT: StrChecker: 32.c

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:357 + + // 'strlen(something) + something' is most likely fine. + // FIXME: Use the 'SValVisitor' to catch every such constructs of the

[PATCH] D71033: [analyzer] CERT: StrChecker: 32.c

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This checker warns on most of the following rules:

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso updated this revision to Diff 232206. Charusso added a comment. - Add back a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70805/new/ https://reviews.llvm.org/D70805 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso added a comment. Here is a possible piece of code from the Tidy world: `anyOf(declRefExpr(), hasDescendant(declRefExpr()), integerLiteral(), hasDescendant(integerLiteral()))`, that is why I recommend to allow equality of symbols to prevent boilerplate. CHANGES SINCE LAST ACTION

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso updated this revision to Diff 232203. Charusso edited the summary of this revision. Charusso added a comment. - Add more tests. - Let us specify the descendant as allowing equality of symbols. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70805/new/

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso added a comment. I have picked `curl` as an example, because it has a measurable amount of reports. I still recommend to make this alpha, because the expression-tracking cannot track members, and peoples are using members everywhere. F10966837: str31-c.tar.gz

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso updated this revision to Diff 232198. Charusso added a comment. - Tweaking. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/ https://reviews.llvm.org/D70411 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

Charusso added a comment. In D70725#1767579 , @xazax.hun wrote: > Just a small side note. If the state was same as the previous we do not end > up creating a new ExplodedNode right? Is this also the case when we add a > NoteTag? It only happens for

[PATCH] D70823: [clang-tidy] Adding cert-pos34-c check

Charusso added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/PutenvWithAutoCheck.cpp:28 + unless(hasDescendant(callExpr(callee(functionDecl(hasAnyName( + "::alloc", "::malloc", "::realloc", "::calloc"))) +

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso added a comment. It suppresses stuff in `curl` like: char buf[4096]; size_t linelen = strlen(line); char *ptr = realloc(line, linelen + strlen(buf) + 1); strcpy([linelen], buf); char buffer[256]; char *string = NULL; size_t buflen = strlen(buffer); char *ptr =

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a parent revision: D70411: [analyzer] CERT:

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso marked 2 inline comments as done. Charusso added a comment. The false positive suppression by going backwards on the bug-path of stored reports was a very simple concept, which turned out to be a useless one. The rough idea was that to invalidate every report in a path, and every other

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso updated this revision to Diff 231341. Charusso added a comment. - Do not emit a report on re-binding a not null-terminated string, it may will be properly terminated (test added). - Added a test for the necessity of `SValVisitor` in this checker. - Some refactor. CHANGES SINCE LAST

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso updated this revision to Diff 230702. Charusso marked an inline comment as done. Charusso retitled this revision from "[analyzer][WIP] CERT: StrChecker: 31.c" to "[analyzer] CERT: StrChecker: 31.c". Charusso added a comment. - Remove the report storing map so we do not traverse

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

Charusso added a comment. Now I have simplified the checker so we do not need any ASCII-art, I believe. Do we have any better logic than the current implementation to catch when the string is read? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70411/new/

[PATCH] D70411: [analyzer][WIP] CERT: StrChecker: 31.c

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:170 +Report->addVisitor( +allocation_state::getMallocBRVisitor(DestV.getAsSymbol())); + } else { We can do the opposite to see whether the destination

[PATCH] D70411: [analyzer][WIP] CERT: StrChecker: 31.c

Charusso added a comment. In D70411#1754356 , @NoQ wrote: > I think it would really help if you draw a state machine for the checker, > like the ASCII-art thing in D70470 ; you > don't need to spend a lot of time turning

[PATCH] D70411: [analyzer][WIP] CERT: StrChecker: 31.c

Charusso updated this revision to Diff 230354. Charusso marked 2 inline comments as done. Charusso retitled this revision from "[analyzer][WIP] StrChecker: 31.c" to "[analyzer][WIP] CERT: StrChecker: 31.c". Charusso added a comment. - Added a report when the not null-terminated string is read. -

[PATCH] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318 +ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal ) { if (auto Reg = Val.getAsRegion()) { Reg = Reg->getMostDerivedObjectRegion(); -

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso abandoned this revision. Charusso added a comment. This patch moved to D70411 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69813/new/ https://reviews.llvm.org/D69813 ___ cfe-commits mailing list

[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

Charusso marked an inline comment as done. Charusso added a comment. Somehow the `MallocBugVisitor` is broken in the wild, and we need to support a lot more to call it non-alpha (see the `FIXME`s). I do not want to spend time on fixing the visitor, but rather I would make this checker alpha,

[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. This checker warns on most of the following rules:

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent , const CallContext , + CheckerContext ) const { +

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso marked 14 inline comments as done. Charusso added a comment. Given that we are having two different projects at first let us create the path-sensitive error-catching + false positive suppression + design of the CERT rules, and when we are fine, we get back to the impossible-to-solve

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent , const CallContext , + CheckerContext ) const { + unsigned DestPos = *CallC.DestinationPos; +

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent , const CallContext , + CheckerContext ) const { + unsigned DestPos = *CallC.DestinationPos; +

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso updated this revision to Diff 228675. Charusso added a comment. - The packaging have not been addressed yet. - Inject the "zombie" size expression to the new function call (`fgets`) if none of the size expression's regions have been modified. The idea is that: When we set up a variable

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso added a comment. In D69813#1736611 , @aaron.ballman wrote: > Would it make sense to use `cert.str.31.c` to remove the random dash? Would > this also help the user to do something like `cert.str.*.cpp`? if they want > just the CERT C++ STR

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso added a comment. In D69813#1735988 , @aaron.ballman wrote: > I'm not @NoQ, but I do agree that there should be a separate check per rule > in terms of the UI presented to the user. The name should follow the rule ID > like they do in

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso added a comment. In D69813#1734193 , @Szelethus wrote: > Hmm, so this checker is rather a collection of CERT rule checkers, right? > Shouldn't the checker name contain the actual rule name (STR31-C)? User > interfacewise, I would much prefer

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso updated this revision to Diff 227892. Charusso added a comment. - Support `alloca()`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69813/new/ https://reviews.llvm.org/D69813 Files: clang/include/clang/Lex/Preprocessor.h

[PATCH] D69813: [analyzer] CERTStrChecker: Model gets()

Charusso updated this revision to Diff 227888. Charusso marked 2 inline comments as done. Charusso retitled this revision from "[analyzer][WIP] CERTStrChecker: Model gets()" to "[analyzer] CERTStrChecker: Model gets()". Charusso added a comment. - Do not try to fix-it an array with offsets.

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

Charusso updated this revision to Diff 227867. Charusso marked 6 inline comments as done. Charusso added a comment. - Use existing visitors. - Make the `MallocBugVisitor` available for every checker. - Fix duplication of fix-its on the warning path piece when we emit a note as well. CHANGES

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124 + if (const SymbolicRegion *SR = DestMR->getSymbolicBase()) +if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR)) + return exprToStr(SizeExpr, C);

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

Charusso added a comment. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124 + if (const SymbolicRegion *SR = DestMR->getSymbolicBase()) +if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR)) + return

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124 + if (const SymbolicRegion *SR = DestMR->getSymbolicBase()) +if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR)) + return exprToStr(SizeExpr, C);

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124 + if (const SymbolicRegion *SR = DestMR->getSymbolicBase()) +if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR)) + return exprToStr(SizeExpr, C);

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. Charusso added a parent revision: D69746: [analyzer]

[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Charusso abandoned this revision. Charusso marked an inline comment as done. Charusso added a comment. In D69745#1732739 , @NoQ wrote: > Basically this, but the other way round: your `BeginAnalysis` is > `BeginFunction` with extra steps (namely, checking

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso marked 3 inline comments as done. Charusso added a comment. In D69726#1732334 , @Szelethus wrote: > Changes to `MallocChecker` really highlight the positive effects of this > patch. Nice! Thanks! Comment at:

[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Charusso marked 3 inline comments as done. Charusso added a comment. In D69745#1732348 , @Szelethus wrote: > YES PLEASE. Debug checkers that only dump from `check::EndAnalysis` won't > rely on the analysis not actually crashing. Which is ironically

[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Charusso updated this revision to Diff 227710. Charusso edited the summary of this revision. Charusso added a comment. - Do not restrict what you supposed to do with the callback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69745/new/ https://reviews.llvm.org/D69745 Files:

[PATCH] D69662: [Checkers] Avoid using evalCall in StreamChecker.

Charusso added a comment. On multiple evaluation of the same call the Analyzer will warn and prevent you to do so. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69662/new/ https://reviews.llvm.org/D69662

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang Tidy's script

Charusso updated this revision to Diff 227640. Charusso marked 2 inline comments as done. Charusso added a comment. - Remove `llvm_unreacheable` from error-handling. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69746/new/ https://reviews.llvm.org/D69746 Files:

[PATCH] D69746: [analyzer] FixItHint: Apply and test hints with the Clang Tidy's script

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:152 + + if (llvm::Error Err = Repls.add(Repl)) +llvm_unreachable("An error occured during fix-it replacements"); zinovy.nis wrote: > Isn't

[PATCH] D69746: [analyzer] FixItHint: apply and test hints with the Clang Tidy's script

Charusso added a comment. Hey! I would like to reuse your script without any reinvention. It serves every needs, but at some point we start to heavily diverge. When I started with the Tidy I really enjoyed that script, and most of the people I know both develop Tidy and the Analyzer, so I

[PATCH] D69746: [analyzer] FixItHint: apply and test hints with the Clang Tidy's script

Charusso created this revision. Charusso added reviewers: NoQ, alexfh, zinovy.nis, JonasToth, hokein, gribozavr, lebedev.ri. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun,

[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Charusso marked an inline comment as done. Charusso added a comment. I felt that it will be easier, eh. Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:118 +SubEng.processBeginWorklist(BuilderCtx, Node, DstBegin, StartLoc);

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso added a comment. In D69731#1730956 , @Charusso wrote: > I am thinking of a callback which is something like: > > void checkBeginAnalysis(const Decl *D, BugReporter ) const; > > > so it would be easy and meaningful to have a place for the

[PATCH] D69745: [analyzer] Checker: check::BeginAnalysis

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a parent revision: D69731: [analyzer]

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso updated this revision to Diff 227557. Charusso added a comment. - Use less `const`, it prevented the usage of non-const methods. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69731/new/ https://reviews.llvm.org/D69731 Files:

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso added a comment. I am thinking of a callback which is something like: void checkBeginAnalysis(const Decl *D, BugReporter ) const; so it would be easy and meaningful to have a place for the `Preprocessor` logic. Do you think it would worth it? Repository: rC Clang CHANGES SINCE

[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

Charusso added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69540/new/ https://reviews.llvm.org/D69540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso added a comment. Thanks for the review! Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:40-48 + if (const llvm::APSInt *SizeInt = SVB.getKnownValue(State, Size)) { +CharUnits RegionSize = CharUnits::fromQuantity(SizeInt->getSExtValue()); + +// If a

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso updated this revision to Diff 227546. Charusso marked 3 inline comments as done. Charusso added a comment. - Done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69599/new/ https://reviews.llvm.org/D69599 Files:

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso added a comment. In D69731#1730899 , @NoQ wrote: > Clang-Tidy's `PPCallbacks` subsystem looks much more realistic. I wanted to make it available from the `AnalysisManager` so that I can obtain the macro definitions once before the analysis

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso added a comment. Thanks, now it is cool! Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:40-48 + if (const llvm::APSInt *SizeInt = SVB.getKnownValue(State, Size)) { +CharUnits RegionSize = CharUnits::fromQuantity(SizeInt->getSExtValue()); + +// If a

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso updated this revision to Diff 227543. Charusso marked 4 inline comments as done. Charusso added a comment. - Old division swapped by `evalBinOp`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69599/new/ https://reviews.llvm.org/D69599 Files:

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso added a comment. In D69731#1730784 , @NoQ wrote: > I'm not sure though - because we somehow survived without this for like 10 > years. Eg. `BugReporterVisitors.cpp`: [...] > I'd love to see some actual use before committing. "Teaser":

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso updated this revision to Diff 227534. Charusso marked 6 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/ https://reviews.llvm.org/D69726 Files:

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:29-31 +/// \returns The stored dynamic size expression for the region \p MR. +const Expr *getDynamicSizeExpr(ProgramStateRef State, const MemRegion *MR); +

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/DynamicSize.cpp:40-48 + if (const llvm::APSInt *SizeInt = SVB.getKnownValue(State, Size)) { +CharUnits RegionSize = CharUnits::fromQuantity(SizeInt->getSExtValue()); + +// If a variable is

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso added a comment. In D69599#1730707 , @NoQ wrote: > > This is the first step to mitigate that issue. > > What's the issue? Well, after I mentioned an issue I have realized the somewhat path-insensitive `getSizeInElements()` does not touch the

[PATCH] D69599: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store

Charusso updated this revision to Diff 227524. Charusso marked 2 inline comments as done. Charusso added a comment. - Fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69599/new/ https://reviews.llvm.org/D69599 Files:

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso marked an inline comment as done. Charusso added a comment. It is needed for my work, and also I have seen other checkers in need of that. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:195 ASTContext *Ctx; - const Preprocessor const

[PATCH] D69731: [analyzer] CheckerContext: Make the Preprocessor available

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Charusso added a parent revision: D69726: [analyzer]

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso marked an inline comment as done. Charusso added a comment. I do not want to reverse engineer the `MallocChecker` to obtain the size on call basis. The current model without D68725 makes it enough difficult to obtain the size even with this generic

[PATCH] D69642: [analyzer] DynamicSize: Simplify the assumption creating of sizes

Charusso abandoned this revision. Charusso added a comment. I think we do not need to create assumptions. However, there is a tiny difference which continues in D69726 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69642/new/

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

Charusso created this revision. Charusso added a reviewer: NoQ. Charusso added a project: clang. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. This patch introduces a way to store the symbolic size and its

[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

Charusso accepted this revision. Charusso marked an inline comment as done. Charusso added a comment. This revision is now accepted and ready to land. My concern was the too heavy `Optional` and `bool` usage. Cool patch! Comment at:

[PATCH] D69642: [analyzer] DynamicSize: Simplify the assumption creating of sizes

Charusso added a comment. @NoQ, I hope this is what you have suggested. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69642/new/ https://reviews.llvm.org/D69642 ___ cfe-commits mailing list

<    1   2   3   4   5   6   7   >