[PATCH] D57890: [analyzer] Fix in self assignment checker
baloghadamsoftware added a comment. Herald added subscribers: ASDenysPetrov, martong, steakhal. I think you can create a unit test for this: create a pre-call checker that checks for the assignment operator and asserts that we are not in top level. Create a test code with a simple class without user provided copy operator and a function that uses the auto-generated copy operator. Run the checker on this sample code. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57890/new/ https://reviews.llvm.org/D57890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57890: [analyzer] Fix in self assignment checker
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. Herald added subscribers: Charusso, jdoerfert, whisperity. Yup, I agree. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57890/new/ https://reviews.llvm.org/D57890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57890: [analyzer] Fix in self assignment checker
NoQ added a comment. Sounds reasonable, but it also sounds like something that should be reproducible on the upstream clang. Do you have a code snippet that causes the problematic AST to appear? Even if we don't have the false positive up here in upstream, Is it something we can test via `-analyzer-display-progress | FileCheck` or with the help of the analysis order checker or something like that? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57890/new/ https://reviews.llvm.org/D57890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57890: [analyzer] Fix in self assignment checker
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Just wanted to make sure I get it right. You did not add a test since it is only reproducible with an internal (non-upstreamed) checker. Since the change is trivial, I think it is ok to commit this without a test. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57890/new/ https://reviews.llvm.org/D57890 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57890: [analyzer] Fix in self assignment checker
bruntib created this revision.
bruntib added reviewers: NoQ, george.karpenkov, Szelethus, xazax.hun,
baloghadamsoftware.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet.
Herald added a project: clang.
For self assignment checker it was necessary to force checking of assignment
operators even if those are not called. The reason of this is to check whether
"this" is equal to the address of the assignee object.
The buffer overlap checker checks if the intervals of the arguments of a
memcpy() call are disjoint. If a class has an array member then the compiler
generated assignment operator copies it with memcpy() function without checking
self assignment at the beginning. Since the analyzer forces the check of
assignment operators, the buffer overflow checker reported a false positive on
classes with compiler generated assignment operator and array member.
This commit prevents the forced check of compiler generated assignment
operators.
Repository:
rC Clang
https://reviews.llvm.org/D57890
Files:
lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -450,7 +450,7 @@
// where it may not. (cplusplus.SelfAssignmentChecker)
if (const auto *MD = dyn_cast(D)) {
if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator())
- return false;
+ return !MD->isUserProvided();
}
// Otherwise, if we visited the function before, do not reanalyze it.
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -450,7 +450,7 @@
// where it may not. (cplusplus.SelfAssignmentChecker)
if (const auto *MD = dyn_cast(D)) {
if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator())
- return false;
+ return !MD->isUserProvided();
}
// Otherwise, if we visited the function before, do not reanalyze it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
