ziqingluo-90 updated this revision to Diff 488429.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141338/new/
https://reviews.llvm.org/D141338
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/unittests/Analysis/CMakeLists.txt
clang/unittests/Analysis/UnsafeBufferUsageTest.cpp
Index: clang/unittests/Analysis/UnsafeBufferUsageTest.cpp
===================================================================
--- /dev/null
+++ clang/unittests/Analysis/UnsafeBufferUsageTest.cpp
@@ -0,0 +1,60 @@
+#include "gtest/gtest.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
+
+using namespace clang;
+
+namespace {
+// The test fixture.
+class UnsafeBufferUsageTest : public ::testing::Test {
+protected:
+ UnsafeBufferUsageTest()
+ : FileMgr(FileMgrOpts), DiagID(new DiagnosticIDs()),
+ Diags(DiagID, new DiagnosticOptions, new IgnoringDiagConsumer()),
+ SourceMgr(Diags, FileMgr) {}
+
+ FileSystemOptions FileMgrOpts;
+ FileManager FileMgr;
+ IntrusiveRefCntPtr<DiagnosticIDs> DiagID;
+ DiagnosticsEngine Diags;
+ SourceManager SourceMgr;
+};
+} // namespace
+
+TEST_F(UnsafeBufferUsageTest, FixItHintsConflict) {
+ const FileEntry *DummyFile = FileMgr.getVirtualFile("<virtual>", 100, 0);
+ FileID DummyFileID = SourceMgr.getOrCreateFileID(DummyFile, SrcMgr::C_User);
+ SourceLocation StartLoc = SourceMgr.getLocForStartOfFile(DummyFileID);
+
+#define MkDummyHint(Begin, End) \
+ FixItHint::CreateReplacement(SourceRange(StartLoc.getLocWithOffset((Begin)), \
+ StartLoc.getLocWithOffset((End))), \
+ "dummy")
+
+ FixItHint H1 = MkDummyHint(0, 5);
+ FixItHint H2 = MkDummyHint(6, 10);
+ FixItHint H3 = MkDummyHint(20, 25);
+ llvm::SmallVector<FixItHint> Fixes;
+
+ // Test non-overlapping fix-its:
+ Fixes = {H1, H2, H3};
+ EXPECT_FALSE(anyConflict(SourceMgr, Fixes));
+ Fixes = {H3, H2, H1}; // re-order
+ EXPECT_FALSE(anyConflict(SourceMgr, Fixes));
+
+ // Test overlapping fix-its:
+ Fixes = {H1, H2, H3, MkDummyHint(0, 4) /* included in H1 */};
+ EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+
+ Fixes = {H1, H2, H3, MkDummyHint(10, 15) /* overlaps H2 */};
+ EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+
+ Fixes = {H1, H2, H3, MkDummyHint(7, 23) /* overlaps H2, H3 */};
+ EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+
+ Fixes = {H1, H2, H3, MkDummyHint(2, 23) /* overlaps H1, H2, and H3 */};
+ EXPECT_TRUE(anyConflict(SourceMgr, Fixes));
+}
\ No newline at end of file
Index: clang/unittests/Analysis/CMakeLists.txt
===================================================================
--- clang/unittests/Analysis/CMakeLists.txt
+++ clang/unittests/Analysis/CMakeLists.txt
@@ -9,6 +9,7 @@
CloneDetectionTest.cpp
ExprMutationAnalyzerTest.cpp
MacroExpansionContextTest.cpp
+ UnsafeBufferUsageTest.cpp
)
clang_target_link_libraries(ClangAnalysisTests
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -109,7 +109,7 @@
AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
-
+
MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All);
return Visitor.findMatch(DynTypedNode::create(Node));
}
@@ -690,6 +690,37 @@
}
}
+bool clang::anyConflict(const SourceManager &SM,
+ const llvm::SmallVectorImpl<FixItHint> &FixIts) {
+ // A simple interval overlap detecting algorithm. Sorts all ranges by their
+ // begin location then find any overlap in one pass.
+ std::vector<const FixItHint *> All; // a copy of `FixIts`
+
+ for (const FixItHint &H : FixIts)
+ All.push_back(&H);
+ std::sort(All.begin(), All.end(),
+ [&SM](const FixItHint *H1, const FixItHint *H2) {
+ return SM.isBeforeInTranslationUnit(H1->RemoveRange.getBegin(),
+ H2->RemoveRange.getBegin());
+ });
+
+ const FixItHint *CurrHint = nullptr;
+
+ for (const FixItHint *Hint : All) {
+ if (!CurrHint ||
+ SM.isBeforeInTranslationUnit(CurrHint->RemoveRange.getEnd(),
+ Hint->RemoveRange.getBegin())) {
+ // Either to initialize `CurrHint` or `CurrHint` does not
+ // overlap with `Hint`:
+ CurrHint = Hint;
+ } else
+ // In case `Hint` overlaps the `CurrHint`, we found at least one
+ // conflict:
+ return true;
+ }
+ return false;
+}
+
void clang::checkUnsafeBufferUsage(const Decl *D,
UnsafeBufferUsageHandler &Handler) {
assert(D && D->getBody());
@@ -777,9 +808,7 @@
Fixes = std::nullopt;
break;
}
-
- for (auto &&Fixit: *F)
- Fixes->push_back(std::move(Fixit));
+ Fixes->insert(Fixes->end(), F->begin(), F->end());
}
}
@@ -789,7 +818,8 @@
Fixes->insert(Fixes->end(), std::make_move_iterator(VarF.begin()),
std::make_move_iterator(VarF.end()));
// If we reach this point, the strategy is applicable.
- Handler.handleFixableVariable(VD, std::move(*Fixes));
+ if (!anyConflict(Ctx.getSourceManager(), *Fixes))
+ Handler.handleFixableVariable(VD, std::move(*Fixes));
} else {
// The strategy has failed. Emit the warning without the fixit.
S.set(VD, Strategy::Kind::Wontfix);
Index: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
===================================================================
--- clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -42,6 +42,10 @@
// through the handler class.
void checkUnsafeBufferUsage(const Decl *D, UnsafeBufferUsageHandler &Handler);
+// Tests if any two `FixItHint`s in `FixIts` conflict. Two `FixItHint`s
+// conflict if they have overlapping source ranges.
+bool anyConflict(const SourceManager &SM, const llvm::SmallVectorImpl<FixItHint> &FixIts);
+
/// The text indicating that the user needs to provide input there:
constexpr static const char *const UserFillPlaceHolder = "...";
} // end namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
