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/workflows/containers/github-action-ci-windows/Dockerfile b/.github/workflows/containers/github-action-ci-windows/Dockerfile index eafe45fac8ea4..c49b6b25ffb68 100644 --- a/.github/workflows/containers/github-action-ci-windows/Dockerfile +++ b/.github/workflows/containers/github-action-ci-windows/Dockerfile @@ -85,7 +85,7 @@ RUN powershell -Command \ RUN git config --system core.longpaths true & \ git config --global core.autocrlf false -ARG RUNNER_VERSION=2.325.0 +ARG RUNNER_VERSION=2.326.0 ENV RUNNER_VERSION=$RUNNER_VERSION RUN powershell -Command \ diff --git a/.github/workflows/containers/github-action-ci/Dockerfile b/.github/workflows/containers/github-action-ci/Dockerfile index f2716f0a9c6a0..197bf083a20c0 100644 --- a/.github/workflows/containers/github-action-ci/Dockerfile +++ b/.github/workflows/containers/github-action-ci/Dockerfile @@ -86,7 +86,7 @@ WORKDIR /home/gha FROM ci-container as ci-container-agent -ENV GITHUB_RUNNER_VERSION=2.325.0 +ENV GITHUB_RUNNER_VERSION=2.326.0 RUN mkdir actions-runner && \ cd actions-runner && \ diff --git a/.github/workflows/hlsl-test-all.yaml b/.github/workflows/hlsl-test-all.yaml index 93a1c6d2662d4..b6530fe11b840 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/libc-fullbuild-tests.yml b/.github/workflows/libc-fullbuild-tests.yml index 24d75f58d45e0..9b2d8dd579702 100644 --- a/.github/workflows/libc-fullbuild-tests.yml +++ b/.github/workflows/libc-fullbuild-tests.yml @@ -15,9 +15,25 @@ jobs: strategy: fail-fast: false matrix: - build_type: [Debug, Release, MinSizeRel] + # Build basic linux configuration with Debug/Release/MinSizeRel and all + # other configurations in Debug only. include: - os: ubuntu-24.04 + build_type: Debug + ccache-variant: sccache + c_compiler: clang-21 + cpp_compiler: clang++-21 + target: x86_64-unknown-linux-llvm + include_scudo: ON + - os: ubuntu-24.04 + build_type: Release + ccache-variant: sccache + c_compiler: clang-21 + cpp_compiler: clang++-21 + target: x86_64-unknown-linux-llvm + include_scudo: ON + - os: ubuntu-24.04 + build_type: MinSizeRel ccache-variant: sccache c_compiler: clang-21 cpp_compiler: clang++-21 @@ -25,12 +41,14 @@ jobs: include_scudo: ON # TODO: remove ccache logic when https://github.com/hendrikmuhs/ccache-action/issues/279 is resolved. - os: ubuntu-24.04-arm + build_type: Debug ccache-variant: ccache c_compiler: clang-21 cpp_compiler: clang++-21 target: aarch64-unknown-linux-llvm include_scudo: ON - os: ubuntu-24.04 + build_type: Debug ccache-variant: ccache c_compiler: clang-21 cpp_compiler: clang++-21 @@ -97,7 +115,7 @@ jobs: -DCMAKE_C_COMPILER_LAUNCHER=${{ matrix.ccache-variant }} \ -DCMAKE_CXX_COMPILER_LAUNCHER=${{ matrix.ccache-variant }} \ -DCMAKE_INSTALL_PREFIX=${{ steps.strings.outputs.build-install-dir }} \ - -DLLVM_RUNTIMES_TARGET=${{ matrix.target }} \ + -DLLVM_RUNTIME_TARGETS=${{ matrix.target }} \ -DLLVM_ENABLE_RUNTIMES="$RUNTIMES" \ -DLLVM_LIBC_FULL_BUILD=ON \ -G Ninja \ diff --git a/.github/workflows/libc-overlay-tests.yml b/.github/workflows/libc-overlay-tests.yml index da82d8d9fe8ab..f001daae030a3 100644 --- a/.github/workflows/libc-overlay-tests.yml +++ b/.github/workflows/libc-overlay-tests.yml @@ -16,7 +16,7 @@ jobs: # Set fail-fast to false to ensure that feedback is delivered for all matrix combinations. fail-fast: false matrix: - build_type: [Debug, Release, MinSizeRel] + os: [ubuntu-24.04, ubuntu-24.04-arm, windows-2022, windows-2025, macos-14] include: # TODO: add linux gcc when it is fixed - os: ubuntu-24.04 @@ -96,7 +96,7 @@ jobs: cmake -B ${{ steps.strings.outputs.build-output-dir }} -DCMAKE_CXX_COMPILER=${{ matrix.compiler.cpp_compiler }} -DCMAKE_C_COMPILER=${{ matrix.compiler.c_compiler }} - -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER_LAUNCHER=${{ matrix.ccache-variant }} -DCMAKE_CXX_COMPILER_LAUNCHER=${{ matrix.ccache-variant }} -DCMAKE_POLICY_DEFAULT_CMP0141=NEW @@ -110,7 +110,6 @@ jobs: cmake --build ${{ steps.strings.outputs.build-output-dir }} --parallel - --config MinSizeRel --target libc - name: Test 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/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 98e4bba872846..c7400b0d45ca8 100644 --- a/bolt/include/bolt/Profile/DataAggregator.h +++ b/bolt/include/bolt/Profile/DataAggregator.h @@ -236,8 +236,7 @@ class DataAggregator : public DataReader { /// Launch a perf subprocess with given args and save output for later /// parsing. - void launchPerfProcess(StringRef Name, PerfProcessInfo &PPI, - const char *ArgsString, bool Wait); + void launchPerfProcess(StringRef Name, PerfProcessInfo &PPI, StringRef Args); /// Delete all temporary files created to hold the output generated by spawned /// subprocesses during the aggregation job 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/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/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index 88229bb31a2ad..c60591c02b6c0 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/bolt/lib/Profile/DataAggregator.cpp @@ -197,34 +197,27 @@ void DataAggregator::start() { if (opts::BasicAggregation) { launchPerfProcess("events without LBR", MainEventsPPI, - "script -F pid,event,ip", - /*Wait = */ false); + "script -F pid,event,ip"); } else if (!opts::ITraceAggregation.empty()) { // Disable parsing memory profile from trace data, unless requested by user. if (!opts::ParseMemProfile.getNumOccurrences()) opts::ParseMemProfile = false; - - std::string ItracePerfScriptArgs = llvm::formatv( - "script -F pid,brstack --itrace={0}", opts::ITraceAggregation); launchPerfProcess("branch events with itrace", MainEventsPPI, - ItracePerfScriptArgs.c_str(), - /*Wait = */ false); + "script -F pid,brstack --itrace=" + + opts::ITraceAggregation); } else { - launchPerfProcess("branch events", MainEventsPPI, "script -F pid,brstack", - /*Wait = */ false); + launchPerfProcess("branch events", MainEventsPPI, "script -F pid,brstack"); } if (opts::ParseMemProfile) - launchPerfProcess("mem events", MemEventsPPI, "script -F pid,event,addr,ip", - /*Wait = */ false); + launchPerfProcess("mem events", MemEventsPPI, + "script -F pid,event,addr,ip"); launchPerfProcess("process events", MMapEventsPPI, - "script --show-mmap-events --no-itrace", - /*Wait = */ false); + "script --show-mmap-events --no-itrace"); launchPerfProcess("task events", TaskEventsPPI, - "script --show-task-events --no-itrace", - /*Wait = */ false); + "script --show-task-events --no-itrace"); } void DataAggregator::abort() { @@ -246,13 +239,13 @@ void DataAggregator::abort() { } void DataAggregator::launchPerfProcess(StringRef Name, PerfProcessInfo &PPI, - const char *ArgsString, bool Wait) { + StringRef Args) { SmallVector Argv; outs() << "PERF2BOLT: spawning perf job to read " << Name << '\n'; Argv.push_back(PerfPath.data()); - StringRef(ArgsString).split(Argv, ' '); + Args.split(Argv, ' '); Argv.push_back("-f"); Argv.push_back("-i"); Argv.push_back(Filename.c_str()); @@ -286,64 +279,45 @@ void DataAggregator::launchPerfProcess(StringRef Name, PerfProcessInfo &PPI, << "\n"; }); - if (Wait) - PPI.PI.ReturnCode = sys::ExecuteAndWait(PerfPath.data(), Argv, - /*envp*/ std::nullopt, Redirects); - else - PPI.PI = sys::ExecuteNoWait(PerfPath.data(), Argv, /*envp*/ std::nullopt, - Redirects); + PPI.PI = sys::ExecuteNoWait(PerfPath.data(), Argv, /*envp*/ std::nullopt, + Redirects); } void DataAggregator::processFileBuildID(StringRef FileBuildID) { + auto WarningCallback = [](int ReturnCode, StringRef ErrBuf) { + errs() << "PERF-ERROR: return code " << ReturnCode << "\n" << ErrBuf; + }; + PerfProcessInfo BuildIDProcessInfo; - launchPerfProcess("buildid list", - BuildIDProcessInfo, - "buildid-list", - /*Wait = */true); - - if (BuildIDProcessInfo.PI.ReturnCode != 0) { - ErrorOr> MB = - MemoryBuffer::getFileOrSTDIN(BuildIDProcessInfo.StderrPath.data()); - StringRef ErrBuf = (*MB)->getBuffer(); - - errs() << "PERF-ERROR: return code " << BuildIDProcessInfo.PI.ReturnCode - << '\n'; - errs() << ErrBuf; + launchPerfProcess("buildid list", BuildIDProcessInfo, "buildid-list"); + if (prepareToParse("buildid", BuildIDProcessInfo, WarningCallback)) return; - } - ErrorOr> MB = - MemoryBuffer::getFileOrSTDIN(BuildIDProcessInfo.StdoutPath.data()); - if (std::error_code EC = MB.getError()) { - errs() << "Cannot open " << BuildIDProcessInfo.StdoutPath.data() << ": " - << EC.message() << "\n"; + std::optional FileName = getFileNameForBuildID(FileBuildID); + if (FileName && *FileName == sys::path::filename(BC->getFilename())) { + outs() << "PERF2BOLT: matched build-id and file name\n"; return; } - FileBuf = std::move(*MB); - ParsingBuf = FileBuf->getBuffer(); - - std::optional FileName = getFileNameForBuildID(FileBuildID); - if (!FileName) { - if (hasAllBuildIDs()) { - errs() << "PERF2BOLT-ERROR: failed to match build-id from perf output. " - "This indicates the input binary supplied for data aggregation " - "is not the same recorded by perf when collecting profiling " - "data, or there were no samples recorded for the binary. " - "Use -ignore-build-id option to override.\n"; - if (!opts::IgnoreBuildID) - abort(); - } else { - errs() << "PERF2BOLT-WARNING: build-id will not be checked because perf " - "data was recorded without it\n"; - return; - } - } else if (*FileName != llvm::sys::path::filename(BC->getFilename())) { + if (FileName) { errs() << "PERF2BOLT-WARNING: build-id matched a different file name\n"; BuildIDBinaryName = std::string(*FileName); - } else { - outs() << "PERF2BOLT: matched build-id and file name\n"; + return; } + + if (!hasAllBuildIDs()) { + errs() << "PERF2BOLT-WARNING: build-id will not be checked because perf " + "data was recorded without it\n"; + return; + } + + errs() << "PERF2BOLT-ERROR: failed to match build-id from perf output. " + "This indicates the input binary supplied for data aggregation " + "is not the same recorded by perf when collecting profiling " + "data, or there were no samples recorded for the binary. " + "Use -ignore-build-id option to override.\n"; + if (!opts::IgnoreBuildID) + abort(); } bool DataAggregator::checkPerfDataMagic(StringRef FileName) { @@ -432,7 +406,8 @@ int DataAggregator::prepareToParse(StringRef Name, PerfProcessInfo &Process, std::string Error; outs() << "PERF2BOLT: waiting for perf " << Name << " collection to finish...\n"; - sys::ProcessInfo PI = sys::Wait(Process.PI, std::nullopt, &Error); + std::optional PS; + sys::ProcessInfo PI = sys::Wait(Process.PI, std::nullopt, &Error, &PS); if (!Error.empty()) { errs() << "PERF-ERROR: " << PerfPath << ": " << Error << "\n"; @@ -440,6 +415,15 @@ int DataAggregator::prepareToParse(StringRef Name, PerfProcessInfo &Process, exit(1); } + LLVM_DEBUG({ + const float UserSec = 1.f * PS->UserTime.count() / 1e6; + const float TotalSec = 1.f * PS->TotalTime.count() / 1e6; + const float PeakGiB = 1.f * PS->PeakMemory / (1 << 20); + dbgs() << formatv("Finished in {0:f2}s user time, {1:f2}s total time, " + "{2:f2} GiB peak RSS\n", + UserSec, TotalSec, PeakGiB); + }); + if (PI.ReturnCode != 0) { ErrorOr> ErrorMB = MemoryBuffer::getFileOrSTDIN(Process.StderrPath.data()); 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 934768c244b31..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; @@ -3856,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; @@ -4155,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(); @@ -4181,16 +4281,7 @@ 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 (NewTextSegmentAddress) - NewTextSegmentSize = NextAvailableAddress - NewTextSegmentAddress; - } else { - NewWritableSegmentSize = NextAvailableAddress - NewWritableSegmentAddress; - } - - if (!NewTextSegmentSize && !NewWritableSegmentSize) { + if (BC->NewSegments.empty()) { BC->outs() << "BOLT-INFO: not adding new segments\n"; return; } @@ -4198,90 +4289,28 @@ void RewriteInstance::patchELFPHDRTable() { const uint64_t SavedPos = OS.tell(); OS.seek(PHDRTableOffset); - auto createNewPhdrs = [&]() { - SmallVector NewPhdrs; - ELF64LEPhdrTy NewPhdr; - NewPhdr.p_type = ELF::PT_LOAD; - 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 (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)); } }; @@ -4317,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; @@ -5946,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 612c1304efd60..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" @@ -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); } @@ -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..a60c1a6bf156e 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; } @@ -2445,9 +2441,10 @@ class X86MCPlusBuilder : public MCPlusBuilder { assert(FKI.TargetOffset == 0 && "0-bit relocation offset expected"); const uint64_t RelOffset = Fixup.getOffset(); + auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup); uint32_t RelType; - if (FKI.Flags & MCFixupKindInfo::FKF_IsPCRel) { + if (Fixup.isPCRel()) { switch (FKI.TargetSize) { default: return std::nullopt; @@ -2456,6 +2453,9 @@ class X86MCPlusBuilder : public MCPlusBuilder { case 32: RelType = ELF::R_X86_64_PC32; break; case 64: RelType = ELF::R_X86_64_PC64; break; } + // Adjust PC-relative fixup offsets, which are calculated from the start + // of the next instruction. + RelAddend -= FKI.TargetSize / 8; } else { switch (FKI.TargetSize) { default: @@ -2467,8 +2467,6 @@ class X86MCPlusBuilder : public MCPlusBuilder { } } - auto [RelSymbol, RelAddend] = extractFixupExpr(Fixup); - return Relocation({RelOffset, RelSymbol, RelType, RelAddend, 0}); } @@ -2738,24 +2736,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 +2776,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 +2785,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 +2795,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 +2804,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 +2843,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 +2891,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 +2902,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 +3329,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 +3406,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 +3419,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 +3439,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 +3542,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 +3568,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 +3582,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/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/program-header.test b/bolt/test/program-header.test index 4552303ea5af2..f5490394eb6d9 100644 --- a/bolt/test/program-header.test +++ b/bolt/test/program-header.test @@ -2,13 +2,13 @@ REQUIRES: system-linux -RUN: %clang %cflags %p/Inputs/hello.c -o %t -no-pie -Wl,-q +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: rewriting .eh_frame_hdr in-place 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/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/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp index 74da80c382120..f756ae6d897c8 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.cpp +++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp @@ -94,6 +94,7 @@ static llvm::Error decodeRecord(const Record &R, InfoType &Field, case InfoType::IT_typedef: case InfoType::IT_concept: case InfoType::IT_variable: + case InfoType::IT_friend: Field = IT; return llvm::Error::success(); } @@ -111,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(); @@ -283,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, @@ -293,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"); @@ -308,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"); @@ -434,6 +452,15 @@ static llvm::Error parseRecord(const Record &R, unsigned ID, } } +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"); @@ -509,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(); @@ -530,6 +569,17 @@ static llvm::Error addReference(T I, Reference &&R, FieldId F) { "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: @@ -651,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"; @@ -684,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) { @@ -725,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 @@ -905,6 +971,10 @@ llvm::Error ClangDocBitcodeReader::readSubBlock(unsigned ID, T I) { 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(), "invalid subblock type"); @@ -1016,6 +1086,8 @@ ClangDocBitcodeReader::readBlockToInfo(unsigned 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"); @@ -1056,6 +1128,7 @@ ClangDocBitcodeReader::readBitcode() { 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/BitcodeWriter.cpp b/clang-tools-extra/clang-doc/BitcodeWriter.cpp index c3351d1decbf5..3cc0d4ad332f0 100644 --- a/clang-tools-extra/clang-doc/BitcodeWriter.cpp +++ b/clang-tools-extra/clang-doc/BitcodeWriter.cpp @@ -131,7 +131,8 @@ static const llvm::IndexedMap {BI_TEMPLATE_PARAM_BLOCK_ID, "TemplateParamBlock"}, {BI_CONSTRAINT_BLOCK_ID, "ConstraintBlock"}, {BI_CONCEPT_BLOCK_ID, "ConceptBlock"}, - {BI_VAR_BLOCK_ID, "VarBlock"}}; + {BI_VAR_BLOCK_ID, "VarBlock"}, + {BI_FRIEND_BLOCK_ID, "FriendBlock"}}; assert(Inits.size() == BlockIdCount); for (const auto &Init : Inits) BlockIdNameMap[Init.first] = Init.second; @@ -161,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}}, @@ -218,7 +225,8 @@ static const llvm::IndexedMap {VAR_USR, {"USR", &genSymbolIdAbbrev}}, {VAR_NAME, {"Name", &genStringAbbrev}}, {VAR_DEFLOCATION, {"DefLocation", &genLocationAbbrev}}, - {VAR_IS_STATIC, {"IsStatic", &genBoolAbbrev}}}; + {VAR_IS_STATIC, {"IsStatic", &genBoolAbbrev}}, + {FRIEND_IS_CLASS, {"IsClass", &genBoolAbbrev}}}; assert(Inits.size() == RecordIdCount); for (const auto &Init : Inits) { @@ -239,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}}, @@ -284,7 +295,8 @@ static const std::vector>> CONCEPT_CONSTRAINT_EXPRESSION}}, // Constraint Block {BI_CONSTRAINT_BLOCK_ID, {CONSTRAINT_EXPRESSION}}, - {BI_VAR_BLOCK_ID, {VAR_NAME, VAR_USR, VAR_DEFLOCATION, VAR_IS_STATIC}}}; + {BI_VAR_BLOCK_ID, {VAR_NAME, VAR_USR, VAR_DEFLOCATION, VAR_IS_STATIC}}, + {BI_FRIEND_BLOCK_ID, {FRIEND_IS_CLASS}}}; // AbbreviationMap @@ -467,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) { @@ -491,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) { @@ -499,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); } @@ -612,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) { @@ -728,6 +762,9 @@ bool ClangDocBitcodeWriter::dispatchInfoForWrite(Info *I) { 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 a70e50b53a61a..d09ec4ca34006 100644 --- a/clang-tools-extra/clang-doc/BitcodeWriter.h +++ b/clang-tools-extra/clang-doc/BitcodeWriter.h @@ -70,6 +70,7 @@ enum BlockId { BI_TYPEDEF_BLOCK_ID, BI_CONCEPT_BLOCK_ID, BI_VAR_BLOCK_ID, + BI_FRIEND_BLOCK_ID, BI_LAST, BI_FIRST = BI_VERSION_BLOCK_ID }; @@ -96,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, @@ -147,6 +154,7 @@ enum RecordId { VAR_NAME, VAR_DEFLOCATION, VAR_IS_STATIC, + FRIEND_IS_CLASS, RI_LAST, RI_FIRST = VERSION }; @@ -163,7 +171,8 @@ enum class FieldId { F_type, F_child_namespace, F_child_record, - F_concept + F_concept, + F_friend }; class ClangDocBitcodeWriter { @@ -195,6 +204,7 @@ 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: diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp index c4303d287da9e..8294ff9118558 100644 --- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp @@ -987,6 +987,7 @@ llvm::Error HTMLGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, break; case InfoType::IT_concept: case InfoType::IT_variable: + case InfoType::IT_friend: break; case InfoType::IT_default: return llvm::createStringError(llvm::inconvertibleErrorCode(), @@ -1018,6 +1019,8 @@ static std::string getRefType(InfoType IT) { 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 c611c946b3937..7aeaa1b7cf67d 100644 --- a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp @@ -588,6 +588,7 @@ Error MustacheHTMLGenerator::generateDocForInfo(Info *I, raw_ostream &OS, 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 1f6167f7f9b8d..0e1a0cc347e45 100644 --- a/clang-tools-extra/clang-doc/JSONGenerator.cpp +++ b/clang-tools-extra/clang-doc/JSONGenerator.cpp @@ -39,8 +39,7 @@ static void serializeArray(const Container &Records, Object &Obj, static auto SerializeInfoLambda = [](const auto &Info, Object &Object) { serializeInfo(Info, Object); }; -static auto SerializeReferenceLambda = [](const Reference &Ref, - Object &Object) { +static auto SerializeReferenceLambda = [](const auto &Ref, Object &Object) { serializeReference(Ref, Object); }; @@ -365,6 +364,22 @@ static void serializeInfo(const BaseRecordInfo &I, Object &Obj, 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, const std::optional &RepositoryUrl) { serializeCommonAttributes(I, Obj, RepositoryUrl); @@ -436,6 +451,9 @@ static void serializeInfo(const RecordInfo &I, json::Object &Obj, if (I.Template) serializeInfo(I.Template.value(), Obj); + if (!I.Friends.empty()) + serializeArray(I.Friends, Obj, "Friends", SerializeInfoLambda); + serializeCommonChildren(I.Children, Obj, RepositoryUrl); } @@ -525,6 +543,7 @@ Error JSONGenerator::generateDocForInfo(Info *I, raw_ostream &OS, 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 608a7f6d4a9d3..6f16f5bd2f528 100644 --- a/clang-tools-extra/clang-doc/MDGenerator.cpp +++ b/clang-tools-extra/clang-doc/MDGenerator.cpp @@ -378,6 +378,9 @@ static llvm::Error genIndex(ClangDocContext &CDCtx) { case InfoType::IT_variable: Type = "Variable"; break; + case InfoType::IT_friend: + Type = "Friend"; + break; case InfoType::IT_default: Type = "Other"; } @@ -472,6 +475,7 @@ llvm::Error MDGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &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/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp index 5b94d37d868b4..422a76d99e5b3 100644 --- a/clang-tools-extra/clang-doc/Representation.cpp +++ b/clang-tools-extra/clang-doc/Representation.cpp @@ -147,6 +147,8 @@ mergeInfos(std::vector> &Values) { 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"); @@ -247,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) @@ -313,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)); @@ -422,6 +435,9 @@ llvm::SmallString<16> Info::extractName() const { 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))); } diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h index 59874f0cfcedf..fe5cc48069d58 100644 --- a/clang-tools-extra/clang-doc/Representation.h +++ b/clang-tools-extra/clang-doc/Representation.h @@ -46,7 +46,8 @@ enum class InfoType { IT_enum, IT_typedef, IT_concept, - IT_variable + IT_variable, + IT_friend }; enum class CommentKind { @@ -379,6 +380,22 @@ 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) {} @@ -454,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 7a9cb8a1eddb9..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" @@ -403,6 +405,7 @@ std::string serialize(std::unique_ptr &I) { 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 ""; @@ -556,6 +559,7 @@ static std::unique_ptr makeAndInsertIntoParent(ChildType Child) { 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"); @@ -947,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) { @@ -970,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); diff --git a/clang-tools-extra/clang-doc/YAMLGenerator.cpp b/clang-tools-extra/clang-doc/YAMLGenerator.cpp index 3ca4d4947fa97..eeccdd804b669 100644 --- a/clang-tools-extra/clang-doc/YAMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/YAMLGenerator.cpp @@ -410,6 +410,7 @@ llvm::Error YAMLGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, 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/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index f4ab93b51f4a7..808515c463b91 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -55,7 +55,8 @@ namespace clang::tidy { namespace { #if CLANG_TIDY_ENABLE_STATIC_ANALYZER -static const char *AnalyzerCheckNamePrefix = "clang-analyzer-"; +static constexpr llvm::StringLiteral AnalyzerCheckNamePrefix = + "clang-analyzer-"; class AnalyzerDiagnosticConsumer : public ento::PathDiagnosticConsumer { public: @@ -351,10 +352,9 @@ ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory( static void setStaticAnalyzerCheckerOpts(const ClangTidyOptions &Opts, clang::AnalyzerOptions &AnalyzerOptions) { - StringRef AnalyzerPrefix(AnalyzerCheckNamePrefix); for (const auto &Opt : Opts.CheckOptions) { StringRef OptName(Opt.getKey()); - if (!OptName.consume_front(AnalyzerPrefix)) + if (!OptName.consume_front(AnalyzerCheckNamePrefix)) continue; // Analyzer options are always local options so we can ignore priority. AnalyzerOptions.Config[OptName] = Opt.getValue().Value; @@ -476,7 +476,8 @@ std::vector ClangTidyASTConsumerFactory::getCheckNames() { #if CLANG_TIDY_ENABLE_STATIC_ANALYZER for (const auto &AnalyzerCheck : getAnalyzerCheckersAndPackages( Context, Context.canEnableAnalyzerAlphaCheckers())) - CheckNames.push_back(AnalyzerCheckNamePrefix + AnalyzerCheck.first); + CheckNames.emplace_back( + (AnalyzerCheckNamePrefix + AnalyzerCheck.first).str()); #endif // CLANG_TIDY_ENABLE_STATIC_ANALYZER llvm::sort(CheckNames); diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index a71813a103df3..f9d75978d0ea8 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; @@ -525,6 +544,9 @@ void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) { case clang::DiagnosticsEngine::ak_attr: Builder << reinterpret_cast(Info.getRawArg(Index)); break; + case clang::DiagnosticsEngine::ak_attr_info: + Builder << reinterpret_cast(Info.getRawArg(Index)); + break; case clang::DiagnosticsEngine::ak_addrspace: Builder << static_cast(Info.getRawArg(Index)); break; 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/abseil/AbseilMatcher.h b/clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h index 1eef86ddc00b9..2ae3c00f7ee3e 100644 --- a/clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h +++ b/clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h @@ -46,12 +46,12 @@ AST_POLYMORPHIC_MATCHER( if (PrefixPosition == StringRef::npos) return false; Path = Path.drop_front(PrefixPosition + AbslPrefix.size()); - static const char *AbseilLibraries[] = { + static constexpr llvm::StringLiteral AbseilLibraries[] = { "algorithm", "base", "container", "debugging", "flags", "hash", "iterator", "memory", "meta", "numeric", "profiling", "random", "status", "strings", "synchronization", "time", "types", "utility"}; - return llvm::any_of(AbseilLibraries, [&](const char *Library) { + return llvm::any_of(AbseilLibraries, [&](llvm::StringLiteral Library) { return Path.starts_with(Library); }); } diff --git a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp index f2746ba159d04..1cbf1e22a33a7 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ComparePointerToMemberVirtualFunctionCheck.cpp @@ -25,7 +25,7 @@ namespace { AST_MATCHER(CXXMethodDecl, isVirtual) { return Node.isVirtual(); } -static const char *const ErrorMsg = +static constexpr llvm::StringLiteral ErrorMsg = "comparing a pointer to member virtual function with other pointer is " "unspecified behavior, only compare it with a null-pointer constant for " "equality."; diff --git a/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp index 4aba5831e6772..23de8d971898e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MisleadingSetterOfReferenceCheck.cpp @@ -50,6 +50,8 @@ void MisleadingSetterOfReferenceCheck::check( const MatchFinder::MatchResult &Result) { const auto *Found = Result.Nodes.getNodeAs("bad-set-function"); const auto *Member = Result.Nodes.getNodeAs("member"); + assert(Found != nullptr); + assert(Member != nullptr); diag(Found->getBeginLoc(), "function '%0' can be mistakenly used in order to change the " diff --git a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp index db0ac281ddfcf..c1ea63cda5003 100644 --- a/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/TaggedUnionMemberCountCheck.cpp @@ -144,7 +144,8 @@ TaggedUnionMemberCountCheck::getNumberOfEnumValues(const EnumDecl *ED) { if (EnableCountingEnumHeuristic && LastEnumConstant && isCountingEnumLikeName(LastEnumConstant->getName()) && - (LastEnumConstant->getInitVal() == (EnumValues.size() - 1))) { + llvm::APSInt::isSameValue(LastEnumConstant->getInitVal(), + llvm::APSInt::get(EnumValues.size() - 1))) { return {EnumValues.size() - 1, LastEnumConstant}; } 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/ProTypeMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp index aaaaf6b66b51a..b413b12cd37ab 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp @@ -373,8 +373,8 @@ static bool isEmpty(ASTContext &Context, const QualType &Type) { return isIncompleteOrZeroLengthArrayType(Context, Type); } -static const char *getInitializer(QualType QT, bool UseAssignment) { - const char *DefaultInitializer = "{}"; +static llvm::StringLiteral getInitializer(QualType QT, bool UseAssignment) { + static constexpr llvm::StringLiteral DefaultInitializer = "{}"; if (!UseAssignment) return DefaultInitializer; 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/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/misc/UnconventionalAssignOperatorCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp index afc4897eeb2ae..3fdaf9239f6af 100644 --- a/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp @@ -66,8 +66,11 @@ void UnconventionalAssignOperatorCheck::registerMatchers( hasArgument(0, cxxThisExpr())), cxxOperatorCallExpr( hasOverloadedOperatorName("="), - hasArgument( - 0, unaryOperator(hasOperatorName("*"), + hasArgument(0, unaryOperator(hasOperatorName("*"), + hasUnaryOperand(cxxThisExpr())))), + binaryOperator( + hasOperatorName("="), + hasLHS(unaryOperator(hasOperatorName("*"), hasUnaryOperand(cxxThisExpr()))))))))); const auto IsGoodAssign = cxxMethodDecl(IsAssign, HasGoodReturnType); 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/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp index 30fcba367db67..936a906651f16 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp @@ -119,7 +119,8 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) { if (StartLoc.isMacroID() && IgnoreMacros) return; - static const char *UseUsingWarning = "use 'using' instead of 'typedef'"; + static constexpr llvm::StringLiteral UseUsingWarning = + "use 'using' instead of 'typedef'"; // Warn at StartLoc but do not fix if there is macro or array. if (MatchedDecl->getUnderlyingType()->isArrayType() || StartLoc.isMacroID()) { 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/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 20a238612a7e4..197c62c40dcf0 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 934d52086b3b9..e021d6350694e 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. @@ -206,6 +212,21 @@ Changes in existing checks ` 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. @@ -245,6 +266,11 @@ Changes in existing checks ` check by providing additional examples and fixing some macro related false positives. +- Improved :doc:`misc-unconventional-assign-operator + ` check by fixing + false positives when copy assignment operator function in a template class + returns the result of another assignment to ``*this`` (``return *this=...``). + - Improved :doc:`misc-unused-using-decls ` check by fixing false positives on ``operator""`` with template parameters. @@ -313,6 +339,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/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/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index ccb78ee45e9c4..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" 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