[PATCH] D149160: [clang][analyzer] Handle special value AT_FDCWD in affected standard functions

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1530-1541 + // Get platform dependent values of some macros. + // Try our best to parse this from the Preprocessor, otherwise fallback to a + // default value (what is

[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Approved, assuming we prefer `std::nullopt` over the default construction of `std::optional` and you `clang-format` the affected hunks, such as the declaration of `computeOffset`. Make sur

[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

steakhal added inline comments. Comment at: clang/test/Analysis/Inputs/std-c-library-functions.h:1-2 +typedef typeof(sizeof(int)) size_t; +typedef signed long ssize_t; +typedef struct { balazske wrote: > steakhal wrote: > > `ssize_t`'s size should match the size

[PATCH] D149160: [clang][analyzer] Handle special value AT_FDCWD in affected standard functions

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1988 + return std::make_shared( + ArgN, WithinRange, Range({AT_FDCWDv, AT_FDCWDv}, {0, IntMax}), ""); +}; Should we define a specific c

[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

steakhal added a comment. Sure, Ill look at this on Tuesday. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149259/new/ https://reviews.llvm.org/D149259 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D149460: [analyzer] ArrayBoundCheckerV2: suppress false positives from ctype macros

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

[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

steakhal added a comment. In D149259#4315393 , @donat.nagy wrote: > @steakhal Could you review this change when you have some time for it? I didn't forget about this one. It's in progress, but it's difficult to squeeze this in. The ETA is probably tomo

[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

steakhal accepted this revision. steakhal added a comment. We only had a single new issue after this patch. This is well within what I would be comfortable with. I'll investigate the case anyway. It's likely something flaky. Feel free to land this in the meantime. Repository: rG LLVM Github

[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

steakhal added a comment. In D149259#4317788 , @steakhal wrote: > We only had a single new issue after this patch. This is well within what I > would be comfortable with. I'll investigate the case anyway. It's likely > something flaky. It did not repro

[PATCH] D152169: [clang][analyzer] Add report of NULL stream to StreamChecker.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Ah yes, finally this sorts itself out. We also had to revert 570bf972f5adf05438c7e08d693bf4b96bfd510a because we use t

[PATCH] D152169: [clang][analyzer] Add report of NULL stream to StreamChecker.

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:553 HelpText<"Check stream handling functions">, + WeakDependencies<[NonNullParamChecker]>, Documentation; What's the purpose of this hunk? Repository:

[PATCH] D152169: [clang][analyzer] Add report of NULL stream to StreamChecker.

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:553 HelpText<"Check stream handling functions">, + WeakDependencies<[NonNullParamChecker]>, Documentation; balazske wrote: > steakhal wrote: > > What's the

[PATCH] D152255: [analyzer][NFC] Pass the diagnostic message to the TrackConstraintBRVisitor

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

[PATCH] D152194: [NFC][CLANG] Fix nullptr dereference issue found by static analyzer tool

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I agree with @erichkeane. However, I can also see that the code could be improved. I don't understand why we have that variable hoisted from the guarded block in the first place.

[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp:151-164 + auto ArgSVal = C.getSVal(E).getAs(); + if (!ArgSVal) +return; + + ProgramStateRef StNonNull, StNull; + std::tie(StNonNull, StNull) = State->assume(*ArgSV

[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp:151-164 + auto ArgSVal = C.getSVal(E).getAs(); + if (!ArgSVal) +return; + + ProgramStateRef StNonNull, StNull; + std::tie(StNonNull, StNull) = State->assume(*ArgSV

[PATCH] D152194: [StaticAnalyzer] Fix nullptr dereference issue found by static analyzer tool

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Please update the title and the summary of this patch to reflect what it actually achieves. We also usually use the `[analyzer]` tag only for changes touching `StaticAnalyzer` stuff. Afte

[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

steakhal added a comment. I had some improvement opportunities in mind scattered, so I decided to do them, and here is how the diff looks for me now: F27853795: recommendation.patch Basically, I reworked the two branches a bit to express the intent more stra

[PATCH] D150446: [analyzer] Check ArraySubscriptExprs in ArrayBoundCheckerV2

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. In D150446#4337657 , @donat.nagy wrote: > After some thinking and discussion with @gamesh411 I decided that it'd be > better to replace

[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

steakhal added a comment. In D152269#4403282 , @tripleCC wrote: > Yes, your changes are aligned with my intent. It seems like you have made > excellent optimizations to this patch. > To eliminate the following two warnings, I add the `-fobjc-arc`(enable

[PATCH] D150446: [analyzer] Check ArraySubscriptExprs in ArrayBoundCheckerV2

steakhal added a comment. Okay. I think we are aligned. I'm still uncomfortable sacrificing reports for fast pace development. IMO if that is the goal, then a brand new alpha checker is the way forward. You can be sure that no external users depend on it as its brand new. Unlike with this V2 ch

[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

steakhal added a comment. T In D152269#4403491 , @tripleCC wrote: > In D152269#4403404 , @steakhal > wrote: > >> In D152269#4403282 , @tripleCC >> wrote: >> >>> Yes, yo

[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfa6b7dd520fc: [StaticAnalyzer] Fix false negative on NilArgChecker

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

steakhal added a comment. I think this patch broke two build bots, and now the CI is red. https://lab.llvm.org/buildbot/#/builders/16/builds/49282 https://lab.llvm.org/buildbot/#/builders/109/builds/65904 Could you please have a look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

steakhal added a comment. In D143984#4405961 , @dzhidzhoev wrote: > In D143984#4405875 , @steakhal > wrote: > >> I think this patch broke two build bots, and now the CI is red. >> https://lab.llvm.org/buildbot/#

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

steakhal added a comment. In D152436#4405558 , @balazske wrote: > These are reports that could be improved: > link >

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

steakhal added a comment. In D152436#4408811 , @balazske wrote: > In D152436#4408301 , @steakhal > wrote: > >> I looked at the TPs, and if the violation was introduced by an assumption >> (instead of an assignme

[PATCH] D152435: [analyzer][CStringChecker] Adjust the invalidation operation on the super region of the destination buffer during string copy

steakhal added a comment. I left my comment at https://github.com/llvm/llvm-project/issues/55019 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152435/new/ https://reviews.llvm.org/D152435 ___ cfe-commits

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

steakhal added a comment. Ping @NoQ @xazax.hun Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151325/new/ https://reviews.llvm.org/D151325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D152435: [analyzer][CStringChecker] Adjust the invalidation operation on the super region of the destination buffer during string copy

steakhal added a comment. Thanks for the elaborated answer on the GH issue. Now it makes sense. And the fix is align with the observations, which is good. I only have concerns about the code quality of the modifier code. It was already in pretty bad shape, and I cannot say we improve it with thi

[PATCH] D152977: [NFC] Fix potential dereferencing of null return value.

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:294-295 // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed. - assert(lstate->isUntouchedAndPossiblyDestroyed() || - lstate->isUnlockedAndPossiblyDestroye

[PATCH] D153017: [StaticAnalyzer] Fix false negative when using a nullable parameter directly without binding to a variable

steakhal added a comment. I believe, `zaks.anna` and `vsavchenko` are no longer involved in the project. I think it makes sense to have the code owners NoQ and xazax.hun as reviewers, and I also tend to review quite a lot nowadays. And we usually use the `[analyzer]` tag instead of `[StaticAnalyz

[PATCH] D153017: [analyzer] Fix false negative when using a nullable parameter directly without binding to a variable

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:569-594 +void NullabilityChecker::checkBeginFunction(CheckerContext &C) const { + const LocationContext *LCtx = C.getLocationContext(); + auto AbstractCall = AnyCall::forDecl(LC

[PATCH] D153017: [analyzer] Fix false negative when using a nullable parameter directly without binding to a variable

steakhal added a comment. Now it looks much better. Let me check it in depth tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153017/new/ https://reviews.llvm.org/D153017 ___ cfe-commits mailing l

[PATCH] D152977: [NFC] Fix potential dereferencing of null return value.

steakhal accepted this revision. steakhal added a comment. Looks good. Thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:294-295 // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed. - assert(lstate->isUntouchedAndPossiblyDestroyed() |

[PATCH] D153017: [analyzer] Fix false negative when using a nullable parameter directly without binding to a variable

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. LGTM; I'll commit this on Monday. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153017/new/ https://reviews.llvm.org/D153017 __

[PATCH] D152435: [analyzer][CStringChecker] Adjust the invalidation operation on the super region of the destination buffer during string copy

steakhal added a comment. Let me come back to this once again in the upcoming days to make sure everything is good. Comment at: clang/test/Analysis/pr22954.c:581 clang_analyzer_eval(m27.s3[i] == 1); // expected-warning{{UNKNOWN}}\ expected-warning{{Potential leak of memo

[PATCH] D139534: [analyzer] Don't escape local static memregions on bind

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

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

steakhal added a comment. For the test cases where we should be able to prove the case but return `Unknown` instead, should be marked by a FIXME stating the expected value. Approximating a concrete value with `Unknown` is (almost) always correct. So, in this case, you don't need to go and fix th

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG77ab7281aa36: [analyzer][solver] Introduce reasoning for not equal to operator (authored by manas, committed by steakhal). Changed prior to commit:

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. Nice catch. I had a look at https://lists.llvm.org/pipermail/llvm-dev/2021-March/149216.html, and http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1169.pdf. Your change makes se

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:157-158 + Loc::isLocType(T)); return APSIntType(Ctx.getIntWidth(T), !T->isSignedIntegerOrEnumerationType()); } -

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

steakhal added a comment. I think we could settle on something like this: APSIntType getAPSIntType(QualType T) const { +if (T->isFixedPointType()) + return APSIntType(Ctx.getIntWidth(T), T->isUnsignedFixedPointType()); + // For the purposes of the analysis and constrain

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

steakhal added a comment. Really good progress. Nicestuff. I really appreciate the unittest. However Ive got some minor comments there :D Comment at: clang/test/Analysis/fixed-point.c:9 + +enum en_t { en_0 = 1 }; + I would suggest 'Kind' or something similar. S

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. The fix feels suboptimal in readability but let it be whatever. Functionally feels good. Tests are there and gets the job done. I wont object. Comment at: clang/include

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

steakhal added a comment. The `StaticAnalyzer` portion looks good to me AFAICT. Comment at: clang/lib/StaticAnalyzer/Core/CallDescription.cpp:39 ento::CallDescription::CallDescription(CallDescriptionFlags Flags, - ArrayRef QualifiedName, +

[PATCH] D140086: [analyzer][solver] Improve reasoning for not equal to operator

steakhal added a comment. Thanks for going the extra mile to address this last thing. I really appreciate it. I've got only a few minor comments and suggestions. I'd recommend spell-checking the comments and the summary of this revision. See my technical comments inline. The test coverage looks

[PATCH] D139604: [PATCH] Github Issue: Create a check that warns about using %p printf specifier #43453

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. I appreciate that you are interested in tackling this. I do think it's a low-hanging fruit, so it's a good choice! However, there are a couple of things we need to improve before

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

steakhal added a comment. In D138037#4003121 , @vabridgers wrote: > Hi @steakhal, thanks for the suggested change. > How we can help move this forward? From what I'm comprehending from the > notes, perhaps we could try running this change through our in

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

steakhal added a comment. I like it. Terse but complete. I think you could polish the example code to look less synthetic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117568/new/ https://reviews.llvm.org/D117568 _

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

steakhal added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-shared-ptr-array-mismatch.rst:22 + std::shared_ptr x(new Foo[10]); // -> std::shared_ptr x(new Foo[10]); + // ^ warning: shared pointer to non-array is initialize

[PATCH] D115349: [analyzer][NFC] Re-enable skipped SValTests by relaxing expectations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG881b6a009fb6: [analyzer][NFC] Re-enable skipped SValTests by relaxing expectations (authored by steakhal). Herald added a project: clang. Herald adde

[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

steakhal accepted this revision. steakhal added a comment. Looks great. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117568/new/ https://reviews.llvm.org/D117568 ___ cfe-commits mailing list cfe-commits

[PATCH] D117717: [clang] Ignore -fconserve-stack

steakhal added a comment. I'm not really familiar with `scan-build`, we at CodeChecker, we ignore this gcc specific flag explicitly: https://github.com/Ericsson/codechecker/blob/85dd52dc8f4d47fcf26419fcb44de1ff2a58924f/analyzer/codechecker_analyzer/buildlog/log_parser.py#L72 So, IMO we should pr

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

steakhal added a subscriber: NoQ. steakhal added a comment. I'm wondering if we should have an assertion within the `SValBuilder::evalBinOpLL()` asserting that the pointers should have the same bitwidths. That's better than having nothing, waiting for a bugreport containing such a constraint on

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:718 +static uint64_t getLocBitwidth(ProgramStateRef State, Loc loc) { + uint64_t LocBitwidth = 0; Why don't you simply call `loc->getType()` end delegate the heavy

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

This revision was automatically updated to reflect the committed changes. steakhal marked 3 inline comments as done. Closed by commit rG9d6a61597301: [analyzer] Prevent misuses of -analyze-function (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits. Chan

[PATCH] D118690: [analyzer] Prevent misuses of -analyze-function

steakhal added a comment. Landed with the requested changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118690/new/ https://reviews.llvm.org/D118690 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D118439: [scan-build] Fix deadlock at failures in libears/ear.c

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd919d027ba2a: [scan-build] Fix deadlock at failures in libears/ear.c (authored by steakhal). Herald added a project: clang. Herald added a subscriber

[PATCH] D118439: [scan-build] Fix deadlock at failures in libears/ear.c

steakhal added a comment. Submitted for backporting to 13.0.1: https://github.com/llvm/llvm-project/issues/53541 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118439/new/ https://reviews.llvm.org/D118439 __

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

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:368 Loc makeNullWithType(QualType type) { -return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type)); - } - - Loc makeNull() { -return loc::ConcreteInt

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:335 - NonLoc makeIntValWithPtrWidth(uint64_t integer, bool isUnsigned) { -return nonloc::ConcreteInt( -BasicVals.getIntWithPtrWidth(integer, isUnsigned)

[PATCH] D120236: Add more sources to Taint analysis

steakhal added a comment. Sorry for the wall-of-nits. Overall, looks great. Comment at: clang/docs/analyzer/checkers.rst:2358 Default sources defined by ``GenericTaintChecker``: -``fdopen``, ``fopen``, ``freopen``, ``getch``, ``getchar``, ``getchar_unlocked``, ``gets``, ``sca

[PATCH] D120236: Add more sources to Taint analysis

steakhal added a comment. Also great to see that the docs are in-sync to the code, which is a great plus! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120236/new/ https://reviews.llvm.org/D120236 ___ cf

[PATCH] D120236: [analyzer] Add more sources to Taint analysis

steakhal added a comment. The pre-merge checks failed due to an unrelated test case: `Driver/hip-link-bundle-archive.hip` (on Windows); so I think we are good to go on that part. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120236/new/ https://r

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Thanks for the patch. LGTM on my part. @NoQ WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120134/new/ https://reviews.llvm.org/D120134

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Besides a couple of nits, it's ready for landing. Thanks for the hard work @vabridgers! Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:726-727 + Lhs

[PATCH] D120310: [clang][analyzer] Add modeling of 'errno' (work-in-progress).

steakhal added a comment. Good start! However, I'm not a big fan of coupling the testing checker with the actual modeling checker. IMO we should have a distinct checker, similarly to `TaintTester`. That way you could do even fancier things like: define `mylib_may_fail()`, bifurcate and return `

[PATCH] D120325: [analyzer] Don't crash if the analyzer-constraint is set to Z3, but llvm is not built with it

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Thanks. LGTM. Besides abusing the `-analyzer-constraints=z3` option we should really have some way displaying whether or not this clang was built w/o Z3. I'm accepting this, given that yo

[PATCH] D119128: [analyzer] Fix taint propagation by remembering to the location context

steakhal added a comment. I'll re-land the whole stack tomorrow, by adding the asserts requirement for the tests to make the tests pass on all bots. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119128/new/ https://reviews.llvm.org/D119128 __

[PATCH] D120310: [clang][analyzer] Add modeling of 'errno' (work-in-progress).

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:767-768 s->getType()->isBlockPointerType()); -assert(isa(sreg) || isa(sreg)); +assert(isa(sreg) || isa(sreg) || + isa(sreg)); }

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

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. I'm satisfied. I cannot see any issues. AFAI remember the tests runs uncovered no report changes - as expected from an NFC change. So it's good to go on my part. Wait for a review from @NoQ

[PATCH] D118987: [analyzer] Add failing test case demonstrating buggy taint propagation

steakhal added a comment. In D118987#3332940 , @jrtc27 wrote: > In D118987#3319697 , @steakhal > wrote: > >> It seems like the `clang-ve-ninja` doesn't really want to accept any patches >> from me :D >> I hope i

[PATCH] D118987: [analyzer] Add failing test case demonstrating buggy taint propagation

steakhal closed this revision. steakhal added a comment. Committed as fa0a80e017ebd58a71bdb4e4493bb022f80fe791 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118987/new/ https:/

[PATCH] D119128: [analyzer] Fix taint propagation by remembering to the location context

steakhal added a comment. Committed as a848a5cf2f2f2f8a621bdc5a12e0fe49dc743176 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119128/new/ https://reviews.llvm.org/D119128

[PATCH] D119129: [analyzer] Fix taint rule of fgets and setproctitle_init

steakhal added a comment. Committed as 7036413dc21254c8bf2f4ac62a3b087bc4b94ce8 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119129/new/ https://reviews.llvm.org/D119129

[PATCH] D120236: [analyzer] Add more sources to Taint analysis

steakhal added a comment. Fewer nits this time. We are converging! Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:559 + {{"gethostname"}, TR::Source({{0}})}, + {{"getnameinfo"}, TR::Source({{2, 4}})}, + {{"getseuserbyname"}, TR::Source({{1,

[PATCH] D120369: [analyzer] Add more propagations to Taint analysis

steakhal added a comment. Huh, this was a long one. 😅 🚀 Comment at: clang/docs/analyzer/checkers.rst:2361 Default propagations defined by ``GenericTaintChecker``: -``atoi``, ``atol``, ``atoll``, ``fgetc``, ``fgetln``, ``fgets``, ``fscanf``, ``sscanf``, ``getc``, ``getc_unlock

[PATCH] D120310: [clang][analyzer] Add modeling of 'errno' (work-in-progress).

steakhal added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:351-353 +def ErrnoChecker : Checker<"Errno">, + HelpText<"Make the special value 'errno' available to other checkers.">, + Documentation; I think `ErrnoModeling`

[PATCH] D119004: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

steakhal added a comment. I think it deserves an accept, however, as I don't agree with the rationale I'll let someone else for doing this. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h:111 - /// \copydoc clang::ento::matchesAny(const

[PATCH] D118987: [analyzer] Add failing test case demonstrating buggy taint propagation

steakhal marked an inline comment as done. steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:782 +if (PropDstArgs.contains(I)) { + LLVM_DEBUG(llvm::dbgs() << "PreCall<"; Call.dump(llvm::dbgs()); +

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

steakhal added a comment. I'm looking forward to this check. Comment at: clang/test/Analysis/bstring.c:300-311 void mempcpy14() { int src[] = {1, 2, 3, 4}; int dst[5] = {0}; int *p; - p = mempcpy(dst, src, 4 * sizeof(int)); + p = mempcpy(dst, src, 4 * sizeof(int))

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

steakhal requested changes to this revision. steakhal added a comment. This revision now requires changes to proceed. Oh wait, should we accept this given this serious limitation? Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:475-479 +def CStringUninitiali

[PATCH] D120310: [clang][analyzer] Add modeling of 'errno'.

steakhal accepted this revision. steakhal added a comment. Looks great, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120310/new/ https://reviews.llvm.org/D120310 ___ cfe-commits mailing list cfe

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

steakhal added inline comments. Comment at: clang/docs/analyzer/checkers.rst:2634-2647 +.. _alpha-unix-cstring-UninitializedRead: + +alpha.unix.cstring.UninitializedRead (C) +"" +Check for uninitialized read from source in memory copy function: ``

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

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

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

steakhal added a comment. In D120555#3345471 , @aaron.ballman wrote: > LGTM, please add a release note for the fix. That being said, should backport this to clang-14? @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D120310: [clang][analyzer] Add modeling of 'errno'.

steakhal added a comment. according to the pre-merge checks it still fails on windows and Debian. https://buildkite.com/llvm-project/premerge-checks/builds/81130#ce8f3062-699e-4043-aed9-b891ec8ebee6 https://buildkite.com/llvm-project/premerge-checks/builds/81130#53beb4a2-2c53-4ff0-b60f-6130ed5d25c

[PATCH] D120310: [clang][analyzer] Add modeling of 'errno'.

steakhal added a comment. In D120310#3345696 , @balazske wrote: > The "hip-link-bundle-archive" test looks really unrelated, the others are > fixed if we go back to `-DERRNO_VAR` (no `"` characters in command line, and > probably `/` does not work too).

[PATCH] D120646: [clang][scan-build] Change mode of installation for scan-build.1

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Oh, that's unfortunate. Thanks for fixing it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120646/new/ https://reviews.llvm.org/D120646 __

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

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Herald added a subscriber: manas. Approved, given that it looks and compiles fine with oxygen. Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D120236: [analyzer] Add more sources to Taint analysis

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Please check the docs if they are still in sync with the content. I think it looks great overall. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

steakhal added a comment. You should copy the `mempcpy14`, `mempcpy15` test cases into a separate file, and cut the `top`; while disabling this `alpha.unix.cstring.UninitializedRead` checker in the existing test file(s) and explicitly enable in the new separated file. This way it wouldn't inter

[PATCH] D120369: [analyzer] Add more propagations to Taint analysis

steakhal accepted this revision. steakhal added a reviewer: NoQ. steakhal added a comment. This revision is now accepted and ready to land. Looks good to me. Check the docs if they are still in sync. Also, postpone this for two days to let others block this if they have objections. Other than tha

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

steakhal added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:377 + if (Access == AccessKind::read) { +if (StInBound->getSVal(ER).isUndef()) { + emitUninitializedReadBug(C, StInBound, Buffer.Expression); You should mak

[PATCH] D120646: [clang][scan-build] Change mode of installation for scan-build.1

This revision was automatically updated to reflect the committed changes. Closed by commit rG3ca109855709: [clang][scan-build] Change mode of installation for scan-build.1 (authored by manas, committed by steakhal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

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

steakhal added a comment. Please mark the done comments as "done" with the corresponding button prior to submitting an update. Comment at: clang/test/Analysis/bstring_UninitRead.c:67-85 +//===--=== +// mempcpy

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

steakhal added inline comments. Comment at: clang/test/Analysis/bstring_UninitRead.c:1-84 +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.cstring \ +// RUN: -analyzer-checker=alpha.unix.cstring \ +// RUN: -analyze

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

steakhal accepted this revision. steakhal added a comment. The `core` checkers should be always enabled. Comment at: clang/test/Analysis/bstring_UninitRead.c:2 +// RUN: %clang_analyze_cc1 -verify %s \ +// RUN: -analyzer-checker=alpha.unix.cstring + Reposito

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

steakhal added a comment. Yup, land it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120489/new/ https://reviews.llvm.org/D120489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

<    9   10   11   12   13   14   15   16   >