Page MenuHomePhabricator

[analyzer][UninitializedObjectChecker] Mark uninitialized regions as interesting.
Changes PlannedPublic

Authored by Szelethus on Jun 5 2019, 4:28 AM.

Diff Detail

Event Timeline

Szelethus created this revision.Jun 5 2019, 4:28 AM

Looks much better than the original one, but why did the warning move to the correct place just because marking the region as interesting?

Ping, anything against this? :)

NoQ added a comment.Jun 14 2019, 5:36 PM

I don't remember what exactly does markInteresting() do and these tests don't really convince me that it does anything at all. Halp? >.<

In D62899#1544630, @NoQ wrote:

I don't remember what exactly does markInteresting() do and these tests don't really convince me that it does anything at all. Halp? >.<

Damnit, forgot my actual test file in the office (did not git add it).

I wanted to just say "Well markInteresting() is about the same as trackExpressionValue()!", and while I suspect that there's some truth to that, honestly, it's explained extremely poorly in the code. As I understand it, interesting symbols and expressions are used in the pruning of the bug path, namely, if a you'd like to prune out B->C->D from bugpath A->B->C->D->E, but C is interesting, no pruning will take place.

I particularly liked your description of BugReporter.cpp as a "military grade portion of spaghetti", because thats pretty much all I could figure out. I'll try harder though, because it seems to be an important part of the bug report construction. @xazax.hun, you seem to be a lot more confident with this, is what I'm saying more or less correct?

Szelethus updated this revision to Diff 205511.EditedJun 18 2019, 11:45 PM

Added a proper testfile. The only downside of it is that it doesn't test anything. Literally nothing would change if I didn't mark the fields interesting. I'll take this back to the drawing board.

Szelethus planned changes to this revision.Jun 18 2019, 11:46 PM
NoQ added a comment.Jun 19 2019, 5:20 PM

Added a proper testfile. The only downside of it is that it doesn't test anything.

Use creduce!

In D62899#1551312, @NoQ wrote:

Added a proper testfile. The only downside of it is that it doesn't test anything.

Use creduce!

I would, but I'm not even sure what to look for, really.

Okay, I have a far better understanding of interestingness, so here's the deal: Visitors, like ConditionBRVisitor, will mark its diagnostics non-prunable if it describes an interesting entity. The problem is, for an uninitialized variable, everything that is worth explaining is probably unexplored by the analyzer, leaving only to the note "Declaring 'x' without an initial value", which we don't emit for C++ constructors.

That said, I'd like ReturnVisitor to step up and place the notes it usually does. Even better, I'd like to use trackRegionValue() to add our beloved selection of visitors, so its a real shame it doesn't exist :^) Since we mark tracked values as interesting anyways, that would be a better solution then this.

NoQ added a comment.Aug 20 2019, 1:35 PM
In D62899#1551312, @NoQ wrote:

Added a proper testfile. The only downside of it is that it doesn't test anything.

Use creduce!

I would, but I'm not even sure what to look for, really.

When you already have code and have a real-world example that it works but you don't have a reduced test case, just use "anything changes" as the reduce condition, because you're basically interested in the answer to "why does this patch change anything at all?".