Author: Ivan Murashko Date: 2022-02-10T09:40:44-08:00 New Revision: 71d7c8d870db3b2da1fe1b3f54be77163c55dcd2
URL: https://github.com/llvm/llvm-project/commit/71d7c8d870db3b2da1fe1b3f54be77163c55dcd2 DIFF: https://github.com/llvm/llvm-project/commit/71d7c8d870db3b2da1fe1b3f54be77163c55dcd2.diff LOG: [clangd] Crash in __memcmp_avx2_movbe There is a clangd crash at `__memcmp_avx2_movbe`. Short problem description is below. The method `HeaderIncludes::addExistingInclude` stores `Include` objects by reference at 2 places: `ExistingIncludes` (primary storage) and `IncludesByPriority` (pointer to the object's location at ExistingIncludes). `ExistingIncludes` is a map where value is a `SmallVector`. A new element is inserted by `push_back`. The operation might do resize. As result pointers stored at `IncludesByPriority` might become invalid. Typical stack trace ``` frame #0: 0x00007f11460dcd94 libc.so.6`__memcmp_avx2_movbe + 308 frame #1: 0x00000000004782b8 clangd`llvm::StringRef::compareMemory(Lhs=" \"t2.h\"", Rhs="", Length=6) at StringRef.h:76:22 frame #2: 0x0000000000701253 clangd`llvm::StringRef::compare(this=0x0000 7f10de7d8610, RHS=(Data = "", Length = 7166742329480737377)) const at String Ref.h:206:34 * frame #3: 0x00000000007603ab clangd`llvm::operator<(llvm::StringRef, llv m::StringRef)(LHS=(Data = "\"t2.h\"", Length = 6), RHS=(Data = "", Length = 7166742329480737377)) at StringRef.h:907:23 frame #4: 0x0000000002d0ad9f clangd`clang::tooling::HeaderIncludes::inse rt(this=0x00007f10de7fb1a0, IncludeName=(Data = "t2.h\"", Length = 4), IsAng led=false) const at HeaderIncludes.cpp:365:22 frame #5: 0x00000000012ebfdd clangd`clang::clangd::IncludeInserter::inse rt(this=0x00007f10de7fb148, VerbatimHeader=(Data = "\"t2.h\"", Length = 6)) const at Headers.cpp:262:70 ``` A unit test test for the crash was created (`HeaderIncludesTest.RepeatedIncludes`). The proposed solution is to use std::list instead of llvm::SmallVector Test Plan ``` ./tools/clang/unittests/Tooling/ToolingTests --gtest_filter=HeaderIncludesTest.RepeatedIncludes ``` Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D118755 Added: Modified: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h clang/unittests/Tooling/HeaderIncludesTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h index c1b7baec7ec51..ea8ad896be89f 100644 --- a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h +++ b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h @@ -14,6 +14,7 @@ #include "clang/Tooling/Inclusions/IncludeStyle.h" #include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" +#include <list> #include <unordered_map> namespace clang { @@ -97,7 +98,8 @@ class HeaderIncludes { // Map from include name (quotation trimmed) to a list of existing includes // (in case there are more than one) with the name in the current file. <x> // and "x" will be treated as the same header when deleting #includes. - llvm::StringMap<llvm::SmallVector<Include, 1>> ExistingIncludes; + // std::list is used for pointers stability (see IncludesByPriority) + llvm::StringMap<std::list<Include>> ExistingIncludes; /// Map from priorities of #include categories to all #includes in the same /// category. This is used to find #includes of the same category when diff --git a/clang/unittests/Tooling/HeaderIncludesTest.cpp b/clang/unittests/Tooling/HeaderIncludesTest.cpp index 37007fbfb65e9..b7b958f0c914f 100644 --- a/clang/unittests/Tooling/HeaderIncludesTest.cpp +++ b/clang/unittests/Tooling/HeaderIncludesTest.cpp @@ -51,6 +51,15 @@ TEST_F(HeaderIncludesTest, NoExistingIncludeWithoutDefine) { EXPECT_EQ(Expected, insert(Code, "\"a.h\"")); } +TEST_F(HeaderIncludesTest, RepeatedIncludes) { + std::string Code; + for (int i = 0; i < 100; ++i) { + Code += "#include \"a.h\"\n"; + } + std::string Expected = Code + "#include \"a2.h\"\n"; + EXPECT_EQ(Expected, insert(Code, "\"a2.h\"")); +} + TEST_F(HeaderIncludesTest, NoExistingIncludeWithDefine) { std::string Code = "#ifndef A_H\n" "#define A_H\n" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits