[PATCH] D79232: [analyzer] Refactor range inference for symbolic expressions

2020-05-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:385 + + RangeSet VisitAndOperator(RangeSet LHS, RangeSet RHS, QualType T) { +// TODO: generalize for the ranged RHS. I always get surprised when I read code

[PATCH] D79232: [analyzer] Refactor range inference for symbolic expressions

2020-05-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:340-345 +// TODO #2: We didn't go into the nested expressions before, so it +// might cause us spending much more time doing the inference. +// This can be a problem

[PATCH] D79156: [analyzer] Merge implementations of SymInt, IntSym, and SymSym exprs

2020-04-30 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Nice catch, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79156/new/ https://reviews.llvm.org/D

[PATCH] D79072: [Analyzer][VLASizeChecker] Check VLA size in typedef and sizeof.

2020-04-29 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Overall the changes look good to me here. I had a small nit inline. But I wonder if we actually should add more code in the analyzer core to better model the sizes of memory regions corresponding to the VLAs. Comment at: clang/lib/StaticAnalyzer/Cor

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2871 + // Overwrite the associated constraint of the Symbol. + Constraints = CF.remove(Constraints, Sym); Constraints = CF.add(Constraints, Sym, C.second);

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. I think this should not be blocked on the gtest update. If getting an updated gtest into the repo takes to much time and the reviewers are happy, I'm fine with doing that change as a fol

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp:24 + do \ +if (!LLVM_WITH_Z3)

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. Overall looks good for me, thanks for tackling this problem! I think this should be good to go once Eli's comment is fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77809/new/ https

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78457/new/ https://reviews.llvm.org/D78457

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Actually, sorry. I just realized that the alignof problem is introduced by this patch. I'd love to see the solution committed together with this patch (it could be a separate patch but preferably, they should be committed together.) Repository: rG LLVM Github Monor

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2751 +Optional hasContradictionUsingZ3(BugReporterContext &BRC, + const ExplodedNode *EndPathNode) { Is this function used anywhere?

[PATCH] D78704: [analyzer][NFC] Add unittest for FalsePositiveRefutationBRVisitor

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Thanks! Having more tests is always welcome! I mentioned some nits inline, but I wonder if you actually need to add a new check. Can't you just reuse existing debug checks? We have the expr inspeciton checker that supports the following functions: clang_analyzer_war

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

2020-04-23 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Thanks, this looks much better now. Could you also update the description of the revision to match the current status? (E.g. type aliases are now supported.) If you do not plan to solve t

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

2020-04-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. Ok, looks good to me. The minor nit regarding the naming is easy to fix before commit. The design question I had is not a blocker, my suggested alternative can be implemented later (if desired) in a backward-compatible way from the us

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

2020-04-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:56 + load_threshold_reached, + ambiguous_compile_commands_database }; The two enum values refer to compilation database and compile command database. I'd prefer to

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D78457#1991780 , @martong wrote: > If a symbol is unused and garbage collected then that is not part of the path > constraint that leads to the ErrorNode, is it? So why should we care about > constraints on an unused symbol?

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. One way to test this change would be to add a statistic that is bumped each time a path is refuted (a report to be refuted we need all of the paths refuted, so using refuted reports might not be fine-grained enough). We can test on some big projects if the refuted pat

[PATCH] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

2020-04-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. > As turned out we don't even need a BugReporterVisitor for doing the > crosscheck. > We should simply use the constraints of the ErrorNode since that has all the > necessary information. This should not be true. But we should definitely have a test case that proves

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

2020-04-16 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VarDecl *VD = dyn_cast(DS->getSingleDecl()); balazske wrote: > Szelethus wrote: > > How about `using`? How about some other shenanigans that obscure the size > > of the VLA? Can'

[PATCH] D78233: [NFC] Correcting minor typo.

2020-04-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78233/new/ https://reviews.llvm.org/D78233

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

2020-04-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun requested changes to this revision. xazax.hun added a comment. This revision now requires changes to proceed. I am not an expert when it comes to VLAs but I do see some problems here. First of all, we do not want to include typedef statements in the CFG as they are noops in terms of th

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

2020-03-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D63279#1939349 , @Szelethus wrote: > (note: I forgot to submit this a couple weeks ago) > > LLVM is crashing on me due to the issue mentioned in D75678 > , but on Bitcoin, Xerces, CppCheck and

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-03-22 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:7727 +// Move function type attribute to the declarator. +case ParsedAttr::AT_LifetimeContract: aaron.ballman wrote: > xazax.hun wrote:

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-03-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 251569. xazax.hun marked an inline comment as done. xazax.hun added a comment. - Rebase. - Address some review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72810/new/ https://reviews.llvm.org/D72810 Files: clang/include/clang/AST/Att

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-03-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 14 inline comments as done. xazax.hun added inline comments. Comment at: clang/include/clang/AST/LifetimeAttr.h:163 +// Maps each annotated entity to a lifetime set. +using LifetimeContracts = std::map; + aaron.ballman wrote: > xazax.hun wrote: >

[PATCH] D76287: [analysis][analyzer] Introduce the skeleton of a reaching definitions calculator

2020-03-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D76287#1929221 , @Szelethus wrote:d: > //Variable// could be practically anything that can be written, but due to > the nature of what we can work this, we have to make severe restrictions. > > - We don't really have a good p

[PATCH] D76287: [analysis][analyzer] Introduce the skeleton of a reaching definitions calculator

2020-03-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added reviewers: gribozavr2, ymandel. xazax.hun added a comment. Herald added a subscriber: ASDenysPetrov. Added some more reviewers who might be interested. I think it is very crucial to make the intentions clear, how do you define `definition` and `variable`? Other than assignments w

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

2020-03-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:33-45 + enum KindTy { +Opened, /// Stream is opened. +Closed, /// Closed stream (an invalid stream pointer after it was closed). +OpenFailed /// The last open operation h

[PATCH] D75682: [Analyzer][StreamChecker] Introduction of stream error handling.

2020-03-11 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:33-45 + enum KindTy { +Opened, /// Stream is opened. +Closed, /// Closed stream (an invalid stream pointer after it was closed). +OpenFailed /// The last open operation h

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

2020-03-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D75842#1912364 , @baloghadamsoftware wrote: > In D75842#1912293 , @xazax.hun wrote: > > > I am not sure if I would call this a bugfix. Enabling a checker without one > > of its depend

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

2020-03-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. I am not sure if I would call this a bugfix. Enabling a checker without one of its dependency will potentially cause the checker to misbehave. I am uncomfortable changing the current behavior to a more dangerous one without any diagnostics. Including the diagnostic wi

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

2020-03-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. If we disable the dependency of a checker explicitly I think we should at least emit a warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75842/new/ https://reviews.llvm.org/D75842

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

2020-03-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D75163#1902921 , @balazske wrote: > In D75163#1902816 , @xazax.hun wrote: > > > If we were to refactor this check I wonder if it would make sense to port > > `evalCall` to `postCall`,

[PATCH] D75430: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

2020-03-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. This might be a silly question but is `CXXDeleteExpr` the only way to invoke a deallocator? What would happen if the user would call it by hand? Should we expect `ExprEngine` to trigger a callback in that case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D75431: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

2020-03-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Other than a nit looks good to me but wait for @NoQ before committing. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:208 + SValBuilder &getSValBuilder() const { +return State->getStateManager().getSValBuilder();

[PATCH] D75432: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

2020-03-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Some nits inline. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:995 -void MallocChecker::checkBasicAlloc(CheckerContext &C, const CallExpr *CE, -ProgramStateRef State) const { - State = MallocMe

[PATCH] D75163: [analyzer][StreamChecker] Adding precall and refactoring.

2020-03-03 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. If we were to refactor this check I wonder if it would make sense to port `evalCall` to `postCall`, so the analyzer engine will conjure the symbol for us. I wonder what @NoQ thinks about the pros and cons of the approaches. As far as I understand the con of evalCall is

[PATCH] D73729: [analyzer] AnalyzerOptions: Remove 'fixits-as-remarks'

2020-02-25 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:310 -ANALYZER_OPTION(bool, ShouldEmitFixItHintsAsRemarks, "fixits-as-remarks", -"Emit fix-it hints as remarks for testing purposes", -false) --

[PATCH] D73729: [analyzer] AnalyzerOptions: Remove 'fixits-as-remarks'

2020-02-25 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a reviewer: Szelethus. xazax.hun added a comment. Herald added subscribers: martong, steakhal. Maybe Kristof has some opinion whether we still need this :) He might be up to date whether CodeChecker is using this feature. Repository: rC Clang CHANGES SINCE LAST ACTION https

[PATCH] D74735: [analyzer] Add support for CXXInheritedCtorInitExpr.

2020-02-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. If the AST is hard to work with would it make sense to try to change the AST a bit? Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:944 + if (Data) { +loc::MemRegionVal MV(static_cast(Data)); +if (SymbolRef Sym = MV.getAsSymbol(true)

[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

2020-02-07 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. You cannot always have constant bounds. E.g. a dynamically allocated array size might depend on a variable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73536/new/ https://reviews.llvm.org/D73536

[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

2020-02-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. I think a crucial part of the design is what would we do for the following case: if (x < y || x > z) return; // Here we might not have ranges for x when y and z were symbolic. mySink(x); // requires x to be in [0, 255] So would we warn for the code above? X

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

2020-02-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:268 CheckerContext &C) { - if (isTainted(State, E, C.getLocationContext()) || isStdin(E, C)) + if (isTainted(State, E, C.ge

[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

2020-02-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:401 +return; + case nonloc::ConcreteIntKind: { +const llvm::APSInt &IntVal = V.castAs().getValue(); Dealing with only concrete ints might be a g

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

2020-02-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Please add a test as well. Otherwise looks good. Comment at: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp:97 .Case("clang_analyzer_express", &ExprInspectionChecker::analyzerExpress) +.StartsWith("clang_analyzer_isTainted", &Ex

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

2020-02-05 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/docs/ReleaseNotes.rst:405 +- New checker: ``fuchsia.HandleChecker`` to detect leaks related to Fuchsia + handles. NoQ wrote: > D74004 > > 1) The checker is now in alpha. > 2) This checker wasn't enabled in th

[PATCH] D74004: [analyzer] Move fuchsia.Lock checker to alpha

2020-02-05 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG643dee903ceb: [analyzer] Move fuchsia.Lock checker to alpha (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D74004?vs=242452&id=242781#toc Repository: rG LLVM Github Monore

[PATCH] D74004: [analyzer] Move fuchsia.Lock checker to alpha

2020-02-05 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D74004#1858185 , @NoQ wrote: > Also you'll need to update release notes (D73966 > ) :) I think this was merged after the release branch was created (https://github.com/llvm/llvm-project/blo

[PATCH] D74003: [analyzer] Prevent assertion failure in PThreadLockChecker when the implementations of the locking functions are available

2020-02-05 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe4f4a6c0f5bb: [analyzer] Prevent an assertion failure in PThreadLockChecker (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D74003?vs=242447&id=242778#toc Repository: rG LL

[PATCH] D74003: [analyzer] Prevent assertion failure in PThreadLockChecker when the implementations of the locking functions are available

2020-02-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D74003#1858144 , @NoQ wrote: > That's a safe default behavior, but ideally you should see if the annotation > on the function can be applied after inlining. Like, it isn't necessarily > always applicable, but when it is, you

[PATCH] D74004: [analyzer] Move fuchsia.Lock checker to alpha

2020-02-04 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. After experimenting with it a bit it looks lik

[PATCH] D74003: [analyzer] Prevent assertion failure in PThreadLockChecker when the implementations of the locking functions are available

2020-02-04 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, jfb, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. This is very unlikely to happen for PThre

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

2020-02-04 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/docs/ReleaseNotes.rst:429 + +- Numerous smaller false positive fixes. I'd just say `Numerous smaller fixes.` or `Numerous other improvements.` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-01-29 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp:109 "execvpe", + "execve", nullptr Well, this is not the case now, but I wonder if it would also make sense to sort this list alphabetically. CH

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

2020-01-27 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf4c26d993bdc: [analyzer] Add FuchsiaLockChecker and C11LockChecker (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D73376?vs=240650&id=240678#toc Repository: rG LLVM Github

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

2020-01-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 240650. xazax.hun added a comment. - Add a FIXME. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73376/new/ https://reviews.llvm.org/D73376 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/lib/StaticAnalyzer/Checkers/Pthrea

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

2020-01-27 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc98d98ba9b0f: [analyzer] Fix handle leak false positive when the handle dies too early (authored by xazax.hun). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

2020-01-27 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/Analysis/c11lock.c:7 +enum { + thrd_success = 0, + thrd_error = 2 Strictly speaking, this is implementation defined. But the C11 implementations I am aware of a

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

2020-01-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D73376#1839818 , @NoQ wrote: > I don't see any immediate solutions to the boilerplate that don't consist in > introducing better checker APIs. Eg., we could have introduced a > `LazyBugType` - a wrapper around `Optional` tha

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

2020-01-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 240624. xazax.hun marked 2 inline comments as done. xazax.hun added a comment. - Add more tests. - Move the C11 checker to alpha.core CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73376/new/ https://reviews.llvm.org/D73376 Files: clang/include/

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

2020-01-27 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. I've rerun this on Fuchsia and it got the same results as the previous approach :). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73151/new/ https://reviews.llvm.org/D73151 ___ cfe-commits mailing list cfe-commit

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

2020-01-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 240337. xazax.hun added a comment. - Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73151/new/ https://reviews.llvm.org/D73151 Files: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp clang/test/Analysis/fuchs

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

2020-01-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 4 inline comments as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:133-135 + static HandleState getWithoutError(HandleState S) { +return HandleState(S.K, nullptr); + } xazax.hu

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-01-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 240335. xazax.hun added a comment. - Address some review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72810/new/ https://reviews.llvm.org/D72810 Files: clang/include/clang/AST/Attr.h clang/include/clang/AST/LifetimeAttr.h clang/i

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-01-24 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 14 inline comments as done. xazax.hun added inline comments. Comment at: clang/include/clang/AST/LifetimeAttr.h:163 +// Maps each annotated entity to a lifetime set. +using LifetimeContracts = std::map; + gribozavr2 wrote: > Generally, DenseMap a

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

2020-01-24 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/c11lock.c:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.C11Lock -verify %s + NoQ wrote: > NoQ wrote: > > I wouldn't mind `alpha.core` given t

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

2020-01-24 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/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:133-135 + static HandleState getWithoutError(HandleState S) { +return HandleState(S.K, nullptr); + } NoQ wrot

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

2020-01-24 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, jfb, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. Basically, the semantics of C11 and Fuchs

[PATCH] D73229: [analyzer] Improve FuchsiaHandleChecker's diagnostic messages

2020-01-23 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5911268e441c: [analyzer] Improve FuchsiaHandleChecker's diagnostic messages (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D73229?vs=239698&id=239924#toc Repository: rG LL

[PATCH] D73229: [analyzer] Improve FuchsiaHandleChecker's diagnostic messages

2020-01-23 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:363 } else -return ""; +return std::string{}; }); NoQ wrote: > What was

[PATCH] D73229: [analyzer] Improve FuchsiaHandleChecker's diagnostic messages

2020-01-22 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. I found that this information is useful when t

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-01-22 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 6 inline comments as done. xazax.hun added inline comments. Comment at: clang/include/clang/AST/LifetimeAttr.h:143 +using LifetimeContractSet = std::set; +using LifetimeContractMap = std::map; + gribozavr2 wrote: > These names are not descriptive

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-01-22 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 239644. xazax.hun marked 10 inline comments as done. xazax.hun added a comment. - Address most of the review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72810/new/ https://reviews.llvm.org/D72810 Files: clang/include/clang/AST/Attr.

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

2020-01-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 239469. xazax.hun added a comment. - Minor refactoring. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73151/new/ https://reviews.llvm.org/D73151 Files: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp clang/test/Analysis/fuchsia_han

[PATCH] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

2020-01-21 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. In case the handle symbol dies too early, even

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

2020-01-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D72982#1832029 , @aganea wrote: > In D72982#1832000 , @xazax.hun wrote: > > > Thanks! Alternatively we could try to push the changes to all three > > versions and revert this patch onc

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

2020-01-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D72982#1831976 , @hans wrote: > Sigh, what a mess. I totally agree and sorry for that :( >> In case it is printed on a separate line the current parsing happens to work >> in all versions of scan-build and this seemed to b

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

2020-01-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D72982#1831817 , @hans wrote: > In D72982#1831648 , @xazax.hun wrote: > > > In D72982#1831595 , @hans wrote: > > > > > Wait, do we really want t

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

2020-01-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D72982#1831595 , @hans wrote: > Wait, do we really want the "(in-process)" marker to be written to a separate > line? I'm not sure that we do. Since the `-###` command had this property of emitting copy pastable `-cc1` inv

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

2020-01-21 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Do you have commit access or need someone to commit on your behalf? Also, in case your change made it into the clang 10 release branch this will need to be cherry picked there as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D72982: [Clang] Un-break scan-build after integrated-cc1 change

2020-01-19 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72982/new/ https://reviews.llvm.org/D72982

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2020-01-18 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. There might be other tools parsing the output of `-###`. I think adding a line break in the output is better/easier than updating all those tools (amd making them potentially slower, not profiting from this patch). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2020-01-17 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added subscribers: phosek, NoQ, haowei, xazax.hun. xazax.hun added a comment. Hi! This patch breaks scan-build-py which parses the output of "-###" to get -cc1 command. There might be other tools with the same problems. Could we either remove `(in-process)` from `CC1Command::Print` or

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-17 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG05c7dc664809: [DataFlow] Factor two worklist implementations out (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D72380?vs=238592&id=238785#toc Repository: rG LLVM Github M

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-16 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 238592. xazax.hun added a comment. - Fix typo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72380/new/ https://reviews.llvm.org/D72380 Files: clang/include/clang/Analysis/FlowSensitive/DataflowValues.h clang/include/clang/Analysis/FlowSensit

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Here is an example: void f() { int i; while (i < 42 && i) { if (i) &i; } } This takes 17 visits before, 16 after. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72380/new/ https://reviews.llvm.org/D72380 __

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D72380#1823019 , @xazax.hun wrote: > In D72380#1822927 , @NoQ wrote: > > > The change in uninitialized values analysis gives me a bit of anxiety. > > Could you explain what exactly has

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. In D72380#1822927 , @NoQ wrote: > The change in uninitialized values analysis gives me a bit of anxiety. Could > you explain what exactly has changed that caused the change in the stats and > why you think it doesn't make a dif

[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-01-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: mgehre, ymandel, gribozavr2, aaron.ballman, rsmith, rjmccall. xazax.hun added a project: clang. Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs, mgorny. This patch corresponds to this RFC: http://lists.llvm.o

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Ping for the other reviewers :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72380/new/ https://reviews.llvm.org/D72380 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailm

[PATCH] D71612: [analyzer] Add PlacementNewChecker

2020-01-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:91 + SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State); + const auto SizeOfTargetCI = S

[PATCH] D71612: [analyzer] Add PlacementNewChecker

2020-01-09 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:91 + SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State); + const auto SizeOfTargetCI = SizeOfTarget.getAs(); + if (!SizeOfTargetCI) martong wrote: > xaza

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-08 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 236886. xazax.hun added a comment. - Added doxygen comments and unit tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72380/new/ https://reviews.llvm.org/D72380 Files: clang/include/clang/Analysis/FlowSensitive/DataflowValues.h clang/incl

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-08 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/include/clang/Analysis/FlowSensitive/DataflowWorklist.h:20 +namespace clang { +template class DataflowWorklistBase { + llvm::BitVector EnqueuedBlocks; xazax.hun wrote

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-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/include/clang/Analysis/FlowSensitive/DataflowWorklist.h:20 +namespace clang { +template class DataflowWorklistBase { + llvm::BitVector EnqueuedBlocks; mgehre wrote: >

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-08 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment. Fortunately, UninitializedValues has some statistics. So I printed it for a big translation unit (SemaExpr.cpp) before and after this change. Before: *** Analysis Based Warnings Stats: 33023 functions analyzed (0 w/o CFGs). 161696 CFG blocks built. 4 avera

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-08 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 236873. xazax.hun added a comment. - Prepopulating the worklist for UninitializedValues seems to be redundant. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72380/new/ https://reviews.llvm.org/D72380 Files: clang/include/clang/Analysis/FlowSens

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-07 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 236734. xazax.hun added a comment. - Add missing new line at the end of file. - Populate the initial worklist in UninitializedValues more efficiently. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72380/new/ https://reviews.llvm.org/D72380 Files:

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

2020-01-07 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun created this revision. xazax.hun added reviewers: gribozavr2, NoQ, Szelethus, rsmith. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, rnkovacs. In Clang, we have the tendency to have ad-hoc worklist implementations everywhere. This patch is a fir

[PATCH] D72289: [analyzer] Update help text to reflect sarif support

2020-01-07 Thread Gábor Horváth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG46ac6a4dcd9b: [analyzer] Update help text to reflect sarif support (authored by xazax.hun). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72289/new/ https:/

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