Skip to content

Commit

Permalink
Revert "[LLDB] Recognize std::noop_coroutine() in `std::coroutine_h…
Browse files Browse the repository at this point in the history
…andle` pretty printer"

This reverts commit 4346318.

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
  • Loading branch information
jasonmolenda committed Nov 25, 2022
1 parent b738ea0 commit 2b2f2f6
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 81 deletions.
94 changes: 21 additions & 73 deletions lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
Expand Up @@ -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();
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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));
Expand All @@ -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);
}
}
}

Expand Down
Expand Up @@ -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.
Expand Down
Expand Up @@ -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
Expand Down

0 comments on commit 2b2f2f6

Please sign in to comment.