[PATCH] D39682: [analyzer] Fix a crash on logical operators with vectors.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:633 +// modeled as short-circuit in Clang CFG but this is incorrect. +StmtNodeBuilder Bldr(Pred, Dst,

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

xazax.hun added a comment. In https://reviews.llvm.org/D35109#916617, @NoQ wrote: > A breakthrough with credit going to Devin: Note that whenever we're not > dealing with `>`/`<`/`<=`/`>=` (but only with additive ops and `==` and `!=`, > and we have everything of the same type) we can rearrange

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun updated this revision to Diff 121700. xazax.hun added a comment. - Rebased on ToT https://reviews.llvm.org/D30691 Files: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/AnalyzerOptions.cpp

[PATCH] D39707: [analyzer] assume bitwise arithmetic axioms

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. This looks like a great addition! Apart from some nits, LGTM. Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:487 + +// result >= constant +

[PATCH] D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast

xazax.hun added a comment. In https://reviews.llvm.org/D39711#917433, @dcoughlin wrote: > @xazax.hun Would you be willing to take a look? Sure, I basically agree with George. Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:587 if (TrackedType && +

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

xazax.hun updated this revision to Diff 121885. xazax.hun marked 3 inline comments as done. xazax.hun added a comment. - Fix review comments https://reviews.llvm.org/D33722 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/CopyConstructorInitCheck.cpp clang-tidy/misc/CopyConstructorIn

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

xazax.hun updated this revision to Diff 121886. xazax.hun marked 2 inline comments as done. xazax.hun added a comment. - Fix doc comments that I overlooked earlier https://reviews.llvm.org/D33722 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/CopyConstructorInitCheck.cpp clang-tidy

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

xazax.hun added a comment. In https://reviews.llvm.org/D33722#916990, @aaron.ballman wrote: > In https://reviews.llvm.org/D33722#916540, @xazax.hun wrote: > > > Also, bugprone might be a better module to put this? > > > I don't have strong opinions on misc vs bugprone (they're both effectively >

[PATCH] D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:587 if (TrackedType && + !isa(CE) && !ASTCtxt.canAssignObjCInterfaces(DestObjectPtrType, *TrackedType) && george.karpenkov wrote: > dcoughlin wrote:

[PATCH] D39886: [ASTImporter] Fix wrong conflict detections for unnamed structures

xazax.hun added a reviewer: doug.gregor. xazax.hun added a comment. Doug added anonymous structure handling, added as a reviewer in case he wants to have a look. https://reviews.llvm.org/D39886 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D39711: [analyzer] ObjCGenerics: Don't warn on cast conversions involving explicit cast

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D39711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D39372: Make DiagnosticIDs::getAllDiagnostics static.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D39372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

xazax.hun added a comment. I agree it might be useful to expose this matcher to everybody. https://reviews.llvm.org/D38921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39803: [analyzer] pr34766: Fix a crash on explicit construction of std::initializer_list.

xazax.hun accepted this revision. xazax.hun added a comment. LGTM! https://reviews.llvm.org/D39803 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39800: [analyzer] pr34404: Fix a crash on pointers to members in nested anonymous structures.

xazax.hun accepted this revision. xazax.hun added a comment. LGTM! https://reviews.llvm.org/D39800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

xazax.hun added a comment. In https://reviews.llvm.org/D39049#910482, @NoQ wrote: > // TODO: once the constraint manager is smart enough to handle non > simplified > // symbolic expressions remove this function. Note that this can not be > used in > // the constraint manager as is, since

[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! But wait for @dcoughlin, @zaks.anna , or @NoQ before commit. https://reviews.llvm.org/D37187 ___ cfe-commits mailing list cfe-commit

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

xazax.hun added inline comments. Comment at: include/clang/Sema/TemplateInstCallback.h:44 +template +void initialize(TemplateInstantiationCallbackPtrs& Callbacks_, const Sema &TheSema) +{ Nit: this file should be clang formatted, it does not follow the LLVM st

[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases

xazax.hun added a comment. In https://reviews.llvm.org/D38171#909346, @leanil wrote: > In https://reviews.llvm.org/D38171#901427, @xazax.hun wrote: > > > One problem to think about when we add all clang-diagnostic as "first or > > second" class citizen, `checkes=*` might now enable all the warni

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:382 +DescFile<"CheckSecuritySyntaxOnly.cpp">; + def DeprecatedBufferHandling : Checker<"DeprecatedBufferHandling">, +HelpText<"Warn on uses of deprecated buffer manipulating fu

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

xazax.hun added a comment. In https://reviews.llvm.org/D35109#926078, @baloghadamsoftware wrote: > So still the options are to fix it in the checker or fix it in the engine > (the max/4 or the type extension solution), but leaving it unfixed is not an > option. I am open to any solution, but on

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

xazax.hun added a comment. In https://reviews.llvm.org/D39049#926597, @danielmarjamaki wrote: > > Could you do a similar analysis that I did above to check why does this not > > work for the multidimensional case? (I.e.: checking what constraints are > > generated and what the analyzer does wit

[PATCH] D39438: [analyzer] Diagnose stack leaks via block captures

xazax.hun added a comment. I found some nits, but overall I think this is getting close. Comment at: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:76 range = CL->getSourceRange(); - } - else if (const AllocaRegion* AR = dyn_cast(R)) { + } else if (const AllocaR

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. A nit, otherwise LG! Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:693 SMng); + } else if (Optional BE = P.getAs()) { +S

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

xazax.hun requested changes to this revision. xazax.hun added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D39049#928620, @danielmarjamaki wrote: > > So what are the arguments that are passed to getSimplifiedOffset() in > that case? 0? That does not see

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

xazax.hun requested changes to this revision. xazax.hun added a comment. This revision now requires changes to proceed. The checker documentation should be updated as well. Comment at: include/clang/ASTMatchers/ASTMatchers.h:2251 +// will generate TypeTraitExpr <...> 'int' +co

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

xazax.hun added a comment. This fix looks good for me for this particular problem, but I wonder whether the solution is general enough. In case the analysis figures out that a call would not return (e.g., the `value` method is called on a provably empty optional, and it would throw instead of r

[PATCH] D146538: [analyzer] Fix crashing getSValFromInitListExpr for nested initlists

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Ugh :/ I wonder if we should also open tickets on GitHub to reduce the chance of forgetting addressing the root cause for these. What do you think? Repository: rG LLVM Github Monorep

[PATCH] D146507: [clang][dataflow][NFC] Eliminate StmtToEnvMap interface.

xazax.hun accepted this revision. xazax.hun added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146507/new/ https://reviews.llvm.org/D146507 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D146514: [clang][dataflow] Fix crash when RHS of `&&` or `||` calls `noreturn` func.

xazax.hun added a comment. In D146514#4212528 , @mboehme wrote: > No, I think this is a different case. Sorry, I might have been unclear. I am 100% agree that these are different cases. I was wondering whether we can/should have a single code path suppo

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

xazax.hun added a comment. While I am OK with this approach, I wonder if it would be too much work to split the HTML generation and emitting data up. E.g., the logger could emit YAML or JSON that could be parsed by a python script to create the HTML (or the HTML itself could use JS to parse it

[PATCH] D144730: [FlowSensitive] Log analysis progress for debugging purposes

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:186 + Logger &logger() const { +return DACtx->getOptions().Log ? *DACtx->getOptions().Log : Logger::null(); + } If we already have a `NullLogger`,

[PATCH] D144730: [FlowSensitive] Log analysis progress for debugging purposes

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Logger.cpp:17 +Logger &Logger::null() { + struct NullLogger : Logger {}; + static auto *Instance = new NullLogger(); Adding `final`? Just in case it can help with devirtualization.

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

xazax.hun added a comment. In D146591#4213614 , @sammccall wrote: > (e.g. does the MLIR dataflow framework have the same idea about how the > analysis timeline relates to the CFG and elements within it? And if we want > to show the clang AST or the Stor

[PATCH] D145069: [analyzer][NFC] Split the no state change logic and bug report suppression into two visitors

xazax.hun added a comment. Do you plan to selectively enable warnings coming from the STL to catch misuses of certain STL types? I think we should probably discuss this direction first. It is, in general, a fragile approach. Users might use Clang with GCC's, LLVM's, or MSVC's standard library.

[PATCH] D152263: [clang][CFG] Add support for partitioning CFG into intervals.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:35 + + std::set Blocks; + Nit: I wonder if we want something like `llvm::DenseSet` when we use smaller types like pointers. Same for `Successors`. =

[PATCH] D153491: [dataflow] Avoid copying environment

xazax.hun added a comment. This sounds extremely error-prone to me. In case copying the analysis state has side effects like this, I would argue we want such operations to be really explicit. What do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D152263: [clang][CFG] Add support for partitioning CFG into intervals.

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/IntervalPartition.cpp:26 + + std::queue Worklist; + for (const CFGBlock *S : Header.succs()) ymandel wrote: > xaza

[PATCH] D153476: [dataflow] document & test determinism of formula structure

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/Arena.cpp:13 +// Compute a canonical key for an unordered poir of operands, so that we +// can intern (A & B) and (B

[PATCH] D153488: [dataflow] HTMLLogger: meaningful names for flow condition variables

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Should we have some documentation or tooltip explaining the users what the meaning of those names are? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D153584: [dataflow] Make SAT solver deterministic

xazax.hun added a comment. Is there a measurable perf cost for this determinism? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153584/new/ https://reviews.llvm.org/D153584 ___ cfe-commits mailing list cf

[PATCH] D153491: [dataflow] Avoid copying environment

xazax.hun added a comment. In D153491#4445051 , @ymandel wrote: > In D153491#4443704 , @xazax.hun > wrote: > >> This sounds extremely error-prone to me. In case copying the analysis state >> has side effects lik

[PATCH] D153674: [dataflow] Disallow implicit copy of Environment, use fork() instead

xazax.hun accepted this revision. xazax.hun added a comment. Nice, looks like this change did catch some unintentional copies! Already paying dividends :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153674/new/ https://reviews.llvm.org/D153674

[PATCH] D152263: [clang][CFG] Add support for partitioning CFG into intervals.

xazax.hun accepted this revision. xazax.hun added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152263/new/ https://reviews.llvm.org/D152263 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D153058: [clang][CFG] Support construction of a weak topological ordering of the CFG.

xazax.hun added a comment. Could you rebase this patch to the latest tip of tree? Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:73 + + // Whether this node is the head of a feedback edge within the interval. + bool IsFeedbackHead = false; -

[PATCH] D153058: [clang][CFG] Support construction of a weak topological ordering of the CFG.

xazax.hun added inline comments. Comment at: clang/unittests/Analysis/IntervalPartitionTest.cpp:21 +namespace { +template using NodeData = CFGInterval::NodeData; +} // namespace Another redundant anonymous namespace here. Repository: rG LLVM Github Monorep

[PATCH] D153058: [clang][CFG] Support construction of a weak topological ordering of the CFG.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/IntervalPartition.cpp:121 +Index.emplace(N, ID); + Graph.Intervals.emplace_back(ID, Header, std::move(Data.Nodes)); } It would probably take for me some time to understand what is going on here

[PATCH] D147326: [clang][dataflow][NFC] Share code between Environment ctor and pushCallInternal().

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:465 + /// referenced in `FuncDecl`. `FuncDecl` must have a body. + void initVars(const FunctionDecl *FuncDecl); I wonder if we should rename this to

[PATCH] D147302: [clang][dataflow] Add `create()` methods to `Environment` and `DataflowAnalysisContext`.

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:100 +// used `StorageLocation` subclasses and make them use a `BumpPtrAllocat

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:77 + +void escape(char C, llvm::raw_ostream &OS) { + switch (C) { We already sort of have a way to escape HTML here: https://github.com/llvm/llvm-project/blob/132f1d31f

[PATCH] D148344: [clang][dataflow] Refine matching of optional types to anchor at top level.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:262 /// Returns true if and only if `Type` is an optional type. bool isOptionalType(QualType Type) { if (!Type->isRecordType()) Why do we n

[PATCH] D148344: [clang][dataflow] Refine matching of optional types to anchor at top level.

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/D148344/new/ https://reviews.llvm.org/D148344 __

[PATCH] D151183: [clang][dataflow] Add a `ControlFlowContext::build()` overload taking a `FunctionDecl`.

xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Comment at: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h:39 + /// Builds a ControlFlowContext from an AST

[PATCH] D151201: [clang][dataflow] Fix a crash in `getLogicOperatorSubExprValue()`.

xazax.hun added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:5064 +bool target() { + return true || false || false || false; +} Should we also test that the value of the expression is `true` in the analysis st

[PATCH] D151194: [clang][dataflow] Add support for return values of reference type.

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:196 + void popCall(const CallExpr *Call, const Environment &CalleeEnv); + void popCall(const CXXConstructExpr *Call, const Environment

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

xazax.hun added a comment. I generally support the idea of a CXXLifetimeExtendedObj region, definitely cleaner than a CXXTempObjectRegion with static lifetime. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151325/new/ https://reviews.llvm.org/D151

[PATCH] D151194: [clang][dataflow] Add support for return values of reference type.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:196 + void popCall(const CallExpr *Call, const Environment &CalleeEnv); + void popCall(const CXXConstructExpr *Call, const Environment &CalleeEnv); m

[PATCH] D151818: [clang][analyzer][NFC] Use the operator new directly with the `BumpPtrAllocator`

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

[PATCH] D143750: [clang-tidy] Clarify documention of `bugprone-unchecked-optional-access`.

xazax.hun added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst:278 + +Given that ``value()`` has well-defined program termination behavior, why treat +it the same as ``operator*()`` which causes undefined behavior (UB

[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

xazax.hun added a comment. I am ok with committing this to unblock people hitting this assert, but at the same time I wonder if we want to open a ticket on GitHub that we might want to rethink how some of this works. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D135495: [clang-tidy] handle exceptions properly `ExceptionAnalyzer`

xazax.hun added a comment. Overall, I like the structure of this patch and the references back to the standard. But every time we compare type pointers, they might compare inequal when one of the types have sugar on it while the other does not. Please review all of those comparisons to see wher

[PATCH] D135495: [clang-tidy] handle exceptions properly `ExceptionAnalyzer`

xazax.hun accepted this revision. xazax.hun added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135495/new/ https://reviews.llvm.org/D135495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D135495: [clang-tidy] handle exceptions properly in `ExceptionAnalyzer`

xazax.hun added a comment. I think at this point it is ok to merge. Any other comments can be addressed in follow-up commits. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135495/new/ https://reviews.llvm.org/D135495 ___ cfe-commits mailing

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329 +static void +builtinTransferScopeEnd(const CFGScopeEnd &Elt, +TypeErasedDataflowAnalysisState &InputState) {

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:329 +static void +builtinTransferScopeEnd(const CFGScopeEnd &Elt, +TypeErasedDataflowAnalysisState &InputState) { mboehme wrote: >

[PATCH] D150656: [clang][dataflow] Use `Strict` accessors in TypeErasedDataflowAnalysis.cpp.

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:303 - auto *InitStmtLoc = Env.getStorageLocation(*InitStmt, SkipPast::Reference); - if (InitStmtLoc == nullptr) -return; - - aut

[PATCH] D150657: [clang][dataflow] Use `Strict` accessors in SignAnalysisTest.cpp.

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp:134 LatticeTransferState &State) { - StorageLocation *Loc = State.Env.getStorageLocation(*BO, SkipPast::None); - if (!

[PATCH] D150655: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:717-723 +Value *SubExprVal = Env.getValueStrict(*SubExpr); +if (SubExprVal == nullptr) return; -Env.setStorageLocation(*S, *SubExprLoc

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

xazax.hun accepted this revision. xazax.hun added a comment. I am a bit overloaded at the moment, feel free to commit. I can still add comments later that could be addressed in a follow up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/

[PATCH] D149464: [clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:189 + /// Deprecated. Use *getDataflowAnalysisContext().getOptions().Log instead. Logger &logger() const { return *DACtx->getOptio

[PATCH] D149464: [clang][dataflow] Expose DataflowAnalysisContext from DataflowEnvironment.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:189 + /// Deprecated. Use *getDataflowAnalysisContext().getOptions().Log instead. Logger &logger() const { return *DACtx->getOptions().Log; } bazuz

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

xazax.hun added a subscriber: dcoughlin. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:616 + bool erased = DeclToLoc.erase(&D); + if (!erased) +D.dump(); Would we dump this in release builds? Do we wan

[PATCH] D152263: [clang][CFG] Add support for partitioning CFG into intervals.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/Analyses/IntervalPartition.h:22 + +struct CFGInterval { + CFGInterval(const CFGBlock *Header) : Header(Header), Blocks({Header}) {} A concise definition of what is an interval with a refer

[PATCH] D152732: [clang][dataflow] Support limits on the SAT solver to force timeouts.

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Huge +1, I think most solvers need to have some resource limits in place as the runtime can explode. I am just not 100% what is the best approach here, putting a global limit on the solv

[PATCH] D152732: [clang][dataflow] Support limits on the SAT solver to force timeouts.

xazax.hun added a comment. In D152732#4414707 , @ymandel wrote: > Ultimately what matters for a user is the global limit. I am not 100% sure about that. While it is true that the user cares about the process not hanging, but global vs local limits can h

[PATCH] D152813: [clang][dataflow] Create `Value`s for integer literals.

xazax.hun added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:89 + /// `Value`. These literals are the same every time. + IntegerValue &makeIntLiteral(llvm::APInt Value); + gribozavr2 wrote: > Should we be taking the type into

[PATCH] D153006: [clang][dataflow] Perform deep copies in copy and move operations.

xazax.hun added a comment. I am not opposed to this solution, but I do anticipate some performance problems in the future. I wonder if copy-on-write, or some persistent data structures where copying is cheap would be a better strategy long term. But getting it right first and fast after sounds

[PATCH] D153006: [clang][dataflow] Perform deep copies in copy and move operations.

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/RecordOps.cpp:56-57 + + llvm::iterator_range< + llvm::DenseMap::const_iterator> + DstChildren = DstVal->children(); I know we usually like to mention types at least once, but

[PATCH] D139360: [clang][dataflow] Support (in)equality operators in `optional` model.

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

[PATCH] D137379: [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

xazax.hun added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:168-169 + static Matcher matcher() { +// FIXME: What if the index is integer literal 0? Should this be +// a safe gadget in this case? +return stmt( NoQ wrote: > aa

[PATCH] D139544: [clang][dataflow] Add support for structured bindings of tuple-like types.

xazax.hun added a comment. This approach looks good to me. Some context: we kept the CFGs lightweight because it looks like we did not need to do any extensions for the Clang Static Analyzer. I'm glad the dataflow framework can also work with the current representation. On the other hand, I thi

[PATCH] D137348: [-Wunsafe-buffer-usage] Introduce an abstraction for fixable code patterns.

xazax.hun added a comment. I don't mind committing these patches, I have a high confidence in you going back and addressing feedback post-commit if something is coming up. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137348/new/ https://reviews.llvm.org/D137348 __

[PATCH] D27710: [analyzer] Prohibit ExplodedGraph's edges duplicating

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/CoreEngine.cpp:662 + bool EdgeExists = false; + for (auto I = N->pred_begin(), E = N->pred_end(); I != E; ++I) +if (*I == FromN) { I prefer having the braces written in this case, but it i

[PATCH] D21298: [Clang-tidy] delete null check

xazax.hun added a comment. Small ping, is this ready to be committed? https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds

xazax.hun added a comment. Did you experience any problems with the array out of bounds check lately? In case it was stable on large code-bases and did not give too many false positives, I think it might be worth to move that check out of alpha at the same time, so users who do not turn on alph

[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width

xazax.hun added a comment. So there is no LLVM supported target with different bit width pointer types? In case we have such a target upstreamed, you can add a test with the host-triple hardcoded into the test. Repository: rL LLVM https://reviews.llvm.org/D31029 _

[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

xazax.hun added a comment. I wonder whether warning on implicit casts still makes sense for example in mission critical code. So maybe it is worth to have a configuration option with the default setting being less strict and chatty. What do you think? Repository: rL LLVM https://reviews.llv

[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls

xazax.hun added a comment. In https://reviews.llvm.org/D31097#704626, @alexfh wrote: > In https://reviews.llvm.org/D31097#704621, @xazax.hun wrote: > > > I wonder whether warning on implicit casts still makes sense for example in > > mission critical code. So maybe it is worth to have a configur

[PATCH] D30489: [analyzer] catch out of bounds for VLA

xazax.hun added a comment. In https://reviews.llvm.org/D30489#689947, @zaks.anna wrote: > Should we just remove ArrayBoundChecker.cpp or is there a value in keeping it > around? I think once ArrayBoundCheckerV2 is in production we should only keep ArrayBoundChecker there as educational purpus

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:60 +// Set timeout to 15000ms = 15s +Z3_set_param_value(Config, "timeout", "15000"); + } Sorry for being a bit late in the party, I was wondering whether this ti

[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

xazax.hun added inline comments. Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126 +} +diag(Ctor->getLocation(), "function %0 can hide copy and move constructors") +<< Ctor; + } aaron.ballman wrote: > aaron.ballman wrote:

[PATCH] D30547: [clang-tidy] Forwarding reference overload in constructors

xazax.hun added inline comments. Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126 +} +diag(Ctor->getLocation(), "function %0 can hide copy and move constructors") +<< Ctor; + } aaron.ballman wrote: > xazax.hun wrote: > >

[PATCH] D19979: [analyzer] ScopeContext - initial implementation

xazax.hun added a comment. What is the status of this? Aleksei, could you upload a new patch with the context available? (And also with a testcase added for jumps/gotos and VLA.) You modified the malloc checker but I did not see a test for that. https://reviews.llvm.org/D19979 __

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun updated this revision to Diff 93658. xazax.hun marked 11 inline comments as done. xazax.hun added a comment. - Fixed some of the review comments and some other cleanups. https://reviews.llvm.org/D30691 Files: include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/clang/

[PATCH] D31886: [analyzer] Simplify values in binary operations more aggressively

xazax.hun added a comment. This is a very welcome addition. I hope the performance will be still good :) Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1023 + +SVal VisitSymIntExpr(const SymIntExpr *S) { + SValBuilder &SVB = State->getStateManager().getSValB

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun updated this revision to Diff 94709. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Fixed some memory leaks. - Removed some unrelated style changes. - Simplifications to the python scripts. - Removed debug prints. https://reviews.llvm.org/D30691 Files: i

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun marked 6 inline comments as done. xazax.hun added inline comments. Comment at: test/Analysis/Inputs/externalFnMap.txt:1 +_Z7h_chaini@x86_64 xtu-chain.cpp.ast +_ZN4chns4chf2Ei@x86_64 xtu-chain.cpp.ast NoQ wrote: > These tests use a pre-made external func

[PATCH] D31887: [clangd] Add documentation page

xazax.hun added a comment. Maybe this is a good time to decide ClangD, Clangd, or clangd is the correct capitalization. Repository: rL LLVM https://reviews.llvm.org/D31887 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

xazax.hun updated this revision to Diff 94814. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Removed a clang tool, replaced with python tool functionality. https://reviews.llvm.org/D30691 Files: include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/

[PATCH] D31886: [analyzer] Simplify values in binary operations more aggressively

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! Thank you for working on this! https://reviews.llvm.org/D31886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

<    7   8   9   10   11   12   13   14   15   >