This revision was automatically updated to reflect the committed changes.
Closed by commit rC339135: [analyzer][UninitializedObjectChecker] New flag to
turn off dereferencing (authored by Szelethus, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D49438
Files:
Szelethus updated this revision to Diff 159486.
Szelethus added a comment.
Added the TODO we were discussing.
https://reviews.llvm.org/D49438
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object-inheritance.cpp
Szelethus added a comment.
I'll be back in office tomorrow, about ~13 hours later. But feel free to commit
it on my behalf if it's urgent.
https://reviews.llvm.org/D49438
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
george.karpenkov added a comment.
@Szelethus in any case, could you commit this change for now? I'm very keen on
seeing the results on a few of our internal projects, and it's much easier to
do that once the change is committed.
https://reviews.llvm.org/D49438
Szelethus added a comment.
If we ignore references, check whether the pointee was constructed within the
constructor, and maybe add some other clever heuristics, I'm very much in favor
of enabling pointer chasing by enabled. But I totally support disabling it for
now.
Szelethus added a comment.
In https://reviews.llvm.org/D49438#1189772, @george.karpenkov wrote:
> > I think what pointer chasing should do, is check whether that pointer owns
> > the pointee
>
> But ownership is a convention, and it's not always deducible from a codebase.
How about the
george.karpenkov added a comment.
> I think what pointer chasing should do, is check whether that pointer owns
> the pointee
But ownership is a convention, and it's not always deducible from a codebase.
I think of those as two separate checks, and I think we should only talk about
enabling the
Szelethus added a comment.
I think what pointer chasing should do, is check whether that pointer owns the
pointee. In that case, it should be fine to analyze it. Do you mind if I put a
TODO around flag's description stating that this should be implemented and
pointer chasing should be enabled
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.
Great, thanks a lot!
https://reviews.llvm.org/D49438
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Szelethus updated this revision to Diff 159291.
Szelethus added a comment.
Fixed the comments.
https://reviews.llvm.org/D49438
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object-inheritance.cpp
Szelethus marked 2 inline comments as done.
Szelethus added a comment.
It looks like I won't be back in office until Monday, so I can't finish this
one until then :(
>> but I haven't seen the current version of the checker crash due to pointer
>> chasing.
>
> @NoQ saw quite a few crashes
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
Great! The comment should be updated though.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:32
+// `-analyzer-config \
george.karpenkov added a comment.
> Dereferencing is disabled by default
Great! Thank you!
> but I haven't seen the current version of the checker crash due to pointer
> chasing.
@NoQ saw quite a few crashes during evaluation, right?
> This might be a little nit-picking, but I don't think
Szelethus updated this revision to Diff 158760.
Szelethus added a comment.
Forgot `git add` and `-U`.
https://reviews.llvm.org/D49438
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object-inheritance.cpp
Szelethus added a comment.
In https://reviews.llvm.org/D49438#1184688, @george.karpenkov wrote:
> @Szelethus Hi, do you plan to finish this patch soon-ish? I would like to
> evaluate it on a few codebases, but the pointer chasing is currently way too
> fragile / generates too many FPs.
What
Szelethus marked an inline comment as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:715
void ento::registerUninitializedObjectChecker(CheckerManager ) {
auto Chk = Mgr.registerChecker();
Chk->IsPedantic =
Szelethus updated this revision to Diff 158749.
Szelethus edited the summary of this revision.
Szelethus added a comment.
Dereferencing is disabled by default. I think there's a great value in this
part of the checker, but I do concede to the fact that it still needs to mature
even for alpha.
george.karpenkov added a comment.
@Szelethus Hi, do you plan to finish this patch soon-ish? I would like to
evaluate it on a few codebases, but the pointer chasing is currently way too
fragile / generates too many FPs.
Repository:
rC Clang
https://reviews.llvm.org/D49438
george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:715
void ento::registerUninitializedObjectChecker(CheckerManager )
Szelethus added a comment.
I left a comment on this issue already:
https://reviews.llvm.org/D49437#1165462. But in short, definitely yes! It's
been very painful to work around pointer/reference objects, too bad I didn't
come up with this idea sooner :)
Repository:
rC Clang
george.karpenkov added a comment.
@Szelethus false positives are a single biggest problem of the analyzer.
By a *huge* margin, most projects would prefer to err on the side of less, more
precise, warnings.
Given that currently in my understanding no actual bugs we are sure about were
found by
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, rnkovacs, xazax.hun.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet,
whisperity.
The idea came from both @george.karpenkov
(https://reviews.llvm.org/D45532#1145592) and from bugzilla
22 matches
Mail list logo
