[clang] 4ade8b7 - [AST] Fix bug in UnresolvedSet::erase of last element

[John Brawn via cfe-commits]

Author: John Brawn
Date: 2023-07-05T16:02:40+01:00
New Revision: 4ade8b7ed9976303b23cff3525223826e65b46e7

URL: https://github.com/llvm/llvm-project/commit/4ade8b7ed9976303b23cff3525223826e65b46e7
DIFF: https://github.com/llvm/llvm-project/commit/4ade8b7ed9976303b23cff3525223826e65b46e7.diff

LOG: [AST] Fix bug in UnresolvedSet::erase of last element

UnresolvedSet::erase works by popping the last element then replacing
the element to be erased with that element. When the element to be
erased is itself the last element this leads to writing past the end
of the set, causing an assertion failure.

Fix this by making erase of the last element just pop that element.

Differential Revision: https://reviews.llvm.org/D154502

Added: 
    clang/unittests/AST/UnresolvedSetTest.cpp

Modified: 
    clang/include/clang/AST/UnresolvedSet.h
    clang/unittests/AST/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/UnresolvedSet.h b/clang/include/clang/AST/UnresolvedSet.h
index 17b47f6ab96bee..ee31be969b6e35 100644
--- a/clang/include/clang/AST/UnresolvedSet.h
+++ b/clang/include/clang/AST/UnresolvedSet.h
@@ -114,9 +114,17 @@ class UnresolvedSetImpl {
     I.I->set(New, AS);
   }
 
-  void erase(unsigned I) { decls()[I] = decls().pop_back_val(); }
+  void erase(unsigned I) {
+    auto val = decls().pop_back_val();
+    if (I < size())
+      decls()[I] = val;
+  }
 
-  void erase(iterator I) { *I.I = decls().pop_back_val(); }
+  void erase(iterator I) {
+    auto val = decls().pop_back_val();
+    if (I != end())
+      *I.I = val;
+  }
 
   void setAccess(iterator I, AccessSpecifier AS) { I.I->setAccess(AS); }
 

diff  --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt
index b664b64070328e..12484be9206e23 100644
--- a/clang/unittests/AST/CMakeLists.txt
+++ b/clang/unittests/AST/CMakeLists.txt
@@ -35,6 +35,7 @@ add_clang_unittest(ASTTests
   StructuralEquivalenceTest.cpp
   TemplateNameTest.cpp
   TypePrinterTest.cpp
+  UnresolvedSetTest.cpp
   )
 
 clang_target_link_libraries(ASTTests

diff  --git a/clang/unittests/AST/UnresolvedSetTest.cpp b/clang/unittests/AST/UnresolvedSetTest.cpp
new file mode 100644
index 00000000000000..6c4d6db9092321
--- /dev/null
+++ b/clang/unittests/AST/UnresolvedSetTest.cpp
@@ -0,0 +1,115 @@
+#include "clang/AST/UnresolvedSet.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+class NamedDecl {
+  int dummy;
+
+public:
+  NamedDecl() {}
+};
+} // namespace clang
+
+using namespace clang;
+
+class UnresolvedSetTest : public ::testing::Test {
+protected:
+  NamedDecl n0, n1, n2, n3;
+  UnresolvedSet<2> set;
+
+  void SetUp() override {
+    set.addDecl(&n0);
+    set.addDecl(&n1);
+    set.addDecl(&n2);
+    set.addDecl(&n3);
+  }
+};
+
+TEST_F(UnresolvedSetTest, Size) { EXPECT_EQ(set.size(), 4u); }
+
+TEST_F(UnresolvedSetTest, ArrayOperator) {
+  EXPECT_EQ(set[0].getDecl(), &n0);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+  EXPECT_EQ(set[2].getDecl(), &n2);
+  EXPECT_EQ(set[3].getDecl(), &n3);
+}
+
+TEST_F(UnresolvedSetTest, EraseIntegerFromStart) {
+  set.erase(0);
+  EXPECT_EQ(set.size(), 3u);
+  EXPECT_EQ(set[0].getDecl(), &n3);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+  EXPECT_EQ(set[2].getDecl(), &n2);
+
+  set.erase(0);
+  EXPECT_EQ(set.size(), 2u);
+  EXPECT_EQ(set[0].getDecl(), &n2);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+
+  set.erase(0);
+  EXPECT_EQ(set.size(), 1u);
+  EXPECT_EQ(set[0].getDecl(), &n1);
+
+  set.erase(0);
+  EXPECT_EQ(set.size(), 0u);
+}
+
+TEST_F(UnresolvedSetTest, EraseIntegerFromEnd) {
+  set.erase(3);
+  EXPECT_EQ(set.size(), 3u);
+  EXPECT_EQ(set[0].getDecl(), &n0);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+  EXPECT_EQ(set[2].getDecl(), &n2);
+
+  set.erase(2);
+  EXPECT_EQ(set.size(), 2u);
+  EXPECT_EQ(set[0].getDecl(), &n0);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+
+  set.erase(1);
+  EXPECT_EQ(set.size(), 1u);
+  EXPECT_EQ(set[0].getDecl(), &n0);
+
+  set.erase(0);
+  EXPECT_EQ(set.size(), 0u);
+}
+
+TEST_F(UnresolvedSetTest, EraseIteratorFromStart) {
+  set.erase(set.begin());
+  EXPECT_EQ(set.size(), 3u);
+  EXPECT_EQ(set[0].getDecl(), &n3);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+  EXPECT_EQ(set[2].getDecl(), &n2);
+
+  set.erase(set.begin());
+  EXPECT_EQ(set.size(), 2u);
+  EXPECT_EQ(set[0].getDecl(), &n2);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+
+  set.erase(set.begin());
+  EXPECT_EQ(set.size(), 1u);
+  EXPECT_EQ(set[0].getDecl(), &n1);
+
+  set.erase(set.begin());
+  EXPECT_EQ(set.size(), 0u);
+}
+
+TEST_F(UnresolvedSetTest, EraseIteratorFromEnd) {
+  set.erase(--set.end());
+  EXPECT_EQ(set.size(), 3u);
+  EXPECT_EQ(set[0].getDecl(), &n0);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+  EXPECT_EQ(set[2].getDecl(), &n2);
+
+  set.erase(--set.end());
+  EXPECT_EQ(set.size(), 2u);
+  EXPECT_EQ(set[0].getDecl(), &n0);
+  EXPECT_EQ(set[1].getDecl(), &n1);
+
+  set.erase(--set.end());
+  EXPECT_EQ(set.size(), 1u);
+  EXPECT_EQ(set[0].getDecl(), &n0);
+
+  set.erase(--set.end());
+  EXPECT_EQ(set.size(), 0u);
+}