Author: Adrian Vogelsgesang Date: 2022-11-20T06:35:16-08:00 New Revision: 01f4c305fae9ff2f165ce0f635a90f8e2292308c
URL: https://github.com/llvm/llvm-project/commit/01f4c305fae9ff2f165ce0f635a90f8e2292308c DIFF: https://github.com/llvm/llvm-project/commit/01f4c305fae9ff2f165ce0f635a90f8e2292308c.diff LOG: Reapply "[LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`" The original commit was missing a `ClangASTImporter::CopyType` call. Original commit message: This commit teaches the `std::coroutine_handle` pretty-printer to devirtualize type-erased promise types. This is particularly useful to resonstruct call stacks, either of asynchronous control flow or of recursive invocations of `std::generator`. For the example recently introduced by https://reviews.llvm.org/D132451, printing the `__promise` variable now shows ``` (std::__coroutine_traits_sfinae<task, void>::promise_type) __promise = { continuation = coro frame = 0x555555562430 { resume = 0x0000555555556310 (a.out`task detail::chain_fn<1>() at llvm-nested-example.cpp:66) destroy = 0x0000555555556700 (a.out`task detail::chain_fn<1>() at llvm-nested-example.cpp:66) promise = { continuation = coro frame = 0x5555555623e0 { resume = 0x0000555555557070 (a.out`task detail::chain_fn<2>() at llvm-nested-example.cpp:66) destroy = 0x0000555555557460 (a.out`task detail::chain_fn<2>() at llvm-nested-example.cpp:66) promise = { ... } } result = 0 } } result = 0 } ``` (shortened to keep the commit message readable) instead of ``` (std::__coroutine_traits_sfinae<task, void>::promise_type) __promise = { continuation = coro frame = 0x555555562430 { resume = 0x0000555555556310 (a.out`task detail::chain_fn<1>() at llvm-nested-example.cpp:66) destroy = 0x0000555555556700 (a.out`task detail::chain_fn<1>() at llvm-nested-example.cpp:66) } result = 0 } ``` Note how the new debug output reveals the complete asynchronous call stack: our own function resumes `chain_fn<1>` which in turn will resume `chain_fn<2>` and so on. Thereby this change allows users of lldb to inspect the logical coroutine call stack without using any custom debug scripts (although the display is still a bit clumsy. It would be nicer to also integrate this into lldb's backtrace feature, but I don't know how to do so) The devirtualization currently works by introspecting the function pointed to by the `destroy` pointer. (The `resume` pointer is not worth much, given that for the final suspend point `resume` is set to a nullptr. We have to use the `destroy` pointer instead.) We then look for a `__promise` variable inside the `destroy` function. This `__promise` variable is synthetically generated by LLVM, and looking at its type reveals the type-erased promise_type. This approach only works for clang-generated code, though. While gcc also adds a `_Coro_promise` variable to the `resume` function, it does not do so for the `destroy` function. However, we can't use the `resume` function, as it will be reset to a nullptr at the final suspension point. For the time being, I am happy with de-virtualization only working for clang. A follow-up commit will further improve devirtualization and also expose the variables spilled to the coroutine frame. As part of this, I will also revisit gcc support. Differential Revision: https://reviews.llvm.org/D132624 Added: Modified: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp lldb/source/Plugins/Language/CPlusPlus/Coroutines.h lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp index de5d1831c01ef..bd9ff99db67b8 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp @@ -8,7 +8,10 @@ #include "Coroutines.h" +#include "Plugins/ExpressionParser/Clang/ClangASTImporter.h" #include "Plugins/TypeSystem/Clang/TypeSystemClang.h" +#include "lldb/Symbol/Function.h" +#include "lldb/Symbol/VariableList.h" using namespace lldb; using namespace lldb_private; @@ -32,6 +35,56 @@ static ValueObjectSP GetCoroFramePtrFromHandle(ValueObject &valobj) { return ptr_sp; } +static Function *ExtractDestroyFunction(ValueObjectSP &frame_ptr_sp) { + lldb::TargetSP target_sp = frame_ptr_sp->GetTargetSP(); + lldb::ProcessSP process_sp = frame_ptr_sp->GetProcessSP(); + auto ptr_size = process_sp->GetAddressByteSize(); + + AddressType addr_type; + lldb::addr_t frame_ptr_addr = frame_ptr_sp->GetPointerValue(&addr_type); + if (!frame_ptr_addr || frame_ptr_addr == LLDB_INVALID_ADDRESS) + return nullptr; + lldbassert(addr_type == AddressType::eAddressTypeLoad); + + Status error; + // The destroy pointer is the 2nd pointer inside the compiler-generated + // `pair<resumePtr,destroyPtr>`. + auto destroy_func_ptr_addr = frame_ptr_addr + ptr_size; + lldb::addr_t destroy_func_addr = + process_sp->ReadPointerFromMemory(destroy_func_ptr_addr, error); + if (error.Fail()) + return nullptr; + + Address destroy_func_address; + if (!target_sp->ResolveLoadAddress(destroy_func_addr, destroy_func_address)) + return nullptr; + + Function *destroy_func = + destroy_func_address.CalculateSymbolContextFunction(); + if (!destroy_func) + return nullptr; + + return destroy_func; +} + +static CompilerType InferPromiseType(Function &destroy_func) { + Block &block = destroy_func.GetBlock(true); + auto variable_list = block.GetBlockVariableList(true); + + // clang generates an artificial `__promise` variable inside the + // `destroy` function. Look for it. + auto promise_var = variable_list->FindVariable(ConstString("__promise")); + if (!promise_var) + return {}; + if (!promise_var->IsArtificial()) + return {}; + + Type *promise_type = promise_var->GetType(); + if (!promise_type) + return {}; + return promise_type->GetForwardCompilerType(); +} + static CompilerType GetCoroutineFrameType(TypeSystemClang &ast_ctx, CompilerType promise_type) { CompilerType void_type = ast_ctx.GetBasicType(lldb::eBasicTypeVoid); @@ -58,13 +111,18 @@ bool lldb_private::formatters::StdlibCoroutineHandleSummaryProvider( if (!ptr_sp) return false; - stream.Printf("coro frame = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0)); + if (!ptr_sp->GetValueAsUnsigned(0)) { + stream << "nullptr"; + } else { + stream.Printf("coro frame = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0)); + } return true; } lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd:: StdlibCoroutineHandleSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp) - : SyntheticChildrenFrontEnd(*valobj_sp) { + : SyntheticChildrenFrontEnd(*valobj_sp), + m_ast_importer(std::make_unique<ClangASTImporter>()) { if (valobj_sp) Update(); } @@ -100,15 +158,27 @@ bool lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd:: if (!ptr_sp) return false; + // Get the `promise_type` from the template argument + CompilerType promise_type( + valobj_sp->GetCompilerType().GetTypeTemplateArgument(0)); + if (!promise_type) + return false; + + // Try to infer the promise_type if it was type-erased auto ts = valobj_sp->GetCompilerType().GetTypeSystem(); auto ast_ctx = ts.dyn_cast_or_null<TypeSystemClang>(); if (!ast_ctx) return false; + if (promise_type.IsVoidType()) { + if (Function *destroy_func = ExtractDestroyFunction(ptr_sp)) { + if (CompilerType inferred_type = InferPromiseType(*destroy_func)) { + // Copy the type over to the correct `TypeSystemClang` instance + promise_type = m_ast_importer->CopyType(*ast_ctx, inferred_type); + } + } + } - CompilerType promise_type( - valobj_sp->GetCompilerType().GetTypeTemplateArgument(0)); - if (!promise_type) - return false; + // Build the coroutine frame type CompilerType coro_frame_type = GetCoroutineFrameType(*ast_ctx, promise_type); m_frame_ptr_sp = ptr_sp->Cast(coro_frame_type.GetPointerType()); diff --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h index f5846c6b8cfbe..c052bd23f4ca9 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h +++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h @@ -15,6 +15,9 @@ #include "lldb/Utility/Stream.h" namespace lldb_private { + +class ClangASTImporter; + namespace formatters { /// Summary provider for `std::coroutine_handle<T>` from libc++, libstdc++ and @@ -45,6 +48,7 @@ class StdlibCoroutineHandleSyntheticFrontEnd private: lldb::ValueObjectSP m_frame_ptr_sp; + std::unique_ptr<lldb_private::ClangASTImporter> m_ast_importer; }; SyntheticChildrenFrontEnd * diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py index 224a550a5346d..ed03fd0961cce 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py @@ -16,6 +16,7 @@ class TestCoroutineHandle(TestBase): def do_test(self, stdlib_type): """Test std::coroutine_handle is displayed correctly.""" self.build(dictionary={stdlib_type: "1"}) + is_clang = self.expectedCompiler(["clang"]) test_generator_func_ptr_re = re.compile( r"^\(a.out`my_generator_func\(\) at main.cpp:[0-9]*\)$") @@ -37,14 +38,31 @@ def do_test(self, stdlib_type): ValueCheck(name="current_value", value = "-1"), ]) ]) - # For type-erased `coroutine_handle<>` we are missing the `promise` - # but still show `resume` and `destroy`. - self.expect_expr("type_erased_hdl", - result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"), - result_children=[ - ValueCheck(name="resume", summary = test_generator_func_ptr_re), - ValueCheck(name="destroy", summary = test_generator_func_ptr_re), - ]) + if is_clang: + # For a type-erased `coroutine_handle<>`, we can still devirtualize + # the promise call and display the correctly typed promise. + self.expect_expr("type_erased_hdl", + result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"), + result_children=[ + ValueCheck(name="resume", summary = test_generator_func_ptr_re), + ValueCheck(name="destroy", summary = test_generator_func_ptr_re), + ValueCheck(name="promise", children=[ + ValueCheck(name="current_value", value = "-1"), + ]) + ]) + # For an incorrectly typed `coroutine_handle`, we use the user-supplied + # incorrect type instead of inferring the correct type. Strictly speaking, + # incorrectly typed coroutine handles are undefined behavior. However, + # it provides probably a better debugging experience if we display the + # promise as seen by the program instead of fixing this bug based on + # the available debug info. + self.expect_expr("incorrectly_typed_hdl", + result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"), + result_children=[ + ValueCheck(name="resume", summary = test_generator_func_ptr_re), + ValueCheck(name="destroy", summary = test_generator_func_ptr_re), + ValueCheck(name="promise", value="-1") + ]) # Run until after the `co_yield` process = self.process() @@ -73,6 +91,18 @@ def do_test(self, stdlib_type): ValueCheck(name="current_value", value = "42"), ]) ]) + if is_clang: + # Devirtualization still works, also at the final suspension point, despite + # the `resume` pointer being reset to a nullptr + self.expect_expr("type_erased_hdl", + result_summary=re.compile("^coro frame = 0x[0-9a-f]*$"), + result_children=[ + ValueCheck(name="resume", value = "0x0000000000000000"), + ValueCheck(name="destroy", summary = test_generator_func_ptr_re), + ValueCheck(name="promise", children=[ + ValueCheck(name="current_value", value = "42"), + ]) + ]) @add_test_categories(["libstdcxx"]) def test_libstdcpp(self): diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp index 439a25866d1c0..8cb81c3bc9f4c 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp @@ -43,6 +43,8 @@ int main() { bool is_supported = is_implementation_supported(); int_generator gen = my_generator_func(); std::coroutine_handle<> type_erased_hdl = gen.hdl; + std::coroutine_handle<int> incorrectly_typed_hdl = + std::coroutine_handle<int>::from_address(gen.hdl.address()); gen.hdl.resume(); // Break at initial_suspend gen.hdl.resume(); // Break after co_yield empty_function_so_we_can_set_a_breakpoint(); // Break at final_suspend _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
