diff --git a/.ci/compute_projects.py b/.ci/compute_projects.py index 80e4e0221b3d7..c3cf714ce6c10 100644 --- a/.ci/compute_projects.py +++ b/.ci/compute_projects.py @@ -77,6 +77,7 @@ DEPENDENT_RUNTIMES_TO_TEST = { "clang": {"compiler-rt"}, "clang-tools-extra": {"libc"}, + "libc": {"libc"}, ".ci": {"compiler-rt", "libc"}, } DEPENDENT_RUNTIMES_TO_TEST_NEEDS_RECONFIG = { diff --git a/.ci/compute_projects_test.py b/.ci/compute_projects_test.py index 5efac26366981..6299931e1ec34 100644 --- a/.ci/compute_projects_test.py +++ b/.ci/compute_projects_test.py @@ -187,7 +187,7 @@ def test_top_level_file(self): self.assertEqual(env_variables["runtimes_check_targets"], "") self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "") - def test_exclude_runtiems_in_projects(self): + def test_exclude_libcxx_in_projects(self): env_variables = compute_projects.get_env_variables( ["libcxx/CMakeLists.txt"], "Linux" ) @@ -197,6 +197,16 @@ def test_exclude_runtiems_in_projects(self): self.assertEqual(env_variables["runtimes_check_targets"], "") self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "") + def test_include_libc_in_runtimes(self): + env_variables = compute_projects.get_env_variables( + ["libc/CMakeLists.txt"], "Linux" + ) + self.assertEqual(env_variables["projects_to_build"], "clang;lld") + self.assertEqual(env_variables["project_check_targets"], "") + self.assertEqual(env_variables["runtimes_to_build"], "libc") + self.assertEqual(env_variables["runtimes_check_targets"], "check-libc") + self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "") + def test_exclude_docs(self): env_variables = compute_projects.get_env_variables( ["llvm/docs/CIBestPractices.rst"], "Linux" diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh index 89447963b8528..8d1faab13986c 100755 --- a/.ci/monolithic-linux.sh +++ b/.ci/monolithic-linux.sh @@ -56,7 +56,7 @@ runtime_targets_needs_reconfig="${5}" lit_args="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --use-unique-output-file-name --timeout=1200 --time-tests" -echo "--- cmake" +echo "::group::cmake" export PIP_BREAK_SYSTEM_PACKAGES=1 pip install -q -r "${MONOREPO_ROOT}"/.ci/all_requirements.txt @@ -85,38 +85,49 @@ cmake -S "${MONOREPO_ROOT}"/llvm -B "${BUILD_DIR}" \ -D LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS=ON \ -D CMAKE_INSTALL_PREFIX="${INSTALL_DIR}" -echo "--- ninja" +echo "::endgroup::" +echo "::group::ninja" + # Targets are not escaped as they are passed as separate arguments. ninja -C "${BUILD_DIR}" -k 0 ${targets} +echo "::endgroup::" + if [[ "${runtime_targets}" != "" ]]; then - echo "--- ninja runtimes" + echo "::group::ninja runtimes" ninja -C "${BUILD_DIR}" ${runtime_targets} + + echo "::endgroup::" fi # Compiling runtimes with just-built Clang and running their tests # as an additional testing for Clang. if [[ "${runtime_targets_needs_reconfig}" != "" ]]; then - echo "--- cmake runtimes C++26" + echo "::group::cmake runtimes C++26" cmake \ -D LIBCXX_TEST_PARAMS="std=c++26" \ -D LIBCXXABI_TEST_PARAMS="std=c++26" \ "${BUILD_DIR}" - echo "--- ninja runtimes C++26" + echo "::endgroup::" + echo "::group::ninja runtimes C++26" ninja -C "${BUILD_DIR}" ${runtime_targets_needs_reconfig} - echo "--- cmake runtimes clang modules" + echo "::endgroup::" + echo "::group::cmake runtimes clang modules" cmake \ -D LIBCXX_TEST_PARAMS="enable_modules=clang" \ -D LIBCXXABI_TEST_PARAMS="enable_modules=clang" \ "${BUILD_DIR}" - echo "--- ninja runtimes clang modules" + echo "::endgroup::" + echo "::group::ninja runtimes clang modules" ninja -C "${BUILD_DIR}" ${runtime_targets_needs_reconfig} + + echo "::endgroup::" fi diff --git a/.ci/monolithic-windows.sh b/.ci/monolithic-windows.sh index dc2913830e929..c27111bf5aa10 100755 --- a/.ci/monolithic-windows.sh +++ b/.ci/monolithic-windows.sh @@ -46,7 +46,7 @@ trap at-exit EXIT projects="${1}" targets="${2}" -echo "--- cmake" +echo "::group::cmake" pip install -q -r "${MONOREPO_ROOT}"/.ci/all_requirements.txt export CC=cl @@ -78,6 +78,10 @@ cmake -S "${MONOREPO_ROOT}"/llvm -B "${BUILD_DIR}" \ -D LLVM_PARALLEL_COMPILE_JOBS=${MAX_PARALLEL_COMPILE_JOBS} \ -D LLVM_PARALLEL_LINK_JOBS=${MAX_PARALLEL_LINK_JOBS} -echo "--- ninja" +echo "::endgroup::" +echo "::group::ninja" + # Targets are not escaped as they are passed as separate arguments. ninja -C "${BUILD_DIR}" -k 0 ${targets} + +echo "::endgroup" diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000000..03748938700e3 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,4 @@ +When performing a code review, pay close attention to code modifying a function's +control flow. Could the change result in the corruption of performance profile +data? Could the change result in invalid debug information, in particular for +branches and calls? diff --git a/.github/new-prs-labeler.yml b/.github/new-prs-labeler.yml index 2f8d5745668d9..863090af9af7e 100644 --- a/.github/new-prs-labeler.yml +++ b/.github/new-prs-labeler.yml @@ -632,6 +632,10 @@ llvm:instcombine: - llvm/test/Transforms/InstCombine/** - llvm/test/Transforms/InstSimplify/** +llvm:vectorcombine: + - llvm/lib/Transforms/Vectorize/VectorCombine.cpp + - llvm/test/Transforms/VectorCombine/** + clangd: - clang-tools-extra/clangd/** diff --git a/.github/workflows/hlsl-test-all.yaml b/.github/workflows/hlsl-test-all.yaml index c2b00ecdf5c40..150bf4eebe54a 100644 --- a/.github/workflows/hlsl-test-all.yaml +++ b/.github/workflows/hlsl-test-all.yaml @@ -43,13 +43,13 @@ jobs: - name: Checkout OffloadTest uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 with: - repository: llvm-beanz/offload-test-suite + repository: llvm/offload-test-suite ref: main path: OffloadTest - name: Checkout Golden Images uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 with: - repository: llvm-beanz/offload-golden-images + repository: llvm/offload-golden-images ref: main path: golden-images - name: Setup Windows diff --git a/.github/workflows/libcxx-restart-preempted-jobs.yaml b/.github/workflows/libcxx-restart-preempted-jobs.yaml index 9706f0459922e..accb84efb5c90 100644 --- a/.github/workflows/libcxx-restart-preempted-jobs.yaml +++ b/.github/workflows/libcxx-restart-preempted-jobs.yaml @@ -20,7 +20,7 @@ permissions: jobs: restart: - if: github.repository_owner == 'llvm' && (github.event.workflow_run.conclusion == 'failure' || github.event.workflow_run.conclusion == 'cancelled') + if: github.repository_owner == 'llvm' && (github.event.workflow_run.conclusion == 'failure') name: "Restart Job" permissions: statuses: read @@ -32,7 +32,10 @@ jobs: uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea #v7.0.1 with: script: | - const failure_regex = /Process completed with exit code 1./ + // The "The run was canceled by" message comes from a user manually canceling a workflow + // the "higher priority" message comes from github canceling a workflow because the user updated the change. + // And the "exit code 1" message indicates a genuine failure. + const failure_regex = /(Process completed with exit code 1.)/ const preemption_regex = /(The runner has received a shutdown signal)|(The operation was canceled)/ const wf_run = context.payload.workflow_run @@ -74,7 +77,7 @@ jobs: console.log('Check run was not completed. Skipping.'); continue; } - if (check_run.conclusion != 'failure' && check_run.conclusion != 'cancelled') { + if (check_run.conclusion != 'failure') { console.log('Check run had conclusion: ' + check_run.conclusion + '. Skipping.'); continue; } @@ -153,91 +156,3 @@ jobs: run_id: context.payload.workflow_run.id }) await create_check_run('success', 'Restarted workflow run due to preempted job') - - restart-test: - if: github.repository_owner == 'llvm' && (github.event.workflow_run.conclusion == 'failure' || github.event.workflow_run.conclusion == 'cancelled') && github.event.actor.login == 'ldionne' # TESTING ONLY - name: "Restart Job (test)" - permissions: - statuses: read - checks: write - actions: write - runs-on: ubuntu-24.04 - steps: - - name: "Restart Job (test)" - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea #v7.0.1 - with: - script: | - const FAILURE_REGEX = /Process completed with exit code 1./ - const PREEMPTION_REGEX = /(The runner has received a shutdown signal)|(The operation was canceled)/ - - function log(msg) { - core.notice(msg) - } - - const wf_run = context.payload.workflow_run - log(`Running on "${wf_run.display_title}" by @${wf_run.actor.login} (event: ${wf_run.event})\nWorkflow run URL: ${wf_run.html_url}`) - - log('Listing check runs for suite') - const check_suites = await github.rest.checks.listForSuite({ - owner: context.repo.owner, - repo: context.repo.repo, - check_suite_id: context.payload.workflow_run.check_suite_id, - per_page: 100 // FIXME: We don't have 100 check runs yet, but we should handle this better. - }) - - preemptions = []; - legitimate_failures = []; - for (check_run of check_suites.data.check_runs) { - log(`Checking check run: ${check_run.id}`); - if (check_run.status != 'completed') { - log('Check run was not completed. Skipping.'); - continue; - } - - if (check_run.conclusion != 'failure' && check_run.conclusion != 'cancelled') { - log(`Check run had conclusion: ${check_run.conclusion}. Skipping.`); - continue; - } - - annotations = await github.rest.checks.listAnnotations({ - owner: context.repo.owner, - repo: context.repo.repo, - check_run_id: check_run.id - }) - - preemption_annotation = annotations.data.find(function(annotation) { - return annotation.annotation_level == 'failure' && - annotation.message.match(PREEMPTION_REGEX) != null; - }); - if (preemption_annotation != null) { - log(`Found preemption message: ${preemption_annotation.message}`); - preemptions.push(check_run); - break; - } - - failure_annotation = annotations.data.find(function(annotation) { - return annotation.annotation_level == 'failure' && - annotation.message.match(FAILURE_REGEX) != null; - }); - if (failure_annotation != null) { - log(`Found legitimate failure annotation: ${failure_annotation.message}`); - legitimate_failures.push(check_run); - break; - } - } - - if (preemptions) { - log('Found some preempted jobs'); - if (legitimate_failures) { - log('Also found some legitimate failures, so not restarting the workflow.'); - } else { - log('Did not find any legitimate failures. Restarting workflow.'); - await github.rest.actions.reRunWorkflowFailedJobs({ - owner: context.repo.owner, - repo: context.repo.repo, - run_id: context.payload.workflow_run.id - }) - } - } else { - log('Did not find any preempted jobs. Not restarting the workflow.'); - } diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h index 64e524e2f13d7..91ecf89da618c 100644 --- a/bolt/include/bolt/Core/BinaryContext.h +++ b/bolt/include/bolt/Core/BinaryContext.h @@ -73,14 +73,15 @@ struct SegmentInfo { uint64_t FileSize; /// Size in file. uint64_t Alignment; /// Alignment of the segment. bool IsExecutable; /// Is the executable bit set on the Segment? + bool IsWritable; /// Is the segment writable. void print(raw_ostream &OS) const { OS << "SegmentInfo { Address: 0x" << Twine::utohexstr(Address) << ", Size: 0x" << Twine::utohexstr(Size) << ", FileOffset: 0x" << Twine::utohexstr(FileOffset) << ", FileSize: 0x" << Twine::utohexstr(FileSize) << ", Alignment: 0x" - << Twine::utohexstr(Alignment) << ", " << (IsExecutable ? "x" : " ") - << "}"; + << Twine::utohexstr(Alignment) << ", " << (IsExecutable ? "x" : "") + << (IsWritable ? "w" : "") << " }"; }; }; @@ -333,9 +334,14 @@ class BinaryContext { std::optional Source, unsigned CUID, unsigned DWARFVersion); + /// Input file segment info + /// /// [start memory address] -> [segment info] mapping. std::map SegmentMapInfo; + /// Newly created segments. + std::vector NewSegments; + /// Symbols that are expected to be undefined in MCContext during emission. std::unordered_set UndefinedSymbols; diff --git a/bolt/include/bolt/Core/BinarySection.h b/bolt/include/bolt/Core/BinarySection.h index ad2fed2cf27eb..154a8d12de5ce 100644 --- a/bolt/include/bolt/Core/BinarySection.h +++ b/bolt/include/bolt/Core/BinarySection.h @@ -523,11 +523,6 @@ inline uint8_t *copyByteArray(const uint8_t *Data, uint64_t Size) { return Array; } -inline uint8_t *copyByteArray(StringRef Buffer) { - return copyByteArray(reinterpret_cast(Buffer.data()), - Buffer.size()); -} - inline uint8_t *copyByteArray(ArrayRef Buffer) { return copyByteArray(reinterpret_cast(Buffer.data()), Buffer.size()); diff --git a/bolt/include/bolt/Core/DIEBuilder.h b/bolt/include/bolt/Core/DIEBuilder.h index 32e455ad3030a..e4a4fc6b2f258 100644 --- a/bolt/include/bolt/Core/DIEBuilder.h +++ b/bolt/include/bolt/Core/DIEBuilder.h @@ -20,8 +20,8 @@ #include "llvm/CodeGen/DIE.h" #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h" #include "llvm/DebugInfo/DWARF/DWARFDie.h" -#include "llvm/DebugInfo/DWARF/DWARFExpression.h" #include "llvm/DebugInfo/DWARF/DWARFUnit.h" +#include "llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h" #include "llvm/Support/Allocator.h" #include diff --git a/bolt/include/bolt/Core/MCPlusBuilder.h b/bolt/include/bolt/Core/MCPlusBuilder.h index 804100db80793..b99cf8b40ef54 100644 --- a/bolt/include/bolt/Core/MCPlusBuilder.h +++ b/bolt/include/bolt/Core/MCPlusBuilder.h @@ -1393,7 +1393,7 @@ class MCPlusBuilder { return getTargetSymbol(BinaryExpr->getLHS()); auto *SymbolRefExpr = dyn_cast(Expr); - if (SymbolRefExpr && SymbolRefExpr->getKind() == MCSymbolRefExpr::VK_None) + if (SymbolRefExpr && SymbolRefExpr->getSpecifier() == 0) return &SymbolRefExpr->getSymbol(); return nullptr; diff --git a/bolt/include/bolt/Profile/DataAggregator.h b/bolt/include/bolt/Profile/DataAggregator.h index cc28a06c151e5..98e4bba872846 100644 --- a/bolt/include/bolt/Profile/DataAggregator.h +++ b/bolt/include/bolt/Profile/DataAggregator.h @@ -374,6 +374,9 @@ class DataAggregator : public DataReader { /// Parse a single pair of binary full path and associated build-id std::optional> parseNameBuildIDPair(); + /// Coordinate reading and parsing of perf.data file + void parsePerfData(BinaryContext &BC); + /// Coordinate reading and parsing of pre-aggregated file /// /// The regular perf2bolt aggregation job is to read perf output directly. diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h index 94dd06e91d6b1..91d62a78de390 100644 --- a/bolt/include/bolt/Rewrite/RewriteInstance.h +++ b/bolt/include/bolt/Rewrite/RewriteInstance.h @@ -202,6 +202,9 @@ class RewriteInstance { /// Map code sections generated by BOLT. void mapCodeSections(BOLTLinker::SectionMapper MapSection); + /// Map code without relocating sections. + void mapCodeSectionsInPlace(BOLTLinker::SectionMapper MapSection); + /// Map the rest of allocatable sections. void mapAllocatableSections(BOLTLinker::SectionMapper MapSection); @@ -297,6 +300,9 @@ class RewriteInstance { return FUNC(ELF64BE); \ } + /// Update loadable segment information based on new sections. + void updateSegmentInfo(); + /// Patch ELF book-keeping info. void patchELFPHDRTable(); diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp index b998d7160aae7..eec68ff5a5fce 100644 --- a/bolt/lib/Core/BinaryFunction.cpp +++ b/bolt/lib/Core/BinaryFunction.cpp @@ -3328,10 +3328,7 @@ void BinaryFunction::duplicateConstantIslands() { // Update instruction reference Operand = MCOperand::createExpr(BC.MIB->getTargetExprFor( - Inst, - MCSymbolRefExpr::create(ColdSymbol, MCSymbolRefExpr::VK_None, - *BC.Ctx), - *BC.Ctx, 0)); + Inst, MCSymbolRefExpr::create(ColdSymbol, *BC.Ctx), *BC.Ctx, 0)); ++OpNum; } } diff --git a/bolt/lib/Core/CMakeLists.txt b/bolt/lib/Core/CMakeLists.txt index 8c1f5d0bb37b5..fc72dc023c590 100644 --- a/bolt/lib/Core/CMakeLists.txt +++ b/bolt/lib/Core/CMakeLists.txt @@ -1,5 +1,6 @@ set(LLVM_LINK_COMPONENTS DebugInfoDWARF + DebugInfoDWARFLowLevel Demangle MC MCDisassembler diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp index d36dbb3459249..b041dc5ea1cce 100644 --- a/bolt/lib/Core/DIEBuilder.cpp +++ b/bolt/lib/Core/DIEBuilder.cpp @@ -14,11 +14,11 @@ #include "llvm/CodeGen/DIE.h" #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h" #include "llvm/DebugInfo/DWARF/DWARFDie.h" -#include "llvm/DebugInfo/DWARF/DWARFExpression.h" #include "llvm/DebugInfo/DWARF/DWARFFormValue.h" #include "llvm/DebugInfo/DWARF/DWARFTypeUnit.h" #include "llvm/DebugInfo/DWARF/DWARFUnit.h" #include "llvm/DebugInfo/DWARF/DWARFUnitIndex.h" +#include "llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Debug.h" #include "llvm/Support/ErrorHandling.h" diff --git a/bolt/lib/Core/DebugNames.cpp b/bolt/lib/Core/DebugNames.cpp index aa1c8f3d42d4b..a9d98a6ba879b 100644 --- a/bolt/lib/Core/DebugNames.cpp +++ b/bolt/lib/Core/DebugNames.cpp @@ -8,8 +8,8 @@ #include "bolt/Core/DebugNames.h" #include "bolt/Core/BinaryContext.h" -#include "llvm/DebugInfo/DWARF/DWARFExpression.h" #include "llvm/DebugInfo/DWARF/DWARFTypeUnit.h" +#include "llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h" #include "llvm/Support/EndianStream.h" #include "llvm/Support/LEB128.h" #include diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp index e356481bbdc7c..5d44e1a1a4902 100644 --- a/bolt/lib/Passes/BinaryPasses.cpp +++ b/bolt/lib/Passes/BinaryPasses.cpp @@ -1765,27 +1765,26 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) { if (opts::ShowDensity) { double Density = 0.0; - // Sorted by the density in descending order. - llvm::stable_sort(FuncDensityList, - [&](const std::pair &A, - const std::pair &B) { - if (A.first != B.first) - return A.first > B.first; - return A.second < B.second; - }); + llvm::sort(FuncDensityList); uint64_t AccumulatedSamples = 0; - uint32_t I = 0; assert(opts::ProfileDensityCutOffHot <= 1000000 && "The cutoff value is greater than 1000000(100%)"); - while (AccumulatedSamples < - TotalSampleCount * - static_cast(opts::ProfileDensityCutOffHot) / - 1000000 && - I < FuncDensityList.size()) { - AccumulatedSamples += FuncDensityList[I].second; - Density = FuncDensityList[I].first; - I++; + // Subtract samples in zero-density functions (no fall-throughs) from + // TotalSampleCount (not used anywhere below). + for (const auto [CurDensity, CurSamples] : FuncDensityList) { + if (CurDensity != 0.0) + break; + TotalSampleCount -= CurSamples; + } + const uint64_t CutoffSampleCount = + 1.f * TotalSampleCount * opts::ProfileDensityCutOffHot / 1000000; + // Process functions in decreasing density order + for (const auto [CurDensity, CurSamples] : llvm::reverse(FuncDensityList)) { + if (AccumulatedSamples >= CutoffSampleCount) + break; + AccumulatedSamples += CurSamples; + Density = CurDensity; } if (Density == 0.0) { BC.errs() << "BOLT-WARNING: the output profile is empty or the " diff --git a/bolt/lib/Passes/Instrumentation.cpp b/bolt/lib/Passes/Instrumentation.cpp index fbf889279f1c0..c2f876f0dff9e 100644 --- a/bolt/lib/Passes/Instrumentation.cpp +++ b/bolt/lib/Passes/Instrumentation.cpp @@ -666,8 +666,7 @@ Error Instrumentation::runOnFunctions(BinaryContext &BC) { auto IsLEA = [&BC](const MCInst &Inst) { return BC.MIB->isLEA64r(Inst); }; const auto LEA = std::find_if( std::next(llvm::find_if(reverse(BB), IsLEA)), BB.rend(), IsLEA); - LEA->getOperand(4).setExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *BC.Ctx)); + LEA->getOperand(4).setExpr(MCSymbolRefExpr::create(Target, *BC.Ctx)); } else { BC.errs() << "BOLT-WARNING: ___GLOBAL_init_65535 not found\n"; } diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp index 95e831fe9c8ca..f928dd49edb25 100644 --- a/bolt/lib/Passes/PAuthGadgetScanner.cpp +++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp @@ -82,6 +82,22 @@ namespace PAuthGadgetScanner { dbgs() << "\n"; } +// Iterates over BinaryFunction's instructions like a range-based for loop: +// +// iterateOverInstrs(BF, [&](MCInstReference Inst) { +// // loop body +// }); +template static void iterateOverInstrs(BinaryFunction &BF, T Fn) { + if (BF.hasCFG()) { + for (BinaryBasicBlock &BB : BF) + for (int64_t I = 0, E = BB.size(); I < E; ++I) + Fn(MCInstInBBReference(&BB, I)); + } else { + for (auto I : BF.instrs()) + Fn(MCInstInBFReference(&BF, I.first)); + } +} + // This class represents mapping from a set of arbitrary physical registers to // consecutive array indexes. class TrackedRegisters { @@ -342,6 +358,29 @@ class SrcSafetyAnalysis { return S; } + /// Computes a reasonably pessimistic estimation of the register state when + /// the previous instruction is not known for sure. Takes the set of registers + /// which are trusted at function entry and removes all registers that can be + /// clobbered inside this function. + SrcState computePessimisticState(BinaryFunction &BF) { + BitVector ClobberedRegs(NumRegs); + iterateOverInstrs(BF, [&](MCInstReference Inst) { + BC.MIB->getClobberedRegs(Inst, ClobberedRegs); + + // If this is a call instruction, no register is safe anymore, unless + // it is a tail call. Ignore tail calls for the purpose of estimating the + // worst-case scenario, assuming no instructions are executed in the + // caller after this point anyway. + if (BC.MIB->isCall(Inst) && !BC.MIB->isTailCall(Inst)) + ClobberedRegs.set(); + }); + + SrcState S = createEntryState(); + S.SafeToDerefRegs.reset(ClobberedRegs); + S.TrustedRegs.reset(ClobberedRegs); + return S; + } + BitVector getClobberedRegs(const MCInst &Point) const { BitVector Clobbered(NumRegs); // Assume a call can clobber all registers, including callee-saved @@ -545,6 +584,10 @@ class DataflowSrcSafetyAnalysis using SrcSafetyAnalysis::BC; using SrcSafetyAnalysis::computeNext; + // Pessimistic initial state for basic blocks without any predecessors + // (not needed for most functions, thus initialized lazily). + SrcState PessimisticState; + public: DataflowSrcSafetyAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId, @@ -585,6 +628,18 @@ class DataflowSrcSafetyAnalysis if (BB.isEntryPoint()) return createEntryState(); + // If a basic block without any predecessors is found in an optimized code, + // this likely means that some CFG edges were not detected. Pessimistically + // assume any register that can ever be clobbered in this function to be + // unsafe before this basic block. + // Warn about this fact in FunctionAnalysis::findUnsafeUses(), as it likely + // means imprecise CFG information. + if (BB.pred_empty()) { + if (PessimisticState.empty()) + PessimisticState = computePessimisticState(*BB.getParent()); + return PessimisticState; + } + return SrcState(); } @@ -682,19 +737,14 @@ template class CFGUnawareAnalysis { // // Then, a function can be split into a number of disjoint contiguous sequences // of instructions without labels in between. These sequences can be processed -// the same way basic blocks are processed by data-flow analysis, assuming -// pessimistically that all registers are unsafe at the start of each sequence. +// the same way basic blocks are processed by data-flow analysis, with the same +// pessimistic estimation of the initial state at the start of each sequence +// (except the first instruction of the function). class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis, public CFGUnawareAnalysis { using SrcSafetyAnalysis::BC; BinaryFunction &BF; - /// Creates a state with all registers marked unsafe (not to be confused - /// with empty state). - SrcState createUnsafeState() const { - return SrcState(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters()); - } - public: CFGUnawareSrcSafetyAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId, @@ -704,6 +754,7 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis, } void run() override { + const SrcState DefaultState = computePessimisticState(BF); SrcState S = createEntryState(); for (auto &I : BF.instrs()) { MCInst &Inst = I.second; @@ -718,7 +769,7 @@ class CFGUnawareSrcSafetyAnalysis : public SrcSafetyAnalysis, LLVM_DEBUG({ traceInst(BC, "Due to label, resetting the state before", Inst); }); - S = createUnsafeState(); + S = DefaultState; } // Attach the state *before* this instruction executes. @@ -1268,6 +1319,90 @@ shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst, return make_gadget_report(RetKind, Inst, *RetReg); } +/// While BOLT already marks some of the branch instructions as tail calls, +/// this function tries to detect less obvious cases, assuming false positives +/// are acceptable as long as there are not too many of them. +/// +/// It is possible that not all the instructions classified as tail calls by +/// this function are safe to be considered as such for the purpose of code +/// transformations performed by BOLT. The intention of this function is to +/// spot some of actually missed tail calls (and likely a number of unrelated +/// indirect branch instructions) as long as this doesn't increase the amount +/// of false positive reports unacceptably. +static bool shouldAnalyzeTailCallInst(const BinaryContext &BC, + const BinaryFunction &BF, + const MCInstReference &Inst) { + // Some BC.MIB->isXYZ(Inst) methods simply delegate to MCInstrDesc::isXYZ() + // (such as isBranch at the time of writing this comment), some don't (such + // as isCall). For that reason, call MCInstrDesc's methods explicitly when + // it is important. + const MCInstrDesc &Desc = + BC.MII->get(static_cast(Inst).getOpcode()); + // Tail call should be a branch (but not necessarily an indirect one). + if (!Desc.isBranch()) + return false; + + // Always analyze the branches already marked as tail calls by BOLT. + if (BC.MIB->isTailCall(Inst)) + return true; + + // Try to also check the branches marked as "UNKNOWN CONTROL FLOW" - the + // below is a simplified condition from BinaryContext::printInstruction. + bool IsUnknownControlFlow = + BC.MIB->isIndirectBranch(Inst) && !BC.MIB->getJumpTable(Inst); + + if (BF.hasCFG() && IsUnknownControlFlow) + return true; + + return false; +} + +static std::optional> +shouldReportUnsafeTailCall(const BinaryContext &BC, const BinaryFunction &BF, + const MCInstReference &Inst, const SrcState &S) { + static const GadgetKind UntrustedLRKind( + "untrusted link register found before tail call"); + + if (!shouldAnalyzeTailCallInst(BC, BF, Inst)) + return std::nullopt; + + // Not only the set of registers returned by getTrustedLiveInRegs() can be + // seen as a reasonable target-independent _approximation_ of "the LR", these + // are *exactly* those registers used by SrcSafetyAnalysis to initialize the + // set of trusted registers on function entry. + // Thus, this function basically checks that the precondition expected to be + // imposed by a function call instruction (which is hardcoded into the target- + // specific getTrustedLiveInRegs() function) is also respected on tail calls. + SmallVector RegsToCheck = BC.MIB->getTrustedLiveInRegs(); + LLVM_DEBUG({ + traceInst(BC, "Found tail call inst", Inst); + traceRegMask(BC, "Trusted regs", S.TrustedRegs); + }); + + // In musl on AArch64, the _start function sets LR to zero and calls the next + // stage initialization function at the end, something along these lines: + // + // _start: + // mov x30, #0 + // ; ... other initialization ... + // b _start_c ; performs "exit" system call at some point + // + // As this would produce a false positive for every executable linked with + // such libc, ignore tail calls performed by ELF entry function. + if (BC.StartFunctionAddress && + *BC.StartFunctionAddress == Inst.getFunction()->getAddress()) { + LLVM_DEBUG({ dbgs() << " Skipping tail call in ELF entry function.\n"; }); + return std::nullopt; + } + + // Returns at most one report per instruction - this is probably OK... + for (auto Reg : RegsToCheck) + if (!S.TrustedRegs[Reg]) + return make_gadget_report(UntrustedLRKind, Inst, Reg); + + return std::nullopt; +} + static std::optional> shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst, const SrcState &S) { @@ -1344,17 +1479,6 @@ shouldReportAuthOracle(const BinaryContext &BC, const MCInstReference &Inst, return make_gadget_report(AuthOracleKind, Inst, *AuthReg); } -template static void iterateOverInstrs(BinaryFunction &BF, T Fn) { - if (BF.hasCFG()) { - for (BinaryBasicBlock &BB : BF) - for (int64_t I = 0, E = BB.size(); I < E; ++I) - Fn(MCInstInBBReference(&BB, I)); - } else { - for (auto I : BF.instrs()) - Fn(MCInstInBFReference(&BF, I.first)); - } -} - static SmallVector collectRegsToTrack(ArrayRef> Reports) { SmallSet RegsToTrack; @@ -1375,17 +1499,60 @@ void FunctionAnalysisContext::findUnsafeUses( BF.dump(); }); + bool UnreachableBBReported = false; + if (BF.hasCFG()) { + // Warn on basic blocks being unreachable according to BOLT (at most once + // per BinaryFunction), as this likely means the CFG reconstructed by BOLT + // is imprecise. A basic block can be + // * reachable from an entry basic block - a hopefully correct non-empty + // state is propagated to that basic block sooner or later. All basic + // blocks are expected to belong to this category under normal conditions. + // * reachable from a "directly unreachable" BB (a basic block that has no + // direct predecessors and this is not because it is an entry BB) - *some* + // non-empty state is propagated to this basic block sooner or later, as + // the initial state of directly unreachable basic blocks is + // pessimistically initialized to "all registers are unsafe" + // - a warning can be printed for the "directly unreachable" basic block + // * neither reachable from an entry nor from a "directly unreachable" BB + // (such as if this BB is in an isolated loop of basic blocks) - the final + // state is computed to be empty for this basic block + // - a warning can be printed for this basic block + for (BinaryBasicBlock &BB : BF) { + MCInst *FirstInst = BB.getFirstNonPseudoInstr(); + // Skip empty basic block early for simplicity. + if (!FirstInst) + continue; + + bool IsDirectlyUnreachable = BB.pred_empty() && !BB.isEntryPoint(); + bool HasNoStateComputed = Analysis->getStateBefore(*FirstInst).empty(); + if (!IsDirectlyUnreachable && !HasNoStateComputed) + continue; + + // Arbitrarily attach the report to the first instruction of BB. + // This is printed as "[message] in function [name], basic block ..., + // at address ..." when the issue is reported to the user. + Reports.push_back(make_generic_report( + MCInstReference::get(FirstInst, BF), + "Warning: possibly imprecise CFG, the analysis quality may be " + "degraded in this function. According to BOLT, unreachable code is " + "found" /* in function [name]... */)); + UnreachableBBReported = true; + break; // One warning per function. + } + } + // FIXME: Warn the user about imprecise analysis when the function has no CFG + // information at all. + iterateOverInstrs(BF, [&](MCInstReference Inst) { if (BC.MIB->isCFI(Inst)) return; const SrcState &S = Analysis->getStateBefore(Inst); - - // If non-empty state was never propagated from the entry basic block - // to Inst, assume it to be unreachable and report a warning. if (S.empty()) { - Reports.push_back( - make_generic_report(Inst, "Warning: unreachable instruction found")); + LLVM_DEBUG( + { traceInst(BC, "Instruction has no state, skipping", Inst); }); + assert(UnreachableBBReported && "Should be reported at least once"); + (void)UnreachableBBReported; return; } @@ -1395,6 +1562,9 @@ void FunctionAnalysisContext::findUnsafeUses( if (PacRetGadgetsOnly) return; + if (auto Report = shouldReportUnsafeTailCall(BC, BF, Inst, S)) + Reports.push_back(*Report); + if (auto Report = shouldReportCallGadget(BC, Inst, S)) Reports.push_back(*Report); if (auto Report = shouldReportSigningOracle(BC, Inst, S)) diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index 5c8af3710720d..88229bb31a2ad 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -466,9 +466,7 @@ int DataAggregator::prepareToParse(StringRef Name, PerfProcessInfo &Process, return PI.ReturnCode; } -Error DataAggregator::preprocessProfile(BinaryContext &BC) { - this->BC = &BC; - +void DataAggregator::parsePerfData(BinaryContext &BC) { auto ErrorCallback = [](int ReturnCode, StringRef ErrBuf) { errs() << "PERF-ERROR: return code " << ReturnCode << "\n" << ErrBuf; exit(1); @@ -481,11 +479,6 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { ErrorCallback(ReturnCode, ErrBuf); }; - if (opts::ReadPreAggregated) { - parsePreAggregated(); - goto heatmap; - } - if (std::optional FileBuildID = BC.getFileBuildID()) { outs() << "BOLT-INFO: binary build-id is: " << *FileBuildID << "\n"; processFileBuildID(*FileBuildID); @@ -534,22 +527,28 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) { << '\n'; deleteTempFiles(); +} -heatmap: - // Sort parsed traces for faster processing. - llvm::sort(Traces, llvm::less_first()); +Error DataAggregator::preprocessProfile(BinaryContext &BC) { + this->BC = &BC; - if (!opts::HeatmapMode) - return Error::success(); + if (opts::ReadPreAggregated) { + parsePreAggregated(); + } else { + parsePerfData(BC); + } - if (std::error_code EC = printLBRHeatMap()) - return errorCodeToError(EC); + // Sort parsed traces for faster processing. + llvm::sort(Traces, llvm::less_first()); - if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Optional) - return Error::success(); + if (opts::HeatmapMode) { + if (std::error_code EC = printLBRHeatMap()) + return errorCodeToError(EC); + if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive) + exit(0); + } - assert(opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive); - exit(0); + return Error::success(); } Error DataAggregator::readProfile(BinaryContext &BC) { diff --git a/bolt/lib/Rewrite/CMakeLists.txt b/bolt/lib/Rewrite/CMakeLists.txt index c83cf36982167..775036063dd5a 100644 --- a/bolt/lib/Rewrite/CMakeLists.txt +++ b/bolt/lib/Rewrite/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS Core DebugInfoDWARF + DebugInfoDWARFLowLevel JITLink MC Object diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp index 9c9bdefe08429..0c1a1bac6c72e 100644 --- a/bolt/lib/Rewrite/DWARFRewriter.cpp +++ b/bolt/lib/Rewrite/DWARFRewriter.cpp @@ -24,10 +24,10 @@ #include "llvm/DebugInfo/DWARF/DWARFContext.h" #include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h" #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h" -#include "llvm/DebugInfo/DWARF/DWARFExpression.h" #include "llvm/DebugInfo/DWARF/DWARFFormValue.h" #include "llvm/DebugInfo/DWARF/DWARFTypeUnit.h" #include "llvm/DebugInfo/DWARF/DWARFUnit.h" +#include "llvm/DebugInfo/DWARF/LowLevel/DWARFExpression.h" #include "llvm/MC/MCAsmBackend.h" #include "llvm/MC/MCAssembler.h" #include "llvm/MC/MCObjectWriter.h" diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp index 93bd93b6cb984..96045a916232c 100644 --- a/bolt/lib/Rewrite/RewriteInstance.cpp +++ b/bolt/lib/Rewrite/RewriteInstance.cpp @@ -547,9 +547,14 @@ Error RewriteInstance::discoverStorage() { NextAvailableOffset = std::max(NextAvailableOffset, Phdr.p_offset + Phdr.p_filesz); - BC->SegmentMapInfo[Phdr.p_vaddr] = SegmentInfo{ - Phdr.p_vaddr, Phdr.p_memsz, Phdr.p_offset, - Phdr.p_filesz, Phdr.p_align, ((Phdr.p_flags & ELF::PF_X) != 0)}; + BC->SegmentMapInfo[Phdr.p_vaddr] = + SegmentInfo{Phdr.p_vaddr, + Phdr.p_memsz, + Phdr.p_offset, + Phdr.p_filesz, + Phdr.p_align, + (Phdr.p_flags & ELF::PF_X) != 0, + (Phdr.p_flags & ELF::PF_W) != 0}; if (BC->TheTriple->getArch() == llvm::Triple::x86_64 && Phdr.p_vaddr >= BinaryContext::KernelStartX86_64) BC->IsLinuxKernel = true; @@ -626,6 +631,9 @@ Error RewriteInstance::discoverStorage() { NextAvailableAddress += BC->PageAlign; } + NewTextSegmentAddress = NextAvailableAddress; + NewTextSegmentOffset = NextAvailableOffset; + if (!opts::UseGnuStack && !BC->IsLinuxKernel) { // This is where the black magic happens. Creating PHDR table in a segment // other than that containing ELF header is tricky. Some loaders and/or @@ -652,6 +660,8 @@ Error RewriteInstance::discoverStorage() { PHDRTableAddress = NextAvailableAddress; PHDRTableOffset = NextAvailableOffset; + NewTextSegmentAddress = NextAvailableAddress; + NewTextSegmentOffset = NextAvailableOffset; // Reserve space for 3 extra pheaders. unsigned Phnum = Obj.getHeader().e_phnum; @@ -664,14 +674,12 @@ Error RewriteInstance::discoverStorage() { NextAvailableAddress += Phnum * sizeof(ELF64LEPhdrTy); NextAvailableOffset += Phnum * sizeof(ELF64LEPhdrTy); - } - // Align at cache line. - NextAvailableAddress = alignTo(NextAvailableAddress, 64); - NextAvailableOffset = alignTo(NextAvailableOffset, 64); + // Align at cache line. + NextAvailableAddress = alignTo(NextAvailableAddress, 64); + NextAvailableOffset = alignTo(NextAvailableOffset, 64); + } - NewTextSegmentAddress = NextAvailableAddress; - NewTextSegmentOffset = NextAvailableOffset; BC->LayoutStartAddress = NextAvailableAddress; // Tools such as objcopy can strip section contents but leave header @@ -3853,111 +3861,138 @@ std::vector RewriteInstance::getCodeSections() { } void RewriteInstance::mapCodeSections(BOLTLinker::SectionMapper MapSection) { - if (BC->HasRelocations) { - // Map sections for functions with pre-assigned addresses. - for (BinaryFunction *InjectedFunction : BC->getInjectedBinaryFunctions()) { - const uint64_t OutputAddress = InjectedFunction->getOutputAddress(); - if (!OutputAddress) - continue; - - ErrorOr FunctionSection = - InjectedFunction->getCodeSection(); - assert(FunctionSection && "function should have section"); - FunctionSection->setOutputAddress(OutputAddress); - MapSection(*FunctionSection, OutputAddress); - InjectedFunction->setImageAddress(FunctionSection->getAllocAddress()); - InjectedFunction->setImageSize(FunctionSection->getOutputSize()); - } + if (!BC->HasRelocations) { + mapCodeSectionsInPlace(MapSection); + return; + } - // Populate the list of sections to be allocated. - std::vector CodeSections = getCodeSections(); + // Map sections for functions with pre-assigned addresses. + for (BinaryFunction *InjectedFunction : BC->getInjectedBinaryFunctions()) { + const uint64_t OutputAddress = InjectedFunction->getOutputAddress(); + if (!OutputAddress) + continue; - // Remove sections that were pre-allocated (patch sections). - llvm::erase_if(CodeSections, [](BinarySection *Section) { - return Section->getOutputAddress(); - }); - LLVM_DEBUG(dbgs() << "Code sections in the order of output:\n"; - for (const BinarySection *Section : CodeSections) - dbgs() << Section->getName() << '\n'; - ); + ErrorOr FunctionSection = + InjectedFunction->getCodeSection(); + assert(FunctionSection && "function should have section"); + FunctionSection->setOutputAddress(OutputAddress); + MapSection(*FunctionSection, OutputAddress); + InjectedFunction->setImageAddress(FunctionSection->getAllocAddress()); + InjectedFunction->setImageSize(FunctionSection->getOutputSize()); + } - uint64_t PaddingSize = 0; // size of padding required at the end + // Populate the list of sections to be allocated. + std::vector CodeSections = getCodeSections(); - // Allocate sections starting at a given Address. - auto allocateAt = [&](uint64_t Address) { - const char *LastNonColdSectionName = BC->HasWarmSection - ? BC->getWarmCodeSectionName() - : BC->getMainCodeSectionName(); - for (BinarySection *Section : CodeSections) { - Address = alignTo(Address, Section->getAlignment()); - Section->setOutputAddress(Address); - Address += Section->getOutputSize(); - - // Hugify: Additional huge page from right side due to - // weird ASLR mapping addresses (4KB aligned) - if (opts::Hugify && !BC->HasFixedLoadAddress && - Section->getName() == LastNonColdSectionName) - Address = alignTo(Address, Section->getAlignment()); - } + // Remove sections that were pre-allocated (patch sections). + llvm::erase_if(CodeSections, [](BinarySection *Section) { + return Section->getOutputAddress(); + }); + LLVM_DEBUG(dbgs() << "Code sections in the order of output:\n"; + for (const BinarySection *Section : CodeSections) dbgs() + << Section->getName() << '\n';); - // Make sure we allocate enough space for huge pages. - ErrorOr TextSection = - BC->getUniqueSectionByName(LastNonColdSectionName); - if (opts::HotText && TextSection && TextSection->hasValidSectionID()) { - uint64_t HotTextEnd = - TextSection->getOutputAddress() + TextSection->getOutputSize(); - HotTextEnd = alignTo(HotTextEnd, BC->PageAlign); - if (HotTextEnd > Address) { - PaddingSize = HotTextEnd - Address; - Address = HotTextEnd; - } - } - return Address; - }; + uint64_t PaddingSize = 0; // size of padding required at the end - // Check if we can fit code in the original .text - bool AllocationDone = false; - if (opts::UseOldText) { - const uint64_t CodeSize = - allocateAt(BC->OldTextSectionAddress) - BC->OldTextSectionAddress; + // Allocate sections starting at a given Address. + auto allocateAt = [&](uint64_t Address) { + const char *LastNonColdSectionName = BC->HasWarmSection + ? BC->getWarmCodeSectionName() + : BC->getMainCodeSectionName(); + for (BinarySection *Section : CodeSections) { + Address = alignTo(Address, Section->getAlignment()); + Section->setOutputAddress(Address); + Address += Section->getOutputSize(); + + // Hugify: Additional huge page from right side due to + // weird ASLR mapping addresses (4KB aligned) + if (opts::Hugify && !BC->HasFixedLoadAddress && + Section->getName() == LastNonColdSectionName) + Address = alignTo(Address, Section->getAlignment()); + } - if (CodeSize <= BC->OldTextSectionSize) { - BC->outs() << "BOLT-INFO: using original .text for new code with 0x" - << Twine::utohexstr(opts::AlignText) << " alignment\n"; - AllocationDone = true; - } else { - BC->errs() - << "BOLT-WARNING: original .text too small to fit the new code" - << " using 0x" << Twine::utohexstr(opts::AlignText) - << " alignment. " << CodeSize << " bytes needed, have " - << BC->OldTextSectionSize << " bytes available.\n"; - opts::UseOldText = false; + // Make sure we allocate enough space for huge pages. + ErrorOr TextSection = + BC->getUniqueSectionByName(LastNonColdSectionName); + if (opts::HotText && TextSection && TextSection->hasValidSectionID()) { + uint64_t HotTextEnd = + TextSection->getOutputAddress() + TextSection->getOutputSize(); + HotTextEnd = alignTo(HotTextEnd, BC->PageAlign); + if (HotTextEnd > Address) { + PaddingSize = HotTextEnd - Address; + Address = HotTextEnd; } } + return Address; + }; - if (!AllocationDone) - NextAvailableAddress = allocateAt(NextAvailableAddress); + // Try to allocate sections before the \p Address and return an address for + // the allocation of the first section, or 0 if [0, Address) range is not + // big enough to fit all sections. + auto allocateBefore = [&](uint64_t Address) -> uint64_t { + for (BinarySection *Section : llvm::reverse(CodeSections)) { + if (Section->getOutputSize() > Address) + return 0; + Address -= Section->getOutputSize(); + Address = alignDown(Address, Section->getAlignment()); + Section->setOutputAddress(Address); + } + return Address; + }; - // Do the mapping for ORC layer based on the allocation. - for (BinarySection *Section : CodeSections) { - LLVM_DEBUG( - dbgs() << "BOLT: mapping " << Section->getName() << " at 0x" - << Twine::utohexstr(Section->getAllocAddress()) << " to 0x" - << Twine::utohexstr(Section->getOutputAddress()) << '\n'); - MapSection(*Section, Section->getOutputAddress()); - Section->setOutputFileOffset( - getFileOffsetForAddress(Section->getOutputAddress())); + // Check if we can fit code in the original .text + bool AllocationDone = false; + if (opts::UseOldText) { + uint64_t StartAddress; + uint64_t EndAddress; + if (opts::HotFunctionsAtEnd) { + EndAddress = BC->OldTextSectionAddress + BC->OldTextSectionSize; + StartAddress = allocateBefore(EndAddress); + } else { + StartAddress = BC->OldTextSectionAddress; + EndAddress = allocateAt(BC->OldTextSectionAddress); + } + + const uint64_t CodeSize = EndAddress - StartAddress; + if (CodeSize <= BC->OldTextSectionSize) { + BC->outs() << "BOLT-INFO: using original .text for new code with 0x" + << Twine::utohexstr(opts::AlignText) << " alignment"; + if (StartAddress != BC->OldTextSectionAddress) + BC->outs() << " at 0x" << Twine::utohexstr(StartAddress); + BC->outs() << '\n'; + AllocationDone = true; + } else { + BC->errs() << "BOLT-WARNING: original .text too small to fit the new code" + << " using 0x" << Twine::utohexstr(opts::AlignText) + << " alignment. " << CodeSize << " bytes needed, have " + << BC->OldTextSectionSize << " bytes available.\n"; + opts::UseOldText = false; } + } - // Check if we need to insert a padding section for hot text. - if (PaddingSize && !opts::UseOldText) - BC->outs() << "BOLT-INFO: padding code to 0x" - << Twine::utohexstr(NextAvailableAddress) - << " to accommodate hot text\n"; + if (!AllocationDone) + NextAvailableAddress = allocateAt(NextAvailableAddress); - return; + // Do the mapping for ORC layer based on the allocation. + for (BinarySection *Section : CodeSections) { + LLVM_DEBUG(dbgs() << "BOLT: mapping " << Section->getName() << " at 0x" + << Twine::utohexstr(Section->getAllocAddress()) + << " to 0x" + << Twine::utohexstr(Section->getOutputAddress()) << '\n'); + MapSection(*Section, Section->getOutputAddress()); + Section->setOutputFileOffset( + getFileOffsetForAddress(Section->getOutputAddress())); } + // Check if we need to insert a padding section for hot text. + if (PaddingSize && !opts::UseOldText) + BC->outs() << "BOLT-INFO: padding code to 0x" + << Twine::utohexstr(NextAvailableAddress) + << " to accommodate hot text\n"; +} + +void RewriteInstance::mapCodeSectionsInPlace( + BOLTLinker::SectionMapper MapSection) { // Processing in non-relocation mode. uint64_t NewTextSectionStartAddress = NextAvailableAddress; @@ -4133,13 +4168,8 @@ void RewriteInstance::mapAllocatableSections( } if (SType == ST_READONLY) { - if (PHDRTableAddress) { - // Segment size includes the size of the PHDR area. - NewTextSegmentSize = NextAvailableAddress - PHDRTableAddress; - } else if (NewTextSegmentAddress) { - // Existing PHDR table would be updated. + if (NewTextSegmentAddress) NewTextSegmentSize = NextAvailableAddress - NewTextSegmentAddress; - } } else if (SType == ST_READWRITE) { NewWritableSegmentSize = NextAvailableAddress - NewWritableSegmentAddress; // Restore NextAvailableAddress if no new writable sections @@ -4157,6 +4187,74 @@ void RewriteInstance::updateOutputValues(const BOLTLinker &Linker) { Function->updateOutputValues(Linker); } +void RewriteInstance::updateSegmentInfo() { + // NOTE Currently .eh_frame_hdr appends to the last segment, recalculate + // last segments size based on the NextAvailableAddress variable. + if (!NewWritableSegmentSize) { + if (NewTextSegmentAddress) + NewTextSegmentSize = NextAvailableAddress - NewTextSegmentAddress; + } else { + NewWritableSegmentSize = NextAvailableAddress - NewWritableSegmentAddress; + } + + if (NewTextSegmentSize) { + SegmentInfo TextSegment = {NewTextSegmentAddress, + NewTextSegmentSize, + NewTextSegmentOffset, + NewTextSegmentSize, + BC->PageAlign, + true, + false}; + if (!opts::Instrument) { + BC->NewSegments.push_back(TextSegment); + } else { + ErrorOr Sec = + BC->getUniqueSectionByName(".bolt.instr.counters"); + assert(Sec && "expected one and only one `.bolt.instr.counters` section"); + const uint64_t Addr = Sec->getOutputAddress(); + const uint64_t Offset = Sec->getOutputFileOffset(); + const uint64_t Size = Sec->getOutputSize(); + assert(Addr > TextSegment.Address && + Addr + Size < TextSegment.Address + TextSegment.Size && + "`.bolt.instr.counters` section is expected to be included in the " + "new text segment"); + + // Set correct size for the previous header since we are breaking the + // new text segment into three segments. + uint64_t Delta = Addr - TextSegment.Address; + TextSegment.Size = Delta; + TextSegment.FileSize = Delta; + BC->NewSegments.push_back(TextSegment); + + // Create RW segment that includes the `.bolt.instr.counters` section. + SegmentInfo RWSegment = {Addr, Size, Offset, Size, BC->RegularPageSize, + false, true}; + BC->NewSegments.push_back(RWSegment); + + // Create RX segment that includes all RX sections from runtime library. + const uint64_t AddrRX = alignTo(Addr + Size, BC->RegularPageSize); + const uint64_t OffsetRX = alignTo(Offset + Size, BC->RegularPageSize); + const uint64_t SizeRX = + NewTextSegmentSize - (AddrRX - TextSegment.Address); + SegmentInfo RXSegment = { + AddrRX, SizeRX, OffsetRX, SizeRX, BC->RegularPageSize, true, false}; + BC->NewSegments.push_back(RXSegment); + } + } + + if (NewWritableSegmentSize) { + SegmentInfo DataSegmentInfo = { + NewWritableSegmentAddress, + NewWritableSegmentSize, + getFileOffsetForAddress(NewWritableSegmentAddress), + NewWritableSegmentSize, + BC->RegularPageSize, + false, + true}; + BC->NewSegments.push_back(DataSegmentInfo); + } +} + void RewriteInstance::patchELFPHDRTable() { auto ELF64LEFile = cast(InputFile); const ELFFile &Obj = ELF64LEFile->getELFFile(); @@ -4183,110 +4281,36 @@ void RewriteInstance::patchELFPHDRTable() { if (opts::Instrument) Phnum += 2; - // 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 if (NewTextSegmentAddress) - NewTextSegmentSize = NextAvailableAddress - NewTextSegmentAddress; - } else { - NewWritableSegmentSize = NextAvailableAddress - NewWritableSegmentAddress; + if (BC->NewSegments.empty()) { + BC->outs() << "BOLT-INFO: not adding new segments\n"; + return; } const uint64_t SavedPos = OS.tell(); OS.seek(PHDRTableOffset); - auto createNewPhdrs = [&]() { - SmallVector NewPhdrs; - ELF64LEPhdrTy NewPhdr; - NewPhdr.p_type = ELF::PT_LOAD; - if (PHDRTableAddress) { - NewPhdr.p_offset = PHDRTableOffset; - NewPhdr.p_vaddr = PHDRTableAddress; - NewPhdr.p_paddr = PHDRTableAddress; - } else { - NewPhdr.p_offset = NewTextSegmentOffset; - NewPhdr.p_vaddr = NewTextSegmentAddress; - NewPhdr.p_paddr = NewTextSegmentAddress; - } - NewPhdr.p_filesz = NewTextSegmentSize; - NewPhdr.p_memsz = NewTextSegmentSize; - NewPhdr.p_flags = ELF::PF_X | ELF::PF_R; - NewPhdr.p_align = BC->PageAlign; - - if (!opts::Instrument) { - NewPhdrs.push_back(NewPhdr); - } else { - ErrorOr Sec = - BC->getUniqueSectionByName(".bolt.instr.counters"); - assert(Sec && "expected one and only one `.bolt.instr.counters` section"); - const uint64_t Addr = Sec->getOutputAddress(); - const uint64_t Offset = Sec->getOutputFileOffset(); - const uint64_t Size = Sec->getOutputSize(); - assert(Addr > NewPhdr.p_vaddr && - Addr + Size < NewPhdr.p_vaddr + NewPhdr.p_memsz && - "`.bolt.instr.counters` section is expected to be included in the " - "new text sgement"); - - // Set correct size for the previous header since we are breaking the - // new text segment into three segments. - uint64_t Delta = Addr - NewPhdr.p_vaddr; - NewPhdr.p_filesz = Delta; - NewPhdr.p_memsz = Delta; - NewPhdrs.push_back(NewPhdr); - - // Create a program header for a RW segment that includes the - // `.bolt.instr.counters` section only. - ELF64LEPhdrTy NewPhdrRWSegment; - NewPhdrRWSegment.p_type = ELF::PT_LOAD; - NewPhdrRWSegment.p_offset = Offset; - NewPhdrRWSegment.p_vaddr = Addr; - NewPhdrRWSegment.p_paddr = Addr; - NewPhdrRWSegment.p_filesz = Size; - NewPhdrRWSegment.p_memsz = Size; - NewPhdrRWSegment.p_flags = ELF::PF_R | ELF::PF_W; - NewPhdrRWSegment.p_align = BC->RegularPageSize; - NewPhdrs.push_back(NewPhdrRWSegment); - - // Create a program header for a RX segment that includes all the RX - // sections from runtime library. - ELF64LEPhdrTy NewPhdrRXSegment; - NewPhdrRXSegment.p_type = ELF::PT_LOAD; - const uint64_t AddrRX = alignTo(Addr + Size, BC->RegularPageSize); - const uint64_t OffsetRX = alignTo(Offset + Size, BC->RegularPageSize); - const uint64_t SizeRX = NewTextSegmentSize - (AddrRX - NewPhdr.p_paddr); - NewPhdrRXSegment.p_offset = OffsetRX; - NewPhdrRXSegment.p_vaddr = AddrRX; - NewPhdrRXSegment.p_paddr = AddrRX; - NewPhdrRXSegment.p_filesz = SizeRX; - NewPhdrRXSegment.p_memsz = SizeRX; - NewPhdrRXSegment.p_flags = ELF::PF_X | ELF::PF_R; - NewPhdrRXSegment.p_align = BC->RegularPageSize; - NewPhdrs.push_back(NewPhdrRXSegment); - } - - return NewPhdrs; + auto createPhdr = [](const SegmentInfo &SI) { + ELF64LEPhdrTy Phdr; + Phdr.p_type = ELF::PT_LOAD; + Phdr.p_offset = SI.FileOffset; + Phdr.p_vaddr = SI.Address; + Phdr.p_paddr = SI.Address; + Phdr.p_filesz = SI.FileSize; + Phdr.p_memsz = SI.Size; + Phdr.p_flags = ELF::PF_R; + if (SI.IsExecutable) + Phdr.p_flags |= ELF::PF_X; + if (SI.IsWritable) + Phdr.p_flags |= ELF::PF_W; + Phdr.p_align = SI.Alignment; + + return Phdr; }; auto writeNewSegmentPhdrs = [&]() { - if (PHDRTableAddress || NewTextSegmentSize) { - SmallVector NewPhdrs = createNewPhdrs(); - OS.write(reinterpret_cast(NewPhdrs.data()), - sizeof(ELF64LE::Phdr) * NewPhdrs.size()); - } - - if (NewWritableSegmentSize) { - 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; - OS.write(reinterpret_cast(&NewPhdr), sizeof(NewPhdr)); + for (const SegmentInfo &SI : BC->NewSegments) { + ELF64LEPhdrTy Phdr = createPhdr(SI); + OS.write(reinterpret_cast(&Phdr), sizeof(Phdr)); } }; @@ -4322,11 +4346,9 @@ void RewriteInstance::patchELFPHDRTable() { case ELF::PT_GNU_STACK: if (opts::UseGnuStack) { // Overwrite the header with the new segment header. - assert(!opts::Instrument); - SmallVector NewPhdrs = createNewPhdrs(); - assert(NewPhdrs.size() == 1 && - "expect exactly one program header was created"); - NewPhdr = NewPhdrs[0]; + assert(BC->NewSegments.size() == 1 && + "Expected exactly one new segment"); + NewPhdr = createPhdr(BC->NewSegments.front()); ModdedGnuStack = true; } break; @@ -5951,8 +5973,10 @@ void RewriteInstance::rewriteFile() { addBATSection(); // Patch program header table. - if (!BC->IsLinuxKernel) + if (!BC->IsLinuxKernel) { + updateSegmentInfo(); patchELFPHDRTable(); + } // Finalize memory image of section string table. finalizeSectionStringTable(); diff --git a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp index e6e0aeba34572..973261765f951 100644 --- a/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp +++ b/bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp @@ -22,7 +22,6 @@ #include "bolt/Core/MCPlusBuilder.h" #include "llvm/BinaryFormat/ELF.h" #include "llvm/MC/MCContext.h" -#include "llvm/MC/MCFixupKindInfo.h" #include "llvm/MC/MCInstBuilder.h" #include "llvm/MC/MCInstrInfo.h" #include "llvm/MC/MCRegister.h" @@ -1081,7 +1080,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { if (isADR(Inst) || RelType == ELF::R_AARCH64_ADR_PREL_LO21 || RelType == ELF::R_AARCH64_TLSDESC_ADR_PREL21) { - return MCSpecifierExpr::create(Expr, AArch64MCExpr::VK_ABS, Ctx); + return MCSpecifierExpr::create(Expr, AArch64::S_ABS, Ctx); } else if (isADRP(Inst) || RelType == ELF::R_AARCH64_ADR_PREL_PG_HI21 || RelType == ELF::R_AARCH64_ADR_PREL_PG_HI21_NC || RelType == ELF::R_AARCH64_TLSDESC_ADR_PAGE21 || @@ -1089,7 +1088,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { RelType == ELF::R_AARCH64_ADR_GOT_PAGE) { // Never emit a GOT reloc, we handled this in // RewriteInstance::readRelocations(). - return MCSpecifierExpr::create(Expr, AArch64MCExpr::VK_ABS_PAGE, Ctx); + return MCSpecifierExpr::create(Expr, AArch64::S_ABS_PAGE, Ctx); } else { switch (RelType) { case ELF::R_AARCH64_ADD_ABS_LO12_NC: @@ -1103,18 +1102,18 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { case ELF::R_AARCH64_TLSDESC_LD64_LO12: case ELF::R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC: case ELF::R_AARCH64_TLSLE_ADD_TPREL_LO12_NC: - return MCSpecifierExpr::create(Expr, AArch64MCExpr::VK_LO12, Ctx); + return MCSpecifierExpr::create(Expr, AArch64::S_LO12, Ctx); case ELF::R_AARCH64_MOVW_UABS_G3: - return MCSpecifierExpr::create(Expr, AArch64MCExpr::VK_ABS_G3, Ctx); + return MCSpecifierExpr::create(Expr, AArch64::S_ABS_G3, Ctx); case ELF::R_AARCH64_MOVW_UABS_G2: case ELF::R_AARCH64_MOVW_UABS_G2_NC: - return MCSpecifierExpr::create(Expr, AArch64MCExpr::VK_ABS_G2_NC, Ctx); + return MCSpecifierExpr::create(Expr, AArch64::S_ABS_G2_NC, Ctx); case ELF::R_AARCH64_MOVW_UABS_G1: case ELF::R_AARCH64_MOVW_UABS_G1_NC: - return MCSpecifierExpr::create(Expr, AArch64MCExpr::VK_ABS_G1_NC, Ctx); + return MCSpecifierExpr::create(Expr, AArch64::S_ABS_G1_NC, Ctx); case ELF::R_AARCH64_MOVW_UABS_G0: case ELF::R_AARCH64_MOVW_UABS_G0_NC: - return MCSpecifierExpr::create(Expr, AArch64MCExpr::VK_ABS_G0_NC, Ctx); + return MCSpecifierExpr::create(Expr, AArch64::S_ABS_G0_NC, Ctx); default: break; } @@ -1206,8 +1205,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { OI = Inst.begin() + 2; } - *OI = MCOperand::createExpr( - MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx)); + *OI = MCOperand::createExpr(MCSymbolRefExpr::create(TBB, *Ctx)); } /// Matches indirect branch patterns in AArch64 related to a jump table (JT), @@ -1633,8 +1631,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { .addImm(0)); Code.emplace_back(MCInstBuilder(AArch64::Bcc) .addImm(AArch64CC::EQ) - .addExpr(MCSymbolRefExpr::create( - Target, MCSymbolRefExpr::VK_None, *Ctx))); + .addExpr(MCSymbolRefExpr::create(Target, *Ctx))); return Code; } @@ -1656,8 +1653,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { .addImm(0)); Code.emplace_back(MCInstBuilder(AArch64::Bcc) .addImm(AArch64CC::NE) - .addExpr(MCSymbolRefExpr::create( - Target, MCSymbolRefExpr::VK_None, *Ctx))); + .addExpr(MCSymbolRefExpr::create(Target, *Ctx))); return Code; } @@ -1957,8 +1953,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { Inst.setOpcode(IsTailCall ? AArch64::B : AArch64::BL); Inst.clear(); Inst.addOperand(MCOperand::createExpr(getTargetExprFor( - Inst, MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx), - *Ctx, 0))); + Inst, MCSymbolRefExpr::create(Target, *Ctx), *Ctx, 0))); if (IsTailCall) convertJmpToTailCall(Inst); } @@ -2028,7 +2023,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { Inst.setOpcode(AArch64::MOVZXi); Inst.addOperand(MCOperand::createReg(AArch64::X16)); Inst.addOperand(MCOperand::createExpr( - MCSpecifierExpr::create(Target, AArch64MCExpr::VK_ABS_G3, *Ctx))); + MCSpecifierExpr::create(Target, AArch64::S_ABS_G3, *Ctx))); Inst.addOperand(MCOperand::createImm(0x30)); Seq.emplace_back(Inst); @@ -2037,7 +2032,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { Inst.addOperand(MCOperand::createReg(AArch64::X16)); Inst.addOperand(MCOperand::createReg(AArch64::X16)); Inst.addOperand(MCOperand::createExpr( - MCSpecifierExpr::create(Target, AArch64MCExpr::VK_ABS_G2_NC, *Ctx))); + MCSpecifierExpr::create(Target, AArch64::S_ABS_G2_NC, *Ctx))); Inst.addOperand(MCOperand::createImm(0x20)); Seq.emplace_back(Inst); @@ -2046,7 +2041,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { Inst.addOperand(MCOperand::createReg(AArch64::X16)); Inst.addOperand(MCOperand::createReg(AArch64::X16)); Inst.addOperand(MCOperand::createExpr( - MCSpecifierExpr::create(Target, AArch64MCExpr::VK_ABS_G1_NC, *Ctx))); + MCSpecifierExpr::create(Target, AArch64::S_ABS_G1_NC, *Ctx))); Inst.addOperand(MCOperand::createImm(0x10)); Seq.emplace_back(Inst); @@ -2055,7 +2050,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { Inst.addOperand(MCOperand::createReg(AArch64::X16)); Inst.addOperand(MCOperand::createReg(AArch64::X16)); Inst.addOperand(MCOperand::createExpr( - MCSpecifierExpr::create(Target, AArch64MCExpr::VK_ABS_G0_NC, *Ctx))); + MCSpecifierExpr::create(Target, AArch64::S_ABS_G0_NC, *Ctx))); Inst.addOperand(MCOperand::createImm(0)); Seq.emplace_back(Inst); @@ -2228,9 +2223,8 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { MCContext *Ctx) const override { Inst.setOpcode(AArch64::B); Inst.clear(); - Inst.addOperand(MCOperand::createExpr(getTargetExprFor( - Inst, MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx), - *Ctx, 0))); + Inst.addOperand(MCOperand::createExpr( + getTargetExprFor(Inst, MCSymbolRefExpr::create(TBB, *Ctx), *Ctx, 0))); } bool shouldRecordCodeRelocation(uint32_t RelType) const override { @@ -2562,7 +2556,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder { else if (Fixup.getKind() == MCFixupKind(AArch64::fixup_aarch64_pcrel_branch26)) RelType = ELF::R_AARCH64_JUMP26; - else if (FKI.Flags & MCFixupKindInfo::FKF_IsPCRel) { + else if (Fixup.isPCRel()) { switch (FKI.TargetSize) { default: return std::nullopt; diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp index c7d664ab09d46..10b4913b6ab7f 100644 --- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp +++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp @@ -171,8 +171,8 @@ class RISCVMCPlusBuilder : public MCPlusBuilder { (void)Result; assert(Result && "unimplemented branch"); - Inst.getOperand(SymOpIndex) = MCOperand::createExpr( - MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx)); + Inst.getOperand(SymOpIndex) = + MCOperand::createExpr(MCSymbolRefExpr::create(TBB, *Ctx)); } IndirectBranchType analyzeIndirectBranch( @@ -233,8 +233,7 @@ class RISCVMCPlusBuilder : public MCPlusBuilder { Inst.setOpcode(RISCV::JAL); Inst.clear(); Inst.addOperand(MCOperand::createReg(RISCV::X0)); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand(MCOperand::createExpr(MCSymbolRefExpr::create(TBB, *Ctx))); } StringRef getTrapFillValue() const override { @@ -246,8 +245,7 @@ class RISCVMCPlusBuilder : public MCPlusBuilder { Inst.setOpcode(Opcode); Inst.clear(); Inst.addOperand(MCOperand::createExpr(MCSpecifierExpr::create( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx), - ELF::R_RISCV_CALL_PLT, *Ctx))); + MCSymbolRefExpr::create(Target, *Ctx), ELF::R_RISCV_CALL_PLT, *Ctx))); } void createCall(MCInst &Inst, const MCSymbol *Target, @@ -563,8 +561,7 @@ class RISCVMCPlusBuilder : public MCPlusBuilder { Insts.emplace_back(MCInstBuilder(RISCV::BEQ) .addReg(RegNo) .addReg(RegTmp) - .addExpr(MCSymbolRefExpr::create( - Target, MCSymbolRefExpr::VK_None, *Ctx))); + .addExpr(MCSymbolRefExpr::create(Target, *Ctx))); return Insts; } @@ -663,14 +660,12 @@ class RISCVMCPlusBuilder : public MCPlusBuilder { if (IsTailCall) { Inst.addOperand(MCOperand::createReg(RISCV::X0)); Inst.addOperand(MCOperand::createExpr(getTargetExprFor( - Inst, MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx), - *Ctx, 0))); + Inst, MCSymbolRefExpr::create(Target, *Ctx), *Ctx, 0))); convertJmpToTailCall(Inst); } else { Inst.addOperand(MCOperand::createReg(RISCV::X1)); Inst.addOperand(MCOperand::createExpr(getTargetExprFor( - Inst, MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx), - *Ctx, 0))); + Inst, MCSymbolRefExpr::create(Target, *Ctx), *Ctx, 0))); } } diff --git a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp index b909d7fb6bf28..09d2322a29ca6 100644 --- a/bolt/lib/Target/X86/X86MCPlusBuilder.cpp +++ b/bolt/lib/Target/X86/X86MCPlusBuilder.cpp @@ -18,7 +18,6 @@ #include "bolt/Core/MCPlusBuilder.h" #include "llvm/BinaryFormat/ELF.h" #include "llvm/MC/MCContext.h" -#include "llvm/MC/MCFixupKindInfo.h" #include "llvm/MC/MCInst.h" #include "llvm/MC/MCInstBuilder.h" #include "llvm/MC/MCInstrInfo.h" @@ -72,9 +71,9 @@ static InstructionListType createIncMemory(const MCSymbol *Target, Insts.back().addOperand(MCOperand::createImm(1)); // ScaleAmt Insts.back().addOperand(MCOperand::createReg(X86::NoRegister)); // IndexReg - Insts.back().addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, - *Ctx))); // Displacement + Insts.back().addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Target, + *Ctx))); // Displacement Insts.back().addOperand( MCOperand::createReg(X86::NoRegister)); // AddrSegmentReg return Insts; @@ -1625,9 +1624,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { Inst.insert(Inst.begin(), MCOperand::createReg(X86::NoRegister)); // AddrSegmentReg Inst.insert(Inst.begin(), - MCOperand::createExpr( // Displacement - MCSymbolRefExpr::create(TargetLocation, - MCSymbolRefExpr::VK_None, *Ctx))); + MCOperand::createExpr( // Displacement + MCSymbolRefExpr::create(TargetLocation, *Ctx))); Inst.insert(Inst.begin(), MCOperand::createReg(X86::NoRegister)); // IndexReg Inst.insert(Inst.begin(), @@ -2420,8 +2418,7 @@ class X86MCPlusBuilder : public MCPlusBuilder { .addReg(RegNo) .addImm(Imm)); Code.emplace_back(MCInstBuilder(X86::JCC_1) - .addExpr(MCSymbolRefExpr::create( - Target, MCSymbolRefExpr::VK_None, *Ctx)) + .addExpr(MCSymbolRefExpr::create(Target, *Ctx)) .addImm(X86::COND_E)); return Code; } @@ -2432,8 +2429,7 @@ class X86MCPlusBuilder : public MCPlusBuilder { InstructionListType Code; Code.emplace_back(MCInstBuilder(X86::CMP64ri8).addReg(RegNo).addImm(Imm)); Code.emplace_back(MCInstBuilder(X86::JCC_1) - .addExpr(MCSymbolRefExpr::create( - Target, MCSymbolRefExpr::VK_None, *Ctx)) + .addExpr(MCSymbolRefExpr::create(Target, *Ctx)) .addImm(X86::COND_NE)); return Code; } @@ -2447,7 +2443,7 @@ class X86MCPlusBuilder : public MCPlusBuilder { const uint64_t RelOffset = Fixup.getOffset(); uint32_t RelType; - if (FKI.Flags & MCFixupKindInfo::FKF_IsPCRel) { + if (Fixup.isPCRel()) { switch (FKI.TargetSize) { default: return std::nullopt; @@ -2738,24 +2734,23 @@ class X86MCPlusBuilder : public MCPlusBuilder { Inst.clear(); Inst.setOpcode(X86::JMP_1); Inst.clear(); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand(MCOperand::createExpr(MCSymbolRefExpr::create(TBB, *Ctx))); } void createLongUncondBranch(MCInst &Inst, const MCSymbol *Target, MCContext *Ctx) const override { Inst.setOpcode(X86::JMP_4); Inst.clear(); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Target, *Ctx))); } void createCall(MCInst &Inst, const MCSymbol *Target, MCContext *Ctx) override { Inst.setOpcode(X86::CALL64pcrel32); Inst.clear(); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Target, *Ctx))); } void createTailCall(MCInst &Inst, const MCSymbol *Target, @@ -2779,8 +2774,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { MCContext *Ctx) const override { Inst.setOpcode(X86::JCC_1); Inst.clear(); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Target, *Ctx))); Inst.addOperand(MCOperand::createImm(CC)); } @@ -2788,8 +2783,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { MCContext *Ctx) const override { Inst.setOpcode(X86::JCC_4); Inst.clear(); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Target, *Ctx))); Inst.addOperand(MCOperand::createImm(CC)); } @@ -2798,8 +2793,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { unsigned InvCC = getInvertedCondCode(getCondCode(Inst)); assert(InvCC != X86::COND_INVALID && "invalid branch instruction"); Inst.getOperand(Info->get(Inst.getOpcode()).NumOperands - 1).setImm(InvCC); - Inst.getOperand(0) = MCOperand::createExpr( - MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx)); + Inst.getOperand(0) = + MCOperand::createExpr(MCSymbolRefExpr::create(TBB, *Ctx)); } bool replaceBranchCondition(MCInst &Inst, const MCSymbol *TBB, MCContext *Ctx, @@ -2807,8 +2802,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { if (CC == X86::COND_INVALID) return false; Inst.getOperand(Info->get(Inst.getOpcode()).NumOperands - 1).setImm(CC); - Inst.getOperand(0) = MCOperand::createExpr( - MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx)); + Inst.getOperand(0) = + MCOperand::createExpr(MCSymbolRefExpr::create(TBB, *Ctx)); return true; } @@ -2846,8 +2841,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { MCContext *Ctx) const override { assert((isCall(Inst) || isBranch(Inst)) && !isIndirectBranch(Inst) && "Invalid instruction"); - Inst.getOperand(0) = MCOperand::createExpr( - MCSymbolRefExpr::create(TBB, MCSymbolRefExpr::VK_None, *Ctx)); + Inst.getOperand(0) = + MCOperand::createExpr(MCSymbolRefExpr::create(TBB, *Ctx)); } MCPhysReg getX86R11() const override { return X86::R11; } @@ -2894,8 +2889,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { bool IsTailCall) override { Inst.clear(); Inst.setOpcode(IsTailCall ? X86::JMP_4 : X86::CALL64pcrel32); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Target, *Ctx))); if (IsTailCall) setTailCall(Inst); } @@ -2905,8 +2900,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { Seq.clear(); MCInst Inst; Inst.setOpcode(X86::JMP_1); - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Target, MCSymbolRefExpr::VK_None, *Ctx))); + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Target, *Ctx))); if (IsTailCall) setTailCall(Inst); Seq.emplace_back(Inst); @@ -3332,8 +3327,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { Target.addOperand(MCOperand::createReg(FuncAddrReg)); if (Targets[i].first) { // Is this OK? - Target.addOperand(MCOperand::createExpr(MCSymbolRefExpr::create( - Targets[i].first, MCSymbolRefExpr::VK_None, *Ctx))); + Target.addOperand(MCOperand::createExpr( + MCSymbolRefExpr::create(Targets[i].first, *Ctx))); } else { const uint64_t Addr = Targets[i].second; // Immediate address is out of sign extended 32 bit range. @@ -3409,8 +3404,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { Je.clear(); Je.setOpcode(X86::JCC_1); if (Targets[i].first) - Je.addOperand(MCOperand::createExpr(MCSymbolRefExpr::create( - Targets[i].first, MCSymbolRefExpr::VK_None, *Ctx))); + Je.addOperand(MCOperand::createExpr( + MCSymbolRefExpr::create(Targets[i].first, *Ctx))); else Je.addOperand(MCOperand::createImm(Targets[i].second)); @@ -3422,8 +3417,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { // Jump to next compare if target addresses don't match. Jne.clear(); Jne.setOpcode(X86::JCC_1); - Jne.addOperand(MCOperand::createExpr(MCSymbolRefExpr::create( - NextTarget, MCSymbolRefExpr::VK_None, *Ctx))); + Jne.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(NextTarget, *Ctx))); Jne.addOperand(MCOperand::createImm(X86::COND_NE)); // Call specific target directly. @@ -3442,8 +3437,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { CallOrJmp.setOpcode(IsTailCall ? X86::JMP_4 : X86::CALL64pcrel32); if (Targets[i].first) - CallOrJmp.addOperand(MCOperand::createExpr(MCSymbolRefExpr::create( - Targets[i].first, MCSymbolRefExpr::VK_None, *Ctx))); + CallOrJmp.addOperand(MCOperand::createExpr( + MCSymbolRefExpr::create(Targets[i].first, *Ctx))); else CallOrJmp.addOperand(MCOperand::createImm(Targets[i].second)); } @@ -3545,8 +3540,8 @@ class X86MCPlusBuilder : public MCPlusBuilder { // Jump to target if indices match JEInst.setOpcode(X86::JCC_1); - JEInst.addOperand(MCOperand::createExpr(MCSymbolRefExpr::create( - Targets[i].first, MCSymbolRefExpr::VK_None, *Ctx))); + JEInst.addOperand(MCOperand::createExpr( + MCSymbolRefExpr::create(Targets[i].first, *Ctx))); JEInst.addOperand(MCOperand::createImm(X86::COND_E)); } @@ -3571,9 +3566,9 @@ class X86MCPlusBuilder : public MCPlusBuilder { Inst.addOperand(MCOperand::createReg(X86::RIP)); // BaseReg Inst.addOperand(MCOperand::createImm(1)); // ScaleAmt Inst.addOperand(MCOperand::createReg(X86::NoRegister)); // IndexReg - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Src, MCSymbolRefExpr::VK_None, - *Ctx))); // Displacement + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Src, + *Ctx))); // Displacement Inst.addOperand(MCOperand::createReg(X86::NoRegister)); // AddrSegmentReg } @@ -3585,9 +3580,9 @@ class X86MCPlusBuilder : public MCPlusBuilder { Inst.addOperand(MCOperand::createReg(X86::RIP)); // BaseReg Inst.addOperand(MCOperand::createImm(1)); // ScaleAmt Inst.addOperand(MCOperand::createReg(X86::NoRegister)); // IndexReg - Inst.addOperand(MCOperand::createExpr( - MCSymbolRefExpr::create(Src, MCSymbolRefExpr::VK_None, - *Ctx))); // Displacement + Inst.addOperand( + MCOperand::createExpr(MCSymbolRefExpr::create(Src, + *Ctx))); // Displacement Inst.addOperand(MCOperand::createReg(X86::NoRegister)); // AddrSegmentReg } }; diff --git a/bolt/test/X86/zero-density.s b/bolt/test/X86/zero-density.s new file mode 100644 index 0000000000000..7843804ffed8c --- /dev/null +++ b/bolt/test/X86/zero-density.s @@ -0,0 +1,32 @@ +## Check that trampoline functions are excluded from density computation. + +# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o +# RUN: ld.lld %t.o -o %t +# RUN: link_fdata %s %t %t.preagg PREAGG +# RUN: llvm-strip -NLjmp %t +# RUN: perf2bolt %t -p %t.preagg --pa -o %t.fdata | FileCheck %s +# CHECK: Functions with density >= {{.*}} account for 99.00% total sample counts. +# CHECK-NOT: the output profile is empty or the --profile-density-cutoff-hot option is set too low. + + .text + .globl trampoline +trampoline: + mov main,%rax + jmpq *%rax +.size trampoline,.-trampoline +# PREAGG: f #trampoline# #trampoline# 2 + + .globl main +main: + .cfi_startproc + vmovaps %zmm31,%zmm3 + + add $0x4,%r9 + add $0x40,%r10 + dec %r14 +Ljmp: + jne main +# PREAGG: T #Ljmp# #main# #Ljmp# 10 + ret + .cfi_endproc +.size main,.-main diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s index 284f0bea607a5..8e991fade2c86 100644 --- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s +++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s @@ -18,6 +18,10 @@ f1: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: ret @@ -40,6 +44,10 @@ f_intermediate_overwrite1: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: autiasp // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 @@ -63,6 +71,10 @@ f_intermediate_overwrite2: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mov x30, x0 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autiasp @@ -102,6 +114,10 @@ f_intermediate_overwrite3: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mov w30, w0 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autiasp @@ -126,6 +142,10 @@ f_nonx30_ret: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mov x16, x30 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: mov x16, x30 @@ -215,7 +235,7 @@ f_callclobbered_calleesaved: .globl f_unreachable_instruction .type f_unreachable_instruction,@function f_unreachable_instruction: -// CHECK-LABEL: GS-PAUTH: Warning: unreachable instruction found in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address +// CHECK-LABEL: GS-PAUTH: Warning: possibly imprecise CFG, the analysis quality may be degraded in this function. According to BOLT, unreachable code is found in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: add x0, x1, x2 // CHECK-NOT: instructions that write to the affected registers after any authentication are: b 1f @@ -224,20 +244,33 @@ f_unreachable_instruction: ret .size f_unreachable_instruction, .-f_unreachable_instruction -// Expected false positive: without CFG, the state is reset to all-unsafe -// after an unconditional branch. +// Without CFG, the state is reset at labels, assuming every register that can +// be clobbered in the function was actually clobbered. - .globl state_is_reset_after_indirect_branch_nocfg - .type state_is_reset_after_indirect_branch_nocfg,@function -state_is_reset_after_indirect_branch_nocfg: -// CHECK-LABEL: GS-PAUTH: non-protected ret found in function state_is_reset_after_indirect_branch_nocfg, at address -// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret + .globl lr_untouched_nocfg + .type lr_untouched_nocfg,@function +lr_untouched_nocfg: +// CHECK-NOT: lr_untouched_nocfg + adr x2, 1f + br x2 +1: + ret + .size lr_untouched_nocfg, .-lr_untouched_nocfg + + .globl lr_clobbered_nocfg + .type lr_clobbered_nocfg,@function +lr_clobbered_nocfg: +// CHECK-LABEL: GS-PAUTH: non-protected ret found in function lr_clobbered_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: ret // CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: adr x2, 1f br x2 1: + b 2f + bl g // never executed, but affects the expected worst-case scenario +2: ret - .size state_is_reset_after_indirect_branch_nocfg, .-state_is_reset_after_indirect_branch_nocfg + .size lr_clobbered_nocfg, .-lr_clobbered_nocfg /// Now do a basic sanity check on every different Authentication instruction: @@ -312,6 +345,10 @@ f_autia1716: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autia1716 @@ -334,6 +371,10 @@ f_autib1716: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autib1716 @@ -356,6 +397,10 @@ f_autiax12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autia x12, sp @@ -378,6 +423,10 @@ f_autibx12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autib x12, sp @@ -429,6 +478,10 @@ f_autdax12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autda x12, sp @@ -451,6 +504,10 @@ f_autdbx12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autdb x12, sp @@ -502,6 +559,10 @@ f_autizax12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autiza x12 @@ -524,6 +585,10 @@ f_autizbx12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autizb x12 @@ -575,6 +640,10 @@ f_autdzax12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autdza x12 @@ -597,6 +666,10 @@ f_autdzbx12: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autdzb x12 @@ -855,6 +928,10 @@ f_autia171615: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autia171615 @@ -877,6 +954,10 @@ f_autib171615: // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: mov x29, sp +// CHECK-NEXT: {{[0-9a-f]+}}: bl g // CHECK-NEXT: {{[0-9a-f]+}}: add x0, x0, #0x3 // CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 // CHECK-NEXT: {{[0-9a-f]+}}: autib171615 @@ -884,3 +965,9 @@ f_autib171615: ret .size f_autib171615, .-f_autib171615 + .globl g + .type g,@function +g: + nop + ret + .size g, .-g diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s b/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s index 717bf40df3d02..c314bc7cfe5a3 100644 --- a/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s +++ b/bolt/test/binary-analysis/AArch64/gs-pauth-authentication-oracles.s @@ -491,10 +491,6 @@ good_address_arith_multi_bb: ret .size good_address_arith_multi_bb, .-good_address_arith_multi_bb -// FIXME: Most *_nocfg test cases contain paciasp+autiasp instructions even if -// LR is not spilled - this is a workaround for RET instructions being -// reported as non-protected, because LR state is reset at every label. - .globl good_ret_nocfg .type good_ret_nocfg,@function good_ret_nocfg: @@ -541,14 +537,12 @@ good_branch_nocfg: .type good_load_other_reg_nocfg,@function good_load_other_reg_nocfg: // CHECK-NOT: good_load_other_reg_nocfg - paciasp adr x2, 1f br x2 1: autia x0, x1 ldr x2, [x0] - autiasp ret .size good_load_other_reg_nocfg, .-good_load_other_reg_nocfg @@ -556,14 +550,12 @@ good_load_other_reg_nocfg: .type good_load_same_reg_nocfg,@function good_load_same_reg_nocfg: // CHECK-NOT: good_load_same_reg_nocfg - paciasp adr x2, 1f br x2 1: autia x0, x1 ldr x0, [x0] - autiasp ret .size good_load_same_reg_nocfg, .-good_load_same_reg_nocfg @@ -575,13 +567,11 @@ bad_unchecked_nocfg: // CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_unchecked_nocfg, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1 // CHECK-NEXT: The 0 instructions that leak the affected registers are: - paciasp adr x2, 1f br x2 1: autia x0, x1 - autiasp ret .size bad_unchecked_nocfg, .-bad_unchecked_nocfg @@ -615,7 +605,6 @@ bad_unknown_usage_read_nocfg: // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1 // CHECK-NEXT: The 1 instructions that leak the affected registers are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mul x3, x0, x1 - paciasp adr x2, 1f br x2 1: @@ -623,7 +612,6 @@ bad_unknown_usage_read_nocfg: mul x3, x0, x1 ldr x2, [x0] - autiasp ret .size bad_unknown_usage_read_nocfg, .-bad_unknown_usage_read_nocfg @@ -634,7 +622,6 @@ bad_unknown_usage_subreg_read_nocfg: // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1 // CHECK-NEXT: The 1 instructions that leak the affected registers are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mul w3, w0, w1 - paciasp adr x2, 1f br x2 1: @@ -642,7 +629,6 @@ bad_unknown_usage_subreg_read_nocfg: mul w3, w0, w1 ldr x2, [x0] - autiasp ret .size bad_unknown_usage_subreg_read_nocfg, .-bad_unknown_usage_subreg_read_nocfg @@ -653,7 +639,6 @@ bad_unknown_usage_update_nocfg: // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1 // CHECK-NEXT: The 1 instructions that leak the affected registers are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: movk x0, #0x2a, lsl #16 - paciasp adr x2, 1f br x2 1: @@ -661,7 +646,6 @@ bad_unknown_usage_update_nocfg: movk x0, #42, lsl #16 // does not overwrite x0 completely ldr x2, [x0] - autiasp ret .size bad_unknown_usage_update_nocfg, .-bad_unknown_usage_update_nocfg @@ -669,14 +653,12 @@ bad_unknown_usage_update_nocfg: .type good_overwrite_with_constant_nocfg,@function good_overwrite_with_constant_nocfg: // CHECK-NOT: good_overwrite_with_constant_nocfg - paciasp adr x2, 1f br x2 1: autia x0, x1 mov x0, #42 - autiasp ret .size good_overwrite_with_constant_nocfg, .-good_overwrite_with_constant_nocfg @@ -684,7 +666,6 @@ good_overwrite_with_constant_nocfg: .type good_address_arith_nocfg,@function good_address_arith_nocfg: // CHECK-NOT: good_address_arith_nocfg - paciasp adr x2, 1f br x2 1: @@ -698,7 +679,6 @@ good_address_arith_nocfg: mov x1, #0 mov x2, #0 - autiasp ret .size good_address_arith_nocfg, .-good_address_arith_nocfg diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s b/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s index c79c5926a05cd..fb0bc7cff2377 100644 --- a/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s +++ b/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s @@ -1428,6 +1428,90 @@ printed_instrs_nocfg: br x0 .size printed_instrs_nocfg, .-printed_instrs_nocfg +// Test handling of unreachable basic blocks. +// +// Basic blocks without any predecessors were observed in real-world optimized +// code. At least sometimes they were actually reachable via jump table, which +// was not detected, but the function was processed as if its CFG was +// reconstructed successfully. +// +// As a more predictable model example, let's use really unreachable code +// for testing. + + .globl bad_unreachable_call + .type bad_unreachable_call,@function +bad_unreachable_call: +// CHECK-LABEL: GS-PAUTH: Warning: possibly imprecise CFG, the analysis quality may be degraded in this function. According to BOLT, unreachable code is found in function bad_unreachable_call, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: blr x0 +// CHECK-NOT: instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: non-protected call found in function bad_unreachable_call, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: blr x0 +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: + paciasp + stp x29, x30, [sp, #-16]! + mov x29, sp + + b 1f + // unreachable basic block: + blr x0 + +1: // reachable basic block: + ldp x29, x30, [sp], #16 + autiasp + ret + .size bad_unreachable_call, .-bad_unreachable_call + + .globl good_unreachable_call + .type good_unreachable_call,@function +good_unreachable_call: +// CHECK-NOT: non-protected call{{.*}}good_unreachable_call +// CHECK-LABEL: GS-PAUTH: Warning: possibly imprecise CFG, the analysis quality may be degraded in this function. According to BOLT, unreachable code is found in function good_unreachable_call, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autia x0, x1 +// CHECK-NOT: instructions that write to the affected registers after any authentication are: +// CHECK-NOT: non-protected call{{.*}}good_unreachable_call + paciasp + stp x29, x30, [sp, #-16]! + mov x29, sp + + b 1f + // unreachable basic block: + autia x0, x1 + blr x0 // <-- this call is definitely protected provided at least + // basic block boundaries are detected correctly + +1: // reachable basic block: + ldp x29, x30, [sp], #16 + autiasp + ret + .size good_unreachable_call, .-good_unreachable_call + + .globl unreachable_loop_of_bbs + .type unreachable_loop_of_bbs,@function +unreachable_loop_of_bbs: +// CHECK-NOT: unreachable basic blocks{{.*}}unreachable_loop_of_bbs +// CHECK-NOT: non-protected call{{.*}}unreachable_loop_of_bbs +// CHECK-LABEL: GS-PAUTH: Warning: possibly imprecise CFG, the analysis quality may be degraded in this function. According to BOLT, unreachable code is found in function unreachable_loop_of_bbs, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: blr x0 +// CHECK-NOT: unreachable basic blocks{{.*}}unreachable_loop_of_bbs +// CHECK-NOT: non-protected call{{.*}}unreachable_loop_of_bbs + paciasp + stp x29, x30, [sp, #-16]! + mov x29, sp + b .Lreachable_epilogue_bb + +.Lfirst_unreachable_bb: + blr x0 // <-- this call is not analyzed + b .Lsecond_unreachable_bb +.Lsecond_unreachable_bb: + blr x1 // <-- this call is not analyzed + b .Lfirst_unreachable_bb + +.Lreachable_epilogue_bb: + ldp x29, x30, [sp], #16 + autiasp + ret + .size unreachable_loop_of_bbs, .-unreachable_loop_of_bbs + .globl main .type main,@function main: diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s b/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s index fbb96a63d41ed..b1cec7f92ad05 100644 --- a/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s +++ b/bolt/test/binary-analysis/AArch64/gs-pauth-debug-output.s @@ -199,8 +199,8 @@ nocfg: // CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( br x0, src-state) // CHECK-NEXT: .. result: (src-state) // CHECK-NEXT: Due to label, resetting the state before: 00000000: ret # Offset: 8 -// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state) -// CHECK-NEXT: .. result: (src-state) +// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state) +// CHECK-NEXT: .. result: (src-state) // CHECK-NEXT: After src register safety analysis: // CHECK-NEXT: Binary Function "nocfg" { // CHECK-NEXT: Number : 3 @@ -223,33 +223,7 @@ nocfg: // PAUTH-NEXT: SafeToDerefRegs: LR W0 W30 X0 W0_HI W30_HI{{[ \t]*$}} // CHECK-NEXT: Found RET inst: 00000000: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state // CHECK-NEXT: RetReg: LR -// CHECK-NEXT: SafeToDerefRegs:{{[ \t]*$}} -// CHECK-EMPTY: -// CHECK-NEXT: Running detailed src register safety analysis... -// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( adr x0, __ENTRY_nocfg@0x[[ENTRY_ADDR]], src-state) -// CHECK-NEXT: .. result: (src-state) -// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( br x0, src-state) -// CHECK-NEXT: .. result: (src-state) -// CHECK-NEXT: Due to label, resetting the state before: 00000000: ret # Offset: 8 -// CHECK-NEXT: SrcSafetyAnalysis::ComputeNext( ret x30, src-state) -// CHECK-NEXT: .. result: (src-state) -// CHECK-NEXT: After detailed src register safety analysis: -// CHECK-NEXT: Binary Function "nocfg" { -// CHECK-NEXT: Number : 3 -// ... -// CHECK: Secondary Entry Points : __ENTRY_nocfg@0x[[ENTRY_ADDR]] -// CHECK-NEXT: } -// CHECK-NEXT: .{{[A-Za-z0-9]+}}: -// CHECK-NEXT: 00000000: adr x0, __ENTRY_nocfg@0x[[ENTRY_ADDR]] # CFGUnawareSrcSafetyAnalysis: src-state -// CHECK-NEXT: 00000004: br x0 # UNKNOWN CONTROL FLOW # Offset: 4 # CFGUnawareSrcSafetyAnalysis: src-state -// CHECK-NEXT: __ENTRY_nocfg@0x[[ENTRY_ADDR]] (Entry Point): -// CHECK-NEXT: .{{[A-Za-z0-9]+}}: -// CHECK-NEXT: 00000008: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state -// CHECK-NEXT: DWARF CFI Instructions: -// CHECK-NEXT: -// CHECK-NEXT: End of Function "nocfg" -// CHECK-EMPTY: -// CHECK-NEXT: Attaching clobbering info to: 00000000: ret # Offset: 8 # CFGUnawareSrcSafetyAnalysis: src-state +// CHECK-NEXT: SafeToDerefRegs: LR W30 W30_HI{{[ \t]*$}} .globl auth_oracle .type auth_oracle,@function diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s b/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s index 334a4108d8ab8..3a4d383ec5bc6 100644 --- a/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s +++ b/bolt/test/binary-analysis/AArch64/gs-pauth-signing-oracles.s @@ -505,21 +505,16 @@ bad_one_auted_one_checked_multi_bb: // * untrusted: not even s-t-d - from arg and from memory // * untrusted: subreg clobbered - between address materialization and use, between auth and check, between check and use // * untrusted: first checked then auted, auted then auted, checked then checked -// -// Note that it is important to sign and authenticate LR, as it is not kept -// safe-to-dereference across unconditional branches. .globl good_sign_addr_mat_nocfg .type good_sign_addr_mat_nocfg,@function good_sign_addr_mat_nocfg: // CHECK-NOT: good_sign_addr_mat_nocfg - paciasp adr x3, 1f br x3 1: adr x0, sym pacda x0, x1 - autiasp ret .size good_sign_addr_mat_nocfg, .-good_sign_addr_mat_nocfg @@ -527,14 +522,12 @@ good_sign_addr_mat_nocfg: .type good_sign_auted_checked_ldr_nocfg,@function good_sign_auted_checked_ldr_nocfg: // CHECK-NOT: good_sign_auted_checked_ldr_nocfg - paciasp adr x3, 1f br x3 1: autda x0, x2 ldr x2, [x0] pacda x0, x1 - autiasp ret .size good_sign_auted_checked_ldr_nocfg, .-good_sign_auted_checked_ldr_nocfg @@ -544,13 +537,11 @@ bad_sign_authed_unchecked_nocfg: // CHECK-LABEL: GS-PAUTH: signing oracle found in function bad_sign_authed_unchecked_nocfg, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: - paciasp adr x3, 1f br x3 1: autda x0, x2 pacda x0, x1 - autiasp ret .size bad_sign_authed_unchecked_nocfg, .-bad_sign_authed_unchecked_nocfg @@ -560,13 +551,11 @@ bad_sign_checked_not_auted_nocfg: // CHECK-LABEL: GS-PAUTH: signing oracle found in function bad_sign_checked_not_auted_nocfg, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: - paciasp adr x3, 1f br x3 1: ldr x2, [x0] pacda x0, x1 - autiasp ret .size bad_sign_checked_not_auted_nocfg, .-bad_sign_checked_not_auted_nocfg @@ -576,12 +565,10 @@ bad_sign_plain_arg_nocfg: // CHECK-LABEL: GS-PAUTH: signing oracle found in function bad_sign_plain_arg_nocfg, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: - paciasp adr x3, 1f br x3 1: pacda x0, x1 - autiasp ret .size bad_sign_plain_arg_nocfg, .-bad_sign_plain_arg_nocfg @@ -592,13 +579,11 @@ bad_sign_plain_mem_nocfg: // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: ldr x0, [x1] - paciasp adr x3, 1f br x3 1: ldr x0, [x1] pacda x0, x1 - autiasp ret .size bad_sign_plain_mem_nocfg, .-bad_sign_plain_mem_nocfg @@ -609,14 +594,12 @@ bad_clobber_between_addr_mat_and_use_nocfg: // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mov w0, w4 - paciasp adr x3, 1f br x3 1: adr x0, sym mov w0, w4 pacda x0, x1 - autiasp ret .size bad_clobber_between_addr_mat_and_use_nocfg, .-bad_clobber_between_addr_mat_and_use_nocfg @@ -627,7 +610,6 @@ bad_clobber_between_auted_and_checked_nocfg: // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mov w0, w4 - paciasp adr x3, 1f br x3 1: @@ -635,7 +617,6 @@ bad_clobber_between_auted_and_checked_nocfg: mov w0, w4 ldr x2, [x0] pacda x0, x1 - autiasp ret .size bad_clobber_between_auted_and_checked_nocfg, .-bad_clobber_between_auted_and_checked_nocfg @@ -646,7 +627,6 @@ bad_clobber_between_checked_and_used_nocfg: // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: // CHECK-NEXT: 1. {{[0-9a-f]+}}: mov w0, w4 - paciasp adr x3, 1f br x3 1: @@ -654,7 +634,6 @@ bad_clobber_between_checked_and_used_nocfg: ldr x2, [x0] mov w0, w4 pacda x0, x1 - autiasp ret .size bad_clobber_between_checked_and_used_nocfg, .-bad_clobber_between_checked_and_used_nocfg @@ -664,14 +643,12 @@ bad_transition_check_then_auth_nocfg: // CHECK-LABEL: GS-PAUTH: signing oracle found in function bad_transition_check_then_auth_nocfg, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: - paciasp adr x3, 1f br x3 1: ldr x2, [x0] autda x0, x2 pacda x0, x1 - autiasp ret .size bad_transition_check_then_auth_nocfg, .-bad_transition_check_then_auth_nocfg @@ -681,14 +658,12 @@ bad_transition_auth_then_auth_nocfg: // CHECK-LABEL: GS-PAUTH: signing oracle found in function bad_transition_auth_then_auth_nocfg, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: - paciasp adr x3, 1f br x3 1: autda x0, x2 autda x0, x2 pacda x0, x1 - autiasp ret .size bad_transition_auth_then_auth_nocfg, .-bad_transition_auth_then_auth_nocfg @@ -698,14 +673,12 @@ bad_transition_check_then_check_nocfg: // CHECK-LABEL: GS-PAUTH: signing oracle found in function bad_transition_check_then_check_nocfg, at address // CHECK-NEXT: The instruction is {{[0-9a-f]+}}: pacda x0, x1 // CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: - paciasp adr x3, 1f br x3 1: ldr x2, [x0] ldr x2, [x0] pacda x0, x1 - autiasp ret .size bad_transition_check_then_check_nocfg, .-bad_transition_check_then_check_nocfg diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-tail-calls.s b/bolt/test/binary-analysis/AArch64/gs-pauth-tail-calls.s new file mode 100644 index 0000000000000..2d3c2f1a632ca --- /dev/null +++ b/bolt/test/binary-analysis/AArch64/gs-pauth-tail-calls.s @@ -0,0 +1,597 @@ +// RUN: %clang %cflags -Wl,--entry=_custom_start -march=armv8.3-a %s -o %t.exe +// RUN: llvm-bolt-binary-analysis --scanners=pacret %t.exe 2>&1 | FileCheck -check-prefix=PACRET %s +// RUN: llvm-bolt-binary-analysis --scanners=pauth %t.exe 2>&1 | FileCheck %s + +// PACRET-NOT: untrusted link register found before tail call + + .text + + .globl callee + .type callee,@function +callee: + ret + .size callee, .-callee + + .globl good_direct_tailcall_no_clobber + .type good_direct_tailcall_no_clobber,@function +good_direct_tailcall_no_clobber: +// CHECK-NOT: good_direct_tailcall_no_clobber + b callee + .size good_direct_tailcall_no_clobber, .-good_direct_tailcall_no_clobber + + .globl good_plt_tailcall_no_clobber + .type good_plt_tailcall_no_clobber,@function +good_plt_tailcall_no_clobber: +// CHECK-NOT: good_plt_tailcall_no_clobber + b callee_ext + .size good_plt_tailcall_no_clobber, .-good_plt_tailcall_no_clobber + + .globl good_indirect_tailcall_no_clobber + .type good_indirect_tailcall_no_clobber,@function +good_indirect_tailcall_no_clobber: +// CHECK-NOT: good_indirect_tailcall_no_clobber + autia x0, x1 + br x0 + .size good_indirect_tailcall_no_clobber, .-good_indirect_tailcall_no_clobber + + .globl bad_direct_tailcall_not_auted + .type bad_direct_tailcall_not_auted,@function +bad_direct_tailcall_not_auted: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_direct_tailcall_not_auted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: b callee # TAILCALL + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + b callee + .size bad_direct_tailcall_not_auted, .-bad_direct_tailcall_not_auted + + .globl bad_plt_tailcall_not_auted + .type bad_plt_tailcall_not_auted,@function +bad_plt_tailcall_not_auted: +// FIXME: Calls via PLT are disassembled incorrectly. Nevertheless, they are +// still detected as tail calls. +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_plt_tailcall_not_auted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b bad_indirect_tailcall_not_auted # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: b bad_indirect_tailcall_not_auted # TAILCALL + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + b callee_ext + .size bad_plt_tailcall_not_auted, .-bad_plt_tailcall_not_auted + + .globl bad_indirect_tailcall_not_auted + .type bad_indirect_tailcall_not_auted,@function +bad_indirect_tailcall_not_auted: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_indirect_tailcall_not_auted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: br x0 # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: autia x0, x1 +// CHECK-NEXT: {{[0-9a-f]+}}: br x0 # TAILCALL + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autia x0, x1 + br x0 + .size bad_indirect_tailcall_not_auted, .-bad_indirect_tailcall_not_auted + + .globl bad_direct_tailcall_untrusted + .type bad_direct_tailcall_untrusted,@function +bad_direct_tailcall_untrusted: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_direct_tailcall_untrusted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_direct_tailcall_untrusted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 1 instructions that leak the affected registers are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: {{[0-9a-f]+}}: b callee # TAILCALL + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + b callee + .size bad_direct_tailcall_untrusted, .-bad_direct_tailcall_untrusted + + .globl bad_plt_tailcall_untrusted + .type bad_plt_tailcall_untrusted,@function +bad_plt_tailcall_untrusted: +// FIXME: Calls via PLT are disassembled incorrectly. Nevertheless, they are +// still detected as tail calls. +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_plt_tailcall_untrusted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b bad_indirect_tailcall_untrusted # TAILCALL +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_plt_tailcall_untrusted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 1 instructions that leak the affected registers are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: b bad_indirect_tailcall_untrusted # TAILCALL +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: {{[0-9a-f]+}}: b bad_indirect_tailcall_untrusted # TAILCALL + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + b callee_ext + .size bad_plt_tailcall_untrusted, .-bad_plt_tailcall_untrusted + + .globl bad_indirect_tailcall_untrusted + .type bad_indirect_tailcall_untrusted,@function +bad_indirect_tailcall_untrusted: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_indirect_tailcall_untrusted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: br x0 # TAILCALL +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_indirect_tailcall_untrusted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 1 instructions that leak the affected registers are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: br x0 # TAILCALL +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: {{[0-9a-f]+}}: autia x0, x1 +// CHECK-NEXT: {{[0-9a-f]+}}: br x0 # TAILCALL + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + autia x0, x1 + br x0 + .size bad_indirect_tailcall_untrusted, .-bad_indirect_tailcall_untrusted + + .globl good_direct_tailcall_trusted + .type good_direct_tailcall_trusted,@function +good_direct_tailcall_trusted: +// CHECK-NOT: good_direct_tailcall_trusted + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + b callee + .size good_direct_tailcall_trusted, .-good_direct_tailcall_trusted + + .globl good_plt_tailcall_trusted + .type good_plt_tailcall_trusted,@function +good_plt_tailcall_trusted: +// CHECK-NOT: good_plt_tailcall_trusted + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + b callee_ext + .size good_plt_tailcall_trusted, .-good_plt_tailcall_trusted + + .globl good_indirect_tailcall_trusted + .type good_indirect_tailcall_trusted,@function +good_indirect_tailcall_trusted: +// CHECK-NOT: good_indirect_tailcall_trusted + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + autia x0, x1 + br x0 + .size good_indirect_tailcall_trusted, .-good_indirect_tailcall_trusted + + .globl good_direct_tailcall_no_clobber_multi_bb + .type good_direct_tailcall_no_clobber_multi_bb,@function +good_direct_tailcall_no_clobber_multi_bb: +// CHECK-NOT: good_direct_tailcall_no_clobber_multi_bb + b 1f +1: + b callee + .size good_direct_tailcall_no_clobber_multi_bb, .-good_direct_tailcall_no_clobber_multi_bb + + .globl good_indirect_tailcall_no_clobber_multi_bb + .type good_indirect_tailcall_no_clobber_multi_bb,@function +good_indirect_tailcall_no_clobber_multi_bb: +// CHECK-NOT: good_indirect_tailcall_no_clobber_multi_bb + autia x0, x1 + b 1f +1: + br x0 + .size good_indirect_tailcall_no_clobber_multi_bb_multi_bb, .-good_indirect_tailcall_no_clobber_multi_bb_multi_bb + + .globl bad_direct_tailcall_not_auted_multi_bb + .type bad_direct_tailcall_not_auted_multi_bb,@function +bad_direct_tailcall_not_auted_multi_bb: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_direct_tailcall_not_auted_multi_bb, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + cbz x3, 1f + autiasp + ldr w2, [x30] +1: + b callee + .size bad_direct_tailcall_not_auted_multi_bb, .-bad_direct_tailcall_not_auted_multi_bb + + .globl bad_indirect_tailcall_not_auted_multi_bb + .type bad_indirect_tailcall_not_auted_multi_bb,@function +bad_indirect_tailcall_not_auted_multi_bb: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_indirect_tailcall_not_auted_multi_bb, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: br x0 # UNKNOWN CONTROL FLOW +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + cbz x3, 1f + autiasp + ldr w2, [x30] +1: + autia x0, x1 + br x0 + .size bad_indirect_tailcall_not_auted_multi_bb, .-bad_indirect_tailcall_not_auted_multi_bb + + .globl bad_direct_tailcall_untrusted_multi_bb + .type bad_direct_tailcall_untrusted_multi_bb,@function +bad_direct_tailcall_untrusted_multi_bb: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_direct_tailcall_untrusted_multi_bb, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_direct_tailcall_untrusted_multi_bb, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 1 instructions that leak the affected registers are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: b callee # TAILCALL + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + cbz x3, 1f + ldr w2, [x30] +1: + b callee + .size bad_direct_tailcall_untrusted_multi_bb, .-bad_direct_tailcall_untrusted_multi_bb + + .globl bad_indirect_tailcall_untrusted_multi_bb + .type bad_indirect_tailcall_untrusted_multi_bb,@function +bad_indirect_tailcall_untrusted_multi_bb: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_indirect_tailcall_untrusted_multi_bb, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: br x0 # UNKNOWN CONTROL FLOW +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_indirect_tailcall_untrusted_multi_bb, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 0 instructions that leak the affected registers are: + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + cbz x3, 1f + ldr w2, [x30] +1: + autia x0, x1 + br x0 + .size bad_indirect_tailcall_untrusted_multi_bb, .-bad_indirect_tailcall_untrusted_multi_bb + + .globl good_direct_tailcall_trusted_multi_bb + .type good_direct_tailcall_trusted_multi_bb,@function +good_direct_tailcall_trusted_multi_bb: +// CHECK-NOT: good_direct_tailcall_trusted_multi_bb + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + b 1f +1: + b callee + .size good_direct_tailcall_trusted_multi_bb, .-good_direct_tailcall_trusted_multi_bb + + .globl good_indirect_tailcall_trusted_multi_bb + .type good_indirect_tailcall_trusted_multi_bb,@function +good_indirect_tailcall_trusted_multi_bb: +// CHECK-NOT: good_indirect_tailcall_trusted_multi_bb + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + b 1f +1: + autia x0, x1 + br x0 + .size good_indirect_tailcall_trusted_multi_bb, .-good_indirect_tailcall_trusted_multi_bb + + .globl good_direct_tailcall_no_clobber_nocfg + .type good_direct_tailcall_no_clobber_nocfg,@function +good_direct_tailcall_no_clobber_nocfg: +// CHECK-NOT: good_direct_tailcall_no_clobber_nocfg + adr x3, 1f + br x3 +1: + b callee + .size good_direct_tailcall_no_clobber_nocfg, .-good_direct_tailcall_no_clobber_nocfg + + .globl good_plt_tailcall_no_clobber_nocfg + .type good_plt_tailcall_no_clobber_nocfg,@function +good_plt_tailcall_no_clobber_nocfg: +// CHECK-NOT: good_plt_tailcall_no_clobber_nocfg + adr x3, 1f + br x3 +1: + b callee_ext + .size good_plt_tailcall_no_clobber_nocfg, .-good_plt_tailcall_no_clobber_nocfg + + .globl good_indirect_tailcall_no_clobber_nocfg + .type good_indirect_tailcall_no_clobber_nocfg,@function +good_indirect_tailcall_no_clobber_nocfg: +// CHECK-NOT: good_indirect_tailcall_no_clobber_nocfg + adr x3, 1f + br x3 +1: + autia x0, x1 + br x0 + .size good_indirect_tailcall_no_clobber_nocfg, .-good_indirect_tailcall_no_clobber_nocfg + + .globl bad_direct_tailcall_not_auted_nocfg + .type bad_direct_tailcall_not_auted_nocfg,@function +bad_direct_tailcall_not_auted_nocfg: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_direct_tailcall_not_auted_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + b callee + .size bad_direct_tailcall_not_auted_nocfg, .-bad_direct_tailcall_not_auted_nocfg + + .globl bad_plt_tailcall_not_auted_nocfg + .type bad_plt_tailcall_not_auted_nocfg,@function +bad_plt_tailcall_not_auted_nocfg: +// FIXME: Calls via PLT are disassembled incorrectly. Nevertheless, they are +// still detected as tail calls. +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_plt_tailcall_not_auted_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b bad_indirect_tailcall_not_auted_nocfg # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + b callee_ext + .size bad_plt_tailcall_not_auted_nocfg, .-bad_plt_tailcall_not_auted_nocfg + + .globl bad_indirect_tailcall_not_auted_nocfg + .type bad_indirect_tailcall_not_auted_nocfg,@function +bad_indirect_tailcall_not_auted_nocfg: +// Known false positive: ignoring UNKNOWN CONTROL FLOW without CFG. +// CHECK-NOT: bad_indirect_tailcall_not_auted_nocfg + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + autia x0, x1 + br x0 + .size bad_indirect_tailcall_not_auted_nocfg, .-bad_indirect_tailcall_not_auted_nocfg + + .globl bad_direct_tailcall_untrusted_nocfg + .type bad_direct_tailcall_untrusted_nocfg,@function +bad_direct_tailcall_untrusted_nocfg: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_direct_tailcall_untrusted_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_direct_tailcall_untrusted_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 1 instructions that leak the affected registers are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: b callee # TAILCALL + paciasp + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + autiasp + b callee + .size bad_direct_tailcall_untrusted_nocfg, .-bad_direct_tailcall_untrusted_nocfg + + .globl bad_plt_tailcall_untrusted_nocfg + .type bad_plt_tailcall_untrusted_nocfg,@function +bad_plt_tailcall_untrusted_nocfg: +// FIXME: Calls via PLT are disassembled incorrectly. Nevertheless, they are +// still detected as tail calls. +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_plt_tailcall_untrusted_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b bad_indirect_tailcall_untrusted_nocfg # TAILCALL +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_plt_tailcall_untrusted_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 1 instructions that leak the affected registers are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: b bad_indirect_tailcall_untrusted_nocfg # TAILCALL + paciasp + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + autiasp + b callee_ext + .size bad_plt_tailcall_untrusted_nocfg, .-bad_plt_tailcall_untrusted_nocfg + + .globl bad_indirect_tailcall_untrusted_nocfg + .type bad_indirect_tailcall_untrusted_nocfg,@function +bad_indirect_tailcall_untrusted_nocfg: +// Known false negative: ignoring UNKNOWN CONTROL FLOW without CFG. +// Authentication oracle is found by a generic checker, though. +// CHECK-NOT: untrusted link register{{.*}}bad_indirect_tailcall_untrusted_nocfg +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_indirect_tailcall_untrusted_nocfg, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 0 instructions that leak the affected registers are: +// CHECK-NOT: untrusted link register{{.*}}bad_indirect_tailcall_untrusted_nocfg + paciasp + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + autiasp + autia x0, x1 + br x0 + .size bad_indirect_tailcall_untrusted_nocfg, .-bad_indirect_tailcall_untrusted_nocfg + + .globl good_direct_tailcall_trusted_nocfg + .type good_direct_tailcall_trusted_nocfg,@function +good_direct_tailcall_trusted_nocfg: +// CHECK-NOT: good_direct_tailcall_trusted_nocfg + paciasp + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + b callee + .size good_direct_tailcall_trusted_nocfg, .-good_direct_tailcall_trusted_nocfg + + .globl good_plt_tailcall_trusted_nocfg + .type good_plt_tailcall_trusted_nocfg,@function +good_plt_tailcall_trusted_nocfg: +// CHECK-NOT: good_plt_tailcall_trusted_nocfg + paciasp + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + b callee_ext + .size good_plt_tailcall_trusted_nocfg, .-good_plt_tailcall_trusted_nocfg + + .globl good_indirect_tailcall_trusted_nocfg + .type good_indirect_tailcall_trusted_nocfg,@function +good_indirect_tailcall_trusted_nocfg: +// CHECK-NOT: good_indirect_tailcall_trusted_nocfg + paciasp + stp x29, x30, [sp, #-0x10]! + adr x3, 1f + br x3 +1: + ldp x29, x30, [sp], #0x10 + autiasp + ldr w2, [x30] + autia x0, x1 + br x0 + .size good_indirect_tailcall_trusted_nocfg, .-good_indirect_tailcall_trusted_nocfg + +// Check Armv8.3-a fused auth+branch instructions. + + .globl good_indirect_tailcall_no_clobber_v83 + .type good_indirect_tailcall_no_clobber_v83,@function +good_indirect_tailcall_no_clobber_v83: +// CHECK-NOT: good_indirect_tailcall_no_clobber_v83 + braa x0, x1 + .size good_indirect_tailcall_no_clobber_v83, .-good_indirect_tailcall_no_clobber_v83 + + .globl bad_indirect_tailcall_untrusted_v83 + .type bad_indirect_tailcall_untrusted_v83,@function +bad_indirect_tailcall_untrusted_v83: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_indirect_tailcall_untrusted_v83, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: braa x0, x1 # TAILCALL +// CHECK-NEXT: The 0 instructions that write to the affected registers after any authentication are: +// CHECK-LABEL: GS-PAUTH: authentication oracle found in function bad_indirect_tailcall_untrusted_v83, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: The 1 instructions that leak the affected registers are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: braa x0, x1 # TAILCALL +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: paciasp +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: autiasp +// CHECK-NEXT: {{[0-9a-f]+}}: braa x0, x1 # TAILCALL + paciasp + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + autiasp + braa x0, x1 + .size bad_indirect_tailcall_untrusted_v83, .-bad_indirect_tailcall_untrusted_v83 + +// Make sure ELF entry function does not generate false positive reports. +// Additionally, check that the correct entry point is read from ELF header. + + .globl _start + .type _start,@function +_start: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function _start, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: b callee # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: mov x30, #0x0 +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: mov x30, #0x0 +// CHECK-NEXT: {{[0-9a-f]+}}: b callee # TAILCALL + mov x30, #0 + b callee + .size _start, .-_start + + .globl _custom_start + .type _custom_start,@function +_custom_start: +// CHECK-NOT: _custom_start + mov x30, #0 + b callee + .size _custom_start, .-_custom_start + +// Test two issues being reported for the same instruction. + + .globl bad_non_protected_indirect_tailcall_not_auted + .type bad_non_protected_indirect_tailcall_not_auted,@function +bad_non_protected_indirect_tailcall_not_auted: +// CHECK-LABEL: GS-PAUTH: untrusted link register found before tail call in function bad_non_protected_indirect_tailcall_not_auted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: br x0 # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: ldr x0, [x1] +// CHECK-NEXT: {{[0-9a-f]+}}: br x0 # TAILCALL +// CHECK-LABEL: GS-PAUTH: non-protected call found in function bad_non_protected_indirect_tailcall_not_auted, basic block {{[^,]+}}, at address +// CHECK-NEXT: The instruction is {{[0-9a-f]+}}: br x0 # TAILCALL +// CHECK-NEXT: The 1 instructions that write to the affected registers after any authentication are: +// CHECK-NEXT: 1. {{[0-9a-f]+}}: ldr x0, [x1] +// CHECK-NEXT: This happens in the following basic block: +// CHECK-NEXT: {{[0-9a-f]+}}: stp x29, x30, [sp, #-0x10]! +// CHECK-NEXT: {{[0-9a-f]+}}: ldp x29, x30, [sp], #0x10 +// CHECK-NEXT: {{[0-9a-f]+}}: ldr x0, [x1] +// CHECK-NEXT: {{[0-9a-f]+}}: br x0 # TAILCALL + stp x29, x30, [sp, #-0x10]! + ldp x29, x30, [sp], #0x10 + ldr x0, [x1] + br x0 + .size bad_non_protected_indirect_tailcall_not_auted, .-bad_non_protected_indirect_tailcall_not_auted + + .globl main + .type main,@function +main: + mov x0, 0 + ret + .size main, .-main diff --git a/bolt/test/code-at-high-address.c b/bolt/test/code-at-high-address.c new file mode 100644 index 0000000000000..fa33c1eb342d6 --- /dev/null +++ b/bolt/test/code-at-high-address.c @@ -0,0 +1,24 @@ +// Check that llvm-bolt pushes code to higher addresses under +// --hot-functions-at-end when rewriting code in-place. + +// REQUIRES: system-linux + +// RUN: %clang %cflags -O0 %s -o %t -no-pie -Wl,-q -falign-functions=64 \ +// RUN: -nostartfiles -nostdlib -ffreestanding +// RUN: llvm-bolt %t -o %t.bolt --use-old-text --align-functions=1 \ +// RUN: --no-huge-pages --align-text=1 --use-gnu-stack --hot-functions-at-end \ +// RUN: | FileCheck %s --check-prefix=CHECK-BOLT +// RUN: llvm-readelf --sections %t.bolt | FileCheck %s + +// CHECK-BOLT: using original .text for new code with 0x1 alignment at {{.*}} + +// As .text is pushed higher, preceding .bolt.org.text should have non-zero +// size. +// CHECK: .bolt.org.text PROGBITS +// CHECK-NOT: {{ 000000 }} +// CHECK-SAME: AX +// CHECK-NEXT: .text PROGBITS + +int foo() { return 0; } + +int main() { return foo(); } diff --git a/bolt/test/perf2bolt/X86/perf2bolt-spe.test b/bolt/test/perf2bolt/X86/perf2bolt-spe.test index 101bd3789a184..e981aef553b31 100644 --- a/bolt/test/perf2bolt/X86/perf2bolt-spe.test +++ b/bolt/test/perf2bolt/X86/perf2bolt-spe.test @@ -6,4 +6,4 @@ RUN: %clang %cflags %p/../../Inputs/asm_foo.s %p/../../Inputs/asm_main.c -o %t.e RUN: touch %t.empty.perf.data RUN: not perf2bolt -p %t.empty.perf.data -o %t.perf.boltdata --spe --pa %t.exe 2>&1 | FileCheck %s -CHECK: perf2bolt{{.*}} -spe is available only on AArch64. +CHECK: -spe is available only on AArch64. diff --git a/bolt/test/program-header.test b/bolt/test/program-header.test new file mode 100644 index 0000000000000..f5490394eb6d9 --- /dev/null +++ b/bolt/test/program-header.test @@ -0,0 +1,14 @@ +# Check that llvm-bolt does not add new segments when writing code in-place. + +REQUIRES: system-linux + +RUN: %clang %cflags %p/Inputs/hello.c -o %t -no-pie -Wl,-q -nostartfiles \ +RUN: -nostdlib -ffreestanding +RUN: llvm-bolt %t -o %t.bolt --use-old-text --align-functions=1 \ +RUN: --no-huge-pages --align-text=1 --use-gnu-stack \ +RUN: | FileCheck %s --check-prefix=CHECK-BOLT +RUN: llvm-readelf -WS %t.bolt | FileCheck %s + +CHECK-BOLT: not adding new segments + +CHECK-NOT: .bolt.org.eh_frame_hdr diff --git a/bolt/unittests/Core/BinaryContext.cpp b/bolt/unittests/Core/BinaryContext.cpp index 3fa0851d01e5a..d7374b323c916 100644 --- a/bolt/unittests/Core/BinaryContext.cpp +++ b/bolt/unittests/Core/BinaryContext.cpp @@ -199,13 +199,13 @@ TEST_P(BinaryContextTester, BaseAddress) { // Check that base address calculation is correct for a binary with the // following segment layout: BC->SegmentMapInfo[0] = - SegmentInfo{0, 0x10e8c2b4, 0, 0x10e8c2b4, 0x1000, true}; - BC->SegmentMapInfo[0x10e8d2b4] = - SegmentInfo{0x10e8d2b4, 0x3952faec, 0x10e8c2b4, 0x3952faec, 0x1000, true}; - BC->SegmentMapInfo[0x4a3bddc0] = - SegmentInfo{0x4a3bddc0, 0x148e828, 0x4a3bbdc0, 0x148e828, 0x1000, true}; - BC->SegmentMapInfo[0x4b84d5e8] = - SegmentInfo{0x4b84d5e8, 0x294f830, 0x4b84a5e8, 0x3d3820, 0x1000, true}; + SegmentInfo{0, 0x10e8c2b4, 0, 0x10e8c2b4, 0x1000, true, false}; + BC->SegmentMapInfo[0x10e8d2b4] = SegmentInfo{ + 0x10e8d2b4, 0x3952faec, 0x10e8c2b4, 0x3952faec, 0x1000, true, false}; + BC->SegmentMapInfo[0x4a3bddc0] = SegmentInfo{ + 0x4a3bddc0, 0x148e828, 0x4a3bbdc0, 0x148e828, 0x1000, true, false}; + BC->SegmentMapInfo[0x4b84d5e8] = SegmentInfo{ + 0x4b84d5e8, 0x294f830, 0x4b84a5e8, 0x3d3820, 0x1000, true, false}; std::optional BaseAddress = BC->getBaseAddressForMapping(0x7f13f5556000, 0x10e8c000); @@ -220,13 +220,14 @@ TEST_P(BinaryContextTester, BaseAddress2) { // Check that base address calculation is correct for a binary if the // alignment in ELF file are different from pagesize. // The segment layout is as follows: - BC->SegmentMapInfo[0] = SegmentInfo{0, 0x2177c, 0, 0x2177c, 0x10000, true}; + BC->SegmentMapInfo[0] = + SegmentInfo{0, 0x2177c, 0, 0x2177c, 0x10000, true, false}; BC->SegmentMapInfo[0x31860] = - SegmentInfo{0x31860, 0x370, 0x21860, 0x370, 0x10000, true}; + SegmentInfo{0x31860, 0x370, 0x21860, 0x370, 0x10000, true, false}; BC->SegmentMapInfo[0x41c20] = - SegmentInfo{0x41c20, 0x1f8, 0x21c20, 0x1f8, 0x10000, true}; + SegmentInfo{0x41c20, 0x1f8, 0x21c20, 0x1f8, 0x10000, true, false}; BC->SegmentMapInfo[0x54e18] = - SegmentInfo{0x54e18, 0x51, 0x24e18, 0x51, 0x10000, true}; + SegmentInfo{0x54e18, 0x51, 0x24e18, 0x51, 0x10000, true, false}; std::optional BaseAddress = BC->getBaseAddressForMapping(0xaaaaea444000, 0x21000); @@ -242,13 +243,14 @@ TEST_P(BinaryContextTester, BaseAddressSegmentsSmallerThanAlignment) { // when multiple segments are close together in the ELF file (closer // than the required alignment in the process space). // See https://github.com/llvm/llvm-project/issues/109384 - BC->SegmentMapInfo[0] = SegmentInfo{0, 0x1d1c, 0, 0x1d1c, 0x10000, false}; + BC->SegmentMapInfo[0] = + SegmentInfo{0, 0x1d1c, 0, 0x1d1c, 0x10000, false, false}; BC->SegmentMapInfo[0x11d40] = - SegmentInfo{0x11d40, 0x11e0, 0x1d40, 0x11e0, 0x10000, true}; + SegmentInfo{0x11d40, 0x11e0, 0x1d40, 0x11e0, 0x10000, true, false}; BC->SegmentMapInfo[0x22f20] = - SegmentInfo{0x22f20, 0x10e0, 0x2f20, 0x1f0, 0x10000, false}; + SegmentInfo{0x22f20, 0x10e0, 0x2f20, 0x1f0, 0x10000, false, false}; BC->SegmentMapInfo[0x33110] = - SegmentInfo{0x33110, 0x89, 0x3110, 0x88, 0x10000, false}; + SegmentInfo{0x33110, 0x89, 0x3110, 0x88, 0x10000, false, false}; std::optional BaseAddress = BC->getBaseAddressForMapping(0xaaaaaaab1000, 0x1000); diff --git a/bolt/unittests/Core/MemoryMaps.cpp b/bolt/unittests/Core/MemoryMaps.cpp index b0cab5431bdd3..8eb8f8ae529b1 100644 --- a/bolt/unittests/Core/MemoryMaps.cpp +++ b/bolt/unittests/Core/MemoryMaps.cpp @@ -100,10 +100,10 @@ TEST_P(MemoryMapsTester, ParseMultipleSegments) { "[0xabc2000000(0x8000000) @ 0x31d0000 103:01 1573523 0]: r-xp {1}\n", Pid, Filename); - BC->SegmentMapInfo[0x11da000] = - SegmentInfo{0x11da000, 0x10da000, 0x11ca000, 0x10da000, 0x10000, true}; - BC->SegmentMapInfo[0x31d0000] = - SegmentInfo{0x31d0000, 0x51ac82c, 0x31d0000, 0x3000000, 0x200000, true}; + BC->SegmentMapInfo[0x11da000] = SegmentInfo{ + 0x11da000, 0x10da000, 0x11ca000, 0x10da000, 0x10000, true, false}; + BC->SegmentMapInfo[0x31d0000] = SegmentInfo{ + 0x31d0000, 0x51ac82c, 0x31d0000, 0x3000000, 0x200000, true, false}; DataAggregator DA(""); BC->setFilename(Filename); @@ -131,12 +131,12 @@ TEST_P(MemoryMapsTester, MultipleSegmentsMismatchedBaseAddress) { "[0xabc2000000(0x8000000) @ 0x31d0000 103:01 1573523 0]: r-xp {1}\n", Pid, Filename); - BC->SegmentMapInfo[0x11da000] = - SegmentInfo{0x11da000, 0x10da000, 0x11ca000, 0x10da000, 0x10000, true}; + BC->SegmentMapInfo[0x11da000] = SegmentInfo{ + 0x11da000, 0x10da000, 0x11ca000, 0x10da000, 0x10000, true, false}; // Using '0x31d0fff' FileOffset which triggers a different base address // for this second text segment. - BC->SegmentMapInfo[0x31d0000] = - SegmentInfo{0x31d0000, 0x51ac82c, 0x31d0fff, 0x3000000, 0x200000, true}; + BC->SegmentMapInfo[0x31d0000] = SegmentInfo{ + 0x31d0000, 0x51ac82c, 0x31d0fff, 0x3000000, 0x200000, true, false}; DataAggregator DA(""); BC->setFilename(Filename); diff --git a/bolt/unittests/Profile/PerfSpeEvents.cpp b/bolt/unittests/Profile/PerfSpeEvents.cpp index 3e3e05395246c..8d023cd7b7e74 100644 --- a/bolt/unittests/Profile/PerfSpeEvents.cpp +++ b/bolt/unittests/Profile/PerfSpeEvents.cpp @@ -65,7 +65,7 @@ struct PerfSpeEventsTestHelper : public testing::Test { BC = cantFail(BinaryContext::createBinaryContext( ObjFile->makeTriple(), std::make_shared(), ObjFile->getFileName(), nullptr, /*IsPIC*/ false, - DWARFContext::create(*ObjFile.get()), {llvm::outs(), llvm::errs()})); + DWARFContext::create(*ObjFile), {llvm::outs(), llvm::errs()})); ASSERT_FALSE(!BC); } diff --git a/bolt/utils/llvm-bolt-wrapper.py b/bolt/utils/llvm-bolt-wrapper.py index b9d6fad825e78..b913394bce659 100755 --- a/bolt/utils/llvm-bolt-wrapper.py +++ b/bolt/utils/llvm-bolt-wrapper.py @@ -79,7 +79,7 @@ def get_cfg(key): # perf2bolt mode -PERF2BOLT_MODE = ["-aggregate-only", "-ignore-build-id"] +PERF2BOLT_MODE = ["-aggregate-only", "-ignore-build-id", "-show-density"] # boltdiff mode BOLTDIFF_MODE = ["-diff-only", "-o", "/dev/null"] diff --git a/bolt/utils/nfc-check-setup.py b/bolt/utils/nfc-check-setup.py index 710b183505853..275ac7b886d00 100755 --- a/bolt/utils/nfc-check-setup.py +++ b/bolt/utils/nfc-check-setup.py @@ -7,6 +7,29 @@ import sys import textwrap +def get_relevant_bolt_changes(dir: str) -> str: + # Return a list of bolt source changes that are relevant to testing. + all_changes = subprocess.run( + shlex.split("git show HEAD --name-only --pretty=''"), + cwd=dir, + text=True, + stdout=subprocess.PIPE, + ) + keep_bolt = subprocess.run( + shlex.split("grep '^bolt'"), + input=all_changes.stdout, + text=True, + stdout=subprocess.PIPE, + ) + keep_relevant = subprocess.run( + shlex.split( + "grep -v -e '^bolt/docs' -e '^bolt/utils/docker' -e '^bolt/utils/dot2html'" + ), + input=keep_bolt.stdout, + text=True, + stdout=subprocess.PIPE, + ) + return keep_relevant.stdout def get_git_ref_or_rev(dir: str) -> str: # Run 'git symbolic-ref -q --short HEAD || git rev-parse --short HEAD' @@ -36,6 +59,12 @@ def main(): default=os.getcwd(), help="Path to BOLT build directory, default is current " "directory", ) + parser.add_argument( + "--check-bolt-sources", + default=False, + action="store_true", + help="Create a marker file (.llvm-bolt.changes) if any relevant BOLT sources are modified", + ) parser.add_argument( "--switch-back", default=False, @@ -71,6 +100,16 @@ def main(): # memorize the old hash for logging old_ref = get_git_ref_or_rev(source_dir) + if args.check_bolt_sources: + marker = f"{args.build_dir}/.llvm-bolt.changes" + if os.path.exists(marker): + os.remove(marker) + file_changes = get_relevant_bolt_changes(source_dir) + # Create a marker file if any relevant BOLT source files changed. + if len(file_changes) > 0: + print(f"BOLT source changes were found:\n{file_changes}") + open(marker, "a").close() + # determine whether a stash is needed stash = subprocess.run( shlex.split("git status --porcelain"), diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp index 66852931226bf..f756ae6d897c8 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.cpp +++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp @@ -93,6 +93,8 @@ static llvm::Error decodeRecord(const Record &R, InfoType &Field, case InfoType::IT_enum: case InfoType::IT_typedef: case InfoType::IT_concept: + case InfoType::IT_variable: + case InfoType::IT_friend: Field = IT; return llvm::Error::success(); } @@ -110,6 +112,7 @@ static llvm::Error decodeRecord(const Record &R, FieldId &Field, case FieldId::F_child_namespace: case FieldId::F_child_record: case FieldId::F_concept: + case FieldId::F_friend: case FieldId::F_default: Field = F; return llvm::Error::success(); @@ -282,7 +285,15 @@ static llvm::Error parseRecord(const Record &R, unsigned ID, static llvm::Error parseRecord(const Record &R, unsigned ID, llvm::StringRef Blob, TypeInfo *I) { - return llvm::Error::success(); + switch (ID) { + case TYPE_IS_BUILTIN: + return decodeRecord(R, I->IsBuiltIn, Blob); + case TYPE_IS_TEMPLATE: + return decodeRecord(R, I->IsTemplate, Blob); + default: + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "invalid field for TypeInfo"); + } } static llvm::Error parseRecord(const Record &R, unsigned ID, @@ -292,6 +303,10 @@ static llvm::Error parseRecord(const Record &R, unsigned ID, return decodeRecord(R, I->Name, Blob); case FIELD_DEFAULT_VALUE: return decodeRecord(R, I->DefaultValue, Blob); + case FIELD_TYPE_IS_BUILTIN: + return decodeRecord(R, I->IsBuiltIn, Blob); + case FIELD_TYPE_IS_TEMPLATE: + return decodeRecord(R, I->IsTemplate, Blob); default: return llvm::createStringError(llvm::inconvertibleErrorCode(), "invalid field for TypeInfo"); @@ -307,6 +322,10 @@ static llvm::Error parseRecord(const Record &R, unsigned ID, return decodeRecord(R, I->Access, Blob); case MEMBER_TYPE_IS_STATIC: return decodeRecord(R, I->IsStatic, Blob); + case MEMBER_TYPE_IS_BUILTIN: + return decodeRecord(R, I->IsBuiltIn, Blob); + case MEMBER_TYPE_IS_TEMPLATE: + return decodeRecord(R, I->IsTemplate, Blob); default: return llvm::createStringError(llvm::inconvertibleErrorCode(), "invalid field for MemberTypeInfo"); @@ -416,6 +435,32 @@ static llvm::Error parseRecord(const Record &R, unsigned ID, "invalid field for ConstraintInfo"); } +static llvm::Error parseRecord(const Record &R, unsigned ID, + llvm::StringRef Blob, VarInfo *I) { + switch (ID) { + case VAR_USR: + return decodeRecord(R, I->USR, Blob); + case VAR_NAME: + return decodeRecord(R, I->Name, Blob); + case VAR_DEFLOCATION: + return decodeRecord(R, I->DefLoc, Blob); + case VAR_IS_STATIC: + return decodeRecord(R, I->IsStatic, Blob); + default: + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "invalid field for VarInfo"); + } +} + +static llvm::Error parseRecord(const Record &R, unsigned ID, StringRef Blob, + FriendInfo *F) { + if (ID == FRIEND_IS_CLASS) { + return decodeRecord(R, F->IsClass, Blob); + } + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "invalid field for Friend"); +} + template static llvm::Expected getCommentInfo(T I) { return llvm::createStringError(llvm::inconvertibleErrorCode(), "invalid type cannot contain CommentInfo"); @@ -458,6 +503,10 @@ template <> llvm::Expected getCommentInfo(ConceptInfo *I) { return &I->Description.emplace_back(); } +template <> Expected getCommentInfo(VarInfo *I) { + return &I->Description.emplace_back(); +} + // When readSubBlock encounters a TypeInfo sub-block, it calls addTypeInfo on // the parent block to set it. The template specializations define what to do // for each supported parent block. @@ -487,6 +536,18 @@ template <> llvm::Error addTypeInfo(FunctionInfo *I, FieldTypeInfo &&T) { return llvm::Error::success(); } +template <> llvm::Error addTypeInfo(FriendInfo *I, FieldTypeInfo &&T) { + if (!I->Params) + I->Params.emplace(); + I->Params->emplace_back(std::move(T)); + return llvm::Error::success(); +} + +template <> llvm::Error addTypeInfo(FriendInfo *I, TypeInfo &&T) { + I->ReturnType.emplace(std::move(T)); + return llvm::Error::success(); +} + template <> llvm::Error addTypeInfo(EnumInfo *I, TypeInfo &&T) { I->BaseType = std::move(T); return llvm::Error::success(); @@ -497,12 +558,28 @@ template <> llvm::Error addTypeInfo(TypedefInfo *I, TypeInfo &&T) { return llvm::Error::success(); } +template <> llvm::Error addTypeInfo(VarInfo *I, TypeInfo &&T) { + I->Type = std::move(T); + return llvm::Error::success(); +} + template static llvm::Error addReference(T I, Reference &&R, FieldId F) { return llvm::createStringError(llvm::inconvertibleErrorCode(), "invalid type cannot contain Reference"); } +template <> llvm::Error addReference(VarInfo *I, Reference &&R, FieldId F) { + switch (F) { + case FieldId::F_namespace: + I->Namespace.emplace_back(std::move(R)); + return llvm::Error::success(); + default: + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "VarInfo cannot contain this Reference"); + } +} + template <> llvm::Error addReference(TypeInfo *I, Reference &&R, FieldId F) { switch (F) { case FieldId::F_type: @@ -624,6 +701,16 @@ llvm::Error addReference(ConstraintInfo *I, Reference &&R, FieldId F) { "ConstraintInfo cannot contain this Reference"); } +template <> +llvm::Error addReference(FriendInfo *Friend, Reference &&R, FieldId F) { + if (F == FieldId::F_friend) { + Friend->Ref = std::move(R); + return llvm::Error::success(); + } + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "Friend cannot contain this Reference"); +} + template static void addChild(T I, ChildInfoType &&R) { llvm::errs() << "invalid child type for info"; @@ -643,6 +730,9 @@ template <> void addChild(NamespaceInfo *I, TypedefInfo &&R) { template <> void addChild(NamespaceInfo *I, ConceptInfo &&R) { I->Children.Concepts.emplace_back(std::move(R)); } +template <> void addChild(NamespaceInfo *I, VarInfo &&R) { + I->Children.Variables.emplace_back(std::move(R)); +} // Record children: template <> void addChild(RecordInfo *I, FunctionInfo &&R) { @@ -654,6 +744,9 @@ template <> void addChild(RecordInfo *I, EnumInfo &&R) { template <> void addChild(RecordInfo *I, TypedefInfo &&R) { I->Children.Typedefs.emplace_back(std::move(R)); } +template <> void addChild(RecordInfo *I, FriendInfo &&R) { + I->Friends.emplace_back(std::move(R)); +} // Other types of children: template <> void addChild(EnumInfo *I, EnumValueInfo &&R) { @@ -695,6 +788,9 @@ template <> void addTemplate(FunctionInfo *I, TemplateInfo &&P) { template <> void addTemplate(ConceptInfo *I, TemplateInfo &&P) { I->Template = std::move(P); } +template <> void addTemplate(FriendInfo *I, TemplateInfo &&P) { + I->Template.emplace(std::move(P)); +} // Template specializations go only into template records. template @@ -770,11 +866,39 @@ llvm::Error ClangDocBitcodeReader::readBlock(unsigned ID, T I) { } } -// TODO: Create a helper that can receive a function to reduce repetition for -// most blocks. +// TODO: fix inconsistentent returning of errors in add callbacks. +// Once that's fixed, we only need one handleSubBlock. +template +llvm::Error ClangDocBitcodeReader::handleSubBlock(unsigned ID, T Parent, + Callback Function) { + InfoType Info; + if (auto Err = readBlock(ID, &Info)) + return Err; + Function(Parent, std::move(Info)); + return llvm::Error::success(); +} + +template +llvm::Error ClangDocBitcodeReader::handleTypeSubBlock(unsigned ID, T Parent, + Callback Function) { + InfoType Info; + if (auto Err = readBlock(ID, &Info)) + return Err; + if (auto Err = Function(Parent, std::move(Info))) + return Err; + return llvm::Error::success(); +} + template llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) { llvm::TimeTraceScope("Reducing infos", "readSubBlock"); + + static auto CreateAddFunc = [](auto AddFunc) { + return [AddFunc](auto Parent, auto Child) { + return AddFunc(Parent, std::move(Child)); + }; + }; + switch (ID) { // Blocks can only have certain types of sub blocks. case BI_COMMENT_BLOCK_ID: { @@ -786,28 +910,16 @@ llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) { return llvm::Error::success(); } case BI_TYPE_BLOCK_ID: { - TypeInfo TI; - if (auto Err = readBlock(ID, &TI)) - return Err; - if (auto Err = addTypeInfo(I, std::move(TI))) - return Err; - return llvm::Error::success(); + return handleTypeSubBlock( + ID, I, CreateAddFunc(addTypeInfo)); } case BI_FIELD_TYPE_BLOCK_ID: { - FieldTypeInfo TI; - if (auto Err = readBlock(ID, &TI)) - return Err; - if (auto Err = addTypeInfo(I, std::move(TI))) - return Err; - return llvm::Error::success(); + return handleTypeSubBlock( + ID, I, CreateAddFunc(addTypeInfo)); } case BI_MEMBER_TYPE_BLOCK_ID: { - MemberTypeInfo TI; - if (auto Err = readBlock(ID, &TI)) - return Err; - if (auto Err = addTypeInfo(I, std::move(TI))) - return Err; - return llvm::Error::success(); + return handleTypeSubBlock( + ID, I, CreateAddFunc(addTypeInfo)); } case BI_REFERENCE_BLOCK_ID: { Reference R; @@ -818,74 +930,50 @@ llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) { return llvm::Error::success(); } case BI_FUNCTION_BLOCK_ID: { - FunctionInfo F; - if (auto Err = readBlock(ID, &F)) - return Err; - addChild(I, std::move(F)); - return llvm::Error::success(); + return handleSubBlock( + ID, I, CreateAddFunc(addChild)); } case BI_BASE_RECORD_BLOCK_ID: { - BaseRecordInfo BR; - if (auto Err = readBlock(ID, &BR)) - return Err; - addChild(I, std::move(BR)); - return llvm::Error::success(); + return handleSubBlock( + ID, I, CreateAddFunc(addChild)); } case BI_ENUM_BLOCK_ID: { - EnumInfo E; - if (auto Err = readBlock(ID, &E)) - return Err; - addChild(I, std::move(E)); - return llvm::Error::success(); + return handleSubBlock(ID, I, + CreateAddFunc(addChild)); } case BI_ENUM_VALUE_BLOCK_ID: { - EnumValueInfo EV; - if (auto Err = readBlock(ID, &EV)) - return Err; - addChild(I, std::move(EV)); - return llvm::Error::success(); + return handleSubBlock( + ID, I, CreateAddFunc(addChild)); } case BI_TEMPLATE_BLOCK_ID: { - TemplateInfo TI; - if (auto Err = readBlock(ID, &TI)) - return Err; - addTemplate(I, std::move(TI)); - return llvm::Error::success(); + return handleSubBlock(ID, I, CreateAddFunc(addTemplate)); } case BI_TEMPLATE_SPECIALIZATION_BLOCK_ID: { - TemplateSpecializationInfo TSI; - if (auto Err = readBlock(ID, &TSI)) - return Err; - addTemplateSpecialization(I, std::move(TSI)); - return llvm::Error::success(); + return handleSubBlock( + ID, I, CreateAddFunc(addTemplateSpecialization)); } case BI_TEMPLATE_PARAM_BLOCK_ID: { - TemplateParamInfo TPI; - if (auto Err = readBlock(ID, &TPI)) - return Err; - addTemplateParam(I, std::move(TPI)); - return llvm::Error::success(); + return handleSubBlock( + ID, I, CreateAddFunc(addTemplateParam)); } case BI_TYPEDEF_BLOCK_ID: { - TypedefInfo TI; - if (auto Err = readBlock(ID, &TI)) - return Err; - addChild(I, std::move(TI)); - return llvm::Error::success(); + return handleSubBlock(ID, I, + CreateAddFunc(addChild)); } case BI_CONSTRAINT_BLOCK_ID: { - ConstraintInfo CI; - if (auto Err = readBlock(ID, &CI)) - return Err; - addConstraint(I, std::move(CI)); - return llvm::Error::success(); + return handleSubBlock(ID, I, + CreateAddFunc(addConstraint)); } case BI_CONCEPT_BLOCK_ID: { - ConceptInfo CI; - if (auto Err = readBlock(ID, &CI)) - return Err; - addChild(I, std::move(CI)); - return llvm::Error::success(); + return handleSubBlock(ID, I, + CreateAddFunc(addChild)); + } + case BI_VAR_BLOCK_ID: { + return handleSubBlock(ID, I, CreateAddFunc(addChild)); + } + case BI_FRIEND_BLOCK_ID: { + return handleSubBlock(ID, I, + CreateAddFunc(addChild)); } default: return llvm::createStringError(llvm::inconvertibleErrorCode(), @@ -996,6 +1084,10 @@ ClangDocBitcodeReader::readBlockToInfo(unsigned ID) { return createInfo(ID); case BI_FUNCTION_BLOCK_ID: return createInfo(ID); + case BI_VAR_BLOCK_ID: + return createInfo(ID); + case BI_FRIEND_BLOCK_ID: + return createInfo(ID); default: return llvm::createStringError(llvm::inconvertibleErrorCode(), "cannot create info"); @@ -1035,6 +1127,8 @@ ClangDocBitcodeReader::readBitcode() { case BI_ENUM_BLOCK_ID: case BI_TYPEDEF_BLOCK_ID: case BI_CONCEPT_BLOCK_ID: + case BI_VAR_BLOCK_ID: + case BI_FRIEND_BLOCK_ID: case BI_FUNCTION_BLOCK_ID: { auto InfoOrErr = readBlockToInfo(ID); if (!InfoOrErr) diff --git a/clang-tools-extra/clang-doc/BitcodeReader.h b/clang-tools-extra/clang-doc/BitcodeReader.h index a046ee2f7a24a..4947721f0a06d 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.h +++ b/clang-tools-extra/clang-doc/BitcodeReader.h @@ -64,6 +64,13 @@ class ClangDocBitcodeReader { // Helper function to set up the appropriate type of Info. llvm::Expected> readBlockToInfo(unsigned ID); + template + llvm::Error handleSubBlock(unsigned ID, T Parent, CallbackFunction Function); + + template + llvm::Error handleTypeSubBlock(unsigned ID, T Parent, + CallbackFunction Function); + llvm::BitstreamCursor &Stream; std::optional BlockInfo; FieldId CurrentReferenceField; diff --git a/clang-tools-extra/clang-doc/BitcodeWriter.cpp b/clang-tools-extra/clang-doc/BitcodeWriter.cpp index b7308c012786f..3cc0d4ad332f0 100644 --- a/clang-tools-extra/clang-doc/BitcodeWriter.cpp +++ b/clang-tools-extra/clang-doc/BitcodeWriter.cpp @@ -130,7 +130,9 @@ static const llvm::IndexedMap {BI_TEMPLATE_SPECIALIZATION_BLOCK_ID, "TemplateSpecializationBlock"}, {BI_TEMPLATE_PARAM_BLOCK_ID, "TemplateParamBlock"}, {BI_CONSTRAINT_BLOCK_ID, "ConstraintBlock"}, - {BI_CONCEPT_BLOCK_ID, "ConceptBlock"}}; + {BI_CONCEPT_BLOCK_ID, "ConceptBlock"}, + {BI_VAR_BLOCK_ID, "VarBlock"}, + {BI_FRIEND_BLOCK_ID, "FriendBlock"}}; assert(Inits.size() == BlockIdCount); for (const auto &Init : Inits) BlockIdNameMap[Init.first] = Init.second; @@ -160,9 +162,15 @@ static const llvm::IndexedMap {COMMENT_ARG, {"Arg", &genStringAbbrev}}, {FIELD_TYPE_NAME, {"Name", &genStringAbbrev}}, {FIELD_DEFAULT_VALUE, {"DefaultValue", &genStringAbbrev}}, + {FIELD_TYPE_IS_BUILTIN, {"IsBuiltin", &genBoolAbbrev}}, + {FIELD_TYPE_IS_TEMPLATE, {"IsTemplate", &genBoolAbbrev}}, {MEMBER_TYPE_NAME, {"Name", &genStringAbbrev}}, {MEMBER_TYPE_ACCESS, {"Access", &genIntAbbrev}}, {MEMBER_TYPE_IS_STATIC, {"IsStatic", &genBoolAbbrev}}, + {MEMBER_TYPE_IS_BUILTIN, {"IsBuiltin", &genBoolAbbrev}}, + {MEMBER_TYPE_IS_TEMPLATE, {"IsTemplate", &genBoolAbbrev}}, + {TYPE_IS_BUILTIN, {"IsBuiltin", &genBoolAbbrev}}, + {TYPE_IS_TEMPLATE, {"IsTemplate", &genBoolAbbrev}}, {NAMESPACE_USR, {"USR", &genSymbolIdAbbrev}}, {NAMESPACE_NAME, {"Name", &genStringAbbrev}}, {NAMESPACE_PATH, {"Path", &genStringAbbrev}}, @@ -213,7 +221,13 @@ static const llvm::IndexedMap {CONCEPT_IS_TYPE, {"IsType", &genBoolAbbrev}}, {CONCEPT_CONSTRAINT_EXPRESSION, {"ConstraintExpression", &genStringAbbrev}}, - {CONSTRAINT_EXPRESSION, {"Expression", &genStringAbbrev}}}; + {CONSTRAINT_EXPRESSION, {"Expression", &genStringAbbrev}}, + {VAR_USR, {"USR", &genSymbolIdAbbrev}}, + {VAR_NAME, {"Name", &genStringAbbrev}}, + {VAR_DEFLOCATION, {"DefLocation", &genLocationAbbrev}}, + {VAR_IS_STATIC, {"IsStatic", &genBoolAbbrev}}, + {FRIEND_IS_CLASS, {"IsClass", &genBoolAbbrev}}}; + assert(Inits.size() == RecordIdCount); for (const auto &Init : Inits) { RecordIdNameMap[Init.first] = Init.second; @@ -233,12 +247,15 @@ static const std::vector>> COMMENT_PARAMNAME, COMMENT_CLOSENAME, COMMENT_SELFCLOSING, COMMENT_EXPLICIT, COMMENT_ATTRKEY, COMMENT_ATTRVAL, COMMENT_ARG}}, // Type Block - {BI_TYPE_BLOCK_ID, {}}, + {BI_TYPE_BLOCK_ID, {TYPE_IS_BUILTIN, TYPE_IS_TEMPLATE}}, // FieldType Block - {BI_FIELD_TYPE_BLOCK_ID, {FIELD_TYPE_NAME, FIELD_DEFAULT_VALUE}}, + {BI_FIELD_TYPE_BLOCK_ID, + {FIELD_TYPE_NAME, FIELD_DEFAULT_VALUE, FIELD_TYPE_IS_BUILTIN, + FIELD_TYPE_IS_TEMPLATE}}, // MemberType Block {BI_MEMBER_TYPE_BLOCK_ID, - {MEMBER_TYPE_NAME, MEMBER_TYPE_ACCESS, MEMBER_TYPE_IS_STATIC}}, + {MEMBER_TYPE_NAME, MEMBER_TYPE_ACCESS, MEMBER_TYPE_IS_STATIC, + MEMBER_TYPE_IS_BUILTIN, MEMBER_TYPE_IS_TEMPLATE}}, // Enum Block {BI_ENUM_BLOCK_ID, {ENUM_USR, ENUM_NAME, ENUM_DEFLOCATION, ENUM_LOCATION, ENUM_SCOPED}}, @@ -277,7 +294,9 @@ static const std::vector>> {CONCEPT_USR, CONCEPT_NAME, CONCEPT_IS_TYPE, CONCEPT_CONSTRAINT_EXPRESSION}}, // Constraint Block - {BI_CONSTRAINT_BLOCK_ID, {CONSTRAINT_EXPRESSION}}}; + {BI_CONSTRAINT_BLOCK_ID, {CONSTRAINT_EXPRESSION}}, + {BI_VAR_BLOCK_ID, {VAR_NAME, VAR_USR, VAR_DEFLOCATION, VAR_IS_STATIC}}, + {BI_FRIEND_BLOCK_ID, {FRIEND_IS_CLASS}}}; // AbbreviationMap @@ -460,9 +479,24 @@ void ClangDocBitcodeWriter::emitBlock(const Reference &R, FieldId Field) { emitRecord((unsigned)Field, REFERENCE_FIELD); } +void ClangDocBitcodeWriter::emitBlock(const FriendInfo &R) { + StreamSubBlockGuard Block(Stream, BI_FRIEND_BLOCK_ID); + emitBlock(R.Ref, FieldId::F_friend); + emitRecord(R.IsClass, FRIEND_IS_CLASS); + if (R.Template) + emitBlock(*R.Template); + if (R.Params) + for (const auto &P : *R.Params) + emitBlock(P); + if (R.ReturnType) + emitBlock(*R.ReturnType); +} + void ClangDocBitcodeWriter::emitBlock(const TypeInfo &T) { StreamSubBlockGuard Block(Stream, BI_TYPE_BLOCK_ID); emitBlock(T.Type, FieldId::F_type); + emitRecord(T.IsBuiltIn, TYPE_IS_BUILTIN); + emitRecord(T.IsTemplate, TYPE_IS_TEMPLATE); } void ClangDocBitcodeWriter::emitBlock(const TypedefInfo &T) { @@ -484,6 +518,8 @@ void ClangDocBitcodeWriter::emitBlock(const FieldTypeInfo &T) { emitBlock(T.Type, FieldId::F_type); emitRecord(T.Name, FIELD_TYPE_NAME); emitRecord(T.DefaultValue, FIELD_DEFAULT_VALUE); + emitRecord(T.IsBuiltIn, FIELD_TYPE_IS_BUILTIN); + emitRecord(T.IsTemplate, FIELD_TYPE_IS_TEMPLATE); } void ClangDocBitcodeWriter::emitBlock(const MemberTypeInfo &T) { @@ -492,6 +528,9 @@ void ClangDocBitcodeWriter::emitBlock(const MemberTypeInfo &T) { emitRecord(T.Name, MEMBER_TYPE_NAME); emitRecord(T.Access, MEMBER_TYPE_ACCESS); emitRecord(T.IsStatic, MEMBER_TYPE_IS_STATIC); + emitRecord(T.IsBuiltIn, MEMBER_TYPE_IS_BUILTIN); + emitRecord(T.IsTemplate, MEMBER_TYPE_IS_TEMPLATE); + emitRecord(T.IsTemplate, MEMBER_TYPE_IS_TEMPLATE); for (const auto &CI : T.Description) emitBlock(CI); } @@ -540,6 +579,8 @@ void ClangDocBitcodeWriter::emitBlock(const NamespaceInfo &I) { emitBlock(C); for (const auto &C : I.Children.Concepts) emitBlock(C); + for (const auto &C : I.Children.Variables) + emitBlock(C); } void ClangDocBitcodeWriter::emitBlock(const EnumInfo &I) { @@ -603,6 +644,8 @@ void ClangDocBitcodeWriter::emitBlock(const RecordInfo &I) { emitBlock(C); if (I.Template) emitBlock(*I.Template); + for (const auto &C : I.Friends) + emitBlock(C); } void ClangDocBitcodeWriter::emitBlock(const BaseRecordInfo &I) { @@ -682,6 +725,20 @@ void ClangDocBitcodeWriter::emitBlock(const ConstraintInfo &C) { emitBlock(C.ConceptRef, FieldId::F_concept); } +void ClangDocBitcodeWriter::emitBlock(const VarInfo &I) { + StreamSubBlockGuard Block(Stream, BI_VAR_BLOCK_ID); + emitRecord(I.USR, VAR_USR); + emitRecord(I.Name, VAR_NAME); + for (const auto &N : I.Namespace) + emitBlock(N, FieldId::F_namespace); + for (const auto &CI : I.Description) + emitBlock(CI); + if (I.DefLoc) + emitRecord(*I.DefLoc, VAR_DEFLOCATION); + emitRecord(I.IsStatic, VAR_IS_STATIC); + emitBlock(I.Type); +} + bool ClangDocBitcodeWriter::dispatchInfoForWrite(Info *I) { switch (I->IT) { case InfoType::IT_namespace: @@ -702,6 +759,12 @@ bool ClangDocBitcodeWriter::dispatchInfoForWrite(Info *I) { case InfoType::IT_concept: emitBlock(*static_cast(I)); break; + case InfoType::IT_variable: + emitBlock(*static_cast(I)); + break; + case InfoType::IT_friend: + emitBlock(*static_cast(I)); + break; case InfoType::IT_default: llvm::errs() << "Unexpected info, unable to write.\n"; return true; diff --git a/clang-tools-extra/clang-doc/BitcodeWriter.h b/clang-tools-extra/clang-doc/BitcodeWriter.h index 4d0c0c07805e7..d09ec4ca34006 100644 --- a/clang-tools-extra/clang-doc/BitcodeWriter.h +++ b/clang-tools-extra/clang-doc/BitcodeWriter.h @@ -69,6 +69,8 @@ enum BlockId { BI_CONSTRAINT_BLOCK_ID, BI_TYPEDEF_BLOCK_ID, BI_CONCEPT_BLOCK_ID, + BI_VAR_BLOCK_ID, + BI_FRIEND_BLOCK_ID, BI_LAST, BI_FIRST = BI_VERSION_BLOCK_ID }; @@ -95,11 +97,17 @@ enum RecordId { COMMENT_ATTRKEY, COMMENT_ATTRVAL, COMMENT_ARG, + TYPE_IS_BUILTIN, + TYPE_IS_TEMPLATE, FIELD_TYPE_NAME, FIELD_DEFAULT_VALUE, + FIELD_TYPE_IS_BUILTIN, + FIELD_TYPE_IS_TEMPLATE, MEMBER_TYPE_NAME, MEMBER_TYPE_ACCESS, MEMBER_TYPE_IS_STATIC, + MEMBER_TYPE_IS_BUILTIN, + MEMBER_TYPE_IS_TEMPLATE, NAMESPACE_USR, NAMESPACE_NAME, NAMESPACE_PATH, @@ -142,6 +150,11 @@ enum RecordId { CONCEPT_IS_TYPE, CONCEPT_CONSTRAINT_EXPRESSION, CONSTRAINT_EXPRESSION, + VAR_USR, + VAR_NAME, + VAR_DEFLOCATION, + VAR_IS_STATIC, + FRIEND_IS_CLASS, RI_LAST, RI_FIRST = VERSION }; @@ -158,7 +171,8 @@ enum class FieldId { F_type, F_child_namespace, F_child_record, - F_concept + F_concept, + F_friend }; class ClangDocBitcodeWriter { @@ -190,6 +204,8 @@ class ClangDocBitcodeWriter { void emitBlock(const ConceptInfo &T); void emitBlock(const ConstraintInfo &T); void emitBlock(const Reference &B, FieldId F); + void emitBlock(const FriendInfo &R); + void emitBlock(const VarInfo &B); private: class AbbreviationMap { diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp index 935bbfee7a9b1..8294ff9118558 100644 --- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp @@ -986,6 +986,8 @@ llvm::Error HTMLGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, genHTML(*static_cast(I), CDCtx, InfoTitle); break; case InfoType::IT_concept: + case InfoType::IT_variable: + case InfoType::IT_friend: break; case InfoType::IT_default: return llvm::createStringError(llvm::inconvertibleErrorCode(), @@ -1015,6 +1017,10 @@ static std::string getRefType(InfoType IT) { return "typedef"; case InfoType::IT_concept: return "concept"; + case InfoType::IT_variable: + return "variable"; + case InfoType::IT_friend: + return "friend"; } llvm_unreachable("Unknown InfoType"); } diff --git a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp index 81ba99c21e374..7aeaa1b7cf67d 100644 --- a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp @@ -587,6 +587,9 @@ Error MustacheHTMLGenerator::generateDocForInfo(Info *I, raw_ostream &OS, break; case InfoType::IT_concept: break; + case InfoType::IT_variable: + case InfoType::IT_friend: + break; case InfoType::IT_default: return createStringError(inconvertibleErrorCode(), "unexpected InfoType"); } diff --git a/clang-tools-extra/clang-doc/JSONGenerator.cpp b/clang-tools-extra/clang-doc/JSONGenerator.cpp index 8a37621597c6a..0e1a0cc347e45 100644 --- a/clang-tools-extra/clang-doc/JSONGenerator.cpp +++ b/clang-tools-extra/clang-doc/JSONGenerator.cpp @@ -22,22 +22,30 @@ class JSONGenerator : public Generator { const char *JSONGenerator::Format = "json"; -static void serializeInfo(const TypedefInfo &I, json::Object &Obj, - std::optional RepositoryUrl); -static void serializeInfo(const EnumInfo &I, json::Object &Obj, - std::optional RepositoryUrl); static void serializeInfo(const ConstraintInfo &I, Object &Obj); +static void serializeInfo(const RecordInfo &I, Object &Obj, + const std::optional &RepositoryUrl); + +static void serializeReference(const Reference &Ref, Object &ReferenceObj); + +template +static void serializeArray(const Container &Records, Object &Obj, + const std::string &Key, + SerializationFunc SerializeInfo); // Convenience lambda to pass to serializeArray. // If a serializeInfo needs a RepositoryUrl, create a local lambda that captures // the optional. -static auto SerializeInfoLambda = [](const ConstraintInfo &Info, - Object &Object) { +static auto SerializeInfoLambda = [](const auto &Info, Object &Object) { serializeInfo(Info, Object); }; +static auto SerializeReferenceLambda = [](const auto &Ref, Object &Object) { + serializeReference(Ref, Object); +}; -static json::Object serializeLocation(const Location &Loc, - std::optional RepositoryUrl) { +static json::Object +serializeLocation(const Location &Loc, + const std::optional &RepositoryUrl) { Object LocationObj = Object(); LocationObj["LineNumber"] = Loc.StartLineNumber; LocationObj["Filename"] = Loc.Filename; @@ -159,8 +167,9 @@ static json::Value serializeComment(const CommentInfo &I) { llvm_unreachable("Unknown comment kind encountered."); } -static void serializeCommonAttributes(const Info &I, json::Object &Obj, - std::optional RepositoryUrl) { +static void +serializeCommonAttributes(const Info &I, json::Object &Obj, + const std::optional &RepositoryUrl) { Obj["Name"] = I.Name; Obj["USR"] = toHex(toStringRef(I.USR)); @@ -198,67 +207,28 @@ static void serializeReference(const Reference &Ref, Object &ReferenceObj) { ReferenceObj["USR"] = toHex(toStringRef(Ref.USR)); } -static void serializeReference(const SmallVector &References, - Object &Obj, std::string Key) { - json::Value ReferencesArray = Array(); - json::Array &ReferencesArrayRef = *ReferencesArray.getAsArray(); - ReferencesArrayRef.reserve(References.size()); - for (const auto &Reference : References) { - json::Value ReferenceVal = Object(); - auto &ReferenceObj = *ReferenceVal.getAsObject(); - serializeReference(Reference, ReferenceObj); - ReferencesArrayRef.push_back(ReferenceVal); - } - Obj[Key] = ReferencesArray; -} - // Although namespaces and records both have ScopeChildren, they serialize them // differently. Only enums, records, and typedefs are handled here. -static void serializeCommonChildren(const ScopeChildren &Children, - json::Object &Obj, - std::optional RepositoryUrl) { - if (!Children.Enums.empty()) { - json::Value EnumsArray = Array(); - auto &EnumsArrayRef = *EnumsArray.getAsArray(); - EnumsArrayRef.reserve(Children.Enums.size()); - for (const auto &Enum : Children.Enums) { - json::Value EnumVal = Object(); - auto &EnumObj = *EnumVal.getAsObject(); - serializeInfo(Enum, EnumObj, RepositoryUrl); - EnumsArrayRef.push_back(EnumVal); - } - Obj["Enums"] = EnumsArray; - } +static void +serializeCommonChildren(const ScopeChildren &Children, json::Object &Obj, + const std::optional &RepositoryUrl) { + static auto SerializeInfo = [&RepositoryUrl](const auto &Info, + Object &Object) { + serializeInfo(Info, Object, RepositoryUrl); + }; - if (!Children.Typedefs.empty()) { - json::Value TypedefsArray = Array(); - auto &TypedefsArrayRef = *TypedefsArray.getAsArray(); - TypedefsArrayRef.reserve(Children.Typedefs.size()); - for (const auto &Typedef : Children.Typedefs) { - json::Value TypedefVal = Object(); - auto &TypedefObj = *TypedefVal.getAsObject(); - serializeInfo(Typedef, TypedefObj, RepositoryUrl); - TypedefsArrayRef.push_back(TypedefVal); - } - Obj["Typedefs"] = TypedefsArray; - } + if (!Children.Enums.empty()) + serializeArray(Children.Enums, Obj, "Enums", SerializeInfo); - if (!Children.Records.empty()) { - json::Value RecordsArray = Array(); - auto &RecordsArrayRef = *RecordsArray.getAsArray(); - RecordsArrayRef.reserve(Children.Records.size()); - for (const auto &Record : Children.Records) { - json::Value RecordVal = Object(); - auto &RecordObj = *RecordVal.getAsObject(); - serializeReference(Record, RecordObj); - RecordsArrayRef.push_back(RecordVal); - } - Obj["Records"] = RecordsArray; - } + if (!Children.Typedefs.empty()) + serializeArray(Children.Typedefs, Obj, "Typedefs", SerializeInfo); + + if (!Children.Records.empty()) + serializeArray(Children.Records, Obj, "Records", SerializeReferenceLambda); } -template -static void serializeArray(const std::vector &Records, Object &Obj, +template +static void serializeArray(const Container &Records, Object &Obj, const std::string &Key, SerializationFunc SerializeInfo) { json::Value RecordsArray = Array(); @@ -278,6 +248,16 @@ static void serializeInfo(const ConstraintInfo &I, Object &Obj) { Obj["Expression"] = I.ConstraintExpr; } +static void serializeInfo(const ArrayRef &Params, + Object &Obj) { + json::Value ParamsArray = Array(); + auto &ParamsArrayRef = *ParamsArray.getAsArray(); + ParamsArrayRef.reserve(Params.size()); + for (const auto &Param : Params) + ParamsArrayRef.push_back(Param.Contents); + Obj["Parameters"] = ParamsArray; +} + static void serializeInfo(const TemplateInfo &Template, Object &Obj) { json::Value TemplateVal = Object(); auto &TemplateObj = *TemplateVal.getAsObject(); @@ -287,25 +267,13 @@ static void serializeInfo(const TemplateInfo &Template, Object &Obj) { auto &TemplateSpecializationObj = *TemplateSpecializationVal.getAsObject(); TemplateSpecializationObj["SpecializationOf"] = toHex(toStringRef(Template.Specialization->SpecializationOf)); - if (!Template.Specialization->Params.empty()) { - json::Value ParamsArray = Array(); - auto &ParamsArrayRef = *ParamsArray.getAsArray(); - ParamsArrayRef.reserve(Template.Specialization->Params.size()); - for (const auto &Param : Template.Specialization->Params) - ParamsArrayRef.push_back(Param.Contents); - TemplateSpecializationObj["Parameters"] = ParamsArray; - } + if (!Template.Specialization->Params.empty()) + serializeInfo(Template.Specialization->Params, TemplateSpecializationObj); TemplateObj["Specialization"] = TemplateSpecializationVal; } - if (!Template.Params.empty()) { - json::Value ParamsArray = Array(); - auto &ParamsArrayRef = *ParamsArray.getAsArray(); - ParamsArrayRef.reserve(Template.Params.size()); - for (const auto &Param : Template.Params) - ParamsArrayRef.push_back(Param.Contents); - TemplateObj["Parameters"] = ParamsArray; - } + if (!Template.Params.empty()) + serializeInfo(Template.Params, TemplateObj); if (!Template.Constraints.empty()) serializeArray(Template.Constraints, TemplateObj, "Constraints", @@ -315,7 +283,7 @@ static void serializeInfo(const TemplateInfo &Template, Object &Obj) { } static void serializeInfo(const ConceptInfo &I, Object &Obj, - std::optional RepositoryUrl) { + const std::optional &RepositoryUrl) { serializeCommonAttributes(I, Obj, RepositoryUrl); Obj["IsType"] = I.IsType; Obj["ConstraintExpression"] = I.ConstraintExpression; @@ -330,8 +298,13 @@ static void serializeInfo(const TypeInfo &I, Object &Obj) { Obj["IsBuiltIn"] = I.IsBuiltIn; } +static void serializeInfo(const FieldTypeInfo &I, Object &Obj) { + Obj["Name"] = I.Name; + Obj["Type"] = I.Type.Name; +} + static void serializeInfo(const FunctionInfo &F, json::Object &Obj, - std::optional RepositoryURL) { + const std::optional &RepositoryURL) { serializeCommonAttributes(F, Obj, RepositoryURL); Obj["IsStatic"] = F.IsStatic; @@ -339,26 +312,23 @@ static void serializeInfo(const FunctionInfo &F, json::Object &Obj, serializeInfo(F.ReturnType, ReturnTypeObj); Obj["ReturnType"] = std::move(ReturnTypeObj); - if (!F.Params.empty()) { - json::Value ParamsArray = json::Array(); - auto &ParamsArrayRef = *ParamsArray.getAsArray(); - ParamsArrayRef.reserve(F.Params.size()); - for (const auto &Param : F.Params) { - json::Value ParamVal = Object(); - auto &ParamObj = *ParamVal.getAsObject(); - ParamObj["Name"] = Param.Name; - ParamObj["Type"] = Param.Type.Name; - ParamsArrayRef.push_back(ParamVal); - } - Obj["Params"] = ParamsArray; - } + if (!F.Params.empty()) + serializeArray(F.Params, Obj, "Params", SerializeInfoLambda); if (F.Template) serializeInfo(F.Template.value(), Obj); } +static void serializeInfo(const EnumValueInfo &I, Object &Obj) { + Obj["Name"] = I.Name; + if (!I.ValueExpr.empty()) + Obj["ValueExpr"] = I.ValueExpr; + else + Obj["Value"] = I.Value; +} + static void serializeInfo(const EnumInfo &I, json::Object &Obj, - std::optional RepositoryUrl) { + const std::optional &RepositoryUrl) { serializeCommonAttributes(I, Obj, RepositoryUrl); Obj["Scoped"] = I.Scoped; @@ -371,26 +341,12 @@ static void serializeInfo(const EnumInfo &I, json::Object &Obj, Obj["BaseType"] = BaseTypeVal; } - if (!I.Members.empty()) { - json::Value MembersArray = Array(); - auto &MembersArrayRef = *MembersArray.getAsArray(); - MembersArrayRef.reserve(I.Members.size()); - for (const auto &Member : I.Members) { - json::Value MemberVal = Object(); - auto &MemberObj = *MemberVal.getAsObject(); - MemberObj["Name"] = Member.Name; - if (!Member.ValueExpr.empty()) - MemberObj["ValueExpr"] = Member.ValueExpr; - else - MemberObj["Value"] = Member.Value; - MembersArrayRef.push_back(MemberVal); - } - Obj["Members"] = MembersArray; - } + if (!I.Members.empty()) + serializeArray(I.Members, Obj, "Members", SerializeInfoLambda); } static void serializeInfo(const TypedefInfo &I, json::Object &Obj, - std::optional RepositoryUrl) { + const std::optional &RepositoryUrl) { serializeCommonAttributes(I, Obj, RepositoryUrl); Obj["TypeDeclaration"] = I.TypeDeclaration; Obj["IsUsing"] = I.IsUsing; @@ -400,8 +356,32 @@ static void serializeInfo(const TypedefInfo &I, json::Object &Obj, Obj["Underlying"] = TypeVal; } +static void serializeInfo(const BaseRecordInfo &I, Object &Obj, + const std::optional &RepositoryUrl) { + serializeInfo(static_cast(I), Obj, RepositoryUrl); + Obj["IsVirtual"] = I.IsVirtual; + Obj["Access"] = getAccessSpelling(I.Access); + Obj["IsParent"] = I.IsParent; +} + +static void serializeInfo(const FriendInfo &I, Object &Obj) { + auto FriendRef = Object(); + serializeReference(I.Ref, FriendRef); + Obj["Reference"] = std::move(FriendRef); + Obj["IsClass"] = I.IsClass; + if (I.Template) + serializeInfo(I.Template.value(), Obj); + if (I.Params) + serializeArray(I.Params.value(), Obj, "Params", SerializeInfoLambda); + if (I.ReturnType) { + auto ReturnTypeObj = Object(); + serializeInfo(I.ReturnType.value(), ReturnTypeObj); + Obj["ReturnType"] = std::move(ReturnTypeObj); + } +} + static void serializeInfo(const RecordInfo &I, json::Object &Obj, - std::optional RepositoryUrl) { + const std::optional &RepositoryUrl) { serializeCommonAttributes(I, Obj, RepositoryUrl); Obj["FullName"] = I.FullName; Obj["TagType"] = getTagType(I.TagType); @@ -454,71 +434,60 @@ static void serializeInfo(const RecordInfo &I, json::Object &Obj, Obj["ProtectedMembers"] = ProtectedMembersArray; } - if (!I.Bases.empty()) { - json::Value BasesArray = Array(); - json::Array &BasesArrayRef = *BasesArray.getAsArray(); - BasesArrayRef.reserve(I.Bases.size()); - for (const auto &BaseInfo : I.Bases) { - json::Value BaseInfoVal = Object(); - auto &BaseInfoObj = *BaseInfoVal.getAsObject(); - serializeInfo(BaseInfo, BaseInfoObj, RepositoryUrl); - BaseInfoObj["IsVirtual"] = BaseInfo.IsVirtual; - BaseInfoObj["Access"] = getAccessSpelling(BaseInfo.Access); - BaseInfoObj["IsParent"] = BaseInfo.IsParent; - BasesArrayRef.push_back(BaseInfoVal); - } - Obj["Bases"] = BasesArray; - } + if (!I.Bases.empty()) + serializeArray( + I.Bases, Obj, "Bases", + [&RepositoryUrl](const BaseRecordInfo &Base, Object &BaseObj) { + serializeInfo(Base, BaseObj, RepositoryUrl); + }); if (!I.Parents.empty()) - serializeReference(I.Parents, Obj, "Parents"); + serializeArray(I.Parents, Obj, "Parents", SerializeReferenceLambda); if (!I.VirtualParents.empty()) - serializeReference(I.VirtualParents, Obj, "VirtualParents"); + serializeArray(I.VirtualParents, Obj, "VirtualParents", + SerializeReferenceLambda); if (I.Template) serializeInfo(I.Template.value(), Obj); + if (!I.Friends.empty()) + serializeArray(I.Friends, Obj, "Friends", SerializeInfoLambda); + serializeCommonChildren(I.Children, Obj, RepositoryUrl); } +static void serializeInfo(const VarInfo &I, json::Object &Obj, + const std::optional &RepositoryUrl) { + serializeCommonAttributes(I, Obj, RepositoryUrl); + Obj["IsStatic"] = I.IsStatic; + auto TypeObj = Object(); + serializeInfo(I.Type, TypeObj); + Obj["Type"] = std::move(TypeObj); +} + static void serializeInfo(const NamespaceInfo &I, json::Object &Obj, - std::optional RepositoryUrl) { + const std::optional &RepositoryUrl) { serializeCommonAttributes(I, Obj, RepositoryUrl); - if (!I.Children.Namespaces.empty()) { - json::Value NamespacesArray = Array(); - auto &NamespacesArrayRef = *NamespacesArray.getAsArray(); - NamespacesArrayRef.reserve(I.Children.Namespaces.size()); - for (auto &Namespace : I.Children.Namespaces) { - json::Value NamespaceVal = Object(); - auto &NamespaceObj = *NamespaceVal.getAsObject(); - serializeReference(Namespace, NamespaceObj); - NamespacesArrayRef.push_back(NamespaceVal); - } - Obj["Namespaces"] = NamespacesArray; - } + if (!I.Children.Namespaces.empty()) + serializeArray(I.Children.Namespaces, Obj, "Namespaces", + SerializeReferenceLambda); - auto SerializeInfo = [RepositoryUrl](const auto &Info, Object &Object) { + static auto SerializeInfo = [&RepositoryUrl](const auto &Info, + Object &Object) { serializeInfo(Info, Object, RepositoryUrl); }; - if (!I.Children.Functions.empty()) { - json::Value FunctionsArray = Array(); - auto &FunctionsArrayRef = *FunctionsArray.getAsArray(); - FunctionsArrayRef.reserve(I.Children.Functions.size()); - for (const auto &Function : I.Children.Functions) { - json::Value FunctionVal = Object(); - auto &FunctionObj = *FunctionVal.getAsObject(); - serializeInfo(Function, FunctionObj, RepositoryUrl); - FunctionsArrayRef.push_back(FunctionVal); - } - Obj["Functions"] = FunctionsArray; - } + if (!I.Children.Functions.empty()) + serializeArray(I.Children.Functions, Obj, "Functions", SerializeInfo); if (!I.Children.Concepts.empty()) serializeArray(I.Children.Concepts, Obj, "Concepts", SerializeInfo); + if (!I.Children.Variables.empty()) + serializeArray(I.Children.Variables, Obj, "Variables", SerializeInfo); + serializeCommonChildren(I.Children, Obj, RepositoryUrl); } @@ -573,6 +542,8 @@ Error JSONGenerator::generateDocForInfo(Info *I, raw_ostream &OS, case InfoType::IT_enum: case InfoType::IT_function: case InfoType::IT_typedef: + case InfoType::IT_variable: + case InfoType::IT_friend: break; case InfoType::IT_default: return createStringError(inconvertibleErrorCode(), "unexpected info type"); diff --git a/clang-tools-extra/clang-doc/MDGenerator.cpp b/clang-tools-extra/clang-doc/MDGenerator.cpp index 6e68e09cfa2a6..6f16f5bd2f528 100644 --- a/clang-tools-extra/clang-doc/MDGenerator.cpp +++ b/clang-tools-extra/clang-doc/MDGenerator.cpp @@ -375,6 +375,12 @@ static llvm::Error genIndex(ClangDocContext &CDCtx) { case InfoType::IT_concept: Type = "Concept"; break; + case InfoType::IT_variable: + Type = "Variable"; + break; + case InfoType::IT_friend: + Type = "Friend"; + break; case InfoType::IT_default: Type = "Other"; } @@ -468,6 +474,8 @@ llvm::Error MDGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, genMarkdown(CDCtx, *static_cast(I), OS); break; case InfoType::IT_concept: + case InfoType::IT_variable: + case InfoType::IT_friend: break; case InfoType::IT_default: return createStringError(llvm::inconvertibleErrorCode(), diff --git a/clang-tools-extra/clang-doc/Mapper.cpp b/clang-tools-extra/clang-doc/Mapper.cpp index 6021e17b4696d..497b80cf97463 100644 --- a/clang-tools-extra/clang-doc/Mapper.cpp +++ b/clang-tools-extra/clang-doc/Mapper.cpp @@ -138,6 +138,12 @@ bool MapASTVisitor::VisitConceptDecl(const ConceptDecl *D) { return mapDecl(D, true); } +bool MapASTVisitor::VisitVarDecl(const VarDecl *D) { + if (D->isCXXClassMember()) + return true; + return mapDecl(D, D->isThisDeclarationADefinition()); +} + comments::FullComment * MapASTVisitor::getComment(const NamedDecl *D, const ASTContext &Context) const { RawComment *Comment = Context.getRawCommentForDeclNoCache(D); diff --git a/clang-tools-extra/clang-doc/Mapper.h b/clang-tools-extra/clang-doc/Mapper.h index 04dc5450c8ba3..322df6d594b3d 100644 --- a/clang-tools-extra/clang-doc/Mapper.h +++ b/clang-tools-extra/clang-doc/Mapper.h @@ -42,6 +42,7 @@ class MapASTVisitor : public clang::RecursiveASTVisitor, bool VisitTypedefDecl(const TypedefDecl *D); bool VisitTypeAliasDecl(const TypeAliasDecl *D); bool VisitConceptDecl(const ConceptDecl *D); + bool VisitVarDecl(const VarDecl *D); private: template bool mapDecl(const T *D, bool IsDefinition); diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp index 286aeeea1001b..422a76d99e5b3 100644 --- a/clang-tools-extra/clang-doc/Representation.cpp +++ b/clang-tools-extra/clang-doc/Representation.cpp @@ -145,6 +145,10 @@ mergeInfos(std::vector> &Values) { return reduce(Values); case InfoType::IT_concept: return reduce(Values); + case InfoType::IT_variable: + return reduce(Values); + case InfoType::IT_friend: + return reduce(Values); case InfoType::IT_default: return llvm::createStringError(llvm::inconvertibleErrorCode(), "unexpected info type"); @@ -245,6 +249,15 @@ void Reference::merge(Reference &&Other) { Path = Other.Path; } +bool FriendInfo::mergeable(const FriendInfo &Other) { + return Ref.USR == Other.Ref.USR && Ref.Name == Other.Ref.Name; +} + +void FriendInfo::merge(FriendInfo &&Other) { + assert(mergeable(Other)); + Ref.merge(std::move(Other.Ref)); +} + void Info::mergeBase(Info &&Other) { assert(mergeable(Other)); if (USR == EmptySID) @@ -291,6 +304,7 @@ void NamespaceInfo::merge(NamespaceInfo &&Other) { reduceChildren(Children.Enums, std::move(Other.Children.Enums)); reduceChildren(Children.Typedefs, std::move(Other.Children.Typedefs)); reduceChildren(Children.Concepts, std::move(Other.Children.Concepts)); + reduceChildren(Children.Variables, std::move(Other.Children.Variables)); mergeBase(std::move(Other)); } @@ -310,6 +324,8 @@ void RecordInfo::merge(RecordInfo &&Other) { Parents = std::move(Other.Parents); if (VirtualParents.empty()) VirtualParents = std::move(Other.VirtualParents); + if (Friends.empty()) + Friends = std::move(Other.Friends); // Reduce children if necessary. reduceChildren(Children.Records, std::move(Other.Children.Records)); reduceChildren(Children.Functions, std::move(Other.Children.Functions)); @@ -368,6 +384,15 @@ void ConceptInfo::merge(ConceptInfo &&Other) { SymbolInfo::merge(std::move(Other)); } +void VarInfo::merge(VarInfo &&Other) { + assert(mergeable(Other)); + if (!IsStatic) + IsStatic = Other.IsStatic; + if (Type.Type.USR == EmptySID && Type.Type.Name == "") + Type = std::move(Other.Type); + SymbolInfo::merge(std::move(Other)); +} + BaseRecordInfo::BaseRecordInfo() : RecordInfo() {} BaseRecordInfo::BaseRecordInfo(SymbolID USR, StringRef Name, StringRef Path, @@ -407,6 +432,12 @@ llvm::SmallString<16> Info::extractName() const { case InfoType::IT_concept: return llvm::SmallString<16>("@nonymous_concept_" + toHex(llvm::toStringRef(USR))); + case InfoType::IT_variable: + return llvm::SmallString<16>("@nonymous_variable_" + + toHex(llvm::toStringRef(USR))); + case InfoType::IT_friend: + return llvm::SmallString<16>("@nonymous_friend_" + + toHex(llvm::toStringRef(USR))); case InfoType::IT_default: return llvm::SmallString<16>("@nonymous_" + toHex(llvm::toStringRef(USR))); } @@ -473,6 +504,7 @@ void ScopeChildren::sort() { llvm::sort(Enums.begin(), Enums.end()); llvm::sort(Typedefs.begin(), Typedefs.end()); llvm::sort(Concepts.begin(), Concepts.end()); + llvm::sort(Variables.begin(), Variables.end()); } } // namespace doc } // namespace clang diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h index b23069f2bd324..fe5cc48069d58 100644 --- a/clang-tools-extra/clang-doc/Representation.h +++ b/clang-tools-extra/clang-doc/Representation.h @@ -36,6 +36,7 @@ struct FunctionInfo; struct Info; struct TypedefInfo; struct ConceptInfo; +struct VarInfo; enum class InfoType { IT_default, @@ -44,7 +45,9 @@ enum class InfoType { IT_function, IT_enum, IT_typedef, - IT_concept + IT_concept, + IT_variable, + IT_friend }; enum class CommentKind { @@ -169,6 +172,7 @@ struct ScopeChildren { std::vector Enums; std::vector Typedefs; std::vector Concepts; + std::vector Variables; void sort(); }; @@ -376,6 +380,31 @@ struct SymbolInfo : public Info { bool IsStatic = false; }; +struct FriendInfo : SymbolInfo { + FriendInfo() : SymbolInfo(InfoType::IT_friend) {} + FriendInfo(SymbolID USR) : SymbolInfo(InfoType::IT_friend, USR) {} + FriendInfo(const InfoType IT, const SymbolID &USR, + const StringRef Name = StringRef()) + : SymbolInfo(IT, USR, Name) {} + bool mergeable(const FriendInfo &Other); + void merge(FriendInfo &&Other); + + Reference Ref; + std::optional Template; + std::optional ReturnType; + std::optional> Params; + bool IsClass = false; +}; + +struct VarInfo : SymbolInfo { + VarInfo() : SymbolInfo(InfoType::IT_variable) {} + explicit VarInfo(SymbolID USR) : SymbolInfo(InfoType::IT_variable, USR) {} + + void merge(VarInfo &&I); + + TypeInfo Type; +}; + // TODO: Expand to allow for documenting templating and default args. // Info for functions. struct FunctionInfo : public SymbolInfo { @@ -442,6 +471,8 @@ struct RecordInfo : public SymbolInfo { Bases; // List of base/parent records; this includes inherited methods and // attributes + std::vector Friends; + ScopeChildren Children; }; diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp index 5f3e5c37fa34d..6cc372ce98a6d 100644 --- a/clang-tools-extra/clang-doc/Serialize.cpp +++ b/clang-tools-extra/clang-doc/Serialize.cpp @@ -8,8 +8,10 @@ #include "Serialize.h" #include "BitcodeWriter.h" + #include "clang/AST/Attr.h" #include "clang/AST/Comment.h" +#include "clang/AST/DeclFriend.h" #include "clang/Index/USRGeneration.h" #include "clang/Lex/Lexer.h" #include "llvm/ADT/StringExtras.h" @@ -401,6 +403,9 @@ std::string serialize(std::unique_ptr &I) { return serialize(*static_cast(I.get())); case InfoType::IT_concept: return serialize(*static_cast(I.get())); + case InfoType::IT_variable: + return serialize(*static_cast(I.get())); + case InfoType::IT_friend: case InfoType::IT_typedef: case InfoType::IT_default: return ""; @@ -508,6 +513,10 @@ static void InsertChild(ScopeChildren &Scope, ConceptInfo Info) { Scope.Concepts.push_back(std::move(Info)); } +static void InsertChild(ScopeChildren &Scope, VarInfo Info) { + Scope.Variables.push_back(std::move(Info)); +} + // Creates a parent of the correct type for the given child and inserts it into // that parent. // @@ -549,6 +558,8 @@ static std::unique_ptr makeAndInsertIntoParent(ChildType Child) { case InfoType::IT_function: case InfoType::IT_typedef: case InfoType::IT_concept: + case InfoType::IT_variable: + case InfoType::IT_friend: break; } llvm_unreachable("Invalid reference type for parent namespace"); @@ -940,6 +951,55 @@ emitInfo(const NamespaceDecl *D, const FullComment *FC, Location Loc, return {std::move(NSI), makeAndInsertIntoParent(*NSI)}; } +static void parseFriends(RecordInfo &RI, const CXXRecordDecl *D) { + if (!D->hasDefinition() || !D->hasFriends()) + return; + + for (const FriendDecl *FD : D->friends()) { + if (FD->isUnsupportedFriend()) + continue; + + FriendInfo F(InfoType::IT_friend, getUSRForDecl(FD)); + const auto *ActualDecl = FD->getFriendDecl(); + if (!ActualDecl) { + const auto *FriendTypeInfo = FD->getFriendType(); + if (!FriendTypeInfo) + continue; + ActualDecl = FriendTypeInfo->getType()->getAsCXXRecordDecl(); + + if (!ActualDecl) + continue; + F.IsClass = true; + } + + if (const auto *ActualTD = dyn_cast_or_null(ActualDecl)) { + if (isa(ActualTD->getTemplatedDecl())) + F.IsClass = true; + F.Template.emplace(); + for (const auto *Param : ActualTD->getTemplateParameters()->asArray()) + F.Template->Params.emplace_back( + getSourceCode(Param, Param->getSourceRange())); + ActualDecl = ActualTD->getTemplatedDecl(); + } + + if (auto *FuncDecl = dyn_cast_or_null(ActualDecl)) { + FunctionInfo TempInfo; + parseParameters(TempInfo, FuncDecl); + F.Params.emplace(); + F.Params = std::move(TempInfo.Params); + F.ReturnType = getTypeInfoForType(FuncDecl->getReturnType(), + FuncDecl->getLangOpts()); + } + + F.Ref = + Reference(getUSRForDecl(ActualDecl), ActualDecl->getNameAsString(), + InfoType::IT_default, ActualDecl->getQualifiedNameAsString(), + getInfoRelativePath(ActualDecl)); + + RI.Friends.push_back(std::move(F)); + } +} + std::pair, std::unique_ptr> emitInfo(const RecordDecl *D, const FullComment *FC, Location Loc, bool PublicOnly) { @@ -963,6 +1023,7 @@ emitInfo(const RecordDecl *D, const FullComment *FC, Location Loc, // TODO: remove first call to parseBases, that function should be deleted parseBases(*RI, C); parseBases(*RI, C, /*IsFileInRootDir=*/true, PublicOnly, /*IsParent=*/true); + parseFriends(*RI, C); } RI->Path = getInfoRelativePath(RI->Namespace); @@ -1164,6 +1225,26 @@ emitInfo(const ConceptDecl *D, const FullComment *FC, const Location &Loc, return {nullptr, makeAndInsertIntoParent(std::move(Concept))}; } +std::pair, std::unique_ptr> +emitInfo(const VarDecl *D, const FullComment *FC, const Location &Loc, + bool PublicOnly) { + VarInfo Var; + bool IsInAnonymousNamespace = false; + populateSymbolInfo(Var, D, FC, Loc, IsInAnonymousNamespace); + if (!shouldSerializeInfo(PublicOnly, IsInAnonymousNamespace, D)) + return {}; + + if (D->getStorageClass() == StorageClass::SC_Static) + Var.IsStatic = true; + Var.Type = + getTypeInfoForType(D->getType(), D->getASTContext().getPrintingPolicy()); + + if (!shouldSerializeInfo(PublicOnly, IsInAnonymousNamespace, D)) + return {}; + + return {nullptr, makeAndInsertIntoParent(std::move(Var))}; +} + } // namespace serialize } // namespace doc } // namespace clang diff --git a/clang-tools-extra/clang-doc/Serialize.h b/clang-tools-extra/clang-doc/Serialize.h index 497b09bb339f8..06c4d64c51494 100644 --- a/clang-tools-extra/clang-doc/Serialize.h +++ b/clang-tools-extra/clang-doc/Serialize.h @@ -72,6 +72,10 @@ std::pair, std::unique_ptr> emitInfo(const ConceptDecl *D, const FullComment *FC, const Location &Loc, bool PublicOnly); +std::pair, std::unique_ptr> +emitInfo(const VarDecl *D, const FullComment *FC, const Location &Loc, + bool PublicOnly); + // Function to hash a given USR value for storage. // As USRs (Unified Symbol Resolution) could be large, especially for functions // with long type arguments, we use 160-bits SHA1(USR) values to diff --git a/clang-tools-extra/clang-doc/YAMLGenerator.cpp b/clang-tools-extra/clang-doc/YAMLGenerator.cpp index f958871046981..eeccdd804b669 100644 --- a/clang-tools-extra/clang-doc/YAMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/YAMLGenerator.cpp @@ -409,6 +409,8 @@ llvm::Error YAMLGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, InfoYAML << *static_cast(I); break; case InfoType::IT_concept: + case InfoType::IT_variable: + case InfoType::IT_friend: break; case InfoType::IT_default: return llvm::createStringError(llvm::inconvertibleErrorCode(), diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index a71813a103df3..c35f0b941c600 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -358,8 +358,27 @@ getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix) { } // namespace clang::tidy +void ClangTidyDiagnosticConsumer::BeginSourceFile(const LangOptions &LangOpts, + const Preprocessor *PP) { + DiagnosticConsumer::BeginSourceFile(LangOpts, PP); + + assert(!InSourceFile); + InSourceFile = true; +} + +void ClangTidyDiagnosticConsumer::EndSourceFile() { + assert(InSourceFile); + InSourceFile = false; + + DiagnosticConsumer::EndSourceFile(); +} + void ClangTidyDiagnosticConsumer::HandleDiagnostic( DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) { + // A diagnostic should not be reported outside of a + // BeginSourceFile()/EndSourceFile() pair if it has a source location. + assert(InSourceFile || Info.getLocation().isInvalid()); + if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note) return; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index a8851e794f24b..6e7cb7bb10e57 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -292,6 +292,11 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer { void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) override; + void BeginSourceFile(const LangOptions &LangOpts, + const Preprocessor *PP = nullptr) override; + + void EndSourceFile() override; + // Retrieve the diagnostics that were captured. std::vector take(); @@ -326,6 +331,11 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer { bool LastErrorRelatesToUserCode = false; bool LastErrorPassesLineFilter = false; bool LastErrorWasIgnored = false; + /// Tracks whether we're currently inside a + /// `BeginSourceFile()/EndSourceFile()` pair. Outside of a source file, we + /// should only receive diagnostics that have to source location, such as + /// command-line warnings. + bool InSourceFile = false; }; } // end namespace tidy diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp index 9eeba867f5211..88d2f2c388d07 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp @@ -72,7 +72,9 @@ SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name, Options.get("WarnOnSizeOfPointerToAggregate", true)), WarnOnSizeOfPointer(Options.get("WarnOnSizeOfPointer", false)), WarnOnOffsetDividedBySizeOf( - Options.get("WarnOnOffsetDividedBySizeOf", true)) {} + Options.get("WarnOnOffsetDividedBySizeOf", true)), + WarnOnSizeOfInLoopTermination( + Options.get("WarnOnSizeOfInLoopTermination", true)) {} void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant); @@ -86,6 +88,8 @@ void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "WarnOnSizeOfPointer", WarnOnSizeOfPointer); Options.store(Opts, "WarnOnOffsetDividedBySizeOf", WarnOnOffsetDividedBySizeOf); + Options.store(Opts, "WarnOnSizeOfInLoopTermination", + WarnOnSizeOfInLoopTermination); } void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { @@ -93,6 +97,13 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { // Some of the checks should not match in template code to avoid false // positives if sizeof is applied on template argument. + auto LoopCondExpr = + [](const ast_matchers::internal::Matcher &InnerMatcher) { + return stmt(anyOf(forStmt(hasCondition(InnerMatcher)), + whileStmt(hasCondition(InnerMatcher)), + doStmt(hasCondition(InnerMatcher)))); + }; + const auto IntegerExpr = ignoringParenImpCasts(integerLiteral()); const auto ConstantExpr = ignoringParenImpCasts( anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)), @@ -130,6 +141,14 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) { this); } + if (WarnOnSizeOfInLoopTermination) { + auto CondExpr = binaryOperator( + allOf(has(SizeOfExpr.bind("sizeof-expr")), isComparisonOperator())); + Finder->addMatcher(LoopCondExpr(anyOf(CondExpr, hasDescendant(CondExpr))) + .bind("loop-expr"), + this); + } + // Detect sizeof(kPtr) where kPtr is 'const char* kPtr = "abc"'; const auto CharPtrType = pointerType(pointee(isAnyCharacter())); const auto ConstStrLiteralDecl = @@ -349,6 +368,23 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) { diag(E->getBeginLoc(), "suspicious usage of 'sizeof(char*)'; do you mean 'strlen'?") << E->getSourceRange(); + } else if (Result.Nodes.getNodeAs("loop-expr")) { + auto *SizeofArgTy = Result.Nodes.getNodeAs("sizeof-arg-type"); + if (const auto member = dyn_cast(SizeofArgTy)) + SizeofArgTy = member->getPointeeType().getTypePtr(); + + const auto *SzOfExpr = Result.Nodes.getNodeAs("sizeof-expr"); + + if (const auto type = dyn_cast(SizeofArgTy)) { + // check if the array element size is larger than one. If true, + // the size of the array is higher than the number of elements + CharUnits sSize = Ctx.getTypeSizeInChars(type->getElementType()); + if (!sSize.isOne()) { + diag(SzOfExpr->getBeginLoc(), + "suspicious usage of 'sizeof' in the loop") + << SzOfExpr->getSourceRange(); + } + } } else if (const auto *E = Result.Nodes.getNodeAs("sizeof-pointer")) { diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression " "of pointer type") diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h index fbd62cb80fb2d..e979b4723cf2e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.h @@ -32,6 +32,7 @@ class SizeofExpressionCheck : public ClangTidyCheck { const bool WarnOnSizeOfPointerToAggregate; const bool WarnOnSizeOfPointer; const bool WarnOnOffsetDividedBySizeOf; + const bool WarnOnSizeOfInLoopTermination; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp index 8ffa44d41fa98..b14587ad7db83 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp @@ -17,8 +17,20 @@ namespace { AST_MATCHER(GotoStmt, isForwardJumping) { return Node.getBeginLoc() < Node.getLabel()->getBeginLoc(); } + +AST_MATCHER(GotoStmt, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} } // namespace +AvoidGotoCheck::AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreMacros(Options.get("IgnoreMacros", false)) {} + +void AvoidGotoCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreMacros", IgnoreMacros); +} + void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { // TODO: This check does not recognize `IndirectGotoStmt` which is a // GNU extension. These must be matched separately and an AST matcher @@ -29,7 +41,10 @@ void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) { auto Loop = mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt); auto NestedLoop = Loop.with(hasAncestor(Loop)); - Finder->addMatcher(gotoStmt(anyOf(unless(hasAncestor(NestedLoop)), + const ast_matchers::internal::Matcher Anything = anything(); + + Finder->addMatcher(gotoStmt(IgnoreMacros ? unless(isInMacro()) : Anything, + anyOf(unless(hasAncestor(NestedLoop)), unless(isForwardJumping()))) .bind("goto"), this); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h index 883ba78855e7c..8eae409462c91 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-goto.html class AvoidGotoCheck : public ClangTidyCheck { public: - AvoidGotoCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + AvoidGotoCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp index cf299609e646d..268b51f76a2c3 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp @@ -120,7 +120,7 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) { equalsBoundNode("param"), equalsBoundNode("var")))))), CapturedInLambda)), callee(unresolvedLookupExpr(hasAnyDeclaration( - namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))), + namedDecl(hasUnderlyingDecl(hasName(ForwardFunction)))))), unless(anyOf(hasAncestor(typeLoc()), hasAncestor(expr(hasUnevaluatedContext()))))); @@ -149,4 +149,13 @@ void MissingStdForwardCheck::check(const MatchFinder::MatchResult &Result) { << Param; } +MissingStdForwardCheck::MissingStdForwardCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + ForwardFunction(Options.get("ForwardFunction", "::std::forward")) {} + +void MissingStdForwardCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "ForwardFunction", ForwardFunction); +} + } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h index 5995ad588855e..f833b8031f8af 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.h @@ -21,8 +21,7 @@ namespace clang::tidy::cppcoreguidelines { /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/missing-std-forward.html class MissingStdForwardCheck : public ClangTidyCheck { public: - MissingStdForwardCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + MissingStdForwardCheck(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 { @@ -31,6 +30,10 @@ class MissingStdForwardCheck : public ClangTidyCheck { std::optional getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const StringRef ForwardFunction; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp index a1494a095f5b6..9ac7b9e057e35 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp @@ -15,16 +15,13 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { void ProBoundsPointerArithmeticCheck::registerMatchers(MatchFinder *Finder) { - if (!getLangOpts().CPlusPlus) - return; - const auto AllPointerTypes = - anyOf(hasType(pointerType()), + anyOf(hasType(hasUnqualifiedDesugaredType(pointerType())), hasType(autoType( hasDeducedType(hasUnqualifiedDesugaredType(pointerType())))), hasType(decltypeType(hasUnderlyingType(pointerType())))); - // Flag all operators +, -, +=, -=, ++, -- that result in a pointer + // Flag all operators +, -, +=, -= that result in a pointer Finder->addMatcher( binaryOperator( hasAnyOperatorName("+", "-", "+=", "-="), AllPointerTypes, @@ -32,8 +29,12 @@ void ProBoundsPointerArithmeticCheck::registerMatchers(MatchFinder *Finder) { .bind("expr"), this); + // Flag all operators ++, -- that result in a pointer Finder->addMatcher( - unaryOperator(hasAnyOperatorName("++", "--"), hasType(pointerType())) + unaryOperator(hasAnyOperatorName("++", "--"), + hasType(hasUnqualifiedDesugaredType(pointerType())), + unless(hasUnaryOperand( + ignoringImpCasts(declRefExpr(to(isImplicit())))))) .bind("expr"), this); @@ -42,7 +43,8 @@ void ProBoundsPointerArithmeticCheck::registerMatchers(MatchFinder *Finder) { arraySubscriptExpr( hasBase(ignoringImpCasts( anyOf(AllPointerTypes, - hasType(decayedType(hasDecayedType(pointerType()))))))) + hasType(decayedType(hasDecayedType(pointerType())))))), + hasIndex(hasType(isInteger()))) .bind("expr"), this); } diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.h index 67f7d1bf9dd69..3466c72a769e9 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.h @@ -23,6 +23,9 @@ class ProBoundsPointerArithmeticCheck : public ClangTidyCheck { public: ProBoundsPointerArithmeticCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, 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; }; diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp index 8c386d5bc7945..272152644d7dd 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp @@ -42,9 +42,9 @@ void RvalueReferenceParamNotMovedCheck::registerMatchers(MatchFinder *Finder) { StatementMatcher MoveCallMatcher = callExpr( argumentCountIs(1), - anyOf(callee(functionDecl(hasName("::std::move"))), + anyOf(callee(functionDecl(hasName(MoveFunction))), callee(unresolvedLookupExpr(hasAnyDeclaration( - namedDecl(hasUnderlyingDecl(hasName("::std::move"))))))), + namedDecl(hasUnderlyingDecl(hasName(MoveFunction))))))), hasArgument( 0, argumentOf( AllowPartialMove, @@ -122,7 +122,8 @@ RvalueReferenceParamNotMovedCheck::RvalueReferenceParamNotMovedCheck( AllowPartialMove(Options.get("AllowPartialMove", false)), IgnoreUnnamedParams(Options.get("IgnoreUnnamedParams", false)), IgnoreNonDeducedTemplateTypes( - Options.get("IgnoreNonDeducedTemplateTypes", false)) {} + Options.get("IgnoreNonDeducedTemplateTypes", false)), + MoveFunction(Options.get("MoveFunction", "::std::move")) {} void RvalueReferenceParamNotMovedCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { @@ -130,6 +131,7 @@ void RvalueReferenceParamNotMovedCheck::storeOptions( Options.store(Opts, "IgnoreUnnamedParams", IgnoreUnnamedParams); Options.store(Opts, "IgnoreNonDeducedTemplateTypes", IgnoreNonDeducedTemplateTypes); + Options.store(Opts, "MoveFunction", MoveFunction); } } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h index d8c3d2bd4ba0e..950c0206745d7 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h @@ -32,6 +32,7 @@ class RvalueReferenceParamNotMovedCheck : public ClangTidyCheck { const bool AllowPartialMove; const bool IgnoreUnnamedParams; const bool IgnoreNonDeducedTemplateTypes; + const StringRef MoveFunction; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp index 0de143dbb1b89..0b6b8d9c97135 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp @@ -18,6 +18,12 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { +namespace { +AST_MATCHER(CXXRecordDecl, isInMacro) { + return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID(); +} +} // namespace + SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck( StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), AllowMissingMoveFunctions(Options.get( @@ -26,7 +32,8 @@ SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck( AllowMissingMoveFunctionsWhenCopyIsDeleted( Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", false)), AllowImplicitlyDeletedCopyOrMove( - Options.get("AllowImplicitlyDeletedCopyOrMove", false)) {} + Options.get("AllowImplicitlyDeletedCopyOrMove", false)), + IgnoreMacros(Options.get("IgnoreMacros", true)) {} void SpecialMemberFunctionsCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { @@ -36,6 +43,7 @@ void SpecialMemberFunctionsCheck::storeOptions( AllowMissingMoveFunctionsWhenCopyIsDeleted); Options.store(Opts, "AllowImplicitlyDeletedCopyOrMove", AllowImplicitlyDeletedCopyOrMove); + Options.store(Opts, "IgnoreMacros", IgnoreMacros); } std::optional @@ -45,11 +53,12 @@ SpecialMemberFunctionsCheck::getCheckTraversalKind() const { } void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) { - auto IsNotImplicitOrDeleted = anyOf(unless(isImplicit()), isDeleted()); + const auto IsNotImplicitOrDeleted = anyOf(unless(isImplicit()), isDeleted()); + const ast_matchers::internal::Matcher Anything = anything(); Finder->addMatcher( cxxRecordDecl( - unless(isImplicit()), + unless(isImplicit()), IgnoreMacros ? unless(isInMacro()) : Anything, eachOf(has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")), has(cxxConstructorDecl(isCopyConstructor(), IsNotImplicitOrDeleted) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h index dee01cb5a9fdd..c18ed7db055ba 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.h @@ -69,6 +69,7 @@ class SpecialMemberFunctionsCheck : public ClangTidyCheck { const bool AllowMissingMoveFunctionsWhenCopyIsDeleted; const bool AllowImplicitlyDeletedCopyOrMove; ClassDefiningSpecialMembersMap ClassWithSpecialMembers; + const bool IgnoreMacros; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp index 7639476980529..79ae5ee98182b 100644 --- a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp @@ -1,5 +1,4 @@ -//===--- ConfusableIdentifierCheck.cpp - -// clang-tidy--------------------------===// +//===--- ConfusableIdentifierCheck.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. @@ -9,6 +8,7 @@ #include "ConfusableIdentifierCheck.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/ConvertUTF.h" @@ -88,99 +88,98 @@ static llvm::SmallString<64U> skeleton(StringRef Name) { return Skeleton; } -static bool mayShadowImpl(const DeclContext *DC0, const DeclContext *DC1) { - return DC0 && DC0 == DC1; -} - -static bool mayShadowImpl(const NamedDecl *ND0, const NamedDecl *ND1) { - return isa(ND0) || isa(ND1); -} - -static bool isMemberOf(const ConfusableIdentifierCheck::ContextInfo *DC0, - const ConfusableIdentifierCheck::ContextInfo *DC1) { - return llvm::is_contained(DC1->Bases, DC0->PrimaryContext); -} - -static bool enclosesContext(const ConfusableIdentifierCheck::ContextInfo *DC0, - const ConfusableIdentifierCheck::ContextInfo *DC1) { - if (DC0->PrimaryContext == DC1->PrimaryContext) - return true; - - return llvm::is_contained(DC0->PrimaryContexts, DC1->PrimaryContext) || - llvm::is_contained(DC1->PrimaryContexts, DC0->PrimaryContext); -} - -static bool mayShadow(const NamedDecl *ND0, - const ConfusableIdentifierCheck::ContextInfo *DC0, - const NamedDecl *ND1, - const ConfusableIdentifierCheck::ContextInfo *DC1) { - - if (!DC0->Bases.empty() && !DC1->Bases.empty()) { - // if any of the declaration is a non-private member of the other - // declaration, it's shadowed by the former - - if (ND1->getAccess() != AS_private && isMemberOf(DC1, DC0)) - return true; +namespace { +struct Entry { + const NamedDecl *ND; + const Decl *Parent; + bool FromDerivedClass; +}; +} // namespace - if (ND0->getAccess() != AS_private && isMemberOf(DC0, DC1)) +// Map from a context to the declarations in that context with the current +// skeleton. At most one entry per distinct identifier is tracked. The +// context is usually a `DeclContext`, but can also be a template declaration +// that has no corresponding context, such as an alias template or variable +// template. +using DeclsWithinContextMap = + llvm::DenseMap>; + +static bool addToContext(DeclsWithinContextMap &DeclsWithinContext, + const Decl *Context, Entry E) { + auto &Decls = DeclsWithinContext[Context]; + if (!Decls.empty() && + Decls.back().ND->getIdentifier() == E.ND->getIdentifier()) { + // Already have a declaration with this identifier in this context. Don't + // track another one. This means that if an outer name is confusable with an + // inner name, we'll only diagnose the outer name once, pointing at the + // first inner declaration with that name. + if (Decls.back().FromDerivedClass && !E.FromDerivedClass) { + // Prefer the declaration that's not from the derived class, because that + // conflicts with more declarations. + Decls.back() = E; return true; - } - - if (!mayShadowImpl(DC0->NonTransparentContext, DC1->NonTransparentContext) && - !mayShadowImpl(ND0, ND1)) + } return false; - - return enclosesContext(DC0, DC1); + } + Decls.push_back(E); + return true; } -const ConfusableIdentifierCheck::ContextInfo * -ConfusableIdentifierCheck::getContextInfo(const DeclContext *DC) { - const DeclContext *PrimaryContext = DC->getPrimaryContext(); - auto [It, Inserted] = ContextInfos.try_emplace(PrimaryContext); - if (!Inserted) - return &It->second; - - ContextInfo &Info = It->second; - Info.PrimaryContext = PrimaryContext; - Info.NonTransparentContext = PrimaryContext; +static void addToEnclosingContexts(DeclsWithinContextMap &DeclsWithinContext, + const Decl *Parent, const NamedDecl *ND) { + const Decl *Outer = Parent; + while (Outer) { + if (const auto *NS = dyn_cast(Outer)) + Outer = NS->getCanonicalDecl(); + + if (!addToContext(DeclsWithinContext, Outer, {ND, Parent, false})) + return; + + if (const auto *RD = dyn_cast(Outer)) { + RD = RD->getDefinition(); + if (RD) { + RD->forallBases([&](const CXXRecordDecl *Base) { + addToContext(DeclsWithinContext, Base, {ND, Parent, true}); + return true; + }); + } + } - while (Info.NonTransparentContext->isTransparentContext()) { - Info.NonTransparentContext = Info.NonTransparentContext->getParent(); - if (!Info.NonTransparentContext) + auto *OuterDC = Outer->getDeclContext(); + if (!OuterDC) break; + Outer = cast_or_null(OuterDC->getNonTransparentContext()); } +} + +void ConfusableIdentifierCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + const auto *ND = Result.Nodes.getNodeAs("nameddecl"); + if (!ND) + return; - if (Info.NonTransparentContext) - Info.NonTransparentContext = - Info.NonTransparentContext->getPrimaryContext(); + addDeclToCheck(ND, + cast(ND->getDeclContext()->getNonTransparentContext())); - while (DC) { - if (!isa(DC) && !isa(DC)) - Info.PrimaryContexts.push_back(DC->getPrimaryContext()); - DC = DC->getParent(); + // Associate template parameters with this declaration of this template. + if (const auto *TD = dyn_cast(ND)) { + for (const NamedDecl *Param : *TD->getTemplateParameters()) + addDeclToCheck(Param, TD->getTemplatedDecl()); } - if (const auto *RD = dyn_cast(PrimaryContext)) { - RD = RD->getDefinition(); - if (RD) { - Info.Bases.push_back(RD); - RD->forallBases([&](const CXXRecordDecl *Base) { - Info.Bases.push_back(Base); - return false; - }); - } + // Associate function parameters with this declaration of this function. + if (const auto *FD = dyn_cast(ND)) { + for (const NamedDecl *Param : FD->parameters()) + addDeclToCheck(Param, ND); } - - return &Info; } -void ConfusableIdentifierCheck::check( - const ast_matchers::MatchFinder::MatchResult &Result) { - const auto *ND = Result.Nodes.getNodeAs("nameddecl"); - if (!ND) +void ConfusableIdentifierCheck::addDeclToCheck(const NamedDecl *ND, + const Decl *Parent) { + if (!ND || !Parent) return; - IdentifierInfo *NDII = ND->getIdentifier(); + const IdentifierInfo *NDII = ND->getIdentifier(); if (!NDII) return; @@ -188,34 +187,78 @@ void ConfusableIdentifierCheck::check( if (NDName.empty()) return; - const ContextInfo *Info = getContextInfo(ND->getDeclContext()); + NameToDecls[NDII].push_back({ND, Parent}); +} + +void ConfusableIdentifierCheck::onEndOfTranslationUnit() { + llvm::StringMap> SkeletonToNames; + // Compute the skeleton for each identifier. + for (auto &[Ident, Decls] : NameToDecls) { + SkeletonToNames[skeleton(Ident->getName())].push_back(Ident); + } - llvm::SmallVector &Mapped = Mapper[skeleton(NDName)]; - for (const Entry &E : Mapped) { - if (!mayShadow(ND, Info, E.Declaration, E.Info)) + // Visit each skeleton with more than one identifier. + for (auto &[Skel, Idents] : SkeletonToNames) { + if (Idents.size() < 2) { continue; + } - const IdentifierInfo *ONDII = E.Declaration->getIdentifier(); - StringRef ONDName = ONDII->getName(); - if (ONDName == NDName) - continue; + // Find the declaration contexts that transitively contain each identifier. + DeclsWithinContextMap DeclsWithinContext; + for (const IdentifierInfo *II : Idents) { + for (auto [ND, Parent] : NameToDecls[II]) { + addToEnclosingContexts(DeclsWithinContext, Parent, ND); + } + } - diag(ND->getLocation(), "%0 is confusable with %1") << ND << E.Declaration; - diag(E.Declaration->getLocation(), "other declaration found here", - DiagnosticIDs::Note); + // Check to see if any declaration is declared in a context that + // transitively contains another declaration with a different identifier but + // the same skeleton. + for (const IdentifierInfo *II : Idents) { + for (auto [OuterND, OuterParent] : NameToDecls[II]) { + for (Entry Inner : DeclsWithinContext[OuterParent]) { + // Don't complain if the identifiers are the same. + if (OuterND->getIdentifier() == Inner.ND->getIdentifier()) + continue; + + // Don't complain about a derived-class name shadowing a base class + // private member. + if (OuterND->getAccess() == AS_private && Inner.FromDerivedClass) + continue; + + // If the declarations are in the same context, only diagnose the + // later one. + if (OuterParent == Inner.Parent && + Inner.ND->getASTContext() + .getSourceManager() + .isBeforeInTranslationUnit(Inner.ND->getLocation(), + OuterND->getLocation())) + continue; + + diag(Inner.ND->getLocation(), "%0 is confusable with %1") + << Inner.ND << OuterND; + diag(OuterND->getLocation(), "other declaration found here", + DiagnosticIDs::Note); + } + } + } } - Mapped.push_back({ND, Info}); -} - -void ConfusableIdentifierCheck::onEndOfTranslationUnit() { - Mapper.clear(); - ContextInfos.clear(); + NameToDecls.clear(); } void ConfusableIdentifierCheck::registerMatchers( ast_matchers::MatchFinder *Finder) { - Finder->addMatcher(ast_matchers::namedDecl().bind("nameddecl"), this); + // Parameter declarations sometimes use the translation unit or some outer + // enclosing context as their `DeclContext`, instead of their parent, so + // we handle them specially in `check`. + auto AnyParamDecl = ast_matchers::anyOf( + ast_matchers::parmVarDecl(), ast_matchers::templateTypeParmDecl(), + ast_matchers::nonTypeTemplateParmDecl(), + ast_matchers::templateTemplateParmDecl()); + Finder->addMatcher(ast_matchers::namedDecl(ast_matchers::unless(AnyParamDecl)) + .bind("nameddecl"), + this); } } // namespace clang::tidy::misc diff --git a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h index f3b0c8ed00306..9cce6cce67682 100644 --- a/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h +++ b/clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.h @@ -1,5 +1,4 @@ -//===--- ConfusableIdentifierCheck.h - clang-tidy -//-------------------------------*- C++ -*-===// +//===--- ConfusableIdentifierCheck.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. @@ -11,7 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CONFUSABLE_IDENTIFIER_CHECK_H #include "../ClangTidyCheck.h" -#include +#include "llvm/ADT/DenseMap.h" namespace clang::tidy::misc { @@ -31,23 +30,13 @@ class ConfusableIdentifierCheck : public ClangTidyCheck { return TK_IgnoreUnlessSpelledInSource; } - struct ContextInfo { - const DeclContext *PrimaryContext; - const DeclContext *NonTransparentContext; - llvm::SmallVector PrimaryContexts; - llvm::SmallVector Bases; - }; - private: - struct Entry { - const NamedDecl *Declaration; - const ContextInfo *Info; - }; - - const ContextInfo *getContextInfo(const DeclContext *DC); + void addDeclToCheck(const NamedDecl *ND, const Decl *Parent); - llvm::StringMap> Mapper; - std::unordered_map ContextInfos; + llvm::DenseMap< + const IdentifierInfo *, + llvm::SmallVector, 1>> + NameToDecls; }; } // namespace clang::tidy::misc diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index bab1167fb15ff..619a27b2f9bb6 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -42,6 +42,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseNullptrCheck.cpp UseOverrideCheck.cpp UseRangesCheck.cpp + UseScopedLockCheck.cpp UseStartsEndsWithCheck.cpp UseStdFormatCheck.cpp UseStdNumbersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 0cf59b6e0216a..fdf38bc4b6308 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -43,6 +43,7 @@ #include "UseNullptrCheck.h" #include "UseOverrideCheck.h" #include "UseRangesCheck.h" +#include "UseScopedLockCheck.h" #include "UseStartsEndsWithCheck.h" #include "UseStdFormatCheck.h" #include "UseStdNumbersCheck.h" @@ -80,6 +81,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck( "modernize-use-integer-sign-comparison"); CheckFactories.registerCheck("modernize-use-ranges"); + CheckFactories.registerCheck( + "modernize-use-scoped-lock"); CheckFactories.registerCheck( "modernize-use-starts-ends-with"); CheckFactories.registerCheck("modernize-use-std-format"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp new file mode 100644 index 0000000000000..9c2fc9e06fb45 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.cpp @@ -0,0 +1,311 @@ +//===--- UseScopedLockCheck.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 "UseScopedLockCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/Twine.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +static bool isLockGuardDecl(const NamedDecl *Decl) { + return Decl->getDeclName().isIdentifier() && + Decl->getName() == "lock_guard" && Decl->isInStdNamespace(); +} + +static bool isLockGuard(const QualType &Type) { + if (const auto *Record = Type->getAs()) + if (const RecordDecl *Decl = Record->getDecl()) + return isLockGuardDecl(Decl); + + if (const auto *TemplateSpecType = Type->getAs()) + if (const TemplateDecl *Decl = + TemplateSpecType->getTemplateName().getAsTemplateDecl()) + return isLockGuardDecl(Decl); + + return false; +} + +static llvm::SmallVector +getLockGuardsFromDecl(const DeclStmt *DS) { + llvm::SmallVector LockGuards; + + for (const Decl *Decl : DS->decls()) { + if (const auto *VD = dyn_cast(Decl)) { + const QualType Type = + VD->getType().getCanonicalType().getUnqualifiedType(); + if (isLockGuard(Type)) + LockGuards.push_back(VD); + } + } + + return LockGuards; +} + +// Scans through the statements in a block and groups consecutive +// 'std::lock_guard' variable declarations together. +static llvm::SmallVector> +findLocksInCompoundStmt(const CompoundStmt *Block, + const ast_matchers::MatchFinder::MatchResult &Result) { + // store groups of consecutive 'std::lock_guard' declarations + llvm::SmallVector> LockGuardGroups; + llvm::SmallVector CurrentLockGuardGroup; + + auto AddAndClearCurrentGroup = [&]() { + if (!CurrentLockGuardGroup.empty()) { + LockGuardGroups.push_back(CurrentLockGuardGroup); + CurrentLockGuardGroup.clear(); + } + }; + + for (const Stmt *Stmt : Block->body()) { + if (const auto *DS = dyn_cast(Stmt)) { + llvm::SmallVector LockGuards = getLockGuardsFromDecl(DS); + + if (!LockGuards.empty()) { + CurrentLockGuardGroup.append(LockGuards); + continue; + } + } + AddAndClearCurrentGroup(); + } + + AddAndClearCurrentGroup(); + + return LockGuardGroups; +} + +static TemplateSpecializationTypeLoc +getTemplateLockGuardTypeLoc(const TypeSourceInfo *SourceInfo) { + const TypeLoc Loc = SourceInfo->getTypeLoc(); + + const auto ElaboratedLoc = Loc.getAs(); + if (!ElaboratedLoc) + return {}; + + return ElaboratedLoc.getNamedTypeLoc().getAs(); +} + +// Find the exact source range of the 'lock_guard' token +static SourceRange getLockGuardRange(const TypeSourceInfo *SourceInfo) { + const TypeLoc LockGuardTypeLoc = SourceInfo->getTypeLoc(); + + return SourceRange(LockGuardTypeLoc.getBeginLoc(), + LockGuardTypeLoc.getEndLoc()); +} + +// Find the exact source range of the 'lock_guard' name token +static SourceRange getLockGuardNameRange(const TypeSourceInfo *SourceInfo) { + const TemplateSpecializationTypeLoc TemplateLoc = + getTemplateLockGuardTypeLoc(SourceInfo); + if (!TemplateLoc) + return {}; + + return SourceRange(TemplateLoc.getTemplateNameLoc(), + TemplateLoc.getLAngleLoc().getLocWithOffset(-1)); +} + +const static StringRef UseScopedLockMessage = + "use 'std::scoped_lock' instead of 'std::lock_guard'"; + +UseScopedLockCheck::UseScopedLockCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + WarnOnSingleLocks(Options.get("WarnOnSingleLocks", true)), + WarnOnUsingAndTypedef(Options.get("WarnOnUsingAndTypedef", true)) {} + +void UseScopedLockCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "WarnOnSingleLocks", WarnOnSingleLocks); + Options.store(Opts, "WarnOnUsingAndTypedef", WarnOnUsingAndTypedef); +} + +void UseScopedLockCheck::registerMatchers(MatchFinder *Finder) { + const auto LockGuardClassDecl = + namedDecl(hasName("lock_guard"), isInStdNamespace()); + + const auto LockGuardType = qualType(anyOf( + hasUnqualifiedDesugaredType( + recordType(hasDeclaration(LockGuardClassDecl))), + elaboratedType(namesType(hasUnqualifiedDesugaredType( + templateSpecializationType(hasDeclaration(LockGuardClassDecl))))))); + + const auto LockVarDecl = varDecl(hasType(LockGuardType)); + + if (WarnOnSingleLocks) { + Finder->addMatcher( + compoundStmt( + unless(isExpansionInSystemHeader()), + has(declStmt(has(LockVarDecl)).bind("lock-decl-single")), + unless(has(declStmt(unless(equalsBoundNode("lock-decl-single")), + has(LockVarDecl))))), + this); + } + + Finder->addMatcher( + compoundStmt(unless(isExpansionInSystemHeader()), + has(declStmt(has(LockVarDecl)).bind("lock-decl-multiple")), + has(declStmt(unless(equalsBoundNode("lock-decl-multiple")), + has(LockVarDecl)))) + .bind("block-multiple"), + this); + + if (WarnOnUsingAndTypedef) { + // Match 'typedef std::lock_guard Lock' + Finder->addMatcher(typedefDecl(unless(isExpansionInSystemHeader()), + hasUnderlyingType(LockGuardType)) + .bind("lock-guard-typedef"), + this); + + // Match 'using Lock = std::lock_guard' + Finder->addMatcher( + typeAliasDecl( + unless(isExpansionInSystemHeader()), + hasType(elaboratedType(namesType(templateSpecializationType( + hasDeclaration(LockGuardClassDecl)))))) + .bind("lock-guard-using-alias"), + this); + + // Match 'using std::lock_guard' + Finder->addMatcher( + usingDecl(unless(isExpansionInSystemHeader()), + hasAnyUsingShadowDecl(hasTargetDecl(LockGuardClassDecl))) + .bind("lock-guard-using-decl"), + this); + } +} + +void UseScopedLockCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *DS = Result.Nodes.getNodeAs("lock-decl-single")) { + llvm::SmallVector Decls = getLockGuardsFromDecl(DS); + diagOnMultipleLocks({Decls}, Result); + return; + } + + if (const auto *Compound = + Result.Nodes.getNodeAs("block-multiple")) { + diagOnMultipleLocks(findLocksInCompoundStmt(Compound, Result), Result); + return; + } + + if (const auto *Typedef = + Result.Nodes.getNodeAs("lock-guard-typedef")) { + diagOnSourceInfo(Typedef->getTypeSourceInfo(), Result); + return; + } + + if (const auto *UsingAlias = + Result.Nodes.getNodeAs("lock-guard-using-alias")) { + diagOnSourceInfo(UsingAlias->getTypeSourceInfo(), Result); + return; + } + + if (const auto *Using = + Result.Nodes.getNodeAs("lock-guard-using-decl")) { + diagOnUsingDecl(Using, Result); + } +} + +void UseScopedLockCheck::diagOnSingleLock( + const VarDecl *LockGuard, const MatchFinder::MatchResult &Result) { + auto Diag = diag(LockGuard->getBeginLoc(), UseScopedLockMessage); + + const SourceRange LockGuardTypeRange = + getLockGuardRange(LockGuard->getTypeSourceInfo()); + + if (LockGuardTypeRange.isInvalid()) + return; + + // Create Fix-its only if we can find the constructor call to properly handle + // 'std::lock_guard l(m, std::adopt_lock)' case. + const auto *CtorCall = dyn_cast(LockGuard->getInit()); + if (!CtorCall) + return; + + if (CtorCall->getNumArgs() == 1) { + Diag << FixItHint::CreateReplacement(LockGuardTypeRange, + "std::scoped_lock"); + return; + } + + if (CtorCall->getNumArgs() == 2) { + const Expr *const *CtorArgs = CtorCall->getArgs(); + + const Expr *MutexArg = CtorArgs[0]; + const Expr *AdoptLockArg = CtorArgs[1]; + + const StringRef MutexSourceText = Lexer::getSourceText( + CharSourceRange::getTokenRange(MutexArg->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + const StringRef AdoptLockSourceText = Lexer::getSourceText( + CharSourceRange::getTokenRange(AdoptLockArg->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + + Diag << FixItHint::CreateReplacement(LockGuardTypeRange, "std::scoped_lock") + << FixItHint::CreateReplacement( + SourceRange(MutexArg->getBeginLoc(), AdoptLockArg->getEndLoc()), + (llvm::Twine(AdoptLockSourceText) + ", " + MutexSourceText) + .str()); + return; + } + + llvm_unreachable("Invalid argument number of std::lock_guard constructor"); +} + +void UseScopedLockCheck::diagOnMultipleLocks( + const llvm::SmallVector> &LockGroups, + const ast_matchers::MatchFinder::MatchResult &Result) { + for (const llvm::SmallVector &Group : LockGroups) { + if (Group.size() == 1) { + if (WarnOnSingleLocks) + diagOnSingleLock(Group[0], Result); + } else { + diag(Group[0]->getBeginLoc(), + "use single 'std::scoped_lock' instead of multiple " + "'std::lock_guard'"); + + for (const VarDecl *Lock : llvm::drop_begin(Group)) + diag(Lock->getLocation(), "additional 'std::lock_guard' declared here", + DiagnosticIDs::Note); + } + } +} + +void UseScopedLockCheck::diagOnSourceInfo( + const TypeSourceInfo *LockGuardSourceInfo, + const ast_matchers::MatchFinder::MatchResult &Result) { + const TypeLoc TL = LockGuardSourceInfo->getTypeLoc(); + + if (const auto ElaboratedTL = TL.getAs()) { + auto Diag = diag(ElaboratedTL.getBeginLoc(), UseScopedLockMessage); + + const SourceRange LockGuardRange = + getLockGuardNameRange(LockGuardSourceInfo); + if (LockGuardRange.isInvalid()) + return; + + Diag << FixItHint::CreateReplacement(LockGuardRange, "scoped_lock"); + } +} + +void UseScopedLockCheck::diagOnUsingDecl( + const UsingDecl *UsingDecl, + const ast_matchers::MatchFinder::MatchResult &Result) { + diag(UsingDecl->getLocation(), UseScopedLockMessage) + << FixItHint::CreateReplacement(UsingDecl->getLocation(), "scoped_lock"); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h new file mode 100644 index 0000000000000..a5697805c15ca --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseScopedLockCheck.h @@ -0,0 +1,54 @@ +//===--- UseScopedLockCheck.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_MODERNIZE_USESCOPEDLOCKCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESCOPEDLOCKCHECK_H + +#include "../ClangTidyCheck.h" +#include "clang/AST/ASTTypeTraits.h" +#include "clang/AST/Stmt.h" +#include + +namespace clang::tidy::modernize { + +/// Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's +/// alternative ``std::scoped_lock``. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-scoped-lock.html +class UseScopedLockCheck : public ClangTidyCheck { +public: + UseScopedLockCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus17; + } + std::optional getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + void diagOnSingleLock(const VarDecl *LockGuard, + const ast_matchers::MatchFinder::MatchResult &Result); + void diagOnMultipleLocks( + const llvm::SmallVector> &LockGroups, + const ast_matchers::MatchFinder::MatchResult &Result); + void diagOnSourceInfo(const TypeSourceInfo *LockGuardSourceInfo, + const ast_matchers::MatchFinder::MatchResult &Result); + void diagOnUsingDecl(const UsingDecl *UsingDecl, + const ast_matchers::MatchFinder::MatchResult &Result); + + const bool WarnOnSingleLocks; + const bool WarnOnUsingAndTypedef; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESCOPEDLOCKCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 2c40a863c5b7d..4be1a8f831339 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -58,7 +58,6 @@ add_clang_library(clangTidyReadabilityModule STATIC UniqueptrDeleteReleaseCheck.cpp UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.cpp - UseNumericLimitsCheck.cpp UseStdMinMaxCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index dc47c2fb31937..d59b0312673b9 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -61,7 +61,6 @@ #include "UniqueptrDeleteReleaseCheck.h" #include "UppercaseLiteralSuffixCheck.h" #include "UseAnyOfAllOfCheck.h" -#include "UseNumericLimitsCheck.h" #include "UseStdMinMaxCheck.h" namespace clang::tidy { @@ -174,8 +173,6 @@ class ReadabilityModule : public ClangTidyModule { "readability-uppercase-literal-suffix"); CheckFactories.registerCheck( "readability-use-anyofallof"); - CheckFactories.registerCheck( - "readability-use-numeric-limits"); CheckFactories.registerCheck( "readability-use-std-min-max"); } diff --git a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp index 969bf2d1add6b..7f1882c775c59 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp @@ -72,11 +72,13 @@ static SourceLocation getInlineTokenLocation(SourceRange RangeLocation, } void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { + const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); Finder->addMatcher( functionDecl(isInlineSpecified(), - anyOf(isConstexpr(), isDeleted(), isDefaulted(), + anyOf(isConstexpr(), isDeleted(), + allOf(isDefaulted(), IsPartOfRecordDecl), isInternalLinkage(StrictMode), - allOf(isDefinition(), hasAncestor(recordDecl())))) + allOf(isDefinition(), IsPartOfRecordDecl))) .bind("fun_decl"), this); @@ -88,7 +90,6 @@ void RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) { this); if (getLangOpts().CPlusPlus17) { - const auto IsPartOfRecordDecl = hasAncestor(recordDecl()); Finder->addMatcher( varDecl( isInlineSpecified(), diff --git a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp deleted file mode 100644 index 334b69755db29..0000000000000 --- a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp +++ /dev/null @@ -1,160 +0,0 @@ -//===--- UseNumericLimitsCheck.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 "UseNumericLimitsCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Preprocessor.h" -#include -#include - -using namespace clang::ast_matchers; - -namespace clang::tidy::readability { - -UseNumericLimitsCheck::UseNumericLimitsCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - SignedConstants{ - {std::numeric_limits::min(), - "std::numeric_limits::min()"}, - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - {std::numeric_limits::min(), - "std::numeric_limits::min()"}, - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - {std::numeric_limits::min(), - "std::numeric_limits::min()"}, - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - {std::numeric_limits::min(), - "std::numeric_limits::min()"}, - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - }, - UnsignedConstants{ - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - {std::numeric_limits::max(), - "std::numeric_limits::max()"}, - }, - Inserter(Options.getLocalOrGlobal("IncludeStyle", - utils::IncludeSorter::IS_LLVM), - areDiagsSelfContained()) {} - -void UseNumericLimitsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "IncludeStyle", Inserter.getStyle()); -} - -void UseNumericLimitsCheck::registerMatchers(MatchFinder *Finder) { - auto PositiveIntegerMatcher = [](auto Value) { - return unaryOperator(hasOperatorName("+"), - hasUnaryOperand(integerLiteral(equals(Value)) - .bind("positive-integer-literal"))) - .bind("unary-op"); - }; - - auto NegativeIntegerMatcher = [](auto Value) { - return unaryOperator(hasOperatorName("-"), - hasUnaryOperand(integerLiteral(equals(-Value)) - .bind("negative-integer-literal"))) - .bind("unary-op"); - }; - - auto BareIntegerMatcher = [](auto Value) { - return integerLiteral(allOf(unless(hasParent(unaryOperator( - hasAnyOperatorName("-", "+")))), - equals(Value))) - .bind("bare-integer-literal"); - }; - - for (const auto &[Value, _] : SignedConstants) { - if (Value < 0) { - Finder->addMatcher(NegativeIntegerMatcher(Value), this); - } else { - Finder->addMatcher( - expr(anyOf(PositiveIntegerMatcher(Value), BareIntegerMatcher(Value))), - this); - } - } - - for (const auto &[Value, _] : UnsignedConstants) { - Finder->addMatcher( - expr(anyOf(PositiveIntegerMatcher(Value), BareIntegerMatcher(Value))), - this); - } -} - -void UseNumericLimitsCheck::registerPPCallbacks( - const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - Inserter.registerPreprocessor(PP); -} - -void UseNumericLimitsCheck::check(const MatchFinder::MatchResult &Result) { - const IntegerLiteral *MatchedDecl = nullptr; - - const IntegerLiteral *NegativeMatchedDecl = - Result.Nodes.getNodeAs("negative-integer-literal"); - const IntegerLiteral *PositiveMatchedDecl = - Result.Nodes.getNodeAs("positive-integer-literal"); - const IntegerLiteral *BareMatchedDecl = - Result.Nodes.getNodeAs("bare-integer-literal"); - - if (NegativeMatchedDecl != nullptr) - MatchedDecl = NegativeMatchedDecl; - else if (PositiveMatchedDecl != nullptr) - MatchedDecl = PositiveMatchedDecl; - else if (BareMatchedDecl != nullptr) - MatchedDecl = BareMatchedDecl; - - const llvm::APInt MatchedIntegerConstant = MatchedDecl->getValue(); - - auto Fixer = [&](auto SourceValue, auto Value, - const std::string &Replacement) { - static_assert(std::is_same_v, - "The types of SourceValue and Value must match"); - - SourceLocation Location = MatchedDecl->getExprLoc(); - SourceRange Range{MatchedDecl->getBeginLoc(), MatchedDecl->getEndLoc()}; - - // Only valid if unary operator is present - const UnaryOperator *UnaryOpExpr = - Result.Nodes.getNodeAs("unary-op"); - - if (MatchedDecl == NegativeMatchedDecl && -SourceValue == Value) { - Range = SourceRange(UnaryOpExpr->getBeginLoc(), UnaryOpExpr->getEndLoc()); - Location = UnaryOpExpr->getExprLoc(); - SourceValue = -SourceValue; - } else if (MatchedDecl == PositiveMatchedDecl && SourceValue == Value) { - Range = SourceRange(UnaryOpExpr->getBeginLoc(), UnaryOpExpr->getEndLoc()); - Location = UnaryOpExpr->getExprLoc(); - } else if (MatchedDecl != BareMatchedDecl || SourceValue != Value) { - return; - } - - diag(Location, - "the constant '%0' is being utilized; consider using '%1' instead") - << SourceValue << Replacement - << FixItHint::CreateReplacement(Range, Replacement) - << Inserter.createIncludeInsertion( - Result.SourceManager->getFileID(Location), ""); - }; - - for (const auto &[Value, Replacement] : SignedConstants) - Fixer(MatchedIntegerConstant.getSExtValue(), Value, Replacement); - - for (const auto &[Value, Replacement] : UnsignedConstants) - Fixer(MatchedIntegerConstant.getZExtValue(), Value, Replacement); -} - -} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h deleted file mode 100644 index 0e7e9abb8562c..0000000000000 --- a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h +++ /dev/null @@ -1,38 +0,0 @@ -//===--- UseNumericLimitsCheck.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_READABILITY_USENUMERICLIMITSCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USENUMERICLIMITSCHECK_H - -#include "../ClangTidyCheck.h" -#include "../utils/IncludeInserter.h" - -namespace clang::tidy::readability { - -/// Finds certain integer literals and suggests replacing them with equivalent -/// ``std::numeric_limits`` calls. -/// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/readability/use-numeric-limits.html -class UseNumericLimitsCheck : public ClangTidyCheck { -public: - UseNumericLimitsCheck(StringRef Name, ClangTidyContext *Context); - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, - Preprocessor *ModuleExpanderPP) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - -private: - const llvm::SmallVector> SignedConstants; - const llvm::SmallVector> UnsignedConstants; - utils::IncludeInserter Inserter; -}; - -} // namespace clang::tidy::readability - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USENUMERICLIMITSCHECK_H diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp index 3b991e5e9013f..e274236527817 100644 --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -440,7 +440,7 @@ QualType declaredType(const TypeDecl *D) { if (const auto *Args = CTSD->getTemplateArgsAsWritten()) return Context.getTemplateSpecializationType( TemplateName(CTSD->getSpecializedTemplate()), Args->arguments(), - /*CanonicalArgs=*/std::nullopt); + /*CanonicalArgs=*/{}); return Context.getTypeDeclType(D); } diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 856b1fd432bbc..d83242fedf341 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -33,7 +33,6 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" -#include "llvm/ADT/identity.h" #include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" @@ -339,53 +338,6 @@ QualType maybeDesugar(ASTContext &AST, QualType QT) { return QT; } -// Given a callee expression `Fn`, if the call is through a function pointer, -// try to find the declaration of the corresponding function pointer type, -// so that we can recover argument names from it. -// FIXME: This function is mostly duplicated in SemaCodeComplete.cpp; unify. -static FunctionProtoTypeLoc getPrototypeLoc(Expr *Fn) { - TypeLoc Target; - Expr *NakedFn = Fn->IgnoreParenCasts(); - if (const auto *T = NakedFn->getType().getTypePtr()->getAs()) { - Target = T->getDecl()->getTypeSourceInfo()->getTypeLoc(); - } else if (const auto *DR = dyn_cast(NakedFn)) { - const auto *D = DR->getDecl(); - if (const auto *const VD = dyn_cast(D)) { - Target = VD->getTypeSourceInfo()->getTypeLoc(); - } - } - - if (!Target) - return {}; - - // Unwrap types that may be wrapping the function type - while (true) { - if (auto P = Target.getAs()) { - Target = P.getPointeeLoc(); - continue; - } - if (auto A = Target.getAs()) { - Target = A.getModifiedLoc(); - continue; - } - if (auto P = Target.getAs()) { - Target = P.getInnerLoc(); - continue; - } - break; - } - - if (auto F = Target.getAs()) { - // In some edge cases the AST can contain a "trivial" FunctionProtoTypeLoc - // which has null parameters. Avoid these as they don't contain useful - // information. - if (llvm::all_of(F.getParams(), llvm::identity())) - return F; - } - - return {}; -} - ArrayRef maybeDropCxxExplicitObjectParameters(ArrayRef Params) { if (!Params.empty() && Params.front()->isExplicitObjectParameter()) @@ -514,7 +466,8 @@ class InlayHintVisitor : public RecursiveASTVisitor { Callee.Decl = FD; else if (const auto *FTD = dyn_cast(CalleeDecls[0])) Callee.Decl = FTD->getTemplatedDecl(); - else if (FunctionProtoTypeLoc Loc = getPrototypeLoc(E->getCallee())) + else if (FunctionProtoTypeLoc Loc = + Resolver->getFunctionProtoTypeLoc(E->getCallee())) Callee.Loc = Loc; else return true; diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp index d6556bba14725..c9704492bf1cd 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp @@ -341,7 +341,7 @@ const FunctionDecl *findTarget(const FunctionDecl *FD) { // Returns the beginning location for a FunctionDecl. Returns location of // template keyword for templated functions. -const SourceLocation getBeginLoc(const FunctionDecl *FD) { +SourceLocation getBeginLoc(const FunctionDecl *FD) { // Include template parameter list. if (auto *FTD = FD->getDescribedFunctionTemplate()) return FTD->getBeginLoc(); diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp index 90dac3b76c648..c74250ccbe9ea 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp @@ -359,9 +359,9 @@ struct ParsedBinaryOperator { // + c <- End // / \ // a b <- Start -const SourceRange getBinaryOperatorRange(const SelectionTree::Node &N, - const SourceManager &SM, - const LangOptions &LangOpts) { +SourceRange getBinaryOperatorRange(const SelectionTree::Node &N, + const SourceManager &SM, + const LangOptions &LangOpts) { // If N is not a suitable binary operator, bail out. ParsedBinaryOperator Op; if (!Op.parse(N.ignoreImplicit()) || !Op.associative() || diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index b11d194da04db..f287439f10cab 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -668,7 +668,6 @@ class FlagsConfigProvider : public config::Provider { std::optional IndexSpec; std::optional BGPolicy; std::optional ArgumentLists; - std::optional HeaderInsertionPolicy; // If --compile-commands-dir arg was invoked, check value and override // default path. @@ -713,11 +712,6 @@ class FlagsConfigProvider : public config::Provider { BGPolicy = Config::BackgroundPolicy::Skip; } - // If CLI has set never, use that regardless of what the config files have - if (HeaderInsertion == Config::HeaderInsertionPolicy::NeverInsert) { - HeaderInsertionPolicy = Config::HeaderInsertionPolicy::NeverInsert; - } - if (std::optional Enable = shouldEnableFunctionArgSnippets()) { ArgumentLists = *Enable ? Config::ArgumentListsPolicy::FullPlaceholders : Config::ArgumentListsPolicy::Delimiters; @@ -732,8 +726,8 @@ class FlagsConfigProvider : public config::Provider { C.Index.Background = *BGPolicy; if (ArgumentLists) C.Completion.ArgumentLists = *ArgumentLists; - if (HeaderInsertionPolicy) - C.Completion.HeaderInsertion = *HeaderInsertionPolicy; + if (HeaderInsertion.getNumOccurrences()) + C.Completion.HeaderInsertion = HeaderInsertion; if (AllScopesCompletion.getNumOccurrences()) C.Completion.AllScopes = AllScopesCompletion; diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 311f0d98904ad..b7c64c7a06745 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -4363,6 +4363,36 @@ TEST(CompletionTest, PreambleFromDifferentTarget) { EXPECT_THAT(Result.Completions, Not(testing::IsEmpty())); EXPECT_THAT(Signatures.signatures, Not(testing::IsEmpty())); } + +TEST(CompletionTest, SkipExplicitObjectParameter) { + Annotations Code(R"cpp( + struct A { + void foo(this auto&& self, int arg); + }; + + int main() { + A a {}; + a.^ + } + )cpp"); + + auto TU = TestTU::withCode(Code.code()); + TU.ExtraArgs = {"-std=c++23"}; + + auto Preamble = TU.preamble(); + ASSERT_TRUE(Preamble); + + CodeCompleteOptions Opts{}; + + MockFS FS; + auto Inputs = TU.inputs(FS); + auto Result = codeComplete(testPath(TU.Filename), Code.point(), + Preamble.get(), Inputs, Opts); + + EXPECT_THAT(Result.Completions, + ElementsAre(AllOf(named("foo"), signature("(int arg)"), + snippetSuffix("(${1:int arg})")))); +} } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 9dede347b8c97..f8f183e9de1cc 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -142,6 +142,12 @@ New checks Finds unscoped (non-class) ``enum`` declarations and suggests using ``enum class`` instead. +- New :doc:`modernize-use-scoped-lock + ` check. + + Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's + alternative ``std::scoped_lock``. + - New :doc:`portability-avoid-pragma-once ` check. @@ -154,12 +160,6 @@ New checks Finds potentially erroneous calls to ``reset`` method on smart pointers when the pointee type also has a ``reset`` method. -- New :doc:`readability-use-numeric-limits - ` check. - - Finds certain integer literals and suggests replacing them with equivalent - ``std::numeric_limits`` calls. - New check aliases ^^^^^^^^^^^^^^^^^ @@ -179,6 +179,11 @@ Changes in existing checks ` check to detect conversion in argument of ``std::make_optional``. +- Improved :doc: `bugprone-sizeof-expression + ` check by adding + `WarnOnSizeOfInLoopTermination` option to detect misuses of ``sizeof`` + expression in loop conditions. + - Improved :doc:`bugprone-string-constructor ` check to find suspicious calls of ``std::string`` constructor with char pointer, start position and @@ -203,11 +208,42 @@ Changes in existing checks ` check by fixing a false positive where ``strerror`` was flagged as MT-unsafe. +- Improved :doc:`cppcoreguidelines-avoid-goto + ` check by adding the option + `IgnoreMacros` to ignore ``goto`` labels defined in macros. + +- Improved :doc:`cppcoreguidelines-missing-std-forward + ` check by adding a + flag to specify the function used for forwarding instead of ``std::forward``. + +- Improved :doc:`cppcoreguidelines-pro-bounds-pointer-arithmetic + ` check by + fixing false positives when calling indexing operators that do not perform + pointer arithmetic in template, for example ``std::map::operator[]`` and + when pointer arithmetic was used through type aliases. + +- Improved :doc:`cppcoreguidelines-rvalue-reference-param-not-moved + ` check + by adding a flag to specify the function used for moving instead of + ``std::move``. + +- Improved :doc:`cppcoreguidelines-special-member-functions + ` check by + adding the option `IgnoreMacros` to ignore classes defined in macros. + - Improved :doc:`google-readability-namespace-comments ` check by adding the option `AllowOmittingNamespaceComments` to accept if a namespace comment is omitted entirely. +- Improved :doc:`hicpp-avoid-goto + ` check by adding the option + `IgnoreMacros` to ignore ``goto`` labels defined in macros. + +- Improved :doc:`hicpp-special-member-functions + ` check by adding the + option `IgnoreMacros` to ignore classes defined in macros. + - Improved :doc:`llvm-namespace-comment ` check by adding the option `AllowOmittingNamespaceComments` to accept if a namespace comment is omitted @@ -298,6 +334,10 @@ Changes in existing checks ` check by adding the option `AllowedTypes`, that excludes specified types from adding qualifiers. +- Improved :doc:`readability-redundant-inline-specifier + ` check by fixing + false positives on out-of-line explicitly defaulted functions. + - Improved :doc:`readability-redundant-smartptr-get ` check by fixing some false positives involving smart pointers to arrays. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst index 29edb26ed7aa2..04824cc1fe0e4 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/sizeof-expression.rst @@ -316,3 +316,12 @@ Options When `true`, the check will warn on pointer arithmetic where the element count is obtained from a division with ``sizeof(...)``, e.g., ``Ptr + Bytes / sizeof(*T)``. Default is `true`. + +.. option:: WarnOnSizeOfInLoopTermination + + When `true`, the check will warn about incorrect use of sizeof expression + in loop termination condition. The warning triggers if the ``sizeof`` + expression appears to be incorrectly used to determine the number of + array/buffer elements. + e.g, ``long arr[10]; for(int i = 0; i < sizeof(arr); i++) { ... }``. Default + is `true`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst index 71b579a4ae99e..1f9dc0a1edb3a 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-goto.rst @@ -50,3 +50,12 @@ Modern C++ needs ``goto`` only to jump out of nested loops. some_operation(); All other uses of ``goto`` are diagnosed in `C++`. + + +Options +------- + +.. option:: IgnoreMacros + + If set to `true`, the check will not warn if a ``goto`` statement is + expanded from a macro. Default is `false`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst index 0c311b59a5d5a..62e38fcd3b9dc 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/missing-std-forward.rst @@ -35,6 +35,13 @@ Example: f(1, 2); // Incorrect - may not invoke the desired qualified function operator } +Options +------- + +.. option:: ForwardFunction + + Specify the function used for forwarding. Default is `::std::forward`. + This check implements `F.19 `_ from the C++ Core Guidelines. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst index ffa3a9d61e48e..2fea9f16b3bb0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst @@ -79,6 +79,10 @@ Options T other = std::forward(t); } +.. option:: MoveFunction + + Specify the function used for moving. Default is `::std::move`. + This check implements `F.18 `_ from the C++ Core Guidelines. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst index 20f898fdab930..982d16fc8d23d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/special-member-functions.rst @@ -85,3 +85,8 @@ Options struct A : boost::noncopyable { ~A() { std::cout << "dtor\n"; } }; + +.. option:: IgnoreMacros + + If set to `true`, the check will not give warnings for classes defined + inside macros. Default is `true`. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 57ae7d330a3ce..5098582d0c42b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -312,6 +312,7 @@ Clang-Tidy Checks :doc:`modernize-use-nullptr `, "Yes" :doc:`modernize-use-override `, "Yes" :doc:`modernize-use-ranges `, "Yes" + :doc:`modernize-use-scoped-lock `, "Yes" :doc:`modernize-use-starts-ends-with `, "Yes" :doc:`modernize-use-std-format `, "Yes" :doc:`modernize-use-std-numbers `, "Yes" @@ -409,7 +410,6 @@ Clang-Tidy Checks :doc:`readability-uniqueptr-delete-release `, "Yes" :doc:`readability-uppercase-literal-suffix `, "Yes" :doc:`readability-use-anyofallof `, - :doc:`readability-use-numeric-limits `, "Yes" :doc:`readability-use-std-min-max `, "Yes" :doc:`zircon-temporary-objects `, diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst new file mode 100644 index 0000000000000..d184f1aefd806 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-scoped-lock.rst @@ -0,0 +1,101 @@ +.. title:: clang-tidy - modernize-use-scoped-lock + +modernize-use-scoped-lock +========================= + +Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's +alternative ``std::scoped_lock``. + +Fix-its are provided for single declarations of ``std::lock_guard`` and warning +is emitted for multiple declarations of ``std::lock_guard`` that can be +replaced with a single declaration of ``std::scoped_lock``. + +Examples +-------- + +Single ``std::lock_guard`` declaration: + +.. code-block:: c++ + + std::mutex M; + std::lock_guard L(M); + + +Transforms to: + +.. code-block:: c++ + + std::mutex M; + std::scoped_lock L(M); + +Single ``std::lock_guard`` declaration with ``std::adopt_lock``: + +.. code-block:: c++ + + std::mutex M; + std::lock(M); + std::lock_guard L(M, std::adopt_lock); + + +Transforms to: + +.. code-block:: c++ + + std::mutex M; + std::lock(M); + std::scoped_lock L(std::adopt_lock, M); + +Multiple ``std::lock_guard`` declarations only emit warnings: + +.. code-block:: c++ + + std::mutex M1, M2; + std::lock(M1, M2); + std::lock_guard Lock1(M, std::adopt_lock); // warning: use single 'std::scoped_lock' instead of multiple 'std::lock_guard' + std::lock_guard Lock2(M, std::adopt_lock); // note: additional 'std::lock_guard' declared here + + +Limitations +----------- + +The check will not emit warnings if ``std::lock_guard`` is used implicitly via +``template`` parameter: + +.. code-block:: c++ + + template