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

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

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

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:

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

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.

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

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=240678#toc Repository: rG LLVM Github

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

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

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

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] D74004: [analyzer] Move fuchsia.Lock checker to alpha

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

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

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

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

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

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

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=242778#toc Repository: rG LLVM

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

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

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

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=242781#toc Repository: rG LLVM Github Monorepo

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

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", ::analyzerExpress) +.StartsWith("clang_analyzer_isTainted", ::analyzerIsTainted)

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

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:401 +return; + case nonloc::ConcreteIntKind: { +const llvm::APSInt = V.castAs().getValue(); Dealing with only concrete ints might be a good

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

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:268 CheckerContext ) { - if (isTainted(State, E, C.getLocationContext()) || isStdin(E, C)) + if (isTainted(State, E,

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

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] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

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] D73229: [analyzer] Improve FuchsiaHandleChecker's diagnostic messages

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG5911268e441c: [analyzer] Improve FuchsiaHandleCheckers diagnostic messages (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D73229?vs=239698=239924#toc Repository: rG LLVM

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

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

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

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

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

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

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

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

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

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

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

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); + }

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

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

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

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,

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

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 =

[PATCH] D71612: [analyzer] Add PlacementNewChecker

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 =

[PATCH] D71612: [analyzer] Add PlacementNewChecker

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: >

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

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

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

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:

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

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

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

xazax.hun added a comment. Here is an example: void f() { int i; while (i < 42 && i) { if (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

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

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

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`

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

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

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

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] D73151: [analyzer] Fix handle leak false positive when the handle dies too early

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

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

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,

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

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:

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

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

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

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

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

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

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

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=238785#toc Repository: rG LLVM Github

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

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

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

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

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

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:

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

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

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

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

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

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

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

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

xazax.hun added a comment. In D72018#1800018 , @aaron.ballman wrote: > This seems like a very specialized attribute for the static analyzer and I'm > not certain how much utility it adds, but that may be because I don't > understand the analyzer

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

xazax.hun added a comment. In D72018#1801725 , @aaron.ballman wrote: > Thank you for the explanation, that makes sense, but I'm still a bit > uncomfortable. In this case, it seems like the author of the API needs to 1) > know that users will be

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

xazax.hun created this revision. xazax.hun added reviewers: aaron.ballman, gribozavr2, mgehre. xazax.hun added a project: clang. Herald added subscribers: Szelethus, Charusso, gamesh411, dkrupp, rnkovacs. It turns out it is useful to be able to define the deref type as void. In case we have a

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

xazax.hun added a comment. In D72097#1801777 , @gribozavr2 wrote: > Could you provide a more fleshed out example of a case where it is useful? Sure! The `std::any` could be one example (`std::variant` is similar, but ultimately, we migh want to be able

[PATCH] D72113: [CMake] clang-scan-deps in Fuchsia distribution

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72113/new/ https://reviews.llvm.org/D72113

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

xazax.hun added a comment. In D72018#1803012 , @aaron.ballman wrote: > In D72018#1802636 , @NoQ wrote: > > > Would changing the literal in the attribute have the same effect? I.e., > >

[PATCH] D72188: [fuchsia] Enable Clang Static Analyzer

xazax.hun created this revision. xazax.hun added a reviewer: phosek. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, a.sidorin, rnkovacs, baloghadamsoftware, mgorny. phosek accepted this revision. phosek added a comment. This

[PATCH] D72188: [fuchsia] Enable Clang Static Analyzer

This revision was automatically updated to reflect the committed changes. Closed by commit rG0458e63d28a6: [fuchsia] Enable Clang Static Analyzer (authored by xazax.hun). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72188/new/

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

xazax.hun updated this revision to Diff 236150. xazax.hun added a comment. - Fix typo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72097/new/ https://reviews.llvm.org/D72097 Files: clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td

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

xazax.hun marked 2 inline comments as done. xazax.hun added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4619 This attribute may be used by analysis tools and has no effect on code -generation. +generation. A ``void`` argument means that the pointer cna

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

xazax.hun updated this revision to Diff 236148. xazax.hun added a comment. - Update the documentation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72097/new/ https://reviews.llvm.org/D72097 Files: clang/include/clang/Basic/AttrDocs.td

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

xazax.hun updated this revision to Diff 235161. xazax.hun added a comment. I decided to fix the unittests. Having CFGs full of dangling pointers to the AST does not look fun at all, so I think this change was long overdue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71791/new/

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:62 Finder->addMatcher( - ifStmt(stmt().bind("if"), + ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())), + stmt().bind("if"),

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Please add new lines to the end of the test files. Also please add a test similar to the godbolt link you gave me to show that `if consexpr (false)`

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

xazax.hun added a comment. Thanks for working on this, please see one of my concerns inline. If you are trying to fix this problem, alternatively, you could also fix it in Clang. Richard outlined the best possible way to solve this class of errors by introducing a new type of AST node in:

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

xazax.hun added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:62 Finder->addMatcher( - ifStmt(stmt().bind("if"), + ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())), + stmt().bind("if"),

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

xazax.hun created this revision. xazax.hun added reviewers: aaron.ballman, NoQ, haowei. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. While the static analyzer is

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG379613d7c7fa: [CFG] Fix an assertion failure with static initializers (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D71791?vs=235161=235185#toc Repository: rG LLVM

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

xazax.hun created this revision. xazax.hun added reviewers: aaron.ballman, NoQ. xazax.hun added a project: clang. Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. Repository: rG LLVM Github Monorepo

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

This revision was automatically updated to reflect the committed changes. xazax.hun marked an inline comment as done. Closed by commit rG247a6032549e: [LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner… (authored by xazax.hun). Repository: rG LLVM Github Monorepo

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

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/

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

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

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

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

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

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

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

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

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

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

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

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

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

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] D78233: [NFC] Correcting minor typo.

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] D75665: [analyzer] On-demand parsing capability for CTU

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

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

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] D78457: [analyzer][Z3-refutation] Fix refutation BugReporterVisitor bug and refactor

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

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

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

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

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

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

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

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

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?

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

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] D77809: [Analyzer] Include typedef statements in CFG build.

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

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

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

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

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:

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

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2751 +Optional hasContradictionUsingZ3(BugReporterContext , + const ExplodedNode *EndPathNode) { Is this function used anywhere?

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

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:

[PATCH] D79336: [analyzer] Generalize bitwise OR rules for ranges

xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:77 + assert(!isEmpty()); + // NOTE: It's a shame that we can't implement 'getMaxValue' without scanning + // the whole tree to get to the last element.

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

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] D79232: [analyzer] Refactor range inference for symbolic expressions

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

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

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/

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

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/

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

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

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