feg208 wrote:
@clayborg I rolled up those requested changes. That'll make a nicer interface
https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
@@ -195,48 +195,66 @@ class ProcessInstanceInfo : public ProcessInfo {
return m_process_session_id != LLDB_INVALID_PROCESS_ID;
}
- struct timespec GetUserTime() const { return m_user_time; }
+ struct timespec GetUserTime() const { return m_user_time.value(); }
@@ -195,48 +195,66 @@ class ProcessInstanceInfo : public ProcessInfo {
return m_process_session_id != LLDB_INVALID_PROCESS_ID;
}
- struct timespec GetUserTime() const { return m_user_time; }
+ struct timespec GetUserTime() const { return m_user_time.value(); }
@@ -195,48 +195,66 @@ class ProcessInstanceInfo : public ProcessInfo {
return m_process_session_id != LLDB_INVALID_PROCESS_ID;
}
- struct timespec GetUserTime() const { return m_user_time; }
+ struct timespec GetUserTime() const { return m_user_time.value(); }
@@ -250,10 +268,12 @@ class ProcessInstanceInfo : public ProcessInfo {
lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID;
- struct timespec m_user_time
@@ -250,10 +268,12 @@ class ProcessInstanceInfo : public ProcessInfo {
lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID;
- struct timespec m_user_time
https://github.com/feg208 edited https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo {
ProcessInstanceInfo() = default;
ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
- : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX),
-
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From 8f694b7ab4436c0540284cbbf280ad557a750920 Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
@@ -250,10 +268,12 @@ class ProcessInstanceInfo : public ProcessInfo {
lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID;
- struct timespec m_user_time
@@ -195,48 +195,66 @@ class ProcessInstanceInfo : public ProcessInfo {
return m_process_session_id != LLDB_INVALID_PROCESS_ID;
}
- struct timespec GetUserTime() const { return m_user_time; }
+ struct timespec GetUserTime() const { return m_user_time.value(); }
@@ -195,48 +195,66 @@ class ProcessInstanceInfo : public ProcessInfo {
return m_process_session_id != LLDB_INVALID_PROCESS_ID;
}
- struct timespec GetUserTime() const { return m_user_time; }
+ struct timespec GetUserTime() const { return m_user_time.value(); }
@@ -195,48 +195,66 @@ class ProcessInstanceInfo : public ProcessInfo {
return m_process_session_id != LLDB_INVALID_PROCESS_ID;
}
- struct timespec GetUserTime() const { return m_user_time; }
+ struct timespec GetUserTime() const { return m_user_time.value(); }
https://github.com/clayborg requested changes to this pull request.
We should either truly use `std::optional` and return `std::optional` for
all getters, or just not use them. This patch adds a `bool IsZombieValid()`
which the user must know to call prior to calling `bool IsZombie()`, which
@@ -195,48 +195,66 @@ class ProcessInstanceInfo : public ProcessInfo {
return m_process_session_id != LLDB_INVALID_PROCESS_ID;
}
- struct timespec GetUserTime() const { return m_user_time; }
+ struct timespec GetUserTime() const { return m_user_time.value(); }
@@ -250,10 +268,12 @@ class ProcessInstanceInfo : public ProcessInfo {
lldb::pid_t m_parent_pid = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_group_id = LLDB_INVALID_PROCESS_ID;
lldb::pid_t m_process_session_id = LLDB_INVALID_PROCESS_ID;
- struct timespec m_user_time
https://github.com/clayborg edited
https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo {
ProcessInstanceInfo() = default;
ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
- : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX),
-
feg208 wrote:
> A message like this that the PR is ready for re-review should be all you
> need. Several folks left comments and they might be waiting for all of them
> to be addressed before they take another look.
Ok. @clayborg I think I have addressed your concerns but I'd love another
JDevlieghere wrote:
> Hello all. I am wondering what I can do to advance this patch? I think it is
> required to support process save-core in linux for lldb. I'd love to move
> this before adding static methods in ThreadEfCore.h to produce populated
> prpsinfo and prstatus structs for
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From e7b1cf56dae4c93b871aeeeafc5b40752baf823e Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
feg208 wrote:
Hello all. I am wondering what I can do to advance this patch? I think it is
required to support process save-core in linux for lldb. I'd love to move this
before adding static methods in ThreadEfCore.h to produce populated prpsinfo
and prstatus structs for inclusion in a
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From 8b9ffe9bfb20c5c911c1c08ef966b4ac1ac587a0 Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo {
ProcessInstanceInfo() = default;
ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
- : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX),
-
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From b35ded1c20bb67df941bb7c54aece789a18cde99 Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From dbb4b4126754be15ffd9d12d9997e4969f7ba5cf Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo {
ProcessInstanceInfo() = default;
ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
- : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX),
-
hawkinsw wrote:
> > Sorry for the nits! I am not smart enough to contribute on the technical
> > merits, so I am trying to find some way to help!
>
> Ehhh grammar is not where I shine. I rolled them up
Not at all! I thought the comments were really excellent and helpful! I just
wanted to
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo {
ProcessInstanceInfo() = default;
ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
- : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX),
-
feg208 wrote:
> Sorry for the nits! I am not smart enough to contribute on the technical
> merits, so I am trying to find some way to help!
Ehhh grammar is not where I shine. I rolled them up
https://github.com/llvm/llvm-project/pull/91544
___
@@ -70,6 +71,12 @@ struct StatFields {
long unsigned stime;
long cutime;
long cstime;
+ // in proc_pid_stat(5) this field is specified as priority
feg208 wrote:
done
https://github.com/llvm/llvm-project/pull/91544
@@ -70,6 +71,12 @@ struct StatFields {
long unsigned stime;
long cutime;
long cstime;
+ // in proc_pid_stat(5) this field is specified as priority
+ // but documented as realtime priority. To keep with the adopted
+ // nomenclature in ProcessInstanceInfo we adopt the
@@ -70,6 +71,12 @@ struct StatFields {
long unsigned stime;
long cutime;
long cstime;
+ // in proc_pid_stat(5) this field is specified as priority
+ // but documented as realtime priority. To keep with the adopted
+ // nomenclature in ProcessInstanceInfo we adopt the
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid,
ProcessInstanceInfo ,
return ts;
};
+ // priority (nice) values run from 19 to -20 inclusive (in linux). In the
feg208 wrote:
done
https://github.com/llvm/llvm-project/pull/91544
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid,
ProcessInstanceInfo ,
return ts;
};
+ // priority (nice) values run from 19 to -20 inclusive (in linux). In the
+ // prpsinfo struct pr_nice is a char
feg208 wrote:
done
@@ -86,4 +89,22 @@ TEST_F(HostTest, GetProcessInfo) {
ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime();
ASSERT_TRUE(user_time.tv_sec <= next_user_time.tv_sec ||
user_time.tv_usec <= next_user_time.tv_usec);
+
+ struct rlimit rlim;
+
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From d01efd10f8a4b7d908acaa3237541bf6a83b4c7c Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
@@ -144,6 +144,19 @@ class ProcessInstanceInfo : public ProcessInfo {
long int tv_usec = 0;
};
+ enum class ProcessState {
+Unknown,
+Dead,
+DiskSleep,
+Idle,
+Paging,
+Parked,
+Running,
+Sleeping,
+TracedOrStopped,
+Zombie,
+
@@ -70,6 +71,12 @@ struct StatFields {
long unsigned stime;
long cutime;
long cstime;
+ // in proc_pid_stat(5) this field is specified as priority
hawkinsw wrote:
```suggestion
// In proc_pid_stat(5) this field is specified as priority
```
@@ -70,6 +71,12 @@ struct StatFields {
long unsigned stime;
long cutime;
long cstime;
+ // in proc_pid_stat(5) this field is specified as priority
+ // but documented as realtime priority. To keep with the adopted
+ // nomenclature in ProcessInstanceInfo we adopt the
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid,
ProcessInstanceInfo ,
return ts;
};
+ // priority (nice) values run from 19 to -20 inclusive (in linux). In the
+ // prpsinfo struct pr_nice is a char
hawkinsw wrote:
```suggestion
//
@@ -70,6 +71,12 @@ struct StatFields {
long unsigned stime;
long cutime;
long cstime;
+ // in proc_pid_stat(5) this field is specified as priority
+ // but documented as realtime priority. To keep with the adopted
+ // nomenclature in ProcessInstanceInfo we adopt the
@@ -115,13 +124,19 @@ static bool GetStatusInfo(::pid_t Pid,
ProcessInstanceInfo ,
return ts;
};
+ // priority (nice) values run from 19 to -20 inclusive (in linux). In the
hawkinsw wrote:
```suggestion
// Priority (nice) values run from 19 to -20
@@ -86,4 +89,22 @@ TEST_F(HostTest, GetProcessInfo) {
ProcessInstanceInfo::timespec next_user_time = Info.GetUserTime();
ASSERT_TRUE(user_time.tv_sec <= next_user_time.tv_sec ||
user_time.tv_usec <= next_user_time.tv_usec);
+
+ struct rlimit rlim;
+
https://github.com/hawkinsw commented:
Sorry for the nits! I am not smart enough to contribute on the technical
merits, so I am trying to find some way to help!
https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
https://github.com/hawkinsw edited
https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -147,96 +148,111 @@ class ProcessInstanceInfo : public ProcessInfo {
ProcessInstanceInfo() = default;
ProcessInstanceInfo(const char *name, const ArchSpec , lldb::pid_t pid)
- : ProcessInfo(name, arch, pid), m_euid(UINT32_MAX), m_egid(UINT32_MAX),
-
@@ -91,14 +87,16 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo
,
if (Rest.empty())
return false;
StatFields stat_fields;
- if (sscanf(Rest.data(),
- "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld",
- _fields.pid,
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From dbf56b2ebe93d2f3ef6e41bceb7359f6e10ae38d Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
@@ -254,6 +280,8 @@ class ProcessInstanceInfo : public ProcessInfo {
struct timespec m_system_time {};
struct timespec m_cumulative_user_time {};
struct timespec m_cumulative_system_time {};
+ std::optional m_priority_value = std::nullopt;
+ bool m_zombie = false;
@@ -144,6 +144,19 @@ class ProcessInstanceInfo : public ProcessInfo {
long int tv_usec = 0;
};
+ enum class ProcessState {
+Unknown,
+Dead,
+DiskSleep,
+Idle,
+Paging,
+Parked,
+Running,
+Sleeping,
+TracedOrStopped,
+Zombie,
+
@@ -12,6 +12,9 @@
#include "lldb/Utility/ProcessInfo.h"
#include "gtest/gtest.h"
+#include
+#include
clayborg wrote:
This is fine if this is a host test, so nothing needs to be done. We are
expecting linux + posix here which is fine.
@@ -254,6 +280,8 @@ class ProcessInstanceInfo : public ProcessInfo {
struct timespec m_system_time {};
struct timespec m_cumulative_user_time {};
struct timespec m_cumulative_system_time {};
+ std::optional m_priority_value = std::nullopt;
+ bool m_zombie = false;
@@ -91,14 +87,16 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo
,
if (Rest.empty())
return false;
StatFields stat_fields;
- if (sscanf(Rest.data(),
- "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld",
- _fields.pid,
@@ -12,6 +12,9 @@
#include "lldb/Utility/ProcessInfo.h"
#include "gtest/gtest.h"
+#include
+#include
emaste wrote:
Much of this file would work just fine on FreeBSD as well so that might make
sense, although I'm not sure what the best structure would be
@@ -237,6 +250,16 @@ class ProcessInstanceInfo : public ProcessInfo {
m_cumulative_system_time.tv_usec > 0;
}
+ int8_t GetNiceValue() const { return m_nice_value; }
feg208 wrote:
prpsinfo has pr_nice field to be populated
@@ -237,6 +250,16 @@ class ProcessInstanceInfo : public ProcessInfo {
m_cumulative_system_time.tv_usec > 0;
}
+ int8_t GetNiceValue() const { return m_nice_value; }
feg208 wrote:
Done. Agreed. I also added a note in the linux specific code. In
@@ -237,6 +250,16 @@ class ProcessInstanceInfo : public ProcessInfo {
m_cumulative_system_time.tv_usec > 0;
}
+ int8_t GetNiceValue() const { return m_nice_value; }
+
+ void SetNiceValue(int8_t nice_value) { m_nice_value = nice_value; }
+
+ bool
@@ -17,6 +17,7 @@
#include
#include
+#include
feg208 wrote:
removed
https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
@@ -117,6 +118,10 @@ bool ProcessInfo::IsScriptedProcess() const {
return m_scripted_metadata_sp && *m_scripted_metadata_sp;
}
+bool ProcessInstanceInfo::NiceValueIsValid() const {
+ return m_nice_value >= PRIO_MIN && m_nice_value <= PRIO_MAX;
feg208
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From 18b0d55d1c4e04842e531c7f7e304998f2b2ad4e Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
@@ -237,6 +250,16 @@ class ProcessInstanceInfo : public ProcessInfo {
m_cumulative_system_time.tv_usec > 0;
}
+ int8_t GetNiceValue() const { return m_nice_value; }
emaste wrote:
nice and priority are not synonymous though, nice is one input
@@ -12,6 +12,9 @@
#include "lldb/Utility/ProcessInfo.h"
#include "gtest/gtest.h"
+#include
+#include
feg208 wrote:
yeah. The cmake file enforces a linux-only build as well. However as @jimingham
pointed out someone could plausibly move this into a
@@ -254,6 +277,8 @@ class ProcessInstanceInfo : public ProcessInfo {
struct timespec m_system_time {};
struct timespec m_cumulative_user_time {};
struct timespec m_cumulative_system_time {};
+ int8_t m_nice_value = INT8_MAX;
feg208 wrote:
yeah. I'll
@@ -144,6 +144,19 @@ class ProcessInstanceInfo : public ProcessInfo {
long int tv_usec = 0;
};
+ enum class ProcessState {
+Unknown,
+Dead,
+DiskSleep,
+Idle,
+Paging,
+Parked,
+Running,
+Sleeping,
+TracedOrStopped,
+Zombie,
+
https://github.com/clayborg edited
https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -12,6 +12,9 @@
#include "lldb/Utility/ProcessInfo.h"
#include "gtest/gtest.h"
+#include
+#include
clayborg wrote:
it is ok to include this header file here because this is a linux specific host
test.
https://github.com/llvm/llvm-project/pull/91544
@@ -17,6 +17,7 @@
#include
#include
+#include
clayborg wrote:
Not every OS will have this header file, so I am not sure we can/should be
importing this header. Windows build will most likely fail and any non unix
based OS as well.
@@ -144,6 +144,19 @@ class ProcessInstanceInfo : public ProcessInfo {
long int tv_usec = 0;
};
+ enum class ProcessState {
+Unknown,
+Dead,
+DiskSleep,
+Idle,
+Paging,
+Parked,
+Running,
+Sleeping,
+TracedOrStopped,
+Zombie,
+
@@ -254,6 +277,8 @@ class ProcessInstanceInfo : public ProcessInfo {
struct timespec m_system_time {};
struct timespec m_cumulative_user_time {};
struct timespec m_cumulative_system_time {};
+ int8_t m_nice_value = INT8_MAX;
clayborg wrote:
maybe make
https://github.com/clayborg requested changes to this pull request.
So `ProcessInstanceInfo` is already very OS specific, so I can't object to
adding more OS specific information to this structure. It would be nice to
abstract OS specific things into a dictionary so allow OS specific things to
@@ -117,6 +118,10 @@ bool ProcessInfo::IsScriptedProcess() const {
return m_scripted_metadata_sp && *m_scripted_metadata_sp;
}
+bool ProcessInstanceInfo::NiceValueIsValid() const {
+ return m_nice_value >= PRIO_MIN && m_nice_value <= PRIO_MAX;
clayborg
https://github.com/clayborg edited
https://github.com/llvm/llvm-project/pull/91544
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -237,6 +250,16 @@ class ProcessInstanceInfo : public ProcessInfo {
m_cumulative_system_time.tv_usec > 0;
}
+ int8_t GetNiceValue() const { return m_nice_value; }
+
+ void SetNiceValue(int8_t nice_value) { m_nice_value = nice_value; }
+
+ bool
@@ -237,6 +250,16 @@ class ProcessInstanceInfo : public ProcessInfo {
m_cumulative_system_time.tv_usec > 0;
}
+ int8_t GetNiceValue() const { return m_nice_value; }
bulbazord wrote:
Suggestion: `nice` -> `priority_level` or something similar.
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 1aeb64c8ec7b96b2301929d8a325a6e1d9ddaa2f
8355e9f14b898572d81721118e6a842d63652a33 --
https://github.com/feg208 updated
https://github.com/llvm/llvm-project/pull/91544
>From 57e47b53682d43d2b1febba721688c6931c43291 Mon Sep 17 00:00:00 2001
From: Fred Grim
Date: Wed, 8 May 2024 15:36:16 -0700
Subject: [PATCH] [lldb] Adds additional fields to ProcessInfo
To implement SaveCore
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Fred Grim (feg208)
Changes
To implement SaveCore for elf binaries we need to populate some additional
fields in the prpsinfo struct. Those fields are the nice value of the process
whose core is to be taken as well as a boolean flag
https://github.com/feg208 created
https://github.com/llvm/llvm-project/pull/91544
To implement SaveCore for elf binaries we need to populate some additional
fields in the prpsinfo struct. Those fields are the nice value of the process
whose core is to be taken as well as a boolean flag
79 matches
Mail list logo
