[clangd] Fix use-after-free in SymbolYAML: strings are owned by yaml::Input!

Summary:
There are a few implementation options here - alternatives are either both
awkward and inefficient, or really inefficient.
This is at least potentially a hot path when used as a reducer for common
symbols.

(Also fix an unused-var that sneaked in)

Reviewers: ioeric

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

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

llvm-svn: 325476
This commit is contained in:
Sam McCall 2018-02-19 09:31:26 +00:00
parent 90c47cad22
commit aeb626920c
4 changed files with 13 additions and 12 deletions

View File

@ -360,9 +360,8 @@ static std::string NamedDeclQualifiedName(const NamedDecl *ND,
static llvm::Optional<std::string> getScopeName(const Decl *D) {
const DeclContext *DC = D->getDeclContext();
if (const TranslationUnitDecl *TUD = dyn_cast<TranslationUnitDecl>(DC))
if (isa<TranslationUnitDecl>(DC))
return std::string("global namespace");
if (const TypeDecl *TD = dyn_cast<TypeDecl>(DC))
return TypeDeclToString(TD);
else if (const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC))

View File

@ -20,7 +20,6 @@
#include "index/SymbolYAML.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Index/IndexDataConsumer.h"
#include "clang/Index/IndexingAction.h"
#include "clang/Tooling/CommonOptionsParser.h"
@ -31,6 +30,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/ThreadPool.h"
#include "llvm/Support/YAMLTraits.h"
using namespace llvm;
using namespace clang::tooling;
@ -117,7 +117,8 @@ SymbolSlab mergeSymbols(tooling::ToolResults *Results) {
Symbol::Details Scratch;
Results->forEachResult([&](llvm::StringRef Key, llvm::StringRef Value) {
Arena.Reset();
auto Sym = clang::clangd::SymbolFromYAML(Value, Arena);
llvm::yaml::Input Yin(Value, &Arena);
auto Sym = clang::clangd::SymbolFromYAML(Yin, Arena);
clang::clangd::SymbolID ID;
Key >> ID;
if (const auto *Existing = UniqueSymbols.find(ID))

View File

@ -12,7 +12,6 @@
#include "llvm/ADT/Optional.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"
LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(clang::clangd::Symbol)
@ -176,11 +175,11 @@ SymbolSlab SymbolsFromYAML(llvm::StringRef YAMLContent) {
return std::move(Syms).build();
}
Symbol SymbolFromYAML(llvm::StringRef YAMLContent,
llvm::BumpPtrAllocator &Arena) {
llvm::yaml::Input Yin(YAMLContent, &Arena);
Symbol SymbolFromYAML(llvm::yaml::Input &Input, llvm::BumpPtrAllocator &Arena) {
// We could grab Arena out of Input, but it'd be a huge hazard for callers.
assert(Input.getContext() == &Arena);
Symbol S;
Yin >> S;
Input >> S;
return S;
}

View File

@ -20,6 +20,7 @@
#include "Index.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"
namespace clang {
@ -28,9 +29,10 @@ namespace clangd {
// Read symbols from a YAML-format string.
SymbolSlab SymbolsFromYAML(llvm::StringRef YAMLContent);
// Read one symbol from a YAML-format string, backed by the arena.
Symbol SymbolFromYAML(llvm::StringRef YAMLContent,
llvm::BumpPtrAllocator &Arena);
// Read one symbol from a YAML-stream.
// The arena must be the Input's context! (i.e. yaml::Input Input(Text, &Arena))
// The returned symbol is backed by both Input and Arena.
Symbol SymbolFromYAML(llvm::yaml::Input &Input, llvm::BumpPtrAllocator &Arena);
// Convert a single symbol to YAML-format string.
// The YAML result is safe to concatenate.