qdelacru created this revision.
qdelacru added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
qdelacru requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Having nested macros in the C code could cause clangd to fail an assert in
clang::Preprocessor::setLoadedMacroDirective() and crash.
#0 0x00000000007acd79 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int)
/qdelacru/llvm-project/llvm/lib/Support/Unix/Signals.inc:565:22
#1 0x00000000007ace30 PrintStackTraceSignalHandler(void*)
/qdelacru/llvm-project/llvm/lib/Support/Unix/Signals.inc:632:1
#2 0x00000000007aaded llvm::sys::RunSignalHandlers()
/qdelacru/llvm-project/llvm/lib/Support/Signals.cpp:76:20
#3 0x00000000007ac7c1 SignalHandler(int)
/qdelacru/llvm-project/llvm/lib/Support/Unix/Signals.inc:407:1
#4 0x00007f096604db20 __restore_rt (/lib64/libpthread.so.0+0x12b20)
#5 0x00007f0964b307ff raise (/lib64/libc.so.6+0x377ff)
#6 0x00007f0964b1ac35 abort (/lib64/libc.so.6+0x21c35)
#7 0x00007f0964b1ab09 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21b09)
#8 0x00007f0964b28de6 (/lib64/libc.so.6+0x2fde6)
#9 0x0000000001004d1a
clang::Preprocessor::setLoadedMacroDirective(clang::IdentifierInfo*,
clang::MacroDirective*, clang::MacroDirective*)
/qdelacru/llvm-project/clang/lib/Lex/PPMacroExpansion.cpp:116:5
#10 0x00000000032287b5
clang::ASTReader::resolvePendingMacro(clang::IdentifierInfo*,
clang::ASTReader::PendingMacroInfo const&)
/qdelacru/llvm-project/clang/lib/Serialization/ASTReader.cpp:2219:31
#11 0x000000000324f130 clang::ASTReader::finishPendingActions()
/qdelacru/llvm-project/clang/lib/Serialization/ASTReader.cpp:9249:7
#12 0x000000000325d15e clang::ASTReader::FinishedDeserializing()
/qdelacru/llvm-project/clang/lib/Serialization/ASTReader.cpp:11575:5
#13 0x000000000093c64b
clang::ExternalASTSource::Deserializing::~Deserializing()
/qdelacru/llvm-project/clang/include/clang/AST/ExternalASTSource.h:89:5
#14 0x000000000324a74f clang::ASTReader::get(llvm::StringRef)
/qdelacru/llvm-project/clang/lib/Serialization/ASTReader.cpp:8038:10
#15 0x00000000011c5708 clang::clangd::(anonymous
namespace)::loadMainFilePreambleMacros(clang::Preprocessor const&,
clang::clangd::PreambleData const&)
/qdelacru/llvm-project/clang-tools-extra/clangd/CodeComplete.cpp:1113:44
#16 0x00000000011c5de0 clang::clangd::(anonymous
namespace)::semaCodeComplete(std::unique_ptr<clang::CodeCompleteConsumer,
std::default_delete<clang::CodeCompleteConsumer> >, clang::CodeCompleteOptions
const&, clang::clangd::(anonymous namespace)::SemaCompleteInput const&,
clang::clangd::IncludeStructure*)
/qdelacru/llvm-project/clang-tools-extra/clangd/CodeComplete.cpp:1190:3
#17 0x00000000011c703b clang::clangd::(anonymous
namespace)::CodeCompleteFlow::run(clang::clangd::(anonymous
namespace)::SemaCompleteInput const&) &&
/qdelacru/llvm-project/clang-tools-extra/clangd/CodeComplete.cpp:1395:21
#18 0x00000000011ca87a clang::clangd::codeComplete(llvm::StringRef,
clang::clangd::Position, clang::clangd::PreambleData const*,
clang::clangd::ParseInputs const&, clang::clangd::CodeCompleteOptions,
clang::clangd::SpeculativeFuzzyFind*)
/qdelacru/llvm-project/clang-tools-extra/clangd/CodeComplete.cpp:1865:78
#19 0x000000000119c351
clang::clangd::ClangdServer::codeComplete(llvm::StringRef,
clang::clangd::Position, clang::clangd::CodeCompleteOptions const&,
llvm::unique_function<void
(llvm::Expected<clang::clangd::CodeCompleteResult>)>)::'lambda'(llvm::Expected<clang::clangd::InputsAndPreamble>)::operator()(llvm::Expected<clang::clangd::InputsAndPreamble>)
/qdelacru/llvm-project/clang-tools-extra/clangd/ClangdServer.cpp:377:61
#20 0x00000000011a821a void llvm::detail::UniqueFunctionBase<void,
llvm::Expected<clang::clangd::InputsAndPreamble>
>::CallImpl<clang::clangd::ClangdServer::codeComplete(llvm::StringRef,
clang::clangd::Position, clang::clangd::CodeCompleteOptions const&,
llvm::unique_function<void
(llvm::Expected<clang::clangd::CodeCompleteResult>)>)::'lambda'(llvm::Expected<clang::clangd::InputsAndPreamble>)>(void*,
llvm::Expected<clang::clangd::InputsAndPreamble>&)
/qdelacru/llvm-project/llvm/include/llvm/ADT/FunctionExtras.h:204:16
#21 0x0000000001554403 llvm::unique_function<void
(llvm::Expected<clang::clangd::InputsAndPreamble>)>::operator()(llvm::Expected<cl
An example of the code that causes the assert failure:
#define ECHO(X) X
#define ECHO2(X) ECHO(X)
...
During code completion in clangd, the macros will be loaded in
loadMainFilePreambleMacros() by iterating over the macro names and calling
PreambleIdentifiers->get(). Since these macro names are store in a StringSet
(has StringMap underlying container), the order of the iterator is not
guaranteed to be same as the order seen in the source code.
When clangd is trying to resolve nested macros it sometimes attempts to load
them out of order which causes a macro to be stored twice. In the example
above, ECHO2 macro gets resolved first, but since it uses another macro that
has not been resolved it will try to resolve/store that as well. Now there are
two MacroDirectives stored in the Preprocessor, ECHO and ECHO2. When clangd
tries to load the next macro, ECHO, the preprocessor fails an assert in
clang::Preprocessor::setLoadedMacroDirective() because there is already a
MacroDirective stored for that macro name.
In this diff, I check if the macro is already inside the IdentifierTable and if
it is skip it so that it is not resolved twice.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D101870
Files:
clang-tools-extra/clangd/CodeComplete.cpp
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1105,14 +1105,19 @@
ExternalPreprocessorSource *PreambleMacros = PP.getExternalSource();
// As we have the names of the macros, we can look up their IdentifierInfo
// and then use this to load just the macros we want.
+ const auto &ITable = PP.getIdentifierTable();
IdentifierInfoLookup *PreambleIdentifiers =
- PP.getIdentifierTable().getExternalIdentifierLookup();
+ ITable.getExternalIdentifierLookup();
+
if (!PreambleIdentifiers || !PreambleMacros)
return;
- for (const auto &MacroName : Preamble.Macros.Names)
+ for (const auto &MacroName : Preamble.Macros.Names) {
+ if (ITable.find(MacroName.getKey()) != ITable.end())
+ continue;
if (auto *II = PreambleIdentifiers->get(MacroName.getKey()))
if (II->isOutOfDate())
PreambleMacros->updateOutOfDateIdentifier(*II);
+ }
}
// Invokes Sema code completion on a file.
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -1105,14 +1105,19 @@
ExternalPreprocessorSource *PreambleMacros = PP.getExternalSource();
// As we have the names of the macros, we can look up their IdentifierInfo
// and then use this to load just the macros we want.
+ const auto &ITable = PP.getIdentifierTable();
IdentifierInfoLookup *PreambleIdentifiers =
- PP.getIdentifierTable().getExternalIdentifierLookup();
+ ITable.getExternalIdentifierLookup();
+
if (!PreambleIdentifiers || !PreambleMacros)
return;
- for (const auto &MacroName : Preamble.Macros.Names)
+ for (const auto &MacroName : Preamble.Macros.Names) {
+ if (ITable.find(MacroName.getKey()) != ITable.end())
+ continue;
if (auto *II = PreambleIdentifiers->get(MacroName.getKey()))
if (II->isOutOfDate())
PreambleMacros->updateOutOfDateIdentifier(*II);
+ }
}
// Invokes Sema code completion on a file.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
