date:20240724

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits


https://github.com/labath commented:

I've made one pass over the PR. I think this would be easier to review if it 
was split into multiple patches:
- core llvm infrastructure
- core lldb infrastructure
- ~one patch per event type

I think the biggest hurdle will be finding someone to approve the addition of 
the new llvm library (I doubt it's going to be someone from the current 
reviewer set). I wouldn't be surprised if this ends up needing its own RFC.

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits


https://github.com/labath edited https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,153 @@
+#ifndef LLDB_CORE_TELEMETRY_H
+#define LLDB_CORE_TELEMETRY_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+using namespace llvm::telemetry;
+
+namespace lldb_private {
+
+struct DebuggerTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  std::string username;
+  std::string lldb_git_sha;
+  std::string lldb_path;
+  std::string cwd;
+
+  std::string ToString() const override;
+};
+
+struct TargetTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // All entries emitted for the same SBTarget will have the same
+  // target_uuid.
+  std::string target_uuid;
+  std::string file_format;
+
+  std::string binary_path;
+  size_t binary_size;
+
+  std::string ToString() const override;
+};
+
+// Entry from client (eg., SB-API)
+struct ClientTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  std::string request_name;
+  std::string error_msg;
+  std::string ToString() const override;
+};
+
+struct CommandTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // If the command is/can be associated with a target entry,
+  // this field contains that target's UUID.
+  //  otherwise.
+  std::string target_uuid;
+  std::string command_uuid;
+
+  // Eg., "breakpoint set"
+  std::string command_name;
+
+  // !!NOTE!!: The following fields may be omitted due to PII risk.
+  // (Configurable via the TelemetryConfig struct)
+  std::string original_command;
+  std::string args;
+
+  std::string ToString() const override;
+};
+
+// The "catch-all" entry to store a set of custom/non-standard
+// data.
+struct MiscTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // If the event is/can be associated with a target entry,
+  // this field contains that target's UUID.
+  //  otherwise.
+  std::string target_uuid;
+
+  // Set of key-value pairs for any optional (or impl-specific) data
+  std::unordered_map meta_data;
+
+  std::string ToString() const override;
+};
+
+class LldbTelemeter : public llvm::telemetry::Telemeter {
+public:
+  static std::shared_ptr CreateInstance(Debugger *);
+
+  virtual ~LldbTelemeter() = default;
+
+  // void LogStartup(llvm::StringRef lldb_path,
+  // TelemetryInfo *entry) override;
+  // void LogExit(llvm::StringRef lldb_path, TelemetryInfo *entry)
+  // override;
+
+  // Invoked upon process exit
+  virtual void LogProcessExit(int status, llvm::StringRef exit_string,
+  TelemetryEventStats stats,
+  Target *target_ptr) = 0;
+
+  // Invoked upon loading the main executable module
+  // We log in a fire-n-forget fashion so that if the load
+  // crashes, we don't lose the entry.
+  virtual void LogMainExecutableLoadStart(lldb::ModuleSP exec_mod,
+  TelemetryEventStats stats) = 0;
+  virtual void LogMainExecutableLoadEnd(lldb::ModuleSP exec_mod,
+TelemetryEventStats stats) = 0;
+
+  // Invoked for each command
+  // We log in a fire-n-forget fashion so that if the command execution
+  // crashes, we don't lose the entry.
+  virtual void LogCommandStart(llvm::StringRef uuid,
+   llvm::StringRef original_command,
+   TelemetryEventStats stats,
+   Target *target_ptr) = 0;
+  virtual void LogCommandEnd(llvm::StringRef uuid, llvm::StringRef 
command_name,
+ llvm::StringRef command_args,
+ TelemetryEventStats stats, Target *target_ptr,
+ CommandReturnObject *result) = 0;
+
+  virtual std::string GetNextUUID() = 0;
+
+  // For client (eg., SB API) to send telemetry entries.
+  virtual void
+  LogClientTelemetry(lldb_private::StructuredData::Object *entry) = 0;
+};
+
+// Logger configs: LLDB users can also supply their own configs via:
+// $HOME/.lldb_telemetry_config
+//
+// We can propose simple syntax: 

labath wrote:

Since this is actual code, it should talk about the (real) syntax, not the 
proposed one.

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,619 @@
+
+//===-- Telemetry.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "lldb/Core/Telemetry.h"
+
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/UUID.h"
+#include "lldb/Version/Version.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+#ifdef HAS_VENDOR_TELEMETRY_PLUGINS
+// TODO: could make this path a build-variable rather than hard-coded
+#include "lldb/Core/VendorTelemetryPlugin.h"
+
+// This must include definitions for these functions
+extern void ApplyVendorSpecificConfigs(
+llvm::telemetry::TelemetryConfig *config) /* __attribute__((weak))*/;
+extern std::shared_ptr SanitizeSensitiveFields(
+const llvm::telemetry::TelemetryInfo *entry) /*__attribute__((weak))*/;
+extern std::shared_ptr
+CreateVendorSpecificTelemeter(
+llvm::telemetry::TelemetryConfig *config) /*__attribute__((weak))*/;
+#endif
+
+namespace lldb_private {
+
+static std::string GetDuration(const TelemetryEventStats &stats) {
+  if (stats.m_end.has_value())
+return std::to_string((stats.m_end.value() - stats.m_start).count()) +
+   "(nanosec)";
+  return "";
+}
+
+std::string DebuggerTelemetryInfo::ToString() const {
+  std::string duration_desc =
+  (exit_description.has_value() ? "  lldb session duration: "
+: "  lldb startup duration: ") +
+  std::to_string((stats.m_end.value() - stats.m_start).count()) +
+  "(nanosec)\n";
+
+  return TelemetryInfo::ToString() + "\n" + ("[DebuggerTelemetryInfo]\n") +
+ ("  username: " + username + "\n") +
+ ("  lldb_git_sha: " + lldb_git_sha + "\n") +
+ ("  lldb_path: " + lldb_path + "\n") + ("  cwd: " + cwd + "\n") +
+ duration_desc + "\n";
+}
+
+std::string ClientTelemetryInfo::ToString() const {
+  return TelemetryInfo::ToString() + "\n" + ("[DapRequestInfoEntry]\n") +
+ ("  request_name: " + request_name + "\n") +
+ ("  request_duration: " + GetDuration(stats) + "(nanosec)\n") +
+ ("  error_msg: " + error_msg + "\n");
+}
+
+std::string TargetTelemetryInfo::ToString() const {
+  std::string exit_or_load_desc;
+  if (exit_description.has_value()) {
+// If this entry was emitted for an exit
+exit_or_load_desc = "  process_duration: " + GetDuration(stats) +
+"  exit: " + exit_description->ToString() + "\n";
+  } else {
+// This was emitted for a load event.
+// See if it was the start-load or end-load entry
+if (stats.m_end.has_value()) {
+  exit_or_load_desc =
+  "  startup_init_duration: " + GetDuration(stats) + "\n";
+} else {
+  exit_or_load_desc = " startup_init_start\n";
+}
+  }
+  return TelemetryInfo::ToString() + "\n" + ("[TargetTelemetryInfo]\n") +
+ ("  target_uuid: " + target_uuid + "\n") +
+ ("  file_format: " + file_format + "\n") +
+ ("  binary_path: " + binary_path + "\n") +
+ ("  binary_size: " + std::to_string(binary_size) + "\n") +
+ exit_or_load_desc;
+}
+
+static std::string StatusToString(CommandReturnObject *result) {
+  std::string msg;
+  switch (result->GetStatus()) {
+  case lldb::eReturnStatusInvalid:
+msg = "invalid";
+break;
+  case lldb::eReturnStatusSuccessFinishNoResult:
+msg = "success_finish_no_result";
+break;
+  case lldb::eReturnStatusSuccessFinishResult:
+msg = "success_finish_result";
+break;
+  case lldb::eReturnStatusSuccessContinuingNoResult:
+msg = "success_continuing_no_result";
+break;
+  case lldb::eReturnStatusSuccessContinuingResult:
+msg = "success_continuing_result";
+break;
+  case lldb::eReturnStatusStarted:
+msg = "started";
+break;
+  case lldb::eReturnStatusFailed:
+msg = "failed";
+break;
+  case lldb::eReturnStatusQuit:
+

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,619 @@
+

labath wrote:

delete empty line

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -9,10 +9,12 @@
 #ifndef LLDB_API_SBDEBUGGER_H
 #define LLDB_API_SBDEBUGGER_H
 
+#include 

labath wrote:

Is this used anywhere?

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,619 @@
+
+//===-- Telemetry.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "lldb/Core/Telemetry.h"
+
+#include 
+#include 

labath wrote:

??

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -34,6 +34,7 @@
 #include "lldb/API/SBTypeNameSpecifier.h"
 #include "lldb/API/SBTypeSummary.h"
 #include "lldb/API/SBTypeSynthetic.h"
+#include 

labath wrote:

`` is 
[banned](https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden)
 in llvm, but we also don't generally use stderr as a reporting channel. If you 
think it's useful, you can have the SB method return some kind of a error 
indicator (bool, SBError).

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,153 @@
+#ifndef LLDB_CORE_TELEMETRY_H
+#define LLDB_CORE_TELEMETRY_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+using namespace llvm::telemetry;
+
+namespace lldb_private {
+
+struct DebuggerTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  std::string username;
+  std::string lldb_git_sha;
+  std::string lldb_path;
+  std::string cwd;
+
+  std::string ToString() const override;
+};
+
+struct TargetTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // All entries emitted for the same SBTarget will have the same
+  // target_uuid.
+  std::string target_uuid;

labath wrote:

What's the relation of this field to the UUID of the executable module? What 
happens if the process does an `execve`?

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,619 @@
+
+//===-- Telemetry.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "lldb/Core/Telemetry.h"
+
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/UUID.h"
+#include "lldb/Version/Version.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+#ifdef HAS_VENDOR_TELEMETRY_PLUGINS
+// TODO: could make this path a build-variable rather than hard-coded
+#include "lldb/Core/VendorTelemetryPlugin.h"
+
+// This must include definitions for these functions
+extern void ApplyVendorSpecificConfigs(
+llvm::telemetry::TelemetryConfig *config) /* __attribute__((weak))*/;
+extern std::shared_ptr SanitizeSensitiveFields(
+const llvm::telemetry::TelemetryInfo *entry) /*__attribute__((weak))*/;
+extern std::shared_ptr
+CreateVendorSpecificTelemeter(
+llvm::telemetry::TelemetryConfig *config) /*__attribute__((weak))*/;
+#endif
+
+namespace lldb_private {
+
+static std::string GetDuration(const TelemetryEventStats &stats) {
+  if (stats.m_end.has_value())
+return std::to_string((stats.m_end.value() - stats.m_start).count()) +
+   "(nanosec)";
+  return "";
+}
+
+std::string DebuggerTelemetryInfo::ToString() const {
+  std::string duration_desc =
+  (exit_description.has_value() ? "  lldb session duration: "
+: "  lldb startup duration: ") +
+  std::to_string((stats.m_end.value() - stats.m_start).count()) +
+  "(nanosec)\n";
+
+  return TelemetryInfo::ToString() + "\n" + ("[DebuggerTelemetryInfo]\n") +
+ ("  username: " + username + "\n") +
+ ("  lldb_git_sha: " + lldb_git_sha + "\n") +
+ ("  lldb_path: " + lldb_path + "\n") + ("  cwd: " + cwd + "\n") +
+ duration_desc + "\n";
+}
+
+std::string ClientTelemetryInfo::ToString() const {
+  return TelemetryInfo::ToString() + "\n" + ("[DapRequestInfoEntry]\n") +
+ ("  request_name: " + request_name + "\n") +
+ ("  request_duration: " + GetDuration(stats) + "(nanosec)\n") +
+ ("  error_msg: " + error_msg + "\n");
+}
+
+std::string TargetTelemetryInfo::ToString() const {
+  std::string exit_or_load_desc;
+  if (exit_description.has_value()) {
+// If this entry was emitted for an exit
+exit_or_load_desc = "  process_duration: " + GetDuration(stats) +
+"  exit: " + exit_description->ToString() + "\n";
+  } else {
+// This was emitted for a load event.
+// See if it was the start-load or end-load entry
+if (stats.m_end.has_value()) {
+  exit_or_load_desc =
+  "  startup_init_duration: " + GetDuration(stats) + "\n";
+} else {
+  exit_or_load_desc = " startup_init_start\n";
+}
+  }
+  return TelemetryInfo::ToString() + "\n" + ("[TargetTelemetryInfo]\n") +
+ ("  target_uuid: " + target_uuid + "\n") +
+ ("  file_format: " + file_format + "\n") +
+ ("  binary_path: " + binary_path + "\n") +
+ ("  binary_size: " + std::to_string(binary_size) + "\n") +
+ exit_or_load_desc;
+}
+
+static std::string StatusToString(CommandReturnObject *result) {
+  std::string msg;
+  switch (result->GetStatus()) {
+  case lldb::eReturnStatusInvalid:
+msg = "invalid";
+break;
+  case lldb::eReturnStatusSuccessFinishNoResult:
+msg = "success_finish_no_result";
+break;
+  case lldb::eReturnStatusSuccessFinishResult:
+msg = "success_finish_result";
+break;
+  case lldb::eReturnStatusSuccessContinuingNoResult:
+msg = "success_continuing_no_result";
+break;
+  case lldb::eReturnStatusSuccessContinuingResult:
+msg = "success_continuing_result";
+break;
+  case lldb::eReturnStatusStarted:
+msg = "started";
+break;
+  case lldb::eReturnStatusFailed:
+msg = "failed";
+break;
+  case lldb::eReturnStatusQuit:
+

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,153 @@
+#ifndef LLDB_CORE_TELEMETRY_H
+#define LLDB_CORE_TELEMETRY_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+using namespace llvm::telemetry;
+
+namespace lldb_private {
+
+struct DebuggerTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  std::string username;
+  std::string lldb_git_sha;
+  std::string lldb_path;
+  std::string cwd;
+
+  std::string ToString() const override;
+};
+
+struct TargetTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // All entries emitted for the same SBTarget will have the same
+  // target_uuid.
+  std::string target_uuid;
+  std::string file_format;
+
+  std::string binary_path;
+  size_t binary_size;
+
+  std::string ToString() const override;
+};
+
+// Entry from client (eg., SB-API)
+struct ClientTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  std::string request_name;
+  std::string error_msg;
+  std::string ToString() const override;
+};
+
+struct CommandTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // If the command is/can be associated with a target entry,
+  // this field contains that target's UUID.
+  //  otherwise.
+  std::string target_uuid;
+  std::string command_uuid;
+
+  // Eg., "breakpoint set"
+  std::string command_name;
+
+  // !!NOTE!!: The following fields may be omitted due to PII risk.
+  // (Configurable via the TelemetryConfig struct)
+  std::string original_command;
+  std::string args;
+
+  std::string ToString() const override;
+};
+
+// The "catch-all" entry to store a set of custom/non-standard
+// data.
+struct MiscTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // If the event is/can be associated with a target entry,
+  // this field contains that target's UUID.
+  //  otherwise.
+  std::string target_uuid;
+
+  // Set of key-value pairs for any optional (or impl-specific) data
+  std::unordered_map meta_data;
+
+  std::string ToString() const override;
+};
+
+class LldbTelemeter : public llvm::telemetry::Telemeter {
+public:
+  static std::shared_ptr CreateInstance(Debugger *);
+
+  virtual ~LldbTelemeter() = default;
+
+  // void LogStartup(llvm::StringRef lldb_path,
+  // TelemetryInfo *entry) override;
+  // void LogExit(llvm::StringRef lldb_path, TelemetryInfo *entry)
+  // override;

labath wrote:

??

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,619 @@
+
+//===-- Telemetry.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "lldb/Core/Telemetry.h"
+
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/UUID.h"
+#include "lldb/Version/Version.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+#ifdef HAS_VENDOR_TELEMETRY_PLUGINS
+// TODO: could make this path a build-variable rather than hard-coded
+#include "lldb/Core/VendorTelemetryPlugin.h"
+
+// This must include definitions for these functions
+extern void ApplyVendorSpecificConfigs(
+llvm::telemetry::TelemetryConfig *config) /* __attribute__((weak))*/;
+extern std::shared_ptr SanitizeSensitiveFields(
+const llvm::telemetry::TelemetryInfo *entry) /*__attribute__((weak))*/;
+extern std::shared_ptr
+CreateVendorSpecificTelemeter(
+llvm::telemetry::TelemetryConfig *config) /*__attribute__((weak))*/;
+#endif
+
+namespace lldb_private {
+
+static std::string GetDuration(const TelemetryEventStats &stats) {
+  if (stats.m_end.has_value())
+return std::to_string((stats.m_end.value() - stats.m_start).count()) +
+   "(nanosec)";
+  return "";
+}
+
+std::string DebuggerTelemetryInfo::ToString() const {
+  std::string duration_desc =
+  (exit_description.has_value() ? "  lldb session duration: "
+: "  lldb startup duration: ") +
+  std::to_string((stats.m_end.value() - stats.m_start).count()) +
+  "(nanosec)\n";

labath wrote:

```suggestion
llvm::formatv(" lldb {0} duration: {1:ns}", 
exit_description?"session":"startup", stats.m_end.value() - stats.m_start);
```

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Pavel Labath via lldb-commits



@@ -0,0 +1,153 @@
+#ifndef LLDB_CORE_TELEMETRY_H
+#define LLDB_CORE_TELEMETRY_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+using namespace llvm::telemetry;

labath wrote:

Can't use this in a header as it effectively defeats the purpose of namespacing.

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correct format specifier for sscanf to prevent buffer overflow (NFC) (PR #94783)

2024-07-24 Thread David Spickett via lldb-commits


DavidSpickett wrote:

Please remove the formatting changes and limit this to just the specifier 
change.

I know most of the time we'd want it formatted but for a fix as limited as 
this, the formatting change just gets in the way, and not formatting it doesn't 
block merging either.

Otherwise this LGTM once that's done.

https://github.com/llvm/llvm-project/pull/94783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correct format specifier for sscanf to prevent buffer overflow (NFC) (PR #94783)

2024-07-24 Thread David Spickett via lldb-commits


DavidSpickett wrote:

The bot will complain again but we will ignore it :)

https://github.com/llvm/llvm-project/pull/94783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correct format specifier for sscanf to prevent buffer overflow (NFC) (PR #94783)

2024-07-24 Thread Pavel Labath via lldb-commits


https://github.com/labath commented:

The field this is consuming is actually 17 bytes long, because the process name 
is in parenthesis. I suspect this will cause the function to reject any process 
whose name is longer than 13 characters.

The name field is actually quite hard to parse this way since it can contain 
any character (esp. parenthesis and spaces). Now, we could devise an algorithm 
to do that, but since the code is later opening `/proc/$PID/status` anyway, and 
`status` contains a superset of information, I think it'd be best to just 
delete this code and extract the information we want from there.

`status` parsing code also uses more modern and less error prone patterns.

https://github.com/llvm/llvm-project/pull/94783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correct format specifier for sscanf to prevent buffer overflow (NFC) (PR #94783)

2024-07-24 Thread David Spickett via lldb-commits


https://github.com/DavidSpickett requested changes to this pull request.

> The field this is consuming is actually 17 bytes long, because the process 
> name is in parenthesis.

Ok then I am confused how this ever worked, but it sounds like scanf was never 
a great way to do this anyway?

https://github.com/llvm/llvm-project/pull/94783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correct format specifier for sscanf to prevent buffer overflow (NFC) (PR #94783)

2024-07-24 Thread Pavel Labath via lldb-commits

labath wrote:

> > The field this is consuming is actually 17 bytes long, because the process 
> > name is in parenthesis.
> 
> Ok then I am confused how this ever worked, but it sounds like scanf was 
> never a great way to do this anyway?

The field it's overwriting is in a struct, so it has a lot of headroom for 
"safely" overflowing without hitting anything important. And since the the 
other fields are parsed after the string field, they probably just immediately 
overwrite the corrupted data.

https://github.com/llvm/llvm-project/pull/94783
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [llvm]Added lib/Telemetry (PR #98528)

2024-07-24 Thread Vy Nguyen via lldb-commits



@@ -0,0 +1,153 @@
+#ifndef LLDB_CORE_TELEMETRY_H
+#define LLDB_CORE_TELEMETRY_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+using namespace llvm::telemetry;
+
+namespace lldb_private {
+
+struct DebuggerTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  std::string username;
+  std::string lldb_git_sha;
+  std::string lldb_path;
+  std::string cwd;
+
+  std::string ToString() const override;
+};
+
+struct TargetTelemetryInfo : public ::llvm::telemetry::TelemetryInfo {
+  // All entries emitted for the same SBTarget will have the same
+  // target_uuid.
+  std::string target_uuid;

oontvoo wrote:

This UUID is the same as the executable module's UUID.
If the process does execve, then a new set of TargetTelemetryInfo will be 
created for the new UUID.

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add an assert to verify sign_bit_pos is within the valid range (NFC) (PR #95678)

2024-07-24 Thread Shivam Gupta via lldb-commits


https://github.com/xgupta updated 
https://github.com/llvm/llvm-project/pull/95678

>From ef5b9cefb408ca3a721c95d8f6702abba77a602b Mon Sep 17 00:00:00 2001
From: Shivam Gupta 
Date: Sun, 16 Jun 2024 00:21:51 +0530
Subject: [PATCH 1/3] [LLDB] Add an assert to verify sign_bit_pos is within the
 valid range

---
 lldb/source/Utility/Scalar.cpp | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index c680101aa9efa..6e2f1ca4c1613 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -745,26 +745,25 @@ Status Scalar::SetValueFromData(const DataExtractor &data,
 
 bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
+  assert(sign_bit_pos < max_bit_pos);
 
-  if (sign_bit_pos < max_bit_pos) {
-switch (m_type) {
-case Scalar::e_void:
-case Scalar::e_float:
-  return false;
+  switch (m_type) {
+  case Scalar::e_void:
+  case Scalar::e_float:
+return false;
 
-case Scalar::e_int:
-  if (sign_bit_pos < (max_bit_pos - 1)) {
-llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
-llvm::APInt bitwize_and = m_integer & sign_bit;
-if (bitwize_and.getBoolValue()) {
-  llvm::APInt mask =
-  ~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
-  m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
-}
-return true;
+  case Scalar::e_int:
+if (sign_bit_pos < (max_bit_pos - 1)) {
+  llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
+  llvm::APInt bitwize_and = m_integer & sign_bit;
+  if (bitwize_and.getBoolValue()) {
+llvm::APInt mask =
+~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
+m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
   }
-  break;
+  return true;
 }
+break;
   }
   return false;
 }

>From 98a183742e3dc6393aa55000fff1931263ff165a Mon Sep 17 00:00:00 2001
From: Shivam Gupta 
Date: Wed, 19 Jun 2024 13:57:55 +0530
Subject: [PATCH 2/3] address review suggestion

---
 lldb/source/Utility/Scalar.cpp | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 6e2f1ca4c1613..496f402a74114 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -747,25 +747,17 @@ bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
   assert(sign_bit_pos < max_bit_pos);
 
-  switch (m_type) {
-  case Scalar::e_void:
-  case Scalar::e_float:
+  if (m_type != Scalar::e_int || sign_bit_pos >= (max_bit_pos - 1)) {
 return false;
+  }
 
-  case Scalar::e_int:
-if (sign_bit_pos < (max_bit_pos - 1)) {
-  llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
-  llvm::APInt bitwize_and = m_integer & sign_bit;
-  if (bitwize_and.getBoolValue()) {
-llvm::APInt mask =
-~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
-m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
-  }
-  return true;
-}
-break;
+  llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
+  llvm::APInt bitwize_and = m_integer & sign_bit;
+  if (bitwize_and.getBoolValue()) {
+llvm::APInt mask = ~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
+m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
   }
-  return false;
+  return true;
 }
 
 size_t Scalar::GetAsMemoryData(void *dst, size_t dst_len,

>From 5fbf491d0781c1dcce6bdc727844e75c527f6473 Mon Sep 17 00:00:00 2001
From: Shivam Gupta 
Date: Wed, 24 Jul 2024 14:36:15 +0200
Subject: [PATCH 3/3] adjust assert to fix
 SymbolFile/DWARF/debug-types-expressions.test

---
 lldb/source/Utility/Scalar.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 496f402a74114..42247f658da13 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -745,8 +745,8 @@ Status Scalar::SetValueFromData(const DataExtractor &data,
 
 bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
-  assert(sign_bit_pos < max_bit_pos);
 
+  assert(sign_bit_pos <= max_bit_pos);
   if (m_type != Scalar::e_int || sign_bit_pos >= (max_bit_pos - 1)) {
 return false;
   }

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [compiler-rt] [libc] [libcxx] [lld] [lldb] [llvm] [mlir] [BOLT][NFC] Track fragment relationships using EquivalenceClasses (PR #99979)

2024-07-24 Thread Amir Ayupov via lldb-commits


https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/99979
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [compiler-rt] [libc] [libcxx] [lld] [lldb] [llvm] [mlir] [BOLT][NFC] Track fragment relationships using EquivalenceClasses (PR #99979)

2024-07-24 Thread Amir Ayupov via lldb-commits


https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/99979
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [compiler-rt] [libc] [libcxx] [lld] [lldb] [llvm] [mlir] [BOLT] Support more than two jump table parents (PR #99988)

2024-07-24 Thread Amir Ayupov via lldb-commits


https://github.com/aaupov edited https://github.com/llvm/llvm-project/pull/99988
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [compiler-rt] [libc] [libcxx] [lld] [lldb] [llvm] [mlir] [BOLT] Support more than two jump table parents (PR #99988)

2024-07-24 Thread Amir Ayupov via lldb-commits


https://github.com/aaupov closed https://github.com/llvm/llvm-project/pull/99988
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][Linux] Parse, but don't store "comm" from /proc/stat file (PR #100387)

2024-07-24 Thread David Spickett via lldb-commits


https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/100387

As reported in https://github.com/llvm/llvm-project/issues/89710, the %s code 
used for `comm` could and probably does, overflow the buffer. Likely we haven't 
seen it cause problems because the following data is overwritten right 
afterwards.

Also scanf isn't a great choice here as this `comm` can include many characters 
that might trip up %s.

We don't actually use `comm`, so parse but don't store it so we're not 
overflowing anything.

>From 0e65e0e28cc4a20fbe91d3a7a76b7a0ed671e0cc Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Wed, 24 Jul 2024 14:02:47 +
Subject: [PATCH] [lldb][Linux] Parse, but don't store "comm" from /proc/stat
 file

As reported in https://github.com/llvm/llvm-project/issues/89710, the %s
code used for `comm` could and probably does, overflow the buffer.
Likely we haven't seen it cause problems because the following data
is overwritten right afterwards.

Also scanf isn't a great choice here as this `comm` can include many characters
that might trip up %s.

We don't actually use `comm`, so parse but don't store it so we're
not overflowing anything.
---
 lldb/source/Host/linux/Host.cpp | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 5545f9ef4d70e..0bc736d90ea76 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -51,11 +51,9 @@ enum class ProcessState {
   Zombie,
 };
 
-constexpr int task_comm_len = 16;
-
 struct StatFields {
   ::pid_t pid = LLDB_INVALID_PROCESS_ID;
-  char comm[task_comm_len];
+  // comm
   char state;
   ::pid_t ppid = LLDB_INVALID_PROCESS_ID;
   ::pid_t pgrp = LLDB_INVALID_PROCESS_ID;
@@ -100,8 +98,8 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
&ProcessInfo,
   StatFields stat_fields;
   if (sscanf(
   Rest.data(),
-  "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld %ld",
-  &stat_fields.pid, stat_fields.comm, &stat_fields.state,
+  "%d %*s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld 
%ld",
+  &stat_fields.pid, /* comm, */ &stat_fields.state,
   &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session,
   &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags,
   &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt,

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][Linux] Parse, but don't store "comm" from /proc/stat file (PR #100387)

2024-07-24 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

As reported in https://github.com/llvm/llvm-project/issues/89710, the %s code 
used for `comm` could and probably does, overflow the buffer. Likely we haven't 
seen it cause problems because the following data is overwritten right 
afterwards.

Also scanf isn't a great choice here as this `comm` can include many characters 
that might trip up %s.

We don't actually use `comm`, so parse but don't store it so we're not 
overflowing anything.

---
Full diff: https://github.com/llvm/llvm-project/pull/100387.diff


1 Files Affected:

- (modified) lldb/source/Host/linux/Host.cpp (+3-5) 


``diff
diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 5545f9ef4d70e..0bc736d90ea76 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -51,11 +51,9 @@ enum class ProcessState {
   Zombie,
 };
 
-constexpr int task_comm_len = 16;
-
 struct StatFields {
   ::pid_t pid = LLDB_INVALID_PROCESS_ID;
-  char comm[task_comm_len];
+  // comm
   char state;
   ::pid_t ppid = LLDB_INVALID_PROCESS_ID;
   ::pid_t pgrp = LLDB_INVALID_PROCESS_ID;
@@ -100,8 +98,8 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
&ProcessInfo,
   StatFields stat_fields;
   if (sscanf(
   Rest.data(),
-  "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld %ld",
-  &stat_fields.pid, stat_fields.comm, &stat_fields.state,
+  "%d %*s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld 
%ld",
+  &stat_fields.pid, /* comm, */ &stat_fields.state,
   &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session,
   &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags,
   &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt,

``




https://github.com/llvm/llvm-project/pull/100387
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [compiler-rt] [libc] [libcxx] [lld] [lldb] [llvm] [mlir] [BOLT] Add profile-use-pseudo-probes option (PR #100299)

2024-07-24 Thread Amir Ayupov via lldb-commits


https://github.com/aaupov edited 
https://github.com/llvm/llvm-project/pull/100299
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [compiler-rt] [libc] [libcxx] [lld] [lldb] [llvm] [mlir] [BOLT] Add profile-use-pseudo-probes option (PR #100299)

2024-07-24 Thread Amir Ayupov via lldb-commits


https://github.com/aaupov closed 
https://github.com/llvm/llvm-project/pull/100299
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (PR #100421)

2024-07-24 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/100421

Prevent passing a nullptr to std::string::insert in
ObjectFileMachO::GetDependentModules. Calling GetCString on an empty
ConstString will return a nullptr, which is undefined behavior. Instead,
use the GetString helper which will return an empty string in that case.

rdar://132388027

>From c114b172f0c984396928ea59022aa74767e6ef84 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 24 Jul 2024 09:32:15 -0700
Subject: [PATCH 1/2] [lldb] Use an early return in
 ObjectFileMachO::GetDependentModules (NFC)

---
 .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 205 +-
 1 file changed, 103 insertions(+), 102 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 2c7005449f9d7..8f444b4adf633 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5116,123 +5116,124 @@ UUID ObjectFileMachO::GetUUID() {
 }
 
 uint32_t ObjectFileMachO::GetDependentModules(FileSpecList &files) {
+  ModuleSP module_sp = GetModule();
+  if (!module_sp)
+return 0;
+
   uint32_t count = 0;
-  ModuleSP module_sp(GetModule());
-  if (module_sp) {
-std::lock_guard guard(module_sp->GetMutex());
-llvm::MachO::load_command load_cmd;
-lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
-std::vector rpath_paths;
-std::vector rpath_relative_paths;
-std::vector at_exec_relative_paths;
-uint32_t i;
-for (i = 0; i < m_header.ncmds; ++i) {
-  const uint32_t cmd_offset = offset;
-  if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
-break;
+  std::lock_guard guard(module_sp->GetMutex());
+  llvm::MachO::load_command load_cmd;
+  lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+  std::vector rpath_paths;
+  std::vector rpath_relative_paths;
+  std::vector at_exec_relative_paths;
+  uint32_t i;
+  for (i = 0; i < m_header.ncmds; ++i) {
+const uint32_t cmd_offset = offset;
+if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
+  break;
 
-  switch (load_cmd.cmd) {
-  case LC_RPATH:
-  case LC_LOAD_DYLIB:
-  case LC_LOAD_WEAK_DYLIB:
-  case LC_REEXPORT_DYLIB:
-  case LC_LOAD_DYLINKER:
-  case LC_LOADFVMLIB:
-  case LC_LOAD_UPWARD_DYLIB: {
-uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
-// For LC_LOAD_DYLIB there is an alternate encoding
-// which adds a uint32_t `flags` field for `DYLD_USE_*`
-// flags.  This can be detected by a timestamp field with
-// the `DYLIB_USE_MARKER` constant value.
-bool is_delayed_init = false;
-uint32_t use_command_marker = m_data.GetU32(&offset);
-if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
-  offset += 4; /* uint32_t current_version */
-  offset += 4; /* uint32_t compat_version */
-  uint32_t flags = m_data.GetU32(&offset);
-  // If this LC_LOAD_DYLIB is marked delay-init,
-  // don't report it as a dependent library -- it
-  // may be loaded in the process at some point,
-  // but will most likely not be load at launch.
-  if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
-is_delayed_init = true;
-}
-const char *path = m_data.PeekCStr(name_offset);
-if (path && !is_delayed_init) {
-  if (load_cmd.cmd == LC_RPATH)
-rpath_paths.push_back(path);
-  else {
-if (path[0] == '@') {
-  if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
-rpath_relative_paths.push_back(path + strlen("@rpath"));
-  else if (strncmp(path, "@executable_path",
-   strlen("@executable_path")) == 0)
-at_exec_relative_paths.push_back(path +
- strlen("@executable_path"));
-} else {
-  FileSpec file_spec(path);
-  if (files.AppendIfUnique(file_spec))
-count++;
-}
+switch (load_cmd.cmd) {
+case LC_RPATH:
+case LC_LOAD_DYLIB:
+case LC_LOAD_WEAK_DYLIB:
+case LC_REEXPORT_DYLIB:
+case LC_LOAD_DYLINKER:
+case LC_LOADFVMLIB:
+case LC_LOAD_UPWARD_DYLIB: {
+  uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
+  // For LC_LOAD_DYLIB there is an alternate encoding
+  // which adds a uint32_t `flags` field for `DYLD_USE_*`
+  // flags.  This can be detected by a timestamp field with
+  // the `DYLIB_USE_MARKER` constant value.
+  bool is_delayed_init = false;
+  uint32_t use_command_marker = m_data.GetU32(&offset);
+  if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
+offset += 4; /* uint32_t current_version */
+offset += 4; /* uin

[Lldb-commits] [lldb] [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (PR #100421)

2024-07-24 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

Prevent passing a nullptr to std::string::insert in
ObjectFileMachO::GetDependentModules. Calling GetCString on an empty
ConstString will return a nullptr, which is undefined behavior. Instead,
use the GetString helper which will return an empty string in that case.

rdar://132388027

---
Full diff: https://github.com/llvm/llvm-project/pull/100421.diff


1 Files Affected:

- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
(+104-103) 


``diff
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 2c7005449f9d7..ce095bcc48374 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -137,6 +137,9 @@ using namespace lldb;
 using namespace lldb_private;
 using namespace llvm::MachO;
 
+static constexpr llvm::StringLiteral g_loader_path = "@loader_path";
+static constexpr llvm::StringLiteral g_executable_path = "@executable_path";
+
 LLDB_PLUGIN_DEFINE(ObjectFileMachO)
 
 static void PrintRegisterValue(RegisterContext *reg_ctx, const char *name,
@@ -5116,123 +5119,121 @@ UUID ObjectFileMachO::GetUUID() {
 }
 
 uint32_t ObjectFileMachO::GetDependentModules(FileSpecList &files) {
+  ModuleSP module_sp = GetModule();
+  if (!module_sp)
+return 0;
+
   uint32_t count = 0;
-  ModuleSP module_sp(GetModule());
-  if (module_sp) {
-std::lock_guard guard(module_sp->GetMutex());
-llvm::MachO::load_command load_cmd;
-lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
-std::vector rpath_paths;
-std::vector rpath_relative_paths;
-std::vector at_exec_relative_paths;
-uint32_t i;
-for (i = 0; i < m_header.ncmds; ++i) {
-  const uint32_t cmd_offset = offset;
-  if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
-break;
+  std::lock_guard guard(module_sp->GetMutex());
+  llvm::MachO::load_command load_cmd;
+  lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+  std::vector rpath_paths;
+  std::vector rpath_relative_paths;
+  std::vector at_exec_relative_paths;
+  uint32_t i;
+  for (i = 0; i < m_header.ncmds; ++i) {
+const uint32_t cmd_offset = offset;
+if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
+  break;
 
-  switch (load_cmd.cmd) {
-  case LC_RPATH:
-  case LC_LOAD_DYLIB:
-  case LC_LOAD_WEAK_DYLIB:
-  case LC_REEXPORT_DYLIB:
-  case LC_LOAD_DYLINKER:
-  case LC_LOADFVMLIB:
-  case LC_LOAD_UPWARD_DYLIB: {
-uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
-// For LC_LOAD_DYLIB there is an alternate encoding
-// which adds a uint32_t `flags` field for `DYLD_USE_*`
-// flags.  This can be detected by a timestamp field with
-// the `DYLIB_USE_MARKER` constant value.
-bool is_delayed_init = false;
-uint32_t use_command_marker = m_data.GetU32(&offset);
-if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
-  offset += 4; /* uint32_t current_version */
-  offset += 4; /* uint32_t compat_version */
-  uint32_t flags = m_data.GetU32(&offset);
-  // If this LC_LOAD_DYLIB is marked delay-init,
-  // don't report it as a dependent library -- it
-  // may be loaded in the process at some point,
-  // but will most likely not be load at launch.
-  if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
-is_delayed_init = true;
-}
-const char *path = m_data.PeekCStr(name_offset);
-if (path && !is_delayed_init) {
-  if (load_cmd.cmd == LC_RPATH)
-rpath_paths.push_back(path);
-  else {
-if (path[0] == '@') {
-  if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
-rpath_relative_paths.push_back(path + strlen("@rpath"));
-  else if (strncmp(path, "@executable_path",
-   strlen("@executable_path")) == 0)
-at_exec_relative_paths.push_back(path +
- strlen("@executable_path"));
-} else {
-  FileSpec file_spec(path);
-  if (files.AppendIfUnique(file_spec))
-count++;
-}
+switch (load_cmd.cmd) {
+case LC_RPATH:
+case LC_LOAD_DYLIB:
+case LC_LOAD_WEAK_DYLIB:
+case LC_REEXPORT_DYLIB:
+case LC_LOAD_DYLINKER:
+case LC_LOADFVMLIB:
+case LC_LOAD_UPWARD_DYLIB: {
+  uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
+  // For LC_LOAD_DYLIB there is an alternate encoding
+  // which adds a uint32_t `flags` field for `DYLD_USE_*`
+  // flags.  This can be detected by a timestamp field with
+  // the `DYLIB_USE_MARKER` constant value.
+  bool is_delayed_init = false;
+

[Lldb-commits] [lldb] [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (PR #100421)

2024-07-24 Thread Jonas Devlieghere via lldb-commits


JDevlieghere wrote:

This PR looks scarier than it is because it contains an NFC commit that 
introduces an early return to save a level of indentation. I recommend 
reviewing the two commits separately and/or ignoring whitespace changes. 

https://github.com/llvm/llvm-project/pull/100421
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix incorrect logical operator in 'if' condition check (NFC) (PR #94779)

2024-07-24 Thread Alex Langford via lldb-commits


https://github.com/bulbazord approved this pull request.

The change looks good to me. Please make sure you update the commit message so 
it accurately reflects the change since there were a few revisions. :)

https://github.com/llvm/llvm-project/pull/94779
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add an assert to verify sign_bit_pos is within the valid range (NFC) (PR #95678)

2024-07-24 Thread Alex Langford via lldb-commits


https://github.com/bulbazord requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/95678
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add an assert to verify sign_bit_pos is within the valid range (NFC) (PR #95678)

2024-07-24 Thread Alex Langford via lldb-commits



@@ -746,27 +746,18 @@ Status Scalar::SetValueFromData(const DataExtractor &data,
 bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
 
-  if (sign_bit_pos < max_bit_pos) {
-switch (m_type) {
-case Scalar::e_void:
-case Scalar::e_float:
-  return false;
+  assert(sign_bit_pos <= max_bit_pos);
+  if (m_type != Scalar::e_int || sign_bit_pos >= (max_bit_pos - 1)) {

bulbazord wrote:

LLVM's Style guidelines state that single-statement blocks for `if/for/while` 
should not have braces.

https://github.com/llvm/llvm-project/pull/95678
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add an assert to verify sign_bit_pos is within the valid range (NFC) (PR #95678)

2024-07-24 Thread Alex Langford via lldb-commits



@@ -746,27 +746,18 @@ Status Scalar::SetValueFromData(const DataExtractor &data,
 bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
 
-  if (sign_bit_pos < max_bit_pos) {
-switch (m_type) {
-case Scalar::e_void:
-case Scalar::e_float:
-  return false;
+  assert(sign_bit_pos <= max_bit_pos);

bulbazord wrote:

I'm a little unsure about this assert. Are there any ways we can get to this 
assertion through user input?
The situation I would like to avoid is somebody typing something into LLDB or 
loading some invalid DWARF that causes this assertion to trigger. Certainly we 
would like to know about ways this invariant can be broken, but developers 
trying to debug generally do not appreciate it when their debug session dies 
for a reason they do not understand or do not control.

cc @JDevlieghere I remember you had some guidance around this involving 
`lldbassert`?

https://github.com/llvm/llvm-project/pull/95678
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (PR #100421)

2024-07-24 Thread Alex Langford via lldb-commits


https://github.com/bulbazord approved this pull request.

LGTM.

It becomes less scary to review if you use Github's "Ignore Whitespace" feature 
in its code review tool. I wish it was more discoverable. :)

https://github.com/llvm/llvm-project/pull/100421
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jacob Lalonde via lldb-commits


https://github.com/Jlalond created 
https://github.com/llvm/llvm-project/pull/100443

In #98403 I enabled the SBSaveCoreOptions object, which allows users via the 
scripting API to define what they want saved into their core file. As the first 
option I've added a threadlist, so users can scan and identify which threads 
and corresponding stacks they want to save.

In order to support this, I had to add a new method to `Process.h` on how we 
identify which threads are to be saved, and I had to change the book keeping in 
minidump to ensure we don't double save the stacks.

Important to @jasonmolenda I also changed the MachO coredump to accept these 
new APIs.

>From 02bcc268831f935a6377a3f32c36683dd66967c1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Tue, 23 Jul 2024 16:50:57 -0700
Subject: [PATCH 1/2] Implemplement a thread list that is currently unused for
 SBSaveCore. Run formatter

---
 lldb/include/lldb/API/SBSaveCoreOptions.h  | 24 +++
 lldb/include/lldb/Symbol/SaveCoreOptions.h | 10 +
 lldb/include/lldb/Target/Process.h |  5 +++
 lldb/source/API/SBSaveCoreOptions.cpp  | 20 +
 lldb/source/Core/PluginManager.cpp |  4 ++
 lldb/source/Symbol/SaveCoreOptions.cpp | 48 ++
 lldb/source/Target/Process.cpp | 12 ++
 7 files changed, 123 insertions(+)

diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h 
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index e77496bd3a4a0..b485371ce8f55 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);
+
+  /// Remove a thread from the list of threads to save.
+  ///
+  /// \param thread_id The thread ID to remove.
+  /// \return True if the thread was removed, false if it was not in the list.
+  bool RemoveThread(lldb::tid_t thread_id);
+
+  /// Get the number of threads to save. If this list is empty all threads will
+  /// be saved.
+  ///
+  /// \return The number of threads to save.
+  uint32_t GetNumThreads() const;
+
+  /// Get the thread ID at the given index.
+  ///
+  /// \param[in] index The index of the thread ID to get.
+  /// \return The thread ID at the given index, or an error
+  /// if there is no thread at the index.
+  lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const;
+
   /// Reset all options.
   void Clear();
 
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h 
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 583bc1f483d04..d583b32b29508 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -14,6 +14,7 @@
 #include "lldb/lldb-types.h"
 
 #include 
+#include 
 #include 
 
 namespace lldb_private {
@@ -32,12 +33,21 @@ class SaveCoreOptions {
   void SetOutputFile(lldb_private::FileSpec file);
   const std::optional GetOutputFile() const;
 
+  void AddThread(lldb::tid_t tid);
+  bool RemoveThread(lldb::tid_t tid);
+  size_t GetNumThreads() const;
+  int64_t GetThreadAtIndex(size_t index) const;
+  bool ShouldSaveThread(lldb::tid_t tid) const;
+
+  Status EnsureValidConfiguration() const;
+
   void Clear();
 
 private:
   std::optional m_plugin_name;
   std::optional m_file;
   std::optional m_style;
+  std::set m_threads_to_save;
 };
 } // namespace lldb_private
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..c551504c8583f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -741,6 +741,11 @@ class Process : public 
std::enable_shared_from_this,
   Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
  CoreFileMemoryRanges &ranges);
 
+  /// Helper function for Process::SaveCore(...) that calculates the thread 
list
+  /// based upon options set within a given \a core_options object.
+  ThreadCollection::ThreadIterable
+  CalculateCoreFileThreadList(SaveCoreOptions &core_options);
+
 protected:
   virtual JITLoaderList &GetJITLoaders();
 
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp 
b/lldb/source/API/SBSaveCoreOptions.cpp
index 6c3f74596203d..1d45313d2426a 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -75,6 +75,26 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const {
   return m_opaque_up->GetStyle();
 }
 
+void SBSaveCoreOptions::AddThread(lldb::tid_t tid) {
+  m_opaque_up->AddThread(tid);
+}
+
+bool SBSaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  return m_opaque_up->RemoveThread(tid);
+}
+
+uint32_t SBSaveCoreOptions::GetNumThreads() const {
+  return m_opaque_up->GetNumThreads();
+}
+
+lldb::tid_t SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx,
+

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)


Changes

In #98403 I enabled the SBSaveCoreOptions object, which allows users 
via the scripting API to define what they want saved into their core file. As 
the first option I've added a threadlist, so users can scan and identify which 
threads and corresponding stacks they want to save.

In order to support this, I had to add a new method to `Process.h` on how we 
identify which threads are to be saved, and I had to change the book keeping in 
minidump to ensure we don't double save the stacks.

Important to @jasonmolenda I also changed the MachO coredump to accept 
these new APIs.

---

Patch is 20.56 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/100443.diff


11 Files Affected:

- (modified) lldb/include/lldb/API/SBSaveCoreOptions.h (+24) 
- (modified) lldb/include/lldb/Symbol/SaveCoreOptions.h (+10) 
- (modified) lldb/include/lldb/Target/Process.h (+6-1) 
- (modified) lldb/source/API/SBSaveCoreOptions.cpp (+20) 
- (modified) lldb/source/Core/PluginManager.cpp (+4) 
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+6-5) 
- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
(+27-29) 
- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h 
(+7-3) 
- (modified) lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
(+3-2) 
- (modified) lldb/source/Symbol/SaveCoreOptions.cpp (+51) 
- (modified) lldb/source/Target/Process.cpp (+25-5) 


``diff
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h 
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index e77496bd3a4a0..b485371ce8f55 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);
+
+  /// Remove a thread from the list of threads to save.
+  ///
+  /// \param thread_id The thread ID to remove.
+  /// \return True if the thread was removed, false if it was not in the list.
+  bool RemoveThread(lldb::tid_t thread_id);
+
+  /// Get the number of threads to save. If this list is empty all threads will
+  /// be saved.
+  ///
+  /// \return The number of threads to save.
+  uint32_t GetNumThreads() const;
+
+  /// Get the thread ID at the given index.
+  ///
+  /// \param[in] index The index of the thread ID to get.
+  /// \return The thread ID at the given index, or an error
+  /// if there is no thread at the index.
+  lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const;
+
   /// Reset all options.
   void Clear();
 
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h 
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 583bc1f483d04..d583b32b29508 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -14,6 +14,7 @@
 #include "lldb/lldb-types.h"
 
 #include 
+#include 
 #include 
 
 namespace lldb_private {
@@ -32,12 +33,21 @@ class SaveCoreOptions {
   void SetOutputFile(lldb_private::FileSpec file);
   const std::optional GetOutputFile() const;
 
+  void AddThread(lldb::tid_t tid);
+  bool RemoveThread(lldb::tid_t tid);
+  size_t GetNumThreads() const;
+  int64_t GetThreadAtIndex(size_t index) const;
+  bool ShouldSaveThread(lldb::tid_t tid) const;
+
+  Status EnsureValidConfiguration() const;
+
   void Clear();
 
 private:
   std::optional m_plugin_name;
   std::optional m_file;
   std::optional m_style;
+  std::set m_threads_to_save;
 };
 } // namespace lldb_private
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..ef3907154c20f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -738,9 +738,14 @@ class Process : public 
std::enable_shared_from_this,
   /// Helper function for Process::SaveCore(...) that calculates the address
   /// ranges that should be saved. This allows all core file plug-ins to save
   /// consistent memory ranges given a \a core_style.
-  Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
+  Status CalculateCoreFileSaveRanges(const SaveCoreOptions &core_options,
  CoreFileMemoryRanges &ranges);
 
+  /// Helper function for Process::SaveCore(...) that calculates the thread 
list
+  /// based upon options set within a given \a core_options object.
+  std::vector
+  CalculateCoreFileThreadList(const SaveCoreOptions &core_options);
+
 protected:
   virtual JITLoaderList &GetJITLoaders();
 
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp 
b/lldb/source/API/SBSaveCoreOptions.cpp
index 6c3f74596203d..1d45313d2426a 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOp

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread via lldb-commits


github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 0ee0eeb4bb9be6aeef6c84121ca1af463840fb6a 
17ff879cd22452fca73c0d4790ebe66c0145ae2a --extensions h,cpp -- 
lldb/include/lldb/API/SBSaveCoreOptions.h 
lldb/include/lldb/Symbol/SaveCoreOptions.h lldb/include/lldb/Target/Process.h 
lldb/source/API/SBSaveCoreOptions.cpp lldb/source/Core/PluginManager.cpp 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h 
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
lldb/source/Symbol/SaveCoreOptions.cpp lldb/source/Target/Process.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index c039492aa5..2e97f3e2fd 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -78,7 +78,7 @@ public:
   const lldb::ProcessSP &process_sp,
   const lldb_private::SaveCoreOptions &save_core_options)
   : m_process_sp(process_sp), m_core_file(std::move(core_file)),
-m_save_core_options(save_core_options){};
+m_save_core_options(save_core_options) {};
 
   MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
   MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;

``




https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits


https://github.com/clayborg commented:

Update the public API to user SBThread instead of the thread IDs and then add 
some tests.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);

clayborg wrote:

Do we want to do things by `lldb::tid_t` or do we want to pass in a 
`lldb::SBThread`? The reason I mention this is there are two integer 
identifiers from a SBThread:
```
  lldb::tid_t lldb::SBThread::GetThreadID() const;
  uint32_t lldb::SBThread::GetIndexID() const;
```
This API could and probably should be:
```
void AddThread(lldb::SBThread thread);
```
Because in order to get a lldb::tid_t users would need to have a SBThread 
already anyway. Then the user can't mess up by specifying the wrong ID (index 
ID instead of `lldb::tid_t`)

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits


https://github.com/clayborg edited 
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -32,12 +33,21 @@ class SaveCoreOptions {
   void SetOutputFile(lldb_private::FileSpec file);
   const std::optional GetOutputFile() const;
 
+  void AddThread(lldb::tid_t tid);
+  bool RemoveThread(lldb::tid_t tid);
+  size_t GetNumThreads() const;
+  int64_t GetThreadAtIndex(size_t index) const;

clayborg wrote:

This should be:
```
lldb::tid_t GetThreadAtIndex(size_t index) const;
```

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);
+
+  /// Remove a thread from the list of threads to save.
+  ///
+  /// \param thread_id The thread ID to remove.
+  /// \return True if the thread was removed, false if it was not in the list.
+  bool RemoveThread(lldb::tid_t thread_id);
+
+  /// Get the number of threads to save. If this list is empty all threads will
+  /// be saved.
+  ///
+  /// \return The number of threads to save.
+  uint32_t GetNumThreads() const;
+
+  /// Get the thread ID at the given index.
+  ///
+  /// \param[in] index The index of the thread ID to get.
+  /// \return The thread ID at the given index, or an error
+  /// if there is no thread at the index.
+  lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const;

clayborg wrote:

returning a `lldb::SBThread`

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+  }
+
+  return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+  return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+  auto iter = m_threads_to_save.begin();
+  while (index >= 0 && iter != m_threads_to_save.end()) {
+if (index == 0)
+  return *iter;
+index--;
+iter++;
+  }
+
+  return -1;
+}
+
+bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+  // If the user specified no threads to save, then we save all threads.
+  if (m_threads_to_save.empty())
+return true;
+  return m_threads_to_save.count(tid) > 0;
+}
+
+Status SaveCoreOptions::EnsureValidConfiguration() const {
+  Status error;
+  std::string error_str;
+  if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull) {
+error_str += "Cannot save a full core with a subset of threads\n";
+  }

clayborg wrote:

Remove `{}` on single line if statement per llvm coding guidelines

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);

clayborg wrote:

No need to check the count, and no need to use `emplace` when we have a 
`std::set` and the type `T` is an integer:
```
m_threads_to_save.insert(tid);
```


https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+  }

clayborg wrote:

FYI this would fail to remove the tid because you have 
`m_threads_to_save.count(tid) == 0` instead of `m_threads_to_save.count(tid) != 
0`, but see the one line replacement code that will work below.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);
+
+  /// Remove a thread from the list of threads to save.
+  ///
+  /// \param thread_id The thread ID to remove.
+  /// \return True if the thread was removed, false if it was not in the list.
+  bool RemoveThread(lldb::tid_t thread_id);

clayborg wrote:

Use `lldb::SBThread` instead of `lldb::tid_t`

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+  }
+
+  return false;

clayborg wrote:

erase returns a bool if something was erased, lines 55-60 can just be:
```
  return m_threads_to_save.erase(tid) > 0;
```

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+  }
+
+  return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+  return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+  auto iter = m_threads_to_save.begin();
+  while (index >= 0 && iter != m_threads_to_save.end()) {
+if (index == 0)
+  return *iter;
+index--;
+iter++;
+  }

clayborg wrote:

If this list is large, then this function can be quite inefficient. But 
probably ok for now.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -32,12 +33,21 @@ class SaveCoreOptions {
   void SetOutputFile(lldb_private::FileSpec file);
   const std::optional GetOutputFile() const;
 
+  void AddThread(lldb::tid_t tid);
+  bool RemoveThread(lldb::tid_t tid);
+  size_t GetNumThreads() const;
+  int64_t GetThreadAtIndex(size_t index) const;
+  bool ShouldSaveThread(lldb::tid_t tid) const;
+
+  Status EnsureValidConfiguration() const;
+
   void Clear();
 
 private:
   std::optional m_plugin_name;
   std::optional m_file;
   std::optional m_style;
+  std::set m_threads_to_save;

clayborg wrote:

FYI: Totally fine to save things internally by lldb::tid_t since we know and 
control this code.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Greg Clayton via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+  }
+
+  return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+  return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+  auto iter = m_threads_to_save.begin();
+  while (index >= 0 && iter != m_threads_to_save.end()) {
+if (index == 0)
+  return *iter;
+index--;
+iter++;
+  }
+
+  return -1;
+}
+
+bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {
+  // If the user specified no threads to save, then we save all threads.
+  if (m_threads_to_save.empty())
+return true;
+  return m_threads_to_save.count(tid) > 0;
+}
+
+Status SaveCoreOptions::EnsureValidConfiguration() const {
+  Status error;
+  std::string error_str;
+  if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull) {
+error_str += "Cannot save a full core with a subset of threads\n";

clayborg wrote:

Should we allow "full" core files to be emitted without some thread stacks? We 
could allow "full" to mean save all memory regions except the thread stacks for 
any threads that were not in the list. This would allow core files to be a bit 
smaller, but still contain all mapped memory except the thread stacks we didn't 
want. We can emit a warning when saving a core file saying something to this 
effect like we do for "stacks" and "modified-memory"

The reason I say this is for core files for and Apple systems. If you save a 
`full` style, all mach-o binaries are fully mapped into memory and you would 
have everything you need to load the core file as all system libraries are 
mapped into memory, and it would be nice to be able to not save all thread 
stacks if you don't need them. So maybe turn this into a warning we can expose 
to the user

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jacob Lalonde via lldb-commits


https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/100443

>From d7940af06873cedf5976dc829dd9377b2851ae25 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Tue, 23 Jul 2024 16:50:57 -0700
Subject: [PATCH 1/5] Implemplement a thread list that is currently unused for
 SBSaveCore. Run formatter

---
 lldb/include/lldb/API/SBSaveCoreOptions.h  | 24 +++
 lldb/include/lldb/Symbol/SaveCoreOptions.h | 10 +
 lldb/include/lldb/Target/Process.h |  5 +++
 lldb/source/API/SBSaveCoreOptions.cpp  | 20 +
 lldb/source/Core/PluginManager.cpp |  4 ++
 lldb/source/Symbol/SaveCoreOptions.cpp | 48 ++
 lldb/source/Target/Process.cpp | 12 ++
 7 files changed, 123 insertions(+)

diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h 
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index e77496bd3a4a0..b485371ce8f55 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);
+
+  /// Remove a thread from the list of threads to save.
+  ///
+  /// \param thread_id The thread ID to remove.
+  /// \return True if the thread was removed, false if it was not in the list.
+  bool RemoveThread(lldb::tid_t thread_id);
+
+  /// Get the number of threads to save. If this list is empty all threads will
+  /// be saved.
+  ///
+  /// \return The number of threads to save.
+  uint32_t GetNumThreads() const;
+
+  /// Get the thread ID at the given index.
+  ///
+  /// \param[in] index The index of the thread ID to get.
+  /// \return The thread ID at the given index, or an error
+  /// if there is no thread at the index.
+  lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const;
+
   /// Reset all options.
   void Clear();
 
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h 
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 583bc1f483d04..d583b32b29508 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -14,6 +14,7 @@
 #include "lldb/lldb-types.h"
 
 #include 
+#include 
 #include 
 
 namespace lldb_private {
@@ -32,12 +33,21 @@ class SaveCoreOptions {
   void SetOutputFile(lldb_private::FileSpec file);
   const std::optional GetOutputFile() const;
 
+  void AddThread(lldb::tid_t tid);
+  bool RemoveThread(lldb::tid_t tid);
+  size_t GetNumThreads() const;
+  int64_t GetThreadAtIndex(size_t index) const;
+  bool ShouldSaveThread(lldb::tid_t tid) const;
+
+  Status EnsureValidConfiguration() const;
+
   void Clear();
 
 private:
   std::optional m_plugin_name;
   std::optional m_file;
   std::optional m_style;
+  std::set m_threads_to_save;
 };
 } // namespace lldb_private
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..c551504c8583f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -741,6 +741,11 @@ class Process : public 
std::enable_shared_from_this,
   Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
  CoreFileMemoryRanges &ranges);
 
+  /// Helper function for Process::SaveCore(...) that calculates the thread 
list
+  /// based upon options set within a given \a core_options object.
+  ThreadCollection::ThreadIterable
+  CalculateCoreFileThreadList(SaveCoreOptions &core_options);
+
 protected:
   virtual JITLoaderList &GetJITLoaders();
 
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp 
b/lldb/source/API/SBSaveCoreOptions.cpp
index 6c3f74596203d..1d45313d2426a 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -75,6 +75,26 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const {
   return m_opaque_up->GetStyle();
 }
 
+void SBSaveCoreOptions::AddThread(lldb::tid_t tid) {
+  m_opaque_up->AddThread(tid);
+}
+
+bool SBSaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  return m_opaque_up->RemoveThread(tid);
+}
+
+uint32_t SBSaveCoreOptions::GetNumThreads() const {
+  return m_opaque_up->GetNumThreads();
+}
+
+lldb::tid_t SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx,
+SBError &error) const {
+  int64_t tid = m_opaque_up->GetThreadAtIndex(idx);
+  if (tid == -1)
+error.SetErrorString("Invalid index");
+  return 0;
+}
+
 void SBSaveCoreOptions::Clear() {
   LLDB_INSTRUMENT_VA(this);
   m_opaque_up->Clear();
diff --git a/lldb/source/Core/PluginManager.cpp 
b/lldb/source/Core/PluginManager.cpp
index 759ef3a8afe02..94e3cb85f31b9 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -714,6 +714,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP 
&process_sp,

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jacob Lalonde via lldb-commits



@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);

Jlalond wrote:

I think SBThread, because it solves my issue where I'm returning a thread at an 
index but need to support an error case

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jacob Lalonde via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);

Jlalond wrote:

Ah, good catch. Originally I was using a vector but then moved to a set.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jacob Lalonde via lldb-commits



@@ -46,8 +46,59 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+void SaveCoreOptions::AddThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0)
+m_threads_to_save.emplace(tid);
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  if (m_threads_to_save.count(tid) == 0) {
+m_threads_to_save.erase(tid);
+return true;
+  }
+
+  return false;
+}
+
+size_t SaveCoreOptions::GetNumThreads() const {
+  return m_threads_to_save.size();
+}
+
+int64_t SaveCoreOptions::GetThreadAtIndex(size_t index) const {
+  auto iter = m_threads_to_save.begin();
+  while (index >= 0 && iter != m_threads_to_save.end()) {
+if (index == 0)
+  return *iter;
+index--;
+iter++;
+  }

Jlalond wrote:

Should we support returning at an index then? To be clear, I do not like this 
code but due to my limited C++ knowledge didn't know a better way to return the 
`Nth item of a set`

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Don't use a vm addr range starting at 0 for local memory (PR #100288)

2024-07-24 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere approved this pull request.


https://github.com/llvm/llvm-project/pull/100288
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)

2024-07-24 Thread Helena Kotas via lldb-commits


https://github.com/hekota edited https://github.com/llvm/llvm-project/pull/97362
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)

2024-07-24 Thread Helena Kotas via lldb-commits


https://github.com/hekota edited https://github.com/llvm/llvm-project/pull/97362
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing lldb/test/Shell/Drive… (PR #100473)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland created 
https://github.com/llvm/llvm-project/pull/100473

I'm currently working on getting the LLDB test suites to pass on Windows x86_64 
which is not currently included in LLVM CI. These tests are currently failing 
on this configuration.

>From 7c63b093423fd30e64cb08e770105eb9ed475005 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 15:18:13 -0700
Subject: [PATCH] [lldb][test][win][x86_64] XFAIL already failing
 lldb/test/Shell/Driver tests

---
 lldb/test/Shell/Driver/TestConvenienceVariables.test | 1 +
 lldb/test/Shell/Driver/TestSingleQuote.test  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lldb/test/Shell/Driver/TestConvenienceVariables.test 
b/lldb/test/Shell/Driver/TestConvenienceVariables.test
index 45dc7673bfc51..36ec0748a5a02 100644
--- a/lldb/test/Shell/Driver/TestConvenienceVariables.test
+++ b/lldb/test/Shell/Driver/TestConvenienceVariables.test
@@ -3,6 +3,7 @@ RUN: mkdir -p %t
 RUN: %build %p/Inputs/hello.cpp -o %t/target.out
 RUN: %lldb %t/target.out -s %p/Inputs/convenience.in -o quit | FileCheck %s
 
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 CHECK: stop reason = breakpoint 1.1
 CHECK: script print(lldb.debugger)
 CHECK-NEXT: Debugger (instance: {{.*}}, id: {{[0-9]+}})
diff --git a/lldb/test/Shell/Driver/TestSingleQuote.test 
b/lldb/test/Shell/Driver/TestSingleQuote.test
index af321ba04db39..5d721b5a3345c 100644
--- a/lldb/test/Shell/Driver/TestSingleQuote.test
+++ b/lldb/test/Shell/Driver/TestSingleQuote.test
@@ -2,5 +2,6 @@
 # RUN: %clang_host %p/Inputs/hello.c -g -o "%t-'pat"
 # RUN: %lldb -s %s "%t-'pat" | FileCheck %s
 
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 br set -p return
 # CHECK: Breakpoint 1: where = TestSingleQuote.test.tmp-'pat`main

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell/Driver tests (PR #100473)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland edited 
https://github.com/llvm/llvm-project/pull/100473
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell/Driver tests (PR #100473)

2024-07-24 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Kendal Harland (kendalharland)


Changes

I'm currently working on getting the LLDB test suites to pass on Windows x86_64 
which is not currently included in LLVM CI. These tests are currently failing 
on this configuration.

---
Full diff: https://github.com/llvm/llvm-project/pull/100473.diff


2 Files Affected:

- (modified) lldb/test/Shell/Driver/TestConvenienceVariables.test (+1) 
- (modified) lldb/test/Shell/Driver/TestSingleQuote.test (+1) 


``diff
diff --git a/lldb/test/Shell/Driver/TestConvenienceVariables.test 
b/lldb/test/Shell/Driver/TestConvenienceVariables.test
index 45dc7673bfc51..36ec0748a5a02 100644
--- a/lldb/test/Shell/Driver/TestConvenienceVariables.test
+++ b/lldb/test/Shell/Driver/TestConvenienceVariables.test
@@ -3,6 +3,7 @@ RUN: mkdir -p %t
 RUN: %build %p/Inputs/hello.cpp -o %t/target.out
 RUN: %lldb %t/target.out -s %p/Inputs/convenience.in -o quit | FileCheck %s
 
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 CHECK: stop reason = breakpoint 1.1
 CHECK: script print(lldb.debugger)
 CHECK-NEXT: Debugger (instance: {{.*}}, id: {{[0-9]+}})
diff --git a/lldb/test/Shell/Driver/TestSingleQuote.test 
b/lldb/test/Shell/Driver/TestSingleQuote.test
index af321ba04db39..5d721b5a3345c 100644
--- a/lldb/test/Shell/Driver/TestSingleQuote.test
+++ b/lldb/test/Shell/Driver/TestSingleQuote.test
@@ -2,5 +2,6 @@
 # RUN: %clang_host %p/Inputs/hello.c -g -o "%t-'pat"
 # RUN: %lldb -s %s "%t-'pat" | FileCheck %s
 
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 br set -p return
 # CHECK: Breakpoint 1: where = TestSingleQuote.test.tmp-'pat`main

``




https://github.com/llvm/llvm-project/pull/100473
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell/Driver tests (PR #100473)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland edited 
https://github.com/llvm/llvm-project/pull/100473
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell/Driver tests (PR #100473)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland edited 
https://github.com/llvm/llvm-project/pull/100473
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)

2024-07-24 Thread Helena Kotas via lldb-commits



@@ -8001,6 +8001,12 @@ NamedDecl *Sema::ActOnVariableDeclarator(
 }
   }
 
+  if (getLangOpts().HLSL) {
+if (R->isHLSLSpecificType() && !NewVD->isImplicit()) {
+  Diag(D.getBeginLoc(), diag::err_hlsl_intangible_type_cannot_be_declared);

hekota wrote:

Yes, the intention was that these types can be created only in the implicit 
header `hlsl.h`. If that's not the case, I will remove this.

https://github.com/llvm/llvm-project/pull/97362
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell tests (PR #100476)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland created 
https://github.com/llvm/llvm-project/pull/100476

I'm currently working on getting the LLDB test suites to pass on Windows x86_64 
which is not currently included in LLVM CI. These tests are currently failing 
in this configuration.

See https://github.com/llvm/llvm-project/issues/100474

>From f293beaaa7d5d0a4c07c5f7e8dd43be4d69984ef Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 15:47:48 -0700
Subject: [PATCH] [lldb][test][win][x86_64] XFAIL already failing Shell tests

---
 lldb/test/Shell/Driver/TestConvenienceVariables.test  | 1 +
 lldb/test/Shell/Driver/TestSingleQuote.test   | 1 +
 lldb/test/Shell/Recognizer/verbose_trap.test  | 4 
 lldb/test/Shell/Settings/TestEchoCommands.test| 1 +
 lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml | 2 ++
 lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp  | 2 ++
 lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp| 2 ++
 lldb/test/Shell/SymbolFile/PDB/ast-restore.test   | 1 +
 lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test   | 1 +
 lldb/test/Shell/SymbolFile/PDB/class-layout.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/enums-layout.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/expressions.test   | 1 +
 lldb/test/Shell/SymbolFile/PDB/func-symbols.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/pointers.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/type-quals.test| 1 +
 lldb/test/Shell/SymbolFile/PDB/typedefs.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/udt-layout.test| 1 +
 lldb/test/Shell/SymbolFile/PDB/variables.test | 1 +
 18 files changed, 24 insertions(+)

diff --git a/lldb/test/Shell/Driver/TestConvenienceVariables.test 
b/lldb/test/Shell/Driver/TestConvenienceVariables.test
index 45dc7673bfc51..32c3d160153fb 100644
--- a/lldb/test/Shell/Driver/TestConvenienceVariables.test
+++ b/lldb/test/Shell/Driver/TestConvenienceVariables.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 REQUIRES: python
 RUN: mkdir -p %t
 RUN: %build %p/Inputs/hello.cpp -o %t/target.out
diff --git a/lldb/test/Shell/Driver/TestSingleQuote.test 
b/lldb/test/Shell/Driver/TestSingleQuote.test
index af321ba04db39..49f6548e0c5b1 100644
--- a/lldb/test/Shell/Driver/TestSingleQuote.test
+++ b/lldb/test/Shell/Driver/TestSingleQuote.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 # Make sure lldb can handle filenames with single quotes in them.
 # RUN: %clang_host %p/Inputs/hello.c -g -o "%t-'pat"
 # RUN: %lldb -s %s "%t-'pat" | FileCheck %s
diff --git a/lldb/test/Shell/Recognizer/verbose_trap.test 
b/lldb/test/Shell/Recognizer/verbose_trap.test
index dafab7bdea688..76ba87f0f115a 100644
--- a/lldb/test/Shell/Recognizer/verbose_trap.test
+++ b/lldb/test/Shell/Recognizer/verbose_trap.test
@@ -1,4 +1,5 @@
 # UNSUPPORTED: system-windows
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 #
 # RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out 
-DVERBOSE_TRAP_TEST_CATEGORY=\"Foo\" -DVERBOSE_TRAP_TEST_MESSAGE=\"Bar\"
 # RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK,CHECK-BOTH
@@ -12,6 +13,9 @@
 # RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out 
-DVERBOSE_TRAP_TEST_CATEGORY=\"\" -DVERBOSE_TRAP_TEST_MESSAGE=\"\"
 # RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK,CHECK-NONE
 
+# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+
 run
 # CHECK-BOTH:  thread #{{.*}}stop reason = Foo: Bar
 # CHECK-MESSAGE_ONLY:  thread #{{.*}}stop reason = : Bar
diff --git a/lldb/test/Shell/Settings/TestEchoCommands.test 
b/lldb/test/Shell/Settings/TestEchoCommands.test
index 234b9742bfa2a..02f50d7b21ba5 100644
--- a/lldb/test/Shell/Settings/TestEchoCommands.test
+++ b/lldb/test/Shell/Settings/TestEchoCommands.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands true'  
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsAll.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck 
%S/Inputs/EchoCommandsNoComments.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsNone.out
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml 
b/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
index 9ac922ef3159c..84175041ea1da 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
@@ -1,3 +1,5 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
+
 # RUN: yaml2obj %s > %t
 # RUN: lldb-test symbols %t | FileCheck %s --che

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell tests (PR #100476)

2024-07-24 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Kendal Harland (kendalharland)


Changes

I'm currently working on getting the LLDB test suites to pass on Windows x86_64 
which is not currently included in LLVM CI. These tests are currently failing 
in this configuration.

See https://github.com/llvm/llvm-project/issues/100474

---
Full diff: https://github.com/llvm/llvm-project/pull/100476.diff


18 Files Affected:

- (modified) lldb/test/Shell/Driver/TestConvenienceVariables.test (+1) 
- (modified) lldb/test/Shell/Driver/TestSingleQuote.test (+1) 
- (modified) lldb/test/Shell/Recognizer/verbose_trap.test (+4) 
- (modified) lldb/test/Shell/Settings/TestEchoCommands.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml (+2) 
- (modified) lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp (+2) 
- (modified) lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp (+2) 
- (modified) lldb/test/Shell/SymbolFile/PDB/ast-restore.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/class-layout.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/enums-layout.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/expressions.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/func-symbols.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/pointers.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/type-quals.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/typedefs.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/udt-layout.test (+1) 
- (modified) lldb/test/Shell/SymbolFile/PDB/variables.test (+1) 


``diff
diff --git a/lldb/test/Shell/Driver/TestConvenienceVariables.test 
b/lldb/test/Shell/Driver/TestConvenienceVariables.test
index 45dc7673bfc51..32c3d160153fb 100644
--- a/lldb/test/Shell/Driver/TestConvenienceVariables.test
+++ b/lldb/test/Shell/Driver/TestConvenienceVariables.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 REQUIRES: python
 RUN: mkdir -p %t
 RUN: %build %p/Inputs/hello.cpp -o %t/target.out
diff --git a/lldb/test/Shell/Driver/TestSingleQuote.test 
b/lldb/test/Shell/Driver/TestSingleQuote.test
index af321ba04db39..49f6548e0c5b1 100644
--- a/lldb/test/Shell/Driver/TestSingleQuote.test
+++ b/lldb/test/Shell/Driver/TestSingleQuote.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 # Make sure lldb can handle filenames with single quotes in them.
 # RUN: %clang_host %p/Inputs/hello.c -g -o "%t-'pat"
 # RUN: %lldb -s %s "%t-'pat" | FileCheck %s
diff --git a/lldb/test/Shell/Recognizer/verbose_trap.test 
b/lldb/test/Shell/Recognizer/verbose_trap.test
index dafab7bdea688..76ba87f0f115a 100644
--- a/lldb/test/Shell/Recognizer/verbose_trap.test
+++ b/lldb/test/Shell/Recognizer/verbose_trap.test
@@ -1,4 +1,5 @@
 # UNSUPPORTED: system-windows
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 #
 # RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out 
-DVERBOSE_TRAP_TEST_CATEGORY=\"Foo\" -DVERBOSE_TRAP_TEST_MESSAGE=\"Bar\"
 # RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK,CHECK-BOTH
@@ -12,6 +13,9 @@
 # RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out 
-DVERBOSE_TRAP_TEST_CATEGORY=\"\" -DVERBOSE_TRAP_TEST_MESSAGE=\"\"
 # RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK,CHECK-NONE
 
+# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+
 run
 # CHECK-BOTH:  thread #{{.*}}stop reason = Foo: Bar
 # CHECK-MESSAGE_ONLY:  thread #{{.*}}stop reason = : Bar
diff --git a/lldb/test/Shell/Settings/TestEchoCommands.test 
b/lldb/test/Shell/Settings/TestEchoCommands.test
index 234b9742bfa2a..02f50d7b21ba5 100644
--- a/lldb/test/Shell/Settings/TestEchoCommands.test
+++ b/lldb/test/Shell/Settings/TestEchoCommands.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands true'  
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsAll.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck 
%S/Inputs/EchoCommandsNoComments.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsNone.out
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml 
b/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
index 9ac922ef3159c..84175041ea1da 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
@@ -1,3 +1,5 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
+
 # RUN: yaml2obj %s > %t
 # RUN: lldb-test symbols %t | FileCheck %s --check-prefix=TEST
 # RUN: %lldb %t -o "image dump line-table a.c" -o "image lookup -n _start" -o 
"image lookup -n f" -o exit | FileCheck %s

[Lldb-commits] [lldb] [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (PR #100421)

2024-07-24 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda approved this pull request.

LGTM.


https://github.com/llvm/llvm-project/pull/100421
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing API tests (PR #100477)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland created 
https://github.com/llvm/llvm-project/pull/100477

I'm currently working on getting the LLDB test suites to pass on Windows x86_64 
which is not currently included in LLVM CI. These tests are currently failing 
in this configuration.

See https://github.com/llvm/llvm-project/issues/100474

See also https://github.com/llvm/llvm-project/issues/75936

>From f8e40b90d65434838d2baa79795a1b160ab4614c Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 16:50:34 -0700
Subject: [PATCH] [lldb][test][win][x86_64] XFAIL already failing API tests

---
 .../apropos/with-process/TestAproposWithProcess.py |  3 ++-
 .../command/nested_alias/TestNestedAlias.py|  3 ++-
 .../expression/entry-bp/TestExprEntryBP.py |  2 ++
 .../API/commands/memory/write/TestMemoryWrite.py   |  1 +
 .../use_source_cache/TestUseSourceCache.py |  2 +-
 .../address_breakpoints/TestAddressBreakpoints.py  |  3 ++-
 .../auto_continue/TestBreakpointAutoContinue.py|  3 ++-
 .../TestBreakpointCommandsFromPython.py|  1 +
 .../breakpoint_options/TestBreakpointOptions.py|  3 ++-
 .../step_over_breakpoint/TestStepOverBreakpoint.py |  2 +-
 .../conditional_break/TestConditionalBreak.py  |  1 +
 .../functionalities/memory/find/TestMemoryFind.py  |  2 +-
 .../multiple-slides/TestMultipleSlides.py  |  2 +-
 .../API/functionalities/var_path/TestVarPath.py|  2 --
 lldb/test/API/lang/c/anonymous/TestAnonymous.py|  7 ++-
 lldb/test/API/lang/c/array_types/TestArrayTypes.py |  3 ++-
 lldb/test/API/lang/c/enum_types/TestEnumTypes.py   |  2 +-
 .../API/lang/c/forward/TestForwardDeclaration.py   |  2 +-
 .../API/lang/c/function_types/TestFunctionTypes.py |  2 +-
 .../test/API/lang/c/non-mangled/TestCNonMangled.py |  2 ++
 .../c/register_variables/TestRegisterVariables.py  |  1 +
 lldb/test/API/lang/c/set_values/TestSetValues.py   |  2 +-
 lldb/test/API/lang/c/shared_lib/TestSharedLib.py   |  5 +++--
 .../API/lang/cpp/bitfields/TestCppBitfields.py |  1 +
 .../API/lang/cpp/class_types/TestClassTypes.py |  3 ++-
 lldb/test/API/lang/cpp/inlines/TestInlines.py  |  2 +-
 .../API/lang/cpp/unique-types4/TestUniqueTypes4.py | 14 --
 .../python_api/compile_unit/TestCompileUnitAPI.py  |  1 +
 lldb/test/API/python_api/thread/TestThreadAPI.py   |  1 +
 lldb/test/API/source-manager/TestSourceManager.py  |  2 ++
 30 files changed, 57 insertions(+), 23 deletions(-)

diff --git 
a/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py 
b/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
index 268317a4bf212..adeb7b4f906c5 100644
--- a/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
+++ b/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
@@ -2,8 +2,8 @@
 Test that apropos env doesn't crash trying to touch the process plugin command
 """
 
-
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -17,6 +17,7 @@ def setUp(self):
 # Find the line number to break inside main().
 self.line = line_number("main.cpp", "// break here")
 
+@expectedFailureAll(triple="x86_64-.*-windows.*")
 def test_apropos_with_process(self):
 """Test that apropos env doesn't crash trying to touch the process 
plugin command."""
 self.build()
diff --git a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py 
b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
index 0919caa7d0056..315576afde703 100644
--- a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -2,8 +2,8 @@
 Test that an alias can reference other aliases without crashing.
 """
 
-
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -17,6 +17,7 @@ def setUp(self):
 # Find the line number to break inside main().
 self.line = line_number("main.cpp", "// break here")
 
+@expectedFailureAll(triple="x86_64-.*-windows.*")
 def test_nested_alias(self):
 """Test that an alias can reference other aliases without crashing."""
 self.build()
diff --git a/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py 
b/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py
index 1e7882b4d0236..0ee7e46d73cd1 100644
--- a/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py
+++ b/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py
@@ -4,12 +4,14 @@
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 
 class ExprEntryBPTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
+@expectedFailureAll(triple="x86_64-.*-windows.*")
 def test_expr_entry_bp(self):
 """Tests exp

[Lldb-commits] [lldb] e846fb4 - [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (#100421)

2024-07-24 Thread via lldb-commits


Author: Jonas Devlieghere
Date: 2024-07-24T15:07:38-07:00
New Revision: e846fb48038a34d8df3ad7412bbdcf37e9e7acc9

URL: 
https://github.com/llvm/llvm-project/commit/e846fb48038a34d8df3ad7412bbdcf37e9e7acc9
DIFF: 
https://github.com/llvm/llvm-project/commit/e846fb48038a34d8df3ad7412bbdcf37e9e7acc9.diff

LOG:  [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO 
(#100421)

Prevent passing a nullptr to std::string::insert in
ObjectFileMachO::GetDependentModules. Calling GetCString on an empty
ConstString will return a nullptr, which is undefined behavior. Instead,
use the GetString helper which will return an empty string in that case.

rdar://132388027

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 2c7005449f9d7..ce095bcc48374 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -137,6 +137,9 @@ using namespace lldb;
 using namespace lldb_private;
 using namespace llvm::MachO;
 
+static constexpr llvm::StringLiteral g_loader_path = "@loader_path";
+static constexpr llvm::StringLiteral g_executable_path = "@executable_path";
+
 LLDB_PLUGIN_DEFINE(ObjectFileMachO)
 
 static void PrintRegisterValue(RegisterContext *reg_ctx, const char *name,
@@ -5116,123 +5119,121 @@ UUID ObjectFileMachO::GetUUID() {
 }
 
 uint32_t ObjectFileMachO::GetDependentModules(FileSpecList &files) {
+  ModuleSP module_sp = GetModule();
+  if (!module_sp)
+return 0;
+
   uint32_t count = 0;
-  ModuleSP module_sp(GetModule());
-  if (module_sp) {
-std::lock_guard guard(module_sp->GetMutex());
-llvm::MachO::load_command load_cmd;
-lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
-std::vector rpath_paths;
-std::vector rpath_relative_paths;
-std::vector at_exec_relative_paths;
-uint32_t i;
-for (i = 0; i < m_header.ncmds; ++i) {
-  const uint32_t cmd_offset = offset;
-  if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
-break;
+  std::lock_guard guard(module_sp->GetMutex());
+  llvm::MachO::load_command load_cmd;
+  lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+  std::vector rpath_paths;
+  std::vector rpath_relative_paths;
+  std::vector at_exec_relative_paths;
+  uint32_t i;
+  for (i = 0; i < m_header.ncmds; ++i) {
+const uint32_t cmd_offset = offset;
+if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
+  break;
 
-  switch (load_cmd.cmd) {
-  case LC_RPATH:
-  case LC_LOAD_DYLIB:
-  case LC_LOAD_WEAK_DYLIB:
-  case LC_REEXPORT_DYLIB:
-  case LC_LOAD_DYLINKER:
-  case LC_LOADFVMLIB:
-  case LC_LOAD_UPWARD_DYLIB: {
-uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
-// For LC_LOAD_DYLIB there is an alternate encoding
-// which adds a uint32_t `flags` field for `DYLD_USE_*`
-// flags.  This can be detected by a timestamp field with
-// the `DYLIB_USE_MARKER` constant value.
-bool is_delayed_init = false;
-uint32_t use_command_marker = m_data.GetU32(&offset);
-if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
-  offset += 4; /* uint32_t current_version */
-  offset += 4; /* uint32_t compat_version */
-  uint32_t flags = m_data.GetU32(&offset);
-  // If this LC_LOAD_DYLIB is marked delay-init,
-  // don't report it as a dependent library -- it
-  // may be loaded in the process at some point,
-  // but will most likely not be load at launch.
-  if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
-is_delayed_init = true;
-}
-const char *path = m_data.PeekCStr(name_offset);
-if (path && !is_delayed_init) {
-  if (load_cmd.cmd == LC_RPATH)
-rpath_paths.push_back(path);
-  else {
-if (path[0] == '@') {
-  if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
-rpath_relative_paths.push_back(path + strlen("@rpath"));
-  else if (strncmp(path, "@executable_path",
-   strlen("@executable_path")) == 0)
-at_exec_relative_paths.push_back(path +
- strlen("@executable_path"));
-} else {
-  FileSpec file_spec(path);
-  if (files.AppendIfUnique(file_spec))
-count++;
-}
+switch (load_cmd.cmd) {
+case LC_RPATH:
+case LC_LOAD_DYLIB:
+case LC_LOAD_WEAK_DYLIB:
+case LC_REEXPORT_DYLIB:
+case LC_LOAD_DYLINKER:
+case LC_LOADFVMLIB:
+case LC_LOAD_UPWARD_DYLIB: {
+  uint32_t name_offset = cmd_offset + m_data.GetU32(&o

[Lldb-commits] [lldb] [lldb] Prevent passing a nullptr to std::string in ObjectFileMachO (PR #100421)

2024-07-24 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere closed 
https://github.com/llvm/llvm-project/pull/100421
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing API tests (PR #100477)

2024-07-24 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Kendal Harland (kendalharland)


Changes

I'm currently working on getting the LLDB test suites to pass on Windows x86_64 
which is not currently included in LLVM CI. These tests are currently failing 
in this configuration.

See https://github.com/llvm/llvm-project/issues/100474

See also https://github.com/llvm/llvm-project/issues/75936

---

Patch is 26.00 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/100477.diff


30 Files Affected:

- (modified) 
lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py (+2-1) 
- (modified) lldb/test/API/commands/command/nested_alias/TestNestedAlias.py 
(+2-1) 
- (modified) lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py (+2) 
- (modified) lldb/test/API/commands/memory/write/TestMemoryWrite.py (+1) 
- (modified) 
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py (+1-1) 
- (modified) 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
 (+2-1) 
- (modified) 
lldb/test/API/functionalities/breakpoint/auto_continue/TestBreakpointAutoContinue.py
 (+2-1) 
- (modified) 
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
 (+1) 
- (modified) 
lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
 (+2-1) 
- (modified) 
lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
 (+1-1) 
- (modified) 
lldb/test/API/functionalities/conditional_break/TestConditionalBreak.py (+1) 
- (modified) lldb/test/API/functionalities/memory/find/TestMemoryFind.py (+1-1) 
- (modified) 
lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py (+1-1) 
- (modified) lldb/test/API/functionalities/var_path/TestVarPath.py (-2) 
- (modified) lldb/test/API/lang/c/anonymous/TestAnonymous.py (+6-1) 
- (modified) lldb/test/API/lang/c/array_types/TestArrayTypes.py (+2-1) 
- (modified) lldb/test/API/lang/c/enum_types/TestEnumTypes.py (+1-1) 
- (modified) lldb/test/API/lang/c/forward/TestForwardDeclaration.py (+1-1) 
- (modified) lldb/test/API/lang/c/function_types/TestFunctionTypes.py (+1-1) 
- (modified) lldb/test/API/lang/c/non-mangled/TestCNonMangled.py (+2) 
- (modified) lldb/test/API/lang/c/register_variables/TestRegisterVariables.py 
(+1) 
- (modified) lldb/test/API/lang/c/set_values/TestSetValues.py (+1-1) 
- (modified) lldb/test/API/lang/c/shared_lib/TestSharedLib.py (+3-2) 
- (modified) lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py (+1) 
- (modified) lldb/test/API/lang/cpp/class_types/TestClassTypes.py (+2-1) 
- (modified) lldb/test/API/lang/cpp/inlines/TestInlines.py (+1-1) 
- (modified) lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py (+12-2) 
- (modified) lldb/test/API/python_api/compile_unit/TestCompileUnitAPI.py (+1) 
- (modified) lldb/test/API/python_api/thread/TestThreadAPI.py (+1) 
- (modified) lldb/test/API/source-manager/TestSourceManager.py (+2) 


``diff
diff --git 
a/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py 
b/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
index 268317a4bf212..adeb7b4f906c5 100644
--- a/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
+++ b/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
@@ -2,8 +2,8 @@
 Test that apropos env doesn't crash trying to touch the process plugin command
 """
 
-
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -17,6 +17,7 @@ def setUp(self):
 # Find the line number to break inside main().
 self.line = line_number("main.cpp", "// break here")
 
+@expectedFailureAll(triple="x86_64-.*-windows.*")
 def test_apropos_with_process(self):
 """Test that apropos env doesn't crash trying to touch the process 
plugin command."""
 self.build()
diff --git a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py 
b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
index 0919caa7d0056..315576afde703 100644
--- a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -2,8 +2,8 @@
 Test that an alias can reference other aliases without crashing.
 """
 
-
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -17,6 +17,7 @@ def setUp(self):
 # Find the line number to break inside main().
 self.line = line_number("main.cpp", "// break here")
 
+@expectedFailureAll(triple="x86_64-.*-windows.*")
 def test_nested_alias(self):
 """Test that an alias can reference other aliases without crashing."""
 self.build()
diff --git a/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py 
b/lldb/test/API/

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing API tests (PR #100477)

2024-07-24 Thread via lldb-commits


github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
70c6e79e6d3e897418f3556a25e22e66ff018dc4...f8e40b90d65434838d2baa79795a1b160ab4614c
 lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py 
lldb/test/API/commands/command/nested_alias/TestNestedAlias.py 
lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py 
lldb/test/API/commands/memory/write/TestMemoryWrite.py 
lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
 
lldb/test/API/functionalities/breakpoint/auto_continue/TestBreakpointAutoContinue.py
 
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommandsFromPython.py
 
lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
 
lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
 lldb/test/API/functionalities/conditional_break/TestConditionalBreak.py 
lldb/test/API/functionalities/memory/find/TestMemoryFind.py 
lldb/test/API/functionalities/multiple-slides/TestMultipleSlides.py 
lldb/test/API/functionalities/var_path/TestVarPath.py 
lldb/test/API/lang/c/anonymous/TestAnonymous.py 
lldb/test/API/lang/c/array_types/TestArrayTypes.py 
lldb/test/API/lang/c/enum_types/TestEnumTypes.py 
lldb/test/API/lang/c/forward/TestForwardDeclaration.py 
lldb/test/API/lang/c/function_types/TestFunctionTypes.py 
lldb/test/API/lang/c/non-mangled/TestCNonMangled.py 
lldb/test/API/lang/c/register_variables/TestRegisterVariables.py 
lldb/test/API/lang/c/set_values/TestSetValues.py 
lldb/test/API/lang/c/shared_lib/TestSharedLib.py 
lldb/test/API/lang/cpp/bitfields/TestCppBitfields.py 
lldb/test/API/lang/cpp/class_types/TestClassTypes.py 
lldb/test/API/lang/cpp/inlines/TestInlines.py 
lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py 
lldb/test/API/python_api/compile_unit/TestCompileUnitAPI.py 
lldb/test/API/python_api/thread/TestThreadAPI.py 
lldb/test/API/source-manager/TestSourceManager.py
``





View the diff from darker here.


``diff
--- lang/cpp/unique-types4/TestUniqueTypes4.py  2024-07-24 22:06:28.00 +
+++ lang/cpp/unique-types4/TestUniqueTypes4.py  2024-07-24 22:10:42.336508 +
@@ -39,22 +39,22 @@
 self.expect_expr("ns::FooDouble::value", result_type="double", 
result_value="0")
 self.expect_expr("ns::FooInt::value", result_type="int", 
result_value="0")
 
 @expectedFailureAll(
 oslist=["windows"],
-# When this issue is fixed, please add `archs=["x86_64"]` as we do not 
run 
+# When this issue is fixed, please add `archs=["x86_64"]` as we do not 
run
 # tests on x86_64 Windows in CI and it may remain broken.
 bugnumber="https://github.com/llvm/llvm-project/issues/75936";,
 )
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler_version=["<", "15.0"])
 def test_simple_template_names(self):
 self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
 
 @expectedFailureAll(
 oslist=["windows"],
-# When this issue is fixed, please add `archs=["x86_64"]` as we do not 
run 
+# When this issue is fixed, please add `archs=["x86_64"]` as we do not 
run
 # tests on x86_64 Windows in CI and it may remain broken.
 bugnumber="https://github.com/llvm/llvm-project/issues/75936";,
 )
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler_version=["<", "15.0"])

``




https://github.com/llvm/llvm-project/pull/100477
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb-dap] Updated README.md for newly added attach properties. (PR #99926)

2024-07-24 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere approved this pull request.

LGTM. Can you please bump the version number in the `package.json` file so we 
can publish an update to the marketplace? 

https://github.com/llvm/llvm-project/pull/99926
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing API tests (PR #100477)

2024-07-24 Thread Jonas Devlieghere via lldb-commits


JDevlieghere wrote:

Rather than a regex for the triple, can you use a combination of `oslist` and 
`archs`? `expectedFailureAll` is supposed to combine the conditions, so the 
regex should be equivalent to: 

```
@expectedFailureAll(oslist=["windows"], archs=["x86_64"])
```

https://github.com/llvm/llvm-project/pull/100477
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing API tests (PR #100477)

2024-07-24 Thread Kendal Harland via lldb-commits


kendalharland wrote:

> Rather than a regex for the triple, can you use a combination of `oslist` and 
> `archs`? `expectedFailureAll` is supposed to combine the conditions, so the 
> regex should be equivalent to:
> 
> ```
> @expectedFailureAll(oslist=["windows"], archs=["x86_64"])
> ```

Yes. I will fix this in each of the PRs I recently mailed you.

https://github.com/llvm/llvm-project/pull/100477
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell/Driver tests (PR #100473)

2024-07-24 Thread Jonas Devlieghere via lldb-commits


JDevlieghere wrote:

How are these tests failing? Neither of them seem to be testing something 
specific to x86_64 and presumably the test are passing on [the aarch64 windows 
buildbot](https://lab.llvm.org/buildbot/#/builders/141)? 

https://github.com/llvm/llvm-project/pull/100473
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell tests (PR #100476)

2024-07-24 Thread Kendal Harland via lldb-commits



@@ -12,6 +13,9 @@
 # RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out 
-DVERBOSE_TRAP_TEST_CATEGORY=\"\" -DVERBOSE_TRAP_TEST_MESSAGE=\"\"
 # RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK,CHECK-NONE
 
+# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out

kendalharland wrote:

I am not sure where these two lines came from. Might be a bad rebase. I'll 
recheck locally then update.

https://github.com/llvm/llvm-project/pull/100476
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing API tests (PR #100477)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland updated 
https://github.com/llvm/llvm-project/pull/100477

>From 3a7e97987140c2d62259a54cb17c00996c0ed6db Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 16:50:34 -0700
Subject: [PATCH] [lldb][test][win][x86_64] XFAIL already failing API tests

---
 .../apropos/with-process/TestAproposWithProcess.py |  3 ++-
 .../command/nested_alias/TestNestedAlias.py|  3 ++-
 .../expression/entry-bp/TestExprEntryBP.py |  2 ++
 .../API/commands/memory/write/TestMemoryWrite.py   |  1 +
 .../use_source_cache/TestUseSourceCache.py |  2 +-
 .../address_breakpoints/TestAddressBreakpoints.py  |  3 ++-
 .../auto_continue/TestBreakpointAutoContinue.py|  3 ++-
 .../TestBreakpointCommandsFromPython.py|  1 +
 .../breakpoint_options/TestBreakpointOptions.py|  3 ++-
 .../step_over_breakpoint/TestStepOverBreakpoint.py |  2 +-
 .../conditional_break/TestConditionalBreak.py  |  1 +
 .../functionalities/memory/find/TestMemoryFind.py  |  2 +-
 .../multiple-slides/TestMultipleSlides.py  |  2 +-
 .../API/functionalities/var_path/TestVarPath.py|  2 --
 lldb/test/API/lang/c/anonymous/TestAnonymous.py|  7 ++-
 lldb/test/API/lang/c/array_types/TestArrayTypes.py |  3 ++-
 lldb/test/API/lang/c/enum_types/TestEnumTypes.py   |  2 +-
 .../API/lang/c/forward/TestForwardDeclaration.py   |  2 +-
 .../API/lang/c/function_types/TestFunctionTypes.py |  2 +-
 .../test/API/lang/c/non-mangled/TestCNonMangled.py |  2 ++
 .../c/register_variables/TestRegisterVariables.py  |  1 +
 lldb/test/API/lang/c/set_values/TestSetValues.py   |  2 +-
 lldb/test/API/lang/c/shared_lib/TestSharedLib.py   |  5 +++--
 .../API/lang/cpp/bitfields/TestCppBitfields.py |  1 +
 .../API/lang/cpp/class_types/TestClassTypes.py |  3 ++-
 lldb/test/API/lang/cpp/inlines/TestInlines.py  |  2 +-
 .../API/lang/cpp/unique-types4/TestUniqueTypes4.py | 14 --
 .../python_api/compile_unit/TestCompileUnitAPI.py  |  1 +
 lldb/test/API/python_api/thread/TestThreadAPI.py   |  3 ++-
 lldb/test/API/source-manager/TestSourceManager.py  |  2 ++
 30 files changed, 58 insertions(+), 24 deletions(-)

diff --git 
a/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py 
b/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
index 268317a4bf212..6a802d544fe9e 100644
--- a/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
+++ b/lldb/test/API/commands/apropos/with-process/TestAproposWithProcess.py
@@ -2,8 +2,8 @@
 Test that apropos env doesn't crash trying to touch the process plugin command
 """
 
-
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -17,6 +17,7 @@ def setUp(self):
 # Find the line number to break inside main().
 self.line = line_number("main.cpp", "// break here")
 
+@expectedFailureAll(oslist=["windows"], archs=["x86_64"])
 def test_apropos_with_process(self):
 """Test that apropos env doesn't crash trying to touch the process 
plugin command."""
 self.build()
diff --git a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py 
b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
index 0919caa7d0056..954578f777c66 100644
--- a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -2,8 +2,8 @@
 Test that an alias can reference other aliases without crashing.
 """
 
-
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -17,6 +17,7 @@ def setUp(self):
 # Find the line number to break inside main().
 self.line = line_number("main.cpp", "// break here")
 
+@expectedFailureAll(oslist=["windows"], archs=["x86_64"])
 def test_nested_alias(self):
 """Test that an alias can reference other aliases without crashing."""
 self.build()
diff --git a/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py 
b/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py
index 1e7882b4d0236..2b1f83e288fff 100644
--- a/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py
+++ b/lldb/test/API/commands/expression/entry-bp/TestExprEntryBP.py
@@ -4,12 +4,14 @@
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 
 class ExprEntryBPTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
+@expectedFailureAll(oslist=["windows"], archs=["x86_64"])
 def test_expr_entry_bp(self):
 """Tests expressions evaluation when the breakpoint on module's entry 
is set."""
 self.build()
diff --git a/lldb/test/API/commands/memory/write/TestMemoryWrite.py 
b/lldb/test/API/commands/memory/write/TestMemoryWrite.py
index 45787243a614d..cb74c77c3c16f 100644
--- a/lldb/tes

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing API tests (PR #100477)

2024-07-24 Thread Kendal Harland via lldb-commits


kendalharland wrote:

> > Rather than a regex for the triple, can you use a combination of `oslist` 
> > and `archs`? `expectedFailureAll` is supposed to combine the conditions, so 
> > the regex should be equivalent to:
> > ```
> > @expectedFailureAll(oslist=["windows"], archs=["x86_64"])
> > ```
> 
> Yes. I will fix this in each of the PRs I recently mailed you.
> 
> Will fix the dark formatter issue above too.

Done

https://github.com/llvm/llvm-project/pull/100477
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell tests (PR #100476)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland updated 
https://github.com/llvm/llvm-project/pull/100476

>From fc88c4ac46e0e9266bf24467b6ad6c5e55d8216c Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 15:47:48 -0700
Subject: [PATCH] [lldb][test][win][x86_64] XFAIL already failing Shell tests

---
 lldb/test/Shell/Driver/TestConvenienceVariables.test  | 1 +
 lldb/test/Shell/Driver/TestSingleQuote.test   | 1 +
 lldb/test/Shell/Recognizer/verbose_trap.test  | 1 +
 lldb/test/Shell/Settings/TestEchoCommands.test| 1 +
 lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml | 2 ++
 lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp  | 2 ++
 lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp| 2 ++
 lldb/test/Shell/SymbolFile/PDB/ast-restore.test   | 1 +
 lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test   | 1 +
 lldb/test/Shell/SymbolFile/PDB/class-layout.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/enums-layout.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/expressions.test   | 1 +
 lldb/test/Shell/SymbolFile/PDB/func-symbols.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/pointers.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/type-quals.test| 1 +
 lldb/test/Shell/SymbolFile/PDB/typedefs.test  | 1 +
 lldb/test/Shell/SymbolFile/PDB/udt-layout.test| 1 +
 lldb/test/Shell/SymbolFile/PDB/variables.test | 1 +
 18 files changed, 21 insertions(+)

diff --git a/lldb/test/Shell/Driver/TestConvenienceVariables.test 
b/lldb/test/Shell/Driver/TestConvenienceVariables.test
index 45dc7673bfc51..32c3d160153fb 100644
--- a/lldb/test/Shell/Driver/TestConvenienceVariables.test
+++ b/lldb/test/Shell/Driver/TestConvenienceVariables.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 REQUIRES: python
 RUN: mkdir -p %t
 RUN: %build %p/Inputs/hello.cpp -o %t/target.out
diff --git a/lldb/test/Shell/Driver/TestSingleQuote.test 
b/lldb/test/Shell/Driver/TestSingleQuote.test
index af321ba04db39..49f6548e0c5b1 100644
--- a/lldb/test/Shell/Driver/TestSingleQuote.test
+++ b/lldb/test/Shell/Driver/TestSingleQuote.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 # Make sure lldb can handle filenames with single quotes in them.
 # RUN: %clang_host %p/Inputs/hello.c -g -o "%t-'pat"
 # RUN: %lldb -s %s "%t-'pat" | FileCheck %s
diff --git a/lldb/test/Shell/Recognizer/verbose_trap.test 
b/lldb/test/Shell/Recognizer/verbose_trap.test
index dafab7bdea688..e8950ad7ad23a 100644
--- a/lldb/test/Shell/Recognizer/verbose_trap.test
+++ b/lldb/test/Shell/Recognizer/verbose_trap.test
@@ -1,4 +1,5 @@
 # UNSUPPORTED: system-windows
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 #
 # RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out 
-DVERBOSE_TRAP_TEST_CATEGORY=\"Foo\" -DVERBOSE_TRAP_TEST_MESSAGE=\"Bar\"
 # RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK,CHECK-BOTH
diff --git a/lldb/test/Shell/Settings/TestEchoCommands.test 
b/lldb/test/Shell/Settings/TestEchoCommands.test
index 234b9742bfa2a..02f50d7b21ba5 100644
--- a/lldb/test/Shell/Settings/TestEchoCommands.test
+++ b/lldb/test/Shell/Settings/TestEchoCommands.test
@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands true'  
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsAll.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck 
%S/Inputs/EchoCommandsNoComments.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsNone.out
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml 
b/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
index 9ac922ef3159c..84175041ea1da 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dead-code-filtering.yaml
@@ -1,3 +1,5 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}
+
 # RUN: yaml2obj %s > %t
 # RUN: lldb-test symbols %t | FileCheck %s --check-prefix=TEST
 # RUN: %lldb %t -o "image dump line-table a.c" -o "image lookup -n _start" -o 
"image lookup -n f" -o exit | FileCheck %s
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp 
b/lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
index 9aa25adf6bcc7..3e767230e04db 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
@@ -1,5 +1,7 @@
 // clang-format off
 
+// XFAIL: target=x86_64-{{.*}}-windows{{.*}}
+
 // REQUIRES: system-windows
 // RUN: %build -o %t.exe -- %s
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
diff --git a/lldb/test/Shell/SymbolFile/NativePDB/stack_unwind

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell tests (PR #100476)

2024-07-24 Thread Kendal Harland via lldb-commits



@@ -12,6 +13,9 @@
 # RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out 
-DVERBOSE_TRAP_TEST_CATEGORY=\"\" -DVERBOSE_TRAP_TEST_MESSAGE=\"\"
 # RUN: %lldb -b -s %s %t.out | FileCheck %s --check-prefixes=CHECK,CHECK-NONE
 
+# RUN: %clang_host -g -O0 %S/Inputs/verbose_trap.cpp -o %t.out

kendalharland wrote:

Removed

https://github.com/llvm/llvm-project/pull/100476
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell tests (PR #100476)

2024-07-24 Thread Kendal Harland via lldb-commits



@@ -1,3 +1,4 @@
+# XFAIL: target=x86_64-{{.*}}-windows{{.*}}

kendalharland wrote:

@JDevlieghere please LMK if the target triple can be expressed more clearly as 
a combination of other filters in `.test` files, here and throughout.

https://github.com/llvm/llvm-project/pull/100476
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] XFAIL already failing Shell/Driver tests (PR #100473)

2024-07-24 Thread Kendal Harland via lldb-commits


kendalharland wrote:

> How are these tests failing? Neither of them seem to be testing something 
> specific to x86_64 and presumably the test are passing on [the aarch64 
> windows buildbot](https://lab.llvm.org/buildbot/#/builders/141)?

I am currently seeing this output:

```
Testing:  0.. 10
FAIL: lldb-shell :: Driver/TestConvenienceVariables.test (48 of 2873)
 TEST 'lldb-shell :: Driver/TestConvenienceVariables.test' 
FAILED 
Script:
--
: 'RUN: at line 2';   mkdir -p 
S:\b\1\tools\lldb\test\Shell\Driver\Output\TestConvenienceVariables.test.tmp
: 'RUN: at line 3';   'C:\Program Files (x86)\Microsoft Visual 
Studio\Shared\Python39_64\python.exe' 
S:\SourceCache\llvm-project\lldb\test\Shell\helper\build.py --compiler=any 
--arch=64 --tools-dir=S:/b/1/./bin --libs-dir=S:/b/1/./lib 
S:\SourceCache\llvm-project\lldb\test\Shell\Driver/Inputs/hello.cpp -o 
S:\b\1\tools\lldb\test\Shell\Driver\Output\TestConvenienceVariables.test.tmp/target.out
: 'RUN: at line 4';   s:\b\1\bin\lldb.exe --no-lldbinit -S 
S:/b/1/tools/lldb\test\Shell\lit-lldb-init-quiet 
S:\b\1\tools\lldb\test\Shell\Driver\Output\TestConvenienceVariables.test.tmp/target.out
 -s S:\SourceCache\llvm-project\lldb\test\Shell\Driver/Inputs/convenience.in -o 
quit | s:\b\1\bin\filecheck.exe 
S:\SourceCache\llvm-project\lldb\test\Shell\Driver\TestConvenienceVariables.test
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 2"
note: command had no output on stdout or stderr
$ "mkdir" "-p" 
"S:\b\1\tools\lldb\test\Shell\Driver\Output\TestConvenienceVariables.test.tmp"
note: command had no output on stdout or stderr
$ ":" "RUN: at line 3"
note: command had no output on stdout or stderr
$ "C:\Program Files (x86)\Microsoft Visual 
Studio\Shared\Python39_64\python.exe" 
"S:\SourceCache\llvm-project\lldb\test\Shell\helper\build.py" "--compiler=any" 
"--arch=64" "--tools-dir=S:/b/1/./bin" "--libs-dir=S:/b/1/./lib" 
"S:\SourceCache\llvm-project\lldb\test\Shell\Driver/Inputs/hello.cpp" "-o" 
"S:\b\1\tools\lldb\test\Shell\Driver\Output\TestConvenienceVariables.test.tmp/target.out"
# command output:
Cleaning hello.ilk
Cleaning target.out-hello.obj
Cleaning target.pdb
Cleaning target.out



compiling hello.cpp -> target.out-hello.obj
  STDOUT:




linking target.out-hello.obj -> target.out
  STDOUT:


$ ":" "RUN: at line 4"
note: command had no output on stdout or stderr
$ "s:\b\1\bin\lldb.exe" "--no-lldbinit" "-S" 
"S:/b/1/tools/lldb\test\Shell\lit-lldb-init-quiet" 
"S:\b\1\tools\lldb\test\Shell\Driver\Output\TestConvenienceVariables.test.tmp/target.out"
 "-s" 
"S:\SourceCache\llvm-project\lldb\test\Shell\Driver/Inputs/convenience.in" "-o" 
"quit"
note: command had no output on stdout or stderr
$ "s:\b\1\bin\filecheck.exe" 
"S:\SourceCache\llvm-project\lldb\test\Shell\Driver\TestConvenienceVariables.test"
# command stderr:
S:\SourceCache\llvm-project\lldb\test\Shell\Driver\TestConvenienceVariables.test:6:8:
 error: CHECK: expected string not found in input
CHECK: stop reason = breakpoint 1.1
   ^
:1:1: note: scanning from here
(lldb) command source -s 0 'S:/b/1/tools/lldb\test\Shell\lit-lldb-init-quiet'
^
:13:14: note: possible intended match here
* thread #1, stop reason = breakpoint 1.2
 ^

Input file: 
Check file: 
S:\SourceCache\llvm-project\lldb\test\Shell\Driver\TestConvenienceVariables.test

-dump-input=help explains the following input dump.

Input was:
<<
   1: (lldb) command source -s 0 
'S:/b/1/tools/lldb\test\Shell\lit-lldb-init-quiet'
check:6'0 
X~ 
error: no match found
   2: Executing commands in 
'S:\b\1\tools\lldb\test\Shell\lit-lldb-init-quiet'.
check:6'0 
~~
   3: (lldb) command source -C --silent-run true lit-lldb-init
check:6'0 ~
   4: (lldb) target create 
"S:\\b\\1\\tools\\lldb\\test\\Shell\\Driver\\Output\\TestConvenienceVariables.test.tmp/target.out"
check:6'0 

   5: Current executable set to 
'S:\b\1\tools\lldb\test\Shell\Driver\Output\TestConvenienceVariables.test.tmp\target.out'
 (x86_64).
check:6'0 
~~
   6: (lldb) command source -s 0 
'S:\SourceCache\llvm-project\lldb\test\Shell\Driver/Inputs/convenience.in'
check:6'0 
~~
   7: Executing commands in 
'S:\SourceCache\llvm-project\lldb\test\Shell\Driver\Inputs\convenience.in'.
check:6'0 
~~

[Lldb-commits] [lldb] [lldb] Don't use a vm addr range starting at 0 for local memory (PR #100288)

2024-07-24 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/100288

>From 28439443b9e44a242e59bf1cdd114486380d54fc Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Tue, 23 Jul 2024 18:54:31 -0700
Subject: [PATCH 1/2] [lldb] Don't use a vm addr range starting at 0 for local
 memory

When an inferior stub cannot allocate memory for lldb, and lldb
needs to store the result of expressions, it will do it in lldb's
own memory range ("host memory").  But it needs to find a virtual
address range that is not used in the inferior process.  It tries
to use the qMemoryRegionInfo gdb remote serial protocol packet to
find a range that is inaccessible, starting at address 0 and moving
up the size of each region.

If the first region found at address 0 is inaccessible, lldb will
use the address range starting at 0 to mean "read lldb's host memory,
not the process memory", and programs that crash with a null
dereference will have poor behavior.

This patch skips consideration of a memory region that starts at
address 0.

I also clarified the documentation of qMemoryRegionInfo to make it
clear that the stub is required to provide permissions for a memory
range that is accessable, it is not an optional key in this response.
This issue was originally found by a stub that did not list
permissions, and lldb treated the first region returned as the one
it would use.
---
 lldb/docs/resources/lldbgdbremote.md   |  8 +++-
 lldb/source/Expression/IRMemoryMap.cpp | 17 -
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/lldb/docs/resources/lldbgdbremote.md 
b/lldb/docs/resources/lldbgdbremote.md
index 7076a75032dae..5cac3736337a8 100644
--- a/lldb/docs/resources/lldbgdbremote.md
+++ b/lldb/docs/resources/lldbgdbremote.md
@@ -1403,6 +1403,12 @@ For instance, with a macOS process which has nothing 
mapped in the first
 The lack of `permissions:` indicates that none of read/write/execute are valid
 for this region.
 
+The stub must include `permissions:` key-value on all memory ranges
+that are valid to access in the inferior process -- the lack of
+`permissions:` means that this is an inaccessible (no page table
+entries exist, in a system using VM) memory range.  If a stub cannot
+determine actual permissions, return `rwx`.
+
 **Priority To Implement:** Medium
 
 This is nice to have, but it isn't necessary. It helps LLDB
@@ -2434,4 +2440,4 @@ The `0x` prefixes are optional - like most of the 
gdb-remote packets,
 omitting them will work fine; these numbers are always base 16.
 
 The length of the payload is not provided.  A reliable, 8-bit clean,
-transport layer is assumed.
\ No newline at end of file
+transport layer is assumed.
diff --git a/lldb/source/Expression/IRMemoryMap.cpp 
b/lldb/source/Expression/IRMemoryMap.cpp
index de631370bb048..bb3e4e0a9a9fc 100644
--- a/lldb/source/Expression/IRMemoryMap.cpp
+++ b/lldb/source/Expression/IRMemoryMap.cpp
@@ -84,7 +84,7 @@ lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
   // any allocations.  Otherwise start at the beginning of memory.
 
   if (m_allocations.empty()) {
-ret = 0x0;
+ret = 0;
   } else {
 auto back = m_allocations.rbegin();
 lldb::addr_t addr = back->first;
@@ -116,10 +116,17 @@ lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
 Status err = process_sp->GetMemoryRegionInfo(ret, region_info);
 if (err.Success()) {
   while (true) {
-if (region_info.GetReadable() != MemoryRegionInfo::OptionalBool::eNo ||
-region_info.GetWritable() != MemoryRegionInfo::OptionalBool::eNo ||
-region_info.GetExecutable() !=
-MemoryRegionInfo::OptionalBool::eNo) {
+if (region_info.GetRange().GetRangeBase() == 0 &&
+region_info.GetRange().GetRangeEnd() - 1 < end_of_memory) {
+  // Don't use a region that starts at address 0,
+  // that can mask null dereferences in the inferior.
+  ret = region_info.GetRange().GetRangeEnd();
+} else if (region_info.GetReadable() !=
+   MemoryRegionInfo::OptionalBool::eNo ||
+   region_info.GetWritable() !=
+   MemoryRegionInfo::OptionalBool::eNo ||
+   region_info.GetExecutable() !=
+   MemoryRegionInfo::OptionalBool::eNo) {
   if (region_info.GetRange().GetRangeEnd() - 1 >= end_of_memory) {
 ret = LLDB_INVALID_ADDRESS;
 break;

>From 6fbe7592b2baccdb7656b20198088a350ab887f1 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Wed, 24 Jul 2024 16:01:21 -0700
Subject: [PATCH 2/2] fix minor nit.

---
 lldb/source/Expression/IRMemoryMap.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Expression/IRMemoryMap.cpp 
b/lldb/source/Expression/IRMemoryMap.cpp
index bb3e4e0a9a9fc..0c1d9016616cb 100644
--- a/lldb/source/Expression/IRMemoryMap.cpp
+++ b/lldb/source/Expression/IRMemoryMap.cpp
@@ -117,9 +117,10 @@ lldb::addr_t

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jacob Lalonde via lldb-commits


https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/100443

>From d7940af06873cedf5976dc829dd9377b2851ae25 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Tue, 23 Jul 2024 16:50:57 -0700
Subject: [PATCH 1/6] Implemplement a thread list that is currently unused for
 SBSaveCore. Run formatter

---
 lldb/include/lldb/API/SBSaveCoreOptions.h  | 24 +++
 lldb/include/lldb/Symbol/SaveCoreOptions.h | 10 +
 lldb/include/lldb/Target/Process.h |  5 +++
 lldb/source/API/SBSaveCoreOptions.cpp  | 20 +
 lldb/source/Core/PluginManager.cpp |  4 ++
 lldb/source/Symbol/SaveCoreOptions.cpp | 48 ++
 lldb/source/Target/Process.cpp | 12 ++
 7 files changed, 123 insertions(+)

diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h 
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index e77496bd3a4a0..b485371ce8f55 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions {
   /// \return The output file spec.
   SBFileSpec GetOutputFile() const;
 
+  /// Add a thread to save in the core file.
+  ///
+  /// \param thread_id The thread ID to save.
+  void AddThread(lldb::tid_t thread_id);
+
+  /// Remove a thread from the list of threads to save.
+  ///
+  /// \param thread_id The thread ID to remove.
+  /// \return True if the thread was removed, false if it was not in the list.
+  bool RemoveThread(lldb::tid_t thread_id);
+
+  /// Get the number of threads to save. If this list is empty all threads will
+  /// be saved.
+  ///
+  /// \return The number of threads to save.
+  uint32_t GetNumThreads() const;
+
+  /// Get the thread ID at the given index.
+  ///
+  /// \param[in] index The index of the thread ID to get.
+  /// \return The thread ID at the given index, or an error
+  /// if there is no thread at the index.
+  lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const;
+
   /// Reset all options.
   void Clear();
 
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h 
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 583bc1f483d04..d583b32b29508 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -14,6 +14,7 @@
 #include "lldb/lldb-types.h"
 
 #include 
+#include 
 #include 
 
 namespace lldb_private {
@@ -32,12 +33,21 @@ class SaveCoreOptions {
   void SetOutputFile(lldb_private::FileSpec file);
   const std::optional GetOutputFile() const;
 
+  void AddThread(lldb::tid_t tid);
+  bool RemoveThread(lldb::tid_t tid);
+  size_t GetNumThreads() const;
+  int64_t GetThreadAtIndex(size_t index) const;
+  bool ShouldSaveThread(lldb::tid_t tid) const;
+
+  Status EnsureValidConfiguration() const;
+
   void Clear();
 
 private:
   std::optional m_plugin_name;
   std::optional m_file;
   std::optional m_style;
+  std::set m_threads_to_save;
 };
 } // namespace lldb_private
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..c551504c8583f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -741,6 +741,11 @@ class Process : public 
std::enable_shared_from_this,
   Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style,
  CoreFileMemoryRanges &ranges);
 
+  /// Helper function for Process::SaveCore(...) that calculates the thread 
list
+  /// based upon options set within a given \a core_options object.
+  ThreadCollection::ThreadIterable
+  CalculateCoreFileThreadList(SaveCoreOptions &core_options);
+
 protected:
   virtual JITLoaderList &GetJITLoaders();
 
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp 
b/lldb/source/API/SBSaveCoreOptions.cpp
index 6c3f74596203d..1d45313d2426a 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -75,6 +75,26 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const {
   return m_opaque_up->GetStyle();
 }
 
+void SBSaveCoreOptions::AddThread(lldb::tid_t tid) {
+  m_opaque_up->AddThread(tid);
+}
+
+bool SBSaveCoreOptions::RemoveThread(lldb::tid_t tid) {
+  return m_opaque_up->RemoveThread(tid);
+}
+
+uint32_t SBSaveCoreOptions::GetNumThreads() const {
+  return m_opaque_up->GetNumThreads();
+}
+
+lldb::tid_t SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx,
+SBError &error) const {
+  int64_t tid = m_opaque_up->GetThreadAtIndex(idx);
+  if (tid == -1)
+error.SetErrorString("Invalid index");
+  return 0;
+}
+
 void SBSaveCoreOptions::Clear() {
   LLDB_INSTRUMENT_VA(this);
   m_opaque_up->Clear();
diff --git a/lldb/source/Core/PluginManager.cpp 
b/lldb/source/Core/PluginManager.cpp
index 759ef3a8afe02..94e3cb85f31b9 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -714,6 +714,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP 
&process_sp,

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Set breakpoint condition on breakpoint instead of location in TestBreakpointConditions (PR #100487)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland created 
https://github.com/llvm/llvm-project/pull/100487

On windows x86_64 this test stops on the set breakpoint when `val == 1` when 
the breakpoint condition is set on the SBBreakpointLocation rather than the 
SBBreakpoint directly.  Setting the condition on the breakpoint itself, seems 
to fix the issue.

This PR also splits the test assertion that verifies we're on the correct line 
and have the correct value of `val` to make the error message more clear. At 
present it just shows `Assertion error: True != False`

https://github.com/llvm/llvm-project/issues/100486

>From 29d5a57eb8cc344e1a93787fe9cb333761923927 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:24:24 -0700
Subject: [PATCH 1/2] [lldb][test][x86_64][win] Split TestBreakpointConditions
 assertion to clarify failure message

When this test fails we see an assertion error `False != True`
This clarifies the error by showing, for example, if `1 != 3` when
comparing `var` to the string "3".
---
 .../breakpoint_conditions/TestBreakpointConditions.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 50ba0317fd094..4e7a8ccb9fbeb 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -176,11 +176,15 @@ def breakpoint_conditions_python(self):
 thread.IsValid(),
 "There should be a thread stopped due to breakpoint condition",
 )
+
 frame0 = thread.GetFrameAtIndex(0)
 var = frame0.FindValue("val", lldb.eValueTypeVariableArgument)
-self.assertTrue(
-frame0.GetLineEntry().GetLine() == self.line1 and var.GetValue() 
== "3"
+self.assertEqual(
+frame0.GetLineEntry().GetLine(),
+self.line1,
+"The debugger stopped on the correct line",
 )
+self.assertEqual(var.GetValue(), "3")
 
 # The hit count for the breakpoint should be 1.
 self.assertEqual(breakpoint.GetHitCount(), 1)

>From 11ee9346ee8ef928e5f31982747557604ba8f91b Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:47:26 -0700
Subject: [PATCH 2/2] [lldb][test][x86_64][win] TestBreakpointConditions set
 condition on breakpoint instead of location

On windows x86_64 this test seems to stop on the breakpoint when val ==
3 iff the condition is set on the breakpoint rather than the location.
Setting the condition on the breakpoint should work for all platforms,
but the fact that setting the condition on the location doens't work on
Windows x86_64 is considered a bug.
---
 .../breakpoint_conditions/TestBreakpointConditions.py   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 4e7a8ccb9fbeb..625ca916d20f1 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -155,13 +155,13 @@ def breakpoint_conditions_python(self):
 breakpoint.GetThreadIndex(), 1, "The thread index has been set 
correctly"
 )
 
+# Set the condition on the breakpoint.
+breakpoint.SetCondition("val == 3")
+
 # Get the breakpoint location from breakpoint after we verified that,
 # indeed, it has one location.
 location = breakpoint.GetLocationAtIndex(0)
 self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
-
-# Set the condition on the breakpoint location.
-location.SetCondition("val == 3")
 self.expect(location.GetCondition(), exe=False, startstr="val == 3")
 
 # Now launch the process, and do not stop at entry point.

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Set breakpoint condition on breakpoint instead of location in TestBreakpointConditions (PR #100487)

2024-07-24 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Kendal Harland (kendalharland)


Changes

On windows x86_64 this test stops on the set breakpoint when `val == 1` when 
the breakpoint condition is set on the SBBreakpointLocation rather than the 
SBBreakpoint directly.  Setting the condition on the breakpoint itself, seems 
to fix the issue.

This PR also splits the test assertion that verifies we're on the correct line 
and have the correct value of `val` to make the error message more clear. At 
present it just shows `Assertion error: True != False`

https://github.com/llvm/llvm-project/issues/100486

---
Full diff: https://github.com/llvm/llvm-project/pull/100487.diff


1 Files Affected:

- (modified) 
lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 (+9-5) 


``diff
diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 50ba0317fd094..625ca916d20f1 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -155,13 +155,13 @@ def breakpoint_conditions_python(self):
 breakpoint.GetThreadIndex(), 1, "The thread index has been set 
correctly"
 )
 
+# Set the condition on the breakpoint.
+breakpoint.SetCondition("val == 3")
+
 # Get the breakpoint location from breakpoint after we verified that,
 # indeed, it has one location.
 location = breakpoint.GetLocationAtIndex(0)
 self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
-
-# Set the condition on the breakpoint location.
-location.SetCondition("val == 3")
 self.expect(location.GetCondition(), exe=False, startstr="val == 3")
 
 # Now launch the process, and do not stop at entry point.
@@ -176,11 +176,15 @@ def breakpoint_conditions_python(self):
 thread.IsValid(),
 "There should be a thread stopped due to breakpoint condition",
 )
+
 frame0 = thread.GetFrameAtIndex(0)
 var = frame0.FindValue("val", lldb.eValueTypeVariableArgument)
-self.assertTrue(
-frame0.GetLineEntry().GetLine() == self.line1 and var.GetValue() 
== "3"
+self.assertEqual(
+frame0.GetLineEntry().GetLine(),
+self.line1,
+"The debugger stopped on the correct line",
 )
+self.assertEqual(var.GetValue(), "3")
 
 # The hit count for the breakpoint should be 1.
 self.assertEqual(breakpoint.GetHitCount(), 1)

``




https://github.com/llvm/llvm-project/pull/100487
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 2ba1aee - [lldb] Don't use a vm addr range starting at 0 for local memory (#100288)

2024-07-24 Thread via lldb-commits


Author: Jason Molenda
Date: 2024-07-24T17:25:57-07:00
New Revision: 2ba1aeed2efd8156717886f89f6d4270b1df7a18

URL: 
https://github.com/llvm/llvm-project/commit/2ba1aeed2efd8156717886f89f6d4270b1df7a18
DIFF: 
https://github.com/llvm/llvm-project/commit/2ba1aeed2efd8156717886f89f6d4270b1df7a18.diff

LOG: [lldb] Don't use a vm addr range starting at 0 for local memory (#100288)

When an inferior stub cannot allocate memory for lldb, and lldb needs to
store the result of expressions, it will do it in lldb's own memory
range ("host memory"). But it needs to find a virtual address range that
is not used in the inferior process. It tries to use the
qMemoryRegionInfo gdb remote serial protocol packet to find a range that
is inaccessible, starting at address 0 and moving up the size of each
region.

If the first region found at address 0 is inaccessible, lldb will use
the address range starting at 0 to mean "read lldb's host memory, not
the process memory", and programs that crash with a null dereference
will have poor behavior.

This patch skips consideration of a memory region that starts at address
0.

I also clarified the documentation of qMemoryRegionInfo to make it clear
that the stub is required to provide permissions for a memory range that
is accessable, it is not an optional key in this response. This issue
was originally found by a stub that did not list permissions in its
response, and lldb treated the first region returned as the one it would
use. (the stub also didn't support the memory-allocate packet)

Added: 


Modified: 
lldb/docs/resources/lldbgdbremote.md
lldb/source/Expression/IRMemoryMap.cpp

Removed: 




diff  --git a/lldb/docs/resources/lldbgdbremote.md 
b/lldb/docs/resources/lldbgdbremote.md
index 7076a75032dae..5cac3736337a8 100644
--- a/lldb/docs/resources/lldbgdbremote.md
+++ b/lldb/docs/resources/lldbgdbremote.md
@@ -1403,6 +1403,12 @@ For instance, with a macOS process which has nothing 
mapped in the first
 The lack of `permissions:` indicates that none of read/write/execute are valid
 for this region.
 
+The stub must include `permissions:` key-value on all memory ranges
+that are valid to access in the inferior process -- the lack of
+`permissions:` means that this is an inaccessible (no page table
+entries exist, in a system using VM) memory range.  If a stub cannot
+determine actual permissions, return `rwx`.
+
 **Priority To Implement:** Medium
 
 This is nice to have, but it isn't necessary. It helps LLDB
@@ -2434,4 +2440,4 @@ The `0x` prefixes are optional - like most of the 
gdb-remote packets,
 omitting them will work fine; these numbers are always base 16.
 
 The length of the payload is not provided.  A reliable, 8-bit clean,
-transport layer is assumed.
\ No newline at end of file
+transport layer is assumed.

diff  --git a/lldb/source/Expression/IRMemoryMap.cpp 
b/lldb/source/Expression/IRMemoryMap.cpp
index de631370bb048..0c1d9016616cb 100644
--- a/lldb/source/Expression/IRMemoryMap.cpp
+++ b/lldb/source/Expression/IRMemoryMap.cpp
@@ -84,7 +84,7 @@ lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
   // any allocations.  Otherwise start at the beginning of memory.
 
   if (m_allocations.empty()) {
-ret = 0x0;
+ret = 0;
   } else {
 auto back = m_allocations.rbegin();
 lldb::addr_t addr = back->first;
@@ -116,10 +116,18 @@ lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
 Status err = process_sp->GetMemoryRegionInfo(ret, region_info);
 if (err.Success()) {
   while (true) {
-if (region_info.GetReadable() != MemoryRegionInfo::OptionalBool::eNo ||
-region_info.GetWritable() != MemoryRegionInfo::OptionalBool::eNo ||
-region_info.GetExecutable() !=
-MemoryRegionInfo::OptionalBool::eNo) {
+if (region_info.GetRange().GetRangeBase() == 0 &&
+region_info.GetRange().GetRangeEnd() < end_of_memory) {
+  // Don't use a region that starts at address 0,
+  // it can make it harder to debug null dereference crashes
+  // in the inferior.
+  ret = region_info.GetRange().GetRangeEnd();
+} else if (region_info.GetReadable() !=
+   MemoryRegionInfo::OptionalBool::eNo ||
+   region_info.GetWritable() !=
+   MemoryRegionInfo::OptionalBool::eNo ||
+   region_info.GetExecutable() !=
+   MemoryRegionInfo::OptionalBool::eNo) {
   if (region_info.GetRange().GetRangeEnd() - 1 >= end_of_memory) {
 ret = LLDB_INVALID_ADDRESS;
 break;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Don't use a vm addr range starting at 0 for local memory (PR #100288)

2024-07-24 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda closed 
https://github.com/llvm/llvm-project/pull/100288
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Set breakpoint condition on breakpoint instead of location in TestBreakpointConditions (PR #100487)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland updated 
https://github.com/llvm/llvm-project/pull/100487

>From 29d5a57eb8cc344e1a93787fe9cb333761923927 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:24:24 -0700
Subject: [PATCH 1/2] [lldb][test][x86_64][win] Split TestBreakpointConditions
 assertion to clarify failure message

When this test fails we see an assertion error `False != True`
This clarifies the error by showing, for example, if `1 != 3` when
comparing `var` to the string "3".
---
 .../breakpoint_conditions/TestBreakpointConditions.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 50ba0317fd094..4e7a8ccb9fbeb 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -176,11 +176,15 @@ def breakpoint_conditions_python(self):
 thread.IsValid(),
 "There should be a thread stopped due to breakpoint condition",
 )
+
 frame0 = thread.GetFrameAtIndex(0)
 var = frame0.FindValue("val", lldb.eValueTypeVariableArgument)
-self.assertTrue(
-frame0.GetLineEntry().GetLine() == self.line1 and var.GetValue() 
== "3"
+self.assertEqual(
+frame0.GetLineEntry().GetLine(),
+self.line1,
+"The debugger stopped on the correct line",
 )
+self.assertEqual(var.GetValue(), "3")
 
 # The hit count for the breakpoint should be 1.
 self.assertEqual(breakpoint.GetHitCount(), 1)

>From 4285ea5e40b92d11748272b763a91b2de180bd24 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:47:26 -0700
Subject: [PATCH 2/2] [lldb][test][x86_64][win] TestBreakpointConditions set
 condition on breakpoint instead of location

On windows x86_64 this test stops on the set breakpoint
when val == 1 when the breakpoint condition is set on the
SBBreakpointLocation rather than the SBBreakpoint directly.
Setting the condition on the breakpoint itself, seems to fix the issue.

This PR also splits the test assertion that verifies we're
on the correct line and have the correct value of val to make the
error message more clear. At present it just shows Assertion error: True != 
False
---
 .../TestBreakpointConditions.py   | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 4e7a8ccb9fbeb..d202c82ea12a1 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -21,6 +21,17 @@ def test_breakpoint_condition_inline_and_run_command(self):
 
 @add_test_categories(["pyapi"])
 def test_breakpoint_condition_and_python_api(self):
+"""Use Python APIs to set breakpoint conditions."""
+self.build()
+self.breakpoint_conditions_python(set_breakpoint_on_location=False)
+
+@add_test_categories(["pyapi"])
+@expectedFailureAll(
+oslist=["windows"],
+archs=["x86_64"],
+bugnumber="https://github.com/llvm/llvm-project/issues/100486";,
+)
+def test_breakpoint_condition_on_location_and_python_api(self):
 """Use Python APIs to set breakpoint conditions."""
 self.build()
 self.breakpoint_conditions_python()
@@ -124,7 +135,7 @@ def breakpoint_conditions(self, inline=False):
 
 self.runCmd("process kill")
 
-def breakpoint_conditions_python(self):
+def breakpoint_conditions_python(self, set_breakpoint_on_location=True):
 """Use Python APIs to set breakpoint conditions."""
 target = self.createTestTarget()
 
@@ -158,10 +169,14 @@ def breakpoint_conditions_python(self):
 # Get the breakpoint location from breakpoint after we verified that,
 # indeed, it has one location.
 location = breakpoint.GetLocationAtIndex(0)
-self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
 
-# Set the condition on the breakpoint location.
-location.SetCondition("val == 3")
+# Set the condition.
+if set_breakpoint_on_location:
+location.SetCondition("val == 3")
+else:
+breakpoint.SetCondition("val == 3")
+
+self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
 self.expect(location.GetCondition(), exe=False, startstr="val == 3")
 
 # Now launch the process, and do not stop at entry point.

__

[Lldb-commits] [lldb] [lldb] IRMemoryMap zero address mapping fix (PR #99045)

2024-07-24 Thread Jason Molenda via lldb-commits


jasonmolenda wrote:

I've merged https://github.com/llvm/llvm-project/pull/100288 I think we should 
close this PR.

https://github.com/llvm/llvm-project/pull/99045
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Set breakpoint condition on breakpoint instead of location in TestBreakpointConditions (PR #100487)

2024-07-24 Thread Kendal Harland via lldb-commits


kendalharland wrote:

Uploaded a new patch set after realizing my original commit removed tests for 
the codepath where `SBBReakpointLocation.SetCondition` is used, which still 
needs to be tested on aarch64. The latest commit tests both 
`SBBreakpointLocation.SetCondition` and `SBBreakpoint.SetCondition`

https://github.com/llvm/llvm-project/pull/100487
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Set breakpoint condition on breakpoint instead of location in TestBreakpointConditions (PR #100487)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland updated 
https://github.com/llvm/llvm-project/pull/100487

>From 29d5a57eb8cc344e1a93787fe9cb333761923927 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:24:24 -0700
Subject: [PATCH 1/2] [lldb][test][x86_64][win] Split TestBreakpointConditions
 assertion to clarify failure message

When this test fails we see an assertion error `False != True`
This clarifies the error by showing, for example, if `1 != 3` when
comparing `var` to the string "3".
---
 .../breakpoint_conditions/TestBreakpointConditions.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 50ba0317fd094..4e7a8ccb9fbeb 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -176,11 +176,15 @@ def breakpoint_conditions_python(self):
 thread.IsValid(),
 "There should be a thread stopped due to breakpoint condition",
 )
+
 frame0 = thread.GetFrameAtIndex(0)
 var = frame0.FindValue("val", lldb.eValueTypeVariableArgument)
-self.assertTrue(
-frame0.GetLineEntry().GetLine() == self.line1 and var.GetValue() 
== "3"
+self.assertEqual(
+frame0.GetLineEntry().GetLine(),
+self.line1,
+"The debugger stopped on the correct line",
 )
+self.assertEqual(var.GetValue(), "3")
 
 # The hit count for the breakpoint should be 1.
 self.assertEqual(breakpoint.GetHitCount(), 1)

>From 25240b86822a3582d843a482fef16322f2fd7528 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:47:26 -0700
Subject: [PATCH 2/2] [lldb][test][x86_64][win] TestBreakpointConditions set
 condition on breakpoint instead of location

On windows x86_64 this test stops on the set breakpoint
when val == 1 when the breakpoint condition is set on the
SBBreakpointLocation rather than the SBBreakpoint directly.
Setting the condition on the breakpoint itself, seems to fix the issue.

This PR also splits the test assertion that verifies we're
on the correct line and have the correct value of val to make the
error message more clear. At present it just shows Assertion error: True != 
False
---
 .../TestBreakpointConditions.py   | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 4e7a8ccb9fbeb..d202c82ea12a1 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -21,6 +21,17 @@ def test_breakpoint_condition_inline_and_run_command(self):
 
 @add_test_categories(["pyapi"])
 def test_breakpoint_condition_and_python_api(self):
+"""Use Python APIs to set breakpoint conditions."""
+self.build()
+self.breakpoint_conditions_python(set_breakpoint_on_location=False)
+
+@add_test_categories(["pyapi"])
+@expectedFailureAll(
+oslist=["windows"],
+archs=["x86_64"],
+bugnumber="https://github.com/llvm/llvm-project/issues/100486";,
+)
+def test_breakpoint_condition_on_location_and_python_api(self):
 """Use Python APIs to set breakpoint conditions."""
 self.build()
 self.breakpoint_conditions_python()
@@ -124,7 +135,7 @@ def breakpoint_conditions(self, inline=False):
 
 self.runCmd("process kill")
 
-def breakpoint_conditions_python(self):
+def breakpoint_conditions_python(self, set_breakpoint_on_location=True):
 """Use Python APIs to set breakpoint conditions."""
 target = self.createTestTarget()
 
@@ -158,10 +169,14 @@ def breakpoint_conditions_python(self):
 # Get the breakpoint location from breakpoint after we verified that,
 # indeed, it has one location.
 location = breakpoint.GetLocationAtIndex(0)
-self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
 
-# Set the condition on the breakpoint location.
-location.SetCondition("val == 3")
+# Set the condition.
+if set_breakpoint_on_location:
+location.SetCondition("val == 3")
+else:
+breakpoint.SetCondition("val == 3")
+
+self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
 self.expect(location.GetCondition(), exe=False, startstr="val == 3")
 
 # Now launch the process, and do not stop at entry point.

__

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Set breakpoint condition on breakpoint instead of location in TestBreakpointConditions (PR #100487)

2024-07-24 Thread Kendal Harland via lldb-commits


https://github.com/kendalharland updated 
https://github.com/llvm/llvm-project/pull/100487

>From 29d5a57eb8cc344e1a93787fe9cb333761923927 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:24:24 -0700
Subject: [PATCH 1/2] [lldb][test][x86_64][win] Split TestBreakpointConditions
 assertion to clarify failure message

When this test fails we see an assertion error `False != True`
This clarifies the error by showing, for example, if `1 != 3` when
comparing `var` to the string "3".
---
 .../breakpoint_conditions/TestBreakpointConditions.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 50ba0317fd094..4e7a8ccb9fbeb 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -176,11 +176,15 @@ def breakpoint_conditions_python(self):
 thread.IsValid(),
 "There should be a thread stopped due to breakpoint condition",
 )
+
 frame0 = thread.GetFrameAtIndex(0)
 var = frame0.FindValue("val", lldb.eValueTypeVariableArgument)
-self.assertTrue(
-frame0.GetLineEntry().GetLine() == self.line1 and var.GetValue() 
== "3"
+self.assertEqual(
+frame0.GetLineEntry().GetLine(),
+self.line1,
+"The debugger stopped on the correct line",
 )
+self.assertEqual(var.GetValue(), "3")
 
 # The hit count for the breakpoint should be 1.
 self.assertEqual(breakpoint.GetHitCount(), 1)

>From beef815e7d6a09c358b1a9cbed6242ddf160a599 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Tue, 23 Jul 2024 10:47:26 -0700
Subject: [PATCH 2/2] [lldb][test][x86_64][win] TestBreakpointConditions set
 condition on breakpoint instead of location

On windows x86_64 this test stops on the set breakpoint
when val == 1 when the breakpoint condition is set on the
SBBreakpointLocation rather than the SBBreakpoint directly.
Setting the condition on the breakpoint itself, seems to fix the issue.

This PR also splits the test assertion that verifies we're
on the correct line and have the correct value of val to make the
error message more clear. At present it just shows Assertion error: True != 
False
---
 .../TestBreakpointConditions.py   | 22 ---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
index 4e7a8ccb9fbeb..96d546c01236d 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
@@ -21,6 +21,17 @@ def test_breakpoint_condition_inline_and_run_command(self):
 
 @add_test_categories(["pyapi"])
 def test_breakpoint_condition_and_python_api(self):
+"""Use Python APIs to set breakpoint conditions."""
+self.build()
+self.breakpoint_conditions_python(set_breakpoint_on_location=False)
+
+@add_test_categories(["pyapi"])
+@expectedFailureAll(
+oslist=["windows"],
+archs=["x86_64"],
+bugnumber="https://github.com/llvm/llvm-project/issues/100486";,
+)
+def test_breakpoint_condition_on_location_and_python_api(self):
 """Use Python APIs to set breakpoint conditions."""
 self.build()
 self.breakpoint_conditions_python()
@@ -124,7 +135,7 @@ def breakpoint_conditions(self, inline=False):
 
 self.runCmd("process kill")
 
-def breakpoint_conditions_python(self):
+def breakpoint_conditions_python(self, set_breakpoint_on_location=True):
 """Use Python APIs to set breakpoint conditions."""
 target = self.createTestTarget()
 
@@ -160,8 +171,13 @@ def breakpoint_conditions_python(self):
 location = breakpoint.GetLocationAtIndex(0)
 self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
 
-# Set the condition on the breakpoint location.
-location.SetCondition("val == 3")
+# Set the condition.
+if set_breakpoint_on_location:
+location.SetCondition("val == 3")
+else:
+breakpoint.SetCondition("val == 3")
+
+self.assertTrue(location and location.IsEnabled(), 
VALID_BREAKPOINT_LOCATION)
 self.expect(location.GetCondition(), exe=False, startstr="val == 3")
 
 # Now launch the process, and do not stop at entry point.

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailm

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda commented:

The MachO changes look fine to me.  I had a few other small pieces of feedback, 
I think they're mostly matters of opinion so just take them as such, not 
something that must be changed.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jason Molenda via lldb-commits


https://github.com/jasonmolenda edited 
https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jason Molenda via lldb-commits



@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+  Status error;
+  if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
+return error;
+  }
+
+  if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+  }
+
+  if (m_process_sp.has_value())
+ClearProcessSpecificData();
+
+  m_process_sp = process_sp;
+  return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+  Status error;
+  if (!thread) {
+error.SetErrorString("Thread is null");
+  }
+
+  if (!m_process_sp.has_value())
+m_process_sp = thread->GetProcess();
+
+  if (m_process_sp.value() != thread->GetProcess()) {
+error.SetErrorString("Cannot add thread from a different process.");
+return error;
+  }
+
+  std::pair tid_pair(thread->GetID(),
+  thread->GetBackingThread());
+  m_threads_to_save.insert(tid_pair);

jasonmolenda wrote:

personal preference, but you could ` m_threads_to_save.insert({thread->GetID(), 
thread->GetBackingThread()})` here.  Fine this way too.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jason Molenda via lldb-commits



@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+  Status error;
+  if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
+return error;
+  }
+
+  if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+  }
+
+  if (m_process_sp.has_value())
+ClearProcessSpecificData();
+
+  m_process_sp = process_sp;
+  return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+  Status error;
+  if (!thread) {
+error.SetErrorString("Thread is null");

jasonmolenda wrote:

Do you want to early-return here?  We might dereference the `thread` a few 
lines below.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)

2024-07-24 Thread Jason Molenda via lldb-commits



@@ -46,8 +48,79 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+  Status error;
+  if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp = std::nullopt;
+return error;
+  }
+
+  if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+  }
+
+  if (m_process_sp.has_value())
+ClearProcessSpecificData();
+
+  m_process_sp = process_sp;
+  return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb_private::Thread *thread) {
+  Status error;
+  if (!thread) {
+error.SetErrorString("Thread is null");
+  }
+
+  if (!m_process_sp.has_value())
+m_process_sp = thread->GetProcess();
+
+  if (m_process_sp.value() != thread->GetProcess()) {
+error.SetErrorString("Cannot add thread from a different process.");
+return error;
+  }
+
+  std::pair tid_pair(thread->GetID(),
+  thread->GetBackingThread());
+  m_threads_to_save.insert(tid_pair);
+  return error;
+}
+
+bool SaveCoreOptions::RemoveThread(lldb_private::Thread *thread) {
+  return thread && m_threads_to_save.erase(thread->GetID()) > 0;
+}
+
+bool SaveCoreOptions::ShouldSaveThread(lldb::tid_t tid) const {

jasonmolenda wrote:

`ShouldSaveThread` makes this method sound like a caller can use this to 
request a thread is included. This is more like `ThreadWillBeSaved` maybe?

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

1 2 >

1 - 100 of 124 matches

Mail list logo