From 86e18055e607b257c6867e444a4e598cb8f812b8 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 1 Aug 2018 10:24:17 +0000 Subject: [DebugInfo] Have custom std::reverse_iterator The DWARFDie is a lightweight utility wrapper that stores a pointer to a compile unit and a debug info entry. Currently, its iterator (used for walking over its children) stores a DWARFDie and returns a const reference when dereferencing it. When the iterator is modified (by incrementing or decrementing it), this reference becomes invalid. This was happening when calling reverse on it, because the std::reverse_iterator is keeping a temporary copy of the iterator (see https://en.cppreference.com/w/cpp/iterator/reverse_iterator for a good illustration). The relevant code in libcxx: reference operator*() const {_Iter __tmp = current; return *--__tmp;} When dereferencing the reverse iterator, we decrement and return a reference to a DWARFDie stored in the stack frame of this function, resulting in UB at runtime. This patch specifies the std::reverse_iterator for DWARFDie to do the right thing. Differential revision: https://reviews.llvm.org/D49679 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@338506 91177308-0d34-0410-b5e6-96231b3b80d8 --- unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp | 71 +++++++++++++++++------- 1 file changed, 51 insertions(+), 20 deletions(-) (limited to 'unittests') diff --git a/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp b/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp index 442dea3c52f..273809fcbd3 100644 --- a/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp +++ b/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp @@ -1122,26 +1122,57 @@ TEST(DWARFDebugInfo, TestRelations) { EXPECT_EQ(C1.getParent(), C); EXPECT_EQ(C2.getParent(), C); - // Make sure bidirectional iterator works as expected. - auto Begin = A.begin(); - auto End = A.end(); - auto It = A.begin(); - - EXPECT_EQ(It, Begin); - EXPECT_EQ(*It, B); - ++It; - EXPECT_EQ(*It, C); - ++It; - EXPECT_EQ(*It, D); - ++It; - EXPECT_EQ(It, End); - --It; - EXPECT_EQ(*It, D); - --It; - EXPECT_EQ(*It, C); - --It; - EXPECT_EQ(*It, B); - EXPECT_EQ(It, Begin); + // Make sure iterators work as expected. + EXPECT_THAT(std::vector(A.begin(), A.end()), + testing::ElementsAre(B, C, D)); + EXPECT_THAT(std::vector(A.rbegin(), A.rend()), + testing::ElementsAre(D, C, B)); + + // Make sure iterator is bidirectional. + { + auto Begin = A.begin(); + auto End = A.end(); + auto It = A.begin(); + + EXPECT_EQ(It, Begin); + EXPECT_EQ(*It, B); + ++It; + EXPECT_EQ(*It, C); + ++It; + EXPECT_EQ(*It, D); + ++It; + EXPECT_EQ(It, End); + --It; + EXPECT_EQ(*It, D); + --It; + EXPECT_EQ(*It, C); + --It; + EXPECT_EQ(*It, B); + EXPECT_EQ(It, Begin); + } + + // Make sure reverse iterator is bidirectional. + { + auto Begin = A.rbegin(); + auto End = A.rend(); + auto It = A.rbegin(); + + EXPECT_EQ(It, Begin); + EXPECT_EQ(*It, D); + ++It; + EXPECT_EQ(*It, C); + ++It; + EXPECT_EQ(*It, B); + ++It; + EXPECT_EQ(It, End); + --It; + EXPECT_EQ(*It, B); + --It; + EXPECT_EQ(*It, C); + --It; + EXPECT_EQ(*It, D); + EXPECT_EQ(It, Begin); + } } TEST(DWARFDebugInfo, TestDWARFDie) { -- cgit v1.2.3