diff --git a/.github/workflows/llvm-bugs.yml b/.github/workflows/llvm-bugs.yml index bfa0c9af946d9..f592dd6ccd903 100644 --- a/.github/workflows/llvm-bugs.yml +++ b/.github/workflows/llvm-bugs.yml @@ -12,6 +12,7 @@ on: jobs: auto-subscribe: runs-on: ubuntu-latest + if: github.repository == 'llvm/llvm-project' steps: - uses: actions/setup-node@v3 with: diff --git a/README.md b/README.md index 1273ba17c2fa0..eb8d624d75cec 100644 --- a/README.md +++ b/README.md @@ -1,27 +1,16 @@ # The LLVM Compiler Infrastructure -This directory and its sub-directories contain the source code for LLVM, -a toolkit for the construction of highly optimized compilers, -optimizers, and run-time environments. - -The README briefly describes how to get started with building LLVM. -For more information on how to contribute to the LLVM project, please -take a look at the -[Contributing to LLVM](https://llvm.org/docs/Contributing.html) guide. - -## Getting Started with the LLVM System - -Taken from [here](https://llvm.org/docs/GettingStarted.html). - -### Overview - Welcome to the LLVM project! +This repository contains the source code for LLVM, a toolkit for the +construction of highly optimized compilers, optimizers, and run-time +environments. + The LLVM project has multiple components. The core of the project is itself called "LLVM". This contains all of the tools, libraries, and header files needed to process intermediate representations and convert them into object files. Tools include an assembler, disassembler, bitcode analyzer, and -bitcode optimizer. It also contains basic regression tests. +bitcode optimizer. C-like languages use the [Clang](http://clang.llvm.org/) frontend. This component compiles C, C++, Objective-C, and Objective-C++ code into LLVM bitcode @@ -31,92 +20,20 @@ Other components include: the [libc++ C++ standard library](https://libcxx.llvm.org), the [LLD linker](https://lld.llvm.org), and more. -### Getting the Source Code and Building LLVM - -The LLVM Getting Started documentation may be out of date. The [Clang -Getting Started](http://clang.llvm.org/get_started.html) page might have more -accurate information. - -This is an example work-flow and configuration to get and build the LLVM source: - -1. Checkout LLVM (including related sub-projects like Clang): - - * ``git clone https://github.com/llvm/llvm-project.git`` - - * Or, on windows, ``git clone --config core.autocrlf=false - https://github.com/llvm/llvm-project.git`` - -2. Configure and build LLVM and Clang: - - * ``cd llvm-project`` - - * ``cmake -S llvm -B build -G [options]`` - - Some common build system generators are: - - * ``Ninja`` --- for generating [Ninja](https://ninja-build.org) - build files. Most llvm developers use Ninja. - * ``Unix Makefiles`` --- for generating make-compatible parallel makefiles. - * ``Visual Studio`` --- for generating Visual Studio projects and - solutions. - * ``Xcode`` --- for generating Xcode projects. - - Some common options: - - * ``-DLLVM_ENABLE_PROJECTS='...'`` and ``-DLLVM_ENABLE_RUNTIMES='...'`` --- - semicolon-separated list of the LLVM sub-projects and runtimes you'd like to - additionally build. ``LLVM_ENABLE_PROJECTS`` can include any of: clang, - clang-tools-extra, cross-project-tests, flang, libc, libclc, lld, lldb, - mlir, openmp, polly, or pstl. ``LLVM_ENABLE_RUNTIMES`` can include any of - libcxx, libcxxabi, libunwind, compiler-rt, libc or openmp. Some runtime - projects can be specified either in ``LLVM_ENABLE_PROJECTS`` or in - ``LLVM_ENABLE_RUNTIMES``. - - For example, to build LLVM, Clang, libcxx, and libcxxabi, use - ``-DLLVM_ENABLE_PROJECTS="clang" -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi"``. - - * ``-DCMAKE_INSTALL_PREFIX=directory`` --- Specify for *directory* the full - path name of where you want the LLVM tools and libraries to be installed - (default ``/usr/local``). Be careful if you install runtime libraries: if - your system uses those provided by LLVM (like libc++ or libc++abi), you - must not overwrite your system's copy of those libraries, since that - could render your system unusable. In general, using something like - ``/usr`` is not advised, but ``/usr/local`` is fine. - - * ``-DCMAKE_BUILD_TYPE=type`` --- Valid options for *type* are Debug, - Release, RelWithDebInfo, and MinSizeRel. Default is Debug. - - * ``-DLLVM_ENABLE_ASSERTIONS=On`` --- Compile with assertion checks enabled - (default is Yes for Debug builds, No for all other build types). - - * ``cmake --build build [-- [options] ]`` or your build system specified above - directly. - - * The default target (i.e. ``ninja`` or ``make``) will build all of LLVM. - - * The ``check-all`` target (i.e. ``ninja check-all``) will run the - regression tests to ensure everything is in working order. - - * CMake will generate targets for each tool and library, and most - LLVM sub-projects generate their own ``check-`` target. - - * Running a serial build will be **slow**. To improve speed, try running a - parallel build. That's done by default in Ninja; for ``make``, use the option - ``-j NNN``, where ``NNN`` is the number of parallel jobs to run. - In most cases, you get the best performance if you specify the number of CPU threads you have. - On some Unix systems, you can specify this with ``-j$(nproc)``. - - * For more information see [CMake](https://llvm.org/docs/CMake.html). +## Getting the Source Code and Building LLVM Consult the -[Getting Started with LLVM](https://llvm.org/docs/GettingStarted.html#getting-started-with-llvm) -page for detailed information on configuring and compiling LLVM. You can visit -[Directory Layout](https://llvm.org/docs/GettingStarted.html#directory-layout) -to learn about the layout of the source code tree. +[Getting Started with LLVM](https://llvm.org/docs/GettingStarted.html#getting-the-source-code-and-building-llvm) +page for information on building and running LLVM. + +For information on how to contribute to the LLVM project, please take a look at +the [Contributing to LLVM](https://llvm.org/docs/Contributing.html) guide. ## Getting in touch -Join [LLVM Discourse forums](https://discourse.llvm.org/), [discord chat](https://discord.gg/xS7Z362) or #llvm IRC channel on [OFTC](https://oftc.net/). +Join the [LLVM Discourse forums](https://discourse.llvm.org/), [Discord +chat](https://discord.gg/xS7Z362), or #llvm IRC channel on +[OFTC](https://oftc.net/). The LLVM project has adopted a [code of conduct](https://llvm.org/docs/CodeOfConduct.html) for participants to all modes of communication within the project. diff --git a/bolt/include/bolt/Core/BinaryBasicBlock.h b/bolt/include/bolt/Core/BinaryBasicBlock.h index d5f52734bcf36..8c044597b5fd0 100644 --- a/bolt/include/bolt/Core/BinaryBasicBlock.h +++ b/bolt/include/bolt/Core/BinaryBasicBlock.h @@ -427,10 +427,6 @@ class BinaryBasicBlock { /// Return branch info corresponding to an edge going to \p Succ basic block. const BinaryBranchInfo &getBranchInfo(const BinaryBasicBlock &Succ) const; - /// Return branch info corresponding to an edge going to a basic block with - /// label \p Label. - BinaryBranchInfo &getBranchInfo(const MCSymbol *Label); - /// Set branch information for the outgoing edge to block \p Succ. void setSuccessorBranchInfo(const BinaryBasicBlock &Succ, uint64_t Count, uint64_t MispredictedCount) { diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h index 051a58547007f..7a99ce2481a1a 100644 --- a/bolt/include/bolt/Core/BinaryFunction.h +++ b/bolt/include/bolt/Core/BinaryFunction.h @@ -167,6 +167,10 @@ class BinaryFunction { /// List of relocations associated with data in the constant island std::map Relocations; + /// Set true if constant island contains dynamic relocations, which may + /// happen if binary is linked with -z notext option. + bool HasDynamicRelocations{false}; + /// Offsets in function that are data values in a constant island identified /// after disassembling std::map Offsets; @@ -1229,96 +1233,11 @@ class BinaryFunction { return InputOffsetToAddressMap; } - void addRelocationAArch64(uint64_t Offset, MCSymbol *Symbol, uint64_t RelType, - uint64_t Addend, uint64_t Value, bool IsCI) { - std::map &Rels = - (IsCI) ? Islands->Relocations : Relocations; - switch (RelType) { - case ELF::R_AARCH64_ABS64: - case ELF::R_AARCH64_ABS32: - case ELF::R_AARCH64_ABS16: - case ELF::R_AARCH64_ADD_ABS_LO12_NC: - case ELF::R_AARCH64_ADR_GOT_PAGE: - case ELF::R_AARCH64_ADR_PREL_LO21: - case ELF::R_AARCH64_ADR_PREL_PG_HI21: - case ELF::R_AARCH64_ADR_PREL_PG_HI21_NC: - case ELF::R_AARCH64_LD64_GOT_LO12_NC: - case ELF::R_AARCH64_LDST8_ABS_LO12_NC: - case ELF::R_AARCH64_LDST16_ABS_LO12_NC: - case ELF::R_AARCH64_LDST32_ABS_LO12_NC: - case ELF::R_AARCH64_LDST64_ABS_LO12_NC: - case ELF::R_AARCH64_LDST128_ABS_LO12_NC: - case ELF::R_AARCH64_TLSDESC_ADD_LO12: - case ELF::R_AARCH64_TLSDESC_ADR_PAGE21: - case ELF::R_AARCH64_TLSDESC_ADR_PREL21: - case ELF::R_AARCH64_TLSDESC_LD64_LO12: - case ELF::R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21: - case ELF::R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC: - case ELF::R_AARCH64_MOVW_UABS_G0: - case ELF::R_AARCH64_MOVW_UABS_G0_NC: - case ELF::R_AARCH64_MOVW_UABS_G1: - case ELF::R_AARCH64_MOVW_UABS_G1_NC: - case ELF::R_AARCH64_MOVW_UABS_G2: - case ELF::R_AARCH64_MOVW_UABS_G2_NC: - case ELF::R_AARCH64_MOVW_UABS_G3: - case ELF::R_AARCH64_PREL16: - case ELF::R_AARCH64_PREL32: - case ELF::R_AARCH64_PREL64: - Rels[Offset] = Relocation{Offset, Symbol, RelType, Addend, Value}; - return; - case ELF::R_AARCH64_CALL26: - case ELF::R_AARCH64_JUMP26: - case ELF::R_AARCH64_TSTBR14: - case ELF::R_AARCH64_CONDBR19: - case ELF::R_AARCH64_TLSDESC_CALL: - case ELF::R_AARCH64_TLSLE_ADD_TPREL_HI12: - case ELF::R_AARCH64_TLSLE_ADD_TPREL_LO12_NC: - return; - default: - llvm_unreachable("Unexpected AArch64 relocation type in code"); - } - } - - void addRelocationX86(uint64_t Offset, MCSymbol *Symbol, uint64_t RelType, - uint64_t Addend, uint64_t Value) { - switch (RelType) { - case ELF::R_X86_64_8: - case ELF::R_X86_64_16: - case ELF::R_X86_64_32: - case ELF::R_X86_64_32S: - case ELF::R_X86_64_64: - case ELF::R_X86_64_PC8: - case ELF::R_X86_64_PC32: - case ELF::R_X86_64_PC64: - case ELF::R_X86_64_GOTPCRELX: - case ELF::R_X86_64_REX_GOTPCRELX: - Relocations[Offset] = Relocation{Offset, Symbol, RelType, Addend, Value}; - return; - case ELF::R_X86_64_PLT32: - case ELF::R_X86_64_GOTPCREL: - case ELF::R_X86_64_TPOFF32: - case ELF::R_X86_64_GOTTPOFF: - return; - default: - llvm_unreachable("Unexpected x86 relocation type in code"); - } - } - /// Register relocation type \p RelType at a given \p Address in the function /// against \p Symbol. /// Assert if the \p Address is not inside this function. void addRelocation(uint64_t Address, MCSymbol *Symbol, uint64_t RelType, - uint64_t Addend, uint64_t Value) { - assert(Address >= getAddress() && Address < getAddress() + getMaxSize() && - "address is outside of the function"); - uint64_t Offset = Address - getAddress(); - if (BC.isAArch64()) { - return addRelocationAArch64(Offset, Symbol, RelType, Addend, Value, - isInConstantIsland(Address)); - } - - return addRelocationX86(Offset, Symbol, RelType, Addend, Value); - } + uint64_t Addend, uint64_t Value); /// Return the name of the section this function originated from. std::optional getOriginSectionName() const { @@ -1835,7 +1754,7 @@ class BinaryFunction { /// Returns if this function is a child of \p Other function. bool isChildOf(const BinaryFunction &Other) const { - return llvm::is_contained(ParentFragments, &Other); + return ParentFragments.contains(&Other); } /// Set the profile data for the number of times the function was called. @@ -1945,6 +1864,28 @@ class BinaryFunction { return Symbol; } + /// Support dynamic relocations in constant islands, which may happen if + /// binary is linked with -z notext option. + void markIslandDynamicRelocationAtAddress(uint64_t Address) { + if (!isInConstantIsland(Address)) { + errs() << "BOLT-ERROR: dynamic relocation found for text section at 0x" + << Twine::utohexstr(Address) << "\n"; + exit(1); + } + + // Mark island to have dynamic relocation + Islands->HasDynamicRelocations = true; + + // Create island access, so we would emit the label and + // move binary data during updateOutputValues, making us emit + // dynamic relocation with the right offset value. + getOrCreateIslandAccess(Address); + } + + bool hasDynamicRelocationAtIsland() const { + return !!(Islands && Islands->HasDynamicRelocations); + } + /// Called by an external function which wishes to emit references to constant /// island symbols of this function. We create a proxy for it, so we emit /// separate symbols when emitting our constant island on behalf of this other diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h index 20b4b5c92576f..d3c3ba485c75b 100644 --- a/bolt/include/bolt/Core/BinarySection.h +++ b/bolt/include/bolt/Core/BinarySection.h @@ -275,6 +275,7 @@ class BinarySection { bool isTBSS() const { return isBSS() && isTLS(); } bool isVirtual() const { return ELFType == ELF::SHT_NOBITS; } bool isRela() const { return ELFType == ELF::SHT_RELA; } + bool isRelr() const { return ELFType == ELF::SHT_RELR; } bool isWritable() const { return (ELFFlags & ELF::SHF_WRITE); } bool isAllocatable() const { if (isELF()) { diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index dfb593e794fc3..e6eb1429d3a18 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -310,6 +310,7 @@ class MCPlusBuilder { MaxAllocatorId++; // Build alias map initAliases(); + initSizeMap(); } /// Create and return target-specific MC symbolizer for the \p Function. @@ -414,6 +415,22 @@ class MCPlusBuilder { return Info->get(Inst.getOpcode()).isPseudo(); } + /// Return true if the relocation type needs to be registered in the function. + /// These code relocations are used in disassembly to better understand code. + /// + /// For ARM, they help us decode instruction operands unambiguously, but + /// sometimes we might discard them because we already have the necessary + /// information in the instruction itself (e.g. we don't need to record CALL + /// relocs in ARM because we can fully decode the target from the call + /// operand). + /// + /// For X86, they might be used in scanExternalRefs when we want to skip + /// a function but still patch references inside it. + virtual bool shouldRecordCodeRelocation(uint64_t RelType) const { + llvm_unreachable("not implemented"); + return false; + } + /// Creates x86 pause instruction. virtual void createPause(MCInst &Inst) const { llvm_unreachable("not implemented"); @@ -450,10 +467,11 @@ class MCPlusBuilder { virtual MCPhysReg getX86R11() const { llvm_unreachable("not implemented"); } /// Create increment contents of target by 1 for Instrumentation - virtual void createInstrIncMemory(InstructionListType &Instrs, - const MCSymbol *Target, MCContext *Ctx, - bool IsLeaf) const { + virtual InstructionListType createInstrIncMemory(const MCSymbol *Target, + MCContext *Ctx, + bool IsLeaf) const { llvm_unreachable("not implemented"); + return InstructionListType(); } /// Return a register number that is guaranteed to not match with @@ -496,15 +514,9 @@ class MCPlusBuilder { return false; } - virtual bool isPrefix(const MCInst &Inst) const { - llvm_unreachable("not implemented"); - return false; - } + virtual bool isPrefix(const MCInst &Inst) const { return false; } - virtual bool isRep(const MCInst &Inst) const { - llvm_unreachable("not implemented"); - return false; - } + virtual bool isRep(const MCInst &Inst) const { return false; } virtual bool deleteREPPrefix(MCInst &Inst) const { llvm_unreachable("not implemented"); @@ -515,10 +527,7 @@ class MCPlusBuilder { return Inst.getOpcode() == TargetOpcode::EH_LABEL; } - virtual bool isPop(const MCInst &Inst) const { - llvm_unreachable("not implemented"); - return false; - } + virtual bool isPop(const MCInst &Inst) const { return false; } /// Return true if the instruction is used to terminate an indirect branch. virtual bool isTerminateBranch(const MCInst &Inst) const { @@ -555,10 +564,7 @@ class MCPlusBuilder { return false; } - virtual bool isLeave(const MCInst &Inst) const { - llvm_unreachable("not implemented"); - return false; - } + virtual bool isLeave(const MCInst &Inst) const { return false; } virtual bool isADRP(const MCInst &Inst) const { llvm_unreachable("not implemented"); @@ -574,10 +580,7 @@ class MCPlusBuilder { llvm_unreachable("not implemented"); } - virtual bool isMoveMem2Reg(const MCInst &Inst) const { - llvm_unreachable("not implemented"); - return false; - } + virtual bool isMoveMem2Reg(const MCInst &Inst) const { return false; } virtual bool isLoad(const MCInst &Inst) const { llvm_unreachable("not implemented"); @@ -861,10 +864,7 @@ class MCPlusBuilder { } /// Return true if the instruction is encoded using EVEX (AVX-512). - virtual bool hasEVEXEncoding(const MCInst &Inst) const { - llvm_unreachable("not implemented"); - return false; - } + virtual bool hasEVEXEncoding(const MCInst &Inst) const { return false; } /// Return true if a pair of instructions represented by \p Insts /// could be fused into a single uop. @@ -1193,7 +1193,10 @@ class MCPlusBuilder { bool OnlySmaller = false) const; /// Initialize aliases tables. - virtual void initAliases(); + void initAliases(); + + /// Initialize register size table. + void initSizeMap(); /// Change \p Regs setting all registers used to pass parameters according /// to the host abi. Do nothing if not implemented. @@ -1236,7 +1239,7 @@ class MCPlusBuilder { } /// Return the register width in bytes (1, 2, 4 or 8) - virtual uint8_t getRegSize(MCPhysReg Reg) const; + uint8_t getRegSize(MCPhysReg Reg) const { return SizeMap[Reg]; } /// For aliased registers, return an alias of \p Reg that has the width of /// \p Size bytes @@ -1986,6 +1989,8 @@ class MCPlusBuilder { // alias (are sub or superregs of itself, including itself). std::vector AliasMap; std::vector SmallerAliasMap; + // SizeMap caches a mapping of registers to their sizes. + std::vector SizeMap; }; MCPlusBuilder *createX86MCPlusBuilder(const MCInstrAnalysis *, diff --git a/bolt/include/bolt/Core/Relocation.h b/bolt/include/bolt/Core/Relocation.h index ed56d41beb05b..51dd769387565 100644 --- a/bolt/include/bolt/Core/Relocation.h +++ b/bolt/include/bolt/Core/Relocation.h @@ -109,6 +109,9 @@ struct Relocation { /// Return code for a ABS 8-byte relocation static uint64_t getAbs64(); + /// Return code for a RELATIVE relocation + static uint64_t getRelative(); + /// Return true if this relocation is PC-relative. Return false otherwise. bool isPCRelative() const { return isPCRelative(Type); } diff --git a/bolt/include/bolt/Profile/DataAggregator.h b/bolt/include/bolt/Profile/DataAggregator.h index 7f96f4fa732ae..45b652c8a3707 100644 --- a/bolt/include/bolt/Profile/DataAggregator.h +++ b/bolt/include/bolt/Profile/DataAggregator.h @@ -315,6 +315,12 @@ class DataAggregator : public DataReader { /// consume it (peek only). bool checkNewLine(); + using PerfProcessErrorCallbackTy = std::function; + /// Prepare to parse data from a given perf script invocation. + /// Returns an invocation exit code. + int prepareToParse(StringRef Name, PerfProcessInfo &Process, + PerfProcessErrorCallbackTy Callback); + /// Parse a single LBR entry as output by perf script -Fbrstack ErrorOr parseLBREntry(); diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h index c8d5f03d085c7..7e4f7c3e2187e 100644 --- a/bolt/include/bolt/Rewrite/RewriteInstance.h +++ b/bolt/include/bolt/Rewrite/RewriteInstance.h @@ -138,6 +138,9 @@ class RewriteInstance { /// Read relocations from a given section. void readDynamicRelocations(const object::SectionRef &Section, bool IsJmpRel); + /// Read relocations from a given RELR section. + void readDynamicRelrRelocations(BinarySection &Section); + /// Print relocation information. void printRelocationInfo(const RelocationRef &Rel, StringRef SymbolName, uint64_t SymbolAddress, uint64_t Addend, @@ -312,6 +315,9 @@ class RewriteInstance { /// Patch allocatable relocation sections. ELF_FUNCTION(void, patchELFAllocatableRelaSections); + /// Patch allocatable relr section. + ELF_FUNCTION(void, patchELFAllocatableRelrSection); + /// Finalize memory image of section header string table. ELF_FUNCTION(void, finalizeSectionStringTable); @@ -474,6 +480,10 @@ class RewriteInstance { uint64_t NewTextSegmentOffset{0}; uint64_t NewTextSegmentSize{0}; + /// New writable segment info. + uint64_t NewWritableSegmentAddress{0}; + uint64_t NewWritableSegmentSize{0}; + /// Track next available address for new allocatable sections. uint64_t NextAvailableAddress{0}; @@ -482,6 +492,11 @@ class RewriteInstance { uint64_t DynamicRelocationsSize{0}; uint64_t DynamicRelativeRelocationsCount{0}; + // Location and size of .relr.dyn relocations. + std::optional DynamicRelrAddress; + uint64_t DynamicRelrSize{0}; + uint64_t DynamicRelrEntrySize{0}; + /// PLT relocations are special kind of dynamic relocations stored separately. std::optional PLTRelocationsAddress; uint64_t PLTRelocationsSize{0}; @@ -547,18 +562,6 @@ class RewriteInstance { ErrorOr LSDASection{std::errc::bad_address}; ErrorOr EHFrameSection{std::errc::bad_address}; - /// .got.plt sections. - /// - /// Contains jump slots (addresses) indirectly referenced by - /// instructions in .plt section. - ErrorOr GOTPLTSection{std::errc::bad_address}; - - /// .rela.plt section. - /// - /// Contains relocations against .got.plt. - ErrorOr RelaPLTSection{std::errc::bad_address}; - ErrorOr RelaDynSection{std::errc::bad_address}; - /// .note.gnu.build-id section. ErrorOr BuildIDSection{std::errc::bad_address}; diff --git a/bolt/lib/Core/BinaryBasicBlock.cpp b/bolt/lib/Core/BinaryBasicBlock.cpp index 0a582d7096667..b271b86ec6992 100644 --- a/bolt/lib/Core/BinaryBasicBlock.cpp +++ b/bolt/lib/Core/BinaryBasicBlock.cpp @@ -593,19 +593,6 @@ BinaryBasicBlock::getBranchInfo(const BinaryBasicBlock &Succ) const { return std::get<1>(*Result); } -BinaryBasicBlock::BinaryBranchInfo & -BinaryBasicBlock::getBranchInfo(const MCSymbol *Label) { - auto BI = branch_info_begin(); - for (BinaryBasicBlock *BB : successors()) { - if (BB->getLabel() == Label) - return *BI; - ++BI; - } - - llvm_unreachable("Invalid successor"); - return *BI; -} - BinaryBasicBlock *BinaryBasicBlock::splitAt(iterator II) { assert(II != end() && "expected iterator pointing to instruction"); diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp index 19e356410198c..09f59f7ad6321 100644 --- a/bolt/lib/Core/BinaryContext.cpp +++ b/bolt/lib/Core/BinaryContext.cpp @@ -391,8 +391,18 @@ BinaryContext::handleAddressRef(uint64_t Address, BinaryFunction &BF, --IslandIter; if (IslandIter != AddressToConstantIslandMap.end()) { - if (MCSymbol *IslandSym = - IslandIter->second->getOrCreateProxyIslandAccess(Address, BF)) { + // Fall-back to referencing the original constant island in the presence + // of dynamic relocs, as we currently do not support cloning them. + // Notice: we might fail to link because of this, if the original constant + // island we are referring would be emitted too far away. + if (IslandIter->second->hasDynamicRelocationAtIsland()) { + MCSymbol *IslandSym = + IslandIter->second->getOrCreateIslandAccess(Address); + if (IslandSym) + return std::make_pair(IslandSym, 0); + } else if (MCSymbol *IslandSym = + IslandIter->second->getOrCreateProxyIslandAccess(Address, + BF)) { BF.createIslandDependency(IslandSym, IslandIter->second); return std::make_pair(IslandSym, 0); } diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index 01e7de53da55b..1cbdd6b2721b8 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -987,7 +987,7 @@ size_t BinaryFunction::getSizeOfDataInCodeAt(uint64_t Offset) const { if (!Islands) return 0; - if (Islands->DataOffsets.find(Offset) == Islands->DataOffsets.end()) + if (!llvm::is_contained(Islands->DataOffsets, Offset)) return 0; auto Iter = Islands->CodeOffsets.upper_bound(Offset); @@ -2589,8 +2589,7 @@ struct CFISnapshotDiff : public CFISnapshot { if (RestoredRegs[Reg]) return true; RestoredRegs[Reg] = true; - const int32_t CurRegRule = - RegRule.find(Reg) != RegRule.end() ? RegRule[Reg] : UNKNOWN; + const int32_t CurRegRule = RegRule.contains(Reg) ? RegRule[Reg] : UNKNOWN; if (CurRegRule == UNKNOWN) { if (Instr.getOperation() == MCCFIInstruction::OpRestore || Instr.getOperation() == MCCFIInstruction::OpSameValue) @@ -2731,7 +2730,7 @@ BinaryFunction::unwindCFIState(int32_t FromState, int32_t ToState, Reg = *R; } - if (ToCFITable.RegRule.find(Reg) == ToCFITable.RegRule.end()) { + if (!ToCFITable.RegRule.contains(Reg)) { FrameInstructions.emplace_back( MCCFIInstruction::createRestore(nullptr, Reg)); if (FromCFITable.isRedundant(FrameInstructions.back())) { @@ -4088,6 +4087,16 @@ void BinaryFunction::updateOutputValues(const MCAsmLayout &Layout) { const uint64_t DataOffset = Layout.getSymbolOffset(*getFunctionConstantIslandLabel()); setOutputDataAddress(BaseAddress + DataOffset); + for (auto It : Islands->Offsets) { + const uint64_t OldOffset = It.first; + BinaryData *BD = BC.getBinaryDataAtAddress(getAddress() + OldOffset); + if (!BD) + continue; + + MCSymbol *Symbol = It.second; + const uint64_t NewOffset = Layout.getSymbolOffset(*Symbol); + BD->setOutputLocation(*getCodeSection(), NewOffset); + } } if (isSplit()) { for (FunctionFragment &FF : getLayout().getSplitFragments()) { @@ -4507,5 +4516,21 @@ bool BinaryFunction::isAArch64Veneer() const { return true; } +void BinaryFunction::addRelocation(uint64_t Address, MCSymbol *Symbol, + uint64_t RelType, uint64_t Addend, + uint64_t Value) { + assert(Address >= getAddress() && Address < getAddress() + getMaxSize() && + "address is outside of the function"); + uint64_t Offset = Address - getAddress(); + LLVM_DEBUG(dbgs() << "BOLT-DEBUG: addRelocation in " + << formatv("{0}@{1:x} against {2}\n", this, Offset, + Symbol->getName())); + bool IsCI = BC.isAArch64() && isInConstantIsland(Address); + std::map &Rels = + IsCI ? Islands->Relocations : Relocations; + if (BC.MIB->shouldRecordCodeRelocation(RelType)) + Rels[Offset] = Relocation{Offset, Symbol, RelType, Addend, Value}; +} + } // namespace bolt } // namespace llvm diff --git a/bolt/lib/Core/MCPlusBuilder.cpp b/bolt/lib/Core/MCPlusBuilder.cpp index a8b2d12d10afc..b2a102d318313 100644 --- a/bolt/lib/Core/MCPlusBuilder.cpp +++ b/bolt/lib/Core/MCPlusBuilder.cpp @@ -464,17 +464,9 @@ void MCPlusBuilder::initAliases() { } // Propagate smaller alias info upwards. Skip reg 0 (mapped to NoRegister) - std::queue Worklist; for (MCPhysReg I = 1, E = RegInfo->getNumRegs(); I < E; ++I) - Worklist.push(I); - while (!Worklist.empty()) { - MCPhysReg I = Worklist.front(); - Worklist.pop(); for (MCSubRegIterator SI(I, RegInfo); SI.isValid(); ++SI) SmallerAliasMap[I] |= SmallerAliasMap[*SI]; - for (MCSuperRegIterator SI(I, RegInfo); SI.isValid(); ++SI) - Worklist.push(*SI); - } LLVM_DEBUG({ dbgs() << "Dumping reg alias table:\n"; @@ -491,22 +483,12 @@ void MCPlusBuilder::initAliases() { }); } -uint8_t MCPlusBuilder::getRegSize(MCPhysReg Reg) const { - // SizeMap caches a mapping of registers to their sizes - static std::vector SizeMap; - - if (SizeMap.size() > 0) { - return SizeMap[Reg]; - } - SizeMap = std::vector(RegInfo->getNumRegs()); +void MCPlusBuilder::initSizeMap() { + SizeMap.resize(RegInfo->getNumRegs()); // Build size map - for (auto I = RegInfo->regclass_begin(), E = RegInfo->regclass_end(); I != E; - ++I) { - for (MCPhysReg Reg : *I) - SizeMap[Reg] = I->getSizeInBits() / 8; - } - - return SizeMap[Reg]; + for (auto RC : RegInfo->regclasses()) + for (MCPhysReg Reg : RC) + SizeMap[Reg] = RC.getSizeInBits() / 8; } bool MCPlusBuilder::setOperandToSymbolRef(MCInst &Inst, int OpNum, diff --git a/bolt/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp index eceecf3ef03c4..b36e321f4e342 100644 --- a/bolt/lib/Core/Relocation.cpp +++ b/bolt/lib/Core/Relocation.cpp @@ -642,6 +642,12 @@ uint64_t Relocation::getAbs64() { return ELF::R_X86_64_64; } +uint64_t Relocation::getRelative() { + if (Arch == Triple::aarch64) + return ELF::R_AARCH64_RELATIVE; + return ELF::R_X86_64_RELATIVE; +} + size_t Relocation::emit(MCStreamer *Streamer) const { const size_t Size = getSizeForType(Type); MCContext &Ctx = Streamer->getContext(); diff --git a/bolt/lib/Passes/FrameAnalysis.cpp b/bolt/lib/Passes/FrameAnalysis.cpp index 9aa5dee4c06a5..1e6be498a4790 100644 --- a/bolt/lib/Passes/FrameAnalysis.cpp +++ b/bolt/lib/Passes/FrameAnalysis.cpp @@ -352,7 +352,7 @@ bool FrameAnalysis::updateArgsTouchedFor(const BinaryFunction &BF, MCInst &Inst, // offset specially after an epilogue, where tailcalls happen. It should be // -8. for (std::pair Elem : Iter->second) { - if (ArgsTouchedMap[&BF].find(Elem) == ArgsTouchedMap[&BF].end()) { + if (!llvm::is_contained(ArgsTouchedMap[&BF], Elem)) { ArgsTouchedMap[&BF].emplace(Elem); Changed = true; } diff --git a/bolt/lib/Passes/IndirectCallPromotion.cpp b/bolt/lib/Passes/IndirectCallPromotion.cpp index 8b20fbd368c07..ea8019431cf52 100644 --- a/bolt/lib/Passes/IndirectCallPromotion.cpp +++ b/bolt/lib/Passes/IndirectCallPromotion.cpp @@ -257,14 +257,14 @@ IndirectCallPromotion::getCallTargets(BinaryBasicBlock &BB, JT->EntrySize == BC.AsmInfo->getCodePointerSize()); for (size_t I = Range.first; I < Range.second; ++I, JI += JIAdj) { MCSymbol *Entry = JT->Entries[I]; - assert(BF.getBasicBlockForLabel(Entry) || - Entry == BF.getFunctionEndLabel() || + const BinaryBasicBlock *ToBB = BF.getBasicBlockForLabel(Entry); + assert(ToBB || Entry == BF.getFunctionEndLabel() || Entry == BF.getFunctionEndLabel(FragmentNum::cold())); if (Entry == BF.getFunctionEndLabel() || Entry == BF.getFunctionEndLabel(FragmentNum::cold())) continue; const Location To(Entry); - const BinaryBasicBlock::BinaryBranchInfo &BI = BB.getBranchInfo(Entry); + const BinaryBasicBlock::BinaryBranchInfo &BI = BB.getBranchInfo(*ToBB); Targets.emplace_back(From, To, BI.MispredictedCount, BI.Count, I - Range.first); } diff --git a/bolt/lib/Passes/Instrumentation.cpp b/bolt/lib/Passes/Instrumentation.cpp index c6e1bf9e6b16a..ddab5e6414dd4 100644 --- a/bolt/lib/Passes/Instrumentation.cpp +++ b/bolt/lib/Passes/Instrumentation.cpp @@ -174,12 +174,9 @@ void Instrumentation::createLeafNodeDescription(FunctionDescription &FuncDesc, InstructionListType Instrumentation::createInstrumentationSnippet(BinaryContext &BC, bool IsLeaf) { auto L = BC.scopeLock(); - MCSymbol *Label; - Label = BC.Ctx->createNamedTempSymbol("InstrEntry"); + MCSymbol *Label = BC.Ctx->createNamedTempSymbol("InstrEntry"); Summary->Counters.emplace_back(Label); - InstructionListType CounterInstrs; - BC.MIB->createInstrIncMemory(CounterInstrs, Label, &*BC.Ctx, IsLeaf); - return CounterInstrs; + return BC.MIB->createInstrIncMemory(Label, BC.Ctx.get(), IsLeaf); } // Helper instruction sequence insertion function @@ -231,8 +228,9 @@ bool Instrumentation::instrumentOneTarget( BinaryBasicBlock &FromBB, uint32_t From, BinaryFunction &ToFunc, BinaryBasicBlock *TargetBB, uint32_t ToOffset, bool IsLeaf, bool IsInvoke, FunctionDescription *FuncDesc, uint32_t FromNodeID, uint32_t ToNodeID) { + BinaryContext &BC = FromFunction.getBinaryContext(); { - auto L = FromFunction.getBinaryContext().scopeLock(); + auto L = BC.scopeLock(); bool Created = true; if (!TargetBB) Created = createCallDescription(*FuncDesc, FromFunction, From, FromNodeID, @@ -245,10 +243,8 @@ bool Instrumentation::instrumentOneTarget( return false; } - InstructionListType CounterInstrs = - createInstrumentationSnippet(FromFunction.getBinaryContext(), IsLeaf); + InstructionListType CounterInstrs = createInstrumentationSnippet(BC, IsLeaf); - BinaryContext &BC = FromFunction.getBinaryContext(); const MCInst &Inst = *Iter; if (BC.MIB->isCall(Inst)) { // This code handles both @@ -341,7 +337,7 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function, const BinaryBasicBlock *Pred; std::tie(Pred, BB) = Stack.top(); Stack.pop(); - if (VisitedSet.find(BB) != VisitedSet.end()) + if (llvm::is_contained(VisitedSet, BB)) continue; VisitedSet.insert(BB); @@ -409,7 +405,7 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function, } if (TargetFunc) { // Do not instrument edges in the spanning tree - if (STOutSet[&BB].find(TargetBB) != STOutSet[&BB].end()) { + if (llvm::is_contained(STOutSet[&BB], TargetBB)) { auto L = BC.scopeLock(); createEdgeDescription(*FuncDesc, Function, FromOffset, BBToID[&BB], Function, ToOffset, BBToID[TargetBB], @@ -426,7 +422,7 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function, if (IsJumpTable) { for (BinaryBasicBlock *&Succ : BB.successors()) { // Do not instrument edges in the spanning tree - if (STOutSet[&BB].find(&*Succ) != STOutSet[&BB].end()) { + if (llvm::is_contained(STOutSet[&BB], &*Succ)) { auto L = BC.scopeLock(); createEdgeDescription(*FuncDesc, Function, FromOffset, BBToID[&BB], Function, Succ->getInputOffset(), @@ -469,7 +465,7 @@ void Instrumentation::instrumentFunction(BinaryFunction &Function, FromOffset = *BC.MIB->getOffset(*LastInstr); // Do not instrument edges in the spanning tree - if (STOutSet[&BB].find(FTBB) != STOutSet[&BB].end()) { + if (llvm::is_contained(STOutSet[&BB], FTBB)) { auto L = BC.scopeLock(); createEdgeDescription(*FuncDesc, Function, FromOffset, BBToID[&BB], Function, FTBB->getInputOffset(), BBToID[FTBB], diff --git a/bolt/lib/Passes/ReorderFunctions.cpp b/bolt/lib/Passes/ReorderFunctions.cpp index 19ccf9f6fc80e..2fc99f652bf1c 100644 --- a/bolt/lib/Passes/ReorderFunctions.cpp +++ b/bolt/lib/Passes/ReorderFunctions.cpp @@ -360,7 +360,7 @@ void ReorderFunctions::runOnFunctions(BinaryContext &BC) { } // Strip LTO suffixes if (std::optional CommonName = getLTOCommonName(Function)) - if (LTOCommonNameMap.find(*CommonName) != LTOCommonNameMap.end()) + if (LTOCommonNameMap.contains(*CommonName)) llvm::append_range(FuncAddrs, LTOCommonNameMap[*CommonName]); } else { FuncAddrs.push_back(BD->getAddress()); diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index c0f4ce258079d..7d72c3b0e372f 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -467,6 +467,45 @@ void DataAggregator::filterBinaryMMapInfo() { } } +int DataAggregator::prepareToParse(StringRef Name, PerfProcessInfo &Process, + PerfProcessErrorCallbackTy Callback) { + std::string Error; + outs() << "PERF2BOLT: waiting for perf " << Name + << " collection to finish...\n"; + sys::ProcessInfo PI = sys::Wait(Process.PI, std::nullopt, &Error); + + if (!Error.empty()) { + errs() << "PERF-ERROR: " << PerfPath << ": " << Error << "\n"; + deleteTempFiles(); + exit(1); + } + + if (PI.ReturnCode != 0) { + ErrorOr> ErrorMB = + MemoryBuffer::getFileOrSTDIN(Process.StderrPath.data()); + StringRef ErrBuf = (*ErrorMB)->getBuffer(); + + deleteTempFiles(); + Callback(PI.ReturnCode, ErrBuf); + return PI.ReturnCode; + } + + ErrorOr> MB = + MemoryBuffer::getFileOrSTDIN(Process.StdoutPath.data()); + if (std::error_code EC = MB.getError()) { + errs() << "Cannot open " << Process.StdoutPath.data() << ": " + << EC.message() << "\n"; + deleteTempFiles(); + exit(1); + } + + FileBuf = std::move(*MB); + ParsingBuf = FileBuf->getBuffer(); + Col = 0; + Line = 1; + return PI.ReturnCode; +} + Error DataAggregator::preprocessProfile(BinaryContext &BC) { this->BC = &BC; @@ -483,42 +522,16 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { "not read one from input binary\n"; } - auto prepareToParse = [&](StringRef Name, PerfProcessInfo &Process) { - std::string Error; - outs() << "PERF2BOLT: waiting for perf " << Name - << " collection to finish...\n"; - sys::ProcessInfo PI = sys::Wait(Process.PI, std::nullopt, &Error); - - if (!Error.empty()) { - errs() << "PERF-ERROR: " << PerfPath << ": " << Error << "\n"; - deleteTempFiles(); - exit(1); - } - - if (PI.ReturnCode != 0) { - ErrorOr> ErrorMB = - MemoryBuffer::getFileOrSTDIN(Process.StderrPath.data()); - StringRef ErrBuf = (*ErrorMB)->getBuffer(); - - errs() << "PERF-ERROR: return code " << PI.ReturnCode << "\n"; - errs() << ErrBuf; - deleteTempFiles(); - exit(1); - } - - ErrorOr> MB = - MemoryBuffer::getFileOrSTDIN(Process.StdoutPath.data()); - if (std::error_code EC = MB.getError()) { - errs() << "Cannot open " << Process.StdoutPath.data() << ": " - << EC.message() << "\n"; - deleteTempFiles(); - exit(1); - } + auto ErrorCallback = [](int ReturnCode, StringRef ErrBuf) { + errs() << "PERF-ERROR: return code " << ReturnCode << "\n" << ErrBuf; + exit(1); + }; - FileBuf = std::move(*MB); - ParsingBuf = FileBuf->getBuffer(); - Col = 0; - Line = 1; + auto MemEventsErrorCallback = [&](int ReturnCode, StringRef ErrBuf) { + Regex NoData("Samples for '.*' event do not have ADDR attribute set. " + "Cannot print 'addr' field."); + if (!NoData.match(ErrBuf)) + ErrorCallback(ReturnCode, ErrBuf); }; if (opts::LinuxKernelMode) { @@ -537,17 +550,17 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { // in Linux kernel mode. opts::IgnoreInterruptLBR = false; } else { - prepareToParse("mmap events", MMapEventsPPI); + prepareToParse("mmap events", MMapEventsPPI, ErrorCallback); if (parseMMapEvents()) errs() << "PERF2BOLT: failed to parse mmap events\n"; } - prepareToParse("task events", TaskEventsPPI); + prepareToParse("task events", TaskEventsPPI, ErrorCallback); if (parseTaskEvents()) errs() << "PERF2BOLT: failed to parse task events\n"; filterBinaryMMapInfo(); - prepareToParse("events", MainEventsPPI); + prepareToParse("events", MainEventsPPI, ErrorCallback); if (opts::HeatmapMode) { if (std::error_code EC = printLBRHeatMap()) { @@ -571,38 +584,9 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { } // Special handling for memory events - std::string Error; - sys::ProcessInfo PI = sys::Wait(MemEventsPPI.PI, std::nullopt, &Error); - if (PI.ReturnCode != 0) { - ErrorOr> MB = - MemoryBuffer::getFileOrSTDIN(MemEventsPPI.StderrPath.data()); - StringRef ErrBuf = (*MB)->getBuffer(); - - deleteTempFiles(); - - Regex NoData("Samples for '.*' event do not have ADDR attribute set. " - "Cannot print 'addr' field."); - if (!NoData.match(ErrBuf)) { - errs() << "PERF-ERROR: return code " << PI.ReturnCode << "\n"; - errs() << ErrBuf; - exit(1); - } + if (prepareToParse("mem events", MemEventsPPI, MemEventsErrorCallback)) return Error::success(); - } - ErrorOr> MB = - MemoryBuffer::getFileOrSTDIN(MemEventsPPI.StdoutPath.data()); - if (std::error_code EC = MB.getError()) { - errs() << "Cannot open " << MemEventsPPI.StdoutPath.data() << ": " - << EC.message() << "\n"; - deleteTempFiles(); - exit(1); - } - - FileBuf = std::move(*MB); - ParsingBuf = FileBuf->getBuffer(); - Col = 0; - Line = 1; if (const std::error_code EC = parseMemEvents()) errs() << "PERF2BOLT: failed to parse memory events: " << EC.message() << '\n'; diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp index d2ccadd62a172..13f56ed51c3ac 100644 --- a/bolt/lib/Profile/YAMLProfileReader.cpp +++ b/bolt/lib/Profile/YAMLProfileReader.cpp @@ -285,10 +285,10 @@ Error YAMLProfileReader::preprocessProfile(BinaryContext &BC) { bool YAMLProfileReader::mayHaveProfileData(const BinaryFunction &BF) { for (StringRef Name : BF.getNames()) { - if (ProfileNameToProfile.find(Name) != ProfileNameToProfile.end()) + if (ProfileNameToProfile.contains(Name)) return true; if (const std::optional CommonName = getLTOCommonName(Name)) { - if (LTOCommonNameMap.find(*CommonName) != LTOCommonNameMap.end()) + if (LTOCommonNameMap.contains(*CommonName)) return true; } } diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index a4460dfeef169..04ccbcf20de11 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -1278,6 +1278,25 @@ void RewriteInstance::discoverFileObjects() { llvm_unreachable("Unknown marker"); } + if (BC->isAArch64()) { + // Check for dynamic relocations that might be contained in + // constant islands. + for (const BinarySection &Section : BC->allocatableSections()) { + const uint64_t SectionAddress = Section.getAddress(); + for (const Relocation &Rel : Section.dynamicRelocations()) { + const uint64_t RelAddress = SectionAddress + Rel.Offset; + BinaryFunction *BF = + BC->getBinaryFunctionContainingAddress(RelAddress, + /*CheckPastEnd*/ false, + /*UseMaxSize*/ true); + if (BF) { + assert(Rel.isRelative() && "Expected relative relocation for island"); + BF->markIslandDynamicRelocationAtAddress(RelAddress); + } + } + } + } + if (opts::LinuxKernelMode) { // Read all special linux kernel sections and their relocations processLKSections(); @@ -1679,9 +1698,6 @@ Error RewriteInstance::readSpecialSections() { HasSymbolTable = (bool)BC->getUniqueSectionByName(".symtab"); LSDASection = BC->getUniqueSectionByName(".gcc_except_table"); EHFrameSection = BC->getUniqueSectionByName(".eh_frame"); - GOTPLTSection = BC->getUniqueSectionByName(".got.plt"); - RelaPLTSection = BC->getUniqueSectionByName(".rela.plt"); - RelaDynSection = BC->getUniqueSectionByName(".rela.dyn"); BuildIDSection = BC->getUniqueSectionByName(".note.gnu.build-id"); SDTSection = BC->getUniqueSectionByName(".note.stapsdt"); PseudoProbeDescSection = BC->getUniqueSectionByName(".pseudo_probe_desc"); @@ -2058,6 +2074,19 @@ bool RewriteInstance::analyzeRelocation( } void RewriteInstance::processDynamicRelocations() { + // Read .relr.dyn section containing compressed R_*_RELATIVE relocations. + if (DynamicRelrSize > 0) { + ErrorOr DynamicRelrSectionOrErr = + BC->getSectionForAddress(*DynamicRelrAddress); + if (!DynamicRelrSectionOrErr) + report_error("unable to find section corresponding to DT_RELR", + DynamicRelrSectionOrErr.getError()); + if (DynamicRelrSectionOrErr->getSize() != DynamicRelrSize) + report_error("section size mismatch for DT_RELRSZ", + errc::executable_format_error); + readDynamicRelrRelocations(*DynamicRelrSectionOrErr); + } + // Read relocations for PLT - DT_JMPREL. if (PLTRelocationsSize > 0) { ErrorOr PLTRelSectionOrErr = @@ -2356,6 +2385,60 @@ void RewriteInstance::readDynamicRelocations(const SectionRef &Section, } } +void RewriteInstance::readDynamicRelrRelocations(BinarySection &Section) { + assert(Section.isAllocatable() && "allocatable expected"); + + LLVM_DEBUG({ + StringRef SectionName = Section.getName(); + dbgs() << "BOLT-DEBUG: reading relocations in section " << SectionName + << ":\n"; + }); + + const uint64_t RType = Relocation::getRelative(); + const uint8_t PSize = BC->AsmInfo->getCodePointerSize(); + const uint64_t MaxDelta = ((CHAR_BIT * DynamicRelrEntrySize) - 1) * PSize; + + auto ExtractAddendValue = [&](uint64_t Address) -> uint64_t { + ErrorOr Section = BC->getSectionForAddress(Address); + assert(Section && "cannot get section for data address from RELR"); + DataExtractor DE = DataExtractor(Section->getContents(), + BC->AsmInfo->isLittleEndian(), PSize); + uint64_t Offset = Address - Section->getAddress(); + return DE.getUnsigned(&Offset, PSize); + }; + + auto AddRelocation = [&](uint64_t Address) { + uint64_t Addend = ExtractAddendValue(Address); + LLVM_DEBUG(dbgs() << "BOLT-DEBUG: R_*_RELATIVE relocation at 0x" + << Twine::utohexstr(Address) << " to 0x" + << Twine::utohexstr(Addend) << '\n';); + BC->addDynamicRelocation(Address, nullptr, RType, Addend); + }; + + DataExtractor DE = DataExtractor(Section.getContents(), + BC->AsmInfo->isLittleEndian(), PSize); + uint64_t Offset = 0, Address = 0; + uint64_t RelrCount = DynamicRelrSize / DynamicRelrEntrySize; + while (RelrCount--) { + assert(DE.isValidOffset(Offset)); + uint64_t Entry = DE.getUnsigned(&Offset, DynamicRelrEntrySize); + if ((Entry & 1) == 0) { + AddRelocation(Entry); + Address = Entry + PSize; + } else { + const uint64_t StartAddress = Address; + while (Entry >>= 1) { + if (Entry & 1) + AddRelocation(Address); + + Address += PSize; + } + + Address = StartAddress + MaxDelta; + } + } +} + void RewriteInstance::printRelocationInfo(const RelocationRef &Rel, StringRef SymbolName, uint64_t SymbolAddress, @@ -3177,6 +3260,12 @@ void RewriteInstance::postProcessFunctions() { for (auto &BFI : BC->getBinaryFunctions()) { BinaryFunction &Function = BFI.second; + // Set function as non-simple if it has dynamic relocations + // in constant island, we don't want this function to be optimized + // e.g. function splitting is unsupported. + if (Function.hasDynamicRelocationAtIsland()) + Function.setSimple(false); + if (Function.empty()) continue; @@ -4042,6 +4131,13 @@ void RewriteInstance::mapAllocatableSections(RuntimeDyld &RTDyld) { // Allocate read-only sections first, then writable sections. enum : uint8_t { ST_READONLY, ST_READWRITE }; for (uint8_t SType = ST_READONLY; SType <= ST_READWRITE; ++SType) { + const uint64_t LastNextAvailableAddress = NextAvailableAddress; + if (SType == ST_READWRITE) { + // Align R+W segment to regular page size + NextAvailableAddress = alignTo(NextAvailableAddress, BC->RegularPageSize); + NewWritableSegmentAddress = NextAvailableAddress; + } + for (BinarySection &Section : BC->allocatableSections()) { if (!Section.hasValidSectionID()) continue; @@ -4088,6 +4184,21 @@ void RewriteInstance::mapAllocatableSections(RuntimeDyld &RTDyld) { NextAvailableAddress += Section.getOutputSize(); } } + + if (SType == ST_READONLY) { + if (PHDRTableAddress) { + // Segment size includes the size of the PHDR area. + NewTextSegmentSize = NextAvailableAddress - PHDRTableAddress; + } else { + // Existing PHDR table would be updated. + NewTextSegmentSize = NextAvailableAddress - NewTextSegmentAddress; + } + } else if (SType == ST_READWRITE) { + NewWritableSegmentSize = NextAvailableAddress - NewWritableSegmentAddress; + // Restore NextAvailableAddress if no new writable sections + if (!NewWritableSegmentSize) + NextAvailableAddress = LastNextAvailableAddress; + } } } @@ -4108,16 +4219,32 @@ void RewriteInstance::patchELFPHDRTable() { // Write/re-write program headers. Phnum = Obj.getHeader().e_phnum; if (PHDRTableOffset) { - // Writing new pheader table. - Phnum += 1; // only adding one new segment - // Segment size includes the size of the PHDR area. - NewTextSegmentSize = NextAvailableAddress - PHDRTableAddress; + // Writing new pheader table and adding one new entry for R+X segment. + Phnum += 1; + if (NewWritableSegmentSize) { + // Adding one more entry for R+W segment. + Phnum += 1; + } } else { assert(!PHDRTableAddress && "unexpected address for program header table"); - // Update existing table. PHDRTableOffset = Obj.getHeader().e_phoff; - NewTextSegmentSize = NextAvailableAddress - NewTextSegmentAddress; + if (NewWritableSegmentSize) { + errs() << "Unable to add writable segment with UseGnuStack option\n"; + exit(1); + } + } + + // NOTE Currently .eh_frame_hdr appends to the last segment, recalculate + // last segments size based on the NextAvailableAddress variable. + if (!NewWritableSegmentSize) { + if (PHDRTableAddress) + NewTextSegmentSize = NextAvailableAddress - PHDRTableAddress; + else + NewTextSegmentSize = NextAvailableAddress - NewTextSegmentAddress; + } else { + NewWritableSegmentSize = NextAvailableAddress - NewWritableSegmentAddress; } + OS.seek(PHDRTableOffset); bool ModdedGnuStack = false; @@ -4149,6 +4276,19 @@ void RewriteInstance::patchELFPHDRTable() { return NewPhdr; }; + auto createNewWritableSectionsPhdr = [&]() { + ELF64LEPhdrTy NewPhdr; + NewPhdr.p_type = ELF::PT_LOAD; + NewPhdr.p_offset = getFileOffsetForAddress(NewWritableSegmentAddress); + NewPhdr.p_vaddr = NewWritableSegmentAddress; + NewPhdr.p_paddr = NewWritableSegmentAddress; + NewPhdr.p_filesz = NewWritableSegmentSize; + NewPhdr.p_memsz = NewWritableSegmentSize; + NewPhdr.p_align = BC->RegularPageSize; + NewPhdr.p_flags = ELF::PF_R | ELF::PF_W; + return NewPhdr; + }; + // Copy existing program headers with modifications. for (const ELF64LE::Phdr &Phdr : cantFail(Obj.program_headers())) { ELF64LE::Phdr NewPhdr = Phdr; @@ -4177,6 +4317,11 @@ void RewriteInstance::patchELFPHDRTable() { ELF64LE::Phdr NewTextPhdr = createNewTextPhdr(); OS.write(reinterpret_cast(&NewTextPhdr), sizeof(NewTextPhdr)); + if (NewWritableSegmentSize) { + ELF64LEPhdrTy NewWritablePhdr = createNewWritableSectionsPhdr(); + OS.write(reinterpret_cast(&NewWritablePhdr), + sizeof(NewWritablePhdr)); + } AddedSegment = true; } OS.write(reinterpret_cast(&NewPhdr), sizeof(NewPhdr)); @@ -4186,6 +4331,11 @@ void RewriteInstance::patchELFPHDRTable() { // Append the new header to the end of the table. ELF64LE::Phdr NewTextPhdr = createNewTextPhdr(); OS.write(reinterpret_cast(&NewTextPhdr), sizeof(NewTextPhdr)); + if (NewWritableSegmentSize) { + ELF64LEPhdrTy NewWritablePhdr = createNewWritableSectionsPhdr(); + OS.write(reinterpret_cast(&NewWritablePhdr), + sizeof(NewWritablePhdr)); + } } assert((!opts::UseGnuStack || ModdedGnuStack) && @@ -5120,6 +5270,101 @@ void RewriteInstance::patchELFSymTabs(ELFObjectFile *File) { ELF::SHT_STRTAB); } +template +void RewriteInstance::patchELFAllocatableRelrSection( + ELFObjectFile *File) { + if (!DynamicRelrAddress) + return; + + raw_fd_ostream &OS = Out->os(); + const uint8_t PSize = BC->AsmInfo->getCodePointerSize(); + const uint64_t MaxDelta = ((CHAR_BIT * DynamicRelrEntrySize) - 1) * PSize; + + auto FixAddend = [&](const BinarySection &Section, const Relocation &Rel) { + // Fix relocation symbol value in place if no static relocation found + // on the same address + if (Section.getRelocationAt(Rel.Offset)) + return; + + // No fixup needed if symbol address was not changed + const uint64_t Addend = getNewFunctionOrDataAddress(Rel.Addend); + if (!Addend) + return; + + uint64_t FileOffset = Section.getOutputFileOffset(); + if (!FileOffset) + FileOffset = Section.getInputFileOffset(); + + FileOffset += Rel.Offset; + OS.pwrite(reinterpret_cast(&Addend), PSize, FileOffset); + }; + + // Fill new relative relocation offsets set + std::set RelOffsets; + for (const BinarySection &Section : BC->allocatableSections()) { + const uint64_t SectionInputAddress = Section.getAddress(); + uint64_t SectionAddress = Section.getOutputAddress(); + if (!SectionAddress) + SectionAddress = SectionInputAddress; + + for (const Relocation &Rel : Section.dynamicRelocations()) { + if (!Rel.isRelative()) + continue; + + uint64_t RelOffset = + getNewFunctionOrDataAddress(SectionInputAddress + Rel.Offset); + + RelOffset = RelOffset == 0 ? SectionAddress + Rel.Offset : RelOffset; + assert((RelOffset & 1) == 0 && "Wrong relocation offset"); + RelOffsets.emplace(RelOffset); + FixAddend(Section, Rel); + } + } + + ErrorOr Section = + BC->getSectionForAddress(*DynamicRelrAddress); + assert(Section && "cannot get .relr.dyn section"); + assert(Section->isRelr() && "Expected section to be SHT_RELR type"); + uint64_t RelrDynOffset = Section->getInputFileOffset(); + const uint64_t RelrDynEndOffset = RelrDynOffset + Section->getSize(); + + auto WriteRelr = [&](uint64_t Value) { + if (RelrDynOffset + DynamicRelrEntrySize > RelrDynEndOffset) { + errs() << "BOLT-ERROR: Offset overflow for relr.dyn section\n"; + exit(1); + } + + OS.pwrite(reinterpret_cast(&Value), DynamicRelrEntrySize, + RelrDynOffset); + RelrDynOffset += DynamicRelrEntrySize; + }; + + for (auto RelIt = RelOffsets.begin(); RelIt != RelOffsets.end();) { + WriteRelr(*RelIt); + uint64_t Base = *RelIt++ + PSize; + while (1) { + uint64_t Bitmap = 0; + for (; RelIt != RelOffsets.end(); ++RelIt) { + const uint64_t Delta = *RelIt - Base; + if (Delta >= MaxDelta || Delta % PSize) + break; + + Bitmap |= (1ULL << (Delta / PSize)); + } + + if (!Bitmap) + break; + + WriteRelr((Bitmap << 1) | 1); + Base += MaxDelta; + } + } + + // Fill the rest of the section with empty bitmap value + while (RelrDynOffset != RelrDynEndOffset) + WriteRelr(1); +} + template void RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { @@ -5133,6 +5378,7 @@ RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { auto setSectionFileOffsets = [&](uint64_t Address, uint64_t &Start, uint64_t &End) { ErrorOr Section = BC->getSectionForAddress(Address); + assert(Section && "cannot get relocation section"); Start = Section->getInputFileOffset(); End = Start + Section->getSize(); }; @@ -5157,6 +5403,11 @@ RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { auto writeRelocations = [&](bool PatchRelative) { for (BinarySection &Section : BC->allocatableSections()) { + const uint64_t SectionInputAddress = Section.getAddress(); + uint64_t SectionAddress = Section.getOutputAddress(); + if (!SectionAddress) + SectionAddress = SectionInputAddress; + for (const Relocation &Rel : Section.dynamicRelocations()) { const bool IsRelative = Rel.isRelative(); if (PatchRelative != IsRelative) @@ -5166,13 +5417,13 @@ RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { ++DynamicRelativeRelocationsCount; Elf_Rela NewRelA; - uint64_t SectionAddress = Section.getOutputAddress(); - SectionAddress = - SectionAddress == 0 ? Section.getAddress() : SectionAddress; MCSymbol *Symbol = Rel.Symbol; uint32_t SymbolIdx = 0; uint64_t Addend = Rel.Addend; + uint64_t RelOffset = + getNewFunctionOrDataAddress(SectionInputAddress + Rel.Offset); + RelOffset = RelOffset == 0 ? SectionAddress + Rel.Offset : RelOffset; if (Rel.Symbol) { SymbolIdx = getOutputDynamicSymbolIndex(Symbol); } else { @@ -5183,11 +5434,10 @@ RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { } NewRelA.setSymbolAndType(SymbolIdx, Rel.Type, EF.isMips64EL()); - NewRelA.r_offset = SectionAddress + Rel.Offset; + NewRelA.r_offset = RelOffset; NewRelA.r_addend = Addend; - const bool IsJmpRel = - !!(IsJmpRelocation.find(Rel.Type) != IsJmpRelocation.end()); + const bool IsJmpRel = IsJmpRelocation.contains(Rel.Type); uint64_t &Offset = IsJmpRel ? RelPltOffset : RelDynOffset; const uint64_t &EndOffset = IsJmpRel ? RelPltEndOffset : RelDynEndOffset; @@ -5206,8 +5456,11 @@ RewriteInstance::patchELFAllocatableRelaSections(ELFObjectFile *File) { } }; - // The dynamic linker expects R_*_RELATIVE relocations to be emitted first - writeRelocations(/* PatchRelative */ true); + // Place R_*_RELATIVE relocations in RELA section if RELR is not presented. + // The dynamic linker expects all R_*_RELATIVE relocations in RELA + // to be emitted first. + if (!DynamicRelrAddress) + writeRelocations(/* PatchRelative */ true); writeRelocations(/* PatchRelative */ false); auto fillNone = [&](uint64_t &Offset, uint64_t EndOffset) { @@ -5413,6 +5666,15 @@ Error RewriteInstance::readELFDynamic(ELFObjectFile *File) { case ELF::DT_RELACOUNT: DynamicRelativeRelocationsCount = Dyn.getVal(); break; + case ELF::DT_RELR: + DynamicRelrAddress = Dyn.getPtr(); + break; + case ELF::DT_RELRSZ: + DynamicRelrSize = Dyn.getVal(); + break; + case ELF::DT_RELRENT: + DynamicRelrEntrySize = Dyn.getVal(); + break; } } @@ -5425,6 +5687,20 @@ Error RewriteInstance::readELFDynamic(ELFObjectFile *File) { PLTRelocationsAddress.reset(); PLTRelocationsSize = 0; } + + if (!DynamicRelrAddress || !DynamicRelrSize) { + DynamicRelrAddress.reset(); + DynamicRelrSize = 0; + } else if (!DynamicRelrEntrySize) { + errs() << "BOLT-ERROR: expected DT_RELRENT to be presented " + << "in DYNAMIC section\n"; + exit(1); + } else if (DynamicRelrSize % DynamicRelrEntrySize) { + errs() << "BOLT-ERROR: expected RELR table size to be divisible " + << "by RELR entry size\n"; + exit(1); + } + return Error::success(); } @@ -5636,6 +5912,7 @@ void RewriteInstance::rewriteFile() { if (BC->HasRelocations) { patchELFAllocatableRelaSections(); + patchELFAllocatableRelrSection(); patchELFGOT(); } diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index 422f7feebb08e..ec9f78d48981f 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -45,8 +45,6 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { *AArch64ExprB.getSubExpr(), Comp); } - bool hasEVEXEncoding(const MCInst &) const override { return false; } - bool isMacroOpFusionPair(ArrayRef Insts) const override { return false; } @@ -1091,13 +1089,51 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { return true; } - bool isMoveMem2Reg(const MCInst &Inst) const override { return false; } - - bool isLeave(const MCInst &Inst) const override { return false; } - - bool isPop(const MCInst &Inst) const override { return false; } - - bool isPrefix(const MCInst &Inst) const override { return false; } + bool shouldRecordCodeRelocation(uint64_t RelType) const override { + switch (RelType) { + case ELF::R_AARCH64_ABS64: + case ELF::R_AARCH64_ABS32: + case ELF::R_AARCH64_ABS16: + case ELF::R_AARCH64_ADD_ABS_LO12_NC: + case ELF::R_AARCH64_ADR_GOT_PAGE: + case ELF::R_AARCH64_ADR_PREL_LO21: + case ELF::R_AARCH64_ADR_PREL_PG_HI21: + case ELF::R_AARCH64_ADR_PREL_PG_HI21_NC: + case ELF::R_AARCH64_LD64_GOT_LO12_NC: + case ELF::R_AARCH64_LDST8_ABS_LO12_NC: + case ELF::R_AARCH64_LDST16_ABS_LO12_NC: + case ELF::R_AARCH64_LDST32_ABS_LO12_NC: + case ELF::R_AARCH64_LDST64_ABS_LO12_NC: + case ELF::R_AARCH64_LDST128_ABS_LO12_NC: + case ELF::R_AARCH64_TLSDESC_ADD_LO12: + case ELF::R_AARCH64_TLSDESC_ADR_PAGE21: + case ELF::R_AARCH64_TLSDESC_ADR_PREL21: + case ELF::R_AARCH64_TLSDESC_LD64_LO12: + case ELF::R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21: + case ELF::R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC: + case ELF::R_AARCH64_MOVW_UABS_G0: + case ELF::R_AARCH64_MOVW_UABS_G0_NC: + case ELF::R_AARCH64_MOVW_UABS_G1: + case ELF::R_AARCH64_MOVW_UABS_G1_NC: + case ELF::R_AARCH64_MOVW_UABS_G2: + case ELF::R_AARCH64_MOVW_UABS_G2_NC: + case ELF::R_AARCH64_MOVW_UABS_G3: + case ELF::R_AARCH64_PREL16: + case ELF::R_AARCH64_PREL32: + case ELF::R_AARCH64_PREL64: + return true; + case ELF::R_AARCH64_CALL26: + case ELF::R_AARCH64_JUMP26: + case ELF::R_AARCH64_TSTBR14: + case ELF::R_AARCH64_CONDBR19: + case ELF::R_AARCH64_TLSDESC_CALL: + case ELF::R_AARCH64_TLSLE_ADD_TPREL_HI12: + case ELF::R_AARCH64_TLSLE_ADD_TPREL_LO12_NC: + return false; + default: + llvm_unreachable("Unexpected AArch64 relocation type in code"); + } + } bool createReturn(MCInst &Inst) const override { Inst.setOpcode(AArch64::RET); diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp index 61a7353d85a5d..ad80255dcf35e 100644 --- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp +++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp @@ -388,6 +388,29 @@ class X86MCPlusBuilder : public MCPlusBuilder { return (Desc.TSFlags & X86II::OpPrefixMask) == X86II::PD; } + bool shouldRecordCodeRelocation(uint64_t RelType) const override { + switch (RelType) { + case ELF::R_X86_64_8: + case ELF::R_X86_64_16: + case ELF::R_X86_64_32: + case ELF::R_X86_64_32S: + case ELF::R_X86_64_64: + case ELF::R_X86_64_PC8: + case ELF::R_X86_64_PC32: + case ELF::R_X86_64_PC64: + case ELF::R_X86_64_GOTPCRELX: + case ELF::R_X86_64_REX_GOTPCRELX: + return true; + case ELF::R_X86_64_PLT32: + case ELF::R_X86_64_GOTPCREL: + case ELF::R_X86_64_TPOFF32: + case ELF::R_X86_64_GOTTPOFF: + return false; + default: + llvm_unreachable("Unexpected x86 relocation type in code"); + } + } + unsigned getTrapFillValue() const override { return 0xCC; } struct IndJmpMatcherFrag1 : MCInstMatcher { @@ -3030,11 +3053,12 @@ class X86MCPlusBuilder : public MCPlusBuilder { Inst.clear(); } - void createInstrIncMemory(InstructionListType &Instrs, const MCSymbol *Target, - MCContext *Ctx, bool IsLeaf) const override { + InstructionListType createInstrIncMemory(const MCSymbol *Target, + MCContext *Ctx, + bool IsLeaf) const override { + InstructionListType Instrs(IsLeaf ? 13 : 11); unsigned int I = 0; - Instrs.resize(IsLeaf ? 13 : 11); // Don't clobber application red zone (ABI dependent) if (IsLeaf) createStackPointerIncrement(Instrs[I++], 128, @@ -3061,6 +3085,7 @@ class X86MCPlusBuilder : public MCPlusBuilder { if (IsLeaf) createStackPointerDecrement(Instrs[I], 128, /*NoFlagsClobber=*/true); + return Instrs; } void createSwap(MCInst &Inst, MCPhysReg Source, MCPhysReg MemBaseReg, diff --git a/bolt/test/AArch64/constant_island_pie_update.s b/bolt/test/AArch64/constant_island_pie_update.s index f4989b9c021d5..c6856988d52f7 100644 --- a/bolt/test/AArch64/constant_island_pie_update.s +++ b/bolt/test/AArch64/constant_island_pie_update.s @@ -1,17 +1,50 @@ -// This test checks that the constant island value is updated if it -// has dynamic relocation. +// This test checks that the constant island offset and value is updated if +// it has dynamic relocation. The tests checks both .rela.dyn and relr.dyn +// dynamic relocations. +// Also check that we don't duplicate CI if it has dynamic relocations. -# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \ -# RUN: %s -o %t.o -# RUN: %clang %cflags -fPIC -pie %t.o -o %t.exe -Wl,-q -nostdlib -Wl,-z,notext -# RUN: llvm-bolt %t.exe -o %t.bolt --use-old-text=0 --lite=0 -# RUN: llvm-objdump -j .text -dR %t.bolt | FileCheck %s +# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o +// .rela.dyn +# RUN: %clang %cflags -fPIC -pie %t.o -o %t.rela.exe -nostdlib \ +# RUN: -Wl,-q -Wl,-z,notext +# RUN: llvm-bolt %t.rela.exe -o %t.rela.bolt --use-old-text=0 --lite=0 +# RUN: llvm-objdump -j .text -d %t.rela.bolt | FileCheck %s +# RUN: llvm-readelf -rsW %t.rela.bolt | FileCheck --check-prefix=ELFCHECK %s +// .relr.dyn +# RUN: %clang %cflags -fPIC -pie %t.o -o %t.relr.exe -nostdlib \ +# RUN: -Wl,-q -Wl,-z,notext -Wl,--pack-dyn-relocs=relr +# RUN: llvm-bolt %t.relr.exe -o %t.relr.bolt --use-old-text=0 --lite=0 +# RUN: llvm-objdump -j .text -d %t.relr.bolt | FileCheck %s +# RUN: llvm-readelf -rsW %t.relr.bolt | FileCheck --check-prefix=ELFCHECK %s +# RUN: llvm-readelf -SW %t.relr.bolt | FileCheck --check-prefix=RELRSZCHECK %s -# CHECK: R_AARCH64_RELATIVE *ABS*+0x[[#%x,ADDR:]] -# CHECK: [[#ADDR]] : +// Check that the CI value was updated +# CHECK: [[#%x,ADDR:]] : # CHECK: {{.*}} <$d>: # CHECK-NEXT: {{.*}} .word 0x{{[0]+}}[[#ADDR]] # CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x{{[0]+}}[[#ADDR]] +# CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x00000000 +# CHECK-NEXT: {{.*}} .word 0x{{[0]+}}[[#ADDR]] +# CHECK-NEXT: {{.*}} .word 0x00000000 + + +// Check that we've relaxed adr to adrp + add to refer external CI +# CHECK: : +# CHECK-NEXT: adrp +# CHECK-NEXT: add + +// Check that relocation offsets were updated +# ELFCHECK: [[#%x,OFF:]] [[#%x,INFO_DYN:]] R_AARCH64_RELATIVE +# ELFCHECK-NEXT: [[#OFF + 8]] {{0*}}[[#INFO_DYN]] R_AARCH64_RELATIVE +# ELFCHECK-NEXT: [[#OFF + 24]] {{0*}}[[#INFO_DYN]] R_AARCH64_RELATIVE +# ELFCHECK: {{.*}}[[#OFF]] {{.*}} $d + +// Check that .relr.dyn size is 2 bytes to ensure that last 2 relocations were +// encoded as a bitmap so the total section size for 3 relocations is 2 bytes. +# RELRSZCHECK: .relr.dyn RELR [[#%x,ADDR:]] [[#%x,OFF:]] {{0*}}10 .text .align 4 @@ -20,6 +53,8 @@ exitLocal: add x1, x1, #1 add x1, x1, #1 + nop + nop ret .size exitLocal, .-exitLocal @@ -34,5 +69,15 @@ _start: bl exitLocal nop .Lci: + .xword exitLocal + .xword exitLocal + .xword 0 .xword exitLocal .size _start, .-_start + + .global addressDynCi + .type addressDynCi, %function +addressDynCi: + adr x1, .Lci + bl _start +.size addressDynCi, .-addressDynCi diff --git a/bolt/test/Inputs/reorder-data-writable-ptload.fdata b/bolt/test/Inputs/reorder-data-writable-ptload.fdata new file mode 100644 index 0000000000000..f5dab9d1dacaf --- /dev/null +++ b/bolt/test/Inputs/reorder-data-writable-ptload.fdata @@ -0,0 +1 @@ +4 _start 0 4 hot1 0 100 diff --git a/bolt/test/X86/instrumentation-eh_frame_hdr.cpp b/bolt/test/X86/instrumentation-eh_frame_hdr.cpp new file mode 100644 index 0000000000000..f6ebd6b76f60a --- /dev/null +++ b/bolt/test/X86/instrumentation-eh_frame_hdr.cpp @@ -0,0 +1,40 @@ +// This test checks that .eh_frame_hdr address is in bounds of the last LOAD +// end address i.e. the section address is smaller then the LOAD end address. + +// REQUIRES: system-linux,bolt-runtime + +// RUN: %clangxx %cxxflags -static -Wl,-q %s -o %t.exe -Wl,--entry=_start +// RUN: llvm-bolt %t.exe -o %t.instr -instrument \ +// RUN: --instrumentation-file=%t.fdata -instrumentation-sleep-time=1 +// RUN: (llvm-readelf -SW %t.instr | grep -v bolt; llvm-readelf -lW %t.instr | \ +// RUN: grep LOAD | tail -n 1) | FileCheck %s + +// CHECK: {{.*}} .eh_frame_hdr PROGBITS [[#%x, EH_ADDR:]] +// CHECK: LOAD 0x[[#%x, LD_OFFSET:]] 0x[[#%x, LD_VADDR:]] 0x[[#%x, LD_FSIZE:]] +// CHECK-SAME: 0x[[#%x, LD_MEMSIZE:]] +// +// If .eh_frame_hdr address bigger then last LOAD segment end address test would +// fail with overflow error, otherwise the result of the expression is 0 that +// could be found on this line e.g. in LOAD align field. +// CHECK-SAME: [[#LD_VADDR + LD_MEMSIZE - max(LD_VADDR + LD_MEMSIZE,EH_ADDR)]] + +#include +#include + +void foo() { throw std::runtime_error("Exception from foo()"); } + +void bar() { foo(); } + +int main() { + try { + bar(); + } catch (const std::exception &e) { + printf("Exception caught: %s\n", e.what()); + } +} + +extern "C" { +void _start(); +} + +void _start() { main(); } diff --git a/bolt/test/reorder-data-writable-ptload.c b/bolt/test/reorder-data-writable-ptload.c new file mode 100644 index 0000000000000..7b384e9655a32 --- /dev/null +++ b/bolt/test/reorder-data-writable-ptload.c @@ -0,0 +1,21 @@ +// This test checks that reorder-data pass puts new hot .data section +// to the writable segment. + +// RUN: %clang %cflags -O3 -nostdlib -Wl,-q %s -o %t.exe +// RUN: llvm-bolt %t.exe -o %t.bolt --reorder-data=".data" \ +// RUN: -data %S/Inputs/reorder-data-writable-ptload.fdata +// RUN: llvm-readelf -SlW %t.bolt | FileCheck %s + +// CHECK: .bolt.org.data +// CHECK: {{.*}} .data PROGBITS [[#%x,ADDR:]] [[#%x,OFF:]] +// CHECK: LOAD 0x{{.*}}[[#OFF]] 0x{{.*}}[[#ADDR]] {{.*}} RW + +volatile int cold1 = 42; +volatile int hot1 = 42; +volatile int cold2 = 42; +volatile int cold3 = 42; + +void _start() { + hot1++; + _start(); +} diff --git a/bolt/test/runtime/X86/instrumentation-xmm.c b/bolt/test/runtime/X86/instrumentation-xmm.c index 06002c08b1733..1ddc104bc7226 100644 --- a/bolt/test/runtime/X86/instrumentation-xmm.c +++ b/bolt/test/runtime/X86/instrumentation-xmm.c @@ -18,7 +18,8 @@ int main() { REQUIRES: system-linux,bolt-runtime RUN: %clang %cflags %s -o %t.exe -fno-inline -Wl,-q -RUN: llvm-bolt %t.exe --instrument -o %t.instrumented +RUN: llvm-bolt %t.exe --instrument -o %t.instrumented \ +RUN: --instrumentation-file=%t.fdata RUN: %t.instrumented | FileCheck %s CHECK: a = 42.000000 diff --git a/clang-tools-extra/clang-doc/BitcodeWriter.cpp b/clang-tools-extra/clang-doc/BitcodeWriter.cpp index 8a5647cb6a755..2ece99a3124ab 100644 --- a/clang-tools-extra/clang-doc/BitcodeWriter.cpp +++ b/clang-tools-extra/clang-doc/BitcodeWriter.cpp @@ -264,13 +264,13 @@ constexpr unsigned char BitCodeConstants::Signature[]; void ClangDocBitcodeWriter::AbbreviationMap::add(RecordId RID, unsigned AbbrevID) { assert(RecordIdNameMap[RID] && "Unknown RecordId."); - assert(Abbrevs.find(RID) == Abbrevs.end() && "Abbreviation already added."); + assert(!Abbrevs.contains(RID) && "Abbreviation already added."); Abbrevs[RID] = AbbrevID; } unsigned ClangDocBitcodeWriter::AbbreviationMap::get(RecordId RID) const { assert(RecordIdNameMap[RID] && "Unknown RecordId."); - assert(Abbrevs.find(RID) != Abbrevs.end() && "Unknown abbreviation."); + assert(Abbrevs.contains(RID) && "Unknown abbreviation."); return Abbrevs.lookup(RID); } diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp index b61780ca4d971..d2466857e5110 100644 --- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp @@ -866,7 +866,7 @@ HTMLGenerator::generateDocs(StringRef RootDir, llvm::SmallString<128> Path; llvm::sys::path::native(RootDir, Path); llvm::sys::path::append(Path, Info->getRelativeFilePath("")); - if (CreatedDirs.find(Path) == CreatedDirs.end()) { + if (!CreatedDirs.contains(Path)) { if (std::error_code Err = llvm::sys::fs::create_directories(Path); Err != std::error_code()) { return llvm::createStringError(Err, "Failed to create directory '%s'.", diff --git a/clang-tools-extra/clang-doc/MDGenerator.cpp b/clang-tools-extra/clang-doc/MDGenerator.cpp index 8261e14c3efb0..2f56fd99b7bdc 100644 --- a/clang-tools-extra/clang-doc/MDGenerator.cpp +++ b/clang-tools-extra/clang-doc/MDGenerator.cpp @@ -387,7 +387,7 @@ MDGenerator::generateDocs(StringRef RootDir, llvm::SmallString<128> Path; llvm::sys::path::native(RootDir, Path); llvm::sys::path::append(Path, Info->getRelativeFilePath("")); - if (CreatedDirs.find(Path) == CreatedDirs.end()) { + if (!CreatedDirs.contains(Path)) { if (std::error_code Err = llvm::sys::fs::create_directories(Path); Err != std::error_code()) { return llvm::createStringError(Err, "Failed to create directory '%s'.", diff --git a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp index 0d0bbd9f0555b..122d2df1f1398 100644 --- a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp +++ b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp @@ -243,6 +243,7 @@ const HeaderMapCollector::RegexHeaderMap *getSTLPostfixHeaderMap() { {"mutex$", ""}, {"new$", ""}, {"numeric$", ""}, + {"optional$", ""}, {"ostream$", ""}, {"queue$", ""}, {"random$", ""}, diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp index 0e21453234f2f..fa7285744be89 100644 --- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp @@ -1581,7 +1581,7 @@ bool lazyMapOfSetsIntersectionExists(const MapTy &Map, const ElemTy &E1, return false; for (const auto &E1SetElem : E1Iterator->second) - if (llvm::is_contained(E2Iterator->second, E1SetElem)) + if (E2Iterator->second.contains(E1SetElem)) return true; return false; diff --git a/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp index ae874210afc1a..f578f7ea71c08 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp @@ -147,7 +147,7 @@ void ForwardDeclarationNamespaceCheck::onEndOfTranslationUnit() { } // Check if a definition in another namespace exists. const auto DeclName = CurDecl->getName(); - if (DeclNameToDefinitions.find(DeclName) == DeclNameToDefinitions.end()) { + if (!DeclNameToDefinitions.contains(DeclName)) { continue; // No definition in this translation unit, we can skip it. } // Make a warning for each definition with the same name (in other diff --git a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp index 133fcabd78392..8ba8b893e03a6 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp @@ -34,6 +34,11 @@ struct MagnitudeBits { bool operator<(const MagnitudeBits &Other) const noexcept { return WidthWithoutSignBit < Other.WidthWithoutSignBit; } + + bool operator!=(const MagnitudeBits &Other) const noexcept { + return WidthWithoutSignBit != Other.WidthWithoutSignBit || + BitFieldWidth != Other.BitFieldWidth; + } }; } // namespace @@ -184,13 +189,19 @@ void TooSmallLoopVariableCheck::check(const MatchFinder::MatchResult &Result) { if (LoopVar->getType() != LoopIncrement->getType()) return; - const QualType LoopVarType = LoopVar->getType(); - const QualType UpperBoundType = UpperBound->getType(); - ASTContext &Context = *Result.Context; + const QualType LoopVarType = LoopVar->getType(); const MagnitudeBits LoopVarMagnitudeBits = calcMagnitudeBits(Context, LoopVarType, LoopVar); + + const MagnitudeBits LoopIncrementMagnitudeBits = + calcMagnitudeBits(Context, LoopIncrement->getType(), LoopIncrement); + // We matched the loop variable incorrectly. + if (LoopIncrementMagnitudeBits != LoopVarMagnitudeBits) + return; + + const QualType UpperBoundType = UpperBound->getType(); const MagnitudeBits UpperBoundMagnitudeBits = calcUpperBoundMagnitudeBits(Context, UpperBound, UpperBoundType); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp new file mode 100644 index 0000000000000..3c99831f9d640 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp @@ -0,0 +1,45 @@ +//===--- AvoidCapturingLambdaCoroutinesCheck.cpp - clang-tidy -------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "AvoidCapturingLambdaCoroutinesCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::cppcoreguidelines { + +namespace { +AST_MATCHER(LambdaExpr, hasCoroutineBody) { + const Stmt *Body = Node.getBody(); + return Body != nullptr && CoroutineBodyStmt::classof(Body); +} + +AST_MATCHER(LambdaExpr, hasCaptures) { return Node.capture_size() != 0U; } +} // namespace + +void AvoidCapturingLambdaCoroutinesCheck::registerMatchers( + MatchFinder *Finder) { + Finder->addMatcher( + lambdaExpr(hasCaptures(), hasCoroutineBody()).bind("lambda"), this); +} + +bool AvoidCapturingLambdaCoroutinesCheck::isLanguageVersionSupported( + const LangOptions &LangOpts) const { + return LangOpts.CPlusPlus20; +} + +void AvoidCapturingLambdaCoroutinesCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedLambda = Result.Nodes.getNodeAs("lambda"); + diag(MatchedLambda->getExprLoc(), + "coroutine lambda may cause use-after-free, avoid captures or ensure " + "lambda closure object has guaranteed lifetime"); +} + +} // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h new file mode 100644 index 0000000000000..b32e2662b5fba --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h @@ -0,0 +1,33 @@ +//===--- AvoidCapturingLambdaCoroutinesCheck.h - clang-tidy -----*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::cppcoreguidelines { + +/// Flags C++20 coroutine lambdas with non-empty capture lists that may cause +/// use-after-free errors and suggests avoiding captures or ensuring the lambda +/// closure object has a guaranteed lifetime. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.html +class AvoidCapturingLambdaCoroutinesCheck : public ClangTidyCheck { +public: + AvoidCapturingLambdaCoroutinesCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override; +}; + +} // namespace clang::tidy::cppcoreguidelines + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index bd7a999a71743..0804675c63949 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangTidyCppCoreGuidelinesModule AvoidCaptureDefaultWhenCapturingThisCheck.cpp + AvoidCapturingLambdaCoroutinesCheck.cpp AvoidConstOrRefDataMembersCheck.cpp AvoidDoWhileCheck.cpp AvoidGotoCheck.cpp @@ -28,6 +29,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule ProTypeStaticCastDowncastCheck.cpp ProTypeUnionAccessCheck.cpp ProTypeVarargCheck.cpp + RvalueReferenceParamNotMovedCheck.cpp SlicingCheck.cpp SpecialMemberFunctionsCheck.cpp VirtualClassDestructorCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index ac70fc8eaa24c..5bd9648fe8371 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -15,6 +15,7 @@ #include "../modernize/UseOverrideCheck.h" #include "../readability/MagicNumbersCheck.h" #include "AvoidCaptureDefaultWhenCapturingThisCheck.h" +#include "AvoidCapturingLambdaCoroutinesCheck.h" #include "AvoidConstOrRefDataMembersCheck.h" #include "AvoidDoWhileCheck.h" #include "AvoidGotoCheck.h" @@ -37,6 +38,7 @@ #include "ProTypeStaticCastDowncastCheck.h" #include "ProTypeUnionAccessCheck.h" #include "ProTypeVarargCheck.h" +#include "RvalueReferenceParamNotMovedCheck.h" #include "SlicingCheck.h" #include "SpecialMemberFunctionsCheck.h" #include "VirtualClassDestructorCheck.h" @@ -50,6 +52,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck( "cppcoreguidelines-avoid-capture-default-when-capturing-this"); + CheckFactories.registerCheck( + "cppcoreguidelines-avoid-capturing-lambda-coroutines"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-c-arrays"); CheckFactories.registerCheck( @@ -101,6 +105,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { "cppcoreguidelines-pro-type-union-access"); CheckFactories.registerCheck( "cppcoreguidelines-pro-type-vararg"); + CheckFactories.registerCheck( + "cppcoreguidelines-rvalue-reference-param-not-moved"); CheckFactories.registerCheck( "cppcoreguidelines-special-member-functions"); CheckFactories.registerCheck("cppcoreguidelines-slicing"); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp new file mode 100644 index 0000000000000..eb67df05411a0 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp @@ -0,0 +1,128 @@ +//===--- RvalueReferenceParamNotMovedCheck.cpp - clang-tidy ---------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "RvalueReferenceParamNotMovedCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::cppcoreguidelines { + +namespace { +AST_MATCHER_P(LambdaExpr, valueCapturesVar, DeclarationMatcher, VarMatcher) { + return std::find_if(Node.capture_begin(), Node.capture_end(), + [&](const LambdaCapture &Capture) { + return Capture.capturesVariable() && + VarMatcher.matches(*Capture.getCapturedVar(), + Finder, Builder) && + Capture.getCaptureKind() == LCK_ByCopy; + }) != Node.capture_end(); +} +AST_MATCHER_P2(Stmt, argumentOf, bool, AllowPartialMove, StatementMatcher, + Ref) { + if (AllowPartialMove) { + return stmt(anyOf(Ref, hasDescendant(Ref))).matches(Node, Finder, Builder); + } else { + return Ref.matches(Node, Finder, Builder); + } +} +} // namespace + +void RvalueReferenceParamNotMovedCheck::registerMatchers(MatchFinder *Finder) { + auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param"))); + + StatementMatcher MoveCallMatcher = + callExpr( + anyOf(callee(functionDecl(hasName("::std::move"))), + callee(unresolvedLookupExpr(hasAnyDeclaration( + namedDecl(hasUnderlyingDecl(hasName("::std::move"))))))), + argumentCountIs(1), + hasArgument( + 0, argumentOf( + AllowPartialMove, + declRefExpr(to(equalsBoundNode("param"))).bind("ref"))), + unless(hasAncestor( + lambdaExpr(valueCapturesVar(equalsBoundNode("param")))))) + .bind("move-call"); + + Finder->addMatcher( + parmVarDecl( + hasType(type(rValueReferenceType())), parmVarDecl().bind("param"), + unless(hasType(references(qualType( + anyOf(isConstQualified(), substTemplateTypeParmType()))))), + optionally(hasType(qualType(references(templateTypeParmType( + hasDeclaration(templateTypeParmDecl().bind("template-type"))))))), + anyOf(hasAncestor(cxxConstructorDecl( + ToParam, isDefinition(), unless(isMoveConstructor()), + optionally(hasDescendant(MoveCallMatcher)))), + hasAncestor(functionDecl( + unless(cxxConstructorDecl()), ToParam, + unless(cxxMethodDecl(isMoveAssignmentOperator())), + hasBody(optionally(hasDescendant(MoveCallMatcher))))))), + this); +} + +void RvalueReferenceParamNotMovedCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *Param = Result.Nodes.getNodeAs("param"); + const auto *TemplateType = + Result.Nodes.getNodeAs("template-type"); + + if (!Param) + return; + + if (IgnoreUnnamedParams && Param->getName().empty()) + return; + + const auto *Function = dyn_cast(Param->getDeclContext()); + if (!Function) + return; + + if (IgnoreNonDeducedTemplateTypes && TemplateType) + return; + + if (TemplateType) { + if (const FunctionTemplateDecl *FuncTemplate = + Function->getDescribedFunctionTemplate()) { + const TemplateParameterList *Params = + FuncTemplate->getTemplateParameters(); + if (llvm::is_contained(*Params, TemplateType)) { + // Ignore forwarding reference + return; + } + } + } + + const auto *MoveCall = Result.Nodes.getNodeAs("move-call"); + if (!MoveCall) { + diag(Param->getLocation(), + "rvalue reference parameter %0 is never moved from " + "inside the function body") + << Param; + } +} + +RvalueReferenceParamNotMovedCheck::RvalueReferenceParamNotMovedCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowPartialMove(Options.getLocalOrGlobal("AllowPartialMove", false)), + IgnoreUnnamedParams( + Options.getLocalOrGlobal("IgnoreUnnamedParams", false)), + IgnoreNonDeducedTemplateTypes( + Options.getLocalOrGlobal("IgnoreNonDeducedTemplateTypes", false)) {} + +void RvalueReferenceParamNotMovedCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowPartialMove", AllowPartialMove); + Options.store(Opts, "IgnoreUnnamedParams", IgnoreUnnamedParams); + Options.store(Opts, "IgnoreNonDeducedTemplateTypes", + IgnoreNonDeducedTemplateTypes); +} + +} // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h new file mode 100644 index 0000000000000..d8c3d2bd4ba0e --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h @@ -0,0 +1,39 @@ +//===--- RvalueReferenceParamNotMovedCheck.h - clang-tidy -------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::cppcoreguidelines { + +/// Warns when an rvalue reference function parameter is never moved within +/// the function body. This check implements CppCoreGuideline F.18. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.html +class RvalueReferenceParamNotMovedCheck : public ClangTidyCheck { +public: + RvalueReferenceParamNotMovedCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const bool AllowPartialMove; + const bool IgnoreUnnamedParams; + const bool IgnoreNonDeducedTemplateTypes; +}; + +} // namespace clang::tidy::cppcoreguidelines + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp index bc094a2f028d6..76754394de760 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp @@ -40,12 +40,15 @@ void SlicingCheck::registerMatchers(MatchFinder *Finder) { const auto HasTypeDerivedFromBaseDecl = anyOf(hasType(IsDerivedFromBaseDecl), hasType(references(IsDerivedFromBaseDecl))); - const auto IsWithinDerivedCtor = - hasParent(cxxConstructorDecl(ofClass(equalsBoundNode("DerivedDecl")))); + const auto IsCallToBaseClass = hasParent(cxxConstructorDecl( + ofClass(isSameOrDerivedFrom(equalsBoundNode("DerivedDecl"))), + hasAnyConstructorInitializer(allOf( + isBaseInitializer(), withInitializer(equalsBoundNode("Call")))))); // Assignment slicing: "a = b;" and "a = std::move(b);" variants. const auto SlicesObjectInAssignment = - callExpr(callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(), + callExpr(expr().bind("Call"), + callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(), isMoveAssignmentOperator()), OfBaseClass)), hasArgument(1, HasTypeDerivedFromBaseDecl)); @@ -53,17 +56,17 @@ void SlicingCheck::registerMatchers(MatchFinder *Finder) { // Construction slicing: "A a{b};" and "f(b);" variants. Note that in case of // slicing the letter will create a temporary and therefore call a ctor. const auto SlicesObjectInCtor = cxxConstructExpr( + expr().bind("Call"), hasDeclaration(cxxConstructorDecl( anyOf(isCopyConstructor(), isMoveConstructor()), OfBaseClass)), hasArgument(0, HasTypeDerivedFromBaseDecl), // We need to disable matching on the call to the base copy/move // constructor in DerivedDecl's constructors. - unless(IsWithinDerivedCtor)); + unless(IsCallToBaseClass)); - Finder->addMatcher(traverse(TK_AsIs, expr(anyOf(SlicesObjectInAssignment, - SlicesObjectInCtor)) - .bind("Call")), - this); + Finder->addMatcher( + traverse(TK_AsIs, expr(SlicesObjectInAssignment).bind("Call")), this); + Finder->addMatcher(traverse(TK_AsIs, SlicesObjectInCtor), this); } /// Warns on methods overridden in DerivedDecl with respect to BaseDecl. diff --git a/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp index 8ffcf476457db..c5bd6055072aa 100644 --- a/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp @@ -24,7 +24,7 @@ constexpr llvm::StringLiteral KDisabledTestPrefix = "DISABLED_"; static bool isGoogletestTestMacro(StringRef MacroName) { static const llvm::StringSet<> MacroNames = {"TEST", "TEST_F", "TEST_P", "TYPED_TEST", "TYPED_TEST_P"}; - return MacroNames.find(MacroName) != MacroNames.end(); + return MacroNames.contains(MacroName); } namespace { diff --git a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp index d3e7bd4b8032c..673c475db630f 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp @@ -246,8 +246,12 @@ void UseDefaultMemberInitCheck::checkDefaultInit( // Check whether we have multiple hand-written constructors and bomb out, as // it is hard to reconcile their sets of member initializers. const auto *ClassDecl = cast(Field->getParent()); - if (llvm::count_if(ClassDecl->ctors(), [](const CXXConstructorDecl *Ctor) { - return !Ctor->isCopyOrMoveConstructor(); + if (llvm::count_if(ClassDecl->decls(), [](const Decl *D) { + if (const auto *FTD = dyn_cast(D)) + D = FTD->getTemplatedDecl(); + if (const auto *Ctor = dyn_cast(D)) + return !Ctor->isCopyOrMoveConstructor(); + return false; }) > 1) return; diff --git a/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp b/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp index 903fd42d12ab1..5abe4f77d6598 100644 --- a/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp +++ b/clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.cpp @@ -75,7 +75,7 @@ static bool isStandardMPIDatatype(StringRef MPIDatatype) { "MPI_CXX_DOUBLE_COMPLEX", "MPI_CXX_LONG_DOUBLE_COMPLEX"}; - return AllTypes.find(MPIDatatype) != AllTypes.end(); + return AllTypes.contains(MPIDatatype); } /// Check if a BuiltinType matches the MPI datatype. diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp index 61421a13d73ba..81d05a8a93cd9 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -230,9 +230,8 @@ static StringRef const HungarainNotationUserDefinedTypes[] = { // clang-format on IdentifierNamingCheck::NamingStyle::NamingStyle( - std::optional Case, - const std::string &Prefix, const std::string &Suffix, - const std::string &IgnoredRegexpStr, HungarianPrefixType HPType) + std::optional Case, StringRef Prefix, + StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType) : Case(Case), Prefix(Prefix), Suffix(Suffix), IgnoredRegexpStr(IgnoredRegexpStr), HPType(HPType) { if (!IgnoredRegexpStr.empty()) { @@ -265,22 +264,21 @@ IdentifierNamingCheck::FileStyle IdentifierNamingCheck::getFileStyleFromOptions( memcpy(&StyleString[StyleSize], "IgnoredRegexp", 13); StyleString.truncate(StyleSize + 13); - StringRef IgnoredRegexpStr = Options.get(StyleString, ""); + std::optional IgnoredRegexpStr = Options.get(StyleString); memcpy(&StyleString[StyleSize], "Prefix", 6); StyleString.truncate(StyleSize + 6); - std::string Prefix(Options.get(StyleString, "")); + std::optional Prefix(Options.get(StyleString)); // Fast replacement of [Pre]fix -> [Suf]fix. memcpy(&StyleString[StyleSize], "Suf", 3); - std::string Postfix(Options.get(StyleString, "")); + std::optional Postfix(Options.get(StyleString)); memcpy(&StyleString[StyleSize], "Case", 4); StyleString.pop_back_n(2); - auto CaseOptional = + std::optional CaseOptional = Options.get(StyleString); - if (CaseOptional || !Prefix.empty() || !Postfix.empty() || - !IgnoredRegexpStr.empty() || HPTOpt) - Styles[I].emplace(std::move(CaseOptional), std::move(Prefix), - std::move(Postfix), IgnoredRegexpStr.str(), + if (CaseOptional || Prefix || Postfix || IgnoredRegexpStr || HPTOpt) + Styles[I].emplace(std::move(CaseOptional), Prefix.value_or(""), + Postfix.value_or(""), IgnoredRegexpStr.value_or(""), HPTOpt.value_or(IdentifierNamingCheck::HPT_Off)); } bool IgnoreMainLike = Options.get("IgnoreMainLikeFunctions", false); @@ -650,7 +648,9 @@ std::string IdentifierNamingCheck::HungarianNotation::getClassPrefix( std::string IdentifierNamingCheck::HungarianNotation::getEnumPrefix( const EnumConstantDecl *ECD) const { - std::string Name = ECD->getType().getAsString(); + const EnumDecl *ED = cast(ECD->getDeclContext()); + + std::string Name = ED->getName().str(); if (std::string::npos != Name.find("enum")) { Name = Name.substr(strlen("enum"), Name.length() - strlen("enum")); Name = Name.erase(0, Name.find_first_not_of(" ")); diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h index cbf0653a280b6..b1db919902e22 100644 --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h @@ -68,8 +68,8 @@ class IdentifierNamingCheck final : public RenamerClangTidyCheck { struct NamingStyle { NamingStyle() = default; - NamingStyle(std::optional Case, const std::string &Prefix, - const std::string &Suffix, const std::string &IgnoredRegexpStr, + NamingStyle(std::optional Case, StringRef Prefix, + StringRef Suffix, StringRef IgnoredRegexpStr, HungarianPrefixType HPType); NamingStyle(const NamingStyle &O) = delete; NamingStyle &operator=(NamingStyle &&O) = default; diff --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp index 64940ede8e1c7..ca81b6e3c6e61 100644 --- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp @@ -14,6 +14,8 @@ #include "MagicNumbersCheck.h" #include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Type.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "llvm/ADT/STLExtras.h" #include @@ -42,6 +44,18 @@ static bool isUsedToInitializeAConstant(const MatchFinder::MatchResult &Result, }); } +static bool isUsedToDefineATypeAlias(const MatchFinder::MatchResult &Result, + const DynTypedNode &Node) { + + if (Node.get() || Node.get()) + return true; + + return llvm::any_of(Result.Context->getParents(Node), + [&Result](const DynTypedNode &Parent) { + return isUsedToDefineATypeAlias(Result, Parent); + }); +} + static bool isUsedToDefineABitField(const MatchFinder::MatchResult &Result, const DynTypedNode &Node) { const auto *AsFieldDecl = Node.get(); @@ -66,6 +80,7 @@ MagicNumbersCheck::MagicNumbersCheck(StringRef Name, ClangTidyContext *Context) IgnoreBitFieldsWidths(Options.get("IgnoreBitFieldsWidths", true)), IgnorePowersOf2IntegerValues( Options.get("IgnorePowersOf2IntegerValues", false)), + IgnoreTypeAliases(Options.get("IgnoreTypeAliases", false)), RawIgnoredIntegerValues( Options.get("IgnoredIntegerValues", DefaultIgnoredIntegerValues)), RawIgnoredFloatingPointValues(Options.get( @@ -114,6 +129,7 @@ void MagicNumbersCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreBitFieldsWidths", IgnoreBitFieldsWidths); Options.store(Opts, "IgnorePowersOf2IntegerValues", IgnorePowersOf2IntegerValues); + Options.store(Opts, "IgnoreTypeAliases", IgnoreTypeAliases); Options.store(Opts, "IgnoredIntegerValues", RawIgnoredIntegerValues); Options.store(Opts, "IgnoredFloatingPointValues", RawIgnoredFloatingPointValues); @@ -137,10 +153,13 @@ bool MagicNumbersCheck::isConstant(const MatchFinder::MatchResult &Result, const Expr &ExprResult) const { return llvm::any_of( Result.Context->getParents(ExprResult), - [&Result](const DynTypedNode &Parent) { + [this, &Result](const DynTypedNode &Parent) { if (isUsedToInitializeAConstant(Result, Parent)) return true; + if (IgnoreTypeAliases && isUsedToDefineATypeAlias(Result, Parent)) + return true; + // Ignore this instance, because this matches an // expanded class enumeration value. if (Parent.get() && diff --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h index ba39f17f4d500..39e9baea3adc8 100644 --- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h +++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h @@ -84,6 +84,7 @@ class MagicNumbersCheck : public ClangTidyCheck { const bool IgnoreAllFloatingPointValues; const bool IgnoreBitFieldsWidths; const bool IgnorePowersOf2IntegerValues; + const bool IgnoreTypeAliases; const StringRef RawIgnoredIntegerValues; const StringRef RawIgnoredFloatingPointValues; diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp index 960d5abcc912e..dfcc82c270540 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp @@ -11,6 +11,8 @@ //===----------------------------------------------------------------------===// #include "RedundantStringCStrCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" #include "clang/Lex/Lexer.h" #include "clang/Tooling/FixIt.h" @@ -52,6 +54,10 @@ formatDereference(const ast_matchers::MatchFinder::MatchResult &Result, if (Text.empty()) return std::string(); + + // Remove remaining '->' from overloaded operator call + Text.consume_back("->"); + // Add leading '*'. if (needParensAfterUnaryOperator(ExprNode)) { return (llvm::Twine("*(") + Text + ")").str(); @@ -65,6 +71,17 @@ AST_MATCHER(MaterializeTemporaryExpr, isBoundToLValue) { } // end namespace +RedundantStringCStrCheck::RedundantStringCStrCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + StringParameterFunctions(utils::options::parseStringList( + Options.get("StringParameterFunctions", ""))) { + if (getLangOpts().CPlusPlus20) + StringParameterFunctions.push_back("::std::format"); + if (getLangOpts().CPlusPlus2b) + StringParameterFunctions.push_back("::std::print"); +} + void RedundantStringCStrCheck::registerMatchers( ast_matchers::MatchFinder *Finder) { // Match expressions of type 'string' or 'string*'. @@ -178,6 +195,18 @@ void RedundantStringCStrCheck::registerMatchers( // directly. hasArgument(0, StringCStrCallExpr))), this); + + if (!StringParameterFunctions.empty()) { + // Detect redundant 'c_str()' calls in parameters passed to std::format in + // C++20 onwards and std::print in C++23 onwards. + Finder->addMatcher( + traverse(TK_AsIs, + callExpr(callee(functionDecl(matchers::matchesAnyListedName( + StringParameterFunctions))), + forEachArgumentWithParam(StringCStrCallExpr, + parmVarDecl()))), + this); + } } void RedundantStringCStrCheck::check(const MatchFinder::MatchResult &Result) { diff --git a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h index 662fb955fc653..e2e6ab1fd939c 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h +++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.h @@ -16,13 +16,15 @@ namespace clang::tidy::readability { /// Finds unnecessary calls to `std::string::c_str()`. class RedundantStringCStrCheck : public ClangTidyCheck { public: - RedundantStringCStrCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + RedundantStringCStrCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::vector StringParameterFunctions; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp b/clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp index 7fe0c0982dc30..14bf22a4d4200 100644 --- a/clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/SuspiciousCallArgumentCheck.cpp @@ -137,11 +137,11 @@ static bool applyEqualityHeuristic(StringRef Arg, StringRef Param) { static bool applyAbbreviationHeuristic( const llvm::StringMap &AbbreviationDictionary, StringRef Arg, StringRef Param) { - if (AbbreviationDictionary.find(Arg) != AbbreviationDictionary.end() && + if (AbbreviationDictionary.contains(Arg) && Param.equals(AbbreviationDictionary.lookup(Arg))) return true; - if (AbbreviationDictionary.find(Param) != AbbreviationDictionary.end() && + if (AbbreviationDictionary.contains(Param) && Arg.equals(AbbreviationDictionary.lookup(Param))) return true; diff --git a/clang-tools-extra/clang-tidy/rename_check.py b/clang-tools-extra/clang-tidy/rename_check.py index 764960e044219..f56776fdb7291 100755 --- a/clang-tools-extra/clang-tidy/rename_check.py +++ b/clang-tools-extra/clang-tidy/rename_check.py @@ -94,10 +94,14 @@ def deleteMatchingLines(fileName, pattern): def getListOfFiles(clang_tidy_path): files = glob.glob(os.path.join(clang_tidy_path, '**'), recursive=True) + files += [os.path.normpath(os.path.join(clang_tidy_path, + '../docs/ReleaseNotes.rst'))] files += glob.glob(os.path.join(clang_tidy_path, '..', 'test', 'clang-tidy', 'checkers', '**'), recursive=True) files += glob.glob(os.path.join(clang_tidy_path, '..', 'docs', - 'clang-tidy', 'checks', '*')) + 'clang-tidy', 'checks', '*.rst')) + files += glob.glob(os.path.join(clang_tidy_path, '..', 'docs', + 'clang-tidy', 'checks', "*", "*.rst"), recursive=True) return [filename for filename in files if os.path.isfile(filename)] @@ -213,9 +217,11 @@ def add_release_notes(clang_tidy_path, old_check_name, new_check_name): if header_found and add_note_here: if not line.startswith('^^^^'): f.write("""- The '%s' check was renamed to :doc:`%s - ` + ` -""" % (old_check_name, new_check_name, new_check_name)) + """ % (old_check_name, new_check_name, + new_check_name.split('-', 1)[0], + '-'.join(new_check_name.split('-')[1:]))) note_added = True f.write(line) @@ -232,15 +238,18 @@ def main(): old_module = args.old_check_name.split('-')[0] new_module = args.new_check_name.split('-')[0] + old_name = '-'.join(args.old_check_name.split('-')[1:]) + new_name = '-'.join(args.new_check_name.split('-')[1:]) + if args.check_class_name: check_name_camel = args.check_class_name else: check_name_camel = (''.join(map(lambda elem: elem.capitalize(), - args.old_check_name.split('-')[1:])) + + old_name.split('-'))) + 'Check') new_check_name_camel = (''.join(map(lambda elem: elem.capitalize(), - args.new_check_name.split('-')[1:])) + + new_name.split('-'))) + 'Check') clang_tidy_path = os.path.dirname(__file__) @@ -255,7 +264,7 @@ def main(): old_module_path = os.path.join(clang_tidy_path, old_module) new_module_path = os.path.join(clang_tidy_path, new_module) - if (args.old_check_name != args.new_check_name): + if (old_module != new_module): # Remove the check from the old module. cmake_lists = os.path.join(old_module_path, 'CMakeLists.txt') check_found = deleteMatchingLines(cmake_lists, '\\b' + check_name_camel) @@ -270,12 +279,10 @@ def main(): deleteMatchingLines(os.path.join(old_module_path, modulecpp), '\\b' + check_name_camel + '|\\b' + args.old_check_name) - old_check_filename = '-'.join(args.old_check_name.split('-')[1:]) - new_check_filename = '-'.join(args.new_check_name.split('-')[1:]) - for filename in getListOfFiles(clang_tidy_path): originalName = filename - filename = fileRename(filename, old_check_filename, new_check_filename) + filename = fileRename(filename, old_module + "/" + old_name, new_module + "/" + new_name) + filename = fileRename(filename, args.old_check_name, args.new_check_name) filename = fileRename(filename, check_name_camel, new_check_name_camel) replaceInFile(filename, generateCommentLineHeader(originalName), generateCommentLineHeader(filename)) @@ -284,7 +291,7 @@ def main(): for header_guard in header_guard_variants: replaceInFile(filename, header_guard, header_guard_new) - if args.new_check_name + '.rst' in filename: + if new_module + '/'+ new_name + '.rst' in filename: replaceInFile( filename, args.old_check_name + '\n' + '=' * len(args.old_check_name) + '\n', @@ -295,6 +302,8 @@ def main(): new_module + '::' + new_check_name_camel) replaceInFile(filename, old_module + '/' + check_name_camel, new_module + '/' + new_check_name_camel) + replaceInFile(filename, old_module + '/' + old_name, + new_module + '/' + new_name) replaceInFile(filename, check_name_camel, new_check_name_camel) if old_module != new_module or new_module == 'llvm': @@ -311,13 +320,12 @@ def main(): 'namespace clang::tidy::' + old_module + '[^ \n]*', 'namespace clang::tidy::' + new_namespace) - if (args.old_check_name == args.new_check_name): - return + if old_module != new_module: - # Add check to the new module. - adapt_cmake(new_module_path, new_check_name_camel) - adapt_module(new_module_path, new_module, args.new_check_name, - new_check_name_camel) + # Add check to the new module. + adapt_cmake(new_module_path, new_check_name_camel) + adapt_module(new_module_path, new_module, args.new_check_name, + new_check_name_camel) os.system(os.path.join(clang_tidy_path, 'add_new_check.py') + ' --update-docs') diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index e3da6fb9b0967..30e29182908d7 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -300,7 +300,7 @@ def main(): build_path) tmpdir = None - if args.fix or (yaml and args.export_fixes): + if args.fix: clang_apply_replacements_binary = find_binary( args.clang_apply_replacements_binary, "clang-apply-replacements", build_path) diff --git a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp index beb4c44467a80..f9555be57e738 100644 --- a/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp +++ b/clang-tools-extra/clang-tidy/utils/ExprSequence.cpp @@ -8,6 +8,7 @@ #include "ExprSequence.h" #include "clang/AST/ParentMapContext.h" +#include "llvm/ADT/SmallVector.h" #include namespace clang::tidy::utils { @@ -49,6 +50,7 @@ static SmallVector getParentStmts(const Stmt *S, } namespace { + bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor, ASTContext *Context) { if (Descendant == Ancestor) @@ -60,6 +62,17 @@ bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor, return false; } + +llvm::SmallVector +getAllInitListForms(const InitListExpr *InitList) { + llvm::SmallVector result = {InitList}; + if (const InitListExpr *AltForm = InitList->getSyntacticForm()) + result.push_back(AltForm); + if (const InitListExpr *AltForm = InitList->getSemanticForm()) + result.push_back(AltForm); + return result; +} + } // namespace ExprSequence::ExprSequence(const CFG *TheCFG, const Stmt *Root, @@ -111,9 +124,12 @@ const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const { } else if (const auto *InitList = dyn_cast(Parent)) { // Initializer list: Each initializer clause is sequenced after the // clauses that precede it. - for (unsigned I = 1; I < InitList->getNumInits(); ++I) { - if (InitList->getInit(I - 1) == S) - return InitList->getInit(I); + for (const InitListExpr *Form : getAllInitListForms(InitList)) { + for (unsigned I = 1; I < Form->getNumInits(); ++I) { + if (Form->getInit(I - 1) == S) { + return Form->getInit(I); + } + } } } else if (const auto *Compound = dyn_cast(Parent)) { // Compound statement: Each sub-statement is sequenced after the diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 98fa285f71051..260c44b2064b5 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -436,9 +436,8 @@ struct CodeCompletionBuilder { Bundled.emplace_back(); BundledEntry &S = Bundled.back(); if (C.SemaResult) { - bool IsPattern = C.SemaResult->Kind == CodeCompletionResult::RK_Pattern; - getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, - &Completion.RequiredQualifier, IsPattern); + getSignature(*SemaCCS, &S.Signature, &S.SnippetSuffix, C.SemaResult->Kind, + C.SemaResult->CursorKind, &Completion.RequiredQualifier); if (!C.SemaResult->FunctionCanBeCall) S.SnippetSuffix.clear(); S.ReturnType = getReturnType(*SemaCCS); diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index 21f83451f7014..26f8e0e602b2b 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "CodeCompletionStrings.h" +#include "clang-c/Index.h" #include "clang/AST/ASTContext.h" #include "clang/AST/RawCommentList.h" #include "clang/Basic/SourceManager.h" @@ -56,6 +57,26 @@ bool looksLikeDocComment(llvm::StringRef CommentText) { return CommentText.find_first_not_of("/*-= \t\r\n") != llvm::StringRef::npos; } +// Determine whether the completion string should be patched +// to replace the last placeholder with $0. +bool shouldPatchPlaceholder0(CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind) { + bool CompletingPattern = ResultKind == CodeCompletionResult::RK_Pattern; + + if (!CompletingPattern) + return false; + + // If the result kind of CodeCompletionResult(CCR) is `RK_Pattern`, it doesn't + // always mean we're completing a chunk of statements. Constructors defined + // in base class, for example, are considered as a type of pattern, with the + // cursor type set to CXCursor_Constructor. + if (CursorKind == CXCursorKind::CXCursor_Constructor || + CursorKind == CXCursorKind::CXCursor_Destructor) + return false; + + return true; +} + } // namespace std::string getDocComment(const ASTContext &Ctx, @@ -95,17 +116,20 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { } void getSignature(const CodeCompletionString &CCS, std::string *Signature, - std::string *Snippet, std::string *RequiredQualifiers, - bool CompletingPattern) { - // Placeholder with this index will be ${0:…} to mark final cursor position. + std::string *Snippet, + CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind, std::string *RequiredQualifiers) { + // Placeholder with this index will be $0 to mark final cursor position. // Usually we do not add $0, so the cursor is placed at end of completed text. unsigned CursorSnippetArg = std::numeric_limits::max(); - if (CompletingPattern) { - // In patterns, it's best to place the cursor at the last placeholder, to - // handle cases like - // namespace ${1:name} { - // ${0:decls} - // } + + // If the snippet contains a group of statements, we replace the + // last placeholder with $0 to leave the cursor there, e.g. + // namespace ${1:name} { + // ${0:decls} + // } + // We try to identify such cases using the ResultKind and CursorKind. + if (shouldPatchPlaceholder0(ResultKind, CursorKind)) { CursorSnippetArg = llvm::count_if(CCS, [](const CodeCompletionString::Chunk &C) { return C.Kind == CodeCompletionString::CK_Placeholder; @@ -185,7 +209,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature, *Snippet += Chunk.Text; break; case CodeCompletionString::CK_Optional: - assert(Chunk.Optional); + assert(Chunk.Optional); // No need to create placeholders for default arguments in Snippet. appendOptionalChunk(*Chunk.Optional, Signature); break; diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h index 6733d0231df49..566756ac9c8cc 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -42,12 +42,15 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D); /// If set, RequiredQualifiers is the text that must be typed before the name. /// e.g "Base::" when calling a base class member function that's hidden. /// -/// When \p CompletingPattern is true, the last placeholder will be of the form -/// ${0:…}, indicating the cursor should stay there. +/// When \p ResultKind is RK_Pattern, the last placeholder will be $0, +/// indicating the cursor should stay there. +/// Note that for certain \p CursorKind like \p CXCursor_Constructor, $0 won't +/// be emitted in order to avoid overlapping normal parameters. void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, - std::string *RequiredQualifiers = nullptr, - bool CompletingPattern = false); + CodeCompletionResult::ResultKind ResultKind, + CXCursorKind CursorKind, + std::string *RequiredQualifiers = nullptr); /// Assembles formatted documentation for a completion result. This includes /// documentation comments and other relevant information like annotations. diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index bcd39b2d38ba5..39b180e002a68 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -14,6 +14,7 @@ #include "clang/Driver/Options.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Tooling/CompilationDatabase.h" +#include "clang/Tooling/Tooling.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" @@ -27,6 +28,7 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" #include "llvm/Support/Program.h" +#include "llvm/TargetParser/Host.h" #include #include #include @@ -185,6 +187,12 @@ static std::string resolveDriver(llvm::StringRef Driver, bool FollowSymlink, } // namespace +CommandMangler::CommandMangler() { + Tokenizer = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows() + ? llvm::cl::TokenizeWindowsCommandLine + : llvm::cl::TokenizeGNUCommandLine; +} + CommandMangler CommandMangler::detect() { CommandMangler Result; Result.ClangPath = detectClangPath(); @@ -201,9 +209,18 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, trace::Span S("AdjustCompileFlags"); // Most of the modifications below assumes the Cmd starts with a driver name. // We might consider injecting a generic driver name like "cc" or "c++", but - // a Cmd missing the driver is probably rare enough in practice and errnous. + // a Cmd missing the driver is probably rare enough in practice and erroneous. if (Cmd.empty()) return; + + // FS used for expanding response files. + // FIXME: ExpandResponseFiles appears not to provide the usual + // thread-safety guarantees, as the access to FS is not locked! + // For now, use the real FS, which is known to be threadsafe (if we don't + // use/change working directory, which ExpandResponseFiles doesn't). + auto FS = llvm::vfs::getRealFileSystem(); + tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS); + auto &OptTable = clang::driver::getDriverOptTable(); // OriginalArgs needs to outlive ArgList. llvm::SmallVector OriginalArgs; @@ -212,7 +229,7 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, OriginalArgs.push_back(S.c_str()); bool IsCLMode = driver::IsClangCL(driver::getDriverMode( OriginalArgs[0], llvm::ArrayRef(OriginalArgs).slice(1))); - // ParseArgs propagates missig arg/opt counts on error, but preserves + // ParseArgs propagates missing arg/opt counts on error, but preserves // everything it could parse in ArgList. So we just ignore those counts. unsigned IgnoredCount; // Drop the executable name, as ParseArgs doesn't expect it. This means @@ -307,12 +324,16 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, // necessary for the system include extractor to identify the file type // - AFTER applying CompileFlags.Edits, because the name of the compiler // that needs to be invoked may come from the CompileFlags->Compiler key + // - BEFORE addTargetAndModeForProgramName(), because gcc doesn't support + // the target flag that might be added. // - BEFORE resolveDriver() because that can mess up the driver path, // e.g. changing gcc to /path/to/clang/bin/gcc if (SystemIncludeExtractor) { SystemIncludeExtractor(Command, File); } + tooling::addTargetAndModeForProgramName(Cmd, Cmd.front()); + // Check whether the flag exists, either as -flag or -flag=* auto Has = [&](llvm::StringRef Flag) { for (llvm::StringRef Arg : Cmd) { diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h index eb318d18baf63..1b37f44f0b9db 100644 --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -12,6 +12,7 @@ #include "support/Threading.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/CommandLine.h" #include #include #include @@ -50,10 +51,11 @@ struct CommandMangler { llvm::StringRef TargetFile) const; private: - CommandMangler() = default; + CommandMangler(); Memoize> ResolvedDrivers; Memoize> ResolvedDriversNoFollow; + llvm::cl::TokenizerCallback Tokenizer; }; // Removes args from a command-line in a semantically-aware way. diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index dffd54b01c459..daadf0ee3d3ce 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -88,12 +88,10 @@ struct Config { bool StandardLibrary = true; } Index; - enum class UnusedIncludesPolicy { - /// Diagnose unused includes. + enum class IncludesPolicy { + /// Diagnose missing and unused includes. Strict, None, - /// The same as Strict, but using the include-cleaner library. - Experiment, }; /// Controls warnings and errors when parsing code. struct { @@ -107,11 +105,12 @@ struct Config { llvm::StringMap CheckOptions; } ClangTidy; - UnusedIncludesPolicy UnusedIncludes = UnusedIncludesPolicy::None; - /// Enable emitting diagnostics using stale preambles. bool AllowStalePreamble = false; + IncludesPolicy UnusedIncludes = IncludesPolicy::None; + IncludesPolicy MissingIncludes = IncludesPolicy::None; + /// IncludeCleaner will not diagnose usages of these headers matched by /// these regexes. struct { diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index 18de6e4d5c3b6..fb6c6e86c1acd 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -430,17 +430,25 @@ struct FragmentCompiler { C.Diagnostics.Suppress.insert(N); }); - if (F.UnusedIncludes) - if (auto Val = - compileEnum("UnusedIncludes", - **F.UnusedIncludes) - .map("Strict", Config::UnusedIncludesPolicy::Strict) - .map("Experiment", Config::UnusedIncludesPolicy::Experiment) - .map("None", Config::UnusedIncludesPolicy::None) - .value()) + if (F.UnusedIncludes) { + auto Val = compileEnum("UnusedIncludes", + **F.UnusedIncludes) + .map("Strict", Config::IncludesPolicy::Strict) + .map("None", Config::IncludesPolicy::None) + .value(); + if (!Val && **F.UnusedIncludes == "Experiment") { + diag(Warning, + "Experiment is deprecated for UnusedIncludes, use Strict instead.", + F.UnusedIncludes->Range); + Val = Config::IncludesPolicy::Strict; + } + if (Val) { Out.Apply.push_back([Val](const Params &, Config &C) { C.Diagnostics.UnusedIncludes = *Val; }); + } + } + if (F.AllowStalePreamble) { if (auto Val = F.AllowStalePreamble) Out.Apply.push_back([Val](const Params &, Config &C) { @@ -448,6 +456,16 @@ struct FragmentCompiler { }); } + if (F.MissingIncludes) + if (auto Val = compileEnum("MissingIncludes", + **F.MissingIncludes) + .map("Strict", Config::IncludesPolicy::Strict) + .map("None", Config::IncludesPolicy::None) + .value()) + Out.Apply.push_back([Val](const Params &, Config &C) { + C.Diagnostics.MissingIncludes = *Val; + }); + compile(std::move(F.Includes)); compile(std::move(F.ClangTidy)); } diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index a5597596196fa..a56e919cbaf7a 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -221,7 +221,7 @@ struct Fragment { /// This often has other advantages, such as skipping some analysis. std::vector> Suppress; - /// Controls how clangd will correct "unnecessary #include directives. + /// Controls how clangd will correct "unnecessary" #include directives. /// clangd can warn if a header is `#include`d but not used, and suggest /// removing it. // @@ -232,13 +232,26 @@ struct Fragment { /// /// Valid values are: /// - Strict - /// - Experiment /// - None std::optional> UnusedIncludes; + /// Enable emitting diagnostics using stale preambles. std::optional> AllowStalePreamble; + /// Controls if clangd should analyze missing #include directives. + /// clangd will warn if no header providing a symbol is `#include`d + /// (missing) directly, and suggest adding it. + /// + /// Strict means a header providing a symbol is missing if it is not + /// *directly #include'd. The file might still compile if the header is + /// included transitively. + /// + /// Valid values are: + /// - Strict + /// - None + std::optional> MissingIncludes; + /// Controls IncludeCleaner diagnostics. struct IncludesBlock { /// Regexes that will be used to avoid diagnosing certain includes as diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 0ec0239fc71e6..84559f5c44f86 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -128,6 +128,9 @@ class Parser { Dict.handle("UnusedIncludes", [&](Node &N) { F.UnusedIncludes = scalarValue(N, "UnusedIncludes"); }); + Dict.handle("MissingIncludes", [&](Node &N) { + F.MissingIncludes = scalarValue(N, "MissingIncludes"); + }); Dict.handle("Includes", [&](Node &N) { parse(F.Includes, N); }); Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); }); Dict.handle("AllowStalePreamble", [&](Node &N) { diff --git a/clang-tools-extra/clangd/FileDistance.cpp b/clang-tools-extra/clangd/FileDistance.cpp index cf10d158f08ca..a5fb3d2bcc35b 100644 --- a/clang-tools-extra/clangd/FileDistance.cpp +++ b/clang-tools-extra/clangd/FileDistance.cpp @@ -78,7 +78,7 @@ FileDistance::FileDistance(llvm::StringMap Sources, Down.push_back(Hash); // We can't just break after MaxUpTraversals, must still set DownEdges. if (I > S.getValue().MaxUpTraversals) { - if (Cache.find(Hash) != Cache.end()) + if (Cache.contains(Hash)) break; } else { unsigned Cost = S.getValue().Cost + I * Opts.UpCost; diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index def43683e0ab7..d1833759917a3 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -244,15 +244,7 @@ static std::unique_ptr parseJSON(PathRef Path, llvm::StringRef Data, std::string &Error) { if (auto CDB = tooling::JSONCompilationDatabase::loadFromBuffer( Data, Error, tooling::JSONCommandLineSyntax::AutoDetect)) { - // FS used for expanding response files. - // FIXME: ExpandResponseFilesDatabase appears not to provide the usual - // thread-safety guarantees, as the access to FS is not locked! - // For now, use the real FS, which is known to be threadsafe (if we don't - // use/change working directory, which ExpandResponseFilesDatabase doesn't). - auto FS = llvm::vfs::getRealFileSystem(); - return tooling::inferTargetAndDriverMode( - tooling::inferMissingCompileCommands( - expandResponseFiles(std::move(CDB), std::move(FS)))); + return tooling::inferMissingCompileCommands(std::move(CDB)); } return nullptr; } diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp index 7000c1cbf4513..344afdba9622a 100644 --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -25,8 +25,7 @@ namespace clang { namespace clangd { -class IncludeStructure::RecordHeaders : public PPCallbacks, - public CommentHandler { +class IncludeStructure::RecordHeaders : public PPCallbacks { public: RecordHeaders(const CompilerInstance &CI, IncludeStructure *Out) : SM(CI.getSourceManager()), @@ -61,8 +60,6 @@ class IncludeStructure::RecordHeaders : public PPCallbacks, SM.getLineNumber(SM.getFileID(HashLoc), Inc.HashOffset) - 1; Inc.FileKind = FileKind; Inc.Directive = IncludeTok.getIdentifierInfo()->getPPKeywordID(); - if (LastPragmaKeepInMainFileLine == Inc.HashLine) - Inc.BehindPragmaKeep = true; if (File) { IncludeStructure::HeaderID HID = Out->getOrCreateID(*File); Inc.HeaderID = static_cast(HID); @@ -108,17 +105,6 @@ class IncludeStructure::RecordHeaders : public PPCallbacks, --Level; if (PrevFID == BuiltinFile) InBuiltinFile = false; - // At file exit time HeaderSearchInfo is valid and can be used to - // determine whether the file was a self-contained header or not. - if (const FileEntry *FE = SM.getFileEntryForID(PrevFID)) { - // isSelfContainedHeader only returns true once the full header-guard - // structure has been seen, i.e. when exiting the *outer* copy of the - // file. So last result wins. - if (tooling::isSelfContainedHeader(FE, SM, HeaderInfo)) - Out->NonSelfContained.erase(*Out->getID(FE)); - else - Out->NonSelfContained.insert(*Out->getID(FE)); - } break; } case PPCallbacks::RenameFile: @@ -127,47 +113,6 @@ class IncludeStructure::RecordHeaders : public PPCallbacks, } } - bool HandleComment(Preprocessor &PP, SourceRange Range) override { - auto Pragma = - tooling::parseIWYUPragma(SM.getCharacterData(Range.getBegin())); - if (!Pragma) - return false; - - if (inMainFile()) { - // Given: - // - // #include "foo.h" - // #include "bar.h" // IWYU pragma: keep - // - // The order in which the callbacks will be triggered: - // - // 1. InclusionDirective("foo.h") - // 2. handleCommentInMainFile("// IWYU pragma: keep") - // 3. InclusionDirective("bar.h") - // - // This code stores the last location of "IWYU pragma: keep" (or export) - // comment in the main file, so that when InclusionDirective is called, it - // will know that the next inclusion is behind the IWYU pragma. - // FIXME: Support "IWYU pragma: begin_exports" and "IWYU pragma: - // end_exports". - if (!Pragma->startswith("export") && !Pragma->startswith("keep")) - return false; - unsigned Offset = SM.getFileOffset(Range.getBegin()); - LastPragmaKeepInMainFileLine = - SM.getLineNumber(SM.getMainFileID(), Offset) - 1; - } else { - // Memorize headers that have export pragmas in them. Include Cleaner - // does not support them properly yet, so they will be not marked as - // unused. - // FIXME: Once IncludeCleaner supports export pragmas, remove this. - if (!Pragma->startswith("export") && !Pragma->startswith("begin_exports")) - return false; - Out->HasIWYUExport.insert( - *Out->getID(SM.getFileEntryForID(SM.getFileID(Range.getBegin())))); - } - return false; - } - private: // Keeps track of include depth for the current file. It's 1 for main file. int Level = 0; @@ -181,9 +126,6 @@ class IncludeStructure::RecordHeaders : public PPCallbacks, bool InBuiltinFile = false; IncludeStructure *Out; - - // The last line "IWYU pragma: keep" was seen in the main file, 0-indexed. - int LastPragmaKeepInMainFileLine = -1; }; bool isLiteralInclude(llvm::StringRef Include) { @@ -234,7 +176,6 @@ void IncludeStructure::collect(const CompilerInstance &CI) { auto &SM = CI.getSourceManager(); MainFileEntry = SM.getFileEntryForID(SM.getMainFileID()); auto Collector = std::make_unique(CI, this); - CI.getPreprocessor().addCommentHandler(Collector.get()); CI.getPreprocessor().addPPCallbacks(std::move(Collector)); } @@ -321,7 +262,7 @@ bool IncludeInserter::shouldInsertInclude( if (FileName == DeclaringHeader || FileName == InsertedHeader.File) return false; auto Included = [&](llvm::StringRef Header) { - return IncludedHeaders.find(Header) != IncludedHeaders.end(); + return IncludedHeaders.contains(Header); }; return !Included(DeclaringHeader) && !Included(InsertedHeader.File); } diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h index a21ea851831aa..f6aeab1dbfd35 100644 --- a/clang-tools-extra/clangd/Headers.h +++ b/clang-tools-extra/clangd/Headers.h @@ -73,7 +73,6 @@ struct Inclusion { int HashLine = 0; // Line number containing the directive, 0-indexed. SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User; std::optional HeaderID; - bool BehindPragmaKeep = false; // Has IWYU pragma: keep right after. }; llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &); bool operator==(const Inclusion &LHS, const Inclusion &RHS); @@ -151,12 +150,6 @@ class IncludeStructure { return RealPathNames[static_cast(ID)]; } - bool isSelfContained(HeaderID ID) const { - return !NonSelfContained.contains(ID); - } - - bool hasIWYUExport(HeaderID ID) const { return HasIWYUExport.contains(ID); } - // Return all transitively reachable files. llvm::ArrayRef allHeaders() const { return RealPathNames; } @@ -199,12 +192,6 @@ class IncludeStructure { // and RealPathName and UniqueID are not preserved in // the preamble. llvm::DenseMap UIDToIndex; - // Contains HeaderIDs of all non self-contained entries in the - // IncludeStructure. - llvm::DenseSet NonSelfContained; - // Contains a set of headers that have either "IWYU pragma: export" or "IWYU - // pragma: begin_exports". - llvm::DenseSet HasIWYUExport; // Maps written includes to indices in MainFileInclude for easier lookup by // spelling. diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 7d523d9a63641..98135529f259b 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -8,32 +8,53 @@ #include "IncludeCleaner.h" #include "Config.h" +#include "Diagnostics.h" #include "Headers.h" #include "ParsedAST.h" #include "Protocol.h" #include "SourceCode.h" +#include "URI.h" #include "clang-include-cleaner/Analysis.h" +#include "clang-include-cleaner/Record.h" #include "clang-include-cleaner/Types.h" -#include "index/CanonicalIncludes.h" #include "support/Logger.h" +#include "support/Path.h" #include "support/Trace.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" -#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/TemplateName.h" +#include "clang/AST/Type.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/Format/Format.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Inclusions/HeaderIncludes.h" +#include "clang/Tooling/Inclusions/StandardLibrary.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" -#include +#include #include +#include +#include namespace clang { namespace clangd { @@ -43,181 +64,6 @@ void setIncludeCleanerAnalyzesStdlib(bool B) { AnalyzeStdlib = B; } namespace { -/// Crawler traverses the AST and feeds in the locations of (sometimes -/// implicitly) used symbols into \p Result. -class ReferencedLocationCrawler - : public RecursiveASTVisitor { -public: - ReferencedLocationCrawler(ReferencedLocations &Result, - const SourceManager &SM) - : Result(Result), SM(SM) {} - - bool VisitDeclRefExpr(DeclRefExpr *DRE) { - add(DRE->getDecl()); - add(DRE->getFoundDecl()); - return true; - } - - bool VisitMemberExpr(MemberExpr *ME) { - add(ME->getMemberDecl()); - add(ME->getFoundDecl().getDecl()); - return true; - } - - bool VisitTagType(TagType *TT) { - add(TT->getDecl()); - return true; - } - - bool VisitFunctionDecl(FunctionDecl *FD) { - // Function definition will require redeclarations to be included. - if (FD->isThisDeclarationADefinition()) - add(FD); - return true; - } - - bool VisitCXXConstructExpr(CXXConstructExpr *CCE) { - add(CCE->getConstructor()); - return true; - } - - bool VisitTemplateSpecializationType(TemplateSpecializationType *TST) { - // Using templateName case is handled by the override TraverseTemplateName. - if (TST->getTemplateName().getKind() == TemplateName::UsingTemplate) - return true; - add(TST->getAsCXXRecordDecl()); // Specialization - return true; - } - - // There is no VisitTemplateName in RAV, thus we override the Traverse version - // to handle the Using TemplateName case. - bool TraverseTemplateName(TemplateName TN) { - VisitTemplateName(TN); - return Base::TraverseTemplateName(TN); - } - // A pseudo VisitTemplateName, dispatched by the above TraverseTemplateName! - bool VisitTemplateName(TemplateName TN) { - if (const auto *USD = TN.getAsUsingShadowDecl()) { - add(USD); - return true; - } - add(TN.getAsTemplateDecl()); // Primary template. - return true; - } - - bool VisitUsingType(UsingType *UT) { - add(UT->getFoundDecl()); - return true; - } - - bool VisitTypedefType(TypedefType *TT) { - add(TT->getDecl()); - return true; - } - - // Consider types of any subexpression used, even if the type is not named. - // This is helpful in getFoo().bar(), where Foo must be complete. - // FIXME(kirillbobyrev): Should we tweak this? It may not be desirable to - // consider types "used" when they are not directly spelled in code. - bool VisitExpr(Expr *E) { - TraverseType(E->getType()); - return true; - } - - bool TraverseType(QualType T) { - if (isNew(T.getTypePtrOrNull())) // don't care about quals - Base::TraverseType(T); - return true; - } - - bool VisitUsingDecl(UsingDecl *D) { - for (const auto *Shadow : D->shadows()) - add(Shadow->getTargetDecl()); - return true; - } - - // Enums may be usefully forward-declared as *complete* types by specifying - // an underlying type. In this case, the definition should see the declaration - // so they can be checked for compatibility. - bool VisitEnumDecl(EnumDecl *D) { - if (D->isThisDeclarationADefinition() && D->getIntegerTypeSourceInfo()) - add(D); - return true; - } - - // When the overload is not resolved yet, mark all candidates as used. - bool VisitOverloadExpr(OverloadExpr *E) { - for (const auto *ResolutionDecl : E->decls()) - add(ResolutionDecl); - return true; - } - -private: - using Base = RecursiveASTVisitor; - - void add(const Decl *D) { - if (!D || !isNew(D->getCanonicalDecl())) - return; - if (auto SS = StdRecognizer(D)) { - Result.Stdlib.insert(*SS); - return; - } - // Special case RecordDecls, as it is common for them to be forward - // declared multiple times. The most common cases are: - // - Definition available in TU, only mark that one as usage. The rest is - // likely to be unnecessary. This might result in false positives when an - // internal definition is visible. - // - There's a forward declaration in the main file, no need for other - // redecls. - if (const auto *RD = llvm::dyn_cast(D)) { - if (const auto *Definition = RD->getDefinition()) { - Result.User.insert(Definition->getLocation()); - return; - } - if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation())) - return; - } - for (const Decl *Redecl : D->redecls()) - Result.User.insert(Redecl->getLocation()); - } - - bool isNew(const void *P) { return P && Visited.insert(P).second; } - - ReferencedLocations &Result; - llvm::DenseSet Visited; - const SourceManager &SM; - tooling::stdlib::Recognizer StdRecognizer; -}; - -// Given a set of referenced FileIDs, determines all the potentially-referenced -// files and macros by traversing expansion/spelling locations of macro IDs. -// This is used to map the referenced SourceLocations onto real files. -struct ReferencedFilesBuilder { - ReferencedFilesBuilder(const SourceManager &SM) : SM(SM) {} - llvm::DenseSet Files; - llvm::DenseSet Macros; - const SourceManager &SM; - - void add(SourceLocation Loc) { add(SM.getFileID(Loc), Loc); } - - void add(FileID FID, SourceLocation Loc) { - if (FID.isInvalid()) - return; - assert(SM.isInFileID(Loc, FID)); - if (Loc.isFileID()) { - Files.insert(FID); - return; - } - // Don't process the same macro FID twice. - if (!Macros.insert(FID).second) - return; - const auto &Exp = SM.getSLocEntry(FID).getExpansion(); - add(Exp.getSpellingLoc()); - add(Exp.getExpansionLocStart()); - add(Exp.getExpansionLocEnd()); - } -}; - // Returns the range starting at '#' and ending at EOL. Escaped newlines are not // handled. clangd::Range getDiagnosticRange(llvm::StringRef Code, unsigned HashOffset) { @@ -232,37 +78,23 @@ clangd::Range getDiagnosticRange(llvm::StringRef Code, unsigned HashOffset) { return Result; } -// Finds locations of macros referenced from within the main file. That includes -// references that were not yet expanded, e.g `BAR` in `#define FOO BAR`. -void findReferencedMacros(const SourceManager &SM, Preprocessor &PP, - const syntax::TokenBuffer *Tokens, - ReferencedLocations &Result) { - trace::Span Tracer("IncludeCleaner::findReferencedMacros"); - // FIXME(kirillbobyrev): The macros from the main file are collected in - // ParsedAST's MainFileMacros. However, we can't use it here because it - // doesn't handle macro references that were not expanded, e.g. in macro - // definitions or preprocessor-disabled sections. - // - // Extending MainFileMacros to collect missing references and switching to - // this mechanism (as opposed to iterating through all tokens) will improve - // the performance of findReferencedMacros and also improve other features - // relying on MainFileMacros. - for (const syntax::Token &Tok : Tokens->spelledTokens(SM.getMainFileID())) { - auto Macro = locateMacroAt(Tok, PP); - if (!Macro) - continue; - auto Loc = Macro->Info->getDefinitionLoc(); - if (Loc.isValid()) - Result.User.insert(Loc); - // FIXME: support stdlib macros + +bool isFilteredByConfig(const Config &Cfg, llvm::StringRef HeaderPath) { + // Convert the path to Unix slashes and try to match against the filter. + llvm::SmallString<64> NormalizedPath(HeaderPath); + llvm::sys::path::native(NormalizedPath, llvm::sys::path::Style::posix); + for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) { + if (Filter(NormalizedPath)) + return true; } + return false; } static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, - const Config &Cfg) { - if (Inc.BehindPragmaKeep) + const Config &Cfg, + const include_cleaner::PragmaIncludes *PI) { + if (PI && PI->shouldKeep(Inc.HashLine + 1)) return false; - // FIXME(kirillbobyrev): We currently do not support the umbrella headers. // System headers are likely to be standard library headers. // Until we have good support for umbrella headers, don't warn about them. @@ -273,10 +105,6 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, } assert(Inc.HeaderID); auto HID = static_cast(*Inc.HeaderID); - // FIXME: Ignore the headers with IWYU export pragmas for now, remove this - // check when we have more precise tracking of exported headers. - if (AST.getIncludeStructure().hasIWYUExport(HID)) - return false; auto FE = AST.getSourceManager().getFileManager().getFileRef( AST.getIncludeStructure().getRealPath(HID)); assert(FE); @@ -288,120 +116,205 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, FE->getName()); return false; } - for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) { - // Convert the path to Unix slashes and try to match against the filter. - llvm::SmallString<64> Path(Inc.Resolved); - llvm::sys::path::native(Path, llvm::sys::path::Style::posix); - if (Filter(Path)) { - dlog("{0} header is filtered out by the configuration", FE->getName()); - return false; - } + + if (isFilteredByConfig(Cfg, Inc.Resolved)) { + dlog("{0} header is filtered out by the configuration", FE->getName()); + return false; } return true; } -// In case symbols are coming from non self-contained header, we need to find -// its first includer that is self-contained. This is the header users can -// include, so it will be responsible for bringing the symbols from given -// header into the scope. -FileID headerResponsible(FileID ID, const SourceManager &SM, - const IncludeStructure &Includes) { - // Unroll the chain of non self-contained headers until we find the one that - // can be included. - for (const FileEntry *FE = SM.getFileEntryForID(ID); ID != SM.getMainFileID(); - FE = SM.getFileEntryForID(ID)) { - // If FE is nullptr, we consider it to be the responsible header. - if (!FE) - break; - auto HID = Includes.getID(FE); - assert(HID && "We're iterating over headers already existing in " - "IncludeStructure"); - if (Includes.isSelfContained(*HID)) - break; - // The header is not self-contained: put the responsibility for its symbols - // on its includer. - ID = SM.getFileID(SM.getIncludeLoc(ID)); +include_cleaner::Includes +convertIncludes(const SourceManager &SM, + const llvm::ArrayRef MainFileIncludes) { + include_cleaner::Includes Includes; + for (const Inclusion &Inc : MainFileIncludes) { + include_cleaner::Include TransformedInc; + llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written); + TransformedInc.Spelled = WrittenRef.trim("\"<>"); + TransformedInc.HashLocation = + SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset); + TransformedInc.Line = Inc.HashLine + 1; + TransformedInc.Angled = WrittenRef.starts_with("<"); + auto FE = SM.getFileManager().getFile(Inc.Resolved); + if (!FE) { + elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}", + Inc.Resolved, FE.getError().message()); + continue; + } + TransformedInc.Resolved = *FE; + Includes.add(std::move(TransformedInc)); } - return ID; + return Includes; } -} // namespace +std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile, + include_cleaner::Header Provider) { + if (Provider.kind() == include_cleaner::Header::Physical) { + if (auto CanonicalPath = + getCanonicalPath(Provider.physical(), AST.getSourceManager())) { + std::string SpelledHeader = + llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath))); + if (!SpelledHeader.empty()) + return SpelledHeader; + } + } + return include_cleaner::spellHeader( + Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile); +} -ReferencedLocations findReferencedLocations(ASTContext &Ctx, Preprocessor &PP, - const syntax::TokenBuffer *Tokens) { - trace::Span Tracer("IncludeCleaner::findReferencedLocations"); - ReferencedLocations Result; - const auto &SM = Ctx.getSourceManager(); - ReferencedLocationCrawler Crawler(Result, SM); - Crawler.TraverseAST(Ctx); - if (Tokens) - findReferencedMacros(SM, PP, Tokens, Result); - return Result; +std::vector +collectMacroReferences(ParsedAST &AST) { + const auto &SM = AST.getSourceManager(); + // FIXME: !!this is a hacky way to collect macro references. + std::vector Macros; + auto &PP = AST.getPreprocessor(); + for (const syntax::Token &Tok : + AST.getTokens().spelledTokens(SM.getMainFileID())) { + auto Macro = locateMacroAt(Tok, PP); + if (!Macro) + continue; + if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid()) + Macros.push_back( + {Tok.location(), + include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)), + DefLoc}, + include_cleaner::RefType::Explicit}); + } + return Macros; +} + +llvm::StringRef getResolvedPath(const include_cleaner::Header &SymProvider) { + switch (SymProvider.kind()) { + case include_cleaner::Header::Physical: + return SymProvider.physical()->tryGetRealPathName(); + case include_cleaner::Header::Standard: + return SymProvider.standard().name().trim("<>\""); + case include_cleaner::Header::Verbatim: + return SymProvider.verbatim().trim("<>\""); + } + llvm_unreachable("Unknown header kind"); } -ReferencedLocations findReferencedLocations(ParsedAST &AST) { - return findReferencedLocations(AST.getASTContext(), AST.getPreprocessor(), - &AST.getTokens()); +std::string getSymbolName(const include_cleaner::Symbol &Sym) { + switch (Sym.kind()) { + case include_cleaner::Symbol::Macro: + return Sym.macro().Name->getName().str(); + case include_cleaner::Symbol::Declaration: + return llvm::dyn_cast(&Sym.declaration()) + ->getQualifiedNameAsString(); + } + llvm_unreachable("Unknown symbol kind"); } -ReferencedFiles findReferencedFiles( - const ReferencedLocations &Locs, const SourceManager &SM, - llvm::function_ref HeaderResponsible, - llvm::function_ref(FileID)> UmbrellaHeader) { - std::vector Sorted{Locs.User.begin(), Locs.User.end()}; - llvm::sort(Sorted); // Group by FileID. - ReferencedFilesBuilder Builder(SM); - for (auto It = Sorted.begin(); It < Sorted.end();) { - FileID FID = SM.getFileID(*It); - Builder.add(FID, *It); - // Cheaply skip over all the other locations from the same FileID. - // This avoids lots of redundant Loc->File lookups for the same file. - do - ++It; - while (It != Sorted.end() && SM.isInFileID(*It, FID)); +std::vector generateMissingIncludeDiagnostics( + ParsedAST &AST, llvm::ArrayRef MissingIncludes, + llvm::StringRef Code) { + std::vector Result; + const Config &Cfg = Config::current(); + if (Cfg.Diagnostics.MissingIncludes != Config::IncludesPolicy::Strict || + Cfg.Diagnostics.SuppressAll || + Cfg.Diagnostics.Suppress.contains("missing-includes")) { + return Result; } - // If a header is not self-contained, we consider its symbols a logical part - // of the including file. Therefore, mark the parents of all used - // non-self-contained FileIDs as used. Perform this on FileIDs rather than - // HeaderIDs, as each inclusion of a non-self-contained file is distinct. - llvm::DenseSet UserFiles; - llvm::StringSet<> PublicHeaders; - for (FileID ID : Builder.Files) { - UserFiles.insert(HeaderResponsible(ID)); - if (auto PublicHeader = UmbrellaHeader(ID)) { - PublicHeaders.insert(*PublicHeader); - } + const SourceManager &SM = AST.getSourceManager(); + const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); + + auto FileStyle = format::getStyle( + format::DefaultFormatStyle, AST.tuPath(), format::DefaultFallbackStyle, + Code, &SM.getFileManager().getVirtualFileSystem()); + if (!FileStyle) { + elog("Couldn't infer style", FileStyle.takeError()); + FileStyle = format::getLLVMStyle(); } - llvm::DenseSet StdlibFiles; - for (const auto &Symbol : Locs.Stdlib) - for (const auto &Header : Symbol.headers()) - StdlibFiles.insert(Header); + tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code, + FileStyle->IncludeStyle); + for (const auto &SymbolWithMissingInclude : MissingIncludes) { + llvm::StringRef ResolvedPath = + getResolvedPath(SymbolWithMissingInclude.Providers.front()); + if (isFilteredByConfig(Cfg, ResolvedPath)) { + dlog("IncludeCleaner: not diagnosing missing include {0}, filtered by " + "config", + ResolvedPath); + continue; + } + + std::string Spelling = + spellHeader(AST, MainFile, SymbolWithMissingInclude.Providers.front()); + llvm::StringRef HeaderRef{Spelling}; + bool Angled = HeaderRef.starts_with("<"); + // We might suggest insertion of an existing include in edge cases, e.g., + // include is present in a PP-disabled region, or spelling of the header + // turns out to be the same as one of the unresolved includes in the + // main file. + std::optional Replacement = HeaderIncludes.insert( + HeaderRef.trim("\"<>"), Angled, tooling::IncludeDirective::Include); + if (!Replacement.has_value()) + continue; - return {std::move(UserFiles), std::move(StdlibFiles), - std::move(PublicHeaders)}; + Diag &D = Result.emplace_back(); + D.Message = + llvm::formatv("No header providing \"{0}\" is directly included", + getSymbolName(SymbolWithMissingInclude.Symbol)); + D.Name = "missing-includes"; + D.Source = Diag::DiagSource::Clangd; + D.File = AST.tuPath(); + D.InsideMainFile = true; + D.Severity = DiagnosticsEngine::Warning; + D.Range = clangd::Range{ + offsetToPosition(Code, + SymbolWithMissingInclude.SymRefRange.beginOffset()), + offsetToPosition(Code, + SymbolWithMissingInclude.SymRefRange.endOffset())}; + auto &F = D.Fixes.emplace_back(); + F.Message = "#include " + Spelling; + TextEdit Edit = replacementToEdit(Code, *Replacement); + F.Edits.emplace_back(std::move(Edit)); + } + return Result; } -ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs, - const IncludeStructure &Includes, - const CanonicalIncludes &CanonIncludes, - const SourceManager &SM) { - return findReferencedFiles( - Locs, SM, - [&SM, &Includes](FileID ID) { - return headerResponsible(ID, SM, Includes); - }, - [&SM, &CanonIncludes](FileID ID) -> std::optional { - auto Entry = SM.getFileEntryRefForID(ID); - if (!Entry) - return std::nullopt; - auto PublicHeader = CanonIncludes.mapHeader(*Entry); - if (PublicHeader.empty()) - return std::nullopt; - return PublicHeader; - }); +std::vector generateUnusedIncludeDiagnostics( + PathRef FileName, llvm::ArrayRef UnusedIncludes, + llvm::StringRef Code) { + std::vector Result; + const Config &Cfg = Config::current(); + if (Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::None || + Cfg.Diagnostics.SuppressAll || + Cfg.Diagnostics.Suppress.contains("unused-includes")) { + return Result; + } + for (const auto *Inc : UnusedIncludes) { + Diag &D = Result.emplace_back(); + D.Message = + llvm::formatv("included header {0} is not used directly", + llvm::sys::path::filename( + Inc->Written.substr(1, Inc->Written.size() - 2), + llvm::sys::path::Style::posix)); + D.Name = "unused-includes"; + D.Source = Diag::DiagSource::Clangd; + D.File = FileName; + D.InsideMainFile = true; + D.Severity = DiagnosticsEngine::Warning; + D.Tags.push_back(Unnecessary); + D.Range = getDiagnosticRange(Code, Inc->HashOffset); + // FIXME(kirillbobyrev): Removing inclusion might break the code if the + // used headers are only reachable transitively through this one. Suggest + // including them directly instead. + // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas + // (keep/export) remove the warning once we support IWYU pragmas. + auto &F = D.Fixes.emplace_back(); + F.Message = "remove #include directive"; + F.Edits.emplace_back(); + F.Edits.back().range.start.line = Inc->HashLine; + F.Edits.back().range.end.line = Inc->HashLine + 1; + } + return Result; } +} // namespace + std::vector getUnused(ParsedAST &AST, @@ -417,7 +330,7 @@ getUnused(ParsedAST &AST, continue; auto IncludeID = static_cast(*MFI.HeaderID); bool Used = ReferencedFiles.contains(IncludeID); - if (!Used && !mayConsiderUnused(MFI, AST, Cfg)) { + if (!Used && !mayConsiderUnused(MFI, AST, Cfg, AST.getPragmaIncludes())) { dlog("{0} was not used, but is not eligible to be diagnosed as unused", MFI.Written); continue; @@ -429,150 +342,80 @@ getUnused(ParsedAST &AST, return Unused; } -#ifndef NDEBUG -// Is FID a , etc? -static bool isSpecialBuffer(FileID FID, const SourceManager &SM) { - const SrcMgr::FileInfo &FI = SM.getSLocEntry(FID).getFile(); - return FI.getName().startswith("<"); -} -#endif - -llvm::DenseSet -translateToHeaderIDs(const ReferencedFiles &Files, - const IncludeStructure &Includes, - const SourceManager &SM) { - trace::Span Tracer("IncludeCleaner::translateToHeaderIDs"); - llvm::DenseSet TranslatedHeaderIDs; - TranslatedHeaderIDs.reserve(Files.User.size()); - for (FileID FID : Files.User) { - const FileEntry *FE = SM.getFileEntryForID(FID); - if (!FE) { - assert(isSpecialBuffer(FID, SM)); - continue; - } - const auto File = Includes.getID(FE); - assert(File); - TranslatedHeaderIDs.insert(*File); - } - for (tooling::stdlib::Header StdlibUsed : Files.Stdlib) - for (auto HID : Includes.StdlibHeaders.lookup(StdlibUsed)) - TranslatedHeaderIDs.insert(HID); - return TranslatedHeaderIDs; -} - -// This is the original clangd-own implementation for computing unused -// #includes. Eventually it will be deprecated and replaced by the -// include-cleaner-lib-based implementation. -std::vector computeUnusedIncludes(ParsedAST &AST) { - const auto &SM = AST.getSourceManager(); - - auto Refs = findReferencedLocations(AST); - auto ReferencedFiles = - findReferencedFiles(Refs, AST.getIncludeStructure(), - AST.getCanonicalIncludes(), AST.getSourceManager()); - auto ReferencedHeaders = - translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM); - return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas); -} -std::vector -computeUnusedIncludesExperimental(ParsedAST &AST) { +IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { const auto &SM = AST.getSourceManager(); const auto &Includes = AST.getIncludeStructure(); + include_cleaner::Includes ConvertedIncludes = + convertIncludes(SM, Includes.MainFileIncludes); + const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); - // FIXME: !!this is a hacky way to collect macro references. - std::vector Macros; - auto &PP = AST.getPreprocessor(); - for (const syntax::Token &Tok : - AST.getTokens().spelledTokens(SM.getMainFileID())) { - auto Macro = locateMacroAt(Tok, PP); - if (!Macro) - continue; - if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid()) - Macros.push_back( - {Tok.location(), - include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)), - DefLoc}, - include_cleaner::RefType::Explicit}); - } + std::vector Macros = + collectMacroReferences(AST); + std::vector MissingIncludes; llvm::DenseSet Used; + trace::Span Tracer("include_cleaner::walkUsed"); include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, AST.getPragmaIncludes(), SM, [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef Providers) { + bool Satisfied = false; for (const auto &H : Providers) { - switch (H.kind()) { - case include_cleaner::Header::Physical: - if (auto HeaderID = Includes.getID(H.physical())) - Used.insert(*HeaderID); - break; - case include_cleaner::Header::Standard: - for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard())) - Used.insert(HeaderID); - break; - case include_cleaner::Header::Verbatim: - for (auto *Inc : - Includes.mainFileIncludesWithSpelling(H.verbatim())) { - if (!Inc->HeaderID.has_value()) - continue; - IncludeStructure::HeaderID ID = - static_cast(*Inc->HeaderID); - Used.insert(ID); - } - break; + if (H.kind() == include_cleaner::Header::Physical && + H.physical() == MainFile) { + Satisfied = true; + continue; + } + for (auto *Inc : ConvertedIncludes.match(H)) { + Satisfied = true; + auto HeaderID = Includes.getID(Inc->Resolved); + assert(HeaderID.has_value() && + "ConvertedIncludes only contains resolved includes."); + Used.insert(*HeaderID); } } + + if (Satisfied || Providers.empty() || + Ref.RT != include_cleaner::RefType::Explicit) + return; + + auto &Tokens = AST.getTokens(); + auto SpelledForExpanded = + Tokens.spelledForExpanded(Tokens.expandedTokens(Ref.RefLocation)); + if (!SpelledForExpanded) + return; + + auto Range = syntax::Token::range(SM, SpelledForExpanded->front(), + SpelledForExpanded->back()); + MissingIncludeDiagInfo DiagInfo{Ref.Target, Range, Providers}; + MissingIncludes.push_back(std::move(DiagInfo)); }); - return getUnused(AST, Used, /*ReferencedPublicHeaders*/ {}); + std::vector UnusedIncludes = + getUnused(AST, Used, /*ReferencedPublicHeaders*/ {}); + return {std::move(UnusedIncludes), std::move(MissingIncludes)}; } -std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, +std::vector issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code) { - const Config &Cfg = Config::current(); - if (Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::None || - Cfg.Diagnostics.SuppressAll || - Cfg.Diagnostics.Suppress.contains("unused-includes")) - return {}; // Interaction is only polished for C/CPP. if (AST.getLangOpts().ObjC) return {}; - trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics"); - std::vector Result; - std::string FileName = - AST.getSourceManager() - .getFileEntryRefForID(AST.getSourceManager().getMainFileID()) - ->getName() - .str(); - const auto &UnusedIncludes = - Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::Experiment - ? computeUnusedIncludesExperimental(AST) - : computeUnusedIncludes(AST); - for (const auto *Inc : UnusedIncludes) { - Diag D; - D.Message = - llvm::formatv("included header {0} is not used directly", - llvm::sys::path::filename( - Inc->Written.substr(1, Inc->Written.size() - 2), - llvm::sys::path::Style::posix)); - D.Name = "unused-includes"; - D.Source = Diag::DiagSource::Clangd; - D.File = FileName; - D.Severity = DiagnosticsEngine::Warning; - D.Tags.push_back(Unnecessary); - D.Range = getDiagnosticRange(Code, Inc->HashOffset); - // FIXME(kirillbobyrev): Removing inclusion might break the code if the - // used headers are only reachable transitively through this one. Suggest - // including them directly instead. - // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas - // (keep/export) remove the warning once we support IWYU pragmas. - D.Fixes.emplace_back(); - D.Fixes.back().Message = "remove #include directive"; - D.Fixes.back().Edits.emplace_back(); - D.Fixes.back().Edits.back().range.start.line = Inc->HashLine; - D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1; - D.InsideMainFile = true; - Result.push_back(std::move(D)); + + trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics"); + + const Config &Cfg = Config::current(); + IncludeCleanerFindings Findings; + if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict || + Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict) { + // will need include-cleaner results, call it once + Findings = computeIncludeCleanerFindings(AST); } + + std::vector Result = generateUnusedIncludeDiagnostics( + AST.tuPath(), Findings.UnusedIncludes, Code); + llvm::move( + generateMissingIncludeDiagnostics(AST, Findings.MissingIncludes, Code), + std::back_inserter(Result)); return Result; } diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h index a7beb9c3c9d4b..d7edca035c965 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -20,74 +20,32 @@ #include "Headers.h" #include "ParsedAST.h" -#include "index/CanonicalIncludes.h" -#include "clang/Basic/SourceLocation.h" -#include "clang/Tooling/Inclusions/StandardLibrary.h" +#include "clang-include-cleaner/Types.h" +#include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/DenseSet.h" -#include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/StringSet.h" -#include +#include #include namespace clang { namespace clangd { -struct ReferencedLocations { - llvm::DenseSet User; - llvm::DenseSet Stdlib; -}; - -/// Finds locations of all symbols used in the main file. -/// -/// - RecursiveASTVisitor finds references to symbols and records their -/// associated locations. These may be macro expansions, and are not resolved -/// to their spelling or expansion location. These locations are later used to -/// determine which headers should be marked as "used" and "directly used". -/// - If \p Tokens is not nullptr, we also examine all identifier tokens in the -/// file in case they reference macros macros. -/// We use this to compute unused headers, so we: -/// -/// - cover the whole file in a single traversal for efficiency -/// - don't attempt to describe where symbols were referenced from in -/// ambiguous cases (e.g. implicitly used symbols, multiple declarations) -/// - err on the side of reporting all possible locations -ReferencedLocations findReferencedLocations(ASTContext &Ctx, Preprocessor &PP, - const syntax::TokenBuffer *Tokens); -ReferencedLocations findReferencedLocations(ParsedAST &AST); +// Data needed for missing include diagnostics. +struct MissingIncludeDiagInfo { + include_cleaner::Symbol Symbol; + syntax::FileRange SymRefRange; + std::vector Providers; -struct ReferencedFiles { - llvm::DenseSet User; - llvm::DenseSet Stdlib; - /// Files responsible for the symbols referenced in the main file and defined - /// in private headers (private headers have IWYU pragma: private, include - /// "public.h"). We store spelling of the public header (with quotes or angle - /// brackets) files here to avoid dealing with full filenames and visibility. - llvm::StringSet<> SpelledUmbrellas; + bool operator==(const MissingIncludeDiagInfo &Other) const { + return std::tie(SymRefRange, Providers, Symbol) == + std::tie(Other.SymRefRange, Other.Providers, Other.Symbol); + } }; -/// Retrieves IDs of all files containing SourceLocations from \p Locs. -/// The output only includes things SourceManager sees as files (not macro IDs). -/// This can include , etc that are not true files. -/// \p HeaderResponsible returns the public header that should be included given -/// symbols from a file with the given FileID (example: public headers should be -/// preferred to non self-contained and private headers). -/// \p UmbrellaHeader returns the public public header is responsible for -/// providing symbols from a file with the given FileID (example: MyType.h -/// should be included instead of MyType_impl.h). -ReferencedFiles findReferencedFiles( - const ReferencedLocations &Locs, const SourceManager &SM, - llvm::function_ref HeaderResponsible, - llvm::function_ref(FileID)> UmbrellaHeader); -ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs, - const IncludeStructure &Includes, - const CanonicalIncludes &CanonIncludes, - const SourceManager &SM); - -/// Maps FileIDs to the internal IncludeStructure representation (HeaderIDs). -/// FileIDs that are not true files ( etc) are dropped. -llvm::DenseSet -translateToHeaderIDs(const ReferencedFiles &Files, - const IncludeStructure &Includes, const SourceManager &SM); +struct IncludeCleanerFindings { + std::vector UnusedIncludes; + std::vector MissingIncludes; +}; /// Retrieves headers that are referenced from the main file but not used. /// In unclear cases, headers are not marked as unused. @@ -96,13 +54,9 @@ getUnused(ParsedAST &AST, const llvm::DenseSet &ReferencedFiles, const llvm::StringSet<> &ReferencedPublicHeaders); -std::vector computeUnusedIncludes(ParsedAST &AST); -// The same as computeUnusedIncludes, but it is an experimental and -// include-cleaner-lib-based implementation. -std::vector -computeUnusedIncludesExperimental(ParsedAST &AST); +IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST); -std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, +std::vector issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code); /// Affects whether standard library includes should be considered for diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 54a0f979c7303..1671eec133b6e 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -605,16 +605,15 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, // Copy over the macros in the preamble region of the main file, and combine // with non-preamble macros below. MainFileMacros Macros; - if (Preamble) - Macros = Preamble->Macros; + std::vector Marks; + if (Preamble) { + Macros = Patch->mainFileMacros(); + Marks = Patch->marks(); + } Clang->getPreprocessor().addPPCallbacks( std::make_unique(Clang->getSourceManager(), Macros)); - std::vector Marks; - // FIXME: We need to patch the marks for stale preambles. - if (Preamble) - Marks = Preamble->Marks; Clang->getPreprocessor().addPPCallbacks( collectPragmaMarksCallback(Clang->getSourceManager(), Marks)); @@ -687,13 +686,9 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, std::move(Macros), std::move(Marks), std::move(ParsedDecls), std::move(Diags), std::move(Includes), std::move(CanonIncludes)); - if (Result.Diags) { - auto UnusedHeadersDiags = - issueUnusedIncludesDiagnostics(Result, Inputs.Contents); - Result.Diags->insert(Result.Diags->end(), - make_move_iterator(UnusedHeadersDiags.begin()), - make_move_iterator(UnusedHeadersDiags.end())); - } + if (Result.Diags) + llvm::move(issueIncludeCleanerDiagnostics(Result, Inputs.Contents), + std::back_inserter(*Result.Diags)); return std::move(Result); } diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index f50597f9ad209..3b0af0ab50a62 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "Preamble.h" +#include "CollectMacros.h" #include "Compiler.h" #include "Config.h" #include "Diagnostics.h" @@ -14,7 +15,9 @@ #include "Protocol.h" #include "SourceCode.h" #include "clang-include-cleaner/Record.h" +#include "index/CanonicalIncludes.h" #include "support/Logger.h" +#include "support/Path.h" #include "support/ThreadsafeFS.h" #include "support/Trace.h" #include "clang/AST/DeclTemplate.h" @@ -26,6 +29,7 @@ #include "clang/Basic/TokenKinds.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" +#include "clang/Frontend/PrecompiledPreamble.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/Lexer.h" #include "clang/Lex/PPCallbacks.h" @@ -41,6 +45,7 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" @@ -49,10 +54,12 @@ #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" #include +#include #include #include #include #include +#include #include #include @@ -128,7 +135,9 @@ class CppFilePreambleCallbacks : public PreambleCallbacks { SourceMgr = &CI.getSourceManager(); Includes.collect(CI); if (Config::current().Diagnostics.UnusedIncludes == - Config::UnusedIncludesPolicy::Experiment) + Config::IncludesPolicy::Strict || + Config::current().Diagnostics.MissingIncludes == + Config::IncludesPolicy::Strict) Pragmas.record(CI); if (BeforeExecuteCallback) BeforeExecuteCallback(CI); @@ -315,6 +324,8 @@ struct ScannedPreamble { // Literal lines of the preamble contents. std::vector Lines; PreambleBounds Bounds = {0, false}; + std::vector Marks; + MainFileMacros Macros; }; /// Scans the preprocessor directives in the preamble section of the file by @@ -363,12 +374,15 @@ scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand &Cmd) { if (!Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0])) return error("failed BeginSourceFile"); Preprocessor &PP = Clang->getPreprocessor(); + const auto &SM = PP.getSourceManager(); IncludeStructure Includes; Includes.collect(*Clang); ScannedPreamble SP; SP.Bounds = Bounds; PP.addPPCallbacks( std::make_unique(PP, SP.TextualDirectives)); + PP.addPPCallbacks(collectPragmaMarksCallback(SM, SP.Marks)); + PP.addPPCallbacks(std::make_unique(SM, SP.Macros)); if (llvm::Error Err = Action.Execute()) return std::move(Err); Action.EndSourceFile(); @@ -641,7 +655,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI, auto BuiltPreamble = PrecompiledPreamble::Build( CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Stats ? TimedFS : StatCacheFS, std::make_shared(), - StoreInMemory, CapturedInfo); + StoreInMemory, /*StoragePath=*/StringRef(), CapturedInfo); PreambleTimer.stopTimer(); // When building the AST for the main file, we do want the function @@ -763,6 +777,7 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName, return PreamblePatch::unmodified(Baseline); PreamblePatch PP; + PP.Baseline = &Baseline; // This shouldn't coincide with any real file name. llvm::SmallString<128> PatchName; llvm::sys::path::append(PatchName, llvm::sys::path::parent_path(FileName), @@ -845,6 +860,8 @@ PreamblePatch PreamblePatch::create(llvm::StringRef FileName, } PP.PatchedDiags = patchDiags(Baseline.Diags, *BaselineScan, *ModifiedScan); + PP.PatchedMarks = std::move(ModifiedScan->Marks); + PP.PatchedMacros = std::move(ModifiedScan->Macros); dlog("Created preamble patch: {0}", Patch.str()); Patch.flush(); return PP; @@ -884,6 +901,7 @@ std::vector PreamblePatch::preambleIncludes() const { PreamblePatch PreamblePatch::unmodified(const PreambleData &Preamble) { PreamblePatch PP; + PP.Baseline = &Preamble; PP.PreambleIncludes = Preamble.Includes.MainFileIncludes; PP.ModifiedBounds = Preamble.Preamble.getBounds(); PP.PatchedDiags = Preamble.Diags; @@ -894,5 +912,16 @@ bool PreamblePatch::preserveDiagnostics() const { return PatchContents.empty() || Config::current().Diagnostics.AllowStalePreamble; } +llvm::ArrayRef PreamblePatch::marks() const { + if (PatchContents.empty()) + return Baseline->Marks; + return PatchedMarks; +} + +const MainFileMacros &PreamblePatch::mainFileMacros() const { + if (PatchContents.empty()) + return Baseline->Macros; + return PatchedMacros; +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index b5a5c107ab48a..a16f497737aa3 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -163,6 +163,9 @@ class PreamblePatch { static constexpr llvm::StringLiteral HeaderName = "__preamble_patch__.h"; + llvm::ArrayRef marks() const; + const MainFileMacros &mainFileMacros() const; + private: static PreamblePatch create(llvm::StringRef FileName, const ParseInputs &Modified, @@ -178,6 +181,9 @@ class PreamblePatch { // Diags that were attached to a line preserved in Modified contents. std::vector PatchedDiags; PreambleBounds ModifiedBounds = {0, false}; + const PreambleData *Baseline = nullptr; + std::vector PatchedMarks; + MainFileMacros PatchedMacros; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp index 19f79f793420d..2122cb86c2834 100644 --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -156,7 +156,7 @@ std::optional kindForDecl(const NamedDecl *D, return HighlightingKind::Concept; if (const auto *UUVD = dyn_cast(D)) { auto Targets = Resolver->resolveUsingValueDecl(UUVD); - if (!Targets.empty()) { + if (!Targets.empty() && Targets[0] != UUVD) { return kindForDecl(Targets[0], Resolver); } return HighlightingKind::Unknown; diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index a4f6a93b616a7..9b366cdd43eb2 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -948,7 +948,8 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags, // rebuild. Newly built preamble cannot emit diagnostics before this call // finishes (ast callbacks are called from astpeer thread), hence we // gurantee eventual consistency. - if (LatestPreamble && Config::current().Diagnostics.AllowStalePreamble) + if (LatestPreamble && WantDiags != WantDiagnostics::No && + Config::current().Diagnostics.AllowStalePreamble) generateDiagnostics(std::move(Invocation), std::move(Inputs), std::move(CompilerInvocationDiags)); diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 24d01043a42d6..3179810b1b185 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -756,7 +756,8 @@ bool SymbolCollector::handleMacroOccurrence(const IdentifierInfo *Name, *PP, *CompletionAllocator, *CompletionTUInfo); std::string Signature; std::string SnippetSuffix; - getSignature(*CCS, &Signature, &SnippetSuffix); + getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, + SymbolCompletion.CursorKind); S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; @@ -933,7 +934,8 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, S.Documentation = Documentation; std::string Signature; std::string SnippetSuffix; - getSignature(*CCS, &Signature, &SnippetSuffix); + getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, + SymbolCompletion.CursorKind); S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; std::string ReturnType = getReturnType(*CCS); diff --git a/clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp b/clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp index e15605191c752..0b86b48422707 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp @@ -8,10 +8,7 @@ #include "ParsedAST.h" #include "refactor/InsertionPoint.h" #include "refactor/Tweak.h" -#include "support/Logger.h" #include "clang/AST/DeclCXX.h" -#include "clang/Basic/SourceLocation.h" -#include "clang/Basic/SourceManager.h" #include "clang/Sema/Sema.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/StringRef.h" @@ -84,7 +81,7 @@ std::string buildSpecialMemberDeclarations(const CXXRecordDecl &Class) { // - to understand the implicit behavior // - to avoid relying on the implicit behavior // - as a baseline for explicit modification -class DeclareCopyMove : public Tweak { +class SpecialMembers : public Tweak { public: const char *id() const final; llvm::StringLiteral kind() const override { @@ -103,7 +100,7 @@ class DeclareCopyMove : public Tweak { // Trigger only on class definitions. if (auto *N = Inputs.ASTSelection.commonAncestor()) Class = const_cast(N->ASTNode.get()); - if (!Class || !Class->isThisDeclarationADefinition()) + if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion()) return false; // Tweak is only available if some members are missing. @@ -146,7 +143,7 @@ class DeclareCopyMove : public Tweak { bool NeedCopy = false, NeedMove = false; CXXRecordDecl *Class = nullptr; }; -REGISTER_TWEAK(DeclareCopyMove) +REGISTER_TWEAK(SpecialMembers) } // namespace } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp index b32875fe19500..e5c9fc5408806 100644 --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -69,7 +69,7 @@ class MemoryShardStorage : public BackgroundIndexStorage { loadShard(llvm::StringRef ShardIdentifier) const override { std::lock_guard Lock(StorageMu); AccessedPaths.insert(ShardIdentifier); - if (Storage.find(ShardIdentifier) == Storage.end()) { + if (!Storage.contains(ShardIdentifier)) { return nullptr; } auto IndexFile = diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index a68d8cb0b78ff..056e5803d711b 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -104,12 +104,13 @@ add_unittest(ClangdUnitTests ClangdTests support/CancellationTests.cpp support/ContextTests.cpp + support/FileCacheTests.cpp support/FunctionTests.cpp support/MarkupTests.cpp support/MemoryTreeTests.cpp support/PathTests.cpp - support/ThreadingTests.cpp support/TestTracer.cpp + support/ThreadingTests.cpp support/TraceTests.cpp tweaks/AddUsingTests.cpp @@ -130,6 +131,7 @@ add_unittest(ClangdUnitTests ClangdTests tweaks/RawStringLiteralTests.cpp tweaks/RemoveUsingNamespaceTests.cpp tweaks/ShowSelectionTreeTests.cpp + tweaks/SpecialMembersTests.cpp tweaks/SwapIfBranchesTests.cpp tweaks/TweakTesting.cpp tweaks/TweakTests.cpp @@ -155,6 +157,7 @@ clang_target_link_libraries(ClangdTests clangLex clangSema clangSerialization + clangTesting clangTooling clangToolingCore clangToolingInclusions diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index f57f266c3c511..25d6a7b189ed5 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -3450,6 +3450,22 @@ TEST(CompletionTest, CursorInSnippets) { EXPECT_THAT(Results.Completions, Contains(AllOf(named("while_foo"), snippetSuffix("(${1:int a}, ${2:int b})")))); + + Results = completions(R"cpp( + struct Base { + Base(int a, int b) {} + }; + + struct Derived : Base { + Derived() : Base^ + }; + )cpp", + /*IndexSymbols=*/{}, Options); + // Constructors from base classes are a kind of pattern that shouldn't end + // with $0. + EXPECT_THAT(Results.Completions, + Contains(AllOf(named("Base"), + snippetSuffix("(${1:int a}, ${2:int b})")))); } TEST(CompletionTest, WorksWithNullType) { diff --git a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp index 329a213c66984..faab6253e2832 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp @@ -24,11 +24,13 @@ class CompletionStringTest : public ::testing::Test { protected: void computeSignature(const CodeCompletionString &CCS, - bool CompletingPattern = false) { + CodeCompletionResult::ResultKind ResultKind = + CodeCompletionResult::ResultKind::RK_Declaration) { Signature.clear(); Snippet.clear(); - getSignature(CCS, &Signature, &Snippet, /*RequiredQualifier=*/nullptr, - CompletingPattern); + getSignature(CCS, &Signature, &Snippet, ResultKind, + /*CursorKind=*/CXCursorKind::CXCursor_NotImplemented, + /*RequiredQualifiers=*/nullptr); } std::shared_ptr Allocator; @@ -145,11 +147,12 @@ TEST_F(CompletionStringTest, SnippetsInPatterns) { Builder.AddChunk(CodeCompletionString::CK_SemiColon); return *Builder.TakeString(); }; - computeSignature(MakeCCS(), /*CompletingPattern=*/false); + computeSignature(MakeCCS()); EXPECT_EQ(Snippet, " ${1:name} = ${2:target};"); // When completing a pattern, the last placeholder holds the cursor position. - computeSignature(MakeCCS(), /*CompletingPattern=*/true); + computeSignature(MakeCCS(), + /*ResultKind=*/CodeCompletionResult::ResultKind::RK_Pattern); EXPECT_EQ(Snippet, " ${1:name} = $0;"); } diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp index be44ce59f6657..28f0d85d332ca 100644 --- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp @@ -11,6 +11,7 @@ #include "TestFS.h" #include "support/Context.h" +#include "clang/Testing/CommandLineArgs.h" #include "clang/Tooling/ArgumentsAdjusters.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" @@ -20,6 +21,7 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/Process.h" +#include "llvm/Support/TargetSelect.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -41,15 +43,18 @@ using ::testing::Not; // Make use of all features and assert the exact command we get out. // Other tests just verify presence/absence of certain args. TEST(CommandMangler, Everything) { + llvm::InitializeAllTargetInfos(); // As in ClangdMain + std::string Target = getAnyTargetForTesting(); auto Mangler = CommandMangler::forTests(); Mangler.ClangPath = testPath("fake/clang"); Mangler.ResourceDir = testPath("fake/resources"); Mangler.Sysroot = testPath("fake/sysroot"); tooling::CompileCommand Cmd; - Cmd.CommandLine = {"clang++", "--", "foo.cc", "bar.cc"}; + Cmd.CommandLine = {Target + "-clang++", "--", "foo.cc", "bar.cc"}; Mangler(Cmd, "foo.cc"); EXPECT_THAT(Cmd.CommandLine, - ElementsAre(testPath("fake/clang++"), + ElementsAre(testPath("fake/" + Target + "-clang++"), + "--target=" + Target, "--driver-mode=g++", "-resource-dir=" + testPath("fake/resources"), "-isysroot", testPath("fake/sysroot"), "--", "foo.cc")); @@ -197,8 +202,26 @@ TEST(CommandMangler, ConfigEdits) { Mangler(Cmd, "foo.cc"); } // Edits are applied in given order and before other mangling and they always - // go before filename. - EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "--hello", "--", "FOO.CC")); + // go before filename. `--driver-mode=g++` here is in lower case because + // options inserted by addTargetAndModeForProgramName are not editable, + // see discussion in https://reviews.llvm.org/D138546 + EXPECT_THAT(Cmd.CommandLine, + ElementsAre(_, "--driver-mode=g++", "--hello", "--", "FOO.CC")); +} + +TEST(CommandMangler, ExpandedResponseFiles) { + SmallString<1024> Path; + int FD; + ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("args", "", FD, Path)); + llvm::raw_fd_ostream OutStream(FD, true); + OutStream << "-Wall"; + OutStream.close(); + + auto Mangler = CommandMangler::forTests(); + tooling::CompileCommand Cmd; + Cmd.CommandLine = {"clang", ("@" + Path).str(), "foo.cc"}; + Mangler(Cmd, "foo.cc"); + EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "-Wall", "--", "foo.cc")); } static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) { diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index 014db131c6224..45dacba5822b7 100644 --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -249,19 +249,19 @@ TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) { // Defaults to None. EXPECT_TRUE(compileAndApply()); EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::UnusedIncludesPolicy::None); + Config::IncludesPolicy::None); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("None"); EXPECT_TRUE(compileAndApply()); EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::UnusedIncludesPolicy::None); + Config::IncludesPolicy::None); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("Strict"); EXPECT_TRUE(compileAndApply()); EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::UnusedIncludesPolicy::Strict); + Config::IncludesPolicy::Strict); Frag = {}; EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty()) diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index a20be39722ec3..289767b0dec0f 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -1900,7 +1900,7 @@ TEST(DiagnosticsTest, IncludeCleaner) { // Off by default. EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty()); Config Cfg; - Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict; + Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict; // Set filtering. Cfg.Diagnostics.Includes.IgnoreHeader.emplace_back( [](llvm::StringRef Header) { return Header.endswith("ignore.h"); }); diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp index bc4a5abd0e46a..8c0eae6bb3cc8 100644 --- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp @@ -147,7 +147,6 @@ MATCHER_P(written, Name, "") { return arg.Written == Name; } MATCHER_P(resolved, Name, "") { return arg.Resolved == Name; } MATCHER_P(includeLine, N, "") { return arg.HashLine == N; } MATCHER_P(directive, D, "") { return arg.Directive == D; } -MATCHER_P(hasPragmaKeep, H, "") { return arg.BehindPragmaKeep == H; } MATCHER_P2(Distance, File, D, "") { if (arg.getFirst() != File) @@ -293,18 +292,6 @@ TEST_F(HeadersTest, IncludeDirective) { directive(tok::pp_include_next))); } -TEST_F(HeadersTest, IWYUPragmaKeep) { - FS.Files[MainFile] = R"cpp( -#include "bar.h" // IWYU pragma: keep -#include "foo.h" -)cpp"; - - EXPECT_THAT( - collectIncludes().MainFileIncludes, - UnorderedElementsAre(AllOf(written("\"foo.h\""), hasPragmaKeep(false)), - AllOf(written("\"bar.h\""), hasPragmaKeep(true)))); -} - TEST_F(HeadersTest, InsertInclude) { std::string Path = testPath("sub/bar.h"); FS.Files[Path] = ""; @@ -415,75 +402,6 @@ TEST_F(HeadersTest, PresumedLocations) { Contains(AllOf(includeLine(2), written("")))); } -TEST_F(HeadersTest, SelfContainedHeaders) { - // Including through non-builtin file has no effects. - FS.Files[MainFile] = R"cpp( -#include "includeguarded.h" -#include "nonguarded.h" -#include "pp_depend.h" -#include "pragmaguarded.h" -#include "recursive.h" -)cpp"; - FS.Files["pragmaguarded.h"] = R"cpp( -#pragma once -)cpp"; - FS.Files["includeguarded.h"] = R"cpp( -#ifndef INCLUDE_GUARDED_H -#define INCLUDE_GUARDED_H -void foo(); -#endif // INCLUDE_GUARDED_H -)cpp"; - FS.Files["nonguarded.h"] = R"cpp( -)cpp"; - FS.Files["pp_depend.h"] = R"cpp( - #ifndef REQUIRED_PP_DIRECTIVE - # error You have to have PP directive set to include this one! - #endif -)cpp"; - FS.Files["recursive.h"] = R"cpp( - #ifndef RECURSIVE_H - #define RECURSIVE_H - - #include "recursive.h" - - #endif // RECURSIVE_H -)cpp"; - - auto Includes = collectIncludes(); - EXPECT_TRUE(Includes.isSelfContained(getID("pragmaguarded.h", Includes))); - EXPECT_TRUE(Includes.isSelfContained(getID("includeguarded.h", Includes))); - EXPECT_TRUE(Includes.isSelfContained(getID("recursive.h", Includes))); - EXPECT_FALSE(Includes.isSelfContained(getID("nonguarded.h", Includes))); - EXPECT_FALSE(Includes.isSelfContained(getID("pp_depend.h", Includes))); -} - -TEST_F(HeadersTest, HasIWYUPragmas) { - FS.Files[MainFile] = R"cpp( -#include "export.h" -#include "begin_exports.h" -#include "none.h" -)cpp"; - FS.Files["export.h"] = R"cpp( -#pragma once -#include "none.h" // IWYU pragma: export -)cpp"; - FS.Files["begin_exports.h"] = R"cpp( -#pragma once -// IWYU pragma: begin_exports -#include "none.h" -// IWYU pragma: end_exports -)cpp"; - FS.Files["none.h"] = R"cpp( -#pragma once -// Not a pragma. -)cpp"; - - auto Includes = collectIncludes(); - EXPECT_TRUE(Includes.hasIWYUExport(getID("export.h", Includes))); - EXPECT_TRUE(Includes.hasIWYUExport(getID("begin_exports.h", Includes))); - EXPECT_FALSE(Includes.hasIWYUExport(getID("none.h", Includes))); -} - } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index 01b8e6a05dab5..409e3cee791c3 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -8,308 +8,59 @@ #include "Annotations.h" #include "Config.h" +#include "Diagnostics.h" #include "IncludeCleaner.h" +#include "ParsedAST.h" #include "SourceCode.h" #include "TestFS.h" #include "TestTU.h" +#include "clang-include-cleaner/Analysis.h" +#include "clang-include-cleaner/Types.h" #include "support/Context.h" +#include "clang/AST/DeclBase.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/ScopedPrinter.h" #include "llvm/Testing/Support/SupportHelpers.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include +#include +#include namespace clang { namespace clangd { namespace { +using ::testing::AllOf; using ::testing::ElementsAre; -using ::testing::ElementsAreArray; using ::testing::IsEmpty; +using ::testing::Matcher; using ::testing::Pointee; using ::testing::UnorderedElementsAre; -std::string guard(llvm::StringRef Code) { - return "#pragma once\n" + Code.str(); +Matcher withFix(::testing::Matcher FixMatcher) { + return Field(&Diag::Fixes, ElementsAre(FixMatcher)); } -TEST(IncludeCleaner, ReferencedLocations) { - struct TestCase { - std::string HeaderCode; - std::string MainCode; - }; - TestCase Cases[] = { - // DeclRefExpr - { - "int ^x();", - "int y = x();", - }, - // RecordDecl - { - "class ^X;", - "X *y;", - }, - // When definition is available, we don't need to mark forward - // declarations as used. - { - "class ^X {}; class X;", - "X *y;", - }, - // We already have forward declaration in the main file, the other - // non-definition declarations are not needed. - { - "class ^X {}; class X;", - "class X; X *y;", - }, - // Nested class definition can occur outside of the parent class - // definition. Bar declaration should be visible to its definition but - // it will always be because we will mark Foo definition as used. - { - "class ^Foo { class Bar; };", - "class Foo::Bar {};", - }, - // TypedefType and UsingDecls - { - "using ^Integer = int;", - "Integer x;", - }, - { - "namespace ns { void ^foo(); void ^foo() {} }", - "using ns::foo;", - }, - { - "namespace ns { void foo(); void foo() {}; }", - "using namespace ns;", - }, - { - // Refs from UsingTypeLoc and implicit constructor! - "struct ^A {}; using B = A; using ^C = B;", - "C a;", - }, - {"namespace ns { template class A {}; } using ns::^A;", - "A* a;"}, - {"namespace ns { template class A {}; } using ns::^A;", - R"cpp( - template