Page MenuHomePhabricator

Szelethus (Kristóf Umann)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 19 2017, 6:59 AM (167 w, 2 h)

Recent Activity

Yesterday

Szelethus added a comment to D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'..

I am not sure if this checker is worth to further development. A part of the found bugs can be detected with other checkers too, specially if the preconditions of many standard functions are checked (more than what is done now) and by modeling the function calls with assumed error return value.

Tue, Sep 29, 9:59 AM · Restricted Project

Mon, Sep 28

Szelethus added inline comments to D88312: "ErrorReturn checker" WIP review.
Mon, Sep 28, 6:01 AM · Restricted Project
Szelethus added a reviewer for D88332: [WIP][Analyzer] PtrToIntegCastLibcChecker: martong.

@martong has been working hard on libc support for the analyzer, I'll add him :)

Mon, Sep 28, 12:10 AM
Szelethus added inline comments to D88332: [WIP][Analyzer] PtrToIntegCastLibcChecker.
Mon, Sep 28, 12:09 AM

Fri, Sep 25

Szelethus added a comment to D87043: [Analyzer] Fix for dereferece of smart pointer after branching on unknown inner pointer.

Congrats on your GSoC! Unless I missed it, it seems like you haven't posted your final evaluation on cfe-dev, even though its an amazing looking document with a lot of examples and explanations. I think it would be great to spread the message, its a work to be proud of!

Fri, Sep 25, 3:00 AM · Restricted Project

Tue, Sep 22

Szelethus accepted D88100: [analyzer][StdLibraryFunctionsChecker] Separate the signature from the summaries.

Yup, we've talked about this before. This is indeed a better interface design. LGTM!

Tue, Sep 22, 10:30 PM · Restricted Project
Szelethus accepted D88092: [analyzer][StdLibraryFunctionsChecker] Fix getline/getdelim signatures.

A joy of reviewing C++ code is that you get to marvel in all the great things the language has, without having to pull fistfuls of hair out to get get to that point. These patches are always a treat. LGTM!

Tue, Sep 22, 10:27 PM · Restricted Project

Mon, Sep 21

Szelethus accepted D87942: [Analyzer] GNU named variadic macros in Plister.

LGTM! Thanks!

Mon, Sep 21, 11:41 AM · Restricted Project

Sun, Sep 20

Szelethus added a comment to D87942: [Analyzer] GNU named variadic macros in Plister.

The fix is great, thank you so much! The test seems to be annoying, it might be worth looking into it some other time, but that is definitely orthogonal to this patch.

Sun, Sep 20, 10:27 PM · Restricted Project
Szelethus added a comment to D71524: [analyzer] Support tainted objects in GenericTaintChecker.

I figured you're still working on this, sorry! I'd really like to chat about my earlier comment D71524#1917251, as it kind of challenges the high level idea.

Sun, Sep 20, 10:17 PM · Restricted Project

Wed, Sep 16

Szelethus added a comment to D87800: [WIP][Analyzer] find stack addresses leaked via out-params.

There are some immediate observation to the patch, like a new kind of check definitely warrants a different warning message. However, I see that this is WIP patch, so I'll try to keep this high level.

Wed, Sep 16, 10:45 PM

Tue, Sep 15

Szelethus added a comment to D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite.

Post-commit LGTM on the post-commit changes!

Tue, Sep 15, 10:11 PM · Restricted Project
Szelethus committed rGdd1d5488e47d: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing… (authored by Szelethus).
[analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing…
Tue, Sep 15, 8:45 AM
Szelethus closed D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist.
Tue, Sep 15, 8:45 AM · Restricted Project
Szelethus committed rG7c6f5b7fbf5a: [analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock (authored by Szelethus).
[analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock
Tue, Sep 15, 7:58 AM
Szelethus closed D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock.
Tue, Sep 15, 7:57 AM · Restricted Project
Szelethus closed D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.
Tue, Sep 15, 7:52 AM · Restricted Project
Szelethus added a comment to D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.

Sorry for the slack (which is kind of ironic with my attention grabbing title :) ). Landed in rG791b7e9f73e0064153a7c3db8045a7333a8c390c. Thanks everyone!

Tue, Sep 15, 7:52 AM · Restricted Project

Mon, Sep 14

Szelethus accepted D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite.

@balazske may have some closing words.

Mon, Sep 14, 10:39 PM · Restricted Project

Fri, Sep 11

Szelethus requested review of D87519: [analyzer][Liveness][NFC] Enqueue the CFGBlocks post-order.
Fri, Sep 11, 9:12 AM · Restricted Project
Szelethus updated the summary of D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment.
Fri, Sep 11, 9:04 AM · Restricted Project
Szelethus requested review of D87518: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment.
Fri, Sep 11, 9:02 AM · Restricted Project
Szelethus updated the diff for D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist.

Rename the live statements checker to live expressions checker. The test file added in a revert commit changed rather heavily, but it makes sense that these entries are removed IMO. Unless somebody objects, I intend to land this as-is.

Fri, Sep 11, 8:40 AM · Restricted Project
Szelethus committed rGb9bca883c970: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a… (authored by Szelethus).
[analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a…
Fri, Sep 11, 6:59 AM
Szelethus closed D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait.
Fri, Sep 11, 6:59 AM · Restricted Project
Szelethus updated the diff for D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait.

Reinstate the assert removed in rG032b78a0762bee129f33e4255ada6d374aa70c71, add a test. Enforce that Environment only makes an exception for ReturnStmt in terms of non-expression statements (and hope that isn't going to stay that way for long).

Fri, Sep 11, 6:43 AM · Restricted Project
Szelethus committed rGbe0d79f32930: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to… (authored by Szelethus).
[analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to…
Fri, Sep 11, 5:08 AM
Szelethus closed D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.
Fri, Sep 11, 5:08 AM · Restricted Project
Szelethus committed rG7527898fef47: [analyzer][MacroExpansion][NFC] Fix a missing test output check (authored by Szelethus).
[analyzer][MacroExpansion][NFC] Fix a missing test output check
Fri, Sep 11, 4:49 AM
Szelethus committed rG26d9a9468105: [analyzer][MacroExpansion][NFC] Fix incorrectly calling parameters arguments (authored by Szelethus).
[analyzer][MacroExpansion][NFC] Fix incorrectly calling parameters arguments
Fri, Sep 11, 4:35 AM
Szelethus committed rG1c08da38676d: [analyzer][MacroExpansion] Add a few dumps functions (authored by Szelethus).
[analyzer][MacroExpansion] Add a few dumps functions
Fri, Sep 11, 4:32 AM

Thu, Sep 10

Szelethus added a comment to D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait.
In D86736#2257880, @NoQ wrote:

i'm very glad that we now consistently use expressions in our Environment.

Thu, Sep 10, 2:09 AM · Restricted Project
Szelethus accepted D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics.

What if we'd add a toString method to the constraints and we'd add this to Msg? This way we'd know the contents of the constraint, thus we we'd know how the constraint is violated.

I mean we'd know what is not satisfied. But, to know why exactly that is not satisfied we should dump the whole State but that's obviously not an option. Perhaps we could track which symbols and expressions are participating in the assumption related to the constraint and we could dump only those, but this seems to be a very complex approach.

Thu, Sep 10, 12:41 AM · Restricted Project

Wed, Sep 9

Szelethus accepted D83678: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator.

LGTM on my end!

Wed, Sep 9, 7:26 AM · Restricted Project
Szelethus retitled D83678: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator from [analyzer] Move ReturnPtrRange checker out of alpha by fixing a false positive on end() iterator to [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator.
Wed, Sep 9, 7:25 AM · Restricted Project

Mon, Sep 7

Szelethus added a comment to D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.

Perfectly clear, thank you. However, I would still rely on the others to accept this :|

BTW why does the plist-macros-with-expansion.cpp.plist change? It makes the diff somewhat noisy :s

Mon, Sep 7, 8:33 AM · Restricted Project
Szelethus accepted D87240: [analyzer][StdLibraryFunctionsChecker] Have proper weak dependencies.

This is exactly how I imagines weak dependencies to work. LGTM on my end.

Mon, Sep 7, 8:30 AM · Restricted Project
Szelethus updated the diff for D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.

Added some documentation to the code snippet pointed out by @martong.

Mon, Sep 7, 7:51 AM · Restricted Project
Szelethus added inline comments to D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.
Mon, Sep 7, 6:46 AM · Restricted Project
Szelethus updated the diff for D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.

Added a line about D78933.

Mon, Sep 7, 6:46 AM · Restricted Project
Szelethus added a comment to D86874: [analyzer] Fix ArrayBoundCheckerV2 false positive regarding size_t indexer.

I can only imagine how long it took for you to write all this, because reading it wasn't that short either. Thank you so much! It really gave me a perspective on how you see this problem, as well as what is actually happening (and should happen) in the checker.

Mon, Sep 7, 6:02 AM · Restricted Project
Szelethus added a reviewer for D87118: Add an explicit toggle for the static analyzer in clang-tidy: NoQ.

@NoQ in particular has been working hard on the common infrastructure in between the static analyzer and clang-tidy, I'll add him :)

Mon, Sep 7, 3:26 AM · Restricted Project
Szelethus added a comment to D76590: [Analyzer] Model `empty()` member function of containers.

I'm in favor of most, if not all of the changes, though I will admit that this patch seems pretty cluttered, you are doing a lot of refactoring under the same hood. You're moving, adding, removing and changing helper functions and their invocations. Would be possible to make this patch a bit leaner?

Mon, Sep 7, 3:19 AM · Restricted Project
Szelethus accepted D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions.

@balazske seems to be very involved, he might have some closing words -- from my end, LGTM!

Mon, Sep 7, 2:30 AM · Restricted Project
Szelethus added a comment to D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers.

The tests look great, thanks! I still lack the confidence to accept, unfortunately.

Mon, Sep 7, 1:24 AM · Restricted Project
Szelethus added a comment to D87081: [analyzer][StdLibraryFunctionsChecker] Elaborate the summary of fread and fwrite.

The patch looks great, in fact, it demonstrates how well thought out your summary crafting machinery is.

Mon, Sep 7, 1:20 AM · Restricted Project

Wed, Sep 2

Szelethus accepted D87004: [analyzer] Evaluate PredefinedExpressions.

LGTM! Nice!

Wed, Sep 2, 2:51 AM · Restricted Project
Szelethus added a comment to D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'..

The summary of this last discussion is that it is not acceptable to have only the simple check for the explicit comparison with a fixed constant. At least not for return types where the "implicit" check (a check that is always true or false for the error return value) is possible, for example the char* case.

Wed, Sep 2, 2:42 AM · Restricted Project

Tue, Sep 1

Szelethus added a comment to D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait.

I don't have anymore immediate concerns, but I will need more time to comb through the rest of the patch in more details... hopefully I can do that in the following days.

Tue, Sep 1, 10:03 AM · Restricted Project
Szelethus added inline comments to D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait.
Tue, Sep 1, 5:01 AM · Restricted Project
Szelethus added a comment to D86873: [analyzer][NFC] Refactor ArrayBoundCheckerV2.

Before we dive into this too much, if you can point to discussion that explains why we have a 2 versions of the same checker, that would be nice. Why did you chose to work on this one, and not the other? I recall us talking about this in a meeting, but its always great to have it set in stone.

Tue, Sep 1, 1:54 AM · Restricted Project
Szelethus updated the summary of D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.
Tue, Sep 1, 1:49 AM · Restricted Project
Szelethus added a comment to D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.

I'd prefer to have a couple more green lights. In particular, another look on the constraint manager and Objective C stuff would be great.

Tue, Sep 1, 1:35 AM · Restricted Project

Aug 31 2020

Szelethus added a comment to D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers.

Shouldn't we create a new test care for this, instead of expanding an existing one? Btw, this looks great, but I lack the confidence to accept.

Why should we? This is just a fix for cases not covered, but it is the same functionality (retrieving the return value under construction). I added the missed cases to the test of this exact functionality.

Aug 31 2020, 2:43 AM · Restricted Project
Szelethus added a comment to D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait.

For any other loops, in order to know whether we should analyze another iteration, among other things, we evaluate it's condition. Which is a problem for ObjCForCollectionStmt, because it simply doesn't have one

Shouldn't we try to fix the ObjCForCollectionStmt in the AST rather? By adding the condition to that as a (sub)expression? There are already many discrepancies and inconsistencies between supposedly similar AST nodes, and people do fix them occasionally (e.g. D81787).

Aug 31 2020, 1:26 AM · Restricted Project
Szelethus added a comment to D86743: [analyzer] Ignore VLASizeChecker case that could cause crash.

I'm not thrilled by this solution. As I understand it, the assertion was put there to enforce our ability to create a new assumption, and its great that we had it, because we managed to catch a fault in that. Seems like this patch is treating the symptoms instead of curing the illness. Given that the code would probably work fine in builds with assertions disabled, I'd prefer to take a look at where the actual problem is. If it turns out to be unsolvable or require unreasonable amount of work, we can still decide to land this as-is.

Aug 31 2020, 12:15 AM · Restricted Project

Aug 27 2020

Szelethus added inline comments to D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.
Aug 27 2020, 2:00 PM · Restricted Project
Szelethus updated the diff for D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.

Fixes according to reviewer comments!

Aug 27 2020, 1:59 PM · Restricted Project
Szelethus requested review of D86736: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait.
Aug 27 2020, 1:25 PM · Restricted Project
Szelethus accepted D86691: [analyzer] Fix wrong parameter name in printFormattedEntry.

Nice, thank you! Did you stumble across this, or found it with a tool?

Aug 27 2020, 2:13 AM · Restricted Project

Aug 26 2020

Szelethus added inline comments to D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.
Aug 26 2020, 4:59 AM · Restricted Project
Szelethus updated the diff for D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.

Fixes according to reviewer comments!

Aug 26 2020, 4:59 AM · Restricted Project
Szelethus added a reviewer for D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes: ASDenysPetrov.
Aug 26 2020, 4:16 AM · Restricted Project
Szelethus requested changes to D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'..

I debated this report with @bruntib, depending from where you look at it, its either a false positive or not -- in short, this is what happens:

char *p = /* some assumed to be valid c string */
end = strchr(p, '\n');
if (end - p > 4) { // <-- warning here on end's invalid use
}

the guarded code wouldn't be executed, should end be zero. Now, if we interpret the rule in a very literal sense, that

Aug 26 2020, 4:10 AM · Restricted Project

Aug 25 2020

Szelethus added a comment to D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.

I'll get the nits I didn't object to fixed, thats the status you're looking for :)

Aug 25 2020, 8:45 AM · Restricted Project
Szelethus added a comment to D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.

@vsavchenko I just realized a significant portion of your work for this release is the following list:

git log llvmorg-11-init..llvmorg-11.0.0-rc2 --oneline -- clang/utils/analyzer/

Is it a correct guess that while your primary audience are the analyzer developers, it wouldn't hurt to mention it in the release notes?

Aug 25 2020, 5:23 AM · Restricted Project
Szelethus added a reviewer for D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock: hans.

We will need to push this to the 11.0.0. release branch as well, sorry for the trouble.

Aug 25 2020, 5:20 AM · Restricted Project
Szelethus requested review of D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes.
Aug 25 2020, 5:18 AM · Restricted Project
Szelethus requested review of D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock.
Aug 25 2020, 5:06 AM · Restricted Project
Szelethus planned changes to D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.

Thanks! I'll get these fixed.

Aug 25 2020, 4:15 AM · Restricted Project
Szelethus added a comment to D73376: [analyzer] Add FuchsiaLockChecker and C11LockChecker.

Documentation under clang/docs/analyzer/checkers.rst seems to be missing. Could we get that fixed before 11.0.0 releases?

Aug 25 2020, 3:29 AM · Restricted Project
Szelethus added a comment to D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.

The fundamental problem is, we simply can't ask Preprocessor what a macro expands into without hacking really hard.

Can you summarize what is the exact problem (or give a link to a discussion, etc)? Is it an architectural problem in Clang itself?

Aug 25 2020, 1:28 AM · Restricted Project

Aug 24 2020

Szelethus accepted D86446: [analyzer][docs][NFC] Document the ento namespace in the llvm/Lexicon.

What a legacy.

Aug 24 2020, 5:49 AM · Restricted Project
Szelethus added a comment to D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting.

Do I sense correctly that the only information CSrtingLengthModeling.cpp requires from the actual CStringChecker is a checker tag? Because if so, I think we should just separate them even more cleanly -- we could just make a CStringLengthModeling checker implement the checker callbacks in that cpp file and we wouldn't need to move CStringChecker to a header. Not that moving it there is fundamentally bad, but in this instance it seems like we're legalizing bloating checkers instead of separating them. Also, this patch seems to have a significant overlap with D84979 -- is this intentional?

Aug 24 2020, 5:28 AM · Restricted Project
Szelethus added a comment to D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size.

I really like the patch, but have nothing to add to what other reviewers already said. Nice!

Aug 24 2020, 3:58 AM · Restricted Project

Aug 18 2020

Szelethus added inline comments to D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.
Aug 18 2020, 7:39 AM · Restricted Project
Szelethus requested review of D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__.
Aug 18 2020, 5:29 AM · Restricted Project

Aug 14 2020

Szelethus added a comment to D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis..
In D67422#2216137, @NoQ wrote:

I agree that there's something here and also that it's not that urgent/critical to rename things at this point. It's not that people suffer horribly from having to link to only some of these things.

Aug 14 2020, 11:37 AM · Restricted Project
Szelethus accepted D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis..

Unless anyone else objects, I'm happy to see this landed! Nice work! ^-^

Aug 14 2020, 11:36 AM · Restricted Project

Aug 13 2020

Szelethus added inline comments to D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis..
Aug 13 2020, 5:02 PM · Restricted Project
Szelethus added a comment to D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics.

Ah, okay, silly me. Didn't catch that. Then the message is OK for now.

Aug 13 2020, 6:22 AM · Restricted Project
Szelethus added a comment to D85351: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers.

Shouldn't we create a new test care for this, instead of expanding an existing one? Btw, this looks great, but I lack the confidence to accept.

Aug 13 2020, 6:05 AM · Restricted Project
Szelethus accepted D84248: [analyzer][StdLibraryFunctionsChecker] Add POSIX time handling functions.

Lets make sure we invite the wider community to see whats going on. Otherwise, LGTM!

Aug 13 2020, 5:59 AM · Restricted Project
Szelethus requested changes to D79431: [analyzer] StdLibraryFunctionsChecker: Add better diagnostics.

Tests c:

Aug 13 2020, 4:55 AM · Restricted Project
Szelethus added a comment to D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis..

The patch looks great. I guess the only remaining discussion remains as to whether this should be in libAnalysis, or somewhere else. Here is my take: Since clang-tidy, CSA, and some other parts of the compiler (like uninitialized variable warnings) are static analyzers within the same infrastructure, it makes sense for them to have a common library. I see that diagnostics aren't really analyses. libFrontend or libDriver, which, as I understand it contains most the diagnostics machinery for clang aren't viable alternatives because of the library dependencies. So, I think if libAnalysis was called libStaticAnalysisCommon, we would be golden, but that would screw over downstream developers for negligible gain. While I'm not terribly experiences in library layout design, for the time being, the move to libAnalysis seems appropriate.

Aug 13 2020, 4:43 AM · Restricted Project

Aug 7 2020

Szelethus added a comment to D84600: [Analyzer] Support note tags for smart ptr checker.

Layering violations are a running theme in the analyzer -- CheckerRegistry and the entire MallocChecker fiasco are two glaring examples. Luckily, this isn't a severe case so I wouldn't worry about it much.

Aug 7 2020, 12:43 PM · Restricted Project
Szelethus added a comment to D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'..

Test results for tmux are available here. (Could not manage to get other results uploaded.)

Aug 7 2020, 8:20 AM · Restricted Project

Aug 6 2020

Szelethus added a comment to D85424: [Analyzer] Crash fix for alpha.cplusplus.IteratorRange.

Yeah, this looks straightforward, but how come you didn't manage to get a small test case? creduce gave up?

Aug 6 2020, 6:27 AM · Restricted Project

Aug 4 2020

Szelethus accepted D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling.

LGTM on my end, but please wait for approval from either @gamesh411 or @NoQ.

Aug 4 2020, 7:53 AM · Restricted Project
Szelethus added a comment to D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'..

Would it be possible to publish these results on a public CodeChecker server?

Aug 4 2020, 3:55 AM · Restricted Project
Szelethus added a comment to D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker..

We definitely need more tests. The idea overall however is amazing, thanks!

Aug 4 2020, 3:29 AM · Restricted Project
Szelethus added a comment to D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis..
In D67422#2192407, @NoQ wrote:

One bit that's not directly related but i decided to keep it from the original patch was moving the plist-html diagnostic builder function into its own file.

Aug 4 2020, 2:59 AM · Restricted Project

Jul 28 2020

Szelethus requested changes to D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling.

Let me double down -- just because we let some select people know about an option, I still don't think it should be user facing. I'm strongly against the philosophy that a core-modifying decision, such as configuring a state split should be presented on the default interface. Having "smart users" isn't an excuse to that either -- the last thing I'd like to see broadcasted is that you need to be a genius* to use or configure a tool that is suppose to serve you, not the other way around.

Jul 28 2020, 5:29 AM · Restricted Project
Szelethus accepted D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC)..

Lets make sure that we invite others from the community to participate, here and on other patches as well. Otherwise, LGTM.

Jul 28 2020, 1:13 AM · Restricted Project

Jul 24 2020

Szelethus added a comment to D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist.
In D82598#2172416, @NoQ wrote:

I still wonder what made this statement live in my example. There must have been some non-trivial liveness analysis going on that caused a statement to be live; probably something to do with the C++ destructor elements.

Jul 24 2020, 12:36 PM · Restricted Project
Szelethus committed rG032b78a0762b: [analyzer] Revert the accidental commit of D82122 (authored by Szelethus).
[analyzer] Revert the accidental commit of D82122
Jul 24 2020, 12:34 PM
Szelethus added a comment to D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist.

Wow, I never realized I accidentally landed that assert (D82122#2172360), but I guess its great to have that covered. Would you prefer to have that reverted as I'm looking to fix this for good?

Jul 24 2020, 8:20 AM · Restricted Project
Szelethus added a comment to D82122: [analyzer][Liveness][RFC][NFC] Propose to turn statement liveness into expression liveness.

This was accidentally committed as a part of rGb6cbe6cb0399d4671e5384dcc326af56bc6bd122.

Jul 24 2020, 8:14 AM · Restricted Project
Szelethus added a comment to D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist.

Thank you so much, you really went out of your way to dig that out! I'll try to patch this somehow.

Jul 24 2020, 8:04 AM · Restricted Project
Szelethus added a comment to D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting.
In D84316#2171270, @NoQ wrote:

Imagine something like re-using the state trait implementation between MallocChecker and StreamChecker because they both model "resources that can be deallocated twice or leaked" - regardless of the specific nature of these resources. These checkers can implement their own API modeling maps, escape rules, warning messages, maybe model additional aspects of their problems, but fundamentally they're solving the same problem: finding leaks and overreleases of resources. This problem should ideally be solved once. This is why i advocate for abstract, generalized, "half-baked" state trait boilerplate implementations that can be re-used across checkers.

Jul 24 2020, 4:41 AM · Restricted Project