[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-25 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:336-338 +ProgramStateRef FuchsiaHandleChecker::evalAssume(ProgramStateRef State, + S

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-26 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231078. xazax.hun added a comment. Herald added subscribers: Charusso, gamesh411. - Use matcher. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46027/new/ https://reviews.llvm.org/D46027 Files: clang-tools-extra/clang-tidy/bugprone/SuspiciousSem

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-26 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231089. xazax.hun added a comment. - Handle cases where the number of args/params mismatch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70470/new/ https://reviews.llvm.org/D70470 Files: clang/docs/analyzer/checkers.rst clang/include/clang/S

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

2019-11-26 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: NoQ, haowei. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. xazax.hun added a reviewer: Szelethus. xazax.hu

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-11-26 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Ok, now I have some real world experience with the results of the check. The false positive ratio for double free and use after free seems to be quite good but the handle leak part is almost unusable at this point. The main problem is somewhat anticipated, we are not d

[PATCH] D70755: [LifetimeAnalysis] Fix PR44150

2019-11-26 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: gribozavr, mgehre. xazax.hun added a project: clang. Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs. We really wanted to avoid special handling of references but it looks like this is inevitable. The main dif

[PATCH] D70755: [LifetimeAnalysis] Fix PR44150

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbcd0798c47ca: [LifetimeAnalysis] Fix PR44150 (authored by xazax.hun). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70755/new/ https://reviews.llvm.org/D707

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Thanks! Updated context for this patch: A superior fix would be to follow through with the approach suggested by Richard in https://reviews.llvm.org/D46234 . However, since I do not have time to finish that and there are people complaining in the PR, I think it is be

[PATCH] D70693: [scan-build-py] Set of small fixes

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/tools/scan-build-py/libscanbuild/clang.py:47 cmd.insert(1, '-###') +cmd.append('-fno-color-diagnostics') phosek wrote: > Alternative would be to set `TERM=du

[PATCH] D46234: Mark if a null statement is the result of constexpr folding

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Herald added subscribers: Charusso, gamesh411, Szelethus. Herald added a project: clang. Just a note for anyone willing to pick this up, PR32203 is also related. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46234/new/ https://reviews.l

[PATCH] D46027: [clang-tidy] Fix PR35824

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5c5e860535d8: [clang-tidy] Fix PR35824 (authored by xazax.hun). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D46027?vs=231078&id=231301#toc Repository: rG LLVM Git

[PATCH] D70693: [scan-build-py] Set of small fixes

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. So after investigating the effects of this patch, the result is the following: Verbose mode (`-v`) that also prints findings to the command line will no longer be colored after the patch. If we do not want this regression there are two easy ways around it: 1. Make th

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6524 + if (auto *PVD = dyn_cast(D)) { +if (PVD->getType()->isIntegerType()) { + S.Diag(AL.getLoc(), diag::err_attribute_output_parameter) ---

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231307. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td clang/

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-11-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3445 + let Spellings = [Clang<"acquire_handle">]; + let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>; + let Documentation = [AcquireHandleDocs]; aaron.ballman wrote: > xaz

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

2019-12-02 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. I was thinking about note tags, but since we iterate on the argument/parameter pairs and might change the state in each iteration we would need to add a new transition after each change. I was wondering if using note tags would worth the cost (both in performance due

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

2019-12-02 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Sorry, my previous comment might not be clear. The point is, the same call might both acquire and release handles (there are such calls in Fuchsia), so we might end up adding more than one note for a call for which we would need to add more than one transitions. If y

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

2019-12-02 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 3 inline comments as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:261 + + // TODO: do we want to emit notes for escapes? + // do we want to emit notes for for error checks? I.e. on which bra

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-02 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. @aaron.ballman Type attributes are definitely more flexible as in they can be applied in more contexts. These attributes, however, make no sense outside of a function declaration, or maybe a type alias. So I feel like we could argue on both sides. I made a minimal ve

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-02 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231784. xazax.hun added a comment. - Convert to type attributes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/i

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. 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? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231955. xazax.hun added a comment. - Use the new notes API for path notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h clang

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:362 } - C.addTransition(State); + const NoteTag *T = C.getNoteTag([this, notes](BugReport &BR) -> std::string { +if (&BR.ge

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. You were right, the new API looks cleaner even in this case :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:363-366 +if (&BR.getBugType() != &UseAfterRelease && +&BR.getBugType() != &LeakBugType && +&BR.getBugType() != &DoubleReleaseBugType) + return "";

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231989. xazax.hun marked 5 inline comments as done. xazax.hun added a comment. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 Files: clang/include/clang/StaticAnalyzer/Core/Bu

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D70725#1767673 , @NoQ wrote: > In D70725#1767643 , @Charusso wrote: > > > In D70725#1767579 , @xazax.hun > > wrote: > > > > > Just a small side

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D70725#1767942 , @NoQ wrote: > Mmm, right, i guess you should also skip adding the tag if the notes array is > empty. > > There might be more complicated use-cases (especially in `ExprEngine` itself) > where you may end up a

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 231996. xazax.hun added a comment. - Only add note tag if we have actual notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70725/new/ https://reviews.llvm.org/D70725 Files: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

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

2019-12-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:307 + std::vector> notes; SymbolRef ResultSymbol = nullptr; I will capitalize this name. CHANGES SINCE LAST

[PATCH] D71001: [Clang-Tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

2019-12-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. This might be a strange argument, but I did see this happen several times at different companies. When a company tries to decide which lint tools to use they are doing an evaluation. Usually, people doing the evaluation are not compiler/static analysis devs. Given th

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: NoQ, dcoughlin, Szelethus, baloghadamsoftware, haowei. xazax.hun added a project: clang. Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet. I was running the Fuc

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 7 inline comments as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/fuchsia_handle.cpp:211 + // have any pointer to that symbolic region. + if (zx_channel_create(0, get_handle_address(), &sb)) +return; This one and i

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/fuchsia_handle.cpp:210 + // Because of arrays, structs, the suggestion is to escape when whe no longer + // have any pointer to that symbolic region. + if (zx_channel_c

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In the meantime, I realized lambdas are actually not that important. Lambdas are only usable in C++, and in C++ one should prefer to use move only RAII wrapper for handles. This reduces the problem into a use after move and we already have a check for that. These anno

[PATCH] D70693: [scan-build-py] Set of small fixes

2019-12-05 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8994d632c8d3: [scan-build-py] Set of small fixes (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D70693?vs=230960&id=232364#toc Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-05 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/fuchsia_handle.cpp:210 + // Because of arrays, structs, the suggestion is to escape when whe no longer + // have any pointer to that symbolic region. + if (zx_channel_c

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-05 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/fuchsia_handle.cpp:210 + // Because of arrays, structs, the suggestion is to escape when whe no longer + // have any pointer to that symbolic region. + if (zx_channel_c

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-05 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/fuchsia_handle.cpp:210 + // Because of arrays, structs, the suggestion is to escape when whe no longer + // have any pointer to that symbolic region. + if (zx_channel_c

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-05 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/fuchsia_handle.cpp:210 + // Because of arrays, structs, the suggestion is to escape when whe no longer + // have any pointer to that symbolic region. + if (zx_channel_c

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-05 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/test/Analysis/fuchsia_handle.cpp:210 + // Because of arrays, structs, the suggestion is to escape when whe no longer + // have any pointer to that symbolic region. + if (zx_channel_c

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. So some of the questions that came to my mind: 1. Should this be exposed to the checkers? 2. Where should we actually have this trait registered? ProgramState? ExprEngine? Both of them need access, and due to layering I feel like it cannot be in ExprEngine but I might

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: NoQ, Szelethus, baloghadamsoftware, dcoughlin, haowei. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet. xazax.hun added a comment. So some

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232675. xazax.hun added a comment. - Add cleanup. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71152/new/ https://reviews.llvm.org/D71152 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h clang/lib/StaticAnalyzer/Co

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:48-61 +namespace { +struct EscapedLocals{}; +} // namespace + +template <> +struct ProgramStateTrait : + public ProgramStatePartialTrait> { ---

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232681. xazax.hun marked 7 inline comments as done. xazax.hun added a comment. - Make the annotation acceptable on both types and declarations. - Fix some TODOs. - Teach TypePrinter to print the annotations. - Address review comments. CHANGES SINCE LAST AC

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7412 + diag::warn_type_attribute_wrong_type_str) +<< Attr.getAttrName()->getName() << "non-function" << CurType; +return; aaron.ballman wrote: > You shoul

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-07 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 2 inline comments as done. xazax.hun added inline comments. Comment at: clang/test/Sema/attr-handles.cpp:20-21 +void ht(int __attribute__((acquire_handle(1))) *a); // expected-error {{'acquire_handle' attribute takes no arguments}} +int it() __attribute__((relea

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-07 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232723. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/AST/T

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-08 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr &Attr) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232873. xazax.hun marked 2 inline comments as done. xazax.hun added a comment. - Make sure typedefs are supported. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D70469 Files: clang/include/clang/Basic/Attr.td

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Just a cross reference, some of the discussed changes are implemented here: https://reviews.llvm.org/D71152 Hopefully, more will follow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71041/new/ https://reviews.llvm.org/

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:48-61 +namespace { +struct EscapedLocals{}; +} // namespace + +template <> +struct ProgramStateTrait : + public ProgramStatePartialTrait> { ---

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: NoQ, dcoughlin, Szelethus, baloghadamsoftware, haowei. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet. xazax.hun marked an inline comment

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:402 + InvalidatedSymbols *IS, + RegionAndSymbolInvalidationTraits *ETraits, +

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:696 // And make the result node. Bldr.generateNode(Call.getProgramPoint(), State, Pred); } After some offline

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Also, it looks like for example `CStringChecker` calls `ProgramState::InvalidateRegions` directly when modeling function calls. So it looks like if checkers are using evalCall, pointerEscape will not be triggered automatically. I am not sure if that would be a bug or

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232948. xazax.hun added a comment. - Prototype a new approach. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h clang/include/clang/Static

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 3 inline comments as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:696 // And make the result node. Bldr.generateNode(Call.getProgramPoint(), State, Pred); } NoQ wrote: > xazax

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232955. xazax.hun added a comment. - Do not track explicitly if a call was conservative. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232968. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Added a test. More rigorous tests will come in the FuchsiaHandleChecker. - Added a new PSK_ entry. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https:

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State;

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State;

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:648 + continue; +State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner); + } NoQ wrote: >

[PATCH] D71041: [analyzer][discussion] Talk about escapes

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. This is the next one: https://reviews.llvm.org/D71224 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71041/new/ https://reviews.llvm.org/D71041 ___ cfe-commits mailing list cf

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State;

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 232982. xazax.hun added a comment. - Reuse `ExprEngine::escapeValue`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/clang/

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf3a28202ef58: [analyzer] Keep track of escaped locals (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D71152?vs=232675&id=233124#toc Repository: rG LLVM Github Monorepo CH

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D70469#1777858 , @aaron.ballman wrote: > In D70469#1776523 , @NoQ wrote: > > > It still mildly worries me that the attributes aren't truly reusable, as > > the exact meaning of the at

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624 +if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion()) + if (!MR->hasStackStorage()) +Escape

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624 +if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion()) + if (!MR->hasStackStorage()) +Escape

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624 +if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion()) + if (!MR->hasStackStorage()) +Escape

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D71224#1778179 , @NoQ wrote: > In any case, every checker is allowed to make their own decisions about > escaping. Escape on its own is not material, it's all about how the checker > reacts to escapes. Say, it's up to Malloc

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D71224#1778231 , @NoQ wrote: > In D71224#1778204 , @xazax.hun wrote: > > > I don't think this is a good enough model currently. The problem is that, > > it does not play well with anno

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D71224#1778303 , @NoQ wrote: > In D71224#1778284 , @xazax.hun wrote: > > > So I was wondering if we got the default right. Maybe a checker should do > > more work to get the escaping r

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. I think I found the main problem with the current model, at least for the FuchsiaHandleCheck. Consider the following two snippets: zx_handle_t *get_handle_address(); void escape_store_to_escaped_region01() { zx_handle_t sb; if (zx_channel_create(0, get_han

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233198. xazax.hun added a comment. - A first take on trying to reuse `processPointerEscapedOnBind`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71152/new/ https://reviews.llvm.org/D71152 Files: clang/include/clang/StaticAnalyzer/Checkers/Chec

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Whoops, this one was updated accidentally. Wrong tab :( CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71152/new/ https://reviews.llvm.org/D71152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233200. xazax.hun added a comment. - First take on trying to reuse `processPointerEscapedOnBind`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/include/clang/StaticAnalyzer/Checkers/Checke

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233235. xazax.hun added a comment. - Rebasing after reverting the pre-escaping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/incl

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

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. Cool! :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71321/new/ https://reviews.llvm.org/D71321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. Less state split, yay! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71322/new/ https://reviews.llvm.org/D71322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D70469#1778609 , @NoQ wrote: > What about `__attribute__((acquire_handle("fuchsia")))` I would by fine with this as well. Aaron? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https://reviews.llvm.org/D7

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr &Attr) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong

[PATCH] D71371: [analyzer] Do not cache out on some shared implicit AST nodes.

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: NoQ, dcoughlin, Szelethus, baloghadamsoftware, haowei. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet. Some AST nodes that stands for imp

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5882e6f36fd9: [analyzer] Escape symbols conjured into specific regions during a conservative… (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D71224?vs=233235&id=233431#toc R

[PATCH] D71371: [analyzer] Do not cache out on some shared implicit AST nodes.

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Hmm loops. Right. So this solution is insufficient because it can prevent legitimate caching out in loops. If we remove those initializers from the CFG wouldn't we loose the Pre/PostStmt callbacks? Is that acceptable? If not, we might need to create a new program poi

[PATCH] D71371: [analyzer] Do not cache out on some shared implicit AST nodes.

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233467. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Omit nodes from CFG instead of trying to make the states distinct. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71371/new/ https://reviews.llvm.org/D71371 Files:

[PATCH] D71371: [analyzer] Do not cache out on some shared implicit AST nodes.

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233468. xazax.hun added a comment. - Removed leftover code from previous approach. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71371/new/ https://reviews.llvm.org/D71371 Files: clang/include/clang/Analysis/CFG.h clang/lib/Analysis/CFG.cpp

[PATCH] D71371: [analyzer] Do not cache out on some shared implicit AST nodes.

2019-12-11 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9fdcae7c81f5: [analyzer] Do not cache out on some shared implicit AST nodes (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D71371?vs=233468&id=233475#toc Repository: rG LL

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

2019-12-12 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233691. xazax.hun added a comment. - Fix some problems discovered during some real world stress test - Add more tests - The ASCII art is not updated yet, but will do so at some point. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70470/new/ https:

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-16 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:7424 + ParsedAttr &Attr) { + if (CurType->isFunctionType()) { +State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234341. xazax.hun added a comment. - Added string argument to specify the handle type. - Only AcquireHandleAttr can be a type attribute and it can only be applied to function types. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70469/new/ https:/

[PATCH] D71612: [analyzer] Add PlacementNewChecker

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:22 +ProgramStateRef State) const; + mutable std::unique_ptr BT_Placement; +}; I think now it is safe to have the bugtype by value and

[PATCH] D71642: [CFG] Add on option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: NoQ, Szelethus, mgehre. xazax.hun added a project: clang. Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, rnkovacs. This is useful for clients that are relying on linearized CFGs for evaluating subexpressions and want

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234427. xazax.hun added a comment. - Add missing tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71642/new/ https://reviews.llvm.org/D71642 Files: clang/include/clang/Analysis/CFG.h clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234434. xazax.hun marked 2 inline comments as done. xazax.hun added a comment. - Fix review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71642/new/ https://reviews.llvm.org/D71642 Files: clang/include/clang/Analysis/CFG.h clang/inc

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 234435. xazax.hun added a comment. - Remove accidentally added files. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71642/new/ https://reviews.llvm.org/D71642 Files: clang/include/clang/Analysis/CFG.h clang/include/clang/StaticAnalyzer/Core/A

[PATCH] D71642: [CFG] Add an option to inline CXXDefaultInitExpr into aggregate initialization

2019-12-17 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGea93d7d64216: [CFG] Add an option to expand CXXDefaultInitExpr into aggregate initialization (authored by xazax.hun). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

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