[PATCH] D67149: Fix argument order for BugType instation for

MTC added a comment. Good catch, the fix is reasonable to me. However, I'm no longer wok on the analyzer now, maybe you can add @NoQ Or some other active reviewers to review the code. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67149/new/ https://reviews.llv

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

MTC added a comment. In D55388#1322601 , @xazax.hun wrote: > Hm. I wonder if it would also make sense to model e.g. the get method to > return nullptr for moved from smart ptrs. This could help null dereference > checker and also aid false path prunning.

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

MTC accepted this revision. MTC added inline comments. This revision is now accepted and ready to land. Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:90 // in a valid manner. // TODO: We can still try to identify *unsafe* use after move, such as // dereference

[PATCH] D55226: [Fix][StaticAnalyzer] Bug 39792 - False positive on strcpy targeting struct member

MTC added a comment. Please add more context using the `-U` option, like `git diff -U9 ...`. In D55226#1317119 , @Pierre-vh wrote: > Sadly I'm not experienced enough to think of every case that should pass this > check, so I limited myself to just fi

[PATCH] D54878: [clangd] NFC: Prefer `isa<>` to `dyn_cast<>` to do the checking.

MTC added a comment. In D54878#1307726 , @ilya-biryukov wrote: > But given that `isa<>` are still better than `dyn_cast<>`, this change might > still be worth landing. We can land this change this time or do the cleaning job in other patches in the fu

[PATCH] D54878: [clangd] NFC: Eliminate the unused variable warning.

MTC updated this revision to Diff 175232. MTC added a comment. Use more concise form. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54878/new/ https://reviews.llvm.org/D54878 Files: clangd/AST.cpp Index: clangd/AST.cpp =

[PATCH] D54878: [clangd] NFC: Eliminate the unused variable warning.

MTC marked an inline comment as done. MTC added inline comments. Comment at: clangd/AST.cpp:98 // The name was empty, so present an anonymous entity. - if (auto *NS = llvm::dyn_cast(&ND)) + if (isa(&ND)) return "(anonymous namespace)"; MaskRay wrote: >

[PATCH] D54878: [clangd] NFC: Eliminate the unused variable warning.

MTC created this revision. MTC added a reviewer: ilya-biryukov. Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, ioeric. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54878 Files: clangd/AST.cpp Index: clangd/AST.cpp =

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:342 + DefaultBool IsOptimistic; + MemFunctionInfoTy MemFunctionInfo; public: Szelethus wrote: > Szelethus wrote: > > Szelethus wrote: > > > MTC wrote: > > > > I can't say that

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

MTC added a comment. Thank you for doing the cleaning that no one else is interested in! Comments is inline below. Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:342 + DefaultBool IsOptimistic; + MemFunctionInfoTy MemFunctionInfo; public: I can't

[PATCH] D54560: [analyzer] MoveChecker Pt.3: Improve warning messages a bit.

MTC added a comment. > The "moved-from" terminology we adopt here still feels a bit weird to me, but > i don't have a better suggestion, so i just removed the single-quotes so that > to at least feel proud about what we have. I am personally fine with this terminology, this checker corresponds

[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp

MTC added a comment. I'm totally fine with this patch personally. However I am not familiar with this part, so can't give substantial help :). Comment at: lib/StaticAnalyzer/Core/CheckerRegistry.cpp:51 -/// Collects the checkers for the supplied \p opt option into \p collect

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

MTC added a comment. Sorry for the long delay for this patch! The implementation is fine for me. However, I'm the newbie on clang diagnostics and have no further insight into this checker. @aaron.ballman may have more valuable insights into this checker. Comment at: test/Sema

[PATCH] D53856: [analyzer] Put llvm.Conventions back in alpha

MTC added a comment. In addition, `clang/lib/StaticAnalyzer/README.txt` and other related docs in `clang/lib/docs/analyzer` are also out of date. Comment at: www/analyzer/alpha_checks.html:570 + + A StringRef should not be bound to a temporary std::string + whose lifetime i

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

MTC added inline comments. Comment at: test/Analysis/NewDelete-sized-deallocation.cpp:1 +// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s +// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -anal

[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div

MTC added a comment. In https://reviews.llvm.org/D52949#1268640, @xbolva00 wrote: > Second thought, I don't think we should recommend std::size here (maybe it > should be okay for clang static analyzers) > > uint32_t data[] = {10, 20, 30, 40}; > len = sizeof(data)/sizeof(*data); // "warn" on va

[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

MTC added inline comments. Comment at: test/Analysis/NewDelete-custom.cpp:7 -#if !(LEAKS && !ALLOCATOR_INLINING) // expected-no-diagnostics Should we continue to keep this line? Comment at: test/Analysis/NewDelete-sized-deallocation.cpp:1

[PATCH] D53024: [analyzer][www] Add more open projects

MTC added subscribers: teemperor, baloghadamsoftware, blitz.opensource. MTC added a comment. Herald added a subscriber: donat.nagy. In https://reviews.llvm.org/D53024#1258976, @Szelethus wrote: > Also, a lot of items on this list is outdated, but I joined the project > relatively recently, so I'

[PATCH] D52984: [analyzer] Checker reviewer's checklist

MTC added a comment. Greate idea! If we can enrich this list, it will not only help the reviewer, but also help beginners, like me, avoid some pitfalls when developing a new checker. I agree with NoQ to classify the lists. In addition to the two categories proposed by NoQ, there is another cla

[PATCH] D51390: [analyzer] CallDescription: Improve array management.

MTC accepted this revision. MTC added a comment. Thank you for digging in to delete that meaningless constructor, NoQ! And sorry for my technology debt : ). Repository: rC Clang https://reviews.llvm.org/D51390 ___ cfe-commits mailing list cfe-co

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. In https://reviews.llvm.org/D48027#1209844, @NoQ wrote: > So i believe that one of the important remaining problems with > `CallDescription` is to teach it to discriminate between global functions and > methods. We can do it in a number of ways: > > 1. Make a special sub-cl

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. In https://reviews.llvm.org/D48027#1207645, @xazax.hun wrote: > Sorry for the delay, I think this is OK to commit. > As a possible improvement, I can imagine making it slightly stricter, e.g. > you could only skip QualifiedNames for inline namespaces and the beginning. > M

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. @xazax.hun What do you think? https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. In https://reviews.llvm.org/D48027#1201248, @xazax.hun wrote: > Generally looks good, I only wonder if this works well with inline > namespaces. Could you test? Probably it will. Thank you for reminding me! Yea, this can handle inline namespaces. However this approach has

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC updated this revision to Diff 161217. MTC added a comment. - rebase - Since we have enhanced the ability of `CallDescription`, remove the helper method `isCalledOnStringObject()`. https://reviews.llvm.org/D48027 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h lib/Sta

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. kindly ping! Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50382: [analyzer] Fix a typo in `RegionStore.txt`.

MTC created this revision. MTC added reviewers: NoQ, george.karpenkov. Herald added subscribers: mikhail.ramalho, a.sidorin, szepet, xazax.hun. The typo of the description for default bindings can be confusing. Repository: rC Clang https://reviews.llvm.org/D50382 Files: docs/analyzer/Regio

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC updated this revision to Diff 157499. MTC added a comment. @xazax.hun Thanks for your tips! After some investigation, `MatchFinder::match` just traverse one ASTNode, that means `match(namedDecl(HasNameMatcher()))` and `match(namedDecl(matchesName()))` both not traverse children. So there are

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. Thanks for your review, NoQ! Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68 : IILockGuard(nullptr), IIUniqueLock(nullptr), - LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"), - FgetsFn("fgets"

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. kindly ping! Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC updated this revision to Diff 152702. MTC added a comment. Sorry for the long long delay, I was on the Dragon Boat Festival a few days ago. This update has two parts: - Use the `matchesName` to match the AST node with the specified name, `matchesName` use regex to match the specified name.

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32 check::PostCall> { - CallDescription CStrFn; + const llvm::SmallVector CStrFnFamily = { +{"std::basic_string::c_str"

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC updated this revision to Diff 151166. MTC added a comment. - Use `hasName` matcher to match the qualified name. - Use the full name, like `std::basic_string::c_str` instead of `c_str` to match the `c_str` method in `DanglingInternalBufferChecker.cpp`. Repository: rC Clang https://revie

[PATCH] D47616: [CFG] [analyzer] Explain copy elision through construction contexts.

MTC added inline comments. Herald added a subscriber: mikhail.ramalho. Comment at: include/clang/Analysis/ConstructionContext.h:351 + + explicit SimpleTemporaryObjectConstructionContext( + const CXXBindTemporaryExpr *BTE, const MaterializeTemporaryExpr *MTE) ---

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. In https://reviews.llvm.org/D48027#1128301, @xazax.hun wrote: > Having C++ support would be awesome! > Thanks for working on this! > > While I do agree matching is not trivial with qualified names, this problem > is already solved with AST matchers. > > I wonder if using ma

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC updated this revision to Diff 150758. MTC added a comment. Remove useless header files for testing. Repository: rC Clang https://reviews.llvm.org/D48027 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h lib/StaticAnalyzer/Core/CallEvent.cpp Index: lib/StaticAnalyze

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC added a comment. The implementation is not complicated, the difficulty is that there is no good way to get the qualified name without template arguments. For 'std::basic_string::c_str()', its qualified name may be `std::__1::basic_string, std::__1::allocator >::c_str`, it is almost impossi

[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.

MTC created this revision. MTC added reviewers: xazax.hun, NoQ, george.karpenkov. Herald added subscribers: mikhail.ramalho, a.sidorin, rnkovacs, szepet. `CallDecription` can only handle function for the time being. If we want to match c++ method, we can only use method name to match and can't im

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

MTC added a comment. LGTM, @NoQ May have further feedback. Thanks! Repository: rC Clang https://reviews.llvm.org/D47044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

MTC added a comment. I didn't test the code, but the code seems correct. Thanks! Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:88 + MatchFinder Finder; + Finder.addMatcher(varDecl(hasType(referenceType())).bind("match"), new Callback(LCtx, MRMgr, ITraits)); + Finder.

[PATCH] D47658: [analyzer] Re-enable lifetime extension for temporaries with destructors and bring back static temporaries.

MTC added inline comments. Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:234 + SVal V = UnknownVal(); + if (MTE) { +if (MTE->getStorageDuration() != SD_FullExpression) { An unrelated question. I want to know, what considerations are you

[PATCH] D47616: [CFG] [analyzer] Explain copy elision through construction contexts.

MTC added inline comments. Comment at: include/clang/Analysis/ConstructionContext.h:351 + + explicit SimpleTemporaryObjectConstructionContext( + const CXXBindTemporaryExpr *BTE, const MaterializeTemporaryExpr *MTE) `explicit` useless? Com

[PATCH] D47451: [analyzer] Remove the redundant check about same state transition in `ArrayBoundCheckerV2.cpp`.

MTC created this revision. MTC added reviewers: NoQ, xazax.hun, george.karpenkov. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. Since the `addTransitionImpl()` has a check about same state transition, there is no need to check it in `ArrayBoundCheckerV2.cpp`. Repository:

[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:399 + + C.addTransition(State); } NoQ wrote: > MTC wrote: > > I have two questions may need @NoQ or @xazax.hun who is more familiar with > > the analyzer engine help to answ

[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:399 + + C.addTransition(State); } I have two questions may need @NoQ or @xazax.hun who is more familiar with the analyzer engine help to answer. - `State` may not change

[PATCH] D47135: [analyzer][WIP] A checker for dangling string pointers in C++

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:59 + QualType RegType = TypedR->getValueType(); + if (RegType.getAsString() != "std::string") +return; A little tip, there are other string types besides `st

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 146816. MTC added a comment. - According to NoQ's suggestion, use `assumeZero()` instead of `isZeroConstant()` to determine whether the value is 0. - Add test `memset26_upper_UCHAR_MAX()` and `memset27_symbol()` - Since `void *memset( void *dest, int ch, size_t c

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1037 + +if (StateWholeReg && !StateNotWholeReg && CharVal.isZeroConstant()) { + // If the 'memset()' acts on the whole region of destination buffer and NoQ wrote: > I t

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. ping. Repository: rC Clang https://reviews.llvm.org/D44934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 145361. MTC added a comment. - Since there is no perfect way to handle the default binding of non-zero character, remove the default binding of non-zero character. Use `bindDefaulrZero()` instead of `overwriteRegion()` to bind the zero character. - Reuse `assume

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. In https://reviews.llvm.org/D44934#1088771, @NoQ wrote: > Hmm, ok, it seems that i've just changed the API in > https://reviews.llvm.org/D46368, and i should have thought about this use > case. Well, at least i have some background understanding of these problems > now. So

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 145019. MTC added a comment. - fix typos - code refactoring, add auxiliary method `memsetAux()` - according to a.sidorin's suggestions, remove the useless state splitting. - make `StoreManager::overwriteRegion()` pure virtual Repository: rC Clang https://revi

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. Sorry for the long delay, I have just finished my holiday. Thanks a lot for the review, I will fix the typo in next update. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2100 +std::tie(StateNullChar, StateNonNullChar) = +assumeZero

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. ping^2 Repository: rC Clang https://reviews.llvm.org/D44934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46007: [analyzer] Add `TaintBugVisitor` to the ArrayBoundV2, DivideZero and VLASize.

MTC updated this revision to Diff 143908. MTC marked an inline comment as done. MTC added a comment. Since `BugReport::addVisitor()` has checks for the null `Visitor`, remove the checks before `BugReport->addVisitor()`. Repository: rC Clang https://reviews.llvm.org/D46007 Files: lib/Stati

[PATCH] D46007: [analyzer] Add `TaintBugVisitor` to the ArrayBoundV2, DivideZero and VLASize.

MTC marked an inline comment as done. MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp:75 auto report = llvm::make_unique(*BT, os.str(), N); + report->addVisitor(std::move(Visitor)); report->addRange(SizeE->getSourceRange()); --

[PATCH] D46007: [analyzer] Add `TaintBugVisitor` to the ArrayBoundV2, DivideZero and VLASize.

MTC created this revision. MTC added reviewers: NoQ, george.karpenkov, xazax.hun, a.sidorin. Herald added subscribers: cfe-commits, rnkovacs, szepet. Add `TaintBugVisitor` to the ArrayBoundV2, DivideZero, VLASize to be able to indicate where the taint information originated from. Repository:

[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

MTC added a comment. In https://reviews.llvm.org/D45682#1074407, @george.karpenkov wrote: > I'm new to the taint visitor, but I am quite confused by your change > description. > > > and many checkers rely on it > > How can other checkers rely on it if it's private to the taint checker? Thanks

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

MTC added a comment. Test files for `initialization` missing? : ) Repository: rC Clang https://reviews.llvm.org/D45774 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

MTC added a comment. In https://reviews.llvm.org/D45532#1069683, @whisperity wrote: > There is something that came up in my mind: > > Consider a construct like this: > > class A > { > A() > { > memset(X, 0, 10 * sizeof(int)); > } > > int X[10]; > }; > > > I think it'

[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

MTC created this revision. MTC added reviewers: NoQ, george.karpenkov, xazax.hun. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. `TaintBugVisitor` is a universal visitor, and many checkers rely on it, such as `ArrayBoundCheckerV2.cpp`, `DivZeroChecker.cpp` and `VLASizeChecker

[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

MTC added a comment. In https://reviews.llvm.org/D45491#1067852, @NoQ wrote: > Yeah, i think this makes sense, thanks! It feels a bit weird that we have to > add it as an exception - i wonder if there are other exceptions that we need > to make. Widening over the stack memory space should be a

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

MTC added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:10 +// +// This file defines a checker that checks cunstructors for possibly +// uninitialized fields. typo :) `cunstructors` -> `constructors`

[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

MTC updated this revision to Diff 142026. MTC marked 2 inline comments as done. MTC added a comment. - Move the `CXXThisRegion`'s check to `LoopWidening.cpp` - Use `isa(R)` instead of `CXXThisRegion::classof(R)`. Repository: rC Clang https://reviews.llvm.org/D45491 Files: lib/StaticAnalyze

[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

MTC marked 2 inline comments as done. MTC added inline comments. Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1024-1027 + // 'this' pointer is not an lvalue, we should not invalidate it. + if (CXXThisRegion::classof(baseR)) +return; + NoQ wrote: > I

[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

MTC added a comment. In https://reviews.llvm.org/D45491#1063364, @george.karpenkov wrote: > @MTC what happens for > > this.j = 0; > for (int i=0; i<100; i++) > this.j++; > > > ? @george.karpenkov `this`'s value will remain unchanged, `j` will be invalidated. 1 void clang_analyz

[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

MTC created this revision. MTC added reviewers: NoQ, george.karpenkov, a.sidorin, seaneveson, szepet. Herald added subscribers: cfe-commits, rnkovacs, xazax.hun. MTC edited the summary of this revision. `this` pointer is not an l-value, although we have modeled `CXXThisRegion` for `this` pointer,

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. Kindly ping! Repository: rC Clang https://reviews.llvm.org/D44934 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 140629. MTC added a comment. > Thank you for your reminding, I overlooked this point. However for > non-concrete character, the symbol value, if we just invalidate the region, > the constraint information of the non-concrete character will be lost. Do we > need

[PATCH] D45086: [analyzer] Unroll the loop when it has a unsigned counter.

MTC updated this revision to Diff 140524. MTC added a comment. Fix typo, `unsinged` -> `unsigned` Repository: rC Clang https://reviews.llvm.org/D45086 Files: lib/StaticAnalyzer/Core/LoopUnrolling.cpp test/Analysis/loop-unrolling.cpp Index: test/Analysis/loop-unrolling.cpp =

[PATCH] D45086: [analyzer] Unroll the loop when it has a unsigned counter.

MTC created this revision. MTC added reviewers: szepet, a.sidorin, NoQ. Herald added subscribers: cfe-commits, rnkovacs, xazax.hun. Herald added a reviewer: george.karpenkov. MTC edited the summary of this revision. The original implementation in the `LoopUnrolling.cpp` didn't consider the case w

[PATCH] D45081: [analyzer] Remove the unused method declaration in `ValistChecker.cpp`.

MTC created this revision. MTC added a reviewer: xazax.hun. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. `getVariableNameFromRegion()` seems useless. Repository: rC Clang https://reviews.llvm.org/D45081 Files: lib/StaticAnal

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC updated this revision to Diff 140405. MTC added a comment. According to @NoQ's suggestion, remove the duplicated code. Repository: rC Clang https://reviews.llvm.org/D44934 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h include/clang/StaticAnalyzer/Core/PathSens

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC added a comment. Thanks for your review, NoQ! > Why do you need separate code for null and non-null character? The function's > semantics doesn't seem to care. Thank you for your advice, I will remove the duplicate code！ > I'd rather consider the case of non-concrete character separately.

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

MTC created this revision. MTC added reviewers: dcoughlin, NoQ, xazax.hun, a.sidorin. Herald added subscribers: cfe-commits, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. This patch originates from https://reviews.llvm.org/D31868. There are two key points in this patch: - Add `Ov

[PATCH] D34260: [StaticAnalyzer] Completely unrolling specific loops with known bound option

MTC added inline comments. Herald added subscribers: dkrupp, rnkovacs. Herald added a reviewer: george.karpenkov. Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:107 + equalsBoundNode("initVarName"), +

[PATCH] D44606: [analyzer] Fix the crash in `IteratorChecker.cpp` when `SymbolConjured` has a null Stmt.

MTC updated this revision to Diff 139075. MTC added a comment. Add the comments as suggested by @szepet . Repository: rC Clang https://reviews.llvm.org/D44606 Files: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp test/Analysis/loop-widening.c Index: test/Analysis/loop-widening.c =

[PATCH] D44557: [analyzer] CStringChecker.cpp - Code refactoring on bug report.

MTC added a comment. In https://reviews.llvm.org/D44557#1042357, @NoQ wrote: > Sorry, one moment, i'm seeing a few regressions after the previous > refactoring but i didn't look at them closely yet to provide a reproducer. > I'll get back to this. Thank you for the code review, @NoQ ! The re

[PATCH] D44606: [analyzer] Fix the crash in `IteratorChecker.cpp` when `SymbolConjured` has a null Stmt.

MTC added a comment. > Just in case: we indeed do not guarantee that `SymbolConjured` corresponds to > a statement; it is, however, not intended, but rather a bug. Thank you for your explanation and the reasonable example, NoQ. Repository: rC Clang https://reviews.llvm.org/D44606 ___

[PATCH] D44606: [analyzer] Fix the crash in `IteratorChecker.cpp` when `SymbolConjured` has a null Stmt.

MTC added a comment. > One small nit for future debugging people: Could you insert a comment line in > the test case where you explain what is this all about? E.g what you just > have written in the description: "invalidateRegions() will construct the > SymbolConjured with null Stmt" or somethi

[PATCH] D44606: [analyzer] Fix the crash in `IteratorChecker.cpp` when `SymbolConjured` has a null Stmt.

MTC created this revision. MTC added reviewers: NoQ, baloghadamsoftware, szepet, dcoughlin. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, xazax.hun. Herald added a reviewer: george.karpenkov. When the loop has a null terminator statement and sets `widen-loops=true`, `invalidateRegio

[PATCH] D44557: [analyzer] CStringChecker.cpp - Code refactoring on bug report.

MTC created this revision. MTC added reviewers: NoQ, george.karpenkov, xazax.hun. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. When improving the modeling `evalMemset()`, some scenes need to emit report of `NotNullTerm`. In this case, there are three places in `CStringChec

[PATCH] D43741: [Analyzer] More accurate modeling about the increment operator of the operand with type bool.

MTC updated this revision to Diff 137154. MTC added a comment. Remove the default configuration `-analyzer-store=region` in the test file. Repository: rC Clang https://reviews.llvm.org/D43741 Files: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h lib/StaticAnalyzer/Co

[PATCH] D44075: [analyzer] CStringChecker.cpp: Remove the duplicated check about null dereference on dest-buffer or src-buffer.

MTC created this revision. MTC added reviewers: dcoughlin, NoQ, xazax.hun, cfe-commits. Herald added subscribers: a.sidorin, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. `CheckBufferAccess()` calls `CheckNonNull()`, so there are some calls to `CheckNonNull()` that are useless. R

[PATCH] D39159: [analyzer] Improves the logic of GenericTaintChecker identifying stdin.

MTC added a comment. @NoQ, Very sorry, I've forgotten about this patch, it has now been updated. Repository: rC Clang https://reviews.llvm.org/D39159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D39159: [analyzer] Improves the logic of GenericTaintChecker identifying stdin.

MTC updated this revision to Diff 136934. MTC set the repository for this revision to rC Clang. MTC added a comment. Herald added subscribers: cfe-commits, a.sidorin. Herald added a reviewer: george.karpenkov. Update the `taint-generic.c` to test both `stdin` declaration variants. Repository:

[PATCH] D43741: [Analyzer] More accurate modeling about the increment operator of the operand with type bool.

MTC added a comment. Thank you for your review, @NoQ! - `isBooleanType()` is used to check `_Bool` in C99/C11 and `bool` in C++. For `_Bool` , there is the same overflow problem. - In C++98/C++11/C++14, for `++bool` and `bool+`, both sets true directly. - In C++, `--bool` and `bool--` is illeag

[PATCH] D43741: [Analyzer] More accurate modeling about the increment operator of the operand with type bool.

MTC updated this revision to Diff 136901. MTC added a comment. - If the operand of the ++ operator is of type `_Bool`, also set to true. - Add test file `_Bool-increment-decement.c`. Repository: rC Clang https://reviews.llvm.org/D43741 Files: include/clang/StaticAnalyzer/Core/PathSensiti

[PATCH] D43741: [Analyzer] More accurate modeling about the increment operator of the operand with type bool.

MTC added a comment. In https://reviews.llvm.org/D43741#1024949, @alexshap wrote: > i see, but just in case - what about the decrement operator ? @alexshap, If I'm not wrong, decrement operation is not allowed on bool type in C++98 and C++14. > 5.2.6 Increment and decrement [expr.post.incr] >

[PATCH] D43741: [Analyzer] More accurate modeling about the increment operator of the operand with type bool.

MTC created this revision. MTC added reviewers: alexshap, NoQ, dcoughlin. Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun. Herald added a reviewer: george.karpenkov. There is a problem with analyzer that a wrong value is given when modeling the increment operator of the operan

[PATCH] D42300: [Analyzer] Add PreStmt and PostStmt callbacks for OffsetOfExpr

MTC added a comment. @NoQ Sorry to bother you again. It seems that this patch is useless to analyzer temporarily, if you think so, I will abandon it : ). Repository: rC Clang https://reviews.llvm.org/D42300 ___ cfe-commits mailing list cfe-commi

[PATCH] D42300: [Analyzer] Add PreStmt and PostStmt callbacks for OffsetOfExpr

MTC updated this revision to Diff 133421. MTC added a comment. Herald added a reviewer: george.karpenkov. rebase Repository: rC Clang https://reviews.llvm.org/D42300 Files: lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp test/Analysis/Inputs/

[PATCH] D43074: [Analyzer] Fix a typo about `categories::MemoryError` in `MallocChecker.cpp`

MTC created this revision. MTC added reviewers: NoQ, xazax.hun. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. Herald added a reviewer: george.karpenkov. It should be an omission when committing https://reviews.llvm.org/rL302016. Repository: rC Clang https://reviews.llvm.

[PATCH] D42785: [Analyzer] Fix a typo in `ExprEngine::VisitMemberExpr`

MTC added a comment. In https://reviews.llvm.org/D42785#995211, @NoQ wrote: > Ew. So it means that checker transitions are currently discarded. Great > catch. I guess we don't use this functionality yet, so we can't test it, but > the fix should definitely go in. You are right, that's why I d

[PATCH] D42785: [Analyzer] Fix a typo in `ExprEngine::VisitMemberExpr`

MTC created this revision. MTC added reviewers: NoQ, dcoughlin. Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun. Herald added a reviewer: george.karpenkov. MTC edited the summary of this revision. `VisitCommonDeclRefExpr()` should accept the ExplodedNode in `CheckedSet` as an

[PATCH] D42300: [Analyzer] Add PreStmt and PostStmt callbacks for OffsetOfExpr

MTC added a comment. In https://reviews.llvm.org/D42300#982187, @NoQ wrote: > My intuition suggests that this checker shouldn't be path-sensitive; our > path-sensitive analysis does very little to help you with this particular > checker, and you might end up with a much easier and more reliable

[PATCH] D42300: [Analyzer] Add PreStmt and PostStmt callbacks for OffsetOfExpr

MTC updated this revision to Diff 130753. MTC added a comment. - Use C++11 range-based for loop to traverse ExplodedNodeSet. - Define the macro `offsetof` in `system-header-simulator.h`. Repository: rC Clang https://reviews.llvm.org/D42300 Files: lib/StaticAnalyzer/Checkers/AnalysisOrderCh

[PATCH] D42300: [Analyzer] Add PreStmt and PostStmt callbacks for OffsetOfExpr

MTC created this revision. MTC added reviewers: NoQ, a.sidorin, dcoughlin. Herald added subscribers: cfe-commits, szepet, xazax.hun. PreStmt and PostStmt callbacks for OffsetOfExpr are necessary to implement `Cert ARR39-C: Do not add or subtract a scaled integer to a pointer`. And should I defin

[PATCH] D37189: Fix an assertion failure that occured when custom 'operator new[]' return non-ElementRegion and 'c++-allocator-inlining' sets true.

MTC added a comment. In https://reviews.llvm.org/D37189#979795, @NoQ wrote: > Oh well, i guess i covered this in my recent patches anyway (esp. > r322787/https://reviews.llvm.org/D41406). Sorry, i just fixed everything > differently and it became unclear how to integrate your patch into the who

[PATCH] D42106: [analyzer] Remove the useless method declararion 'BugReporter::RemoveUnneededCalls()'.

MTC created this revision. MTC added reviewers: NoQ, dcoughlin, xazax.hun. Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet. MTC retitled this revision from "[analyzer] Remove the useless method declararion 'BugReporter::RemoveUnneedCalls()'." to "[analyzer] Remove the useless m

  1   2   >