Author: Jason Molenda Date: 2022-11-25T12:32:33-08:00 New Revision: 2b2f2f66141d52dc0d3082ddd12805d36872a189
URL: https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189 DIFF: https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189.diff LOG: Revert "[LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer" This reverts commit 4346318f5c700f4e85f866610fb8328fc429319b. This test case is failing on macOS, reverting until it can be looked at more closely to unblock the macOS CI bots. ``` File "/Volumes/work/llvm/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py", line 121, in test_libcpp self.do_test(USE_LIBCPP) File "/Volumes/work/llvm/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py", line 45, in do_test self.expect_expr("noop_hdl", File "/Volumes/work/llvm/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2441, in expect_expr value_check.check_value(self, eval_result, str(eval_result)) File "/Volumes/work/llvm/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 306, in check_value test_base.assertEqual(self.expect_summary, val.GetSummary(), AssertionError: 'noop_coroutine' != 'coro frame = 0x100004058' - noop_coroutine+ coro frame = 0x100004058 : (std::coroutine_handle<void>) $1 = coro frame = 0x100004058 { resume = 0x0000000100003344 (a.out`___lldb_unnamed_symbol223) destroy = 0x0000000100003344 (a.out`___lldb_unnamed_symbol223) } Checking SBValue: (std::coroutine_handle<void>) $1 = coro frame = 0x100004058 { resume = 0x0000000100003344 (a.out`___lldb_unnamed_symbol223) destroy = 0x0000000100003344 (a.out`___lldb_unnamed_symbol223) } ``` Those lldb_unnamed_symbols are synthetic names that ObjectFileMachO adds to the symbol table, most often seen with stripped binaries, based off of the function start addresses for all the functions - if a function has no symbol name, lldb adds one of these names. This change was originally landed via https://reviews.llvm.org/D132624 Added: Modified: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp 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 aa6a6ef7e56ae..bd9ff99db67b8 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp @@ -35,7 +35,7 @@ static ValueObjectSP GetCoroFramePtrFromHandle(ValueObject &valobj) { return ptr_sp; } -static Function *ExtractFunction(ValueObjectSP &frame_ptr_sp, int offset) { +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(); @@ -47,64 +47,24 @@ static Function *ExtractFunction(ValueObjectSP &frame_ptr_sp, int offset) { lldbassert(addr_type == AddressType::eAddressTypeLoad); Status error; - auto func_ptr_addr = frame_ptr_addr + offset * ptr_size; - lldb::addr_t func_addr = - process_sp->ReadPointerFromMemory(func_ptr_addr, 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 func_address; - if (!target_sp->ResolveLoadAddress(func_addr, func_address)) + Address destroy_func_address; + if (!target_sp->ResolveLoadAddress(destroy_func_addr, destroy_func_address)) return nullptr; - return func_address.CalculateSymbolContextFunction(); -} - -static Function *ExtractResumeFunction(ValueObjectSP &frame_ptr_sp) { - return ExtractFunction(frame_ptr_sp, 0); -} - -static Function *ExtractDestroyFunction(ValueObjectSP &frame_ptr_sp) { - return ExtractFunction(frame_ptr_sp, 1); -} - -static bool IsNoopCoroFunction(Function *f) { - if (!f) - return false; - - // clang's `__builtin_coro_noop` gets lowered to - // `_NoopCoro_ResumeDestroy`. This is used by libc++ - // on clang. - auto mangledName = f->GetMangled().GetMangledName(); - if (mangledName == "__NoopCoro_ResumeDestroy") - return true; - - // libc++ uses the following name as a fallback on - // compilers without `__builtin_coro_noop`. - auto name = f->GetNameNoArguments(); - static RegularExpression libcxxRegex( - "^std::coroutine_handle<std::noop_coroutine_promise>::" - "__noop_coroutine_frame_ty_::__dummy_resume_destroy_func$"); - lldbassert(libcxxRegex.IsValid()); - if (libcxxRegex.Execute(name.GetStringRef())) - return true; - static RegularExpression libcxxRegexAbiNS( - "^std::__[[:alnum:]]+::coroutine_handle<std::__[[:alnum:]]+::" - "noop_coroutine_promise>::__noop_coroutine_frame_ty_::" - "__dummy_resume_destroy_func$"); - lldbassert(libcxxRegexAbiNS.IsValid()); - if (libcxxRegexAbiNS.Execute(name.GetStringRef())) - return true; - - // libstdc++ uses the following name on both gcc and clang. - static RegularExpression libstdcppRegex( - "^std::__[[:alnum:]]+::coroutine_handle<std::__[[:alnum:]]+::" - "noop_coroutine_promise>::__frame::__dummy_resume_destroy$"); - lldbassert(libstdcppRegex.IsValid()); - if (libstdcppRegex.Execute(name.GetStringRef())) - return true; + Function *destroy_func = + destroy_func_address.CalculateSymbolContextFunction(); + if (!destroy_func) + return nullptr; - return false; + return destroy_func; } static CompilerType InferPromiseType(Function &destroy_func) { @@ -153,15 +113,9 @@ bool lldb_private::formatters::StdlibCoroutineHandleSummaryProvider( if (!ptr_sp->GetValueAsUnsigned(0)) { stream << "nullptr"; - return true; - } - if (IsNoopCoroFunction(ExtractResumeFunction(ptr_sp)) && - IsNoopCoroFunction(ExtractDestroyFunction(ptr_sp))) { - stream << "noop_coroutine"; - return true; + } else { + stream.Printf("coro frame = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0)); } - - stream.Printf("coro frame = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0)); return true; } @@ -204,14 +158,6 @@ bool lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd:: if (!ptr_sp) return false; - Function *resume_func = ExtractResumeFunction(ptr_sp); - Function *destroy_func = ExtractDestroyFunction(ptr_sp); - - if (IsNoopCoroFunction(resume_func) && IsNoopCoroFunction(destroy_func)) { - // For `std::noop_coroutine()`, we don't want to display any child nodes. - return false; - } - // Get the `promise_type` from the template argument CompilerType promise_type( valobj_sp->GetCompilerType().GetTypeTemplateArgument(0)); @@ -223,10 +169,12 @@ bool lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd:: auto ast_ctx = ts.dyn_cast_or_null<TypeSystemClang>(); if (!ast_ctx) return false; - if (promise_type.IsVoidType() && destroy_func) { - 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); + 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); + } } } 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 d1b4d59de5f5f..44e5e6451c10d 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 @@ -38,13 +38,6 @@ def do_test(self, stdlib_type): ValueCheck(name="current_value", value = "-1"), ]) ]) - # We recognize and pretty-print `std::noop_coroutine`. We don't display - # any children as those are irrelevant for the noop coroutine. - # clang version < 16 did not yet write debug info for the noop coroutines. - if not (is_clang and self.expectedCompilerVersion(["<", "16"])): - self.expect_expr("noop_hdl", - result_summary="noop_coroutine", - result_children=[]) if is_clang: # For a type-erased `coroutine_handle<>`, we can still devirtualize # the promise call and display the correctly typed promise. 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 e34637f09f6ed..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 @@ -45,7 +45,6 @@ int main() { std::coroutine_handle<> type_erased_hdl = gen.hdl; std::coroutine_handle<int> incorrectly_typed_hdl = std::coroutine_handle<int>::from_address(gen.hdl.address()); - std::coroutine_handle<> noop_hdl = std::noop_coroutine(); 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
