[lldb] Show coro_frame in std::coroutine_handle pretty printer

[vogelsgesang]

This commit adjusts the pretty printer for std::coroutine_handle based on recent personal experiences with debugging C++20 coroutines:

1. It adds the coro_frame member. This member exposes the complete coroutine frame contents, including the suspension point id and all internal variables which the compiler decided to persist into the coroutine frame. While this data is highly compiler-specific, inspecting it can help identify the internal state of suspended coroutines.
2. It includes the promise and coro_frame members, even if devirtualization failed and we could not infer the promise type / the coro_frame type. Having them available as void* pointers can still be useful to identify, e.g., which two coroutine handles have the same frame / promise pointers.

[llvmbot]

@llvm/pr-subscribers-coroutines

@llvm/pr-subscribers-lldb

Author: Adrian Vogelsgesang (vogelsgesang)

Changes

This commit adjusts the pretty printer for std::corotoutine_handle based on recent personal experiences with debugging C++20 coroutines:

1. It adds the coro_frame member. This member exposes the complete coroutine frame contents, including the suspension point id and all internal variables which the compiler decided to persist into the coroutine frame. While this data is highly compiler-specific, inspecting it can help identify the internal state of suspended coroutines.
2. It includes the promise and coro_frame members, even if devirtualization failed and we could not infer the promise type / the coro_frame type. Having them available as void* pointers can still be useful to identify, e.g., which two coroutines have the same frame / promise pointers.

---

Full diff: https://github.com/llvm/llvm-project/pull/141516.diff

5 Files Affected:

• (modified) lldb/include/lldb/DataFormatters/TypeSynthetic.h (+1-1)
• (modified) lldb/source/DataFormatters/TypeSynthetic.cpp (+2-2)
• (modified) lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp (+67-81)
• (modified) lldb/source/Plugins/Language/CPlusPlus/Coroutines.h (+1-3)
• (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py (+29-17)

diff --git a/lldb/include/lldb/DataFormatters/TypeSynthetic.h b/lldb/include/lldb/DataFormatters/TypeSynthetic.h
index c8d7d15588065..138d297305b53 100644
--- a/lldb/include/lldb/DataFormatters/TypeSynthetic.h
+++ b/lldb/include/lldb/DataFormatters/TypeSynthetic.h
@@ -92,7 +92,7 @@ class SyntheticChildrenFrontEnd {
   lldb::ValueObjectSP
   CreateValueObjectFromAddress(llvm::StringRef name, uint64_t address,
                                const ExecutionContext &exe_ctx,
-                               CompilerType type);
+                               CompilerType type, bool do_deref = true);
 
   lldb::ValueObjectSP CreateValueObjectFromData(llvm::StringRef name,
                                                 const DataExtractor &data,
diff --git a/lldb/source/DataFormatters/TypeSynthetic.cpp b/lldb/source/DataFormatters/TypeSynthetic.cpp
index 57009b07dc553..e8440d07f9593 100644
--- a/lldb/source/DataFormatters/TypeSynthetic.cpp
+++ b/lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -138,9 +138,9 @@ lldb::ValueObjectSP SyntheticChildrenFrontEnd::CreateValueObjectFromExpression(
 
 lldb::ValueObjectSP SyntheticChildrenFrontEnd::CreateValueObjectFromAddress(
     llvm::StringRef name, uint64_t address, const ExecutionContext &exe_ctx,
-    CompilerType type) {
+    CompilerType type, bool do_deref) {
   ValueObjectSP valobj_sp(
-      ValueObject::CreateValueObjectFromAddress(name, address, exe_ctx, type));
+      ValueObject::CreateValueObjectFromAddress(name, address, exe_ctx, type, do_deref));
   if (valobj_sp)
     valobj_sp->SetSyntheticChildrenGenerated(true);
   return valobj_sp;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
index 376555936e89d..903b5f7cc9203 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -11,22 +11,20 @@
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/VariableList.h"
-#include "lldb/Utility/LLDBLog.h"
-#include "lldb/Utility/Log.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
-static lldb::addr_t GetCoroFramePtrFromHandle(ValueObjectSP valobj_sp) {
-  if (!valobj_sp)
+static lldb::addr_t GetCoroFramePtrFromHandle(ValueObjectSP corohandle_sp) {
+  if (!corohandle_sp)
     return LLDB_INVALID_ADDRESS;
 
   // We expect a single pointer in the `coroutine_handle` class.
   // We don't care about its name.
-  if (valobj_sp->GetNumChildrenIgnoringErrors() != 1)
+  if (corohandle_sp->GetNumChildrenIgnoringErrors() != 1)
     return LLDB_INVALID_ADDRESS;
-  ValueObjectSP ptr_sp(valobj_sp->GetChildAtIndex(0));
+  ValueObjectSP ptr_sp(corohandle_sp->GetChildAtIndex(0));
   if (!ptr_sp)
     return LLDB_INVALID_ADDRESS;
   if (!ptr_sp->GetCompilerType().IsPointerType())
@@ -62,19 +60,20 @@ static Function *ExtractDestroyFunction(lldb::TargetSP target_sp,
   return destroy_func_address.CalculateSymbolContextFunction();
 }
 
-static CompilerType InferPromiseType(Function &destroy_func) {
+// clang generates aritifical `__promise` and `__coro_frame` variables inside
+// the destroy function. Look for those variables and extract their type.
+static CompilerType InferArtificialCoroType(Function &destroy_func,
+                                            ConstString var_name) {
   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)
+  auto var = variable_list->FindVariable(var_name);
+  if (!var)
     return {};
-  if (!promise_var->IsArtificial())
+  if (!var->IsArtificial())
     return {};
 
-  Type *promise_type = promise_var->GetType();
+  Type *promise_type = var->GetType();
   if (!promise_type)
     return {};
   return promise_type->GetForwardCompilerType();
@@ -108,30 +107,17 @@ lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
 
 llvm::Expected<uint32_t> lldb_private::formatters::
     StdlibCoroutineHandleSyntheticFrontEnd::CalculateNumChildren() {
-  if (!m_resume_ptr_sp || !m_destroy_ptr_sp)
-    return 0;
-
-  return m_promise_ptr_sp ? 3 : 2;
+  return m_children.size();
 }
 
 lldb::ValueObjectSP lldb_private::formatters::
     StdlibCoroutineHandleSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
-  switch (idx) {
-  case 0:
-    return m_resume_ptr_sp;
-  case 1:
-    return m_destroy_ptr_sp;
-  case 2:
-    return m_promise_ptr_sp;
-  }
-  return lldb::ValueObjectSP();
+  return idx < m_children.size() ? m_children[idx] : lldb::ValueObjectSP();
 }
 
 lldb::ChildCacheState
 lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::Update() {
-  m_resume_ptr_sp.reset();
-  m_destroy_ptr_sp.reset();
-  m_promise_ptr_sp.reset();
+  m_children.clear();
 
   ValueObjectSP valobj_sp = m_backend.GetNonSyntheticValue();
   if (!valobj_sp)
@@ -141,60 +127,65 @@ lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::Update() {
   if (frame_ptr_addr == 0 || frame_ptr_addr == LLDB_INVALID_ADDRESS)
     return lldb::ChildCacheState::eRefetch;
 
-  auto ts = valobj_sp->GetCompilerType().GetTypeSystem();
-  auto ast_ctx = ts.dyn_cast_or_null<TypeSystemClang>();
-  if (!ast_ctx)
-    return lldb::ChildCacheState::eRefetch;
-
-  // Create the `resume` and `destroy` children.
   lldb::TargetSP target_sp = m_backend.GetTargetSP();
   auto &exe_ctx = m_backend.GetExecutionContextRef();
   lldb::ProcessSP process_sp = target_sp->GetProcessSP();
   auto ptr_size = process_sp->GetAddressByteSize();
-  CompilerType void_type = ast_ctx->GetBasicType(lldb::eBasicTypeVoid);
-  CompilerType coro_func_type = ast_ctx->CreateFunctionType(
-      /*result_type=*/void_type, /*args=*/&void_type, /*num_args=*/1,
-      /*is_variadic=*/false, /*qualifiers=*/0);
-  CompilerType coro_func_ptr_type = coro_func_type.GetPointerType();
-  m_resume_ptr_sp = CreateValueObjectFromAddress(
-      "resume", frame_ptr_addr + 0 * ptr_size, exe_ctx, coro_func_ptr_type);
-  lldbassert(m_resume_ptr_sp);
-  m_destroy_ptr_sp = CreateValueObjectFromAddress(
-      "destroy", frame_ptr_addr + 1 * ptr_size, exe_ctx, coro_func_ptr_type);
-  lldbassert(m_destroy_ptr_sp);
-
-  // Get the `promise_type` from the template argument
-  CompilerType promise_type(
-      valobj_sp->GetCompilerType().GetTypeTemplateArgument(0));
-  if (!promise_type)
+  auto ts = valobj_sp->GetCompilerType().GetTypeSystem();
+  auto ast_ctx = ts.dyn_cast_or_null<TypeSystemClang>();
+  if (!ast_ctx)
     return lldb::ChildCacheState::eRefetch;
 
-  // Try to infer the promise_type if it was type-erased
+  // Determine the coroutine frame type and the promise type. Fall back
+  // to `void`, since even the pointer itself might be useful, even if the
+  // type inference failed.
+  Function *destroy_func = ExtractDestroyFunction(target_sp, frame_ptr_addr);
+  CompilerType void_type = ast_ctx->GetBasicType(lldb::eBasicTypeVoid);
+  CompilerType promise_type;
+  if (CompilerType template_argt =
+          valobj_sp->GetCompilerType().GetTypeTemplateArgument(0))
+    promise_type = std::move(template_argt);
   if (promise_type.IsVoidType()) {
-    if (Function *destroy_func =
-            ExtractDestroyFunction(target_sp, frame_ptr_addr)) {
-      if (CompilerType inferred_type = InferPromiseType(*destroy_func)) {
+    // Try to infer the promise_type if it was type-erased
+    if (destroy_func) {
+      if (CompilerType inferred_type = InferArtificialCoroType(
+              *destroy_func, ConstString("__promise"))) {
         promise_type = inferred_type;
       }
     }
   }
+  CompilerType coro_frame_type =
+      InferArtificialCoroType(*destroy_func, ConstString("__coro_frame"));
+  if (!coro_frame_type)
+    coro_frame_type = void_type;
 
-  // If we don't know the promise type, we don't display the `promise` member.
-  // `CreateValueObjectFromAddress` below would fail for `void` types.
-  if (promise_type.IsVoidType()) {
-    return lldb::ChildCacheState::eRefetch;
-  }
-
-  // Add the `promise` member. We intentionally add `promise` as a pointer type
-  // instead of a value type, and don't automatically dereference this pointer.
-  // We do so to avoid potential very deep recursion in case there is a cycle
-  // formed between `std::coroutine_handle`s and their promises.
-  lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
-      "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);
-  Status error;
-  lldb::ValueObjectSP promisePtr = promise->AddressOf(error);
-  if (error.Success())
-    m_promise_ptr_sp = promisePtr->Clone(ConstString("promise"));
+  // Create the `resume` and `destroy` children.
+  CompilerType coro_func_type = ast_ctx->CreateFunctionType(
+      /*result_type=*/void_type, /*args=*/&coro_frame_type, /*num_args=*/1,
+      /*is_variadic=*/false, /*qualifiers=*/0);
+  CompilerType coro_func_ptr_type = coro_func_type.GetPointerType();
+  ValueObjectSP resume_ptr_sp = CreateValueObjectFromAddress(
+      "resume", frame_ptr_addr + 0 * ptr_size, exe_ctx, coro_func_ptr_type);
+  lldbassert(resume_ptr_sp);
+  m_children.push_back(std::move(resume_ptr_sp));
+  ValueObjectSP destroy_ptr_sp = CreateValueObjectFromAddress(
+      "destroy", frame_ptr_addr + 1 * ptr_size, exe_ctx, coro_func_ptr_type);
+  lldbassert(destroy_ptr_sp);
+  m_children.push_back(std::move(destroy_ptr_sp));
+
+  // Add promise and coro_frame
+  // Add the `promise` and `coro_frame` member. We intentionally add them as
+  // pointer types instead of a value type, and don't automatically dereference
+  // those pointers. We do so to avoid potential very deep recursion in case
+  // there is a cycle formed between `std::coroutine_handle`s and their
+  // promises.
+  ValueObjectSP promise_ptr_sp = CreateValueObjectFromAddress(
+      "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx,
+      promise_type.GetPointerType(), /*do_deref=*/false);
+  m_children.push_back(std::move(promise_ptr_sp));
+  ValueObjectSP coroframe_ptr_sp = CreateValueObjectFromAddress(
+      "coro_frame", frame_ptr_addr, exe_ctx, coro_frame_type);
+  m_children.push_back(std::move(coroframe_ptr_sp));
 
   return lldb::ChildCacheState::eRefetch;
 }
@@ -202,16 +193,11 @@ lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::Update() {
 llvm::Expected<size_t>
 StdlibCoroutineHandleSyntheticFrontEnd::GetIndexOfChildWithName(
     ConstString name) {
-  if (!m_resume_ptr_sp || !m_destroy_ptr_sp)
-    return llvm::createStringError("Type has no child named '%s'",
-                                   name.AsCString());
-
-  if (name == ConstString("resume"))
-    return 0;
-  if (name == ConstString("destroy"))
-    return 1;
-  if (name == ConstString("promise_ptr") && m_promise_ptr_sp)
-    return 2;
+  for (size_t i = 0, limit = m_children.size(); i < limit; ++i) {
+    if (m_children[i]->GetName() == name) {
+      return i;
+    }
+  }
 
   return llvm::createStringError("Type has no child named '%s'",
                                  name.AsCString());
diff --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
index fd9445d46e6a0..520d8e0b3c79d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
@@ -43,9 +43,7 @@ class StdlibCoroutineHandleSyntheticFrontEnd
   llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override;
 
 private:
-  lldb::ValueObjectSP m_resume_ptr_sp;
-  lldb::ValueObjectSP m_destroy_ptr_sp;
-  lldb::ValueObjectSP m_promise_ptr_sp;
+  std::vector<lldb::ValueObjectSP> m_children;
 };
 
 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 ae1a0c86b45d8..f216b9be8d610 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
@@ -43,11 +43,17 @@ def do_test(self, stdlib_type):
                         ValueCheck(name="current_value", value="-1"),
                     ],
                 ),
+                # We don not check any members inside the `coro_frame`,
+                # as its contents are highly compiler-specific.
+                ValueCheck(name="coro_frame"),
             ],
         )
+
+        # For a type-erased `coroutine_handle<>`, we can still devirtualize
+        # the promise call and display the correctly typed promise. This
+        # currently only works in clang, because gcc is not adding the
+        # artificial `__promise` variable to the destroy function.
         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]*$"),
@@ -60,23 +66,26 @@ def do_test(self, stdlib_type):
                             ValueCheck(name="current_value", value="-1"),
                         ],
                     ),
+                    ValueCheck(name="coro_frame"),
                 ],
             )
-            # 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", dereference=ValueCheck(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", dereference=ValueCheck(value="-1")),
+                ValueCheck(name="coro_frame"),
+            ],
+        )
 
         process = self.process()
 
@@ -107,6 +116,7 @@ def do_test(self, stdlib_type):
                         ValueCheck(name="current_value", value="42"),
                     ],
                 ),
+                ValueCheck(name="coro_frame"),
             ],
         )
 
@@ -130,6 +140,7 @@ def do_test(self, stdlib_type):
                         ValueCheck(name="current_value", value="42"),
                     ],
                 ),
+                ValueCheck(name="coro_frame"),
             ],
         )
         if is_clang:
@@ -147,6 +158,7 @@ def do_test(self, stdlib_type):
                             ValueCheck(name="current_value", value="42"),
                         ],
                     ),
+                    ValueCheck(name="coro_frame"),
                 ],
             )
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[vogelsgesang]

CC @ChuanqiXu9 @hokein, since the both of you also looked into debuggability of C++20 coroutines earlier
CC @labath @adrian-prantl as reviewers of the latest significant changes in this area (https://reviews.llvm.org/D132624, https://reviews.llvm.org/D132735, https://reviews.llvm.org/D132815)

Would appreciate your reviews on this pull request :)

[hokein]

The enhancement to the pretty-printer looks reasonable to me.
I'm not familiar with the LLDB codebase, so I'll leave the approval to the LLDB maintainers.

[ChuanqiXu9]

CC @ChuanqiXu9 @hokein, since the both of you also looked into debuggability of C++20 coroutines earlier CC @labath @adrian-prantl as reviewers of the latest significant changes in this area (https://reviews.llvm.org/D132624, https://reviews.llvm.org/D132735, https://reviews.llvm.org/D132815)

Would appreciate your reviews on this pull request :)

LGTM too. But I'd like to leave the formal approval to LLDB devs too.

[vogelsgesang]

@labath @adrian-prantl friendly ping 🙂

[labath]

I don't know much about coroutines, but the code seems fine.

[vogelsgesang]

Thanks for the review!

I don't know much about coroutines, but the code seems fine.

in case you are interested to read more about it: In #142651, I updated LLVM's "Debugging C++ coroutines" tutorial