Skip to content

[LLDB] Add IsCoreDumping to ProcessInstanceInfo #138580

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.

Sign up for GitHub

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
merged 3 commits into from
May 7, 2025
Merged

[LLDB] Add IsCoreDumping to ProcessInstanceInfo #138580

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03d5449
Add IsCoredumping to ProcessInfo to use in the future of the ptrace_s…
Jlalond May 5, 2025
7e2b11f
Add test
Jlalond May 5, 2025
697912d
Handle error in case we fail to parse CoreDumping
Jlalond May 6, 2025
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
6 changes: 6 additions & 0 deletions lldb/include/lldb/Utility/ProcessInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ class ProcessInstanceInfo : public ProcessInfo {

std::optional<bool> IsZombie() const { return m_zombie; }

// proc/../status specifies CoreDumping as the field
// so we match the case here.
void SetIsCoreDumping(bool is_coredumping) { m_coredumping = is_coredumping; }
std::optional<bool> IsCoreDumping() const { return m_coredumping; }

void Dump(Stream &s, UserIDResolver &resolver) const;

static void DumpTableHeader(Stream &s, bool show_args, bool verbose);
Expand All @@ -266,6 +271,7 @@ class ProcessInstanceInfo : public ProcessInfo {
struct timespec m_cumulative_system_time;
std::optional<int8_t> m_priority_value = std::nullopt;
std::optional<bool> m_zombie = std::nullopt;
std::optional<bool> m_coredumping = std::nullopt;
};

typedef std::vector<ProcessInstanceInfo> ProcessInstanceInfoList;
Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Host/linux/Host.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo &ProcessInfo,
} else if (Line.consume_front("Tgid:")) {
Line = Line.ltrim();
Line.consumeInteger(10, Tgid);
} else if (Line.consume_front("CoreDumping:")) {
uint32_t coredumping;
Line = Line.ltrim();
Line.consumeInteger(1, coredumping);
Copy link
Collaborator

@DavidSpickett DavidSpickett May 6, 2025

Choose a reason for hiding this comment

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

This is calling:

    template <typename T> bool consumeInteger(unsigned Radix, T &Result) {
  • Radix of 1 means binary?
  • Parsing can fail, though now I look above and nothing else checks the return value, sigh...

This is new though, so can you make it check the return value and not set if it's false? The type is optional, so anyone making use of this is handling the "I don't know" state already.

Copy link
Contributor Author

@Jlalond Jlalond May 6, 2025

Choose a reason for hiding this comment

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

Good suggestion, I'll validate the return.

ProcessInfo.SetIsCoreDumping(coredumping);
}
}
return true;
Expand Down
3 changes: 3 additions & 0 deletions lldb/unittests/Host/posix/HostTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,8 @@ TEST_F(HostTest, GetProcessInfoSetsPriority) {
}
ASSERT_TRUE(Info.IsZombie().has_value());
ASSERT_FALSE(Info.IsZombie().value());

ASSERT_TRUE(Info.IsCoreDumping().has_value());
ASSERT_FALSE(Info.IsCoreDumping().value());
}
#endif
Loading