[PATCH] D96905: [libclang] Add missing fixed point literal CursorKind to cindex.py

steakhal added a comment. In D96905#2574398 , @steakhal wrote: > You could call it like `c-index-getCursor-fixed-point.c`. Hmm, to be honest, this was probably not what you wanted to test. What about simply inserting your own use-case which uncovered this

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

steakhal added a comment. Looks fine to me. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:466-467 + return; +OS << "Obtained null inner pointer from"; +checkAndPrettyPrintRegion(OS, ThisRegion); + })); You should emit a //sort

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

steakhal added a comment. In D96090#2574526 , @ASDenysPetrov wrote: > @steakhal wrote: > >> This patch preserves all previous reports as expected. Even with or without Z3 refutation. > If this patch is technically full-baked and does not bring any harm,

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

steakhal added a reviewer: xazax.hun. steakhal added inline comments. Herald added a subscriber: rnkovacs. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:466-467 + return; +OS << "Obtained null inner pointer from"; +checkAndPrettyPrintRegion(OS, T

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

steakhal added a comment. In D96976#2585498 , @RedDocMD wrote: > @steakhal, could you please review this? The longer I stare at it the more questions I have. However, I don't know how member pointers work in the analyzer and I'm not really in the mood to

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

steakhal added a comment. There is so much fancy stuff going on upstream. Awesome to see. I'm trying to catch up ASAP, I'm finally done with my master's thesis. In D103096#2798238 , @NoQ wrote: > Additionally, presence of cast symbols is extremely valuab

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

steakhal added a comment. Overall I think it's a useful checker not only for checking the `getenv()` but a bunch of other functions as well, which might return a pointer to a statically allocated buffer. The implementation could be polished a bit but it's ok I think. About the produced reports,

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:180 +// Note: This pointer has type 'const MemRegion *' +REGISTER_TRAIT_WITH_PROGRAMSTATE(EnvPtrRegion, const void *) + balazske wrote: > Is it not possible t

[PATCH] D103967: [Analyzer][solver] Add dump methods for (dis)equality classes.

steakhal added a comment. I suspect that the exploded-graph-rewriter should be updated as well so that the HTML dumps also carry this information. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103967/new/ https://reviews.llvm.org/D103967

[PATCH] D90399: [clang-tidy] non-portable-integer-constant check

steakhal added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/NonportableintegerconstantCheck.cpp:54 + +MaskStr = StringRef(MaskStr.lower()); +if (!MaskStr.consume_front("0x")) You are assigning a pointer to a temporal std::string. It

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal updated this revision to Diff 363101. steakhal added a comment. Removed the deceiving `-DNO_ELIDE_FLAG` define from the c++17 run line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107078/new/ https://reviews.llvm.org/D107078 Files: cl

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/test/Analysis/copy-elision.cpp:9-10 +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 \ +// RUN:-analyzer-config elide-constructors=false -DNO_ELIDE_

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal marked an inline comment as done. steakhal added a comment. In D107078#2917892 , @NoQ wrote: > Y'all writing really good patches lately. Awesome, thank you! Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal updated this revision to Diff 364393. steakhal added a comment. - `The temporary` -> `Temporary ` - `full expression` -> `full-expression` I did not fix the extra blue 'bubble' note issue, and I think that is out of the scope of this patch. CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

steakhal added a comment. First and foremost, I think this is a great change. I think the diagnostic is on the point. BTW it seems like you missed two notes in `misc-ps-region-store.m` causing a test fail: error: 'note' diagnostics seen but not expected: File /home/steakhal/git/llvm-proj

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

steakhal added a comment. In D107051#2928089 , @steakhal wrote: > First and foremost, I think this is a great change. I think the diagnostic is > on the point. > BTW it seems like you missed two notes in `misc-ps-region-store.m` causing a > test fail: >

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

steakhal added a comment. In D107051#2928536 , @balazske wrote: > If the original memory object is not known the static size is not known too. > Every pointer with unknown source can point into a bigger data structure. You are right, but IMO pointers to

[PATCH] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/ASTLookup.cpp:26 + // and we have a TypedefDecl with the name 'FILE'. + for (Decl *D : LookupRes) +if (auto *TD = dyn_cast(D)) `Decl *D` -> `const Decl *D`. Same for the other loo

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Aside from the inline nit, I think it's good to go. Let some time for the others to catch up, they might have objections. Comment at: clang/test/Analysis/return-ptr-range

[PATCH] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

steakhal added a comment. Let's see if we can cache the stream symbols across top-level analyses. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:476 + + for (Decl *D : LookupRes) { +D = D->getCanonicalDecl(); Comment at

[PATCH] D107636: [analyzer][solver] Compute adjustment for unsupported symbols as well

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Seems reasonable to me. Let's wait for someone else as well. This is a really elegant patch, I should tell! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:504 + + OrigStdin = findStdStream("stdin", C); + OrigStdout = findStdStream("stdout", C); balazske wrote: > steakhal wrote: > > balazske wrote: > > > steakhal wrote:

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal updated this revision to Diff 365133. steakhal marked 2 inline comments as done. steakhal added a comment. Removed the extra note about the temporary expression. --- In D107078#2933682 , @NoQ wrote: > In D107078#2927899

[PATCH] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. It's fine on my part. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106644/new/ https://reviews.llvm.org/D106644 __

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

steakhal accepted this revision. steakhal added inline comments. Comment at: clang/test/Analysis/return-ptr-range.cpp:17 return arr; // no-warning - } while (0); - return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buff

[PATCH] D107960: [clang][analyzer] Make ReturnPtrRange checker warn at negative index.

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. If `assumeInBound()` behaves unexpectedly, we should probably fix that instead. This change would workaround the issue, instead of fixing it. By fixing the root cause all users of

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:325-326 + if (ReferrerMemSpace && ReferredMemSpace) { +if (ReferredFrame == PoppedFrame && +ReferrerFrame->

[PATCH] D105017: [analyzer] LValueToRValueBitCasts should evaluate to an r-value

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

[PATCH] D105101: [analyzer] Make CheckerManager::hasPathSensitiveCheckers() complete again

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

[PATCH] D105101: [analyzer] Make CheckerManager::hasPathSensitiveCheckers() complete again

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3dae01911b69: [analyzer] Make CheckerManager::hasPathSensitiveCheckers() complete again (authored by steakhal). Changed prior to commit: https://

[PATCH] D105017: [analyzer] LValueToRValueBitCasts should evaluate to an r-value

steakhal updated this revision to Diff 355245. steakhal marked an inline comment as done. steakhal added a comment. Evaluate `LValueToRValueBitCast`s the same way as we evaluate `LValueToRValue` casts. Extended the tests accordingly. Thanks for the hint @NoQ! It looks much better now. CHANGES

[PATCH] D105017: [analyzer] LValueToRValueBitCasts should evaluate to an r-value

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:543 case CK_VectorSplat: { state = handleLVectorSplat(state, LCtx, CastE, Bldr, Pred); continue; NoQ wrote: > You didn't start it but that's, ugh, n

[PATCH] D105017: [analyzer] LValueToRValueBitCasts should evaluate to an r-value

steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:543 case CK_VectorSplat: { state = handleLVectorSplat(state, LCtx, CastE, Bldr, Pred); continue; steakhal

[PATCH] D105017: [analyzer] LValueToRValueBitCasts should evaluate to an r-value

steakhal added a comment. In D105017#2848465 , @NoQ wrote: > So it worked out of the box? Great! Yes, it did. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105017/new/ https://reviews.llvm.org/D105017 _

[PATCH] D105167: [analyzer] Fix HTML report deduplication.

steakhal added a comment. AFAIK from CodeChecker's side we are fine with this change. I think we are not using the `stable-report-filename` analyzer config either. Aside from these, the less //Perl// the better, I guess. Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostic

[PATCH] D105017: [analyzer] LValueToRValueBitCasts should evaluate to an r-value

This revision was automatically updated to reflect the committed changes. Closed by commit rGaa454dda2eed: [analyzer] LValueToRValueBitCasts should evaluate to an r-value (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105017/ne

[PATCH] D105125: [analyzer][NFC] Inline ExprEngine::handleLVectorSplat()

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. steakhal marked an inline comment as done. Closed by commit rG55662b24a475: [analyzer][NFC] Inline ExprEngine::handleLVectorSplat() (authored

[PATCH] D105125: [analyzer][NFC] Inline ExprEngine::handleLVectorSplat()

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp:523 } +LLVM_FALLTHROUGH; // Various C++ casts that are not handled yet. steakhal wrote: > NoQ wrote: > > Dunno, is this the intended formatting for fal

[PATCH] D105340: [analyzer] Produce SymbolCast symbols for integral types in SValBuilder::evalCast

steakhal added inline comments. Comment at: clang/test/Analysis/produce-symbolcast.cpp:1 +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-config handle-integral-cast-for-ranges=true -verify %s + vsavchenko wrote: > ASDenysPetrov wrote

[PATCH] D103967: [Analyzer][solver] Add dump methods for (dis)equality classes.

steakhal added a comment. AFAICT this patch does not introduce significant overhead to the exploded graph DOT dumps. Before the patch, an exploded graph took 12G and about 55 secs for a single top-level function. After the patch, it increased to 13G, and the runtime remained the same as expecte

[PATCH] D105708: [analyzer][NFC] Display the correct function name even in crash dumps

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

[PATCH] D105708: [analyzer][NFC] Display the correct function name even in crash dumps

This revision was automatically updated to reflect the committed changes. Closed by commit rGd3e14fafc69a: [analyzer][NFC] Display the correct function name even in crash dumps (authored by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105

[PATCH] D112296: [Analyzer][solver] Handle adjustments in constraint assignor remainder

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1623 if (!Constraint.containsZero()) { - State = RCM.assumeSymNE(State, LHS, Zero, Zero); + State = RCM.assumeSymRel(State, LHS, BO_NE, Zero); if (!State) --

[PATCH] D112551: [analyzer] Fix StringChecker for Unknown params

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc18407217e91: [analyzer] Fix StringChecker for Unknown params (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-c

[PATCH] D112296: [Analyzer][solver] Handle adjustments in constraint assignor remainder

steakhal accepted this revision. steakhal added inline comments. Comment at: clang/test/Analysis/constraint-assignor.c:7 void clang_analyzer_warnIfReached(); +void clang_analyzer_eval(); Expect an `int` parameter. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. Please also demonstrate that the patch can deal with sign mismatching ranges. Aside from that, it looks clean and sweet. Although, I still miss the point of how it would bypass anything. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. It seems like the test does not exercise the modified code segment. Please investigate, and make sure that the code you submit is actually exercised by the test you provide. Rep

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

steakhal added a comment. It looks good to me. I don't mind that FIXME. That being said, I'm not even sure it's a FIXME. Check my comment inline about this. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1757-1760 + // FIXME: Nevertheless, we can't do the same for

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1630-1641 +/// Returns true if the stored value can be accessed through the pointer to +/// another type: +/// const int arr[42] = {}; +/// auto* pchar = (char*)arr; +/// auto* punsig

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:200-204 +} else { + //[ First ]--> + //[ Second ]---> + // MIN^ + // The First range is entirely inside the Second one. Might

[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

steakhal added a comment. Please, try to focus on marking inline comments //done// if you accomplished them. It takes some time to reevaluate them one-by-one on each update. Aside from that, I'd like to apply these and play with them but I'm frequently having conflicts applying your patches. Re

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

steakhal added a comment. I'm still worried about regressions. Please split the patch into two by separating the tests into an NFC patch, on which you would apply the behavioral change. That way it would be clear what and why changed. It would also help us to see what previously had defects you

[PATCH] D112409: [clang-tidy] Add check 'cert-err33-c'.

steakhal added a comment. It looks good to me. But I'm leaving the approval up to the //tidy// folks. BTW shouldn't we use //backticks// in the giant list? Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:45-50 +// The following functions are +// deliberately ex

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1757-1760 + // FIXME: Nevertheless, we can't do the same for cases, like: + // const char *str = "123"; // literal length is 4 + // char c = str[41];// offset is 41 + // It s

[PATCH] D112558: [analyzer] sprintf is a taint propagator not a source

This revision was automatically updated to reflect the committed changes. Closed by commit rG49285f43e5ed: [analyzer] sprintf is a taint propagator not a source (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://review

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. There is no need to do anything with this. You can commit as-is. I've done it myself, and it seems like your patch only turns `TRUE` for the cases where it was `UNKNOWN` or `FALSE` previous

[PATCH] D98726: [analyzer] Enabling MallocChecker to take up after SmartPtrModelling

steakhal added a comment. What's the status on this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98726/new/ https://reviews.llvm.org/D98726 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D112889: [analyzer] Dump checker name if multiple checkers evaluate the same call

This revision was automatically updated to reflect the committed changes. steakhal marked 2 inline comments as done. Closed by commit rG9b5c9c469d90: [analyzer] Dump checker name if multiple checkers evaluate the same call (authored by steakhal). Herald added a project: clang. Herald added a subsc

[PATCH] D112889: [analyzer] Dump checker name if multiple checkers evaluate the same call

steakhal added a comment. Committed with the requested changes. Comment at: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:679 +#ifndef NDEBUG + if (evaluated && evaluatorChecker.hasValue()) { +const auto dump = [](const CallEvent &Call) -> std::string {

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1732 + if (!State) +return false; +} martong wrote: > steakhal wrote: > > I'd love to see a coverage report of the tests you add with this patch.

[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

steakhal added a comment. I'm still worried about the fact that you assume that there is a correspondence between `ElementRegions` and InitListExprs. I cannot see why this assumption holds, since reinterpret casts might introduce `ElementRegions` which could mess with this assumption. Aside from

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

steakhal added a comment. I very much like the code. L1364 is uncovered according to my coverage results. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1360-1364 +// Special case: We allow a struct defined in a function to be equivalent +// with a similar struc

[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

steakhal added a comment. In D111654#3109585 , @ASDenysPetrov wrote: > @steakhal Thank you for your suggestion. I'll make corresponding changes. > >> I'm still worried about the fact that you assume that there is a >> correspondence between `ElementRegi

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

steakhal added subscribers: whisperity, steakhal. steakhal added a comment. Herald added a subscriber: carlosgalvezp. It seems like the checker is documented as `readability-data-pointer` but in the tests it reports issues under the `readability-container-data-pointer` name. Shouldn't they be the

[PATCH] D106823: [analyzer][solver] Iterate to a fixpoint during symbol simplification with constants

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. In D106823#3109469 , @martong wrote: >> As of diff 5, line 1767 and all the code in the block at line 2184 are >> uncovered by the tests you provi

[PATCH] D113261: [analyzer][solver] Remove reference to RangedConstraintManager

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Yey, it looks good. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1608 LLVM_NODISCARD static ProgramStateRef - assign(ProgramStateRef State, Ra

[PATCH] D113251: [analyzer][doc] Add user documenation for taint analysis

steakhal added a comment. Excellent work. Don't be afraid of the number of nits I dump on you! Good job. Comment at: clang/docs/analyzer/checkers.rst:2338 +Default sources defined by `GenericTaintChecker`: +``fdopen``, ``fopen``, ``freopen``, ``getch``, ``getchar``, ``getcha

[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Feel free to commit your change given that you fix my final nits. Yeah, I'm soo bad in review. I should have proposed these previously. Sorry. Comment at: clang/lib/Stati

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. Hmm, something is still bad. F20158447: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Looks great. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp:155 -/// Look through conversion/copy constructors to find the explicit +/// Look t

[PATCH] D113397: [analyzer][docs] Fix the incorrect structure of the checker docs

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Looks great. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113397/new/ https://reviews.llvm.org/D113397 ___ cfe-commits mailin

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

steakhal added a comment. For testing this I would recommend using a separate test file. That being said, you should avoid globals even in tests when you can. The distance between its declaration and use just makes it harder to comprehend and reason about. You could have a parameter, and take it

[PATCH] D113558: [clang-tidy] Fix cppcoreguidelines-virtual-base-class-destructor in macros

steakhal created this revision. steakhal added reviewers: aaron.ballman, njames93, alexfh, mgartmann, whisperity. Herald added subscribers: carlosgalvezp, martong, shchenz, rnkovacs, kbarton, xazax.hun, nemanjai. steakhal requested review of this revision. Herald added a project: clang-tools-extr

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

steakhal added a comment. In D110927#3118936 , @ASDenysPetrov wrote: >> You could have a parameter, and take its address to accomplish your >> reinterpret casts and type puns. > > Do you mean: ... > If so, IMO it doesn't matter. I see. Sorry about the

[PATCH] D113480: [analyzer] Fix region cast between the same types with different qualifiers.

steakhal added a comment. Very solid patch! Comment at: clang/lib/StaticAnalyzer/Core/Store.cpp:98 QualType PointeeTy = CastToTy->getPointeeType(); QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy); You only use `CanonPointeeTy.getLocalUnqualifi

[PATCH] D113480: [analyzer] Fix region cast between the same types with different qualifiers.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Please clang-format the patch to reflow the touched comment. Given that's done, you are good to go. Also, wait a couple days before committing to give chance to the rest of the reviewers t

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

steakhal added subscribers: njames93, whisperity, steakhal. steakhal added a comment. Herald added subscribers: carlosgalvezp, manas, ASDenysPetrov. Herald added a project: clang-tools-extra. It seems like the list got pretty outdated by the time. Do you think we should really refer to the clang-a

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

steakhal added inline comments. Comment at: clang/test/Analysis/initialization.cpp:295-299 +void glob_cast_opposite_sign1() { + auto *ptr = (unsigned int *)glob_arr2; + auto x1 = ptr[0]; // no-warning + auto x2 = ptr[1]; // expected-warning{{garbage or undefined}} +} -

[PATCH] D113754: [Analyzer][Core] Simplify IntSym in SValBuilder

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Great stuff. Have you checked the coverage? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113754/new/ https://reviews.llvm.org/D113754

[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:378 + SVal simplifiedLhs = simplifySVal(state, lhs); + SVal simplifiedRhs = simplifySVal(state, rhs); + if (auto simplifiedLhsAsNonLoc = simplifiedLhs.getAs()) It s

[PATCH] D103317: [Analyzer][Core] Make SValBuilder to better simplify svals with 3 symbols in the tree

steakhal added a comment. To me at least, the patch looks good. Please post some comparative measurements to demonstrate it won't introduce runtime regression. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1144 + :

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

steakhal added inline comments. Comment at: clang/test/Analysis/initialization.cpp:295-299 +void glob_cast_opposite_sign1() { + auto *ptr = (unsigned int *)glob_arr2; + auto x1 = ptr[0]; // no-warning + auto x2 = ptr[1]; // expected-warning{{garbage or undefined}} +} -

[PATCH] D103511: [clang-tidy] Special member functions check should allow sole dtors by default

steakhal abandoned this revision. steakhal added a comment. Herald added a subscriber: carlosgalvezp. I forgot to abandon this earlier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103511/new/ https://reviews.llvm.org/D103511

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

steakhal added a comment. In D96976#2587513 , @RedDocMD wrote: > Many thanks for you comments, @steakhal! > I will address the issues you have pointed out in this comment. To clean > things up I should perhaps add some more clarification to the summary. >

[PATCH] D97388: [analyzer] Replace StoreManager::evalIntegralCast with SValBuilder::evalCast

steakhal added a comment. Please make sure that you specify the parent revision or the patch can be applied on top of the tree. Your lines clearly not match the top of the tree state. https://github.com/llvm/llvm-project/blob/13f4448ae7db1a477ec2d48776e46415a3401314/clang/include/clang/StaticAnal

[PATCH] D96586: [analyzer][CTU][NFC] Add an extra regression test

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

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

steakhal added a comment. In D96976#2590200 , @RedDocMD wrote: > Unfortunately, all the methods on CXXRecordDecl, like the one you mentioned, > are for querying and thus returns a `bool`, while I need the entire path. AFAIK the second overload accepts an

[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. I have serious concerns inline. Comment at: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp:20 int DoubleDerived::*ddf = &Base::field; int Base

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

steakhal added a comment. If the inner pointer participates in a branch condition guarding the dereference, that memory region is gotta be important, right? So, we should mark it so. A BugreportVisitor could easily transfer the information about the fact that the dereference was guarded by that

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

steakhal added a comment. I'm somewhat bothered that this patch is not landed yet. :| I made a **crude** mock to trigger the bug using `LD_PRELOAD`. F15707493: LD_PRELOAD-workaround.patch The test reproduces the issue and passes if you apply the fix from this

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

steakhal added inline comments. Comment at: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp:29-31 + Some some; + some.*sf = 14; + clang_analyzer_eval(some.*sf == 14); // expected-warning{{UNKNOWN}} RedDocMD wrote: > steakhal wrote: > > The assignmen

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

steakhal added a comment. In D97183#2597099 , @RedDocMD wrote: > The InnerPointerVal memory region is not marked as interesting as of now, I > have tried that out. The branch condition constraint is set by the > ConstraintManager and it is queried via in

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

steakhal added inline comments. Comment at: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp:29-31 + Some some; + some.*sf = 14; + clang_analyzer_eval(some.*sf == 14); // expected-warning{{UNKNOWN}} RedDocMD wrote: > steakhal wrote: > > RedDocMD wrot

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

steakhal added a comment. In D97849#2600201 , @aaron.ballman wrote: > This looks reasonable to me (good catch!), but is there a way for us to add a > regression test for it? I'm not sure if it's possible to write a test for deterministically demonstrat

[PATCH] D96586: [analyzer][CTU][NFC] Add an extra regression test

steakhal added a comment. In D96586#2601856 , @NoQ wrote: >> `-analyzer-opt-analyze-headers` > > I'm actually shocked that we provide such option at all. And that it's the > first option in the list in `scan-build --help`. And that I haven't noticed > it

[PATCH] D97936: [analyzer][docs][NFC] Fix typo in checkers.rst

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

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

steakhal added a comment. In D96976#2605313 , @RedDocMD wrote: > Talking on the mailing list, I got a link on reinterpret_casting for > pointer-to-member: > https://timsong-cpp.github.io/cppwp/n4861/expr.reinterpret.cast#10 > The gist is that this sort

[PATCH] D97388: [analyzer] Replace StoreManager::evalIntegralCast with SValBuilder::evalCast

steakhal added a comment. All reports and crashes are preserved at this point of the patch stack - with or without z3 crosscheck on multiple projects - even on llvm. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97388/new/ https://reviews.llvm.org/D97388 __

[PATCH] D97388: [analyzer] Replace StoreManager::evalIntegralCast with SValBuilder::evalCast

steakhal added a comment. If @NoQ doesn't have any objections I'm ok with this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97388/new/ https://reviews.llvm.org/D97388 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D98244: [analyzer] Fix StdLibraryFunctionsChecker performance issue

steakhal accepted this revision. steakhal added a comment. > For projects not using standard libraries, the speed-up can reach 50% after > this patch. Uh, that's something serious! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98244/new/ https://

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