maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Ivan Murashko 2022-02-10 09:40:24 -08:00
parent b0e77d5e48
commit 71d7c8d870
2 changed files with 12 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
View File

@ -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 @@ private:
// 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

9
clang/unittests/Tooling/HeaderIncludesTest.cpp
View File

@ -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"