This is an archive of the discontinued LLVM Phabricator instance.

[LLDB] Add data formatter for std::coroutine_handle
ClosedPublic

Authored by avogelsgesang on Aug 22 2022, 2:56 PM.

Details

Summary

This patch adds a formatter for std::coroutine_handle, both for libc++
and libstdc++. For the type-erased coroutine_handle<>, it shows the
resume and destroy function pointers. For a non-type-erased
coroutine_handle<promise_type> it also shows the promise value.

With this change, executing the v t command on the example from
https://clang.llvm.org/docs/DebuggingCoroutines.html now outputs

(task) t = {
  handle = coro frame = 0x55555555b2a0 {
    resume = 0x0000555555555a10 (a.out`coro_task(int, int) at llvm-example.cpp:36)
    destroy = 0x0000555555556090 (a.out`coro_task(int, int) at llvm-example.cpp:36)
  }
}

instead of just

(task) t = {
  handle = {
    __handle_ = 0x55555555b2a0
  }
}

Note, how the symbols for the resume and destroy function pointer
reveal which coroutine is stored inside the std::coroutine_handle.
A follow-up commit will use this fact to infer the coroutine's promise
type and the representation of its internal coroutine state based on
the resume and destroy pointers.

The same formatter is used for both libc++ and libstdc++. It would
also work for MSVC's standard library, however it is not registered
for MSVC, given that lldb does not provide pretty printers for other
MSVC types, either.

The formatter is in a new added Coroutines.{h,cpp} file because there
does not seem to be an already existing place where we could share
formatters across libc++ and libstdc++. Also, I expect this code to grow
as we improve debugging experience for coroutines further.

Testing

  • Added API test

Diff Detail

Event Timeline

avogelsgesang created this revision.Aug 22 2022, 2:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2022, 2:56 PM
Herald added a subscriber: mgorny. · View Herald Transcript
avogelsgesang requested review of this revision.Aug 22 2022, 2:56 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 22 2022, 2:56 PM
avogelsgesang added inline comments.Aug 22 2022, 3:38 PM
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
1147

I need to remove this. This is a left-over from an earlier implementation sketch

LGTM. But I am not familiar with debugger internals. So I'll leave the formal acceptance for other reviewers.

lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
138

Add the new line.

lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
62–64 ↗(On Diff #454622)

Unrelated change? Generally we could do this in a separate NFC patch.

remove unrelated changes

avogelsgesang edited the summary of this revision. (Show Details)Aug 23 2022, 1:17 AM
aprantl added inline comments.Aug 23 2022, 10:12 AM
lldb/source/Plugins/Language/CPlusPlus/Coroutines.h
2

typo: -*- C++ -*-

21

Can you convert this to a slightly more verbose Doxygen comment? It's not clear to me what exactly this comment means.

22

It would be nice to add Doxygen comments here, too. At least for the class itself.

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
16

Nice test!

45

Does this launch a new process? It might be faster to just set an additional breakpoint and run process.Continue().

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp
36

This looks risky, can you add a call to an empty function here, so we can be reasonably sure that there is code at this line?

avogelsgesang marked 6 inline comments as done.

addressed comments from @aprantl

avogelsgesang marked an inline comment as done.Aug 23 2022, 1:05 PM
avogelsgesang added inline comments.
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
45

somehow, I assumed run_to_source_breakpoint would actually continue the running process instead of starting a new process. But looking into run_to_source_breakpoint, you are right, this launches a new process...

I changed this to instead continue execution of the already launched process

jingham added inline comments.
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
16

Would you mind changing moving this into lldbutil.py? This seems generally useful. If you return the thread & breakpoint it would be even more generally useful?

45

There is also lldbutil.continue_to_breakpoint,

jingham added inline comments.Aug 23 2022, 3:48 PM
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
45

Forgot to delete that inline comment before saving...

avogelsgesang marked 4 inline comments as done.
avogelsgesang added inline comments.
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
16

moving this into lldbutil.py

done

shafik added a subscriber: shafik.Aug 23 2022, 9:12 PM
shafik added inline comments.
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
40

/*result_type=*/ see https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html

applies to the rest of them as well.

avogelsgesang marked an inline comment as done.
avogelsgesang added inline comments.
lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
40

done

aprantl accepted this revision.Aug 24 2022, 11:52 AM
This revision is now accepted and ready to land.Aug 24 2022, 11:52 AM
This revision was automatically updated to reflect the committed changes.