[Lldb-commits] [lldb] [lldb][AIX] Taking Linux Host Info header's base for AIX (PR #106910)
labath wrote: (+1 to everything that David said) https://github.com/llvm/llvm-project/pull/106910 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)
https://github.com/labath approved this pull request. Thanks. Looks good. There are a bunch of linux failures reported by the linux precommit bot. I don't expect you'll get all of them, but you could try to get at least those that are reported there so that there's one less post-commit iteration. https://github.com/llvm/llvm-project/pull/107163 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
@@ -206,3 +213,29 @@ void llvm::format_provider::format(
llvm::format_provider::format(error.AsCString(), OS,
Options);
}
+
+const char *lldb_private::ExecutionResultAsCString(ExpressionResults result) {
labath wrote:
I think this also belongs to the subsequent patch.
https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
@@ -80,8 +80,9 @@ TEST_F(RemoteAwarePlatformTest,
TestResolveExecutabelOnClientByPlatform) {
static const ArchSpec process_host_arch;
EXPECT_CALL(platform, GetSupportedArchitectures(process_host_arch))
.WillRepeatedly(Return(std::vector()));
+ Status success;
labath wrote:
delete?
https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
@@ -109,6 +109,13 @@ llvm::Error Status::ToError() const {
Status::~Status() = default;
+const Status &Status::operator=(Status &&other) {
+ m_code = other.m_code;
+ m_type = other.m_type;
+ m_string = other.m_string;
labath wrote:
`std::move(other.m_string)` ?
https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
@@ -34,7 +34,7 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP
breakpoint_sp) {
}
PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
labath wrote:
Maybe take the object by value and then move it into SBError, so that the
caller has a chance to avoid copies (I expect most of them will not want to
retain the status object)?
https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
labath wrote: > > Ah I've just noticed the other PR > > Do we actually have abetter solution for stacked commits than what I'm doing > here? This setup works fine for me, but I think it's worth calling out that you intend it to be viewed that way, as it's not the completely typical way to use github. ( I probably wouldn't have realized it if Felipe did not mention this) https://github.com/llvm/llvm-project/pull/107170 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change ValueObject::AddressOf() to return Expected (NFC) (PR #106831)
AbdAlRahmanGad wrote: @jimingham This change was suggested by @adrian-prantl as good first issue for me. it's similar to this [Pull Request](https://github.com/llvm/llvm-project/pull/92979/). You can read the conversation here https://discourse.llvm.org/t/rich-disassembler-for-lldb/76952/14 https://github.com/llvm/llvm-project/pull/106831 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/106774 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -170,12 +215,8 @@ class Status {
bool Success() const;
protected:
- Status(llvm::Error error);
- /// Status code as an integer value.
- ValueType m_code = 0;
- /// The type of the above error code.
- lldb::ErrorType m_type = lldb::eErrorTypeInvalid;
- /// A string representation of the error code.
+ Status(llvm::Error error) : m_error(std::move(error)) {}
+ mutable llvm::Error m_error;
labath wrote:
Any chance to avoid the mutable thing? This sort of means that accessing the
object (even const methods) from multiple threads may not be thread-safe. I
guess that sort of already was a problem with the m_string thingy, but I expect
it to be more pronounced now.
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -37,48 +39,75 @@ class raw_ostream;
using namespace lldb;
using namespace lldb_private;
-Status::Status() {}
+char CloneableError::ID;
+char MachKernelError::ID;
+char Win32Error::ID;
+char ExpressionError::ID;
+
+namespace {
+/// A std::error_code category for eErrorTypeGeneric.
+class GenericCategory : public std::error_category {
+ const char *name() const override { return "LLDBGenericCategory"; }
+ std::string message(int __ev) const override { return "generic LLDB error";
};
+};
+GenericCategory &generic_category() {
+ static GenericCategory g_generic_category;
+ return g_generic_category;
+}
+
+/// A std::error_code category for eErrorTypeExpression.
+class ExpressionCategory : public std::error_category {
+ const char *name() const override { return "LLDBExpressionCategory"; }
+ std::string message(int __ev) const override {
+return
ExecutionResultAsCString(static_cast(__ev));
+ };
+};
+ExpressionCategory &expression_category() {
+ static ExpressionCategory g_expression_category;
+ return g_expression_category;
+}
+} // namespace
+
+Status::Status() : m_error(llvm::Error::success()) {}
+
+static llvm::Error ErrorFromEnums(Status::ValueType err, ErrorType type,
+ std::string msg) {
+ switch (type) {
+ case eErrorTypeMachKernel:
+return llvm::make_error(
+std::error_code(err, std::system_category()), msg);
+ case eErrorTypePOSIX:
+return llvm::errorCodeToError(
+std::error_code(err, std::generic_category()));
+ case eErrorTypeWin32:
+return llvm::make_error(
+std::error_code(err, std::system_category()), msg);
+ default:
+return llvm::createStringError(std::move(msg),
+ std::error_code(err, generic_category()));
+ }
+}
Status::Status(ValueType err, ErrorType type, std::string msg)
-: m_code(err), m_type(type), m_string(std::move(msg)) {}
+: m_error(ErrorFromEnums(err, type, msg)) {}
-// This logic is confusing because c++ calls the traditional (posix) errno
codes
+// This logic is confusing because C++ calls the traditional (posix) errno
codes
// "generic errors", while we use the term "generic" to mean completely
// arbitrary (text-based) errors.
Status::Status(std::error_code EC)
-: m_code(EC.value()),
- m_type(EC.category() == std::generic_category() ? eErrorTypePOSIX
- : eErrorTypeGeneric),
- m_string(EC.message()) {}
+: m_error(!EC ? llvm::Error::success()
+ : (EC.category() == std::generic_category()
+ ? llvm::errorCodeToError(EC)
+ : llvm::createStringError(EC, "generic error"))) {}
labath wrote:
Why not call `errorCodeToError` for all error categories?
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -174,33 +233,91 @@ const char *Status::AsCString(const char
*default_error_str) const {
// Clear the error and any cached error string that it might contain.
void Status::Clear() {
- m_code = 0;
- m_type = eErrorTypeInvalid;
- m_string.clear();
+ if (m_error)
+LLDB_LOG_ERRORV(GetLog(LLDBLog::API), std::move(m_error),
+"dropping error {0}");
+ m_error = llvm::Error::success();
+ llvm::consumeError(std::move(m_error));
}
// Access the error value.
-Status::ValueType Status::GetError() const { return m_code; }
+Status::ValueType Status::GetError() const {
+ Status::ValueType result = 0;
+ llvm::visitErrors(m_error, [&](const llvm::ErrorInfoBase &error) {
+std::error_code ec = error.convertToErrorCode();
+if (ec.category() == std::generic_category() ||
+ec.category() == generic_category() ||
+ec.category() == expression_category())
+ result = ec.value();
+else
+ result = 0xff;
labath wrote:
?
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -26,6 +26,52 @@ class raw_ostream;
namespace lldb_private {
+/// Going a bit against the spirit of llvm::Error,
+/// lldb_private::Status need to store errors long-term and sometimes
+/// copy them. This base class defines an interface for this
+/// operation.
+class CloneableError
+: public llvm::ErrorInfo {
labath wrote:
I think it'd be better if this inherited from ErrorInfoBase. StringError
provides the ability to store the error as a string, but I don't think that's
strictly required for all subclasses. (If a particular subclass want that
ability, it can always add it itself, and I think it'd be slightly weird to say
that e.g. MachKernelError "is a" StringError)
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -37,48 +39,75 @@ class raw_ostream;
using namespace lldb;
using namespace lldb_private;
-Status::Status() {}
+char CloneableError::ID;
+char MachKernelError::ID;
+char Win32Error::ID;
+char ExpressionError::ID;
+
+namespace {
+/// A std::error_code category for eErrorTypeGeneric.
+class GenericCategory : public std::error_category {
+ const char *name() const override { return "LLDBGenericCategory"; }
+ std::string message(int __ev) const override { return "generic LLDB error";
};
+};
+GenericCategory &generic_category() {
+ static GenericCategory g_generic_category;
+ return g_generic_category;
+}
+
+/// A std::error_code category for eErrorTypeExpression.
+class ExpressionCategory : public std::error_category {
+ const char *name() const override { return "LLDBExpressionCategory"; }
+ std::string message(int __ev) const override {
+return
ExecutionResultAsCString(static_cast(__ev));
+ };
+};
+ExpressionCategory &expression_category() {
+ static ExpressionCategory g_expression_category;
+ return g_expression_category;
+}
labath wrote:
Do we really need this? My understanding was that the std::error_code
conversion is for compatibility, and so we may not need it if we can ensure
that everyone gets the error string via `info->message()` rather than
`info->convertToErrorCode().message()`.
Since we did have this class up to this point, I don't think we have any
instances of the second pattern.
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -174,33 +233,91 @@ const char *Status::AsCString(const char
*default_error_str) const {
// Clear the error and any cached error string that it might contain.
void Status::Clear() {
- m_code = 0;
- m_type = eErrorTypeInvalid;
- m_string.clear();
+ if (m_error)
+LLDB_LOG_ERRORV(GetLog(LLDBLog::API), std::move(m_error),
+"dropping error {0}");
+ m_error = llvm::Error::success();
+ llvm::consumeError(std::move(m_error));
}
// Access the error value.
-Status::ValueType Status::GetError() const { return m_code; }
+Status::ValueType Status::GetError() const {
+ Status::ValueType result = 0;
+ llvm::visitErrors(m_error, [&](const llvm::ErrorInfoBase &error) {
+std::error_code ec = error.convertToErrorCode();
+if (ec.category() == std::generic_category() ||
+ec.category() == generic_category() ||
+ec.category() == expression_category())
+ result = ec.value();
labath wrote:
?
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
https://github.com/labath commented: I am worried about the thread safety aspects of this, in particular for these "long term storage" errors. I don't know if we have any users like this, but I think it wouldn't be unreasonable for someone to expect that they can access these errors from multiple threads (concurrently). I'm not sure if the current implementation is safe in this respect, but I suspect it isn't because of the fiddling with the "checked" flag that the Error class does. If it is a problem, one way around it might be to store the Error in a "deconstructed" form -- as a list of ErrorInfoBase objects... https://github.com/llvm/llvm-project/pull/106774 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -94,26 +123,49 @@ Status Status::FromErrorStringWithFormat(const char
*format, ...) {
return Status(string);
}
-Status Status::FromError(llvm::Error error) { return Status(std::move(error));
}
+Status Status::FromExpressionError(lldb::ExpressionResults result,
+ std::string msg) {
+ return Status(llvm::make_error(
+ std::error_code(result, expression_category()), msg));
+}
-llvm::Error Status::ToError() const {
- if (Success())
+/// Creates a deep copy of all known errors and converts all other
+/// errors to a new llvm::StringError.
+static llvm::Error CloneError(llvm::Error &error) {
+ std::vector> info;
labath wrote:
You could avoid the temporary vector by using llvm::joinErrors directly from
the callback.
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
labath wrote: > Wasn't half as bad. @labath This update introduces a `ClonableError` base > class. This will enable us (after a bit more refactoring) to move the various > Error subclass declarations closer to where they are needed. Right now the > ErrorFromEnums() method still prevents that. I think that's fine. We can do something about that, and at least it enables us to avoid adding new error types to this file. https://github.com/llvm/llvm-project/pull/106774 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
labath wrote: Also, some (unit) tests would be nice. https://github.com/llvm/llvm-project/pull/106774 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
return 0;
}
- const bool children_inherit_listen_socket = false;
+ if (gdbserver_port != 0 &&
+ (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+ }
+
// the test suite makes many connections in parallel, let's not miss any.
// The highest this should get reasonably is a function of the number
// of target CPUs. For now, let's just use 100.
const int backlog = 100;
- std::unique_ptr acceptor_up(Acceptor::Create(
- listen_host_port, children_inherit_listen_socket, error));
- if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+ std::unique_ptr sock_named;
+ std::unique_ptr sock_platform;
+ std::unique_ptr sock_gdb;
+ std::string hostname;
+ uint16_t platform_port = 0;
+ Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+ // Try to match socket name as URL - e.g., tcp://localhost:
+ if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+ fprintf(stderr, "Unknown protocol scheme \"%s\".",
+ uri->scheme.str().c_str());
+ return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+ hostname = uri->hostname;
+ if (uri->port) {
+platform_port = *(uri->port);
+ }
+} else
+ hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+ } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+ protocol = Socket::ProtocolTcp;
+ hostname = host_port->hostname;
+ platform_port = host_port->port;
+} else
+ hostname = listen_host_port;
}
- error = acceptor_up->Listen(backlog);
- if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+ std::string socket_id;
+ if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+ fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+ return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen platform: %s\n", error.AsCString());
+ return socket_error;
+}
+if (platform_port == 0)
+ platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+ socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+ /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen gdb: %s\n", error.AsCString());
+ return socket_error;
+}
+if (gdbserver_port == 0)
+ gdbserver_port = sock_gdb->GetLocalPortNumber();
+ } else {
+if (g_server) {
+ fprintf(stderr,
+ "Ambiguous parameters --server --listen %s\n"
+ "The protocol must be tcp for the server mode.",
+ listen_host_port.c_str());
+ return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+ fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+ return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+ printf("Failed to listen: %s\n", error.AsCString());
+ return socket_error;
+}
+socket_id = hostname;
}
+
if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
if (error.Fail()) {
fprintf(stderr, "failed to write socket id to %s: %s\n",
socket_file.GetPath().c_str(), error.AsCString());
return 1;
}
}
- GDBRemoteCommunicationServerPlatform platform(
- acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+ lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+ GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
if (port_offset > 0)
platform.SetPortOffset(port_offset);
- do {
-const bool children_inherit_accept_socket = true;
-
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -150,12 +153,17 @@ static void
client_handle(GDBRemoteCommunicationServerPlatform &platform,
printf("Disconnected.\n");
}
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
- std::lock_guard guard(gdbserver_portmap_mutex);
- gdbserver_portmap.FreePortForProcess(pid);
+ if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+// If not running as a server and the platform connection is closed
+if (!g_terminate) {
+ g_terminate = true;
labath wrote:
Yes, we need to be very careful, but I don't think this is how being careful
looks like. `SetMonitorProcessCallback` takes a std::function, so we can add
(or remove) as many arguments as we like.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
labath wrote: I think this file looks good now. In the interests of moving this forward, and reducing the size of this patch, could you split it into a separate patch? https://github.com/llvm/llvm-project/pull/104238 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
return 0;
}
- const bool children_inherit_listen_socket = false;
+ if (gdbserver_port != 0 &&
+ (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+ }
+
// the test suite makes many connections in parallel, let's not miss any.
// The highest this should get reasonably is a function of the number
// of target CPUs. For now, let's just use 100.
const int backlog = 100;
- std::unique_ptr acceptor_up(Acceptor::Create(
- listen_host_port, children_inherit_listen_socket, error));
- if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+ std::unique_ptr sock_named;
+ std::unique_ptr sock_platform;
+ std::unique_ptr sock_gdb;
+ std::string hostname;
+ uint16_t platform_port = 0;
+ Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+ // Try to match socket name as URL - e.g., tcp://localhost:
+ if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+ fprintf(stderr, "Unknown protocol scheme \"%s\".",
+ uri->scheme.str().c_str());
+ return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+ hostname = uri->hostname;
+ if (uri->port) {
+platform_port = *(uri->port);
+ }
+} else
+ hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+ } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+ protocol = Socket::ProtocolTcp;
+ hostname = host_port->hostname;
+ platform_port = host_port->port;
+} else
+ hostname = listen_host_port;
}
- error = acceptor_up->Listen(backlog);
- if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+ std::string socket_id;
+ if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+ fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+ return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen platform: %s\n", error.AsCString());
+ return socket_error;
+}
+if (platform_port == 0)
+ platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+ socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+ /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen gdb: %s\n", error.AsCString());
+ return socket_error;
+}
+if (gdbserver_port == 0)
+ gdbserver_port = sock_gdb->GetLocalPortNumber();
+ } else {
+if (g_server) {
+ fprintf(stderr,
+ "Ambiguous parameters --server --listen %s\n"
+ "The protocol must be tcp for the server mode.",
+ listen_host_port.c_str());
+ return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+ fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+ return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+ printf("Failed to listen: %s\n", error.AsCString());
+ return socket_error;
+}
+socket_id = hostname;
}
+
if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
if (error.Fail()) {
fprintf(stderr, "failed to write socket id to %s: %s\n",
socket_file.GetPath().c_str(), error.AsCString());
return 1;
}
}
- GDBRemoteCommunicationServerPlatform platform(
- acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+ lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+ GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
if (port_offset > 0)
platform.SetPortOffset(port_offset);
- do {
-const bool children_inherit_accept_socket = true;
-
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -160,66 +95,56 @@
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
- if (!port) {
-llvm::Expected available_port =
m_port_map.GetNextAvailablePort();
-if (available_port)
- port = *available_port;
-else
- return Status(available_port.takeError());
- }
-
- // Spawn a new thread to accept the port that gets bound after binding to
- // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+ std::ostringstream url;
+ if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+ // Just check that GDBServer exists. GDBServer must be launched after
+ // accepting the connection.
+ if (!GetDebugserverPath(nullptr))
+return Status::FromErrorString("unable to locate debugserver");
+ return Status();
labath wrote:
Could this be done in the caller? Having a function called `LaunchGDBServer`
not launch a gdb server looks somewhat suspicious.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -150,12 +153,17 @@ static void
client_handle(GDBRemoteCommunicationServerPlatform &platform,
printf("Disconnected.\n");
}
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
- std::lock_guard guard(gdbserver_portmap_mutex);
- gdbserver_portmap.FreePortForProcess(pid);
+ if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+// If not running as a server and the platform connection is closed
+if (!g_terminate) {
+ g_terminate = true;
labath wrote:
Ok, I see what you meant with
https://github.com/llvm/llvm-project/issues/101475 now. That said I don't think
this solves anything, cause the same thing could now happen with the mainloop
pointer (if it is destroyed just as this function is exiting). The way I'd
ideally solve this is by ensuring that the main loop does not terminate (which
means the main loop and the platform objects will not be destroyed) until this
function is called.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -160,66 +95,56 @@
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
- if (!port) {
-llvm::Expected available_port =
m_port_map.GetNextAvailablePort();
-if (available_port)
- port = *available_port;
-else
- return Status(available_port.takeError());
- }
-
- // Spawn a new thread to accept the port that gets bound after binding to
- // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+ std::ostringstream url;
+ if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+ // Just check that GDBServer exists. GDBServer must be launched after
+ // accepting the connection.
+ if (!GetDebugserverPath(nullptr))
+return Status::FromErrorString("unable to locate debugserver");
+ return Status();
+}
- // ignore the hostname send from the remote end, just use the ip address that
- // we're currently communicating with as the hostname
+// debugserver does not accept the URL scheme prefix.
+#if !defined(__APPLE__)
+url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
+#endif
+socket_name = GetDomainSocketPath("gdbserver").GetPath();
+url << socket_name;
+ } else {
+if (m_socket_protocol != Socket::ProtocolTcp)
+ return Status::FromErrorString("protocol must be tcp");
+ }
// Spawn a debugserver and try to get the port it listens to.
ProcessLaunchInfo debugserver_launch_info;
- if (hostname.empty())
-hostname = "127.0.0.1";
-
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-*port);
+ LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), fd);
// Do not run in a new session so that it can not linger after the platform
// closes.
debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
debugserver_launch_info.SetMonitorProcessCallback(
-
std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
-this, std::placeholders::_1));
-
- std::ostringstream url;
-// debugserver does not accept the URL scheme prefix.
-#if !defined(__APPLE__)
- url << m_socket_scheme << "://";
-#endif
- uint16_t *port_ptr = &*port;
- if (m_socket_protocol == Socket::ProtocolTcp) {
-std::string platform_uri = GetConnection()->GetURI();
-std::optional parsed_uri = URI::Parse(platform_uri);
-url << '[' << parsed_uri->hostname.str() << "]:" << *port;
- } else {
-socket_name = GetDomainSocketPath("gdbserver").GetPath();
-url << socket_name;
-port_ptr = nullptr;
- }
+ &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped);
Status error = StartDebugserverProcess(
- url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args,
-1);
+ url.str().c_str(), nullptr, debugserver_launch_info, nullptr, &args, fd);
- pid = debugserver_launch_info.GetProcessID();
- if (pid != LLDB_INVALID_PROCESS_ID) {
-std::lock_guard guard(m_spawned_pids_mutex);
-m_spawned_pids.insert(pid);
-if (*port > 0)
- m_port_map.AssociatePortWithProcess(*port, pid);
+ if (error.Success()) {
+pid = debugserver_launch_info.GetProcessID();
+if (pid != LLDB_INVALID_PROCESS_ID)
+ AddSpawnedProcess(pid);
+LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "debugserver launched successfully as pid %" PRIu64,
labath wrote:
Claiming a successful launch after a valid pid check looks suspicious. Could we
assume that a non-error Status mean the process was launched (and that the
`pid` is valid)?
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -666,7 +756,23 @@ ConnectionStatus
ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
socket_id_callback_type socket_id_callback,
Status *error_ptr) {
-#if LLDB_ENABLE_POSIX
+#ifdef _WIN32
+ int64_t fd = -1;
+ if (!s.getAsInteger(0, fd)) {
+// Assume we own fd.
+std::unique_ptr tcp_socket =
+std::make_unique((NativeSocket)fd, true, false);
+m_io_sp = std::move(tcp_socket);
+m_uri = s.str();
+return eConnectionStatusSuccess;
+ }
labath wrote:
Okay, I think I can live with that.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -25,90 +25,30 @@ namespace process_gdb_remote {
class GDBRemoteCommunicationServerPlatform
: public GDBRemoteCommunicationServerCommon {
public:
- class PortMap {
- public:
-// This class is used to restrict the range of ports that
-// platform created debugserver/gdbserver processes will
-// communicate on.
-
-// Construct an empty map, where empty means any port is allowed.
-PortMap() = default;
-
-// Make a port map with a range of free ports
-// from min_port to max_port-1.
-PortMap(uint16_t min_port, uint16_t max_port);
-
-// Add a port to the map. If it is already in the map do not modify
-// its mapping. (used ports remain used, new ports start as free)
-void AllowPort(uint16_t port);
-
-// If we are using a port map where we can only use certain ports,
-// get the next available port.
-//
-// If we are using a port map and we are out of ports, return an error.
-//
-// If we aren't using a port map, return 0 to indicate we should bind to
-// port 0 and then figure out which port we used.
-llvm::Expected GetNextAvailablePort();
-
-// Tie a port to a process ID. Returns false if the port is not in the port
-// map. If the port is already in use it will be moved to the given pid.
-// FIXME: This is and GetNextAvailablePort make create a race condition if
-// the portmap is shared between processes.
-bool AssociatePortWithProcess(uint16_t port, lldb::pid_t pid);
-
-// Free the given port. Returns false if the port is not in the map.
-bool FreePort(uint16_t port);
-
-// Free the port associated with the given pid. Returns false if there is
-// no port associated with the pid.
-bool FreePortForProcess(lldb::pid_t pid);
-
-// Returns true if there are no ports in the map, regardless of the state
-// of those ports. Meaning a map with 1 used port is not empty.
-bool empty() const;
-
- private:
-std::map m_port_map;
- };
-
GDBRemoteCommunicationServerPlatform(
- const Socket::SocketProtocol socket_protocol, const char *socket_scheme);
+ const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port);
~GDBRemoteCommunicationServerPlatform() override;
Status LaunchProcess() override;
- // Set both ports to zero to let the platform automatically bind to
- // a port chosen by the OS.
- void SetPortMap(PortMap &&port_map);
-
void SetPortOffset(uint16_t port_offset);
void SetInferiorArguments(const lldb_private::Args &args);
- // Set port if you want to use a specific port number.
- // Otherwise port will be set to the port that was chosen for you.
- Status LaunchGDBServer(const lldb_private::Args &args, std::string hostname,
- lldb::pid_t &pid, std::optional &port,
- std::string &socket_name);
+ Status LaunchGDBServer(const lldb_private::Args &args, lldb::pid_t &pid,
+ std::string &socket_name, shared_fd_t fd);
- void SetPendingGdbServer(lldb::pid_t pid, uint16_t port,
- const std::string &socket_name);
+ void SetPendingGdbServer(const std::string &socket_name);
protected:
const Socket::SocketProtocol m_socket_protocol;
- const std::string m_socket_scheme;
- std::recursive_mutex m_spawned_pids_mutex;
- std::set m_spawned_pids;
+ static std::mutex g_spawned_pids_mutex;
+ static std::set g_spawned_pids;
labath wrote:
I guess this is related to #101475, but I think we should figure out a solution
that does not require making these static.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -195,18 +195,31 @@ void ConnectToRemote(MainLoop &mainloop,
bool reverse_connect, llvm::StringRef host_and_port,
const char *const progname, const char *const subcommand,
const char *const named_pipe_path, pipe_t unnamed_pipe,
- int connection_fd) {
+ shared_fd_t connection_fd) {
Status error;
std::unique_ptr connection_up;
std::string url;
- if (connection_fd != -1) {
+ if (connection_fd != SharedSocket::kInvalidFD) {
+#ifdef _WIN32
+NativeSocket sockfd;
+error = SharedSocket::GetNativeSocket(connection_fd, sockfd);
+if (error.Fail()) {
+ llvm::errs() << llvm::formatv("error: GetNativeSocket failed: {0}\n",
+error.AsCString());
+ exit(-1);
+}
+connection_up =
+std::unique_ptr(new ConnectionFileDescriptor(new TCPSocket(
+sockfd, /*should_close=*/true,
/*child_processes_inherit=*/false)));
+#else
url = llvm::formatv("fd://{0}", connection_fd).str();
// Create the connection.
-#if LLDB_ENABLE_POSIX && !defined _WIN32
+#if LLDB_ENABLE_POSIX
labath wrote:
I don't think we actually need this. _WIN32 is basically the opposite of POSIX.
AFAICT, https://reviews.llvm.org/D33213 was just being (too) defensive.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
return 0;
}
- const bool children_inherit_listen_socket = false;
+ if (gdbserver_port != 0 &&
+ (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+ }
+
// the test suite makes many connections in parallel, let's not miss any.
// The highest this should get reasonably is a function of the number
// of target CPUs. For now, let's just use 100.
const int backlog = 100;
- std::unique_ptr acceptor_up(Acceptor::Create(
- listen_host_port, children_inherit_listen_socket, error));
- if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+ std::unique_ptr sock_named;
+ std::unique_ptr sock_platform;
+ std::unique_ptr sock_gdb;
+ std::string hostname;
+ uint16_t platform_port = 0;
+ Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+ // Try to match socket name as URL - e.g., tcp://localhost:
+ if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+ fprintf(stderr, "Unknown protocol scheme \"%s\".",
+ uri->scheme.str().c_str());
+ return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+ hostname = uri->hostname;
+ if (uri->port) {
+platform_port = *(uri->port);
+ }
+} else
+ hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+ } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+ protocol = Socket::ProtocolTcp;
+ hostname = host_port->hostname;
+ platform_port = host_port->port;
+} else
+ hostname = listen_host_port;
}
- error = acceptor_up->Listen(backlog);
- if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+ std::string socket_id;
+ if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+ fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+ return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen platform: %s\n", error.AsCString());
+ return socket_error;
+}
+if (platform_port == 0)
+ platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+ socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+ /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen gdb: %s\n", error.AsCString());
+ return socket_error;
+}
+if (gdbserver_port == 0)
+ gdbserver_port = sock_gdb->GetLocalPortNumber();
+ } else {
+if (g_server) {
+ fprintf(stderr,
+ "Ambiguous parameters --server --listen %s\n"
+ "The protocol must be tcp for the server mode.",
+ listen_host_port.c_str());
+ return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+ fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+ return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+ printf("Failed to listen: %s\n", error.AsCString());
+ return socket_error;
+}
+socket_id = hostname;
}
+
if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
if (error.Fail()) {
fprintf(stderr, "failed to write socket id to %s: %s\n",
socket_file.GetPath().c_str(), error.AsCString());
return 1;
}
}
- GDBRemoteCommunicationServerPlatform platform(
- acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+ lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+ GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
if (port_offset > 0)
platform.SetPortOffset(port_offset);
- do {
-const bool children_inherit_accept_socket = true;
-
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/104238 >From 18b8a835a3182e5be5f0dd848977333d9b8a26fa Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 30 Aug 2024 01:08:24 +0400 Subject: [PATCH 1/3] [lldb] Removed gdbserver ports map from lldb-server Listen to gdbserver-port, accept the connection and run lldb-server gdbserver --fd on all platforms. Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now. This is the part 2 of #101283. Fixes #97537, fixes #101475. --- lldb/docs/man/lldb-server.rst | 11 +- lldb/docs/resources/qemu-testing.rst | 19 +- .../posix/ConnectionFileDescriptorPosix.cpp | 21 +- .../gdb-remote/GDBRemoteCommunication.cpp | 45 ++- .../gdb-remote/GDBRemoteCommunication.h | 8 +- .../GDBRemoteCommunicationServerPlatform.cpp | 287 +-- .../GDBRemoteCommunicationServerPlatform.h| 82 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- .../TestPlatformLaunchGDBServer.py| 12 +- .../Shell/lldb-server/TestGdbserverPort.test | 4 - lldb/tools/lldb-server/Acceptor.cpp | 98 - lldb/tools/lldb-server/Acceptor.h | 60 lldb/tools/lldb-server/CMakeLists.txt | 1 - lldb/tools/lldb-server/lldb-gdbserver.cpp | 23 +- lldb/tools/lldb-server/lldb-platform.cpp | 335 -- .../Process/gdb-remote/CMakeLists.txt | 1 - .../Process/gdb-remote/PortMapTest.cpp| 115 -- .../tools/lldb-server/tests/LLGSTest.cpp | 4 - .../tools/lldb-server/tests/TestClient.cpp| 4 - .../tools/lldb-server/tests/TestClient.h | 4 + 20 files changed, 415 insertions(+), 721 deletions(-) delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test delete mode 100644 lldb/tools/lldb-server/Acceptor.cpp delete mode 100644 lldb/tools/lldb-server/Acceptor.h delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst index a67c00b305f6d2..31f5360df5e23e 100644 --- a/lldb/docs/man/lldb-server.rst +++ b/lldb/docs/man/lldb-server.rst @@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS .. option:: --gdbserver-port - Define a port to be used for gdb-server connections. Can be specified multiple - times to allow multiple ports. Has no effect if --min-gdbserver-port - and --max-gdbserver-port are specified. - -.. option:: --min-gdbserver-port -.. option:: --max-gdbserver-port - - Specify the range of ports that can be used for gdb-server connections. Both - options need to be specified simultaneously. Overrides --gdbserver-port. + Define a port to be used for gdb-server connections. This port is used for + multiple connections. .. option:: --port-offset diff --git a/lldb/docs/resources/qemu-testing.rst b/lldb/docs/resources/qemu-testing.rst index 51a30b11717a87..e102f84a1d31f4 100644 --- a/lldb/docs/resources/qemu-testing.rst +++ b/lldb/docs/resources/qemu-testing.rst @@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific options). * At least one to connect to the intial ``lldb-server``. * One more if you want to use ``lldb-server`` in ``platform mode``, and have it start a ``gdbserver`` instance for you. -* A bunch more if you want to run tests against the ``lldb-server`` platform. If you are doing either of the latter 2 you should also restrict what ports ``lldb-server tries`` to use, otherwise it will randomly pick one that is almost @@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown below. :: - $ lldb-server plaform --server --listen 0.0.0.0:54321 \ ---min-gdbserver-port 49140 --max-gdbserver-port 49150 + $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140 The result of this is that: * ``lldb-server`` platform mode listens externally on port ``54321``. -* When it is asked to start a new gdbserver mode instance, it will use a port - in the range ``49140`` to ``49150``. +* When it is asked to start a new gdbserver mode instance, it will use the port + ``49140``. -Your VM configuration should have ports ``54321``, and ``49140`` to ``49150`` -forwarded for this to work. - -.. note:: - These options are used to create a "port map" within ``lldb-server``. - Unfortunately this map is not cleaned up on Windows on connection close, - and across a few uses you may run out of valid ports. To work around this, - restart the platform every so often, especially after running a set of tests. - This is tracked here: https://github.com/llvm/llvm-project/issues/90923 +Your VM configuration should have ports ``54321`` and ``49140`` forwarded for +this to work. diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index 6a40f66be39b18..0c9672ea6aa753 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.c
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -150,12 +153,17 @@ static void
client_handle(GDBRemoteCommunicationServerPlatform &platform,
printf("Disconnected.\n");
}
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
- std::lock_guard guard(gdbserver_portmap_mutex);
- gdbserver_portmap.FreePortForProcess(pid);
+ if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+// If not running as a server and the platform connection is closed
+if (!g_terminate) {
+ g_terminate = true;
slydiman wrote:
Note this callback is called from an uncontrollable thread and any pending
arguments in the std::function wrapper may be invalid during the process
destroying procedure.
I have updated the logic around g_server and removed and g_single_pid
at all.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
https://github.com/slydiman deleted https://github.com/llvm/llvm-project/pull/104238 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/104238 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)
@@ -50,7 +50,8 @@ class ElaboratingDIEIterator
m_worklist.pop_back();
// And add back any items that elaborate it.
-for (dw_attr_t attr : {DW_AT_specification, DW_AT_abstract_origin}) {
+for (dw_attr_t attr :
+ {DW_AT_specification, DW_AT_abstract_origin, DW_AT_specification}) {
labath wrote:
Yes, I meant that to be DW_AT_signature. Oops.
This isn't really needed for this patch, but I thought it makes sense to add it
for symmetry. The class is only currently used in `DWARFDIE::IsMethod` which
does not make sense for types, but I could imagine this class being used
elsewhere as well.
https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)
@@ -377,11 +378,6 @@ static void GetDeclContextImpl(DWARFDIE die,
die = spec;
continue;
}
-// To find the name of a type in a type unit, we must follow the signature.
-if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_signature)) {
labath wrote:
Because die.GetName() will now find the correct name on its own. And (unlike
with DW_AT_specification) the correct context can be reconstructed by following
the parent chain in the original dwarf unit. For nested structs like
`Outer::Middle::Inner` we get something like this in the main dwarf unit:
```
0x0029: DW_TAG_structure_type
DW_AT_declaration (true)
DW_AT_signature (0xd351fbfc6a4cee7c) => points to Outer
0x0032: DW_TAG_structure_type
DW_AT_declaration (true)
DW_AT_signature (0x5f726fb54ccb6c95) => points to
Outer::Middle
0x003b: DW_TAG_structure_type
DW_AT_declaration (true)
DW_AT_signature (0xd3cee531644665ec) => points to
Outer::Middle::Inner
```
https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)
@@ -60,44 +60,45 @@ class DWARFDebugInfoEntry {
return attrs;
}
- dw_offset_t
- GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
-DWARFFormValue &formValue,
-dw_offset_t *end_attr_offset_ptr = nullptr,
-bool check_specification_or_abstract_origin = false) const;
+ dw_offset_t GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
+DWARFFormValue &formValue,
+dw_offset_t *end_attr_offset_ptr = nullptr,
+bool check_elaborating_dies = false) const;
- const char *GetAttributeValueAsString(
- const DWARFUnit *cu, const dw_attr_t attr, const char *fail_value,
- bool check_specification_or_abstract_origin = false) const;
+ const char *
+ GetAttributeValueAsString(const DWARFUnit *cu, const dw_attr_t attr,
+const char *fail_value,
+bool check_elaborating_dies = false) const;
- uint64_t GetAttributeValueAsUnsigned(
- const DWARFUnit *cu, const dw_attr_t attr, uint64_t fail_value,
- bool check_specification_or_abstract_origin = false) const;
+ uint64_t
+ GetAttributeValueAsUnsigned(const DWARFUnit *cu, const dw_attr_t attr,
+ uint64_t fail_value,
+ bool check_elaborating_dies = false) const;
labath wrote:
Most of the time, I'd say. DW_AT_specification says "this DIE is an out of line
definition of the function declared in ". It normally does not list the
name of the function because the name is already given on the other die, but
it'd perfectly reasonable for `GetName()` on the first die to return the value
from the other die. The same holds for most of the other attributes (with
DW_AT_declaration and DW_AT_sibling being the most notable exceptions). I think
this situation is similar enough to how DW_AT_signature works for types for it
to get the same treatment.
https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -421,97 +417,210 @@ int main_platform(int argc, char *argv[]) {
return 0;
}
- const bool children_inherit_listen_socket = false;
+ if (gdbserver_port != 0 &&
+ (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+ }
+
// the test suite makes many connections in parallel, let's not miss any.
// The highest this should get reasonably is a function of the number
// of target CPUs. For now, let's just use 100.
const int backlog = 100;
- std::unique_ptr acceptor_up(Acceptor::Create(
- listen_host_port, children_inherit_listen_socket, error));
- if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+ std::unique_ptr sock_named;
+ std::unique_ptr sock_platform;
+ std::unique_ptr sock_gdb;
+ std::string hostname;
+ uint16_t platform_port = 0;
+ Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
+ // Try to match socket name as URL - e.g., tcp://localhost:
+ if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+ fprintf(stderr, "Unknown protocol scheme \"%s\".",
+ uri->scheme.str().c_str());
+ return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+ hostname = uri->hostname;
+ if (uri->port) {
+platform_port = *(uri->port);
+ }
+} else
+ hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+ } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+ protocol = Socket::ProtocolTcp;
+ hostname = host_port->hostname;
+ platform_port = host_port->port;
+} else
+ hostname = listen_host_port;
}
- error = acceptor_up->Listen(backlog);
- if (error.Fail()) {
-printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+ std::string socket_id;
+ if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+ fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+ return socket_error;
+}
+
+sock_platform = std::make_unique(
+/*should_close=*/true, /*child_processes_inherit=*/false);
+error = sock_platform->Listen(
+llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen platform: %s\n", error.AsCString());
+ return socket_error;
+}
+if (platform_port == 0)
+ platform_port = sock_platform->GetLocalPortNumber();
+if (platform_port != 0)
+ socket_id = llvm::to_string(platform_port);
+
+sock_gdb = std::make_unique(/*should_close=*/true,
+ /*child_processes_inherit=*/false);
+error = sock_gdb->Listen(
+llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog);
+if (error.Fail()) {
+ printf("Failed to listen gdb: %s\n", error.AsCString());
+ return socket_error;
+}
+if (gdbserver_port == 0)
+ gdbserver_port = sock_gdb->GetLocalPortNumber();
+ } else {
+if (g_server) {
+ fprintf(stderr,
+ "Ambiguous parameters --server --listen %s\n"
+ "The protocol must be tcp for the server mode.",
+ listen_host_port.c_str());
+ return socket_error;
+}
+sock_named =
+Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+if (error.Fail()) {
+ fprintf(stderr, "Failed to create socket: %s", error.AsCString());
+ return socket_error;
+}
+error = sock_named->Listen(hostname, backlog);
+if (error.Fail()) {
+ printf("Failed to listen: %s\n", error.AsCString());
+ return socket_error;
+}
+socket_id = hostname;
}
+
if (socket_file) {
-error =
-save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file);
+error = save_socket_id_to_file(socket_id, socket_file);
if (error.Fail()) {
fprintf(stderr, "failed to write socket id to %s: %s\n",
socket_file.GetPath().c_str(), error.AsCString());
return 1;
}
}
- GDBRemoteCommunicationServerPlatform platform(
- acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme());
+ lldb_private::Args gdb_inferior_arguments(inferior_arguments);
+ GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
if (port_offset > 0)
platform.SetPortOffset(port_offset);
- do {
-const bool children_inherit_accept_socket = true;
-
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -160,66 +95,56 @@
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
- if (!port) {
-llvm::Expected available_port =
m_port_map.GetNextAvailablePort();
-if (available_port)
- port = *available_port;
-else
- return Status(available_port.takeError());
- }
-
- // Spawn a new thread to accept the port that gets bound after binding to
- // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+ std::ostringstream url;
+ if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+ // Just check that GDBServer exists. GDBServer must be launched after
+ // accepting the connection.
+ if (!GetDebugserverPath(nullptr))
+return Status::FromErrorString("unable to locate debugserver");
+ return Status();
+}
- // ignore the hostname send from the remote end, just use the ip address that
- // we're currently communicating with as the hostname
+// debugserver does not accept the URL scheme prefix.
+#if !defined(__APPLE__)
+url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
+#endif
+socket_name = GetDomainSocketPath("gdbserver").GetPath();
+url << socket_name;
+ } else {
+if (m_socket_protocol != Socket::ProtocolTcp)
+ return Status::FromErrorString("protocol must be tcp");
+ }
// Spawn a debugserver and try to get the port it listens to.
ProcessLaunchInfo debugserver_launch_info;
- if (hostname.empty())
-hostname = "127.0.0.1";
-
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-*port);
+ LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), fd);
// Do not run in a new session so that it can not linger after the platform
// closes.
debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
debugserver_launch_info.SetMonitorProcessCallback(
-
std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
-this, std::placeholders::_1));
-
- std::ostringstream url;
-// debugserver does not accept the URL scheme prefix.
-#if !defined(__APPLE__)
- url << m_socket_scheme << "://";
-#endif
- uint16_t *port_ptr = &*port;
- if (m_socket_protocol == Socket::ProtocolTcp) {
-std::string platform_uri = GetConnection()->GetURI();
-std::optional parsed_uri = URI::Parse(platform_uri);
-url << '[' << parsed_uri->hostname.str() << "]:" << *port;
- } else {
-socket_name = GetDomainSocketPath("gdbserver").GetPath();
-url << socket_name;
-port_ptr = nullptr;
- }
+ &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped);
Status error = StartDebugserverProcess(
- url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args,
-1);
+ url.str().c_str(), nullptr, debugserver_launch_info, nullptr, &args, fd);
- pid = debugserver_launch_info.GetProcessID();
- if (pid != LLDB_INVALID_PROCESS_ID) {
-std::lock_guard guard(m_spawned_pids_mutex);
-m_spawned_pids.insert(pid);
-if (*port > 0)
- m_port_map.AssociatePortWithProcess(*port, pid);
+ if (error.Success()) {
+pid = debugserver_launch_info.GetProcessID();
+if (pid != LLDB_INVALID_PROCESS_ID)
+ AddSpawnedProcess(pid);
+LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "debugserver launched successfully as pid %" PRIu64,
slydiman wrote:
Exactly.
A non-error Status mean the process was launched. But don't add
LLDB_INVALID_PROCESS_ID to the list.
We can add assert() here but I don't like it.
```
if (error.Success()) {
pid = debugserver_launch_info.GetProcessID();
if (pid != LLDB_INVALID_PROCESS_ID)
AddSpawnedProcess(pid);
LLDB_LOGF(log,
"GDBRemoteCommunicationServerPlatform::%s() "
"debugserver launched successfully as pid %" PRIu64,
__FUNCTION__, pid);
} else {
LLDB_LOGF(log,
"GDBRemoteCommunicationServerPlatform::%s() "
"debugserver launch failed: %s",
__FUNCTION__, error.AsCString());
}
```
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -150,12 +153,17 @@ static void
client_handle(GDBRemoteCommunicationServerPlatform &platform,
printf("Disconnected.\n");
}
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
- std::lock_guard guard(gdbserver_portmap_mutex);
- gdbserver_portmap.FreePortForProcess(pid);
+ if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+// If not running as a server and the platform connection is closed
+if (!g_terminate) {
+ g_terminate = true;
labath wrote:
I'm not particularly happy with the design of this callback, but that doesn't
mean it can't be used safely. The "uncontrollable" aspect means it is up to you
(its user) to ensure the callback does not access any arguments after they are
destroyed. It doesn't matter if they're local or global or how they're passed
into the callback. The new implementation is better (because it's easier to
reason about), but it still accesses the global main loop object, which will
get destroyed some time after `main` returns. So, the question here is "is
there anything ensuring that does not happen?" and not "is mainloop a global?".
If there is (for example, because the main thread waits for the callback to
call `RequestTermination()`), then I'm pretty this could be refactored to avoid
the global. If there isn't, then this is a problem even though the variable is
global.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] gdb rsp file error pass fix (PR #106950)
@@ -3064,22 +3064,41 @@ static int gdb_errno_to_system(int err) {
static uint64_t ParseHostIOPacketResponse(StringExtractorGDBRemote &response,
uint64_t fail_result, Status &error)
{
+ // The packet is expected to have the following format:
+ // 'F,'
+
response.SetFilePos(0);
if (response.GetChar() != 'F')
return fail_result;
+
int32_t result = response.GetS32(-2, 16);
if (result == -2)
return fail_result;
- if (response.GetChar() == ',') {
-int result_errno = gdb_errno_to_system(response.GetS32(-1, 16));
-if (result_errno != -1)
- error = Status(result_errno, eErrorTypePOSIX);
-else
- error = Status(-1, eErrorTypeGeneric);
- } else
+
+ if (response.GetChar() != ',') {
error.Clear();
+return result;
+ }
+
+ // Response packet should contain a error code at the end. This code
+ // corresponds either to the gdb IOFile error code, or to the posix errno.
dlav-sc wrote:
I've taken one more look at the documentation, and there are two options that
seem likely now:
1. According to the gdb rsp, error values can only be from the gdb internal
list
(https://sourceware.org/gdb/current/onlinedocs/gdb.html/Errno-Values.html#Errno-Values).
So, in the case of ETXTBSY, the response packet value cannot contain 26 as the
error code, and it must be instead, which corresponds to the unknown
error. In this case, the error is most likely on the lldb server side.
2. Lldb documentation (see lldb/docs/lldb-platform-packets.txt) states that
vFile:open, vFile:close, vFile:symlink, vFile:unlink, vFile:mode, and
vFile:size reply packets contain errno. I think there was an attempt to extend
gdb rsp so that a respond packet could contain any posix error, instead of the
limited set suggested by the rsp. I think this can be possible, because as far
as I know, the lldb server does not currently run on systems that are not
compatible with posix.
Please let me know what you think of this matter.
https://github.com/llvm/llvm-project/pull/106950
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -160,66 +95,56 @@
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
- if (!port) {
-llvm::Expected available_port =
m_port_map.GetNextAvailablePort();
-if (available_port)
- port = *available_port;
-else
- return Status(available_port.takeError());
- }
-
- // Spawn a new thread to accept the port that gets bound after binding to
- // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+ std::ostringstream url;
+ if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+ // Just check that GDBServer exists. GDBServer must be launched after
+ // accepting the connection.
+ if (!GetDebugserverPath(nullptr))
+return Status::FromErrorString("unable to locate debugserver");
+ return Status();
+}
- // ignore the hostname send from the remote end, just use the ip address that
- // we're currently communicating with as the hostname
+// debugserver does not accept the URL scheme prefix.
+#if !defined(__APPLE__)
+url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
+#endif
+socket_name = GetDomainSocketPath("gdbserver").GetPath();
+url << socket_name;
+ } else {
+if (m_socket_protocol != Socket::ProtocolTcp)
+ return Status::FromErrorString("protocol must be tcp");
+ }
// Spawn a debugserver and try to get the port it listens to.
ProcessLaunchInfo debugserver_launch_info;
- if (hostname.empty())
-hostname = "127.0.0.1";
-
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-*port);
+ LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), fd);
// Do not run in a new session so that it can not linger after the platform
// closes.
debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
debugserver_launch_info.SetMonitorProcessCallback(
-
std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
-this, std::placeholders::_1));
-
- std::ostringstream url;
-// debugserver does not accept the URL scheme prefix.
-#if !defined(__APPLE__)
- url << m_socket_scheme << "://";
-#endif
- uint16_t *port_ptr = &*port;
- if (m_socket_protocol == Socket::ProtocolTcp) {
-std::string platform_uri = GetConnection()->GetURI();
-std::optional parsed_uri = URI::Parse(platform_uri);
-url << '[' << parsed_uri->hostname.str() << "]:" << *port;
- } else {
-socket_name = GetDomainSocketPath("gdbserver").GetPath();
-url << socket_name;
-port_ptr = nullptr;
- }
+ &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped);
Status error = StartDebugserverProcess(
- url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args,
-1);
+ url.str().c_str(), nullptr, debugserver_launch_info, nullptr, &args, fd);
- pid = debugserver_launch_info.GetProcessID();
- if (pid != LLDB_INVALID_PROCESS_ID) {
-std::lock_guard guard(m_spawned_pids_mutex);
-m_spawned_pids.insert(pid);
-if (*port > 0)
- m_port_map.AssociatePortWithProcess(*port, pid);
+ if (error.Success()) {
+pid = debugserver_launch_info.GetProcessID();
+if (pid != LLDB_INVALID_PROCESS_ID)
+ AddSpawnedProcess(pid);
+LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "debugserver launched successfully as pid %" PRIu64,
labath wrote:
Why not? That's exactly what the assert is for. It says "it you say the process
was launched, you better be able to give me its PID"
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/104238 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/107241
>From 2fd6f97ed01eee17b28c1c843f0d891a460a2fb3 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Wed, 4 Sep 2024 13:36:07 +0200
Subject: [PATCH 1/2] [lldb] Recurse through DW_AT_signature when looking for
attributes
This allows e.g. DWARFDIE::GetName() to return the name of the type when
looking at its declaration (which contains only
DW_AT_declaration+DW_AT_signature). This is similar to how we recurse
through DW_AT_specification when looking for a function name. Llvm dwarf
parser has obtained the same functionality through #99495.
This fixes a bug where we would confuse a type like NS::Outer::Struct
with NS::Struct (because NS::Outer (and its name) was in a type unit).
---
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 18 +
.../SymbolFile/DWARF/DWARFDebugInfoEntry.cpp | 78 ---
.../SymbolFile/DWARF/DWARFDebugInfoEntry.h| 57 +++---
.../DWARF/x86/type-unit-same-basename.cpp | 45 +++
4 files changed, 110 insertions(+), 88 deletions(-)
create mode 100644
lldb/test/Shell/SymbolFile/DWARF/x86/type-unit-same-basename.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 0a13c457a307ae..e94b3198c63d90 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -50,7 +50,8 @@ class ElaboratingDIEIterator
m_worklist.pop_back();
// And add back any items that elaborate it.
-for (dw_attr_t attr : {DW_AT_specification, DW_AT_abstract_origin}) {
+for (dw_attr_t attr :
+ {DW_AT_specification, DW_AT_abstract_origin, DW_AT_specification}) {
if (DWARFDIE d = die.GetReferencedDIE(attr))
if (m_seen.insert(die.GetDIE()).second)
m_worklist.push_back(d);
@@ -129,10 +130,10 @@ DWARFDIE
DWARFDIE::GetAttributeValueAsReferenceDIE(const dw_attr_t attr) const {
if (IsValid()) {
DWARFUnit *cu = GetCU();
-const bool check_specification_or_abstract_origin = true;
+const bool check_elaborating_dies = true;
DWARFFormValue form_value;
if (m_die->GetAttributeValue(cu, attr, form_value, nullptr,
- check_specification_or_abstract_origin))
+ check_elaborating_dies))
return form_value.Reference();
}
return DWARFDIE();
@@ -377,11 +378,6 @@ static void GetDeclContextImpl(DWARFDIE die,
die = spec;
continue;
}
-// To find the name of a type in a type unit, we must follow the signature.
-if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_signature)) {
- die = spec;
- continue;
-}
// Add this DIE's contribution at the end of the chain.
auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
@@ -434,12 +430,6 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
std::vector &context) {
// Stop if we hit a cycle.
while (die && seen.insert(die.GetID()).second) {
-// To find the name of a type in a type unit, we must follow the signature.
-if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_signature)) {
- die = spec;
- continue;
-}
-
// Add this DIE's contribution at the end of the chain.
auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
context.push_back({kind, ConstString(name)});
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index e2660735ea7dec..77ef59d605c9c4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -355,8 +355,7 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
// would be a compile unit header).
dw_offset_t DWARFDebugInfoEntry::GetAttributeValue(
const DWARFUnit *cu, const dw_attr_t attr, DWARFFormValue &form_value,
-dw_offset_t *end_attr_offset_ptr,
-bool check_specification_or_abstract_origin) const {
+dw_offset_t *end_attr_offset_ptr, bool check_elaborating_dies) const {
if (const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu)) {
std::optional attr_idx = abbrevDecl->findAttributeIndex(attr);
@@ -380,25 +379,18 @@ dw_offset_t DWARFDebugInfoEntry::GetAttributeValue(
}
}
- if (check_specification_or_abstract_origin) {
-if (GetAttributeValue(cu, DW_AT_specification, form_value)) {
+ if (check_elaborating_dies) {
+for (dw_attr_t elaborating_attr :
+ {DW_AT_specification, DW_AT_abstract_origin, DW_AT_signature}) {
+ if (!GetAttributeValue(cu, elaborating_attr, form_value))
+continue;
DWARFDIE die = form_value.Reference();
- if (die) {
-dw_offset_t die_offset = die.GetDIE()->GetAttributeValue(
-die.GetCU(), attr, form_value, end_attr_offset_ptr, false);
-
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -25,90 +25,30 @@ namespace process_gdb_remote {
class GDBRemoteCommunicationServerPlatform
: public GDBRemoteCommunicationServerCommon {
public:
- class PortMap {
- public:
-// This class is used to restrict the range of ports that
-// platform created debugserver/gdbserver processes will
-// communicate on.
-
-// Construct an empty map, where empty means any port is allowed.
-PortMap() = default;
-
-// Make a port map with a range of free ports
-// from min_port to max_port-1.
-PortMap(uint16_t min_port, uint16_t max_port);
-
-// Add a port to the map. If it is already in the map do not modify
-// its mapping. (used ports remain used, new ports start as free)
-void AllowPort(uint16_t port);
-
-// If we are using a port map where we can only use certain ports,
-// get the next available port.
-//
-// If we are using a port map and we are out of ports, return an error.
-//
-// If we aren't using a port map, return 0 to indicate we should bind to
-// port 0 and then figure out which port we used.
-llvm::Expected GetNextAvailablePort();
-
-// Tie a port to a process ID. Returns false if the port is not in the port
-// map. If the port is already in use it will be moved to the given pid.
-// FIXME: This is and GetNextAvailablePort make create a race condition if
-// the portmap is shared between processes.
-bool AssociatePortWithProcess(uint16_t port, lldb::pid_t pid);
-
-// Free the given port. Returns false if the port is not in the map.
-bool FreePort(uint16_t port);
-
-// Free the port associated with the given pid. Returns false if there is
-// no port associated with the pid.
-bool FreePortForProcess(lldb::pid_t pid);
-
-// Returns true if there are no ports in the map, regardless of the state
-// of those ports. Meaning a map with 1 used port is not empty.
-bool empty() const;
-
- private:
-std::map m_port_map;
- };
-
GDBRemoteCommunicationServerPlatform(
- const Socket::SocketProtocol socket_protocol, const char *socket_scheme);
+ const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port);
~GDBRemoteCommunicationServerPlatform() override;
Status LaunchProcess() override;
- // Set both ports to zero to let the platform automatically bind to
- // a port chosen by the OS.
- void SetPortMap(PortMap &&port_map);
-
void SetPortOffset(uint16_t port_offset);
void SetInferiorArguments(const lldb_private::Args &args);
- // Set port if you want to use a specific port number.
- // Otherwise port will be set to the port that was chosen for you.
- Status LaunchGDBServer(const lldb_private::Args &args, std::string hostname,
- lldb::pid_t &pid, std::optional &port,
- std::string &socket_name);
+ Status LaunchGDBServer(const lldb_private::Args &args, lldb::pid_t &pid,
+ std::string &socket_name, shared_fd_t fd);
- void SetPendingGdbServer(lldb::pid_t pid, uint16_t port,
- const std::string &socket_name);
+ void SetPendingGdbServer(const std::string &socket_name);
protected:
const Socket::SocketProtocol m_socket_protocol;
- const std::string m_socket_scheme;
- std::recursive_mutex m_spawned_pids_mutex;
- std::set m_spawned_pids;
+ static std::mutex g_spawned_pids_mutex;
+ static std::set g_spawned_pids;
slydiman wrote:
There is no a safe way except static.
Note GDBRemoteCommunicationServerPlatform works in a separated single thread
process. I think it is ok.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -195,18 +195,31 @@ void ConnectToRemote(MainLoop &mainloop,
bool reverse_connect, llvm::StringRef host_and_port,
const char *const progname, const char *const subcommand,
const char *const named_pipe_path, pipe_t unnamed_pipe,
- int connection_fd) {
+ shared_fd_t connection_fd) {
Status error;
std::unique_ptr connection_up;
std::string url;
- if (connection_fd != -1) {
+ if (connection_fd != SharedSocket::kInvalidFD) {
+#ifdef _WIN32
+NativeSocket sockfd;
+error = SharedSocket::GetNativeSocket(connection_fd, sockfd);
+if (error.Fail()) {
+ llvm::errs() << llvm::formatv("error: GetNativeSocket failed: {0}\n",
+error.AsCString());
+ exit(-1);
+}
+connection_up =
+std::unique_ptr(new ConnectionFileDescriptor(new TCPSocket(
+sockfd, /*should_close=*/true,
/*child_processes_inherit=*/false)));
+#else
url = llvm::formatv("fd://{0}", connection_fd).str();
// Create the connection.
-#if LLDB_ENABLE_POSIX && !defined _WIN32
+#if LLDB_ENABLE_POSIX
slydiman wrote:
`!defined _WIN32` is redunded here because this block is inside the another
#ifdef now:
```
#ifdef _WIN32
...
#else
// no need to check !defined _WIN32 here
#endif
```
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/104238 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [compiler-rt] [libcxx] [lldb] [llvm] Rename Sanitizer Coverage => Coverage Sanitizer (PR #106505)
AaronBallman wrote:
Precommit CI test failures look related:
```
_bk;t=1725284334938 TEST 'LLVM ::
tools/sancov/symbolize.test' FAILED
_bk;t=1725284334938Exit Code: 1
_bk;t=1725284334938
_bk;t=1725284334938Command Output (stderr):
_bk;t=1725284334938--
_bk;t=1725284334938RUN: at line 2:
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/build/bin/sancov
-symbolize -strip_path_prefix="llvm/"
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/Inputs/test-linux_x86_64
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/Inputs/test-linux_x86_64.0.sancov
|
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/build/bin/FileCheck
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/symbolize.test
--check-prefixes=CHECK,STRIP
_bk;t=1725284334938+
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/build/bin/sancov
-symbolize -strip_path_prefix=llvm/
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/Inputs/test-linux_x86_64
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/Inputs/test-linux_x86_64.0.sancov
_bk;t=1725284334938+
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/build/bin/FileCheck
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/symbolize.test
--check-prefixes=CHECK,STRIP
_bk;t=1725284334938/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/symbolize.test:13:13:
error: CHECK-NEXT: expected string not found in input
_bk;t=1725284334938CHECK-NEXT: "binary-hash":
"BB3CDD5045AED83906F6ADCC1C4DAF7E2596A6B5",
_bk;t=1725284334938^
_bk;t=1725284334938:8:4: note: scanning from here
_bk;t=1725284334938 ],
_bk;t=1725284334938 ^
_bk;t=1725284334938:9:2: note: possible intended match here
_bk;t=1725284334938 "binary-hash": "A13A863C18DD4DA9E926DB113A369DA45AE33134",
_bk;t=1725284334938 ^
_bk;t=1725284334938
_bk;t=1725284334938Input file:
_bk;t=1725284334938Check file:
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/llvm/test/tools/sancov/symbolize.test
_bk;t=1725284334938
_bk;t=1725284334938-dump-input=help explains the following input dump.
_bk;t=1725284334938
_bk;t=1725284334938Input was:
_bk;t=1725284334938<<
_bk;t=1725284334938 1: {
_bk;t=1725284334938 2: "covered-points": [
_bk;t=1725284334938 3: "4e132b",
_bk;t=1725284334938 4: "4e1472",
_bk;t=1725284334938 5: "4e1520",
_bk;t=1725284334938 6: "4e1553",
_bk;t=1725284334938 7: "4e1586"
_bk;t=1725284334938 8: ],
_bk;t=1725284334938next:13'0X error: no match found
_bk;t=1725284334938 9: "binary-hash":
"A13A863C18DD4DA9E926DB113A369DA45AE33134",
_bk;t=1725284334938next:13'0
_bk;t=1725284334938next:13'1 ?
possible intended match
_bk;t=1725284334938 10: "point-symbol-info": {
_bk;t=1725284334938next:13'0
_bk;t=1725284334938 11: "test/tools/sancov/Inputs/test.cpp": {
_bk;t=1725284334938next:13'0
_bk;t=1725284334938 12: "bar(std::string)": {
_bk;t=1725284334938next:13'0 ~~~
_bk;t=1725284334938 13: "4e132b": "12:0"
_bk;t=1725284334938next:13'0 ~~
_bk;t=1725284334938 14: },
_bk;t=1725284334938next:13'0
_bk;t=1725284334938 .
_bk;t=1725284334938 .
_bk;t=1725284334938 .
_bk;t=1725284334938>>
_bk;t=1725284334938
_bk;t=1725284334938--
_bk;t=1725284334938
_bk;t=1725284334938
_bk;t=1725284334939FAIL: LLVM ::
tools/sancov/symbolize_noskip_dead_files.test (86063 of 96817)
_bk;t=1725284334939 TEST 'LLVM ::
tools/sancov/symbolize_noskip_dead_files.test' FAILED
_bk;t=1725284334939Exit Code: 1
_bk;t=1725284334939
_bk;t=1725284334939Command Output (stderr):
_bk;t=1725284334939--
_bk;t=1725284334939RUN: at line 2:
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-c4bpt-1/llvm-project/github-pull-requests/build/bin/sancov
-symbolize -skip-dead-fil
[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)
@@ -377,11 +378,6 @@ static void GetDeclContextImpl(DWARFDIE die,
die = spec;
continue;
}
-// To find the name of a type in a type unit, we must follow the signature.
-if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_signature)) {
Michael137 wrote:
Ahh right, makes sense.
So for `DW_AT_specification`s, the referring DIE doesn't have the parent chain
structure. So we *must* follow the DIE it referred to in order to reconstruct
the context. With `DW_AT_signature` the context structure is maintained, so
this now becomes redundant.
https://github.com/llvm/llvm-project/pull/107241
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)
https://github.com/Michael137 approved this pull request. LGTM Wouldn't block the PR on this, but is there a way the `ElaboratingDIIterator` changes can be tested? Maybe a unit-test where we instantiate `elaborating_dies` on something that has `DW_AT_signature` and makes sure we actually iterate over the original DWARF? https://github.com/llvm/llvm-project/pull/107241 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -150,12 +153,17 @@ static void
client_handle(GDBRemoteCommunicationServerPlatform &platform,
printf("Disconnected.\n");
}
-static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
-static std::mutex gdbserver_portmap_mutex;
-
static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {
- std::lock_guard guard(gdbserver_portmap_mutex);
- gdbserver_portmap.FreePortForProcess(pid);
+ if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) {
+// If not running as a server and the platform connection is closed
+if (!g_terminate) {
+ g_terminate = true;
slydiman wrote:
There are a lot of discussions around `Detached threads accessing global or
static objects` in the internet.
Detached threads are bad and the design of this callback is bad.
But if GDBRemoteCommunicationServerPlatform::g_spawned_pids is static (now) it
reduces the chances of a crash to 0.(0)1%.
We have no handle of this thread. So the static global g_main_loop is the
safest way and g_main_loop must be destroyed (almost) last. No 100% guarantee.
But it is the better solution w/o redesigning the child process monitoring.
Ultimately we can just remove `fixes #101475` in the description because this
patch improves the situation but do not fix it 100%.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Separated GDBRemoteCommunication::GetDebugserverPath() (PR #107388)
https://github.com/slydiman created
https://github.com/llvm/llvm-project/pull/107388
This is the prerequisite for #104238.
>From 9f1612513d5feffe7c0b5b5e5b743d932d233934 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Thu, 5 Sep 2024 16:04:44 +0400
Subject: [PATCH] [lldb][NFC] Separated
GDBRemoteCommunication::GetDebugserverPath()
This is the prerequisite for #104238.
---
.../gdb-remote/GDBRemoteCommunication.cpp | 28 +++
.../gdb-remote/GDBRemoteCommunication.h | 3 ++
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 50fa11e916c5f5..3ac58859cecafb 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -879,19 +879,11 @@ lldb::thread_result_t
GDBRemoteCommunication::ListenThread() {
return {};
}
-Status GDBRemoteCommunication::StartDebugserverProcess(
-const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
-uint16_t *port, const Args *inferior_args, int pass_comm_fd) {
+FileSpec GDBRemoteCommunication::GetDebugserverPath(Platform *platform) {
Log *log = GetLog(GDBRLog::Process);
- LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
-__FUNCTION__, url ? url : "", port ? *port : uint16_t(0));
-
- Status error;
// If we locate debugserver, keep that located version around
static FileSpec g_debugserver_file_spec;
-
- char debugserver_path[PATH_MAX];
- FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
+ FileSpec debugserver_file_spec;
Environment host_env = Host::GetEnvironment();
@@ -943,8 +935,20 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
}
}
}
+ return debugserver_file_spec;
+}
- if (debugserver_exists) {
+Status GDBRemoteCommunication::StartDebugserverProcess(
+const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
+uint16_t *port, const Args *inferior_args, shared_fd_t pass_comm_fd) {
+ Log *log = GetLog(GDBRLog::Process);
+ LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
+__FUNCTION__, url ? url : "", port ? *port : uint16_t(0));
+
+ Status error;
+ FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
+ if (debugserver_file_spec = GetDebugserverPath(platform)) {
+char debugserver_path[PATH_MAX];
debugserver_file_spec.GetPath(debugserver_path, sizeof(debugserver_path));
Args &debugserver_args = launch_info.GetArguments();
@@ -1059,6 +1063,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
}
}
}
+
+Environment host_env = Host::GetEnvironment();
std::string env_debugserver_log_file =
host_env.lookup("LLDB_DEBUGSERVER_LOG_FILE");
if (!env_debugserver_log_file.empty()) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 4e59bd5ec8be6b..a182291f6c8594 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -146,6 +146,9 @@ class GDBRemoteCommunication : public Communication {
std::chrono::seconds GetPacketTimeout() const { return m_packet_timeout; }
+ // Get the debugserver path and check that it exist.
+ FileSpec GetDebugserverPath(Platform *platform);
+
// Start a debugserver instance on the current host using the
// supplied connection URL.
Status StartDebugserverProcess(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Separated GDBRemoteCommunication::GetDebugserverPath() (PR #107388)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Dmitry Vasilyev (slydiman)
Changes
This is the prerequisite for #104238.
---
Full diff: https://github.com/llvm/llvm-project/pull/107388.diff
2 Files Affected:
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
(+17-11)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
(+3)
``diff
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 50fa11e916c5f5..3ac58859cecafb 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -879,19 +879,11 @@ lldb::thread_result_t
GDBRemoteCommunication::ListenThread() {
return {};
}
-Status GDBRemoteCommunication::StartDebugserverProcess(
-const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
-uint16_t *port, const Args *inferior_args, int pass_comm_fd) {
+FileSpec GDBRemoteCommunication::GetDebugserverPath(Platform *platform) {
Log *log = GetLog(GDBRLog::Process);
- LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
-__FUNCTION__, url ? url : "", port ? *port : uint16_t(0));
-
- Status error;
// If we locate debugserver, keep that located version around
static FileSpec g_debugserver_file_spec;
-
- char debugserver_path[PATH_MAX];
- FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
+ FileSpec debugserver_file_spec;
Environment host_env = Host::GetEnvironment();
@@ -943,8 +935,20 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
}
}
}
+ return debugserver_file_spec;
+}
- if (debugserver_exists) {
+Status GDBRemoteCommunication::StartDebugserverProcess(
+const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
+uint16_t *port, const Args *inferior_args, shared_fd_t pass_comm_fd) {
+ Log *log = GetLog(GDBRLog::Process);
+ LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
+__FUNCTION__, url ? url : "", port ? *port : uint16_t(0));
+
+ Status error;
+ FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
+ if (debugserver_file_spec = GetDebugserverPath(platform)) {
+char debugserver_path[PATH_MAX];
debugserver_file_spec.GetPath(debugserver_path, sizeof(debugserver_path));
Args &debugserver_args = launch_info.GetArguments();
@@ -1059,6 +1063,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
}
}
}
+
+Environment host_env = Host::GetEnvironment();
std::string env_debugserver_log_file =
host_env.lookup("LLDB_DEBUGSERVER_LOG_FILE");
if (!env_debugserver_log_file.empty()) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 4e59bd5ec8be6b..a182291f6c8594 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -146,6 +146,9 @@ class GDBRemoteCommunication : public Communication {
std::chrono::seconds GetPacketTimeout() const { return m_packet_timeout; }
+ // Get the debugserver path and check that it exist.
+ FileSpec GetDebugserverPath(Platform *platform);
+
// Start a debugserver instance on the current host using the
// supplied connection URL.
Status StartDebugserverProcess(
``
https://github.com/llvm/llvm-project/pull/107388
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
slydiman wrote: I have created #107388. Note I did not include `#include "lldb/Host/Socket.h"` and shared_fd_t using because it requires changes in lldb/unittests/tools/lldb-server/tests/TestClient.* https://github.com/llvm/llvm-project/pull/104238 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -160,66 +95,56 @@
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
- if (!port) {
-llvm::Expected available_port =
m_port_map.GetNextAvailablePort();
-if (available_port)
- port = *available_port;
-else
- return Status(available_port.takeError());
- }
-
- // Spawn a new thread to accept the port that gets bound after binding to
- // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+ std::ostringstream url;
+ if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+ // Just check that GDBServer exists. GDBServer must be launched after
+ // accepting the connection.
+ if (!GetDebugserverPath(nullptr))
+return Status::FromErrorString("unable to locate debugserver");
+ return Status();
slydiman wrote:
It is inconvenient. It can be called from
GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer()
and
lldb-platform.cpp, client_handle().
We can rename it to something like `CheckOrLaunchGDBServer()`.
BTW, Handle_qLaunchGDBServer() does not really launch the process now too in
case of TCP protocol.
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/104238 >From 18b8a835a3182e5be5f0dd848977333d9b8a26fa Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 30 Aug 2024 01:08:24 +0400 Subject: [PATCH 1/4] [lldb] Removed gdbserver ports map from lldb-server Listen to gdbserver-port, accept the connection and run lldb-server gdbserver --fd on all platforms. Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now. This is the part 2 of #101283. Fixes #97537, fixes #101475. --- lldb/docs/man/lldb-server.rst | 11 +- lldb/docs/resources/qemu-testing.rst | 19 +- .../posix/ConnectionFileDescriptorPosix.cpp | 21 +- .../gdb-remote/GDBRemoteCommunication.cpp | 45 ++- .../gdb-remote/GDBRemoteCommunication.h | 8 +- .../GDBRemoteCommunicationServerPlatform.cpp | 287 +-- .../GDBRemoteCommunicationServerPlatform.h| 82 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- .../TestPlatformLaunchGDBServer.py| 12 +- .../Shell/lldb-server/TestGdbserverPort.test | 4 - lldb/tools/lldb-server/Acceptor.cpp | 98 - lldb/tools/lldb-server/Acceptor.h | 60 lldb/tools/lldb-server/CMakeLists.txt | 1 - lldb/tools/lldb-server/lldb-gdbserver.cpp | 23 +- lldb/tools/lldb-server/lldb-platform.cpp | 335 -- .../Process/gdb-remote/CMakeLists.txt | 1 - .../Process/gdb-remote/PortMapTest.cpp| 115 -- .../tools/lldb-server/tests/LLGSTest.cpp | 4 - .../tools/lldb-server/tests/TestClient.cpp| 4 - .../tools/lldb-server/tests/TestClient.h | 4 + 20 files changed, 415 insertions(+), 721 deletions(-) delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test delete mode 100644 lldb/tools/lldb-server/Acceptor.cpp delete mode 100644 lldb/tools/lldb-server/Acceptor.h delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst index a67c00b305f6d2..31f5360df5e23e 100644 --- a/lldb/docs/man/lldb-server.rst +++ b/lldb/docs/man/lldb-server.rst @@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS .. option:: --gdbserver-port - Define a port to be used for gdb-server connections. Can be specified multiple - times to allow multiple ports. Has no effect if --min-gdbserver-port - and --max-gdbserver-port are specified. - -.. option:: --min-gdbserver-port -.. option:: --max-gdbserver-port - - Specify the range of ports that can be used for gdb-server connections. Both - options need to be specified simultaneously. Overrides --gdbserver-port. + Define a port to be used for gdb-server connections. This port is used for + multiple connections. .. option:: --port-offset diff --git a/lldb/docs/resources/qemu-testing.rst b/lldb/docs/resources/qemu-testing.rst index 51a30b11717a87..e102f84a1d31f4 100644 --- a/lldb/docs/resources/qemu-testing.rst +++ b/lldb/docs/resources/qemu-testing.rst @@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific options). * At least one to connect to the intial ``lldb-server``. * One more if you want to use ``lldb-server`` in ``platform mode``, and have it start a ``gdbserver`` instance for you. -* A bunch more if you want to run tests against the ``lldb-server`` platform. If you are doing either of the latter 2 you should also restrict what ports ``lldb-server tries`` to use, otherwise it will randomly pick one that is almost @@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown below. :: - $ lldb-server plaform --server --listen 0.0.0.0:54321 \ ---min-gdbserver-port 49140 --max-gdbserver-port 49150 + $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140 The result of this is that: * ``lldb-server`` platform mode listens externally on port ``54321``. -* When it is asked to start a new gdbserver mode instance, it will use a port - in the range ``49140`` to ``49150``. +* When it is asked to start a new gdbserver mode instance, it will use the port + ``49140``. -Your VM configuration should have ports ``54321``, and ``49140`` to ``49150`` -forwarded for this to work. - -.. note:: - These options are used to create a "port map" within ``lldb-server``. - Unfortunately this map is not cleaned up on Windows on connection close, - and across a few uses you may run out of valid ports. To work around this, - restart the platform every so often, especially after running a set of tests. - This is tracked here: https://github.com/llvm/llvm-project/issues/90923 +Your VM configuration should have ports ``54321`` and ``49140`` forwarded for +this to work. diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index 6a40f66be39b18..0c9672ea6aa753 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.c
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
@@ -160,66 +95,56 @@
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-std::optional &port, std::string &socket_name) {
- if (!port) {
-llvm::Expected available_port =
m_port_map.GetNextAvailablePort();
-if (available_port)
- port = *available_port;
-else
- return Status(available_port.takeError());
- }
-
- // Spawn a new thread to accept the port that gets bound after binding to
- // port 0 (zero).
+const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+shared_fd_t fd) {
+ std::ostringstream url;
+ if (fd == SharedSocket::kInvalidFD) {
+if (m_socket_protocol == Socket::ProtocolTcp) {
+ // Just check that GDBServer exists. GDBServer must be launched after
+ // accepting the connection.
+ if (!GetDebugserverPath(nullptr))
+return Status::FromErrorString("unable to locate debugserver");
+ return Status();
+}
- // ignore the hostname send from the remote end, just use the ip address that
- // we're currently communicating with as the hostname
+// debugserver does not accept the URL scheme prefix.
+#if !defined(__APPLE__)
+url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
+#endif
+socket_name = GetDomainSocketPath("gdbserver").GetPath();
+url << socket_name;
+ } else {
+if (m_socket_protocol != Socket::ProtocolTcp)
+ return Status::FromErrorString("protocol must be tcp");
+ }
// Spawn a debugserver and try to get the port it listens to.
ProcessLaunchInfo debugserver_launch_info;
- if (hostname.empty())
-hostname = "127.0.0.1";
-
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-*port);
+ LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), fd);
// Do not run in a new session so that it can not linger after the platform
// closes.
debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
debugserver_launch_info.SetMonitorProcessCallback(
-
std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
-this, std::placeholders::_1));
-
- std::ostringstream url;
-// debugserver does not accept the URL scheme prefix.
-#if !defined(__APPLE__)
- url << m_socket_scheme << "://";
-#endif
- uint16_t *port_ptr = &*port;
- if (m_socket_protocol == Socket::ProtocolTcp) {
-std::string platform_uri = GetConnection()->GetURI();
-std::optional parsed_uri = URI::Parse(platform_uri);
-url << '[' << parsed_uri->hostname.str() << "]:" << *port;
- } else {
-socket_name = GetDomainSocketPath("gdbserver").GetPath();
-url << socket_name;
-port_ptr = nullptr;
- }
+ &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped);
Status error = StartDebugserverProcess(
- url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args,
-1);
+ url.str().c_str(), nullptr, debugserver_launch_info, nullptr, &args, fd);
- pid = debugserver_launch_info.GetProcessID();
- if (pid != LLDB_INVALID_PROCESS_ID) {
-std::lock_guard guard(m_spawned_pids_mutex);
-m_spawned_pids.insert(pid);
-if (*port > 0)
- m_port_map.AssociatePortWithProcess(*port, pid);
+ if (error.Success()) {
+pid = debugserver_launch_info.GetProcessID();
+if (pid != LLDB_INVALID_PROCESS_ID)
+ AddSpawnedProcess(pid);
+LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "debugserver launched successfully as pid %" PRIu64,
slydiman wrote:
Added assert() to AddSpawnedProcess().
https://github.com/llvm/llvm-project/pull/104238
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/104238 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Finish implementing support for DW_FORM_data16 (PR #106799)
aainscow wrote: Vote. This is affecting the Ceph project. https://github.com/llvm/llvm-project/pull/106799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] gdb rsp file error pass fix (PR #106950)
@@ -3064,22 +3064,41 @@ static int gdb_errno_to_system(int err) {
static uint64_t ParseHostIOPacketResponse(StringExtractorGDBRemote &response,
uint64_t fail_result, Status &error)
{
+ // The packet is expected to have the following format:
+ // 'F,'
+
response.SetFilePos(0);
if (response.GetChar() != 'F')
return fail_result;
+
int32_t result = response.GetS32(-2, 16);
if (result == -2)
return fail_result;
- if (response.GetChar() == ',') {
-int result_errno = gdb_errno_to_system(response.GetS32(-1, 16));
-if (result_errno != -1)
- error = Status(result_errno, eErrorTypePOSIX);
-else
- error = Status(-1, eErrorTypeGeneric);
- } else
+
+ if (response.GetChar() != ',') {
error.Clear();
+return result;
+ }
+
+ // Response packet should contain a error code at the end. This code
+ // corresponds either to the gdb IOFile error code, or to the posix errno.
DavidSpickett wrote:
Yes it is a bit vague but going by
https://lldb.llvm.org/resources/lldbgdbremote.html#vfile-open "the errno"
doesn't say to me "the posix errno" necessarily. You are right that for
`lldb-server` specifically, it would defacto mean the posix errno, but there is
also Windows to consider.
But the code you are editing here is the client side (`lldb`). The client could
be connected to a debug server running on anything.
So I don't think you can mark the unknown values `eErrorTypePOSIX` because at
this point we don't know that the other end is posix. So that's why I'd go with
`eErrorTypeGeneric`.
(I suspect this doesn't really matter in the end, we probably just print the
error number the same way either way)
The point of your patch is to include the number instead of overwriting it with
-1, and that is fine I understand why that's useful.
https://github.com/llvm/llvm-project/pull/106950
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [mlir] [APInt] Assert correct values in APInt constructor (PR #80309)
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/80309
>From f6002ad249359131be6d668036b4f17ff43e67c7 Mon Sep 17 00:00:00 2001
From: Nikita Popov
Date: Tue, 13 Aug 2024 15:11:18 +0200
Subject: [PATCH] apint only
---
clang/lib/AST/ByteCode/IntegralAP.h | 6 +-
clang/lib/CodeGen/CGVTT.cpp | 4 +-
clang/lib/CodeGen/ItaniumCXXABI.cpp | 4 +-
clang/lib/Parse/ParseInit.cpp | 4 +-
clang/lib/Sema/SemaExpr.cpp | 7 +-
clang/lib/Sema/SemaOpenMP.cpp | 4 +-
lldb/source/Expression/DWARFExpression.cpp| 7 +-
llvm/include/llvm/ADT/APFixedPoint.h | 4 +-
llvm/include/llvm/ADT/APInt.h | 2 +-
llvm/lib/Analysis/ConstantFolding.cpp | 3 +-
llvm/lib/Analysis/Loads.cpp | 6 +-
llvm/lib/Analysis/MemoryBuiltins.cpp | 2 +
llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 3 +-
.../lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 5 +-
.../SelectionDAG/SelectionDAGBuilder.cpp | 3 +-
.../CodeGen/SelectionDAG/SelectionDAGISel.cpp | 4 +-
.../CodeGen/SelectionDAG/TargetLowering.cpp | 8 +-
llvm/lib/ExecutionEngine/MCJIT/MCJIT.cpp | 2 +-
llvm/lib/IR/Constants.cpp | 4 +-
.../Target/AArch64/AArch64ISelLowering.cpp| 32 ++--
llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.cpp | 2 +-
.../AMDGPU/AMDGPUInstructionSelector.cpp | 2 +-
.../Disassembler/AMDGPUDisassembler.cpp | 2 +-
.../MCTargetDesc/AMDGPUMCTargetDesc.cpp | 2 +-
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp| 13 +-
.../Target/AMDGPU/SIShrinkInstructions.cpp| 4 +-
.../lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 4 +-
.../Hexagon/HexagonConstPropagation.cpp | 3 +-
llvm/lib/Target/Hexagon/HexagonGenExtract.cpp | 2 +-
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 4 +-
llvm/lib/Target/X86/X86ISelLowering.cpp | 6 +-
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 3 +-
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 2 +-
llvm/unittests/ADT/APFixedPointTest.cpp | 9 +-
llvm/unittests/ADT/APIntTest.cpp | 180 +-
llvm/unittests/ADT/APSIntTest.cpp | 8 +-
llvm/unittests/ADT/StringExtrasTest.cpp | 10 +-
.../Analysis/ScalarEvolutionTest.cpp | 2 +-
llvm/unittests/IR/MetadataTest.cpp| 22 ++-
.../Support/DivisionByConstantTest.cpp| 2 +-
mlir/include/mlir/IR/BuiltinAttributes.td | 4 +-
mlir/include/mlir/IR/OpImplementation.h | 3 +-
.../Conversion/TosaToArith/TosaToArith.cpp| 2 +-
.../Dialect/ControlFlow/IR/ControlFlowOps.cpp | 2 +-
mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp| 2 +-
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 2 +-
mlir/lib/IR/Builders.cpp | 16 +-
.../SPIRV/Deserialization/Deserializer.cpp| 6 +-
.../Dialect/SPIRV/SerializationTest.cpp | 2 +-
49 files changed, 245 insertions(+), 190 deletions(-)
diff --git a/clang/lib/AST/ByteCode/IntegralAP.h
b/clang/lib/AST/ByteCode/IntegralAP.h
index a4d656433344b7..6ab3d09ec85d5b 100644
--- a/clang/lib/AST/ByteCode/IntegralAP.h
+++ b/clang/lib/AST/ByteCode/IntegralAP.h
@@ -61,7 +61,7 @@ template class IntegralAP final {
IntegralAP(APInt V) : V(V) {}
/// Arbitrary value for uninitialized variables.
- IntegralAP() : IntegralAP(-1, 3) {}
+ IntegralAP() : IntegralAP(Signed ? -1 : 7, 3) {}
IntegralAP operator-() const { return IntegralAP(-V); }
IntegralAP operator-(const IntegralAP &Other) const {
@@ -112,7 +112,9 @@ template class IntegralAP final {
template
static IntegralAP from(Integral I, unsigned BitWidth) {
-APInt Copy = APInt(BitWidth, static_cast(I), InputSigned);
+// TODO: Avoid implicit trunc?
+APInt Copy = APInt(BitWidth, static_cast(I), InputSigned,
+ /*implicitTrunc=*/true);
return IntegralAP(Copy);
}
diff --git a/clang/lib/CodeGen/CGVTT.cpp b/clang/lib/CodeGen/CGVTT.cpp
index 20bd2c2fc2c642..c861e1f9203058 100644
--- a/clang/lib/CodeGen/CGVTT.cpp
+++ b/clang/lib/CodeGen/CGVTT.cpp
@@ -85,8 +85,8 @@ CodeGenVTables::EmitVTTDefinition(llvm::GlobalVariable *VTT,
cast(VTable->getValueType())
->getElementType(AddressPoint.VTableIndex));
unsigned Offset = ComponentSize * AddressPoint.AddressPointIndex;
- llvm::ConstantRange InRange(llvm::APInt(32, -Offset, true),
- llvm::APInt(32, VTableSize - Offset, true));
+ llvm::ConstantRange InRange(llvm::APInt(32, -Offset),
+ llvm::APInt(32, VTableSize - Offset));
llvm::Constant *Init = llvm::ConstantExpr::getGetElementPtr(
VTable->getValueType(), VTable, Idxs, /*InBounds=*/true, InRange);
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 0cde8a192eda08..bfab28613f9807 100644
---
[Lldb-commits] [lldb] 2ed510d - [LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base and gs_base (#106767)
Author: Jacob Lalonde
Date: 2024-09-05T09:38:45-07:00
New Revision: 2ed510dc9789ca0b9172f0593527bee9d53496c4
URL:
https://github.com/llvm/llvm-project/commit/2ed510dc9789ca0b9172f0593527bee9d53496c4
DIFF:
https://github.com/llvm/llvm-project/commit/2ed510dc9789ca0b9172f0593527bee9d53496c4.diff
LOG: [LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base
and gs_base (#106767)
A follow up to #106473 Minidump wasn't collecting fs or gs_base. This
patch extends the x86_64 register context and gated reading it behind an
lldb specific flag. Additionally these registers are explicitly checked
in the tests.
Added:
Modified:
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
Removed:
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 13355afb58dbd1..5c9ba223ad143e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -473,7 +473,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
lldb_private::minidump::MinidumpContext_x86_64_Flags::x86_64_Flag |
lldb_private::minidump::MinidumpContext_x86_64_Flags::Control |
lldb_private::minidump::MinidumpContext_x86_64_Flags::Segments |
- lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer);
+ lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer |
+ lldb_private::minidump::MinidumpContext_x86_64_Flags::LLDBSpecific);
thread_context.rax = read_register_u64(reg_ctx, "rax");
thread_context.rbx = read_register_u64(reg_ctx, "rbx");
thread_context.rcx = read_register_u64(reg_ctx, "rcx");
@@ -499,6 +500,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
thread_context.gs = read_register_u64(reg_ctx, "gs");
thread_context.ss = read_register_u64(reg_ctx, "ss");
thread_context.ds = read_register_u64(reg_ctx, "ds");
+ thread_context.fs_base = read_register_u64(reg_ctx, "fs_base");
+ thread_context.gs_base = read_register_u64(reg_ctx, "gs_base");
return thread_context;
}
diff --git
a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
index 917140cab29767..e879c493156593 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
@@ -67,6 +67,7 @@ lldb::DataBufferSP
lldb_private::minidump::ConvertMinidumpContext_x86_64(
auto ControlFlag = MinidumpContext_x86_64_Flags::Control;
auto IntegerFlag = MinidumpContext_x86_64_Flags::Integer;
auto SegmentsFlag = MinidumpContext_x86_64_Flags::Segments;
+ auto LLDBSpecificFlag = MinidumpContext_x86_64_Flags::LLDBSpecific;
if ((context_flags & x86_64_Flag) != x86_64_Flag)
return nullptr;
@@ -104,6 +105,13 @@ lldb::DataBufferSP
lldb_private::minidump::ConvertMinidumpContext_x86_64(
writeRegister(&context->r15, result_base, reg_info[lldb_r15_x86_64]);
}
+ if ((context_flags & LLDBSpecificFlag) == LLDBSpecificFlag) {
+writeRegister(&context->fs_base, result_base,
+ reg_info[x86_64_with_base::lldb_fs_base]);
+writeRegister(&context->gs_base, result_base,
+ reg_info[x86_64_with_base::lldb_gs_base]);
+ }
+
// TODO parse the floating point registers
return result_context_buf;
diff --git
a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
index d920ea9d823f4f..f8d38e4ba53786 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -153,6 +153,11 @@ struct MinidumpContext_x86_64 {
llvm::support::ulittle64_t last_branch_from_rip;
llvm::support::ulittle64_t last_exception_to_rip;
llvm::support::ulittle64_t last_exception_from_rip;
+
+ // LLDB can save core files and save extra information that isn't available
+ // from Google breakpad, or similar, minidump files.
+ llvm::support::ulittle64_t fs_base;
+ llvm::support::ulittle64_t gs_base;
};
// For context_flags. These values indicate the type of
@@ -168,9 +173,10 @@ enum class MinidumpContext_x86_64_Flags : uint32_t {
FloatingPoint = x86_64_Flag | 0x0008,
DebugRegisters = x86_64_Flag | 0x0010,
XState = x86_64_Flag | 0x0040,
+ LLDBSpecific = x86_64_Flag | 0x8000,
Full = Control | Integer | FloatingPoint,
- All = Full | Segments | D
[Lldb-commits] [lldb] [LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base and gs_base (PR #106767)
https://github.com/Jlalond closed https://github.com/llvm/llvm-project/pull/106767 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 18ad98e - [lldb] Fix a format string in ClangASTSource (#107325)
Author: Alex Langford
Date: 2024-09-05T09:53:49-07:00
New Revision: 18ad98e7947502da0c8f6dcbbf485bb34fe8d204
URL:
https://github.com/llvm/llvm-project/commit/18ad98e7947502da0c8f6dcbbf485bb34fe8d204
DIFF:
https://github.com/llvm/llvm-project/commit/18ad98e7947502da0c8f6dcbbf485bb34fe8d204.diff
LOG: [lldb] Fix a format string in ClangASTSource (#107325)
Without this, LLDB asserts when enabling the expression logs.
Added:
Modified:
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
Removed:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 1fdd272dcbeceb..e41efdd3f61c75 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -293,7 +293,7 @@ void ClangASTSource::CompleteType(clang::ObjCInterfaceDecl
*interface_decl) {
LLDB_LOG(log,
"[CompleteObjCInterfaceDecl] on (ASTContext*){0:x} '{1}' "
- "Completing an ObjCInterfaceDecl named {1}",
+ "Completing an ObjCInterfaceDecl named {2}",
m_ast_context, m_clang_ast_context->getDisplayName(),
interface_decl->getName());
LLDB_LOG(log, " [COID] Before:\n{0}",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix a format string in ClangASTSource (PR #107325)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/107325 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Finish implementing support for DW_FORM_data16 (PR #106799)
https://github.com/JDevlieghere approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/106799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Finish implementing support for DW_FORM_data16 (PR #106799)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/106799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Minidump] Support multiple exceptions in a minidump (PR #107319)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/107319
>From 70f94e152ba2a6826a3c4f8071b5e0ee2bb81d2d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Wed, 4 Sep 2024 14:06:04 -0700
Subject: [PATCH 1/7] Cherry-pick llvm only changes from minidump multiple
exceptions PR.
---
llvm/include/llvm/Object/Minidump.h | 39 +++-
llvm/lib/Object/Minidump.cpp | 31 ++
llvm/lib/ObjectYAML/MinidumpYAML.cpp | 2 +-
3 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/Object/Minidump.h
b/llvm/include/llvm/Object/Minidump.h
index 65ad6b171c2dc1..eb6f4a2714340a 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -83,15 +83,25 @@ class MinidumpFile : public Binary {
return getListStream(minidump::StreamType::ThreadList);
}
- /// Returns the contents of the Exception stream. An error is returned if
the
- /// file does not contain this stream, or the stream is smaller than the size
- /// of the ExceptionStream structure. The internal consistency of the stream
- /// is not checked in any way.
- Expected getExceptionStream() const {
-return getStream(
-minidump::StreamType::Exception);
+ /// Returns the contents of the Exception stream. An error is returned if the
+ /// associated stream is smaller than the size of the ExceptionStream
+ /// structure. Or the directory supplied is not of kind exception stream.
+ Expected
+ getExceptionStream(minidump::Directory Directory) const {
+if (Directory.Type != minidump::StreamType::Exception) {
+ return createError("Not an exception stream");
+}
+
+return getStreamFromDirectory(Directory);
}
+ /// Returns the contents of the Exception streams. An error is returned if
+ /// any of the streams are smaller than the size of the ExceptionStream
+ /// structure. The internal consistency of the stream is not checked in any
+ /// way.
+ Expected>
+ getExceptionStreams() const;
+
/// Returns the list of descriptors embedded in the MemoryList stream. The
/// descriptors provide the content of interesting regions of memory at the
/// time the minidump was taken. An error is returned if the file does not
@@ -264,6 +274,12 @@ class MinidumpFile : public Binary {
return arrayRefFromStringRef(Data.getBuffer());
}
+ /// Return the stream of the given type, cast to the appropriate type. Checks
+ /// that the stream is large enough to hold an object of this type.
+ template
+ Expected
+ getStreamFromDirectory(minidump::Directory Directory) const;
+
/// Return the stream of the given type, cast to the appropriate type. Checks
/// that the stream is large enough to hold an object of this type.
template
@@ -279,6 +295,15 @@ class MinidumpFile : public Binary {
DenseMap StreamMap;
};
+template
+Expected
+MinidumpFile::getStreamFromDirectory(minidump::Directory Directory) const {
+ ArrayRef Stream = getRawStream(Directory);
+ if (Stream.size() >= sizeof(T))
+return *reinterpret_cast(Stream.data());
+ return createEOFError();
+}
+
template
Expected MinidumpFile::getStream(minidump::StreamType Type) const {
if (std::optional> Stream = getRawStream(Type)) {
diff --git a/llvm/lib/Object/Minidump.cpp b/llvm/lib/Object/Minidump.cpp
index 93b2e2cecd1053..cd78f1b10ca17a 100644
--- a/llvm/lib/Object/Minidump.cpp
+++ b/llvm/lib/Object/Minidump.cpp
@@ -53,6 +53,27 @@ Expected MinidumpFile::getString(size_t Offset)
const {
return Result;
}
+Expected>
+MinidumpFile::getExceptionStreams() const {
+
+ std::vector exceptionStreamList;
+ for (const auto &directory : Streams) {
+if (directory.Type == StreamType::Exception) {
+ llvm::Expected ExpectedStream =
+ getStreamFromDirectory(directory);
+ if (!ExpectedStream)
+return ExpectedStream.takeError();
+
+ exceptionStreamList.push_back(ExpectedStream.get());
+}
+ }
+
+ if (exceptionStreamList.empty())
+return createError("No exception streams found");
+
+ return exceptionStreamList;
+}
+
Expected>
MinidumpFile::getMemoryInfoList() const {
std::optional> Stream =
@@ -128,6 +149,7 @@ MinidumpFile::create(MemoryBufferRef Source) {
return ExpectedStreams.takeError();
DenseMap StreamMap;
+ std::vector ExceptionStreams;
for (const auto &StreamDescriptor : llvm::enumerate(*ExpectedStreams)) {
StreamType Type = StreamDescriptor.value().Type;
const LocationDescriptor &Loc = StreamDescriptor.value().Location;
@@ -143,6 +165,15 @@ MinidumpFile::create(MemoryBufferRef Source) {
continue;
}
+// We treat exceptions differently here because the LLDB minidump
+// makes some assumptions about uniqueness, all the streams other than
+// exceptions are lists. But exceptions are not a list, they are single
+// streams that point back to their thread So we will omit them here, and
+// will find them when
[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
@@ -258,6 +258,14 @@ class TypeSummaryImpl {
virtual std::string GetDescription() = 0;
+ /// Get the name of the Type Summary Provider, either a C++ class, a summary
+ /// string, or a script function name.
+ virtual std::string GetName() = 0;
+
+ /// Get the name of the kind of Summary Provider, either c++, summary string,
+ /// script or python.
+ virtual std::string GetSummaryKindName() = 0;
Jlalond wrote:
I'm not opposed to the enum, but I think if we're using an enum we should
leverage 'TypeSummaryKind`, the existing kind enum and we should to string that.
My question is how we handle script, currently I check if it's a `.py` and if
it is we call it Python. Do we want to default to Python for all scripts?
@Michael137 I talked to Greg offline about this, but do we support any SWIG
interfaced languages other than Python? I believe I've seen Lua in the wild. If
that's the case I'd like to keep the differentiation, but for this first patch
we could get away with just `Python`
https://github.com/llvm/llvm-project/pull/102708
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/107163
>From a56b0ef1e5c914a7a513f643de608f1c29aa21ef Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Tue, 3 Sep 2024 15:29:25 -0700
Subject: [PATCH] [lldb] Make conversions from llvm::Error explicit with
Status::FromError() [NFC]
---
lldb/include/lldb/Utility/Status.h| 7 +-
lldb/source/API/SBDebugger.cpp| 2 +-
lldb/source/API/SBTarget.cpp | 2 +-
.../Commands/CommandObjectBreakpoint.cpp | 103 ++
.../Commands/CommandObjectMemoryTag.cpp | 10 +-
lldb/source/Commands/CommandObjectStats.cpp | 6 +-
lldb/source/Commands/CommandObjectTrace.cpp | 2 +-
lldb/source/Core/PluginManager.cpp| 2 +-
lldb/source/Core/ThreadedCommunication.cpp| 2 +-
lldb/source/Core/ValueObjectVTable.cpp| 2 +-
lldb/source/Core/ValueObjectVariable.cpp | 2 +-
lldb/source/DataFormatters/VectorType.cpp | 2 +-
lldb/source/Host/common/FileCache.cpp | 2 +-
.../Host/common/NativeProcessProtocol.cpp | 2 +-
lldb/source/Host/common/TCPSocket.cpp | 6 +-
lldb/source/Host/macosx/objcxx/Host.mm| 2 +-
.../posix/ConnectionFileDescriptorPosix.cpp | 6 +-
lldb/source/Interpreter/CommandObject.cpp | 2 +-
lldb/source/Interpreter/OptionValueRegex.cpp | 2 +-
.../Language/CPlusPlus/BlockPointer.cpp | 2 +-
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +-
.../Minidump/ObjectFileMinidump.cpp | 2 +-
...latformiOSSimulatorCoreSimulatorSupport.mm | 2 +-
.../Process/Linux/NativeProcessLinux.cpp | 16 +--
.../NativeRegisterContextDBReg_arm64.cpp | 10 +-
.../Process/elf-core/ProcessElfCore.cpp | 2 +-
.../gdb-remote/GDBRemoteCommunication.cpp | 2 +-
.../GDBRemoteCommunicationClient.cpp | 2 +-
.../GDBRemoteCommunicationServer.cpp | 2 +-
.../GDBRemoteCommunicationServerLLGS.cpp | 8 +-
.../GDBRemoteCommunicationServerPlatform.cpp | 2 +-
.../Process/minidump/ProcessMinidump.cpp | 2 +-
.../Interfaces/ScriptedPythonInterface.h | 2 +-
.../Python/PythonDataObjects.cpp | 22 ++--
.../Python/ScriptInterpreterPython.cpp| 10 +-
.../DarwinLog/StructuredDataDarwinLog.cpp | 2 +-
lldb/source/Target/ModuleCache.cpp| 2 +-
lldb/source/Target/Platform.cpp | 2 +-
lldb/source/Target/Process.cpp| 4 +-
lldb/source/Target/StackFrame.cpp | 2 +-
lldb/source/Target/Thread.cpp | 3 +-
lldb/source/Utility/Scalar.cpp| 2 +-
lldb/source/Utility/Status.cpp| 8 +-
lldb/source/Utility/StructuredData.cpp| 2 +-
.../Host/NativeProcessTestUtils.h | 4 +-
lldb/unittests/Utility/StatusTest.cpp | 14 ++-
46 files changed, 157 insertions(+), 140 deletions(-)
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index b304291ffae00e..3813a3c1604700 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -91,9 +91,9 @@ class Status {
~Status();
- // llvm::Error support
- explicit Status(llvm::Error error) { *this = std::move(error); }
- const Status &operator=(llvm::Error error);
+ /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
+ static Status FromError(llvm::Error error);
+ /// FIXME: Replace this with a takeError method.
llvm::Error ToError() const;
/// Get the error string associated with the current error.
@@ -145,6 +145,7 @@ class Status {
bool Success() const;
protected:
+ Status(llvm::Error error);
/// Status code as an integer value.
ValueType m_code = 0;
/// The type of the above error code.
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 72501570320d57..c226acc15018ef 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -220,7 +220,7 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() {
SBError error;
if (auto e = g_debugger_lifetime->Initialize(
std::make_unique(), LoadPlugin)) {
-error.SetError(Status(std::move(e)));
+error.SetError(Status::FromError(std::move(e)));
}
return error;
}
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index e927cb854cd88c..41eb77e5506bc5 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1658,7 +1658,7 @@ SBError SBTarget::SetLabel(const char *label) {
if (!target_sp)
return Status::FromErrorString("Couldn't get internal target object.");
- return Status(target_sp->SetLabel(label));
+ return Status::FromError(target_sp->SetLabel(label));
}
uint32_t SBTarget::GetDataByteSize() {
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index ed
[Lldb-commits] [lldb] a0dd90e - [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (#107163)
Author: Adrian Prantl
Date: 2024-09-05T12:19:31-07:00
New Revision: a0dd90eb7dc318c9b3fccb9ba02e1e22fb073094
URL:
https://github.com/llvm/llvm-project/commit/a0dd90eb7dc318c9b3fccb9ba02e1e22fb073094
DIFF:
https://github.com/llvm/llvm-project/commit/a0dd90eb7dc318c9b3fccb9ba02e1e22fb073094.diff
LOG: [lldb] Make conversions from llvm::Error explicit with Status::FromEr…
(#107163)
…ror() [NFC]
Added:
Modified:
lldb/include/lldb/Utility/Status.h
lldb/source/API/SBDebugger.cpp
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectMemoryTag.cpp
lldb/source/Commands/CommandObjectStats.cpp
lldb/source/Commands/CommandObjectTrace.cpp
lldb/source/Core/PluginManager.cpp
lldb/source/Core/ThreadedCommunication.cpp
lldb/source/Core/ValueObjectVTable.cpp
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/DataFormatters/VectorType.cpp
lldb/source/Host/common/FileCache.cpp
lldb/source/Host/common/NativeProcessProtocol.cpp
lldb/source/Host/common/TCPSocket.cpp
lldb/source/Host/macosx/objcxx/Host.mm
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Interpreter/OptionValueRegex.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
lldb/source/Target/ModuleCache.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/Thread.cpp
lldb/source/Utility/Scalar.cpp
lldb/source/Utility/Status.cpp
lldb/source/Utility/StructuredData.cpp
lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
lldb/unittests/Utility/StatusTest.cpp
Removed:
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index b304291ffae00e..3813a3c1604700 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -91,9 +91,9 @@ class Status {
~Status();
- // llvm::Error support
- explicit Status(llvm::Error error) { *this = std::move(error); }
- const Status &operator=(llvm::Error error);
+ /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
+ static Status FromError(llvm::Error error);
+ /// FIXME: Replace this with a takeError method.
llvm::Error ToError() const;
/// Get the error string associated with the current error.
@@ -145,6 +145,7 @@ class Status {
bool Success() const;
protected:
+ Status(llvm::Error error);
/// Status code as an integer value.
ValueType m_code = 0;
/// The type of the above error code.
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 72501570320d57..c226acc15018ef 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -220,7 +220,7 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() {
SBError error;
if (auto e = g_debugger_lifetime->Initialize(
std::make_unique(), LoadPlugin)) {
-error.SetError(Status(std::move(e)));
+error.SetError(Status::FromError(std::move(e)));
}
return error;
}
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index e927cb854cd88c..41eb77e5506bc5 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1658,7 +1658,7 @@ SBError SBTarget::SetLabel(const char *label) {
if (!target_sp)
return Status::FromErrorString("Couldn't get internal target object.");
- return Status(target_sp->SetLabel(label));
+ return Status::FromError(target_sp->SetLabel(label));
}
uint32_t SBTarget::GetDataByteSize() {
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/107163 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
@@ -109,6 +109,13 @@ llvm::Error Status::ToError() const {
Status::~Status() = default;
+const Status &Status::operator=(Status &&other) {
+ m_code = other.m_code;
+ m_type = other.m_type;
+ m_string = other.m_string;
adrian-prantl wrote:
Sure, but this is going to be deleted in the next commit anyway.
https://github.com/llvm/llvm-project/pull/107170
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/107170
>From 6500c9a1c2cf2812332dd66e35e625f1bc233b89 Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Tue, 3 Sep 2024 16:40:41 -0700
Subject: [PATCH] [lldb] Make deep copies of Status explicit (NFC)
---
lldb/bindings/python/python-swigsafecast.swig | 2 +-
lldb/include/lldb/API/SBError.h | 4 +-
lldb/include/lldb/API/SBValueList.h | 2 +-
.../lldb/Core/ValueObjectConstResult.h| 4 +-
lldb/include/lldb/Target/Process.h| 2 -
lldb/include/lldb/Utility/Status.h| 29 -
lldb/source/API/SBBreakpoint.cpp | 6 +--
lldb/source/API/SBBreakpointLocation.cpp | 4 +-
lldb/source/API/SBBreakpointName.cpp | 17
lldb/source/API/SBDebugger.cpp| 2 +-
lldb/source/API/SBError.cpp | 15 ---
lldb/source/API/SBFile.cpp| 15 +++
lldb/source/API/SBFormat.cpp | 2 +-
lldb/source/API/SBFrame.cpp | 9 ++--
lldb/source/API/SBPlatform.cpp| 4 +-
lldb/source/API/SBProcess.cpp | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 3 +-
lldb/source/API/SBStructuredData.cpp | 2 +-
lldb/source/API/SBTarget.cpp | 5 ++-
lldb/source/API/SBThread.cpp | 2 +-
lldb/source/API/SBValue.cpp | 4 +-
lldb/source/API/SBValueList.cpp | 13 +++---
lldb/source/API/SBWatchpoint.cpp | 2 +-
.../source/Commands/CommandObjectCommands.cpp | 4 +-
lldb/source/Core/Debugger.cpp | 2 +-
lldb/source/Core/ModuleList.cpp | 5 +--
lldb/source/Core/ValueObject.cpp | 4 +-
lldb/source/Core/ValueObjectCast.cpp | 2 +-
lldb/source/Core/ValueObjectConstResult.cpp | 9 ++--
lldb/source/Core/ValueObjectDynamicValue.cpp | 2 +-
.../Core/ValueObjectSyntheticFilter.cpp | 2 +-
lldb/source/Expression/FunctionCaller.cpp | 3 +-
lldb/source/Expression/LLVMUserExpression.cpp | 6 +--
lldb/source/Expression/Materializer.cpp | 2 +-
lldb/source/Expression/UserExpression.cpp | 8 ++--
lldb/source/Host/common/LockFileBase.cpp | 4 +-
.../Host/common/NativeProcessProtocol.cpp | 8 ++--
.../posix/ConnectionFileDescriptorPosix.cpp | 12 +++---
.../source/Interpreter/CommandInterpreter.cpp | 2 +-
lldb/source/Interpreter/ScriptInterpreter.cpp | 2 +-
.../Plugins/Platform/Android/AdbClient.cpp| 8 ++--
.../PlatformAndroidRemoteGDBServer.cpp| 4 +-
...PlatformiOSSimulatorCoreSimulatorSupport.h | 2 +-
...latformiOSSimulatorCoreSimulatorSupport.mm | 12 +++---
.../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 4 +-
.../Platform/Windows/PlatformWindows.cpp | 4 +-
.../ScriptedProcessPythonInterface.cpp| 6 +--
.../Interfaces/ScriptedPythonInterface.h | 8 +++-
.../Plugins/SymbolFile/DWARF/DWARFUnit.h | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +-
.../DWARF/SymbolFileDWARFDebugMap.cpp | 2 +-
lldb/source/Target/ModuleCache.cpp| 2 +-
lldb/source/Target/Platform.cpp | 4 +-
lldb/source/Target/Process.cpp| 41 +--
lldb/source/Target/StackFrame.cpp | 2 +-
lldb/source/Target/Target.cpp | 12 +++---
lldb/source/Utility/Status.cpp| 33 +++
.../Target/RemoteAwarePlatformTest.cpp| 9 ++--
58 files changed, 202 insertions(+), 182 deletions(-)
diff --git a/lldb/bindings/python/python-swigsafecast.swig
b/lldb/bindings/python/python-swigsafecast.swig
index 34f8c6f0ff8d35..bffd10ae7b3315 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -34,7 +34,7 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP
breakpoint_sp) {
}
PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
- return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
+ return ToSWIGHelper(new lldb::SBError(status.Clone()),
SWIGTYPE_p_lldb__SBError);
}
PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr
data_sb) {
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 17f2c6c3027af7..9f55f92131c06e 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -97,7 +97,7 @@ class LLDB_API SBError {
friend class lldb_private::ScriptInterpreter;
friend class lldb_private::python::SWIGBridge;
- SBError(const lldb_private::Status &error);
+ SBError(lldb_private::Status &&error);
lldb_private::Status *get();
@@ -107,7 +107,7 @@ class LLDB_API SBError {
lldb_private::Status &ref();
- void SetError(const lldb_private::Status &lldb_error);
+ void SetError(lldb_private::Status &&lldb_error);
private:
std::unique_ptr m_opaque_up;
diff --gi
[Lldb-commits] [lldb] 5515b08 - Factor Process::ExecutionResultAsCString() into a global function (NFC)
Author: Adrian Prantl
Date: 2024-09-05T12:36:05-07:00
New Revision: 5515b086f35c2c1402234b9b94338f10fc9d16f7
URL:
https://github.com/llvm/llvm-project/commit/5515b086f35c2c1402234b9b94338f10fc9d16f7
DIFF:
https://github.com/llvm/llvm-project/commit/5515b086f35c2c1402234b9b94338f10fc9d16f7.diff
LOG: Factor Process::ExecutionResultAsCString() into a global function (NFC)
Added:
Modified:
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Utility/Status.h
lldb/source/Expression/FunctionCaller.cpp
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Target/Process.cpp
lldb/source/Utility/Status.cpp
Removed:
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index a7de991104434d..c66cfb2c245efd 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1296,8 +1296,6 @@ class Process : public
std::enable_shared_from_this,
const EvaluateExpressionOptions &options,
DiagnosticManager &diagnostic_manager);
- static const char *ExecutionResultAsCString(lldb::ExpressionResults result);
-
void GetStatus(Stream &ostrm);
size_t GetThreadStatus(Stream &ostrm, bool only_threads_with_stop_reason,
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index 3813a3c1604700..308532e7c633dc 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -26,6 +26,8 @@ class raw_ostream;
namespace lldb_private {
+const char *ExpressionResultAsCString(lldb::ExpressionResults result);
+
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
///
/// This class is designed to be able to hold any error code that can be
diff --git a/lldb/source/Expression/FunctionCaller.cpp
b/lldb/source/Expression/FunctionCaller.cpp
index 5ac2b0681ebbec..5ce0175fedf453 100644
--- a/lldb/source/Expression/FunctionCaller.cpp
+++ b/lldb/source/Expression/FunctionCaller.cpp
@@ -390,8 +390,7 @@ lldb::ExpressionResults FunctionCaller::ExecuteFunction(
LLDB_LOGF(log,
"== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "
"completed abnormally: %s ==",
-m_name.c_str(),
-Process::ExecutionResultAsCString(return_value));
+m_name.c_str(), ExpressionResultAsCString(return_value));
} else {
LLDB_LOGF(log,
"== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "
diff --git a/lldb/source/Expression/LLVMUserExpression.cpp
b/lldb/source/Expression/LLVMUserExpression.cpp
index b4fdfc4d1fa8ba..9e07d4a7374a78 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -235,9 +235,9 @@ LLVMUserExpression::DoExecute(DiagnosticManager
&diagnostic_manager,
expr_thread_id);
return execution_result;
} else if (execution_result != lldb::eExpressionCompleted) {
- diagnostic_manager.Printf(
- lldb::eSeverityError, "Couldn't execute function; result was %s",
- Process::ExecutionResultAsCString(execution_result));
+ diagnostic_manager.Printf(lldb::eSeverityError,
+"Couldn't execute function; result was %s",
+ExpressionResultAsCString(execution_result));
return execution_result;
}
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 3d7ddbe294a49c..f2a631a466b35d 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5743,43 +5743,6 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx,
return return_value;
}
-const char *Process::ExecutionResultAsCString(ExpressionResults result) {
- const char *result_name = "";
-
- switch (result) {
- case eExpressionCompleted:
-result_name = "eExpressionCompleted";
-break;
- case eExpressionDiscarded:
-result_name = "eExpressionDiscarded";
-break;
- case eExpressionInterrupted:
-result_name = "eExpressionInterrupted";
-break;
- case eExpressionHitBreakpoint:
-result_name = "eExpressionHitBreakpoint";
-break;
- case eExpressionSetupError:
-result_name = "eExpressionSetupError";
-break;
- case eExpressionParseError:
-result_name = "eExpressionParseError";
-break;
- case eExpressionResultUnavailable:
-result_name = "eExpressionResultUnavailable";
-break;
- case eExpressionTimedOut:
-result_name = "eExpressionTimedOut";
-break;
- case eExpressionStoppedForDebug:
-result_name = "eExpressionStoppedForDebug";
-break;
- case eExpressionThreadVanished:
-result_name = "eExpressionThreadVanished";
- }
- return result_name;
-}
-
void Process::GetStatus(Stream &strm) {
const StateType state = GetState();
if (StateIsStoppedState(state, false)) {
dif
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/107170
>From e0a98558ed5c543e4d65c2c560e15062d82f150a Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Tue, 3 Sep 2024 16:40:41 -0700
Subject: [PATCH] [lldb] Make deep copies of Status explicit (NFC)
---
lldb/bindings/python/python-swigsafecast.swig | 2 +-
lldb/include/lldb/API/SBError.h | 4 +--
lldb/include/lldb/API/SBValueList.h | 2 +-
.../lldb/Core/ValueObjectConstResult.h| 4 +--
lldb/include/lldb/Utility/Status.h| 27 +--
lldb/source/API/SBBreakpoint.cpp | 6 ++---
lldb/source/API/SBBreakpointLocation.cpp | 4 +--
lldb/source/API/SBBreakpointName.cpp | 17 ++--
lldb/source/API/SBDebugger.cpp| 2 +-
lldb/source/API/SBError.cpp | 15 ++-
lldb/source/API/SBFile.cpp| 15 ---
lldb/source/API/SBFormat.cpp | 2 +-
lldb/source/API/SBFrame.cpp | 9 ---
lldb/source/API/SBPlatform.cpp| 4 +--
lldb/source/API/SBProcess.cpp | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 3 +--
lldb/source/API/SBStructuredData.cpp | 2 +-
lldb/source/API/SBTarget.cpp | 5 ++--
lldb/source/API/SBThread.cpp | 2 +-
lldb/source/API/SBValue.cpp | 4 +--
lldb/source/API/SBValueList.cpp | 13 -
lldb/source/API/SBWatchpoint.cpp | 2 +-
.../source/Commands/CommandObjectCommands.cpp | 4 +--
lldb/source/Core/Debugger.cpp | 2 +-
lldb/source/Core/ModuleList.cpp | 5 ++--
lldb/source/Core/ValueObject.cpp | 4 +--
lldb/source/Core/ValueObjectCast.cpp | 2 +-
lldb/source/Core/ValueObjectConstResult.cpp | 9 ---
lldb/source/Core/ValueObjectDynamicValue.cpp | 2 +-
.../Core/ValueObjectSyntheticFilter.cpp | 2 +-
lldb/source/Expression/Materializer.cpp | 2 +-
lldb/source/Expression/UserExpression.cpp | 8 +++---
lldb/source/Host/common/LockFileBase.cpp | 4 +--
.../Host/common/NativeProcessProtocol.cpp | 8 +++---
.../posix/ConnectionFileDescriptorPosix.cpp | 12 -
.../source/Interpreter/CommandInterpreter.cpp | 2 +-
lldb/source/Interpreter/ScriptInterpreter.cpp | 2 +-
.../Plugins/Platform/Android/AdbClient.cpp| 8 +++---
.../PlatformAndroidRemoteGDBServer.cpp| 4 +--
...PlatformiOSSimulatorCoreSimulatorSupport.h | 2 +-
...latformiOSSimulatorCoreSimulatorSupport.mm | 12 -
.../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 4 +--
.../Platform/Windows/PlatformWindows.cpp | 4 +--
.../ScriptedProcessPythonInterface.cpp| 6 ++---
.../Interfaces/ScriptedPythonInterface.h | 8 --
.../Plugins/SymbolFile/DWARF/DWARFUnit.h | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +-
.../DWARF/SymbolFileDWARFDebugMap.cpp | 2 +-
lldb/source/Target/ModuleCache.cpp| 2 +-
lldb/source/Target/Platform.cpp | 4 +--
lldb/source/Target/Process.cpp| 4 +--
lldb/source/Target/StackFrame.cpp | 2 +-
lldb/source/Target/Target.cpp | 12 -
lldb/source/Utility/Status.cpp| 7 +
.../Target/RemoteAwarePlatformTest.cpp| 9 ---
55 files changed, 170 insertions(+), 138 deletions(-)
diff --git a/lldb/bindings/python/python-swigsafecast.swig
b/lldb/bindings/python/python-swigsafecast.swig
index 34f8c6f0ff8d35..bffd10ae7b3315 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -34,7 +34,7 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP
breakpoint_sp) {
}
PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
- return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
+ return ToSWIGHelper(new lldb::SBError(status.Clone()),
SWIGTYPE_p_lldb__SBError);
}
PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr
data_sb) {
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 17f2c6c3027af7..9f55f92131c06e 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -97,7 +97,7 @@ class LLDB_API SBError {
friend class lldb_private::ScriptInterpreter;
friend class lldb_private::python::SWIGBridge;
- SBError(const lldb_private::Status &error);
+ SBError(lldb_private::Status &&error);
lldb_private::Status *get();
@@ -107,7 +107,7 @@ class LLDB_API SBError {
lldb_private::Status &ref();
- void SetError(const lldb_private::Status &lldb_error);
+ void SetError(lldb_private::Status &&lldb_error);
private:
std::unique_ptr m_opaque_up;
diff --git a/lldb/include/lldb/API/SBValueList.h
b/lldb/include/lldb/API/SBValueList.h
index a5017bccc50533..52a86f989e153a 100644
--
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/107170
>From 8cdcc708ce0cd07445c36781cc40db04a6d94b60 Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Tue, 3 Sep 2024 16:40:41 -0700
Subject: [PATCH] [lldb] Make deep copies of Status explicit (NFC)
---
lldb/bindings/python/python-swigsafecast.swig | 2 +-
lldb/include/lldb/API/SBError.h | 4 +--
lldb/include/lldb/API/SBValueList.h | 2 +-
.../lldb/Core/ValueObjectConstResult.h| 4 +--
lldb/include/lldb/Utility/Status.h| 27 +--
lldb/source/API/SBBreakpoint.cpp | 6 ++---
lldb/source/API/SBBreakpointLocation.cpp | 4 +--
lldb/source/API/SBBreakpointName.cpp | 17 ++--
lldb/source/API/SBDebugger.cpp| 2 +-
lldb/source/API/SBError.cpp | 15 ++-
lldb/source/API/SBFile.cpp| 15 ---
lldb/source/API/SBFormat.cpp | 2 +-
lldb/source/API/SBFrame.cpp | 9 ---
lldb/source/API/SBPlatform.cpp| 4 +--
lldb/source/API/SBProcess.cpp | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 3 +--
lldb/source/API/SBStructuredData.cpp | 2 +-
lldb/source/API/SBTarget.cpp | 5 ++--
lldb/source/API/SBThread.cpp | 2 +-
lldb/source/API/SBValue.cpp | 4 +--
lldb/source/API/SBValueList.cpp | 13 -
lldb/source/API/SBWatchpoint.cpp | 2 +-
.../source/Commands/CommandObjectCommands.cpp | 4 +--
lldb/source/Core/Debugger.cpp | 2 +-
lldb/source/Core/ModuleList.cpp | 5 ++--
lldb/source/Core/ValueObject.cpp | 4 +--
lldb/source/Core/ValueObjectCast.cpp | 2 +-
lldb/source/Core/ValueObjectConstResult.cpp | 9 ---
lldb/source/Core/ValueObjectDynamicValue.cpp | 2 +-
.../Core/ValueObjectSyntheticFilter.cpp | 2 +-
lldb/source/Expression/Materializer.cpp | 2 +-
lldb/source/Expression/UserExpression.cpp | 8 +++---
lldb/source/Host/common/LockFileBase.cpp | 4 +--
.../Host/common/NativeProcessProtocol.cpp | 8 +++---
.../posix/ConnectionFileDescriptorPosix.cpp | 12 -
.../source/Interpreter/CommandInterpreter.cpp | 2 +-
lldb/source/Interpreter/ScriptInterpreter.cpp | 2 +-
.../Plugins/Platform/Android/AdbClient.cpp| 8 +++---
.../PlatformAndroidRemoteGDBServer.cpp| 4 +--
...PlatformiOSSimulatorCoreSimulatorSupport.h | 2 +-
...latformiOSSimulatorCoreSimulatorSupport.mm | 12 -
.../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 4 +--
.../Platform/Windows/PlatformWindows.cpp | 4 +--
.../ScriptedProcessPythonInterface.cpp| 6 ++---
.../Interfaces/ScriptedPythonInterface.h | 8 --
.../Plugins/SymbolFile/DWARF/DWARFUnit.h | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +-
.../DWARF/SymbolFileDWARFDebugMap.cpp | 2 +-
lldb/source/Target/ModuleCache.cpp| 2 +-
lldb/source/Target/Platform.cpp | 4 +--
lldb/source/Target/Process.cpp| 4 +--
lldb/source/Target/StackFrame.cpp | 2 +-
lldb/source/Target/Target.cpp | 12 -
lldb/source/Utility/Status.cpp| 7 +
.../Target/RemoteAwarePlatformTest.cpp| 8 +++---
55 files changed, 169 insertions(+), 138 deletions(-)
diff --git a/lldb/bindings/python/python-swigsafecast.swig
b/lldb/bindings/python/python-swigsafecast.swig
index 34f8c6f0ff8d35..bffd10ae7b3315 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -34,7 +34,7 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP
breakpoint_sp) {
}
PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
- return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
+ return ToSWIGHelper(new lldb::SBError(status.Clone()),
SWIGTYPE_p_lldb__SBError);
}
PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr
data_sb) {
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 17f2c6c3027af7..9f55f92131c06e 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -97,7 +97,7 @@ class LLDB_API SBError {
friend class lldb_private::ScriptInterpreter;
friend class lldb_private::python::SWIGBridge;
- SBError(const lldb_private::Status &error);
+ SBError(lldb_private::Status &&error);
lldb_private::Status *get();
@@ -107,7 +107,7 @@ class LLDB_API SBError {
lldb_private::Status &ref();
- void SetError(const lldb_private::Status &lldb_error);
+ void SetError(lldb_private::Status &&lldb_error);
private:
std::unique_ptr m_opaque_up;
diff --git a/lldb/include/lldb/API/SBValueList.h
b/lldb/include/lldb/API/SBValueList.h
index a5017bccc50533..52a86f989e153a 100644
---
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/107170
>From 287735cb1a5f6ceeb55aa6fbef8b86c99583d727 Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Tue, 3 Sep 2024 16:40:41 -0700
Subject: [PATCH] [lldb] Make deep copies of Status explicit (NFC)
---
lldb/bindings/python/python-swigsafecast.swig | 4 +--
lldb/include/lldb/API/SBError.h | 4 +--
lldb/include/lldb/API/SBValueList.h | 2 +-
.../lldb/Core/ValueObjectConstResult.h| 4 +--
lldb/include/lldb/Utility/Status.h| 27 +--
lldb/source/API/SBBreakpoint.cpp | 6 ++---
lldb/source/API/SBBreakpointLocation.cpp | 4 +--
lldb/source/API/SBBreakpointName.cpp | 17 ++--
lldb/source/API/SBDebugger.cpp| 2 +-
lldb/source/API/SBError.cpp | 15 ++-
lldb/source/API/SBFile.cpp| 15 ---
lldb/source/API/SBFormat.cpp | 2 +-
lldb/source/API/SBFrame.cpp | 9 ---
lldb/source/API/SBPlatform.cpp| 4 +--
lldb/source/API/SBProcess.cpp | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 3 +--
lldb/source/API/SBStructuredData.cpp | 2 +-
lldb/source/API/SBTarget.cpp | 5 ++--
lldb/source/API/SBThread.cpp | 2 +-
lldb/source/API/SBValue.cpp | 4 +--
lldb/source/API/SBValueList.cpp | 13 -
lldb/source/API/SBWatchpoint.cpp | 2 +-
.../source/Commands/CommandObjectCommands.cpp | 4 +--
lldb/source/Core/Debugger.cpp | 2 +-
lldb/source/Core/ModuleList.cpp | 5 ++--
lldb/source/Core/ValueObject.cpp | 4 +--
lldb/source/Core/ValueObjectCast.cpp | 2 +-
lldb/source/Core/ValueObjectConstResult.cpp | 9 ---
lldb/source/Core/ValueObjectDynamicValue.cpp | 2 +-
.../Core/ValueObjectSyntheticFilter.cpp | 2 +-
lldb/source/Expression/Materializer.cpp | 2 +-
lldb/source/Expression/UserExpression.cpp | 8 +++---
lldb/source/Host/common/LockFileBase.cpp | 4 +--
.../Host/common/NativeProcessProtocol.cpp | 8 +++---
.../posix/ConnectionFileDescriptorPosix.cpp | 12 -
.../source/Interpreter/CommandInterpreter.cpp | 2 +-
lldb/source/Interpreter/ScriptInterpreter.cpp | 2 +-
.../Plugins/Platform/Android/AdbClient.cpp| 8 +++---
.../PlatformAndroidRemoteGDBServer.cpp| 4 +--
...PlatformiOSSimulatorCoreSimulatorSupport.h | 2 +-
...latformiOSSimulatorCoreSimulatorSupport.mm | 12 -
.../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 4 +--
.../Platform/Windows/PlatformWindows.cpp | 4 +--
.../ScriptedProcessPythonInterface.cpp| 6 ++---
.../Interfaces/ScriptedPythonInterface.h | 8 --
.../Python/SWIGPythonBridge.h | 2 +-
.../Plugins/SymbolFile/DWARF/DWARFUnit.h | 2 +-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +-
.../DWARF/SymbolFileDWARFDebugMap.cpp | 2 +-
lldb/source/Target/ModuleCache.cpp| 2 +-
lldb/source/Target/Platform.cpp | 4 +--
lldb/source/Target/Process.cpp| 4 +--
lldb/source/Target/StackFrame.cpp | 2 +-
lldb/source/Target/Target.cpp | 12 -
lldb/source/Utility/Status.cpp| 7 +
.../Python/PythonTestSuite.cpp| 2 +-
.../Target/RemoteAwarePlatformTest.cpp| 8 +++---
57 files changed, 172 insertions(+), 141 deletions(-)
diff --git a/lldb/bindings/python/python-swigsafecast.swig
b/lldb/bindings/python/python-swigsafecast.swig
index 34f8c6f0ff8d35..0127ac6bfa4681 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -33,8 +33,8 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP
breakpoint_sp) {
SWIGTYPE_p_lldb__SBBreakpoint);
}
-PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
- return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
+PythonObject SWIGBridge::ToSWIGWrapper(Status status) {
+ return ToSWIGHelper(new lldb::SBError(std::move(status)),
SWIGTYPE_p_lldb__SBError);
}
PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr
data_sb) {
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 17f2c6c3027af7..9f55f92131c06e 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -97,7 +97,7 @@ class LLDB_API SBError {
friend class lldb_private::ScriptInterpreter;
friend class lldb_private::python::SWIGBridge;
- SBError(const lldb_private::Status &error);
+ SBError(lldb_private::Status &&error);
lldb_private::Status *get();
@@ -107,7 +107,7 @@ class LLDB_API SBError {
lldb_private::Status &ref();
- void SetError(const lldb_private::Status &lldb_error);
+ voi
[Lldb-commits] [lldb] b798f4b - [lldb] Make deep copies of Status explicit (NFC) (#107170)
Author: Adrian Prantl
Date: 2024-09-05T12:44:13-07:00
New Revision: b798f4bd50bbf0f5eb46804afad10629797c73aa
URL:
https://github.com/llvm/llvm-project/commit/b798f4bd50bbf0f5eb46804afad10629797c73aa
DIFF:
https://github.com/llvm/llvm-project/commit/b798f4bd50bbf0f5eb46804afad10629797c73aa.diff
LOG: [lldb] Make deep copies of Status explicit (NFC) (#107170)
Added:
Modified:
lldb/bindings/python/python-swigsafecast.swig
lldb/include/lldb/API/SBError.h
lldb/include/lldb/API/SBValueList.h
lldb/include/lldb/Core/ValueObjectConstResult.h
lldb/include/lldb/Utility/Status.h
lldb/source/API/SBBreakpoint.cpp
lldb/source/API/SBBreakpointLocation.cpp
lldb/source/API/SBBreakpointName.cpp
lldb/source/API/SBDebugger.cpp
lldb/source/API/SBError.cpp
lldb/source/API/SBFile.cpp
lldb/source/API/SBFormat.cpp
lldb/source/API/SBFrame.cpp
lldb/source/API/SBPlatform.cpp
lldb/source/API/SBProcess.cpp
lldb/source/API/SBSaveCoreOptions.cpp
lldb/source/API/SBStructuredData.cpp
lldb/source/API/SBTarget.cpp
lldb/source/API/SBThread.cpp
lldb/source/API/SBValue.cpp
lldb/source/API/SBValueList.cpp
lldb/source/API/SBWatchpoint.cpp
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/ModuleList.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectCast.cpp
lldb/source/Core/ValueObjectConstResult.cpp
lldb/source/Core/ValueObjectDynamicValue.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
lldb/source/Expression/Materializer.cpp
lldb/source/Expression/UserExpression.cpp
lldb/source/Host/common/LockFileBase.cpp
lldb/source/Host/common/NativeProcessProtocol.cpp
lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/ScriptInterpreter.cpp
lldb/source/Plugins/Platform/Android/AdbClient.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.h
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface.cpp
lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Target/ModuleCache.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/Target.cpp
lldb/source/Utility/Status.cpp
lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
lldb/unittests/Target/RemoteAwarePlatformTest.cpp
Removed:
diff --git a/lldb/bindings/python/python-swigsafecast.swig
b/lldb/bindings/python/python-swigsafecast.swig
index 34f8c6f0ff8d35..0127ac6bfa4681 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -33,8 +33,8 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP
breakpoint_sp) {
SWIGTYPE_p_lldb__SBBreakpoint);
}
-PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
- return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
+PythonObject SWIGBridge::ToSWIGWrapper(Status status) {
+ return ToSWIGHelper(new lldb::SBError(std::move(status)),
SWIGTYPE_p_lldb__SBError);
}
PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr
data_sb) {
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 17f2c6c3027af7..9f55f92131c06e 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -97,7 +97,7 @@ class LLDB_API SBError {
friend class lldb_private::ScriptInterpreter;
friend class lldb_private::python::SWIGBridge;
- SBError(const lldb_private::Status &error);
+ SBError(lldb_private::Status &&error);
lldb_private::Status *get();
@@ -107,7 +107,7 @@ class LLDB_API SBError {
lldb_private::Status &ref();
- void SetError(const lldb_private::Status &lldb_error);
+ void SetError(lldb_private::Status &&lldb_error);
private:
std::unique_ptr m_opaque_up;
diff --git a/lldb/include/lldb/API/SBValueList.h
b/lldb/include/lldb/API/SBValueList.h
index a5017bccc50533..52a86f989e153a 100644
--- a/lldb/include/lldb/API/SBValueList.h
+++ b/lldb/include/lldb/API/SBValueList.h
@@ -96,7 +96,7 @@ class LLDB_API SBValueList {
std::unique_ptr m_opaque_up;
- voi
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/107170 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make deep copies of Status explicit (NFC) (PR #107170)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `lldb-x86_64-debian` running on `lldb-x86_64-debian` while building `lldb` at step 4 "build". Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/5655 Here is the relevant piece of the build log for the reference ``` Step 4 (build) failure: build (failure) ... 39.282 [446/72/460] Building CXX object tools/lldb/source/Plugins/Process/Utility/CMakeFiles/lldbPluginProcessUtility.dir/RegisterContextLinux_x86_64.cpp.o 39.379 [445/72/461] Building CXX object tools/lldb/source/Plugins/ObjectFile/ELF/CMakeFiles/lldbPluginObjectFileELF.dir/ObjectFileELF.cpp.o 39.394 [444/72/462] Building CXX object tools/lldb/source/Plugins/Process/Utility/CMakeFiles/lldbPluginProcessUtility.dir/RegisterContextNetBSD_x86_64.cpp.o 39.531 [443/72/463] Building CXX object tools/lldb/source/Plugins/Language/CPlusPlus/CMakeFiles/lldbPluginCPlusPlusLanguage.dir/CPlusPlusLanguage.cpp.o 39.562 [442/72/464] Building CXX object tools/lldb/source/Plugins/Process/Utility/CMakeFiles/lldbPluginProcessUtility.dir/RegisterInfos_x86_64_with_base_shared.cpp.o 39.685 [441/72/465] Building CXX object tools/lldb/source/Plugins/Platform/FreeBSD/CMakeFiles/lldbPluginPlatformFreeBSD.dir/PlatformFreeBSD.cpp.o 39.902 [440/72/466] Building CXX object tools/lldb/source/Plugins/Process/minidump/CMakeFiles/lldbPluginProcessMinidump.dir/MinidumpTypes.cpp.o 39.932 [439/72/467] Building CXX object tools/lldb/source/Plugins/Platform/Android/CMakeFiles/lldbPluginPlatformAndroid.dir/PlatformAndroid.cpp.o 40.205 [438/72/468] Building CXX object tools/lldb/source/Plugins/Process/gdb-remote/CMakeFiles/lldbPluginProcessGDBRemote.dir/GDBRemoteCommunicationServerPlatform.cpp.o 40.294 [437/72/469] Building CXX object tools/lldb/source/Plugins/Process/Linux/CMakeFiles/lldbPluginProcessLinux.dir/NativeProcessLinux.cpp.o FAILED: tools/lldb/source/Plugins/Process/Linux/CMakeFiles/lldbPluginProcessLinux.dir/NativeProcessLinux.cpp.o /usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/source/Plugins/Process/Linux -I/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/source/Plugins/Process/Linux -I/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/include -I/home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/include -I/home/worker/2.0.1/lldb-x86_64-debian/build/include -I/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/llvm/include -I/usr/include/python3.11 -I/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/llvm/../clang/include -I/home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/../clang/include -I/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/source -I/home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/source -isystem /usr/include/libxml2 -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -O3 -DNDEBUG -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/lldb/source/Plugins/Process/Linux/CMakeFiles/lldbPluginProcessLinux.dir/NativeProcessLinux.cpp.o -MF tools/lldb/source/Plugins/Process/Linux/CMakeFiles/lldbPluginProcessLinux.dir/NativeProcessLinux.cpp.o.d -o tools/lldb/source/Plugins/Process/Linux/CMakeFiles/lldbPluginProcessLinux.dir/NativeProcessLinux.cpp.o -c /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1100:13: error: object of type 'lldb_private::Status' cannot be assigned because its copy assignment operator is implicitly deleted error = ^ /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/include/lldb/Utility/Status.h:71:3: note: copy assignment operator is implicitly deleted because 'Status' has a user-declared move constructor Status(Status &&other) = default; ^ /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1911:12: error: call to implicitly-deleted copy constructor of 'lldb_private::Status' return resume_result; ^ /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/include/lldb/Utility/Status.h:71:3: note: copy constructor is implicitly de
[Lldb-commits] [lldb] 7b76089 - [lldb] Convert NativeProcessLinux to new Status API (NFC)
Author: Adrian Prantl
Date: 2024-09-05T12:49:16-07:00
New Revision: 7b760894f247f4fa1b27c01c767c8599c169f996
URL:
https://github.com/llvm/llvm-project/commit/7b760894f247f4fa1b27c01c767c8599c169f996
DIFF:
https://github.com/llvm/llvm-project/commit/7b760894f247f4fa1b27c01c767c8599c169f996.diff
LOG: [lldb] Convert NativeProcessLinux to new Status API (NFC)
Added:
Modified:
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
Removed:
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index cc0e34eecdf300..cea3fbf9112f52 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1096,9 +1096,9 @@ Status NativeProcessLinux::Detach() {
for (const auto &thread : m_threads) {
Status e = Detach(thread->GetID());
+ // Save the error, but still attempt to detach from other threads.
if (e.Fail())
- error =
- e; // Save the error, but still attempt to detach from other threads.
+ error = e.Clone;
}
m_intel_pt_collector.Clear();
@@ -1905,13 +1905,13 @@ Status
NativeProcessLinux::ResumeThread(NativeThreadLinux &thread,
// reflect it is running after this completes.
switch (state) {
case eStateRunning: {
-const auto resume_result = thread.Resume(signo);
+Status resume_result = thread.Resume(signo);
if (resume_result.Success())
SetState(eStateRunning, true);
return resume_result;
}
case eStateStepping: {
-const auto step_result = thread.SingleStep(signo);
+Status step_result = thread.SingleStep(signo);
if (step_result.Success())
SetState(eStateRunning, true);
return step_result;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3b426a8 - [lldb] Convert NativeProcessLinux to new Status API (NFC)
Author: Adrian Prantl
Date: 2024-09-05T12:53:08-07:00
New Revision: 3b426a8951caa543b65f20ff265353fd79f436e5
URL:
https://github.com/llvm/llvm-project/commit/3b426a8951caa543b65f20ff265353fd79f436e5
DIFF:
https://github.com/llvm/llvm-project/commit/3b426a8951caa543b65f20ff265353fd79f436e5.diff
LOG: [lldb] Convert NativeProcessLinux to new Status API (NFC)
Added:
Modified:
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
Removed:
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index cea3fbf9112f52..5c262db8db7fde 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1098,7 +1098,7 @@ Status NativeProcessLinux::Detach() {
Status e = Detach(thread->GetID());
// Save the error, but still attempt to detach from other threads.
if (e.Fail())
- error = e.Clone;
+ error = e.Clone();
}
m_intel_pt_collector.Clear();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/106774
>From 6e4559f792d692da6bb92c463e86f26382cc150b Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Wed, 4 Sep 2024 12:50:37 -0700
Subject: [PATCH] [lldb] Change the implementation of Status to store an
llvm::Error (NFC)
Most APIs that currently vend a Status would be better served by
returning llvm::Expected<> instead. Where possible, APIs should be
refactored to avoid Status. The only legitimate long-term uses of
Status are objects that need to store an error for a long time (which
should be questioned as a design decision, too).
This patch makes the transition to llvm::Error easier by making the
places that cannot switch to llvm::Error explicit: They are marked
with a call to Status::clone(). Every other API can and should be
refactored to use llvm::Expected. In the end Status should only be
used in very few places.
Whenever an unchecked Error is dropped by Status it logs this to the
verbose API channel.
Implementation notes:
This patch introduces two new kinds of error_category as well as new
llvm::Error types. Here is the mapping of lldb::ErrorType to
llvm::Errors:
(eErrorTypeInvalid)
eErrorTypeGeneric llvm::StringError
eErrorTypePOSIXllvm::ECError
eErrorTypeMachKernel MachKernelError
eErrorTypeExpression llvm::ErrorList
eErrorTypeWin32Win32Error
---
lldb/include/lldb/Utility/Status.h | 59 +--
lldb/source/Utility/Status.cpp | 245 -
2 files changed, 218 insertions(+), 86 deletions(-)
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index 795c830b965173..a0d7ca76a4c346 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -28,6 +28,52 @@ namespace lldb_private {
const char *ExpressionResultAsCString(lldb::ExpressionResults result);
+/// Going a bit against the spirit of llvm::Error,
+/// lldb_private::Status need to store errors long-term and sometimes
+/// copy them. This base class defines an interface for this
+/// operation.
+class CloneableError
+: public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ CloneableError(std::error_code ec, const llvm::Twine &msg = {})
+ : ErrorInfo(msg, ec) {}
+ virtual std::unique_ptr Clone() const = 0;
+ static char ID;
+};
+
+/// FIXME: Move these declarations closer to where they're used.
+class MachKernelError
+: public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ MachKernelError(std::error_code ec, const llvm::Twine &msg = {})
+ : ErrorInfo(msg, ec) {}
+ std::string message() const override;
+ std::unique_ptr Clone() const override;
+ static char ID;
+};
+
+class Win32Error : public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ Win32Error(std::error_code ec, const llvm::Twine &msg = {})
+ : ErrorInfo(msg, ec) {}
+ std::string message() const override;
+ std::unique_ptr Clone() const override;
+ static char ID;
+};
+
+class ExpressionError
+: public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ ExpressionError(std::error_code ec, std::string msg = {})
+ : ErrorInfo(msg, ec) {}
+ std::unique_ptr Clone() const override;
+ static char ID;
+};
+
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
///
/// This class is designed to be able to hold any error code that can be
@@ -100,9 +146,7 @@ class Status {
}
static Status FromExpressionError(lldb::ExpressionResults result,
-std::string msg) {
-return Status(result, lldb::eErrorTypeExpression, msg);
- }
+std::string msg);
/// Set the current error to errno.
///
@@ -115,6 +159,7 @@ class Status {
const Status &operator=(Status &&);
/// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
static Status FromError(llvm::Error error);
+
/// FIXME: Replace this with a takeError() method.
llvm::Error ToError() const;
/// Don't call this function in new code. Instead, redesign the API
@@ -170,12 +215,8 @@ class Status {
bool Success() const;
protected:
- Status(llvm::Error error);
- /// Status code as an integer value.
- ValueType m_code = 0;
- /// The type of the above error code.
- lldb::ErrorType m_type = lldb::eErrorTypeInvalid;
- /// A string representation of the error code.
+ Status(llvm::Error error) : m_error(std::move(error)) {}
+ mutable llvm::Error m_error;
mutable std::string m_string;
};
diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp
index 4af3af5fba0185..5ae1c4da41a597 100644
--- a/lldb/source/Utility/Status.cpp
+++ b/lldb/source/Utility/Status.cpp
@@ -8,6 +8,8 @@
#include "lldb/Utility/Status.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
#include "lldb/Utility/VASPrintf.h"
#i
[Lldb-commits] [lldb] [lldb] Make conversions from llvm::Error explicit with Status::FromEr… (PR #107163)
nico wrote: Looks like this breaks building on windows: http://45.33.8.238/win/93597/step_3.txt Please take a look and revert for now if it takes a while to fix. https://github.com/llvm/llvm-project/pull/107163 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/106774
>From 12bf3fba23b130455bed2a10866a37433a4b471c Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Wed, 4 Sep 2024 12:50:37 -0700
Subject: [PATCH] [lldb] Change the implementation of Status to store an
llvm::Error (NFC)
Most APIs that currently vend a Status would be better served by
returning llvm::Expected<> instead. Where possible, APIs should be
refactored to avoid Status. The only legitimate long-term uses of
Status are objects that need to store an error for a long time (which
should be questioned as a design decision, too).
This patch makes the transition to llvm::Error easier by making the
places that cannot switch to llvm::Error explicit: They are marked
with a call to Status::clone(). Every other API can and should be
refactored to use llvm::Expected. In the end Status should only be
used in very few places.
Whenever an unchecked Error is dropped by Status it logs this to the
verbose API channel.
Implementation notes:
This patch introduces two new kinds of error_category as well as new
llvm::Error types. Here is the mapping of lldb::ErrorType to
llvm::Errors:
(eErrorTypeInvalid)
eErrorTypeGeneric llvm::StringError
eErrorTypePOSIXllvm::ECError
eErrorTypeMachKernel MachKernelError
eErrorTypeExpression llvm::ErrorList
eErrorTypeWin32Win32Error
---
lldb/include/lldb/Utility/Status.h | 73 +++--
lldb/source/Utility/Status.cpp | 246 -
2 files changed, 233 insertions(+), 86 deletions(-)
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index 795c830b965173..db77977fb17c41 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -28,6 +28,66 @@ namespace lldb_private {
const char *ExpressionResultAsCString(lldb::ExpressionResults result);
+/// Going a bit against the spirit of llvm::Error,
+/// lldb_private::Status need to store errors long-term and sometimes
+/// copy them. This base class defines an interface for this
+/// operation.
+class CloneableError
+: public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ CloneableError() : ErrorInfo() {}
+ virtual std::unique_ptr Clone() const = 0;
+ static char ID;
+};
+
+/// Common base class for all error-code errors.
+class CloneableECError
+: public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ CloneableECError(std::error_code ec) : ErrorInfo() {}
+ std::error_code convertToErrorCode() const override { return EC; }
+ void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
+ static char ID;
+
+protected:
+ std::error_code EC;
+};
+
+/// FIXME: Move these declarations closer to where they're used.
+class MachKernelError
+: public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ MachKernelError(std::error_code ec) : ErrorInfo(ec) {}
+ std::string message() const override;
+ std::unique_ptr Clone() const override;
+ static char ID;
+};
+
+class Win32Error : public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ Win32Error(std::error_code ec, const llvm::Twine &msg = {}) : ErrorInfo(ec)
{}
+ std::string message() const override;
+ std::unique_ptr Clone() const override;
+ static char ID;
+};
+
+class ExpressionError
+: public llvm::ErrorInfo {
+public:
+ using llvm::ErrorInfo::ErrorInfo;
+ ExpressionError(std::error_code ec, std::string msg = {})
+ : ErrorInfo(ec), m_string(msg) {}
+ std::unique_ptr Clone() const override;
+ static char ID;
+
+protected:
+ std::string m_string;
+};
+
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
///
/// This class is designed to be able to hold any error code that can be
@@ -100,9 +160,7 @@ class Status {
}
static Status FromExpressionError(lldb::ExpressionResults result,
-std::string msg) {
-return Status(result, lldb::eErrorTypeExpression, msg);
- }
+std::string msg);
/// Set the current error to errno.
///
@@ -115,6 +173,7 @@ class Status {
const Status &operator=(Status &&);
/// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
static Status FromError(llvm::Error error);
+
/// FIXME: Replace this with a takeError() method.
llvm::Error ToError() const;
/// Don't call this function in new code. Instead, redesign the API
@@ -170,12 +229,8 @@ class Status {
bool Success() const;
protected:
- Status(llvm::Error error);
- /// Status code as an integer value.
- ValueType m_code = 0;
- /// The type of the above error code.
- lldb::ErrorType m_type = lldb::eErrorTypeInvalid;
- /// A string representation of the error code.
+ Status(llvm::Error error) : m_error(std::move(error)) {}
+ mutable llvm::Error m_error;
mutable std::string m_string;
};
di
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -26,6 +26,52 @@ class raw_ostream;
namespace lldb_private {
+/// Going a bit against the spirit of llvm::Error,
+/// lldb_private::Status need to store errors long-term and sometimes
+/// copy them. This base class defines an interface for this
+/// operation.
+class CloneableError
+: public llvm::ErrorInfo {
adrian-prantl wrote:
Done.
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -37,48 +39,75 @@ class raw_ostream;
using namespace lldb;
using namespace lldb_private;
-Status::Status() {}
+char CloneableError::ID;
+char MachKernelError::ID;
+char Win32Error::ID;
+char ExpressionError::ID;
+
+namespace {
+/// A std::error_code category for eErrorTypeGeneric.
+class GenericCategory : public std::error_category {
+ const char *name() const override { return "LLDBGenericCategory"; }
+ std::string message(int __ev) const override { return "generic LLDB error";
};
+};
+GenericCategory &generic_category() {
+ static GenericCategory g_generic_category;
+ return g_generic_category;
+}
+
+/// A std::error_code category for eErrorTypeExpression.
+class ExpressionCategory : public std::error_category {
+ const char *name() const override { return "LLDBExpressionCategory"; }
+ std::string message(int __ev) const override {
+return
ExecutionResultAsCString(static_cast(__ev));
+ };
+};
+ExpressionCategory &expression_category() {
+ static ExpressionCategory g_expression_category;
+ return g_expression_category;
+}
adrian-prantl wrote:
For the time being, we do. I'm not opposed to get rid of this later, either.
Right now expressions return an ExpressionResult enum, which is encoded as this
custom ExpressionCategory error code in this patch.
A failing expression returns an
ErrorList({
ECError(ExpressionCategory, ExpressionResult),
ExpressionError(diagnostic1),
ExpressionError(diagnostic2),
...
})
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)
@@ -37,48 +39,75 @@ class raw_ostream;
using namespace lldb;
using namespace lldb_private;
-Status::Status() {}
+char CloneableError::ID;
+char MachKernelError::ID;
+char Win32Error::ID;
+char ExpressionError::ID;
+
+namespace {
+/// A std::error_code category for eErrorTypeGeneric.
+class GenericCategory : public std::error_category {
+ const char *name() const override { return "LLDBGenericCategory"; }
+ std::string message(int __ev) const override { return "generic LLDB error";
};
+};
+GenericCategory &generic_category() {
+ static GenericCategory g_generic_category;
+ return g_generic_category;
+}
+
+/// A std::error_code category for eErrorTypeExpression.
+class ExpressionCategory : public std::error_category {
+ const char *name() const override { return "LLDBExpressionCategory"; }
+ std::string message(int __ev) const override {
+return
ExecutionResultAsCString(static_cast(__ev));
+ };
+};
+ExpressionCategory &expression_category() {
+ static ExpressionCategory g_expression_category;
+ return g_expression_category;
+}
+} // namespace
+
+Status::Status() : m_error(llvm::Error::success()) {}
+
+static llvm::Error ErrorFromEnums(Status::ValueType err, ErrorType type,
+ std::string msg) {
+ switch (type) {
+ case eErrorTypeMachKernel:
+return llvm::make_error(
+std::error_code(err, std::system_category()), msg);
+ case eErrorTypePOSIX:
+return llvm::errorCodeToError(
+std::error_code(err, std::generic_category()));
+ case eErrorTypeWin32:
+return llvm::make_error(
+std::error_code(err, std::system_category()), msg);
+ default:
+return llvm::createStringError(std::move(msg),
+ std::error_code(err, generic_category()));
+ }
+}
Status::Status(ValueType err, ErrorType type, std::string msg)
-: m_code(err), m_type(type), m_string(std::move(msg)) {}
+: m_error(ErrorFromEnums(err, type, msg)) {}
-// This logic is confusing because c++ calls the traditional (posix) errno
codes
+// This logic is confusing because C++ calls the traditional (posix) errno
codes
// "generic errors", while we use the term "generic" to mean completely
// arbitrary (text-based) errors.
Status::Status(std::error_code EC)
-: m_code(EC.value()),
- m_type(EC.category() == std::generic_category() ? eErrorTypePOSIX
- : eErrorTypeGeneric),
- m_string(EC.message()) {}
+: m_error(!EC ? llvm::Error::success()
+ : (EC.category() == std::generic_category()
+ ? llvm::errorCodeToError(EC)
+ : llvm::createStringError(EC, "generic error"))) {}
adrian-prantl wrote:
This is necessary to preserve the expected round-tripping between
eErrorTypePosix versus eErrorTypeGeneric. Either we do this, or we need to
introduce a special POSIX ErrorInfo (which would be weird because it would
literally be a reimplementation of ECError).
https://github.com/llvm/llvm-project/pull/106774
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix some tests that fail with system libstdc++ (PR #106885)
tstellar wrote: @felipepiovezan Are you OK if we push this as-is, or would you like smarter logic for detecting libc++ on the system? https://github.com/llvm/llvm-project/pull/106885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix some tests that fail with system libstdc++ (PR #106885)
felipepiovezan wrote: > @felipepiovezan Are you OK if we push this as-is, or would you like smarter > logic for detecting libc++ on the system? I'm fine with this; in hindsight, I think the original workaround was not good enough. The longer term fix would be set both "use system std library" "use libcxx" and skip the test if it's impossible to satisfy both requirements at the same time. But we can do that later if this becomes a problem. https://github.com/llvm/llvm-project/pull/106885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix some tests that fail with system libstdc++ (PR #106885)
https://github.com/felipepiovezan approved this pull request. https://github.com/llvm/llvm-project/pull/106885 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1e98aa4 - [lldb] Convert ConnectionGenericFileWindows.cpp to new Status API (NFC)
Author: Adrian Prantl Date: 2024-09-05T15:09:25-07:00 New Revision: 1e98aa4730b1b3b93205af74be26e04d5f876d10 URL: https://github.com/llvm/llvm-project/commit/1e98aa4730b1b3b93205af74be26e04d5f876d10 DIFF: https://github.com/llvm/llvm-project/commit/1e98aa4730b1b3b93205af74be26e04d5f876d10.diff LOG: [lldb] Convert ConnectionGenericFileWindows.cpp to new Status API (NFC) Added: Modified: lldb/source/Host/windows/ConnectionGenericFileWindows.cpp Removed: diff --git a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp index 170444f56b3107..6a6153d6e34a08 100644 --- a/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp +++ b/lldb/source/Host/windows/ConnectionGenericFileWindows.cpp @@ -236,7 +236,7 @@ size_t ConnectionGenericFile::Read(void *dst, size_t dst_len, finish: status = return_info.GetStatus(); if (error_ptr) -*error_ptr = Status::FromError(return_info.GetError()); +*error_ptr = return_info.GetError().Clone(); // kBytesAvailableEvent is a manual reset event. Make sure it gets reset // here so that any subsequent operations don't immediately see bytes @@ -290,7 +290,7 @@ size_t ConnectionGenericFile::Write(const void *src, size_t src_len, finish: status = return_info.GetStatus(); if (error_ptr) -*error_ptr = Status::FromError(return_info.GetError()); +*error_ptr = return_info.GetError().Clone(); IncrementFilePointer(return_info.GetBytes()); Log *log = GetLog(LLDBLog::Connection); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Make env and source map dictionaries #95137 (PR #106919)
https://github.com/JDevlieghere approved this pull request. LGTM. I like having both options. That way we don't break existing use cases while also supporting the more intuitive way of expressing this as a dictionary. https://github.com/llvm/llvm-project/pull/106919 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Separated GDBRemoteCommunication::GetDebugserverPath() (PR #107388)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/107388 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Separated GDBRemoteCommunication::GetDebugserverPath() (PR #107388)
https://github.com/JDevlieghere approved this pull request. LGTM modulo one nit. https://github.com/llvm/llvm-project/pull/107388 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Separated GDBRemoteCommunication::GetDebugserverPath() (PR #107388)
@@ -943,8 +935,20 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
}
}
}
+ return debugserver_file_spec;
+}
- if (debugserver_exists) {
+Status GDBRemoteCommunication::StartDebugserverProcess(
+const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
+uint16_t *port, const Args *inferior_args, shared_fd_t pass_comm_fd) {
+ Log *log = GetLog(GDBRLog::Process);
+ LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
+__FUNCTION__, url ? url : "", port ? *port : uint16_t(0));
+
+ Status error;
+ FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
+ if (debugserver_file_spec = GetDebugserverPath(platform)) {
+char debugserver_path[PATH_MAX];
JDevlieghere wrote:
Looks like we're immediately converting this to a `StringRef`. Since we're
already touching this line, could we make this `std::string debugserver_path =
debugserver_file_spec.GetPath()`?
https://github.com/llvm/llvm-project/pull/107388
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
https://github.com/cmtice created
https://github.com/llvm/llvm-project/pull/107485
Update lldb-dap so if the user just presses return, which sends an empty
expression, it re-evaluates the most recent non-empty expression/command. Also
udpated test to test this case.
>From 15541f354decf80586d590db9f9cb353be04b122 Mon Sep 17 00:00:00 2001
From: Caroline Tice
Date: Thu, 5 Sep 2024 15:51:35 -0700
Subject: [PATCH] [lldb-dap] Add feature to remember last non-empty expression.
Update lldb-dap so if the user just presses return, which sends an
empty expression, it re-evaluates the most recent non-empty
expression/command. Also udpated test to test this case.
---
lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py | 3 +++
lldb/tools/lldb-dap/lldb-dap.cpp | 8
2 files changed, 11 insertions(+)
diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
index 29548a835c6919..9ed0fc564268a7 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -60,7 +60,10 @@ def run_test_evaluate_expressions(
# Expressions at breakpoint 1, which is in main
self.assertEvaluate("var1", "20")
+# Empty expression should equate to the previous expression.
+self.assertEvaluate("", "20")
self.assertEvaluate("var2", "21")
+self.assertEvaluate("", "21")
self.assertEvaluate("static_int", "42")
self.assertEvaluate("non_static_int", "43")
self.assertEvaluate("struct1.foo", "15")
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index c5c4b09f15622b..a6a701dc2219fa 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1363,6 +1363,14 @@ void request_evaluate(const llvm::json::Object &request)
{
lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
std::string expression = GetString(arguments, "expression").str();
llvm::StringRef context = GetString(arguments, "context");
+ static std::string last_nonempty_expression;
+
+ // Remember the last non-empty expression from the user, and use that if
+ // the current expression is empty (i.e. the user hit plain 'return').
+ if (!expression.empty())
+last_nonempty_expression = expression;
+ else
+expression = last_nonempty_expression;
if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) ==
ExpressionContext::Command) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][NFC] Separated GDBRemoteCommunication::GetDebugserverPath() (PR #107388)
@@ -943,8 +935,20 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
}
}
}
+ return debugserver_file_spec;
+}
- if (debugserver_exists) {
+Status GDBRemoteCommunication::StartDebugserverProcess(
+const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
+uint16_t *port, const Args *inferior_args, shared_fd_t pass_comm_fd) {
+ Log *log = GetLog(GDBRLog::Process);
+ LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
+__FUNCTION__, url ? url : "", port ? *port : uint16_t(0));
+
+ Status error;
+ FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
+ if (debugserver_file_spec = GetDebugserverPath(platform)) {
+char debugserver_path[PATH_MAX];
jasonmolenda wrote:
Agree with this comment, otherwise this change looks fine to me.
https://github.com/llvm/llvm-project/pull/107388
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: None (cmtice)
Changes
Update lldb-dap so if the user just presses return, which sends an empty
expression, it re-evaluates the most recent non-empty expression/command. Also
udpated test to test this case.
---
Full diff: https://github.com/llvm/llvm-project/pull/107485.diff
2 Files Affected:
- (modified) lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py (+3)
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+8)
``diff
diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
index 29548a835c6919..9ed0fc564268a7 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -60,7 +60,10 @@ def run_test_evaluate_expressions(
# Expressions at breakpoint 1, which is in main
self.assertEvaluate("var1", "20")
+# Empty expression should equate to the previous expression.
+self.assertEvaluate("", "20")
self.assertEvaluate("var2", "21")
+self.assertEvaluate("", "21")
self.assertEvaluate("static_int", "42")
self.assertEvaluate("non_static_int", "43")
self.assertEvaluate("struct1.foo", "15")
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index c5c4b09f15622b..a6a701dc2219fa 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1363,6 +1363,14 @@ void request_evaluate(const llvm::json::Object &request)
{
lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
std::string expression = GetString(arguments, "expression").str();
llvm::StringRef context = GetString(arguments, "context");
+ static std::string last_nonempty_expression;
+
+ // Remember the last non-empty expression from the user, and use that if
+ // the current expression is empty (i.e. the user hit plain 'return').
+ if (!expression.empty())
+last_nonempty_expression = expression;
+ else
+expression = last_nonempty_expression;
if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) ==
ExpressionContext::Command) {
``
https://github.com/llvm/llvm-project/pull/107485
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
