[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:191 +.replace('\\>', '>') \ +.rstrip(',') +logging.debug(raw_json)

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

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

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ added a comment. I'll add some tests as soon as i'm sure tests for this thing actually work (by landing D62638 ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62761/new/ https://reviews.llvm.org/D62761 ___

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ updated this revision to Diff 202514. NoQ added a comment. Remove "-" from program point dumps because it resembles a removal marker in diffs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62761/new/ https://reviews.llvm.org/D62761 Files: clang/utils/analyzer/exploded-graph-rewr

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

NoQ added a comment. I disabled tests on Windows for now (rC362343 -> rC362347 ), but other than that the tests are working well, there was nothing to worry about. Repository: rL LLVM CHANGES SINCE LAST ACTION https:/

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

NoQ added a comment. Thanks!! Will fix as soon as i get to my desktop. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62638/new/ https://reviews.llvm.org/D62638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D62638: [analyzer] A Python script to prettify the ExplodedGraph dumps.

NoQ marked an inline comment as done. NoQ added a comment. In D62638#1529170 , @krytarowski wrote: > This commit breaks the NetBSD buildbot node. rC362574 . Comment at: cfe/trunk/utils/analyzer/explo

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added a comment. Yay, this thing really works! > Now, whether this change is any good is practically impossible to tell > without evaluation on production code, so I'll get back with that once I > gather some data. Yes. This patch deserves some data on how much have reports grown in length

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

NoQ added a comment. Aha, that's something! And nice to see we've already had this bug covered with tests. Because, of course, i added these tests a year ago without even thinking about what the correct behavior should look like :/ Comment at: clang/lib/StaticAnalyzer/Core/Bu

[PATCH] D62978: [analyzer] ReturnVisitor: Handle non-null ReturnStmts

NoQ added a comment. Ah, that positive! No, i don't think this is a valid way to suppress it. I'll tease you a bit more though, and only give a hint: the null value that we're null-dereferencing was in fact assumed to be null //in the top frame//, not within any inline function. Therefore the

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass constructing objects to see inlined calls

NoQ added a comment. Aha, nice, that's much cleaner! Indeed, we have to skip the construct-expression, which is going to be on our way to the program point of new-expression which corresponds to calling the allocator, despite being nested within the new-expression. An annoying corner-case. I'd

[PATCH] D62885: [analyzer] Add werror flag for analyzer warnings

NoQ added a comment. Looks great! Feel free to add a Driver flag as well (i.e., --analyzer-werror or something like that) so that not to type `-Xclang` every time. In D62885#1530573 , @xazax.hun wrote: > The only problem I see with this approach is that

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

NoQ added a comment. Thanks! I think we should: - Pre-normalize our expected outputs so that we didn't have to normalize them in every run-line. - Treat the lack of newline in plist output as a bug and try to fix it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

NoQ added a comment. In D59555#1514602 , @NoQ wrote: > I'm still in doubts on how to connect your work with the `CallDescription` > effort. I'll think more about that. I guess i'll just make a yaml reader for `CallDescription`s as soon as the interface

[PATCH] D62946: [analyzer] ProgramPoint: more explicit printJson()

NoQ added a comment. Fair enough! Tests are welcome. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62946/new/ https://reviews.llvm.org/D62946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D62946: [analyzer] ProgramPoint: more explicit printJson()

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Aha, ok, right! I wouldn't be surprised if some of these are entirely unused. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62946/new/ https://reviews.llvm.org/D629

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Ok! I'll be happy to have this addressed incrementally. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62949/new/ https://reviews.llvm.org/D62949 _

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass constructing objects to see inlined calls

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:842-849 + if (Optional CEE = Node->getLocationAs()) if (CEE->getCalleeContext()->getCallSite() == S) break; - if (auto SP = Node->getLocationAs()) -i

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

NoQ accepted this revision. NoQ added a comment. Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63041/new/ https://reviews.llvm.org/D63041 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

NoQ added a comment. Herald added a subscriber: Charusso. Herald added a project: clang. Hey, i'm seeing a crash in this checker, would you like to look at it? It looks as if you're not being careful about dereferences/lvalue-to-rvalue-casts so it tries to compare `&e` to `e1`. **$ `cat repro.c

[PATCH] D62978: [analyzer] ReturnVisitor: Handle unknown ReturnStmts better

NoQ added inline comments. Comment at: clang/test/Analysis/diagnostics/find_last_store.c:9 void no_find_last_store() { - c *e = d(); // expected-note{{Calling 'd'}} - // expected-note@-1{{Returning from 'd'}} - // expected-note@-2{{'e' initialized here

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:842-849 + if (Optional CEE = Node->getLocationAs()) if (CEE->getCalleeContext()->getCallSite() == S) break; - if (auto SP = Node->getLocationAs()) -i

[PATCH] D63086: [analyzer][NoStoreFuncVisitor][NFC] Move methods out-of-line, turn some to static functions

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thx for the cleanup! Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:340 +private: + /// Attempts to find the region of interest in a given CXX decl, + /// by e

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added a comment. In D62883#1536894 , @Szelethus wrote: > I'm not sure how long it'll take for me to figure out what's wrong, but these > numbers are so ridiculous, I suspect a programming error rather then an > algorithmic issue. > > edit: I seem to

[PATCH] D63080: [analyzer] Track indices of arrays

NoQ added a comment. Whoa, this looks like a much needed improvement, i'm glad that you found it! Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1681 +trackExpressionValue( +LVNode, Arr->getIdx(), report, EnableNullFPSuppression); + --

[PATCH] D63093: [analyzer] WIP: MallocChecker: Release temporary CXXNewExpr

NoQ added a comment. In such cases i recommend starting with writing down a test. Like in TDD: first test, //then// code. The general direction doesn't seem reasonable to me; it introduces some pattern-matching for a specific scenario, but it's unclear why is this scenario a problem on its own

[PATCH] D63117: [analyzer] RetainCount: Add support for OSRequiredCast().

NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. It's a new API for custom RTTI in Apple IOKit/DriverKit fra

[PATCH] D63118: [analyzer] DeadStores: Add a crude suppression files generated by DriverKit IIG.

NoQ created this revision. NoQ added a reviewer: dcoughlin. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. IIG is a replacement for MIG in DriverKit: IIG is autogener

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ added a comment. In D62761#1525923 , @Charusso wrote: > In D62761#1525917 , @NoQ wrote: > > > Remove "-" from program point dumps because it resembles a removal marker > > in diffs. > > > Could you add an image

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ added a comment. In D62761#1539137 , @Charusso wrote: > I asked for the new behavior, but I think it should be cool. The new behavior is on the original screenshot, i just uploaded the wrong patch initially >.< CHANGES SINCE LAST ACTION https://

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

NoQ added a comment. All right, it seems that i'm completely misunderstanding this problem and we've been talking past each other this whole time. The problem is not that we need to skip the `CXXConstructExpr`. The problem is that we need to skip `CXXNewExpr` (!!). CFG elements for an operator-

[PATCH] D62978: [analyzer] trackExpressionValue(): Handle unknown values better

NoQ added a comment. This problem is fairly complicated. We clearly need both behaviors (sometimes track until the definition, sometimes track until the collapse-to-null), and it's not clear to me right now when exactly do we need each of them. This is also not a high priority for GSoC because

[PATCH] D63227: [analyzer] Better timers.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. `-analyzer-stats` now allows you to fi

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

NoQ added a comment. Yup, this makes sense now! I'll do some nit-picking for a little longer. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:839 +// as a 'StmtPoint' so we have to bypass it. +const bool IsBypass = isa(S); + For now th

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:855 + + CurrentSFC = Node->getStackFrame(); + Mmm, wait a sec. This way the loop condition is trivially true. The second check on line 862 is also trivially true. I

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Great, thanks!! Let's commit this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62926/new/ https://reviews.llvm.org/D62926 ___ cfe-commits ma

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added a comment. //*sloowly catches up >.<*// CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62883/new/ https://reviews.llvm.org/D62883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added a comment. > I am not sure about assuming operator bool being correct. I think we first > could think about other tricks to limit the tracking (see my idea above) and > if we fail I would only add such rules as a last resort. I think this depends greatly on the stack frame layout. I s

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added a comment. In D62883#1542802 , @Szelethus wrote: > Some conclusions: > > - Cases where the condition is also a variable initialization tend to expand > the bug path greatly. This isn't always bad, but should be noted. In general, > unless we ha

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added inline comments. Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:80-81 + + if (bar) // expected-note {{Taking true branch}} + // expected-note@-1{{Assuming 'bar' is not equal to 0}} +if (flag) // expected-note {{Taking true bra

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added a comment. How about we track the condition value past its collapse point only if it involves an overwrite of a variable (or other region) from which the value is loaded? Like, if there are no overwrites, stop at the collapse point. If there are overwrites, stop at the oldest overwrit

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:170-171 super(Store, self).__init__() -self.clusters = [StoreCluster(c) for c in json_s] +self.clusters = collections.OrderedDict(

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added a comment. (then, again, not sure what happens if more nested stack frames are around) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62883/new/ https://reviews.llvm.org/D62883 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D63362: [analyzer] Fix JSON dumps for store clusters.

NoQ created this revision. NoQ added a reviewer: Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Include a unique pointer so that it was possible to figure out if it's

[PATCH] D63362: [analyzer] Fix JSON dumps for store clusters.

NoQ updated this revision to Diff 204874. NoQ added a comment. Fair enough, they aren't really needed here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63362/new/ https://reviews.llvm.org/D63362 Files: clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/test/Analysis/dump_egraph.

[PATCH] D62899: [analyzer][UninitializedObjectChecker] Mark uninitialized regions as interesting.

NoQ added a comment. I don't remember what exactly does `markInteresting()` do and these tests don't really convince me that it does anything at all. Halp? >.< Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62899/new/ https://reviews.llvm.org/D62899 __

[PATCH] D63080: [analyzer] Track indices of arrays

NoQ accepted this revision. NoQ added a comment. Great, thanks!! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63080/new/ https://reviews.llvm.org/D63080 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D63438: [analyzer] print() JSONify: ProgramPoint revision

NoQ added a comment. Yes, this would be helpful! Tests? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63438/new/ https://reviews.llvm.org/D63438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

NoQ accepted this revision. NoQ added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/Analysis/dump_egraph.c:3 // RUN: cat %t.dot | FileCheck %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot -trim-egraph

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ updated this revision to Diff 205177. NoQ added a comment. Add tests, rebase. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62761/new/ https://reviews.llvm.org/D62761 Files: clang/test/Analysis/exploded-graph-rewriter/environment_diff.dot clang/test/Analysis/exploded-graph-rewr

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:287 .replace('\\}', '}') \ +.replace('', '\\') \

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1609-1613 + if (B->rbegin()->getKind() != CFGElement::Kind::Statement) +return nullptr; + + // This should be the condition of the terminator block. + const Stmt *S = B->rbegin()->

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1640-1642 + CFGBlock *OriginB = GetRelevantBlock(Origin); + if (!OriginB || !NB) +return nullptr; Szelethus wrote: > NoQ wrote: > > `// TODO: This can be cached.` >

[PATCH] D63438: [analyzer] print() JSONify: ProgramPoint revision

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Great, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63438/new/ https://reviews.llvm.org/D63438 ___ cfe-commits mailing list cfe-commi

[PATCH] D63462: [analyzer] JsonSupport: Escape escapes

NoQ added a comment. Hmm, this doesn't seem to solve my problem in D62761 . Let me write some actual test case so that you knew it's fixed when it's fixed. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63462/new/ https://reviews

[PATCH] D63462: [analyzer] JsonSupport: Escape escapes

NoQ added a comment. I mean, i'm removing backslashes but you're adding more backslashes. Therefore i think we're talking about different issues. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63462/new/ https://reviews.llvm.org/D63462

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

NoQ updated this revision to Diff 205443. NoQ added a comment. Don't try to sneak in an unrelated change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62761/new/ https://reviews.llvm.org/D62761 Files: clang/test/Analysis/exploded-graph-rewriter/environment_diff.dot clang/test/Anal

[PATCH] D63519: [analyzer] exploded-graph-rewriter: Fix escaping and unescaping of StringRegions.

NoQ created this revision. NoQ added a reviewer: Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. Additionally, add a forgotten escape for Store values. Repository:

[PATCH] D63462: [analyzer] JsonSupport: Escape escapes

NoQ added a comment. My problem is demonstrated (and solved) by D63519 . If i revert my changes but apply this patch instead, my test keeps failing. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63462/new/ https://reviews.llvm.o

[PATCH] D63519: [analyzer] exploded-graph-rewriter: Fix escaping and unescaping of StringRegions.

NoQ updated this revision to Diff 205449. NoQ added a comment. Oh, right, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63519/new/ https://reviews.llvm.org/D63519 Files: clang/lib/StaticAnalyzer/Core/RegionStore.cpp clang/test/Analysis/exploded-graph-rewriter/escapes.c cl

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

NoQ added a comment. Hey, thanks! That's a nice catch. The code looks fine, but i don't think your test actually tests it - see inline comments. Also i think we should put the test into the clang repo (i.e., `test/Analysis`), because that's where the change is. I'd like to know it if i break y

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

NoQ added inline comments. Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot %s +// RUN: cat %t.dot | FileCheck %s Ugh, you picked an exotic test as an example. Let's t

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

NoQ added inline comments. Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot %s +// RUN: cat %t.dot | FileCheck %s NoQ wrote: > Ugh, you picked an exotic test as an exam

[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

NoQ added inline comments. Comment at: clang/test/Analysis/egraph-asm-goto-no-crash.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot %s +// RUN: cat %t.dot | FileCheck %s NoQ wrote: > NoQ wrote: > > Ugh, you picked an exotic

[PATCH] D63227: [analyzer] Better timers.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:228 -ExprEngine::~ExprEngine() { - BR.FlushReports(); xazax.hun wrote: > Hmm. Maybe add an assert that all the bug reports are flushed at

[PATCH] D62899: [analyzer][UninitializedObjectChecker] Mark uninitialized regions as interesting.

NoQ added a comment. In D62899#1549715 , @Szelethus wrote: > Added a proper testfile. The only downside of it is that it doesn't test > anything. Use creduce! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62899/new/ https://reviews.llvm.org/D

[PATCH] D63538: [analyzer][CFG] Return the correct terminator condition

NoQ added a comment. I think this kinda makes sense, but also it looks like a risky change, because who knows how many times we have took the old behavior for granted. In particular, i'm slightly worried about this breaking the wonky logic of `ExprEngine::VisitLogicalExpr` (cf. D59857

[PATCH] D64611: [analyzer] exploded-graph-rewriter: Improve source location dumps.

NoQ created this revision. NoQ added a reviewer: Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. - Correctly display macro expansion and spelling locations. - Use the

[PATCH] D64611: [analyzer] exploded-graph-rewriter: Improve source location dumps.

NoQ closed this revision. NoQ added a comment. Whoops, forgot the phabricator link. Committed as rC365861 . Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64611/new/ https://reviews.llvm.org/D64611

[PATCH] D64526: [NFC] Unforget a colon in a few CHECK: directives.

NoQ added a comment. Thanks all! I guess i'll commit. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64526/new/ https://reviews.llvm.org/D64526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D64646: [OPENMP]Add support for analysis of if clauses.

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great! Thank you for improving the tests even further. Comment at: test/Analysis/cfg-openmp.cpp:58-67 #pragma omp distribute simd for (int i = 0; i < 10; ++i) argc =

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

NoQ added a comment. Oh damn, i just realized that this way we track much more pointers than before, because we cannot restrict ourselves to pointers that have been explicitly malloc()ed during analysis. After all, we don't need to see the allocation site to diagnose use-after-free. I'm afraid

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

NoQ added a comment. In D64680#1584130 , @Charusso wrote: > Here is an example of the mentioned use-after-free by pointer-escaping as an > argument: > > https://llvm.org/reports/scan-build/report-DeclBase.cpp-getFromVoidPointer-0-1.html#EndPath Not su

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yeah, i think this is good to go :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64287/new/ https://reviews.llvm.org/D64287 __

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Yup, looks good! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:146-149 + ///This is useful, because for non-tracked regions, notes

[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1052 +// to return that every time. +if (N->getCFG().isLinear()) + WouldEventBeMeaningless = true; This time i'd rather go for "has exactly 3 blo

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2547-2548 + StringRef FunctionStr = ""; + if (const Decl *D = C.getStackFrame()->getDecl()) +if (const FunctionDecl *FD = D->getAsFunction()) + FunctionStr = Lexer::getSourceText

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ added a comment. In D64274#1584974 , @baloghadamsoftware wrote: > Hmm, I still fail to understand the problem with the current `VirtualCall` > checker. Is it unstable? Does it report many false positives? Yeah, pretty much. It's basically defined t

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81 + +bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, + const Expr *E, BugReport &report, Szelethus wrote: > x

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1447-1454 // If we have an expression that provided the value, try to track where it // came from. if (InitE) { if (!IsParam) InitE = InitE->IgnoreParenCasts();

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552 + FunctionStr = Lexer::getSourceText( + CharSourceRange::getTokenRange( + {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}), + C.getSourceMana

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks great to me once @Szelethus's comments are addressed. Thanks!! Comment at: lib/StaticAnalyzer/Checkers/Yaml.h:34 +std::string(Conf

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Great, thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:363-364 + /// Check whether we do not model the memory allocation. + bool isNotModeled(const Call

[PATCH] D63279: [Analyzer] Unroll for-loops where the upper boundary is a variable with know value

NoQ added a comment. /me has just noticed that it's not D34812 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63279/new/ https://reviews.llvm.org/D63279 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

NoQ added inline comments. Comment at: test/Analysis/taint-generic.c:24 +// CHECK-INVALID-FILE-SAME: 'alpha.security.taint.TaintPropagation:Config', +// CHECK-INVALID-FILE-SAME:that expects a valid filename Szelethus wrote: > NoQ wrote: > > Szel

[PATCH] D64646: [OPENMP]Add support for analysis of if clauses.

NoQ added inline comments. Comment at: test/Analysis/cfg-openmp.cpp:58-67 #pragma omp distribute simd for (int i = 0; i < 10; ++i) argc = x; -// CHECK-NEXT: 27: x -// CHECK-NEXT: 28: [B1.27] (ImplicitCastExpr, LValueToRValue, int) -// CHECK-NEXT: 29: argc -// CHECK-NE

[PATCH] D64765: [OPENMP]Add support for analysis of firstprivate variables.

NoQ accepted this revision. NoQ added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/AST/OpenMPClause.h:2102-2103 child_range used_children() { -return child_range(child_iterator(), child_iterator()); +return child_range(rei

[PATCH] D64991: [analyzer][WIP] Implement a primitive reaching definitions analysis

NoQ added a comment. In D64991#1595853 , @Szelethus wrote: > `CFGElementRef` Wait, is it a thing already?? Did i miss anything??? > This analysis isn't conservative enough yet, I really should include function > calls with non-const references into the

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

NoQ added a comment. The preprocessor is defined in `Lex`, so i guess i'm curious about a more precise description of what's impossible to do without `Frontend`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64638/new/ https://reviews.llvm.org/D64638

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

NoQ marked an inline comment as done. NoQ added a comment. In D64274#1586282 , @Szelethus wrote: > //Ackchyually//, it doesn't per se break anything, but will result in > CodeChecker no longer enabling `optin.cplusplus.VirtualCall` :^) Sorry, > oversigh

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. NoQ added a parent revision: D64274: [a

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

NoQ updated this revision to Diff 211397. NoQ added a comment. - Print only class name::method name instead of a fully qualified name, because fully qualified names may get pretty long. - Remove more dead code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65180/new/ https://reviews.ll

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

NoQ updated this revision to Diff 211399. NoQ added a comment. - Hmm, also i'd rather say "method" than "function". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65180/new/ https://reviews.llvm.org/D65180 Files: clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp clang/test/An

[PATCH] D65182: [analyzer] WIP: Add fix-it hint support.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, kristof.beyls, javed.absar. Herald added a project: clang. That's a p

[PATCH] D65182: [analyzer] WIP: Add fix-it hint support.

NoQ added a comment. Also if we ever implement any fixits for path-sensitive reports that actually make sense, i suspect that it might as well make sense to attach fixits to individual path notes. For examples, if we report a double-free, we may attach a removal of the second free to the warnin

[PATCH] D65212: [analyzer] Fix exporting SARIF files from scan-build on Windows

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

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

NoQ updated this revision to Diff 211587. NoQ added a comment. One more thing: explain the consequences of the bug, so that it was obvious what the problem is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65180/new/ https://reviews.llvm.org/D65180 Files: clang/lib/StaticAnalyzer/Ch

[PATCH] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

NoQ added a comment. Aha, great, the overall structure of the code is correct! Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:477 + + // For all of the bitwise operations, + // if they remain in that 'SymIntExpr' form that means we cannot evaluate the --

[PATCH] D65182: [analyzer] WIP: Add fix-it hint support.

NoQ updated this revision to Diff 211595. NoQ added a comment. Unforget a `FileCheck`-based test file for the virtual call checker that i already had. In D65182#1598614 , @Szelethus wrote: > How does an `-analyzer-config fixits-as-warnings` option sound

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. Change the default behavior: the tool

<    6   7   8   9   10   11   12   13   14   15   >