From 10f0f98eac5b20796ae804a2df2a9d853d59d3bd Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 29 Dec 2019 19:26:11 +0000 Subject: [PATCH 1/3] Add a way to set traversal mode in clang-query Reviewers: aaron.ballman Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D73037 --- clang-tools-extra/clang-query/Query.cpp | 12 +++++++++ clang-tools-extra/clang-query/Query.h | 5 ++++ clang-tools-extra/clang-query/QueryParser.cpp | 25 ++++++++++++++++- clang-tools-extra/clang-query/QueryParser.h | 2 ++ clang-tools-extra/clang-query/QuerySession.h | 5 +++- .../unittests/clang-query/QueryParserTest.cpp | 27 +++++++++++++++++++ 6 files changed, 74 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-query/Query.cpp b/clang-tools-extra/clang-query/Query.cpp index 8eafc5eed7507e..2fc7af6a56e14d 100644 --- a/clang-tools-extra/clang-query/Query.cpp +++ b/clang-tools-extra/clang-query/Query.cpp @@ -43,6 +43,15 @@ bool HelpQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const { "Set whether to bind the root matcher to \"root\".\n" " set print-matcher (true|false) " "Set whether to print the current matcher,\n" + " set traversal " + "Set traversal kind of clang-query session. Available kinds are:\n" + " AsIs " + "Print and match the AST as clang sees it.\n" + " IgnoreImplicitCastsAndParentheses " + "Omit implicit casts and parens in matching and dumping.\n" + " IgnoreUnlessSpelledInSource " + "Omit AST nodes unless spelled in the source. This mode is the " + "default.\n" " set output " "Set whether to output only content.\n" " enable output " @@ -98,6 +107,8 @@ bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const { OS << "Not a valid top-level matcher.\n"; return false; } + + AST->getASTContext().getParentMapContext().setTraversalKind(QS.TK); Finder.matchAST(AST->getASTContext()); if (QS.PrintMatcher) { @@ -148,6 +159,7 @@ bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const { const SourceManager &SM = Ctx.getSourceManager(); ASTDumper Dumper(OS, &Ctx.getCommentCommandTraits(), &SM, SM.getDiagnostics().getShowColors(), Ctx.getPrintingPolicy()); + Dumper.SetTraversalKind(QS.TK); Dumper.Visit(BI->second); OS << "\n"; } diff --git a/clang-tools-extra/clang-query/Query.h b/clang-tools-extra/clang-query/Query.h index 78bcbc79cdf872..223644fe2e515b 100644 --- a/clang-tools-extra/clang-query/Query.h +++ b/clang-tools-extra/clang-query/Query.h @@ -28,6 +28,7 @@ enum QueryKind { QK_Match, QK_SetBool, QK_SetOutputKind, + QK_SetTraversalKind, QK_EnableOutputKind, QK_DisableOutputKind, QK_Quit @@ -119,6 +120,10 @@ template <> struct SetQueryKind { static const QueryKind value = QK_SetOutputKind; }; +template <> struct SetQueryKind { + static const QueryKind value = QK_SetTraversalKind; +}; + /// Query for "set VAR VALUE". template struct SetQuery : Query { SetQuery(T QuerySession::*Var, T Value) diff --git a/clang-tools-extra/clang-query/QueryParser.cpp b/clang-tools-extra/clang-query/QueryParser.cpp index ecc189a7db2f5d..2f1965e77ab4b4 100644 --- a/clang-tools-extra/clang-query/QueryParser.cpp +++ b/clang-tools-extra/clang-query/QueryParser.cpp @@ -128,6 +128,24 @@ template QueryRef QueryParser::parseSetOutputKind() { llvm_unreachable("Invalid output kind"); } +QueryRef QueryParser::parseSetTraversalKind( + ast_type_traits::TraversalKind QuerySession::*Var) { + StringRef ValStr; + unsigned Value = + LexOrCompleteWord(this, ValStr) + .Case("AsIs", ast_type_traits::TK_AsIs) + .Case("IgnoreImplicitCastsAndParentheses", + ast_type_traits::TK_IgnoreImplicitCastsAndParentheses) + .Case("IgnoreUnlessSpelledInSource", + ast_type_traits::TK_IgnoreUnlessSpelledInSource) + .Default(~0u); + if (Value == ~0u) { + return new InvalidQuery("expected traversal kind, got '" + ValStr + "'"); + } + return new SetQuery( + Var, static_cast(Value)); +} + QueryRef QueryParser::endQuery(QueryRef Q) { StringRef Extra = Line; StringRef ExtraTrimmed = Extra.drop_while( @@ -171,7 +189,8 @@ enum ParsedQueryVariable { PQV_Invalid, PQV_Output, PQV_BindRoot, - PQV_PrintMatcher + PQV_PrintMatcher, + PQV_Traversal }; QueryRef makeInvalidQueryFromDiagnostics(const Diagnostics &Diag) { @@ -272,6 +291,7 @@ QueryRef QueryParser::doParse() { .Case("output", PQV_Output) .Case("bind-root", PQV_BindRoot) .Case("print-matcher", PQV_PrintMatcher) + .Case("traversal", PQV_Traversal) .Default(PQV_Invalid); if (VarStr.empty()) return new InvalidQuery("expected variable name"); @@ -289,6 +309,9 @@ QueryRef QueryParser::doParse() { case PQV_PrintMatcher: Q = parseSetBool(&QuerySession::PrintMatcher); break; + case PQV_Traversal: + Q = parseSetTraversalKind(&QuerySession::TK); + break; case PQV_Invalid: llvm_unreachable("Invalid query kind"); } diff --git a/clang-tools-extra/clang-query/QueryParser.h b/clang-tools-extra/clang-query/QueryParser.h index 12664777ee447f..68f420dc0994ef 100644 --- a/clang-tools-extra/clang-query/QueryParser.h +++ b/clang-tools-extra/clang-query/QueryParser.h @@ -43,6 +43,8 @@ class QueryParser { template struct LexOrCompleteWord; QueryRef parseSetBool(bool QuerySession::*Var); + QueryRef + parseSetTraversalKind(ast_type_traits::TraversalKind QuerySession::*Var); template QueryRef parseSetOutputKind(); QueryRef completeMatcherExpression(); diff --git a/clang-tools-extra/clang-query/QuerySession.h b/clang-tools-extra/clang-query/QuerySession.h index 0f3bc1aa64641a..1660e4039f613b 100644 --- a/clang-tools-extra/clang-query/QuerySession.h +++ b/clang-tools-extra/clang-query/QuerySession.h @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_QUERY_QUERY_SESSION_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_QUERY_QUERY_SESSION_H +#include "clang/AST/ASTTypeTraits.h" #include "clang/ASTMatchers/Dynamic/VariantValue.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringMap.h" @@ -25,7 +26,7 @@ class QuerySession { QuerySession(llvm::ArrayRef> ASTs) : ASTs(ASTs), PrintOutput(false), DiagOutput(true), DetailedASTOutput(false), BindRoot(true), PrintMatcher(false), - Terminate(false) {} + Terminate(false), TK(ast_type_traits::TK_IgnoreUnlessSpelledInSource) {} llvm::ArrayRef> ASTs; @@ -36,6 +37,8 @@ class QuerySession { bool BindRoot; bool PrintMatcher; bool Terminate; + + ast_type_traits::TraversalKind TK; llvm::StringMap NamedValues; }; diff --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp index 4ebe1237b21cf2..4a0a80146af4be 100644 --- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp +++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp @@ -110,6 +110,18 @@ TEST_F(QueryParserTest, Set) { ASSERT_TRUE(isa >(Q)); EXPECT_EQ(&QuerySession::BindRoot, cast >(Q)->Var); EXPECT_EQ(true, cast >(Q)->Value); + + Q = parse("set traversal AsIs"); + ASSERT_TRUE(isa>(Q)); + EXPECT_EQ(&QuerySession::TK, + cast>(Q)->Var); + EXPECT_EQ(ast_type_traits::TK_AsIs, + cast>(Q)->Value); + + Q = parse("set traversal NotATraversal"); + ASSERT_TRUE(isa(Q)); + EXPECT_EQ("expected traversal kind, got 'NotATraversal'", + cast(Q)->ErrStr); } TEST_F(QueryParserTest, Match) { @@ -197,6 +209,11 @@ TEST_F(QueryParserTest, Complete) { EXPECT_EQ("utput ", Comps[0].TypedText); EXPECT_EQ("output", Comps[0].DisplayText); + Comps = QueryParser::complete("set t", 5, QS); + ASSERT_EQ(1u, Comps.size()); + EXPECT_EQ("raversal ", Comps[0].TypedText); + EXPECT_EQ("traversal", Comps[0].DisplayText); + Comps = QueryParser::complete("enable ", 7, QS); ASSERT_EQ(1u, Comps.size()); EXPECT_EQ("output ", Comps[0].TypedText); @@ -214,6 +231,16 @@ TEST_F(QueryParserTest, Complete) { EXPECT_EQ("dump ", Comps[3].TypedText); EXPECT_EQ("dump", Comps[3].DisplayText); + Comps = QueryParser::complete("set traversal ", 14, QS); + ASSERT_EQ(3u, Comps.size()); + + EXPECT_EQ("AsIs ", Comps[0].TypedText); + EXPECT_EQ("AsIs", Comps[0].DisplayText); + EXPECT_EQ("IgnoreImplicitCastsAndParentheses ", Comps[1].TypedText); + EXPECT_EQ("IgnoreImplicitCastsAndParentheses", Comps[1].DisplayText); + EXPECT_EQ("IgnoreUnlessSpelledInSource ", Comps[2].TypedText); + EXPECT_EQ("IgnoreUnlessSpelledInSource", Comps[2].DisplayText); + Comps = QueryParser::complete("match while", 11, QS); ASSERT_EQ(1u, Comps.size()); EXPECT_EQ("Stmt(", Comps[0].TypedText); From 38c5d6f70060046bbbfec7491c7ba54a50fa5d16 Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Thu, 14 May 2020 21:44:06 +0300 Subject: [PATCH 2/3] [yaml2obj] - Add a technical prefix for each unnamed chunk. This change does not affect the produced binary. In this patch I assign a technical suffix to each section/fill (i.e. chunk) name when it is empty. It allows to simplify the code slightly and improve error messages reported. In the code we have the section to index mapping, SN2I, which is globally used. With this change we can use it to map "empty" names to indexes now, what is helpful. Differential revision: https://reviews.llvm.org/D79984 --- llvm/lib/ObjectYAML/ELFEmitter.cpp | 46 ++++++++++++------- .../yaml2obj/ELF/custom-null-section.yaml | 5 +- .../test/tools/yaml2obj/ELF/section-link.yaml | 12 +++-- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp index 2a3839509fafc6..1ca1dfe0dc4ef5 100644 --- a/llvm/lib/ObjectYAML/ELFEmitter.cpp +++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp @@ -218,6 +218,7 @@ template class ELFState { void assignSectionAddress(Elf_Shdr &SHeader, ELFYAML::Section *YAMLSec); + BumpPtrAllocator StringAlloc; uint64_t alignToOffset(ContiguousBlobAccumulator &CBA, uint64_t Align, llvm::Optional Offset); @@ -241,11 +242,6 @@ template ELFState::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH) : Doc(D), ErrHandler(EH) { std::vector Sections = Doc.getSections(); - StringSet<> DocSections; - for (const ELFYAML::Section *Sec : Sections) - if (!Sec->Name.empty()) - DocSections.insert(Sec->Name); - // Insert SHT_NULL section implicitly when it is not defined in YAML. if (Sections.empty() || Sections.front()->Type != ELF::SHT_NULL) Doc.Chunks.insert( @@ -253,6 +249,21 @@ ELFState::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH) std::make_unique( ELFYAML::Chunk::ChunkKind::RawContent, /*IsImplicit=*/true)); + // We add a technical suffix for each unnamed section/fill. It does not affect + // the output, but allows us to map them by name in the code and report better + // error messages. + StringSet<> DocSections; + for (size_t I = 0; I < Doc.Chunks.size(); ++I) { + const std::unique_ptr &C = Doc.Chunks[I]; + if (C->Name.empty()) { + std::string NewName = ELFYAML::appendUniqueSuffix( + /*Name=*/"", "index " + Twine(I)); + C->Name = StringRef(NewName).copy(StringAlloc); + assert(ELFYAML::dropUniqueSuffix(C->Name).empty()); + } + DocSections.insert(C->Name); + } + std::vector ImplicitSections; if (Doc.DynamicSymbols) ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"}); @@ -401,22 +412,28 @@ bool ELFState::initImplicitHeader(ContiguousBlobAccumulator &CBA, return true; } -constexpr StringRef SuffixStart = " ("; +constexpr char SuffixStart = '('; constexpr char SuffixEnd = ')'; std::string llvm::ELFYAML::appendUniqueSuffix(StringRef Name, const Twine &Msg) { - return (Name + SuffixStart + Msg + Twine(SuffixEnd)).str(); + // Do not add a space when a Name is empty. + std::string Ret = Name.empty() ? "" : Name.str() + ' '; + return Ret + (Twine(SuffixStart) + Msg + Twine(SuffixEnd)).str(); } StringRef llvm::ELFYAML::dropUniqueSuffix(StringRef S) { if (S.empty() || S.back() != SuffixEnd) return S; + // A special case for empty names. See appendUniqueSuffix() above. size_t SuffixPos = S.rfind(SuffixStart); - if (SuffixPos == StringRef::npos) + if (SuffixPos == 0) + return ""; + + if (SuffixPos == StringRef::npos || S[SuffixPos - 1] != ' ') return S; - return S.substr(0, SuffixPos); + return S.substr(0, SuffixPos - 1); } template @@ -426,7 +443,6 @@ void ELFState::initSectionHeaders(std::vector &SHeaders, // valid SHN_UNDEF entry since SHT_NULL == 0. SHeaders.resize(Doc.getSections().size()); - size_t SecNdx = -1; for (const std::unique_ptr &D : Doc.Chunks) { if (ELFYAML::Fill *S = dyn_cast(D.get())) { S->Offset = alignToOffset(CBA, /*Align=*/1, S->Offset); @@ -435,16 +451,16 @@ void ELFState::initSectionHeaders(std::vector &SHeaders, continue; } - ++SecNdx; ELFYAML::Section *Sec = cast(D.get()); - if (SecNdx == 0 && Sec->IsImplicit) + bool IsFirstUndefSection = D == Doc.Chunks.front(); + if (IsFirstUndefSection && Sec->IsImplicit) continue; // We have a few sections like string or symbol tables that are usually // added implicitly to the end. However, if they are explicitly specified // in the YAML, we need to write them here. This ensures the file offset // remains correct. - Elf_Shdr &SHeader = SHeaders[SecNdx]; + Elf_Shdr &SHeader = SHeaders[SN2I.get(Sec->Name)]; if (initImplicitHeader(CBA, SHeader, Sec->Name, Sec->IsImplicit ? nullptr : Sec)) continue; @@ -461,7 +477,6 @@ void ELFState::initSectionHeaders(std::vector &SHeaders, // Set the offset for all sections, except the SHN_UNDEF section with index // 0 when not explicitly requested. - bool IsFirstUndefSection = SecNdx == 0; if (!IsFirstUndefSection || Sec->Offset) SHeader.sh_offset = alignToOffset(CBA, SHeader.sh_addralign, Sec->Offset); @@ -1427,9 +1442,6 @@ template void ELFState::buildSectionIndex() { if (IsSection) ++SecNdx; - if (C->Name.empty()) - continue; - if (!Seen.insert(C->Name).second) reportError("repeated section/fill name: '" + C->Name + "' at YAML section/fill number " + Twine(I)); diff --git a/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml b/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml index 20ea73ec834ee2..0ccff19eaf8578 100644 --- a/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml +++ b/llvm/test/tools/yaml2obj/ELF/custom-null-section.yaml @@ -128,7 +128,8 @@ Sections: # RUN: not yaml2obj --docnum=6 %s -o %t6 2>&1 | FileCheck %s --check-prefix=CASE4 -# CASE4: error: unknown section referenced: '.foo' by YAML section '' +# CASE4: error: unknown section referenced: '.foo' by YAML section '(index 0)' +# CASE4-NEXT: error: unknown section referenced: '.bar' by YAML section '(index 1)' --- !ELF FileHeader: @@ -139,6 +140,8 @@ FileHeader: Sections: - Type: SHT_NULL Link: .foo + - Type: SHT_NULL + Link: .bar ## Check that null section fields are set to zero, if they are unspecified. diff --git a/llvm/test/tools/yaml2obj/ELF/section-link.yaml b/llvm/test/tools/yaml2obj/ELF/section-link.yaml index efe4d3fea74d57..bd82058a92ac56 100644 --- a/llvm/test/tools/yaml2obj/ELF/section-link.yaml +++ b/llvm/test/tools/yaml2obj/ELF/section-link.yaml @@ -28,8 +28,10 @@ Sections: # RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=ERR -# ERR: error: unknown section referenced: '.unknown1' by YAML section '.foo' -# ERR: error: unknown section referenced: '.unknown2' by YAML section '.bar' +# ERR: error: unknown section referenced: '.unknown1' by YAML section '.foo' +# ERR-NEXT: error: unknown section referenced: '.unknown2' by YAML section '(index 2)' +# ERR-NEXT: error: unknown section referenced: '.unknown3' by YAML section '.bar' +# ERR-NEXT: error: unknown section referenced: '.unknown4' by YAML section '(index 4)' --- !ELF FileHeader: @@ -41,9 +43,13 @@ Sections: - Name: .foo Type: SHT_PROGBITS Link: .unknown1 + - Type: SHT_PROGBITS + Link: .unknown2 - Name: .bar Type: SHT_PROGBITS - Link: .unknown2 + Link: .unknown3 + - Type: SHT_PROGBITS + Link: .unknown4 ## Check we link SHT_GROUP to a symbol table by default if it exists. ## Also, check we can set an arbitrary value for sh_link. From 304b0ed40392830e6f833c56b38cdc8296f58ce4 Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Thu, 14 May 2020 22:46:58 +0300 Subject: [PATCH 3/3] [yaml2obj] - Move "repeated section/fill name" check earlier. This allows to simplify the code. Doing checks early is generally useful. Differential revision: https://reviews.llvm.org/D79985 --- llvm/lib/ObjectYAML/ELFEmitter.cpp | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp index 1ca1dfe0dc4ef5..95d74eeeb6e6ea 100644 --- a/llvm/lib/ObjectYAML/ELFEmitter.cpp +++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp @@ -261,7 +261,10 @@ ELFState::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH) C->Name = StringRef(NewName).copy(StringAlloc); assert(ELFYAML::dropUniqueSuffix(C->Name).empty()); } - DocSections.insert(C->Name); + + if (!DocSections.insert(C->Name).second) + reportError("repeated section/fill name: '" + C->Name + + "' at YAML section/fill number " + Twine(I)); } std::vector ImplicitSections; @@ -1435,18 +1438,10 @@ void ELFState::writeFill(ELFYAML::Fill &Fill, template void ELFState::buildSectionIndex() { size_t SecNdx = -1; - StringSet<> Seen; - for (size_t I = 0; I < Doc.Chunks.size(); ++I) { - const std::unique_ptr &C = Doc.Chunks[I]; - bool IsSection = isa(C.get()); - if (IsSection) - ++SecNdx; - - if (!Seen.insert(C->Name).second) - reportError("repeated section/fill name: '" + C->Name + - "' at YAML section/fill number " + Twine(I)); - if (!IsSection || HasError) + for (const std::unique_ptr &C : Doc.Chunks) { + if (!isa(C.get())) continue; + ++SecNdx; if (!SN2I.addName(C->Name, SecNdx)) llvm_unreachable("buildSectionIndex() failed"); @@ -1509,6 +1504,8 @@ template bool ELFState::writeELF(raw_ostream &OS, ELFYAML::Object &Doc, yaml::ErrorHandler EH) { ELFState State(Doc, EH); + if (State.HasError) + return false; // Finalize .strtab and .dynstr sections. We do that early because want to // finalize the string table builders before writing the content of the @@ -1516,9 +1513,6 @@ bool ELFState::writeELF(raw_ostream &OS, ELFYAML::Object &Doc, State.finalizeStrings(); State.buildSectionIndex(); - if (State.HasError) - return false; - State.buildSymbolIndexes(); std::vector PHeaders;