Skip to content

[lldb] Add parent address to Task synthetic provider #10936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 48 additions & 14 deletions lldb/source/Plugins/Language/Swift/SwiftFormatters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,13 @@ namespace lldb_private {
namespace formatters {
namespace swift {

namespace {
/// The size of Swift Tasks. Fragments are tail allocated.
static constexpr size_t AsyncTaskSize = sizeof(::swift::AsyncTask);
/// The offset of ChildFragment, which is the first fragment of an AsyncTask.
static constexpr offset_t ChildFragmentOffset = AsyncTaskSize;
} // namespace

class EnumSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
public:
EnumSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
Expand Down Expand Up @@ -806,6 +813,7 @@ class TaskSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
"address",
"id",
"enqueuePriority",
"parent",
"children",

// Children below this point are hidden.
Expand Down Expand Up @@ -873,6 +881,33 @@ class TaskSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
RETURN_CHILD(m_enqueue_priority_sp, enqueuePriority, priority_type);
}
case 3: {
if (!m_parent_task_sp) {
// TypeMangling for "Swift.UnsafeRawPointer"
CompilerType raw_pointer_type =
m_ts->GetTypeFromMangledTypename(ConstString("$sSVD"));

addr_t parent_addr = 0;
if (m_task_info.isChildTask) {
if (auto process_sp = m_backend.GetProcessSP()) {
Status status;
// Read ChildFragment::Parent, the first field of the ChildFragment.
parent_addr = process_sp->ReadPointerFromMemory(
m_task_ptr + ChildFragmentOffset, status);
if (status.Fail() || parent_addr == LLDB_INVALID_ADDRESS)
parent_addr = 0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really what we want to do here? When the task has no parent, does the runtime write a 0 to its slot, or do we expect an error?

If the runtime is explicitly writing a 0 and we don't expect the memory read to fail, I think we can just return nullptr here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the case of a parent-less task, should we try to customize the formatter output to say something "root task" or "no parent", instead of 0s?

I think 0s could give users the impression that the debugger failed to get the value

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the task has no parent, does the runtime write a 0 to its slot

A swift Task can have tail allocated "fragments", including a ChildFragment. Child tasks have a ChildFragment, while root tasks do not.

One way to model that is to make the existence of a parent conditional. In other words, for a root task, no parent member will be shown at all. The reason I didn't do that is:

  1. Users might find it confusing (why is there a parent some times, but not other times)
  2. It makes the synthetic child provider more complex (which is fine if we have good reason)

For the case of a parent-less task, should we try to customize the formatter output to say something "root task" or "no parent", instead of 0s?

The parent member needs a data type, which currently is UnsafeRawPointer. To show a task-specific string like "root task", we'd need to use some other data type, one we invent ourselves. This adds complexity, and will be exposed to users who might wonder "what's a LLDBParentTaskPointer?".

We could put a string like "root task" into Task's summary formatter. In which case the parent = 0x0000... would be accompanied with "root task" in the summary string, which should explain the parent address.

On this subject, one of the questions I have for reviewers is the name of this member. I used parent, but felt like make it should be parent_addr. The main reason I didn't do that is the provider has an address field, and I didn't like the inconsistency of using address and addr, and I didn't like the verbose choice of parent_address. Maybe we can rename address to addr, and then use parent_addr. Thoughts? I think including addr in the name might make it more clear that a bunch of zeros means root task/no parent.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A swift Task can have tail allocated "fragments", including a ChildFragment. Child tasks have a ChildFragment, while root tasks do not.

I misspoke. A root task will of course have a ChildFragment if it has children.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adrian's suggestion is to have the parent's type be UnsafeRawPointer?, and will show nil when there's no parent. Works for me, what do you think Felipe?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup that sounds good to me!

}
}
addr_t value = parent_addr;
DataExtractor data{reinterpret_cast<const void *>(&value),
sizeof(value), endian::InlHostByteOrder(),
sizeof(void *)};
m_parent_task_sp = ValueObject::CreateValueObjectFromData(
"parent", data, m_backend.GetExecutionContextRef(),
raw_pointer_type);
}
return m_parent_task_sp;
}
case 4: {
if (!m_child_tasks_sp) {
using task_type = decltype(m_task_info.childTasks)::value_type;
std::vector<task_type> tasks = m_task_info.childTasks;
Expand Down Expand Up @@ -901,26 +936,26 @@ class TaskSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
}
return m_child_tasks_sp;
}
case 4:
RETURN_CHILD(m_is_child_task_sp, isChildTask, bool_type);
case 5:
RETURN_CHILD(m_is_future_sp, isFuture, bool_type);
RETURN_CHILD(m_is_child_task_sp, isChildTask, bool_type);
case 6:
RETURN_CHILD(m_is_group_child_task_sp, isGroupChildTask, bool_type);
RETURN_CHILD(m_is_future_sp, isFuture, bool_type);
case 7:
RETURN_CHILD(m_is_async_let_task_sp, isAsyncLetTask, bool_type);
RETURN_CHILD(m_is_group_child_task_sp, isGroupChildTask, bool_type);
case 8:
RETURN_CHILD(m_is_cancelled_sp, isCancelled, bool_type);
RETURN_CHILD(m_is_async_let_task_sp, isAsyncLetTask, bool_type);
case 9:
RETURN_CHILD(m_is_cancelled_sp, isCancelled, bool_type);
case 10:
RETURN_CHILD(m_is_status_record_locked_sp, isStatusRecordLocked,
bool_type);
case 10:
RETURN_CHILD(m_is_escalated_sp, isEscalated, bool_type);
case 11:
RETURN_CHILD(m_is_enqueued_sp, isEnqueued, bool_type);
RETURN_CHILD(m_is_escalated_sp, isEscalated, bool_type);
case 12:
RETURN_CHILD(m_is_enqueued_sp, isEnqueued, bool_type);
case 13:
RETURN_CHILD(m_is_complete_sp, isComplete, bool_type);
case 13: {
case 14: {
if (m_task_info.hasIsRunning)
RETURN_CHILD(m_is_running_sp, isRunning, bool_type);
return {};
Expand Down Expand Up @@ -953,8 +988,8 @@ class TaskSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
m_is_child_task_sp, m_is_future_sp, m_is_group_child_task_sp,
m_is_async_let_task_sp, m_is_cancelled_sp,
m_is_status_record_locked_sp, m_is_escalated_sp,
m_is_enqueued_sp, m_is_complete_sp, m_child_tasks_sp,
m_is_running_sp})
m_is_enqueued_sp, m_is_complete_sp, m_parent_task_sp,
m_child_tasks_sp, m_is_running_sp})
child.reset();
}
}
Expand Down Expand Up @@ -990,6 +1025,7 @@ class TaskSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
ValueObjectSP m_is_escalated_sp;
ValueObjectSP m_is_enqueued_sp;
ValueObjectSP m_is_complete_sp;
ValueObjectSP m_parent_task_sp;
ValueObjectSP m_child_tasks_sp;
ValueObjectSP m_is_running_sp;
};
Expand Down Expand Up @@ -1280,8 +1316,6 @@ class TaskGroupSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
bool operator==(const Task &other) const { return addr == other.addr; }
bool operator!=(const Task &other) const { return !(*this == other); }

static constexpr offset_t AsyncTaskSize = sizeof(::swift::AsyncTask);
static constexpr offset_t ChildFragmentOffset = AsyncTaskSize;
static constexpr offset_t NextChildOffset = ChildFragmentOffset + 0x8;

Task getNextChild(Status &status) {
Expand Down