[Lldb-commits] [lldb] d7e1c27 - Revert "[LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`"

Adrian Vogelsgesang via lldb-commits lldb-commits at lists.llvm.org
Fri Nov 11 10:01:08 PST 2022


Author: Adrian Vogelsgesang
Date: 2022-11-11T10:00:58-08:00
New Revision: d7e1c2770fa57fb8df2f09f74d433ac9cf80a595

URL: https://github.com/llvm/llvm-project/commit/d7e1c2770fa57fb8df2f09f74d433ac9cf80a595
DIFF: https://github.com/llvm/llvm-project/commit/d7e1c2770fa57fb8df2f09f74d433ac9cf80a595.diff

LOG: Revert "[LLDB] Devirtualize coroutine promise types for `std::coroutine_handle`"

This reverts commit 558db7787005348e2efaabb628ec36f1c461a741 due to
buildbot failures on ARM
* https://lab.llvm.org/buildbot/#/builders/96/builds/31416
* https://lab.llvm.org/buildbot/#/builders/17/builds/30086

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 89ee4bb67f931..5427a7a2b7850 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -9,8 +9,6 @@
 #include "Coroutines.h"
 
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
-#include "lldb/Symbol/Function.h"
-#include "lldb/Symbol/VariableList.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -34,57 +32,6 @@ 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) {
-  SymbolContext sc;
-  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);
@@ -111,11 +58,7 @@ bool lldb_private::formatters::StdlibCoroutineHandleSummaryProvider(
   if (!ptr_sp)
     return false;
 
-  if (!ptr_sp->GetValueAsUnsigned(0)) {
-    stream << "nullptr";
-  } else {
-    stream.Printf("coro frame = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0));
-  }
+  stream.Printf("coro frame = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0));
   return true;
 }
 
@@ -157,26 +100,15 @@ bool lldb_private::formatters::StdlibCoroutineHandleSyntheticFrontEnd::
   if (!ptr_sp)
     return false;
 
-  // Get the `promise_type` from the template argument
+  TypeSystemClang *ast_ctx = llvm::dyn_cast_or_null<TypeSystemClang>(
+      valobj_sp->GetCompilerType().GetTypeSystem());
+  if (!ast_ctx)
+    return false;
+
   CompilerType promise_type(
       valobj_sp->GetCompilerType().GetTypeTemplateArgument(0));
   if (!promise_type)
     return false;
-
-  // Try to infer the promise_type if it was type-erased
-  if (promise_type.IsVoidType()) {
-    if (Function *destroy_func = ExtractDestroyFunction(ptr_sp)) {
-      if (CompilerType inferred_type = InferPromiseType(*destroy_func)) {
-        promise_type = inferred_type;
-      }
-    }
-  }
-
-  // Build the coroutine frame type
-  TypeSystemClang *ast_ctx = llvm::dyn_cast_or_null<TypeSystemClang>(
-      ptr_sp->GetCompilerType().GetTypeSystem());
-  if (!ast_ctx)
-    return {};
   CompilerType coro_frame_type = GetCoroutineFrameType(*ast_ctx, promise_type);
 
   m_frame_ptr_sp = ptr_sp->Cast(coro_frame_type.GetPointerType());

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 ed03fd0961cce..224a550a5346d 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,7 +16,6 @@ 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]*\)$")
@@ -38,31 +37,14 @@ def do_test(self, stdlib_type):
                     ValueCheck(name="current_value", value = "-1"),
                 ])
             ])
-        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")
-                ])
+        # 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),
+            ])
 
         # Run until after the `co_yield`
         process = self.process()
@@ -91,18 +73,6 @@ 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 8cb81c3bc9f4c..439a25866d1c0 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,8 +43,6 @@ 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


        


More information about the lldb-commits mailing list