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

2019-11-01 Thread Csaba Dabis via Phabricator via cfe-commits
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: clang/include/clang/StaticAnalyzer/Core/BugReporte

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

2019-11-01 Thread Csaba Dabis via Phabricator via cfe-commits
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 &BR) const; > > > so it would be easy and meaningful to have a place for the `Pre

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

2019-11-01 Thread Csaba Dabis via Phabricator via cfe-commits
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] CheckerCont

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

2019-11-01 Thread Csaba Dabis via Phabricator via cfe-commits
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); SubEng.processBeginOfFunction(Builde

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

2019-11-01 Thread Csaba Dabis via Phabricator via cfe-commits
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, mg

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

2019-11-01 Thread Csaba Dabis via Phabricator via cfe-commits
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 would

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

2019-11-03 Thread Csaba Dabis via Phabricator via cfe-commits
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: clang/include/clang/St

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

2019-11-03 Thread Csaba Dabis via Phabricator via cfe-commits
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 `llv

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

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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] D69745: [analyzer] Checker: check::BeginAnalysis

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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: clang/in

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

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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 exactl

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

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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: clang/lib/Static

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

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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] Fix

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

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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()

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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()

2019-11-04 Thread Csaba Dabis via Phabricator via cfe-commits
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 exprToS

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

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
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()

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
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 SIN

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

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
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. CHA

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

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
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 clang/include/clang/StaticAnalyzer/Checkers/Checkers

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

2019-11-05 Thread Csaba Dabis via Phabricator via cfe-commits
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 sma

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

2019-11-06 Thread Csaba Dabis via Phabricator via cfe-commits
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 clang-tidy

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

2019-11-07 Thread Csaba Dabis via Phabricator via cfe-commits
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 rules

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

2019-11-11 Thread Csaba Dabis via Phabricator via cfe-commits
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()

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + unsigned DestPos = *CallC.Destina

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

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207 +void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC, + CheckerContext &C) const { + unsigned DestPos = *CallC.Destina

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

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
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 pro

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

2019-11-13 Thread Csaba Dabis via Phabricator via cfe-commits
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 &Call, const CallContext &CallC, + CheckerContext &C) c

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

2019-11-18 Thread Csaba Dabis via Phabricator via cfe-commits
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: http

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

2019-11-18 Thread Csaba Dabis via Phabricator via cfe-commits
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, and

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

2019-11-18 Thread Csaba Dabis via Phabricator via cfe-commits
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] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

2019-11-20 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318 +ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val) { if (auto Reg = Val.getAsRegion()) { Reg = Reg->getMostDerivedObjectRegion(); -

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

2019-11-20 Thread Csaba Dabis via Phabricator via cfe-commits
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] D70411: [analyzer][WIP] CERT: StrChecker: 31.c

2019-11-20 Thread Csaba Dabis via Phabricator via cfe-commits
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

2019-11-20 Thread Csaba Dabis via Phabricator via cfe-commits
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 a

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

2019-11-22 Thread Csaba Dabis via Phabricator via cfe-commits
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/ https://reviews.llvm.org/D7041

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

2019-11-22 Thread Csaba Dabis via Phabricator via cfe-commits
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 backwards

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

2019-11-27 Thread Csaba Dabis via Phabricator via cfe-commits
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 ACT

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

2019-11-27 Thread Csaba Dabis via Phabricator via cfe-commits
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] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

2019-11-28 Thread Csaba Dabis via Phabricator via cfe-commits
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: StrCh

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

2019-11-28 Thread Csaba Dabis via Phabricator via cfe-commits
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(&line[linelen], buf); char buffer[256]; char *string = NULL; size_t buflen = strlen(buffer); char *ptr = rea

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

2019-11-28 Thread Csaba Dabis via Phabricator via cfe-commits
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"))) + .b

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

2019-12-03 Thread Csaba Dabis via Phabricator via cfe-commits
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 eva

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

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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 clang/include/clang/StaticAnalyzer/Core/BugRe

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

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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/ https://reviews.llvm.org/D70

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

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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 clang/lib/StaticA

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

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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 htt

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

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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: https://wiki

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

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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 s

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

2019-12-04 Thread Csaba Dabis via Phabricator via cfe-commits
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] D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools

2019-12-05 Thread Csaba Dabis via Phabricator via cfe-commits
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 to

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

2019-12-06 Thread Csaba Dabis via Phabricator via cfe-commits
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: clang/d

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

2019-12-06 Thread Csaba Dabis via Phabricator via cfe-commits
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] D71033: [analyzer] CERT: StrChecker: 32.c

2019-12-06 Thread Csaba Dabis via Phabricator via cfe-commits
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 clang

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

2019-12-06 Thread Csaba Dabis via Phabricator via cfe-commits
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: D710

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

2019-12-06 Thread Csaba Dabis via Phabricator via cfe-commits
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] D70052: [clang-tidy] Add misc-mutating-copy check

2019-12-08 Thread Csaba Dabis via Phabricator via cfe-commits
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 co

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

2019-12-08 Thread Csaba Dabis via Phabricator via cfe-commits
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] D71155: [analyzer] CERT: StrChecker: 30.c

2019-12-09 Thread Csaba Dabis via Phabricator via cfe-commits
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 = State->Bind

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

2019-12-09 Thread Csaba Dabis via Phabricator via cfe-commits
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] D71321: [analyzer] CStringChecker: Warning text fixes.

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
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 ___ c

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

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
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` t

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

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
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 comm

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

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
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 checke

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

2019-12-13 Thread Csaba Dabis via Phabricator via cfe-commits
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: clang/docs/analyzer/c

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

2019-12-13 Thread Csaba Dabis via Phabricator via cfe-commits
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 t

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

2019-12-13 Thread Csaba Dabis via Phabricator via cfe-commits
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/ ht

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

2019-12-13 Thread Csaba Dabis via Phabricator via cfe-commits
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: clang/docs/analyzer/c

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

2019-12-13 Thread Csaba Dabis via Phabricator via cfe-commits
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] D71033: [analyzer] CERT: STR32-C

2019-12-13 Thread Csaba Dabis via Phabricator via cfe-commits
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] D71155: [analyzer] CERT: STR30-C

2019-12-13 Thread Csaba Dabis via Phabricator via cfe-commits
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 hav

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

2019-12-20 Thread Csaba Dabis via Phabricator via cfe-commits
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] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

2020-03-09 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso accepted this revision. Charusso added a comment. In D75529#1912849 , @vabridgers wrote: > @Charusso, I cannot find a way to create a test for this change, since this > was found using an architecture that supports 16-bit chars. I really felt t

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

2020-03-12 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 250020. Charusso marked 3 inline comments as done. Charusso retitled this revision from "[analyzer][WIP] add-new-checker.py: Introduction" to "[analyzer] add-new-checker.py: Introduction". Charusso added a comment. Herald added a subscriber: mgorny. - Try to

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

2020-03-12 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked an inline comment as done. Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:24 def CoreBuiltin : Package<"builtin">, ParentPackage, Hidden; -def CoreUninitialized : Package<"uninitialized">, ParentPackage; +def

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

2020-03-13 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. In D75529#1920796 , @vabridgers wrote: > I believe all comments have been addressed. Please let me know if there's > anything else required. Thanks I think you have solved everything. Do you have commit access? May you would r

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-17 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. In D76229#1925363 , @f00kat wrote: > In D76229#1925268 , @aaron.ballman > wrote: > > > Have you considered making this a static analyzer check as opposed to a > > clang-tidy check? I thin

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

2020-01-31 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 241749. Charusso edited the summary of this revision. Charusso added a comment. - Let us reuse this patch. - Remove the expression storing feature. - Only store known sizes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69726/new/ https://reviews.l

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

2020-01-31 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 241752. Charusso edited the summary of this revision. Charusso added a comment. - 2020-ify the checker writing - Remove extra bullet-points of CERT checker documentation: we only need the checker's documentation, not the packages. CHANGES SINCE LAST ACTION

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

2020-01-31 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked 2 inline comments as done. Charusso added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h:55 + bool VisitSymbolRegionValue(const SymbolRegionValue *S) { +return Visit(S->getRegion()); + } --

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

2020-01-31 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 241758. Charusso marked an inline comment as done. Charusso added a comment. - Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70805/new/ https://reviews.llvm.org/D70805 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasD

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

2020-01-31 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 241761. Charusso added a comment. - Rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71033/new/ https://reviews.llvm.org/D71033 Files: clang/docs/analyzer/checkers.rst clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/includ

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

2020-02-03 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 242159. Charusso marked 8 inline comments as done. Charusso added a comment. - Remove the modeling from CStringChecker. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71155/new/ https://reviews.llvm.org/D71155 Files: clang/docs/analyzer/checkers.

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

2020-02-03 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. Thanks! I have felt that may I create a too complex symbolic value. Sorry for stealing your time. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2097-2098 +void CStringChecker::evalCharPtrCommon(CheckerContext &C, +

[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

2020-02-04 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. Thanks for the fix! The `PopUpRanges` is very important, please revert it back in its original shape. Sorry for the inconvenience. I have ran a quick scan-build with this patch on LLVM because I wanted to give you a real world example (which you cannot visibly see at t

[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

2020-02-04 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:925 HighlightRange(R, LPosInfo.first, Range); - } } Here the gray highlighting goes, so the `PopUpRanges` store whether we have already highlighted the range a

[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

2020-02-04 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Cool, thanks you! Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:612 + return !(Range.getBegin().isMacroID() || Range.getEnd().isMacroID()); +} +

[PATCH] D73966: [analyzer][WIP] Add 10.0.0 release notes.

2020-02-05 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added inline comments. Comment at: clang/docs/ReleaseNotes.rst:408 + +- New checker: ``alpha.plusplus.PlacementNew`` to detect whether the storage + provided for default placement new is sufficiently large. balazske wrote: > Szelethus wrote: > > NoQ wro

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

2020-02-07 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment. In D71433#1808316 , @Charusso wrote: > In D71433#1784238 , @NoQ wrote: > > > Currently the check may warn on non-bugs of the following kind: > > > > void foo() { > > char env[] = "NAM

[PATCH] D74467: [analyzer] Teach scan-build how to rebuild index.html without analyzing.

2020-02-12 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. Thanks! I would mention in the Summary the necessary flags to perform index-only output. (May write some release notes?) That option sounds very strange, but I like it. For example to run

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

2020-01-27 Thread Csaba Dabis via Phabricator via cfe-commits
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 Fi

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

2020-01-27 Thread Csaba Dabis via Phabricator via cfe-commits
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 c

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

2020-01-27 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 240759. Charusso added a comment. - Make it runable. Whoops. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73521/new/ https://reviews.llvm.org/D73521 Files: clang/test/Analysis/add-new-checker/add-main-package.rst clang/test/Analysis/add-new-c

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

2020-01-27 Thread Csaba Dabis via Phabricator via cfe-commits
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 small-stri

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

2020-01-27 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso marked an inline comment as done. Charusso added a comment. This script dumps out a dummy checker which is the continuation of the Clang Tidy's https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/add_new_check.py However, we could stick to the main-function-re

[PATCH] D73629: [analyzer] vfork checker: allow execve after vfork

2020-01-29 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a reviewer: NoQ. Charusso added a comment. Hey, thanks! The patch looks great, but please note that we do the reviews with context using `git diff -U99` or uploading with `arc` (https://secure.phabricator.com/book/phabricator/article/arcanist/). Repository: rG LLVM Github

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

2020-01-30 Thread Csaba Dabis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG601687bf731a: [analyzer] DynamicSize: Remove 'getExtent()' from regions (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D69540?vs=227013&id=241447#toc Repository: rG LLVM Gi

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

2020-01-30 Thread Csaba Dabis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGaf3d0d16286a: [analyzer] DynamicSize: Remove 'getSizeInElements()' from store (authored by Charusso). Changed prior to commit: https://reviews.llvm.org/D69599?vs=227546&id=241460#toc Repository: rG L

<    1   2   3   4   5   6   7   >