alexfh added a comment.
Thank you, Artem!
Repository:
rL LLVM
https://reviews.llvm.org/D37023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314287: [analyzer] Fix and refactor
bugreporter::getDerefExpr() API. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D37023?vs=116673=116781#toc
Repository:
rL LLVM
dcoughlin accepted this revision.
dcoughlin added a comment.
LGTM! Thanks Artem.
https://reviews.llvm.org/D37023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ updated this revision to Diff 116673.
NoQ added a comment.
Add no-crash test cases from https://bugs.llvm.org/show_bug.cgi?id=34373 and
https://bugs.llvm.org/show_bug.cgi?id=34731 .
https://reviews.llvm.org/D37023
Files:
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
NoQ updated this revision to Diff 116542.
NoQ added a comment.
@dcoughlin: You're right, my reasoning and understanding was not correct, and
your explanation is much more clear. My code still makes sense to me though, so
i updated the comments to match. And moved the unusual logic for the
alexfh added a comment.
Any news here? I'm wondering mainly because this patch is supposed to fix
https://bugs.llvm.org/show_bug.cgi?id=34373.
https://reviews.llvm.org/D37023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
dcoughlin added a comment.
This seems like it is on the right track, but I think the emphasis here on
lvalue-to-rvalue casts is a bit misplaced. The logic for "what is the pointer
being dereferenced" and "what is the lvalue that holds the pointer being
dereferenced" is mixed together in a way
xazax.hun added a comment.
In https://reviews.llvm.org/D37023#853941, @NoQ wrote:
> Thank you for the review!
>
> > Though it looks like some of the cases covered in the code do not have
> > corresponding tests (e.g.: the parenexprs).
>
> These are covered by tests in
NoQ added a comment.
Thank you for the review!
> Though it looks like some of the cases covered in the code do not have
> corresponding tests (e.g.: the parenexprs).
These are covered by tests in `inline-defensive-checks.c:150,156,169,179` (old
code had `IgnoreParenCasts`). This function is
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Generally, it looks good to me. Though it looks like some of the cases covered
in the code do not have corresponding tests (e.g.: the parenexprs).
I think this approach is good in a
a.sidorin added a comment.
Sorry, missed that.
https://reviews.llvm.org/D37023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
NoQ added a comment.
Yeah, line 86.
https://reviews.llvm.org/D37023
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
a.sidorin added a comment.
Hi Artem,
I have a question after quick look. The original code considered `ParenExpr`s
but I cannot find nothing paren-related in the patch. Is case `(x->y).z`
handled as expected?
https://reviews.llvm.org/D37023
___
NoQ created this revision.
This patch continues work that was started in https://reviews.llvm.org/D32291.
Our `bugreporter::getDerefExpr()` API tries to find out what has been
dereferenced. For example, if we have an lvalue expression `x->y.z` which
causes a null dereference when dereferenced,
14 matches
Mail list logo
