[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

steakhal added a comment. Looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120824/new/ https://reviews.llvm.org/D120824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

steakhal added a comment. I would recommend committing changes in the morning, to give some time for the bots to chew through your commit. This way you could react to breakages and revert if needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

steakhal added a comment. In addition to the excellent summary, I'd like to highlight that this is intended to catch only the cases where the use of the constrained pointer is in the very same stack frame where it was constrained. This leads to a really nice property: local reasoning, which gre

[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

steakhal added a comment. Looks great. I'm loving it! BTW what is the semantics of `[p retain]` in ObjC? Can `p` be null in that context? Or does it count as a dereferences, hence it constraints the pointer? Why did you touch the `AnalysisOrderChecker`, should we separate those changes? Additio

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

steakhal added a comment. Land it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119601/new/ https://reviews.llvm.org/D119601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

[PATCH] D121045: [analyzer][NFC] Merge similar conditional paths

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I would probably query the opcode only once and reuse it, but is also fine. Btw whats your intention making this change? Do you plan greater refactorings/cleanups? Repository: rG LLVM

[PATCH] D121045: [analyzer][NFC] Merge similar conditional paths

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. Shadowing potential names is not a good idea. BTW OpCodes are often abbreviated to `Op` in variable names in this context. Comment at: clang/lib/StaticAnalyzer/

[PATCH] D120992: [analyzer] ReverseNull: New checker to warn for pointer value conditions, if the pointer value is unconditionally non-null

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullPtrInterferenceChecker.cpp:166 +/// child is a sink node. +static bool unconditionallyLeadsHere(const ExplodedNode *N) { + size_t NonSinkNodeCount = llvm::count_if( xazax.hun wrote

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

steakhal created this revision. steakhal added reviewers: whisperity, aaron.ballman. Herald added subscribers: jeroen.dobbelaere, martong, rnkovacs, xazax.hun. Herald added a project: All. steakhal requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscrib

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2715097 , @OikawaKirie wrote: > In D83660#2675064 , @mikhail.ramalho > wrote: > >> Indeed it looks like a copy & paste error, I'm surprised no one found it >> earlier. >> >> Reg

[PATCH] D101041: [analyzer] Find better description for tracked symbolic values

steakhal added a comment. This is an improvement! Good job. I had no time reviewing this, but I think it's already in a pretty good shape. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101041/new/ https://reviews.llvm.org/D101041 _

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:836-837 + NewState, NewNode, + C.getNoteTag([Msg](PathSensitiveBugReport &BR, + llvm::raw_ostream &OS) { OS << Msg; }));

[PATCH] D101635: [analyzer] Fix assertion in SVals.h

steakhal added a comment. I don't know how did we miss this. I run your patch on several projects and it seemed good. Does anyone have an idea how to prevent such a silly mistake from happening again? I was thinking of coverage data, but that wouldn't be enough for this example.

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:688-703 std::string StdLibraryFunctionsChecker::BufferSizeConstraint::describe( -ProgramStateRef State, const Summary &Summary) const { +DescritptionKind DK, Pro

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Awesome! Seems good to me. Though I've got limited experience on CTU stuff. It would be nice to have tests, but it seems pretty hard to come up with one for this. Given that this is just a 'performance' issue, I'm fine with it. Somehow try to check if this resolved your

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd882750f1105: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in… (authored by OikawaKirie, committed by steakhal). Changed prior to commit: https://reviews.llvm.org/D83660?vs

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Overall, it looks promising. But I don't quite get this test. There is no invocation yaml in the temp directory. So, you are probably not testing the right thing. You wanted to test if the invocation yaml exists, and could be opened **but the parsing fails**. You should

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. Okay, so you 'just' want an indication for the given open call. What about using `strace`? `strace -e trace=openat %clang_cc1 ... 2>&1 | grep '"invocations.yaml"' | FileCheck %s` This way the given test file could contain the contents of the `importer.c`. CHANGES SIN

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. I would rather match on the complete line (except the file descriptor ofc). By inspecting the output that is piped to the `count`, I have a suspicion: openat(AT_FDCWD, "invocations.yaml", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) It should have fou

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I would love to see `PreviousParsingResult` combined with `InvocationList` using a `llvm::Error`. I'm pretty sure it can be done. Regardless, I think it's already better than it was. CHAN

[PATCH] D102062: [analyzer][ctu] Append ctu-dir to ctu-invocation-list for non-absolute paths

steakhal added a comment. On second thought the current behavior is reasonable. We call clang from a command line, so I think it's fair to expect that relative paths are resolved using the CWD. AFAIK if one does not supply the `ctu-invocation-list`, then it would be substituted by `ctu-dir/invoc

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

steakhal added a comment. In D101763#2745802 , @OikawaKirie wrote: > I do not know why the test case always fails on the build server, it runs > perfectly on my computer. : ( I've locally run your patch and seemed good to me, I regret not having a look

[PATCH] D102240: [analyzer][solver] Prevent use of a null state

steakhal added a comment. Dam, we will be always haunted by these. Looks good btw. The test looks somewhat artificial - like a raw `creduce` result. Do you think It worth the effort to make it look like it was written by a human? I also somewhy prefer function parameters to globals for this speci

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

steakhal added reviewers: NoQ, vsavchenko, steakhal. steakhal added a comment. I'm gonna have a look at this tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102273/new/ https://reviews.llvm.org/D102273 _

[PATCH] D99260: [analyzer] Fix false positives in inner pointer checker (PR49628)

steakhal added a comment. In D99260#2704102 , @NoQ wrote: > In https://bugs.llvm.org/show_bug.cgi?id=45786 the godbolt link shows that > there are still problems with `addressof` (yes, their "trunk" clang is fresh > enough). They seem to have `__addresso

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

steakhal added a comment. In D91000#3225369 , @balazske wrote: > The functions `asctime` and `asctime_r` are discouraged according to CERT > MSC33-C rule. These could be added to this check as well. There is a clang SA > checker `SecuritySyntaxChecker` t

[PATCH] D116025: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

steakhal added a comment. Sweet stuff! Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:55 /// Check if tainted data is used as a buffer size ins strn.. functions, /// and allocators. typo Comment at: clang/lib/Static

[PATCH] D116025: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

steakhal accepted this revision. steakhal added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116025/new/ https://reviews.llvm.org/D116025 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

This revision was automatically updated to reflect the committed changes. Closed by commit rGa6816b957d28: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[PATCH] D114938: [Analyzer][NFCi] SValBuilder: Simlify a SymExpr to the absolute simplest form

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. You mentioned in the summary that there are different places where simplification-like machinary kicks in, which hindered the test case synthesis. What places did you refer to exactly? R

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal added a comment. Sorry for my late response, I'm busy with other things right now. I plan to come back to this in a couple of weeks. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:52 static bool areEquivalentNameSpecifier(const NestedNam

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

steakhal marked an inline comment as done. steakhal added a comment. Sorry for my late response, I have a bunch of other tasks to do. --- In D115149#3175068 , @NoQ wrote: >> It can happen if the `Loc` was perfectly constrained to a concrete >> value (`n

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

steakhal marked an inline comment as done. steakhal added a comment. In D115149#3182196 , @ASDenysPetrov wrote: > @steakhal > Please provide a case which asserts before your patch. I don't get this one. I've provided a bunch of tests, even annotated wit

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I think it looks great. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102669/new/ https://reviews.llvm.org/D102669 ___ cfe-com

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal added a comment. In D102669#3194405 , @OikawaKirie wrote: > Should I disable this test case on Windows? Or is there any other approaches > to make it work on Windows? I'm fine with disabling this test on Windows. CHANGES SINCE LAST ACTION

[PATCH] D113622: [analyzer] support ignoring use-after-free checking with reference_counted attribute

steakhal added a comment. In D113622#3193319 , @chrisdangelo wrote: > I've successfully exercised these changes against a large C / C++ project and > studied the output with @NoQ. Could you please share the results to have look? How can I reproduce and

[PATCH] D113622: [analyzer] support ignoring use-after-free checking with reference_counted attribute

steakhal added a comment. In D113622#3195985 , @NoQ wrote: > In D113622#3194580 , @steakhal > wrote: > >> Could you please share the results to have look? How can I reproduce and >> evaluate the effect of this c

[PATCH] D114718: [analyzer] Implement a new checker for Strict Aliasing Rule.

steakhal added a comment. I haven't checked the code, but only the tests and expectations for scalar values. I'll dive deeper if we settled on those. A couple of remarks: 1. I would rather use top-level function parameters of the given type instead of declaring an instance of that type and tak

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG333d66b09494: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space… (authored by OikawaKirie, committed by steakhal). Repository

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal edited reviewers, added: a.sidorin, shafik, xazax.hun, teemperor; removed: hgabii. steakhal added a comment. We shouldn't skip mac targets. I CC ASTImporter folks, they probably have an M1 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D115934: [analyzer] Add range constructor to CallDescriptionMap

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp:24-28 +static_assert( +std::is_constructible, + decltype(std::declval

[PATCH] D115931: [analyzer] Enable move semantics for CallDescriptionMap

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp:22 +static_assert(std::is_move_constructible>() && + std::is_move_assignable>()

[PATCH] D115932: [Analyzer] Create and handle SymbolCast for pointer to integral conversion

steakhal added a comment. Many thanks for digging into this @martong. I really enjoyed it! I also believe that this is the fix for the underlying issue. I also think the `getAsSymbol()` should be somewhere where we can create new symbols. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal updated this revision to Diff 395150. steakhal added a comment. Sorry for the late update, but here we are: Now, I properly handle all kinds of `NestedNameSpecifiers`. Basically, I verify if the decls are matching, then I use the nested name specifiers for finding mismatches. I look thr

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

steakhal marked an inline comment as done. steakhal added a comment. In D114622#3200678 , @NoQ wrote: > These checks are almost 2000 lines of code each and it looks like all they do > is figure out if two statements are the same and we have a very flexib

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

steakhal reopened this revision. steakhal added a comment. This revision is now accepted and ready to land. In D102669#3205889 , @OikawaKirie wrote: > In D102669#3199270 , @steakhal > wrote: > >> We shouldn't sk

[PATCH] D98707: [clang][ASTImporter] Fix import of VarDecl regarding thread local storage spec

steakhal created this revision. steakhal added reviewers: a.sidorin, shafik, martong, balazske. Herald added subscribers: teemperor, rnkovacs. steakhal requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. After the import, we did not copy the `T

[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

steakhal added a comment. I'm really sorry about being sooo picky about this patch. It's not my expertise and the change seems to address a corner-case, so we have to especially careful not introducing bugs. My concern is that I still don't understand why do we want to do anything with reinterp

[PATCH] D98726: [analyzer] Remove unnecessary TODO

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I don't think the `TODO` is addressed. By checking the //git blame// quickly, there was no change committed to the `SmartPtrChecker` affecting the collaboration with the `MallocCh

[PATCH] D98741: [analyzer] Introduce common bug category "Unused code".

steakhal accepted this revision. steakhal added a comment. `Unused code` seems to be broader and probably a better fit for a generic //bug category//. Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:263-264 -BR.EmitBasicReport(AC->getDecl(), Checker,

[PATCH] D98707: [clang][ASTImporter] Fix import of VarDecl regarding thread local storage spec

steakhal updated this revision to Diff 331191. steakhal added a comment. Moved to line 3481, changed the test logic accordingly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98707/new/ https://reviews.llvm.org/D98707 Files: clang/lib/AST/ASTImp

[PATCH] D98707: [clang][ASTImporter] Fix import of VarDecl regarding thread local storage spec

steakhal updated this revision to Diff 331302. steakhal added a comment. Revert to the previous version and move to line 738 just before "ImportRecordTypeInFunc". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98707/new/ https://reviews.llvm.org/D9

[PATCH] D98918: [clang][lit] Allow test cases to use the compiler that are used to compile Clang

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Oh, wait a minute. The `config.host_cxx` was already there. Where is the symmetry :D It looks much better now, thanks. Regarding D83660 , I'm really looking

[PATCH] D98341: [analyzer][solver] Prevent infeasible states (PR49490)

steakhal added a comment. The assertion `areFeasible(Constraints)` triggered on trunk. More details in the Bugzilla ticket . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98341/new/ https://reviews.llvm

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

steakhal added a comment. Ah, I wanted to give it a go, but the bots caught an assertion failure for the parent revision of this. See the details at the Bugzilla ticket . What is more concerning is that this patch resolves that assertion failure. I

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

steakhal added a comment. I did not follow the discussion closely but we (CodeChecker team) might have a similar problem. Consider this: https://godbolt.org/z/835P38 int do_bifurcation(int p) { return p < 0; } int b(int x, int y) { int tmp = 13 / y; // y can't be 0. (void)tmp;

[PATCH] D98948: [analyzer][solver] Fix infeasible constraints (PR49642)

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Aa, get it. Looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98948/new/ https://reviews.llvm.org/D98948 __

[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

steakhal added inline comments. Comment at: clang/test/Analysis/pthreadlock_state.c:55 + // CHECK-NEXT: "mtx: conj_$11{int, LC1, S1874, #1}", + // CHECK-NEXT: "" + // CHECK-NEXT:]} Out of curiosity, what is the purpose of this 'empty' line? I've

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Given that it did not change any reports in our testbench it seems to be safe to land it. It clearly improves the API significantly, so I'm not opposing. Really good work @vsavchenko. PS:

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

steakhal added a comment. In D86465#2640765 , @vsavchenko wrote: > I do have a bit of a struggle here. This is a unit-test and thus should be > compiled for all of the supported architectures by all of the supported > compilers. > Is there a `__has_feat

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

steakhal added a comment. In D86465#2640875 , @vsavchenko wrote: > This is not only a compiler feature, it also should be supported by the > target architecture: > https://godbolt.org/z/ddjYYx9x6 It's getting complicated then xD. I guess we should comple

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

steakhal added a comment. In D86465#2640958 , @vsavchenko wrote: > In D86465#2640954 , @steakhal wrote: > >> It's getting complicated then xD. I guess we should complement unittests >> with LIT tests? >> You can kn

[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

steakhal added inline comments. Comment at: clang/test/Analysis/pthreadlock_state.c:70 + // CHECK-NEXT: "Mutexes in unresolved possibly destroyed state:", + // CHECK-NEXT: "SymRegion{reg_$12}: conj_$15{int, LC1, S1921, #1}", + // CHECK-NEXT: "" --

[PATCH] D99194: [analyzer] Fix body farm for Obj-C++ properties

steakhal resigned from this revision. steakhal added a comment. Herald added a subscriber: steakhal. I'm not familiar with Objective-C. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99194/new/ https://reviews.llvm.org/D99194 __

[PATCH] D99181: [analyzer] Fix crash on spaceship operator (PR47511)

steakhal added a comment. I'm always in favor of bugfixes. However, I have some concerns about this one. Comment at: clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:169-171 + } else if (B->getOpcode() == BinaryOperatorKind::BO_Cmp) { +// We can't reason ab

[PATCH] D98918: [clang][lit] Allow test cases to use the compiler that are used to compile Clang

steakhal added a comment. In D98918#2636790 , @OikawaKirie wrote: > Please commit this patch on my behalf (Ella Ma ), so > that I can continue with D83660 with your > mocked solver. Oh, I forgot about this one. My apolo

[PATCH] D99274: [analyzer] Fix crash when reasoning about C11 atomics (PR49422)

steakhal accepted this revision. steakhal 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/D99274/new/ https://reviews.llvm.org/D99274 _

[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. This patch does not model faithfully how reinterpretcasting member pointers should work. The base specifiers represent the history for the member pointer (aka. mptr), describing

[PATCH] D99262: [analyzer] Fix dead store checker false positive

steakhal added a comment. I see your point. Would it report this issue? int test() { struct Foo foo = {0, 0}; // I would expect a warning here, that 'foo' was initialized but never read. (void)foo; return 0; } Only nits besides this. Comment at: clang/lib/Sta

[PATCH] D99262: [analyzer] Fix dead store checker false positive

steakhal added a comment. In D99262#2648533 , @vsavchenko wrote: > In D99262#2648512 , @steakhal wrote: > >> I see your point. >> >> Would it report this issue? >> >> int test() { >> struct Foo foo = {0, 0}; /

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

steakhal added a comment. In D97183#2640559 , @RedDocMD wrote: > @NoQ, why does the following trigger a null-dereference warning? > (https://godbolt.org/z/Kxox8qd16) > > void g(std::unique_ptr a) { > A *aptr = a.get(); > if (!aptr) {} > a->f

[PATCH] D99260: [analyzer] Fix false positives in inner pointer checker (PR49628)

steakhal added a comment. I recommend splitting this into two. I would happily accept the part about `std::data()`. IMO `std::addressof()` should be modelled via `evalCall` as a pure function, instead of hacking it into the `InnerPtrChecker`. It is overloaded to accept `const T&` as well, so th

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp:106-109 +if (const auto *DS = llvm::dyn_cast(S)) { + for (const Decl *D : DS->getDeclGroup()) { +if (const VarDecl *VD = llvm::dyn_cast(D)) { + const Expr *

[PATCH] D99344: [Analyzer] Track RValue expressions

steakhal added a comment. I really like it. Looks good. I'm letting someone else accept this as I've not really touched the trackExpression parts. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:119-124 +/// Attempts to add visitors to tr

[PATCH] D99344: [Analyzer] Track RValue expressions

steakhal added a comment. Some minor logical issues inline. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1944-1945 +return; + if (!BO->isMultiplicativeOp()) +return; + There are only 3 multiplicative operators: ``` BINARY_OPERATION

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I don't know. I think it's already way better than it was. I think we can reiterate this later. I want to get the `llvm_unreachable("Unknown SVal kind")` statements back. Besides those, I

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. Lets see what others think about this. Im fine with it on my part. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83660/new/ https://reviews.llvm.org/D83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D99344: [Analyzer] Track RValue expressions

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1958-1959 + EnableNullFPSuppression); + } else { // Track only the LHS of divisions. +if (LHSV.isZeroConstant()) + trackExpressionValue(InputNode,

[PATCH] D99576: [ASTImporter][NFC] Improve test coverage

steakhal created this revision. steakhal added reviewers: martong, teemperor, shafik, balazske, a.sidorin. Herald added subscribers: whisperity, rnkovacs, kristof.beyls. steakhal requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. All three cas

[PATCH] D99344: [Analyzer] Track RValue expressions

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. land it Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1938 + ProgramStateRef RVState = RVNode->getState(); + SVal V = RVState->getSValAsScalarOrLoc(

[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Everything looks fine. I like that regexp matcher so much. <3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98502/new/ https://reviews.llvm.

[PATCH] D99181: [analyzer] Fix crash on spaceship operator (PR47511)

steakhal accepted this revision. steakhal added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp:331 // Note: LAnd, LOr, Comma are handled specially by higher-level logic.

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

steakhal added reviewers: erichkeane, rnk, majnemer, EricWF. steakhal added a comment. I'm adding a couple of reviewers to get this going. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89649/new/ https://reviews.llvm.org/D89649 ___ cfe-commit

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added reviewers: martong, balazske, Szelethus, ASDenysPetrov. steakhal added a comment. Herald added a subscriber: rnkovacs. spam reviewers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83660/new/ https://reviews.llvm.org/D83660 ___

[PATCH] D99576: [ASTImporter][NFC] Improve test coverage

steakhal added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:3089 +struct ImportBlock : ASTImporterOptionSpecificTestBase {}; +const internal::VariadicDynCastAllOfMatcher blockDecl; +TEST_P(ImportBlock, ImportBlocksAreUnsupported) { thakis

[PATCH] D99576: [ASTImporter][NFC] Improve test coverage

steakhal marked 2 inline comments as done. steakhal added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:3089 +struct ImportBlock : ASTImporterOptionSpecificTestBase {}; +const internal::VariadicDynCastAllOfMatcher blockDecl; +TEST_P(ImportBlock, ImportBlock

[PATCH] D99576: [ASTImporter][NFC] Improve test coverage

steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:3089 +struct ImportBlock : ASTImporterOptionSpecificTestBase {}; +const internal::VariadicDynCastAllOfMatcher blockDecl; +TEST_P(ImportBlock, ImportBlock

[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

steakhal added inline comments. Comment at: clang/test/Analysis/pthreadlock_state.c:59 + // CHECK-NEXT: "mtx: destroyed", + // CHECK-NEXT: "SymRegion{reg_$[[REG:[0-9]+]]}: not tracked, possibly destroyed", + // CHECK-NEXT: "Mutexes in unresolved possibly destro

[PATCH] D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions

steakhal created this revision. steakhal added reviewers: NoQ, vsavchenko, martong, xazax.hun, balazske, Szelethus. Herald added subscribers: ASDenysPetrov, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. steakhal requested review of thi

[PATCH] D99659: [analyzer][taint] Extent of heap regions should get taint sometimes

steakhal created this revision. steakhal added reviewers: NoQ, vsavchenko, martong, balazske, xazax.hun. Herald added subscribers: ASDenysPetrov, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a reviewer: Szeleth

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added inline comments. Comment at: clang/test/Analysis/z3/Inputs/MockZ3_solver_check.c:6 + +// Mock implementation: return UNDEF for the 5th invocation, otherwise it just +// returns the result of the real invocation. martong wrote: > It's not clear why

[PATCH] D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions

steakhal planned changes to this revision. steakhal added a comment. In D99658#2661452 , @martong wrote: > Ah, I see you need `nonloc::SymbolVal` in your next patch, and > `getDynamicSizeWithOffset` returns an Unknown if the extend is symbolic. > > Anyway

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2661369 , @martong wrote: > I went through the change and it looks good, seems like this is indeed a copy > paste error from line 132. > I checked the related conversation, and thanks for all the effort spent with > th

[PATCH] D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions

steakhal updated this revision to Diff 334470. steakhal marked 3 inline comments as done. steakhal added a comment. This revision is now accepted and ready to land. Fix comments. I could not manage to create an `unknown` extent, where the behavior would diverge. Repository: rG LLVM Github Mon

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2661646 , @OikawaKirie wrote: > Can we automatically enable all test cases requiring z3 if clang is built > with z3? I do not think the patch D83677 > really make the problem fixed. Z

[PATCH] D99659: [analyzer][taint] Extent of heap regions should get taint sometimes

steakhal updated this revision to Diff 334474. steakhal added a comment. Add a FIXME about placing a NoteTag describing why the extent was getting tainted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99659/new/ https://reviews.llvm.org/D99659 F

[PATCH] D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC).

steakhal accepted this revision. steakhal added inline comments. Comment at: clang/test/Analysis/pthreadlock_state.c:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.unix.PthreadLock,debug.ExprInspection 2>&1 %s | FileCheck %s + AFAIK `core` should be enab

[PATCH] D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions

steakhal added a comment. In D99658#2665730 , @NoQ wrote: > What about `clang_analyzer_getExtent(&x[2])` then? I guess `(extent of heap segment that starts at symbol of type 'int *' conjured at statement 'new int [ext]') - 8` is the value I expect - whic

[PATCH] D99959: [analyzer][NFC] Add tests for extents

steakhal created this revision. steakhal added reviewers: NoQ, vsavchenko, Charusso. Herald added subscribers: ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a reviewer: Szelethus. steakhal request

[PATCH] D99659: [analyzer][taint] Extent of heap regions should get taint sometimes

steakhal abandoned this revision. steakhal added a comment. Obsoleted by D69726 . This effort continues as the NFC D99959 patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99659/

<    10   11   12   13   14   15   16   >