From 2f5dbfd99349d76d24d242a7ad053c09e3af0630 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 24 Aug 2023 18:04:13 +0800 Subject: [PATCH] Backport patch from LLVM 18 to fix a miscompliation related to coroutine Signed-off-by: Kefu Chai --- PR59723.diff | 114 +++++++++++++++++++++++++++++++++++++++++++++++++++ llvm.spec | 8 +++- 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 PR59723.diff diff --git a/PR59723.diff b/PR59723.diff new file mode 100644 index 0000000..726699c --- /dev/null +++ b/PR59723.diff @@ -0,0 +1,114 @@ +diff --git a/llvm/lib/Transforms/Coroutines/CoroElide.cpp b/llvm/lib/Transforms/Coroutines/CoroElide.cpp +index f032c568449b..3e31d95586c5 100644 +--- a/llvm/lib/Transforms/Coroutines/CoroElide.cpp ++++ b/llvm/lib/Transforms/Coroutines/CoroElide.cpp +@@ -192,12 +192,49 @@ bool Lowerer::hasEscapePath(const CoroBeginInst *CB, + for (auto *DA : It->second) + Visited.insert(DA->getParent()); + ++ SmallPtrSet EscapingBBs; ++ for (auto *U : CB->users()) { ++ // The use from coroutine intrinsics are not a problem. ++ if (isa(U)) ++ continue; ++ ++ // Think all other usages may be an escaping candidate conservatively. ++ // ++ // Note that the major user of switch ABI coroutine (the C++) will store ++ // resume.fn, destroy.fn and the index to the coroutine frame immediately. ++ // So the parent of the coro.begin in C++ will be always escaping. ++ // Then we can't get any performance benefits for C++ by improving the ++ // precision of the method. ++ // ++ // The reason why we still judge it is we want to make LLVM Coroutine in ++ // switch ABIs to be self contained as much as possible instead of a ++ // by-product of C++20 Coroutines. ++ EscapingBBs.insert(cast(U)->getParent()); ++ } ++ ++ bool PotentiallyEscaped = false; ++ + do { + const auto *BB = Worklist.pop_back_val(); + if (!Visited.insert(BB).second) + continue; +- if (TIs.count(BB)) +- return true; ++ ++ // A Path insensitive marker to test whether the coro.begin escapes. ++ // It is intentional to make it path insensitive while it may not be ++ // precise since we don't want the process to be too slow. ++ PotentiallyEscaped |= EscapingBBs.count(BB); ++ ++ if (TIs.count(BB)) { ++ if (!BB->getTerminator()->isExceptionalTerminator() || PotentiallyEscaped) ++ return true; ++ ++ // If the function ends with the exceptional terminator, the memory used ++ // by the coroutine frame can be released by stack unwinding ++ // automatically. So we can think the coro.begin doesn't escape if it ++ // exits the function by exceptional terminator. ++ ++ continue; ++ } + + // Conservatively say that there is potentially a path. + if (!--Limit) +@@ -234,36 +271,36 @@ bool Lowerer::shouldElide(Function *F, DominatorTree &DT) const { + // memory location storing that value and not the virtual register. + + SmallPtrSet Terminators; +- // First gather all of the non-exceptional terminators for the function. ++ // First gather all of the terminators for the function. + // Consider the final coro.suspend as the real terminator when the current + // function is a coroutine. +- for (BasicBlock &B : *F) { +- auto *TI = B.getTerminator(); +- if (TI->getNumSuccessors() == 0 && !TI->isExceptionalTerminator() && +- !isa(TI)) +- Terminators.insert(&B); +- } ++ for (BasicBlock &B : *F) { ++ auto *TI = B.getTerminator(); ++ ++ if (TI->getNumSuccessors() != 0 || isa(TI)) ++ continue; ++ ++ Terminators.insert(&B); ++ } + + // Filter out the coro.destroy that lie along exceptional paths. + SmallPtrSet ReferencedCoroBegins; + for (const auto &It : DestroyAddr) { +- // If there is any coro.destroy dominates all of the terminators for the +- // coro.begin, we could know the corresponding coro.begin wouldn't escape. +- for (Instruction *DA : It.second) { +- if (llvm::all_of(Terminators, [&](auto *TI) { +- return DT.dominates(DA, TI->getTerminator()); +- })) { +- ReferencedCoroBegins.insert(It.first); +- break; +- } +- } +- +- // Whether there is any paths from coro.begin to Terminators which not pass +- // through any of the coro.destroys. ++ // If every terminators is dominated by coro.destroy, we could know the ++ // corresponding coro.begin wouldn't escape. ++ // ++ // Otherwise hasEscapePath would decide whether there is any paths from ++ // coro.begin to Terminators which not pass through any of the ++ // coro.destroys. + // + // hasEscapePath is relatively slow, so we avoid to run it as much as + // possible. +- if (!ReferencedCoroBegins.count(It.first) && ++ if (llvm::all_of(Terminators, ++ [&](auto *TI) { ++ return llvm::any_of(It.second, [&](auto *DA) { ++ return DT.dominates(DA, TI->getTerminator()); ++ }); ++ }) || + !hasEscapePath(It.first, Terminators)) + ReferencedCoroBegins.insert(It.first); + } diff --git a/llvm.spec b/llvm.spec index 21c1079..8776f88 100644 --- a/llvm.spec +++ b/llvm.spec @@ -75,7 +75,7 @@ Name: %{pkg_name} Version: %{maj_ver}.%{min_ver}.%{patch_ver}%{?rc_ver:~rc%{rc_ver}} -Release: 2%{?dist} +Release: 3%{?dist} Summary: The Low Level Virtual Machine License: Apache-2.0 WITH LLVM-exception OR NCSA @@ -92,6 +92,8 @@ Source6: release-keys.asc Patch2: 0001-llvm-Add-install-targets-for-gtest.patch # Backport of https://reviews.llvm.org/D156379 from LLVM 18. Patch3: D156379.diff +# Backport of https://reviews.llvm.org/rG7037331a2f05990cd59f35a7c0f6ce87c0f3cb5f from LLVM 18 +Patch4: PR59723.diff # RHEL-specific patch to avoid unwanted recommonmark dep Patch101: 0101-Deactivate-markdown-doc.patch @@ -566,6 +568,10 @@ fi %endif %changelog +* Thu Aug 24 2023 Kefu Chai - 16.0.6-3 +- Fix the stack-use-after-return when using coroutine + See https://github.com/llvm/llvm-project/issues/59723 + * Thu Aug 03 2023 Tulio Magno Quites Machado Filho - 16.0.6-2 - Fix rhbz #2224885