[Lldb-commits] [lldb] Improve type lookup using .debug_names parent chain (PR #108907)

2024-09-25 Thread via lldb-commits

https://github.com/jeffreytan81 updated 
https://github.com/llvm/llvm-project/pull/108907

>From 6e84ab9a14e63c58e1facdbf9a695c093882b37b Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Mon, 19 Aug 2024 10:57:35 -0700
Subject: [PATCH 1/2] Fix StartDebuggingRequestHandler/ReplModeRequestHandler
 in lldb-dap

---
 lldb/tools/lldb-dap/DAP.h| 2 --
 lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57562a14983519..7828272aa15a7d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -192,8 +192,6 @@ struct DAP {
   std::mutex call_mutex;
   std::map
   inflight_reverse_requests;
-  StartDebuggingRequestHandler start_debugging_request_handler;
-  ReplModeRequestHandler repl_mode_request_handler;
   ReplMode repl_mode;
   std::string command_escape_prefix = "`";
   lldb::SBFormat frame_format;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..f50a6c17310739 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object 
&request) {
   "lldb-dap", "Commands for managing lldb-dap.");
   if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) {
 cmd.AddCommand(
-"startDebugging", &g_dap.start_debugging_request_handler,
+"startDebugging", new StartDebuggingRequestHandler(),
 "Sends a startDebugging request from the debug adapter to the client "
 "to start a child debug session of the same type as the caller.");
   }
   cmd.AddCommand(
-  "repl-mode", &g_dap.repl_mode_request_handler,
+  "repl-mode", new ReplModeRequestHandler(),
   "Get or set the repl behavior of lldb-dap evaluation requests.");
 
   g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction);

>From bb42917c2bea4cde9f7b48685d85df89f0bc7eb8 Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Mon, 23 Sep 2024 16:40:16 -0700
Subject: [PATCH 2/2] Improve type query using .debug_names parent chain

---
 .../Plugins/SymbolFile/DWARF/DWARFIndex.cpp   |  25 
 .../Plugins/SymbolFile/DWARF/DWARFIndex.h |  12 ++
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 128 +-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  34 +
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |   5 +-
 5 files changed, 201 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index eafddbad03f57b..2b454d1e676098 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -126,3 +126,28 @@ bool DWARFIndex::GetFullyQualifiedTypeImpl(
 return callback(die);
   return true;
 }
+
+void DWARFIndex::GetTypesWithQuery(
+TypeQuery &query, llvm::function_ref callback) {
+  GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) {
+return ProcessTypeDieMatchQuery(query, die, callback);
+  });
+}
+
+bool DWARFIndex::ProcessTypeDieMatchQuery(
+TypeQuery &query, DWARFDIE die,
+llvm::function_ref callback) {
+  // Nothing to match from query
+  if (query.GetContextRef().size() <= 1)
+return callback(die);
+
+  std::vector die_context;
+  if (query.GetModuleSearch())
+die_context = die.GetDeclContext();
+  else
+die_context = die.GetTypeLookupContext();
+
+  if (!query.ContextMatches(die_context))
+return true;
+  return callback(die);
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index cb3ae8a06d7885..d87aca8ef8bf63 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -64,6 +64,13 @@ class DWARFIndex {
   virtual void
   GetNamespaces(ConstString name,
 llvm::function_ref callback) = 0;
+  /// Get type DIEs meeting requires of \a query.
+  /// in its decl parent chain as subset.  A base implementation is provided,
+  /// Specializations should override this if they are able to provide a faster
+  /// implementation.
+  virtual void
+  GetTypesWithQuery(TypeQuery &query,
+llvm::function_ref callback);
   virtual void
   GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf,
const CompilerDeclContext &parent_decl_ctx,
@@ -115,6 +122,11 @@ class DWARFIndex {
   bool
   GetFullyQualifiedTypeImpl(const DWARFDeclContext &context, DWARFDIE die,
 llvm::function_ref callback);
+
+  /// Check if the type \a die can meet the requirements of \a query.
+  bool
+  ProcessTypeDieMatchQuery(TypeQuery &query, DWARFDIE die,
+   llvm::function_ref callback);
 };
 } // namespace dwarf
 } // namespace lldb_private::plugin
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/Deb

[Lldb-commits] [lldb] Improve type lookup using .debug_names parent chain (PR #108907)

2024-09-25 Thread via lldb-commits

https://github.com/jeffreytan81 updated 
https://github.com/llvm/llvm-project/pull/108907

>From 6e84ab9a14e63c58e1facdbf9a695c093882b37b Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Mon, 19 Aug 2024 10:57:35 -0700
Subject: [PATCH 1/2] Fix StartDebuggingRequestHandler/ReplModeRequestHandler
 in lldb-dap

---
 lldb/tools/lldb-dap/DAP.h| 2 --
 lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57562a14983519..7828272aa15a7d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -192,8 +192,6 @@ struct DAP {
   std::mutex call_mutex;
   std::map
   inflight_reverse_requests;
-  StartDebuggingRequestHandler start_debugging_request_handler;
-  ReplModeRequestHandler repl_mode_request_handler;
   ReplMode repl_mode;
   std::string command_escape_prefix = "`";
   lldb::SBFormat frame_format;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..f50a6c17310739 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object 
&request) {
   "lldb-dap", "Commands for managing lldb-dap.");
   if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) {
 cmd.AddCommand(
-"startDebugging", &g_dap.start_debugging_request_handler,
+"startDebugging", new StartDebuggingRequestHandler(),
 "Sends a startDebugging request from the debug adapter to the client "
 "to start a child debug session of the same type as the caller.");
   }
   cmd.AddCommand(
-  "repl-mode", &g_dap.repl_mode_request_handler,
+  "repl-mode", new ReplModeRequestHandler(),
   "Get or set the repl behavior of lldb-dap evaluation requests.");
 
   g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction);

>From 3f92956c6f35db7cb4762edf6ce2c93fa267b308 Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Mon, 23 Sep 2024 16:40:16 -0700
Subject: [PATCH 2/2] Improve type query using .debug_names parent chain

---
 .../Plugins/SymbolFile/DWARF/DWARFIndex.cpp   |  25 
 .../Plugins/SymbolFile/DWARF/DWARFIndex.h |  12 ++
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 123 +-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  34 +
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |   5 +-
 5 files changed, 196 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index eafddbad03f57b..2b454d1e676098 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -126,3 +126,28 @@ bool DWARFIndex::GetFullyQualifiedTypeImpl(
 return callback(die);
   return true;
 }
+
+void DWARFIndex::GetTypesWithQuery(
+TypeQuery &query, llvm::function_ref callback) {
+  GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) {
+return ProcessTypeDieMatchQuery(query, die, callback);
+  });
+}
+
+bool DWARFIndex::ProcessTypeDieMatchQuery(
+TypeQuery &query, DWARFDIE die,
+llvm::function_ref callback) {
+  // Nothing to match from query
+  if (query.GetContextRef().size() <= 1)
+return callback(die);
+
+  std::vector die_context;
+  if (query.GetModuleSearch())
+die_context = die.GetDeclContext();
+  else
+die_context = die.GetTypeLookupContext();
+
+  if (!query.ContextMatches(die_context))
+return true;
+  return callback(die);
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index cb3ae8a06d7885..d87aca8ef8bf63 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -64,6 +64,13 @@ class DWARFIndex {
   virtual void
   GetNamespaces(ConstString name,
 llvm::function_ref callback) = 0;
+  /// Get type DIEs meeting requires of \a query.
+  /// in its decl parent chain as subset.  A base implementation is provided,
+  /// Specializations should override this if they are able to provide a faster
+  /// implementation.
+  virtual void
+  GetTypesWithQuery(TypeQuery &query,
+llvm::function_ref callback);
   virtual void
   GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf,
const CompilerDeclContext &parent_decl_ctx,
@@ -115,6 +122,11 @@ class DWARFIndex {
   bool
   GetFullyQualifiedTypeImpl(const DWARFDeclContext &context, DWARFDIE die,
 llvm::function_ref callback);
+
+  /// Check if the type \a die can meet the requirements of \a query.
+  bool
+  ProcessTypeDieMatchQuery(TypeQuery &query, DWARFDIE die,
+   llvm::function_ref callback);
 };
 } // namespace dwarf
 } // namespace lldb_private::plugin
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/Deb

[Lldb-commits] [lldb] Improve type lookup using .debug_names parent chain (PR #108907)

2024-09-25 Thread via lldb-commits


@@ -301,7 +301,8 @@ using Entry = llvm::DWARFDebugNames::Entry;
 /// If any parent does not have an `IDX_parent`, or the Entry data is 
corrupted,
 /// nullopt is returned.
 std::optional>
-getParentChain(Entry entry, uint32_t max_parents) {
+getParentChain(Entry entry,
+   uint32_t max_parents = std::numeric_limits::max()) {
   llvm::SmallVector parent_entries;

jeffreytan81 wrote:

Do you know what branch in this function will cut short during "intermediate 
types is in a type unit" situation? Is it "entry.hasParentInformation()" 
returning false, or "entry.getParentDIEEntry()" return failure? 

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


[Lldb-commits] [lldb] Add warning message to `session save` when transcript isn't saved. (PR #109020)

2024-09-25 Thread Tom Yang via lldb-commits


@@ -3306,6 +3306,8 @@ bool CommandInterpreter::SaveTranscript(
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
   result.AppendMessageWithFormat("Session's transcripts saved to %s\n",
  output_file->c_str());
+  if (!GetSaveTranscript())
+result.AppendError("Note: the setting interpreter.save-transcript is set 
to false, so the transcript might not have been recorded.");

zhyty wrote:

@medismailben friendly ping on any followup comments. If not, I'll go ahead and 
merge.

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


[Lldb-commits] [lldb] 3d04695 - [lldb] fix one-off error in vformat specifier

2024-09-25 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2024-09-25T21:36:51-07:00
New Revision: 3d0469516c687b6dad3e6482fd94d64c65fa4a01

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

LOG: [lldb] fix one-off error in vformat specifier

Results in an assert at runtime, when run on an improperly formed
corefile.

rdar://136659551

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 06da83e26a26a5..b542e237f023d4 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -1409,7 +1409,7 @@ void ObjectFileMachO::SanitizeSegmentCommand(
 seg_cmd.cmd == LC_SEGMENT_64 ? "LC_SEGMENT_64" : "LC_SEGMENT";
 GetModule()->ReportWarning(
 "load command {0} {1} has a fileoff + filesize ({2:x16}) that "
-"extends beyond the end of the file ({4:x16}), the segment will be "
+"extends beyond the end of the file ({3:x16}), the segment will be "
 "truncated to match",
 cmd_idx, lc_segment_name, seg_cmd.fileoff + seg_cmd.filesize, 
m_length);
 



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


[Lldb-commits] [lldb] 0f98497 - [lldb] [Mach-O corefiles] Sanity check malformed dyld

2024-09-25 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2024-09-25T21:51:38-07:00
New Revision: 0f984976897857a8f4003063be6fa38a733fa624

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

LOG: [lldb] [Mach-O corefiles] Sanity check malformed dyld

lldb scans the corefile for dyld, the dynamic loader, and when it
finds a mach-o header that looks like dyld, it tries to read all
of the load commands and symbol table out of the corefile memory.
If the load comamnds and symbol table are absent or malformed,
it doesn't handle this case and can crash.  Back out when we
fail to create a Module from the dyld binary.

rdar://136659551

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 624848dee6ec33..30242038a5f66f 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -594,7 +594,7 @@ void 
DynamicLoaderDarwin::UpdateSpecialBinariesFromNewImageInfos(
   }
 }
 
-void DynamicLoaderDarwin::UpdateDYLDImageInfoFromNewImageInfo(
+bool DynamicLoaderDarwin::UpdateDYLDImageInfoFromNewImageInfo(
 ImageInfo &image_info) {
   if (image_info.header.filetype == llvm::MachO::MH_DYLINKER) {
 const bool can_create = true;
@@ -605,8 +605,10 @@ void 
DynamicLoaderDarwin::UpdateDYLDImageInfoFromNewImageInfo(
   target.GetImages().AppendIfNeeded(dyld_sp);
   UpdateImageLoadAddress(dyld_sp.get(), image_info);
   SetDYLDModule(dyld_sp);
+  return true;
 }
   }
+  return false;
 }
 
 std::optional DynamicLoaderDarwin::GetStartAddress() {

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
index 3613c4c29b1785..45c693163f8105 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
@@ -208,7 +208,7 @@ class DynamicLoaderDarwin : public 
lldb_private::DynamicLoader {
   UpdateSpecialBinariesFromNewImageInfos(ImageInfo::collection &image_infos);
 
   // if image_info is a dyld binary, call this method
-  void UpdateDYLDImageInfoFromNewImageInfo(ImageInfo &image_info);
+  bool UpdateDYLDImageInfoFromNewImageInfo(ImageInfo &image_info);
 
   // If image_infos contains / may contain executable image, call this method
   // to keep our internal record keeping of the special dyld binary up-to-date.

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index fe0224483b7c21..debd0f6ee83f40 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -259,10 +259,13 @@ bool 
DynamicLoaderMacOSXDYLD::ReadDYLDInfoFromMemoryAndSetNotificationCallback(
   ModuleSP dyld_module_sp;
   if (ParseLoadCommands(data, m_dyld, &m_dyld.file_spec)) {
 if (m_dyld.file_spec) {
-  UpdateDYLDImageInfoFromNewImageInfo(m_dyld);
+  if (!UpdateDYLDImageInfoFromNewImageInfo(m_dyld))
+return false;
 }
   }
   dyld_module_sp = GetDYLDModule();
+  if (!dyld_module_sp)
+return false;
 
   Target &target = m_process->GetTarget();
 



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


[Lldb-commits] [lldb] [DWARF]Set uuid for symbol file spec (PR #110066)

2024-09-25 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

Change makes sense, but is there a way to test this? 

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


[Lldb-commits] [lldb] Improve namespace lookup review (PR #110062)

2024-09-25 Thread via lldb-commits

https://github.com/jeffreytan81 created 
https://github.com/llvm/llvm-project/pull/110062

## Summary
This PR is a continuation of https://github.com/llvm/llvm-project/pull/108907 
by using `.debug_names` parent chain faster lookup for namespaces.


## Implementation
Similar to https://github.com/llvm/llvm-project/pull/108907. This PR adds a new 
API: `GetNamespacesWithParents` in `DWARFIndex` base class. The API performs 
the same function as `GetNamespaces()` with additional filtering using parents 
`CompilerDeclContext`. A default implementation is given in `DWARFIndex` class 
which parses debug info and performs the matching. In the `DebugNameDWARFIndex` 
override, parents `CompilerDeclContext` is cross checked with parent chain in 
`.debug_names` for much faster filtering before fallback to base implementation 
for final filtering. 

## Performance Results
For the same benchmark used in 
https://github.com/llvm/llvm-project/pull/108907, this PR improves: 48s => 28s






>From 6e84ab9a14e63c58e1facdbf9a695c093882b37b Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Mon, 19 Aug 2024 10:57:35 -0700
Subject: [PATCH 1/3] Fix StartDebuggingRequestHandler/ReplModeRequestHandler
 in lldb-dap

---
 lldb/tools/lldb-dap/DAP.h| 2 --
 lldb/tools/lldb-dap/lldb-dap.cpp | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57562a14983519..7828272aa15a7d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -192,8 +192,6 @@ struct DAP {
   std::mutex call_mutex;
   std::map
   inflight_reverse_requests;
-  StartDebuggingRequestHandler start_debugging_request_handler;
-  ReplModeRequestHandler repl_mode_request_handler;
   ReplMode repl_mode;
   std::string command_escape_prefix = "`";
   lldb::SBFormat frame_format;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..f50a6c17310739 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1627,12 +1627,12 @@ void request_initialize(const llvm::json::Object 
&request) {
   "lldb-dap", "Commands for managing lldb-dap.");
   if (GetBoolean(arguments, "supportsStartDebuggingRequest", false)) {
 cmd.AddCommand(
-"startDebugging", &g_dap.start_debugging_request_handler,
+"startDebugging", new StartDebuggingRequestHandler(),
 "Sends a startDebugging request from the debug adapter to the client "
 "to start a child debug session of the same type as the caller.");
   }
   cmd.AddCommand(
-  "repl-mode", &g_dap.repl_mode_request_handler,
+  "repl-mode", new ReplModeRequestHandler(),
   "Get or set the repl behavior of lldb-dap evaluation requests.");
 
   g_dap.progress_event_thread = std::thread(ProgressEventThreadFunction);

>From 3f92956c6f35db7cb4762edf6ce2c93fa267b308 Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Mon, 23 Sep 2024 16:40:16 -0700
Subject: [PATCH 2/3] Improve type query using .debug_names parent chain

---
 .../Plugins/SymbolFile/DWARF/DWARFIndex.cpp   |  25 
 .../Plugins/SymbolFile/DWARF/DWARFIndex.h |  12 ++
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 123 +-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  34 +
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |   5 +-
 5 files changed, 196 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index eafddbad03f57b..2b454d1e676098 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -126,3 +126,28 @@ bool DWARFIndex::GetFullyQualifiedTypeImpl(
 return callback(die);
   return true;
 }
+
+void DWARFIndex::GetTypesWithQuery(
+TypeQuery &query, llvm::function_ref callback) {
+  GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) {
+return ProcessTypeDieMatchQuery(query, die, callback);
+  });
+}
+
+bool DWARFIndex::ProcessTypeDieMatchQuery(
+TypeQuery &query, DWARFDIE die,
+llvm::function_ref callback) {
+  // Nothing to match from query
+  if (query.GetContextRef().size() <= 1)
+return callback(die);
+
+  std::vector die_context;
+  if (query.GetModuleSearch())
+die_context = die.GetDeclContext();
+  else
+die_context = die.GetTypeLookupContext();
+
+  if (!query.ContextMatches(die_context))
+return true;
+  return callback(die);
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index cb3ae8a06d7885..d87aca8ef8bf63 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -64,6 +64,13 @@ class DWARFIndex {
   virtual void
   GetNamespaces(ConstString name,
 llvm::function_ref callback) = 0;
+  /// Get type DIEs meeting requires of \a query.
+  /// in its decl parent chain as subset.

[Lldb-commits] [lldb] Improve namespace lookup review (PR #110062)

2024-09-25 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)


Changes

## Summary
This PR is a continuation of https://github.com/llvm/llvm-project/pull/108907 
by using `.debug_names` parent chain faster lookup for namespaces.


## Implementation
Similar to https://github.com/llvm/llvm-project/pull/108907. This PR adds a new 
API: `GetNamespacesWithParents` in `DWARFIndex` base class. The API performs 
the same function as `GetNamespaces()` with additional filtering using parents 
`CompilerDeclContext`. A default implementation is given in `DWARFIndex` class 
which parses debug info and performs the matching. In the `DebugNameDWARFIndex` 
override, parents `CompilerDeclContext` is cross checked with parent chain in 
`.debug_names` for much faster filtering before fallback to base implementation 
for final filtering. 

## Performance Results
For the same benchmark used in 
https://github.com/llvm/llvm-project/pull/108907, this PR improves: 48s => 
28s






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


5 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp (+41) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h (+23) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
(+164-1) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h (+36) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+4-3) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index eafddbad03f57b..c6c5b8e8af1450 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
@@ -126,3 +126,44 @@ bool DWARFIndex::GetFullyQualifiedTypeImpl(
 return callback(die);
   return true;
 }
+
+void DWARFIndex::GetTypesWithQuery(
+TypeQuery &query, llvm::function_ref callback) {
+  GetTypes(query.GetTypeBasename(), [&](DWARFDIE die) {
+return ProcessTypeDieMatchQuery(query, die, callback);
+  });
+}
+
+bool DWARFIndex::ProcessTypeDieMatchQuery(
+TypeQuery &query, DWARFDIE die,
+llvm::function_ref callback) {
+  // Nothing to match from query
+  if (query.GetContextRef().size() <= 1)
+return callback(die);
+
+  std::vector die_context;
+  if (query.GetModuleSearch())
+die_context = die.GetDeclContext();
+  else
+die_context = die.GetTypeLookupContext();
+
+  if (!query.ContextMatches(die_context))
+return true;
+  return callback(die);
+}
+
+void DWARFIndex::GetNamespacesWithParents(
+ConstString name, const CompilerDeclContext &parent_decl_ctx,
+llvm::function_ref callback) {
+  GetNamespaces(name, [&](DWARFDIE die) {
+return ProcessNamespaceDieMatchParents(parent_decl_ctx, die, callback);
+  });
+}
+
+bool DWARFIndex::ProcessNamespaceDieMatchParents(
+const CompilerDeclContext &parent_decl_ctx, DWARFDIE die,
+llvm::function_ref callback) {
+  if (!SymbolFileDWARF::DIEInDeclContext(parent_decl_ctx, die))
+return true;
+  return callback(die);
+}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index cb3ae8a06d7885..91f4a8d4713543 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -64,6 +64,21 @@ class DWARFIndex {
   virtual void
   GetNamespaces(ConstString name,
 llvm::function_ref callback) = 0;
+  /// Get type DIEs meeting requires of \a query.
+  /// in its decl parent chain as subset.  A base implementation is provided,
+  /// Specializations should override this if they are able to provide a faster
+  /// implementation.
+  virtual void
+  GetTypesWithQuery(TypeQuery &query,
+llvm::function_ref callback);
+  /// Get namespace DIEs whose base name match \param name with \param
+  /// parent_decl_ctx in its decl parent chain.  A base implementation
+  /// is provided. Specializations should override this if they are able to
+  /// provide a faster implementation.
+  virtual void
+  GetNamespacesWithParents(ConstString name,
+   const CompilerDeclContext &parent_decl_ctx,
+   llvm::function_ref callback);
   virtual void
   GetFunctions(const Module::LookupInfo &lookup_info, SymbolFileDWARF &dwarf,
const CompilerDeclContext &parent_decl_ctx,
@@ -115,6 +130,14 @@ class DWARFIndex {
   bool
   GetFullyQualifiedTypeImpl(const DWARFDeclContext &context, DWARFDIE die,
 llvm::function_ref callback);
+
+  /// Check if the type \a die can meet the requirements of \a query.
+  bool
+  ProcessTypeDieMatchQuery(TypeQuery &query, DWARFDIE die,
+   llvm::function_ref callback);
+  bool ProcessNamespaceDieMatchParents(
+  const CompilerDeclContext &parent_decl_ctx, DWARFDIE die,
+  llvm::function_ref callback);
 };
 } // 

[Lldb-commits] [lldb] Improve namespace lookup review (PR #110062)

2024-09-25 Thread via lldb-commits

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


[Lldb-commits] [lldb] Improve namespace lookup review (PR #110062)

2024-09-25 Thread via lldb-commits

jeffreytan81 wrote:

This is a stack PR from https://github.com/llvm/llvm-project/pull/108907, so 
please review the new namespaces lookup changes and ignore the changes in 
https://github.com/llvm/llvm-project/pull/108907. Let me know if there is a way 
to only show the delta changes (not merged view). 

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


[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)

2024-09-25 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/106470

>From 9f4ba3fdb8b144736e51134ced3019a2c57ca861 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Wed, 28 Aug 2024 10:04:33 -0700
Subject: [PATCH 1/2] [lldb] Store expression evaluator diagnostics in an
 llvm::Error (NFC)

This patch is a reworking of Pete Lawrence's (@PortalPete) proposal
for better expression evaluator error messages:
https://github.com/llvm/llvm-project/pull/80938

This patch is preparatory patch for improving the rendering of
expression evaluator diagnostics. Currently diagnostics are rendered
into a string and the command interpreter layer then textually parses
words like "error:" to (sometimes) color the output accordingly. In
order to enable user interfaces to do better with diagnostics, we need
to store them in a machine-readable fromat. This patch does this by
adding a new llvm::Error kind wrapping a DiagnosticDetail struct that
is used when the error type is eErrorTypeExpression. Multiple
diagnostics are modeled using llvm::ErrorList.

Right now the extra information is not used by the CommandInterpreter,
this will be added in a follow-up patch!
---
 .../lldb/Expression/DiagnosticManager.h   |  89 +++
 lldb/include/lldb/Utility/Status.h|  23 ++--
 lldb/source/Breakpoint/BreakpointLocation.cpp |  11 +-
 lldb/source/Expression/DiagnosticManager.cpp  | 103 +++---
 lldb/source/Expression/ExpressionParser.cpp   |   5 +-
 lldb/source/Expression/UserExpression.cpp |  49 +
 lldb/source/Expression/UtilityFunction.cpp|  18 +--
 .../ExpressionParser/Clang/ClangDiagnostic.h  |   5 +-
 .../Clang/ClangExpressionParser.cpp   |  69 
 .../Clang/ClangUserExpression.h   |   2 +
 .../Plugins/Platform/POSIX/PlatformPOSIX.cpp  |  12 +-
 .../Platform/Windows/PlatformWindows.cpp  |  12 +-
 lldb/source/Target/Target.cpp |   3 +-
 lldb/source/Utility/Status.cpp|  29 +
 .../TestModulesCompileError.py|   2 +-
 .../Expression/DiagnosticManagerTest.cpp  |  22 +++-
 16 files changed, 292 insertions(+), 162 deletions(-)

diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h 
b/lldb/include/lldb/Expression/DiagnosticManager.h
index d49b7c99b114fb..3e363ee5c0139c 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -12,6 +12,9 @@
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Status.h"
+
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -20,6 +23,53 @@
 
 namespace lldb_private {
 
+/// A compiler-independent representation of a Diagnostic. Expression
+/// evaluation failures often have more than one diagnostic that a UI
+/// layer might want to render differently, for example to colorize
+/// it.
+///
+/// Running example:
+///   (lldb) expr 1+foo
+///   error: :1:3: use of undeclared identifier 'foo'
+///   1+foo
+/// ^
+struct DiagnosticDetail {
+  struct SourceLocation {
+FileSpec file;
+unsigned line = 0;
+uint16_t column = 0;
+uint16_t length = 0;
+bool in_user_input = false;
+  };
+  /// Contains {{}, 1, 3, 3, true} in the example above.
+  std::optional source_location;
+  /// Contains eSeverityError in the example above.
+  lldb::Severity severity = lldb::eSeverityInfo;
+  /// Contains "use of undeclared identifier 'x'" in the example above.
+  std::string message;
+  /// Contains the fully rendered error message.
+  std::string rendered;
+};
+
+/// An llvm::Error used to communicate diagnostics in Status. Multiple
+/// diagnostics may be chained in an llvm::ErrorList.
+class ExpressionError
+: public llvm::ErrorInfo {
+  std::string m_message;
+  std::vector m_details;
+
+public:
+  static char ID;
+  using llvm::ErrorInfo::ErrorInfo;
+  ExpressionError(lldb::ExpressionResults result, std::string msg,
+  std::vector details = {});
+  std::string message() const override;
+  llvm::ArrayRef GetDetail() const { return m_details; }
+  std::error_code convertToErrorCode() const override;
+  void log(llvm::raw_ostream &OS) const override;
+  std::unique_ptr Clone() const override;
+};
+
 enum DiagnosticOrigin {
   eDiagnosticOriginUnknown = 0,
   eDiagnosticOriginLLDB,
@@ -49,37 +99,28 @@ class Diagnostic {
 }
   }
 
-  Diagnostic(llvm::StringRef message, lldb::Severity severity,
- DiagnosticOrigin origin, uint32_t compiler_id)
-  : m_message(message), m_severity(severity), m_origin(origin),
-m_compiler_id(compiler_id) {}
-
-  Diagnostic(const Diagnostic &rhs)
-  : m_message(rhs.m_message), m_severity(rhs.m_severity),
-m_origin(rhs.m_origin), m_compiler_id(rhs.m_compiler_id) {}
+  Diagnostic(DiagnosticOrigin origin, uint32_t compiler_id,
+ DiagnosticDetail detail)
+  : m_origin(origin), m_compiler_id(com

[Lldb-commits] [lldb] [LLDB][ProcessELFCore] Add Description to ProcessELFCore/ELFThread stop reasons (PR #110065)

2024-09-25 Thread Jacob Lalonde via lldb-commits

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

This fixes a functionality gap with GDB, where GDB will properly decode the 
stop reason and give the address for SIGSEGV. I also added descriptions to all 
stop reasons, following the same code path that the Native Linux Thread uses.

>From 0e31c0ad2b9db1ba9055bb4b984557100c0a9feb Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Tue, 24 Sep 2024 17:42:49 -0700
Subject: [PATCH 1/3] Add the addr info when appropriate for NIX' crash signals

---
 .../Process/elf-core/ProcessElfCore.cpp   |  1 +
 .../Process/elf-core/ThreadElfCore.cpp| 44 ---
 .../Plugins/Process/elf-core/ThreadElfCore.h  | 18 ++--
 .../postmortem/elf-core/TestLinuxCore.py  | 17 +++
 .../elf-core/linux-x86_64-sigsev.yaml |  8 
 5 files changed, 80 insertions(+), 8 deletions(-)
 create mode 100644 
lldb/test/API/functionalities/postmortem/elf-core/linux-x86_64-sigsev.yaml

diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 7955594bf5d94c..468a3b8934e741 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -931,6 +931,7 @@ llvm::Error 
ProcessElfCore::parseLinuxNotes(llvm::ArrayRef notes) {
 return status.ToError();
   thread_data.signo = siginfo.si_signo;
   thread_data.code = siginfo.si_code;
+  thread_data.description = siginfo.GetDescription();
   break;
 }
 case ELF::NT_FILE: {
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 52b96052bdbeca..3260caabd70ac6 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -6,9 +6,11 @@
 //
 
//===--===//
 
+#include "Plugins/Process/POSIX/CrashReason.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/UnixSignals.h"
 #include "lldb/Target/Unwind.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -41,6 +43,7 @@
 #include "RegisterContextPOSIXCore_x86_64.h"
 #include "ThreadElfCore.h"
 
+#include "bits/types/siginfo_t.h"
 #include 
 
 using namespace lldb;
@@ -49,8 +52,8 @@ using namespace lldb_private;
 // Construct a Thread object with given data
 ThreadElfCore::ThreadElfCore(Process &process, const ThreadData &td)
 : Thread(process, td.tid), m_thread_name(td.name), m_thread_reg_ctx_sp(),
-  m_signo(td.signo), m_code(td.code), m_gpregset_data(td.gpregset),
-  m_notes(td.notes) {}
+  m_signo(td.signo), m_code(td.code), m_sig_description(td.description),
+  m_gpregset_data(td.gpregset), m_notes(td.notes) {}
 
 ThreadElfCore::~ThreadElfCore() { DestroyThread(); }
 
@@ -241,7 +244,7 @@ bool ThreadElfCore::CalculateStopInfo() {
 return false;
 
   SetStopInfo(StopInfo::CreateStopReasonWithSignal(
-  *this, m_signo, /*description=*/nullptr, m_code));
+  *this, m_signo, m_sig_description.c_str(), m_code));
   return true;
 }
 
@@ -543,7 +546,8 @@ size_t ELFLinuxSigInfo::GetSize(const 
lldb_private::ArchSpec &arch) {
 
 Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) 
{
   Status error;
-  if (GetSize(arch) > data.GetByteSize()) {
+  uint64_t size = GetSize(arch);
+  if (size > data.GetByteSize()) {
 error = Status::FromErrorStringWithFormat(
 "NT_SIGINFO size should be %zu, but the remaining bytes are: %" PRIu64,
 GetSize(arch), data.GetByteSize());
@@ -556,6 +560,36 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, 
const ArchSpec &arch) {
   si_signo = data.GetU32(&offset);
   si_errno = data.GetU32(&offset);
   si_code = data.GetU32(&offset);
+  // 64b ELF have a 4 byte pad.
+  if (data.GetAddressByteSize() == 8)
+offset += 4;
+  switch (si_signo) {
+case SIGFPE:
+case SIGILL:
+case SIGSEGV:
+case SIGBUS:
+case SIGTRAP:
+  addr = (void*)data.GetAddress(&offset);
+  addr_lsb = data.GetU16(&offset);
+  return error;
+default:
+  return error;
+  }
+}
 
-  return error;
+std::string ELFLinuxSigInfo::GetDescription() {
+  switch (si_signo) {
+case SIGFPE:
+case SIGILL:
+case SIGSEGV:
+case SIGBUS:
+case SIGTRAP:
+  return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription(
+si_signo, si_code,
+reinterpret_cast(addr));
+default:
+  return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription(
+si_signo, si_code
+  );
+  }
 }
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
index 3fa0b8b0eedb0b..0dc21a10ded5e5 100644
--- a/lldb/source/Plugins/Process/elf

[Lldb-commits] [lldb] [LLDB][ProcessELFCore] Add Description to ProcessELFCore/ELFThread stop reasons (PR #110065)

2024-09-25 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)


Changes

This fixes a functionality gap with GDB, where GDB will properly decode the 
stop reason and give the address for SIGSEGV. I also added descriptions to all 
stop reasons, following the same code path that the Native Linux Thread uses.

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


3 Files Affected:

- (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp (+1) 
- (modified) lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp (+35-5) 
- (modified) lldb/source/Plugins/Process/elf-core/ThreadElfCore.h (+15-3) 


``diff
diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index 7955594bf5d94c..468a3b8934e741 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -931,6 +931,7 @@ llvm::Error 
ProcessElfCore::parseLinuxNotes(llvm::ArrayRef notes) {
 return status.ToError();
   thread_data.signo = siginfo.si_signo;
   thread_data.code = siginfo.si_code;
+  thread_data.description = siginfo.GetDescription();
   break;
 }
 case ELF::NT_FILE: {
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 52b96052bdbeca..e9ae5211c28a53 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/UnixSignals.h"
 #include "lldb/Target/Unwind.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -49,8 +50,8 @@ using namespace lldb_private;
 // Construct a Thread object with given data
 ThreadElfCore::ThreadElfCore(Process &process, const ThreadData &td)
 : Thread(process, td.tid), m_thread_name(td.name), m_thread_reg_ctx_sp(),
-  m_signo(td.signo), m_code(td.code), m_gpregset_data(td.gpregset),
-  m_notes(td.notes) {}
+  m_signo(td.signo), m_code(td.code), m_sig_description(td.description),
+  m_gpregset_data(td.gpregset), m_notes(td.notes) {}
 
 ThreadElfCore::~ThreadElfCore() { DestroyThread(); }
 
@@ -241,7 +242,7 @@ bool ThreadElfCore::CalculateStopInfo() {
 return false;
 
   SetStopInfo(StopInfo::CreateStopReasonWithSignal(
-  *this, m_signo, /*description=*/nullptr, m_code));
+  *this, m_signo, m_sig_description.c_str(), m_code));
   return true;
 }
 
@@ -543,7 +544,8 @@ size_t ELFLinuxSigInfo::GetSize(const 
lldb_private::ArchSpec &arch) {
 
 Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) 
{
   Status error;
-  if (GetSize(arch) > data.GetByteSize()) {
+  uint64_t size = GetSize(arch);
+  if (size > data.GetByteSize()) {
 error = Status::FromErrorStringWithFormat(
 "NT_SIGINFO size should be %zu, but the remaining bytes are: %" PRIu64,
 GetSize(arch), data.GetByteSize());
@@ -556,6 +558,34 @@ Status ELFLinuxSigInfo::Parse(const DataExtractor &data, 
const ArchSpec &arch) {
   si_signo = data.GetU32(&offset);
   si_errno = data.GetU32(&offset);
   si_code = data.GetU32(&offset);
+  // 64b ELF have a 4 byte pad.
+  if (data.GetAddressByteSize() == 8)
+offset += 4;
+  switch (si_signo) {
+  case SIGFPE:
+  case SIGILL:
+  case SIGSEGV:
+  case SIGBUS:
+  case SIGTRAP:
+addr = (void *)data.GetAddress(&offset);
+addr_lsb = data.GetU16(&offset);
+return error;
+  default:
+return error;
+  }
+}
 
-  return error;
+std::string ELFLinuxSigInfo::GetDescription() {
+  switch (si_signo) {
+  case SIGFPE:
+  case SIGILL:
+  case SIGSEGV:
+  case SIGBUS:
+  case SIGTRAP:
+return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription(
+si_signo, si_code, reinterpret_cast(addr));
+  default:
+return lldb_private::UnixSignals::CreateForHost()->GetSignalDescription(
+si_signo, si_code);
+  }
 }
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h 
b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
index 3fa0b8b0eedb0b..3c6c02f73efae8 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -75,16 +75,25 @@ struct ELFLinuxPrStatus {
 static_assert(sizeof(ELFLinuxPrStatus) == 112,
   "sizeof ELFLinuxPrStatus is not correct!");
 
+union ELFSigval {
+  int sival_int;
+  void *sival_ptr;
+};
+
 struct ELFLinuxSigInfo {
-  int32_t si_signo;
-  int32_t si_code;
+  int32_t si_signo; // Order matters for the first 3.
   int32_t si_errno;
+  int32_t si_code;
+  void *addr;   /* faulting insn/memory ref. */
+  int32_t addr_lsb; /* Valid LSB of the reported address.  */
 
   ELFLinuxSigInfo();
 
   lldb_private::Status Parse(const lldb_private::DataExtractor &data,
  

[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)

2024-09-25 Thread Adrian Prantl via lldb-commits

adrian-prantl wrote:

> > > This seems like it could be problematic for IDEs, if they don't print the 
> > > error in the same window as the expression being evaluated. The arrows 
> > > could end up pointing to nowhere, or to the wrong place in the expression 
> > > (if we don't get the right offset).
> > 
> > 
> > Eventually I want IDEs to get access to the same kind of machine-readable 
> > diagnostics, so they can format errors however they like (or, if they 
> > aren't interested, dump the pre-rendered textual error that is still 
> > available).
> 
> I'm still worried about this. Yes, the IDEs can dump the pre-rendered error 
> (and right now, that's all they can do), but this rendering assumes that the 
> error message will be printed in a particular way (right under the original 
> command, which will include the prompt and everything). I think that's fine 
> for the commands entered through the LLDB command line, but I don't think 
> it's reasonable to expect that every caller of 
> `SBCommandInterpreter::HandleCommand` will do the same thing. I've looked at 
> what lldb-dap does, and it looks like it will mostly be okay, because it 
> explicitly echoes the command to be executed (although it [hardcodes the 
> prompt 
> string](https://github.com/llvm/llvm-project/blob/60ed2361c0917b4f8d54cb85935cfbf8904aa51d/lldb/tools/lldb-dap/LLDBUtils.cpp#L61)
>  instead of getting it from lldb), but if you're looking for a example, you 
> don't need to look further than your test case. Even though you've formatted 
> the test input nicely, this is how its trace looks like when you run it:
> 
> ```
> Ran command:
> "expr -i 0 -o 0 -- a"
> 
> Got output:
>  ^
>  ╰─ error: use of undeclared identifier 'a'
> ```
> 
> (sure we can fix this so that the output makes sense, but I wonder how many 
> other such callers are out there)

Just to explain my motivation., in the end what I'm most interested in is 
exposing the machine-readable diagnostics through the SBAPI, so that is where 
this is going next.
I'm actually surprised that lldb-dap sees the new diagnostics because I thought 
it would run the expression through a lower-level API like 
SBFrame::EvaluateExpression(). But I guess it just provides a "terminal" window 
that passes everything through HandleCommand. To support this better, I'm 
thinking about adding a setting to opt into the inline diagnostics that only 
`lldb` sets, so we don't get unexpected results like this.

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


[Lldb-commits] [lldb] [DWARF]Set uuid for symbol file spec (PR #110066)

2024-09-25 Thread via lldb-commits

https://github.com/GeorgeHuyubo created 
https://github.com/llvm/llvm-project/pull/110066

The gnu build id was lost while creating the new module spec, this result in 
the SymbolLocatorDebuginfod not able to locate symbol file.

before this change while connecting to debuginfod server I am seeing this 
warning unable to locate debug file, also parameter not shown.
```
warning: (x86_64) 
/home/hyubo/.cache/llvm-debuginfod/client/llvmcache-18224759554195557949 unable 
to locate separate debug file (dwo, dwp). Debugging will be degraded. 
(troubleshoot with https://fburl.com/missing_dwo)
(lldb) bt
* thread #1, name = 'crasher', stop reason = signal SIGABRT
  * frame #0: 0x7fe9f909c993 libc.so.6`__GI___pthread_kill [inlined] 
__pthread_kill_internal(signo=6, threadid=) at pthread_kill.c:46:37
...
frame #4: 0x0020c46a 
llvmcache-18224759554195557949`c4crasher::Crasher::crasher(unsigned long) at 
crasher.h:428
...
```
After this change:
```
# no warnings
(lldb) bt
* thread #1, name = 'crasher', stop reason = signal SIGABRT
  * frame #0: 0x7fe9f909c993 libc.so.6`__GI___pthread_kill [inlined] 
__pthread_kill_internal(signo=6, threadid=) at pthread_kill.c:46:37
...
frame #4: 0x0020c46a 
llvmcache-18224759554195557949`c4crasher::Crasher::crasher(this=0x7fff44c51998,
 depth=0) at crasher.h:428
```

   

>From 3befdbeb8fdb5389891568550cf1d802a7e38535 Mon Sep 17 00:00:00 2001
From: George Hu 
Date: Wed, 25 Sep 2024 17:56:45 -0700
Subject: [PATCH] [DWARF]Set uuid for symbol file spec

---
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index a40d6d9e37978b..811f41702fb63a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4357,6 +4357,7 @@ const std::shared_ptr 
&SymbolFileDWARF::GetDwpSymbolFile() {
 FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
 ModuleSpec module_spec;
 module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
+module_spec.GetUUID() = m_objfile_sp->GetUUID();
 FileSpec dwp_filespec;
 for (const auto &symfile : symfiles.files()) {
   module_spec.GetSymbolFileSpec() =

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


[Lldb-commits] [lldb] [DWARF]Set uuid for symbol file spec (PR #110066)

2024-09-25 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (GeorgeHuyubo)


Changes

The gnu build id was lost while creating the new module spec, this result in 
the SymbolLocatorDebuginfod not able to locate symbol file.

before this change while connecting to debuginfod server I am seeing this 
warning unable to locate debug file, also parameter not shown.
```
warning: (x86_64) 
/home/hyubo/.cache/llvm-debuginfod/client/llvmcache-18224759554195557949 unable 
to locate separate debug file (dwo, dwp). Debugging will be degraded. 
(troubleshoot with https://fburl.com/missing_dwo)
(lldb) bt
* thread #1, name = 'crasher', stop reason = signal SIGABRT
  * frame #0: 0x7fe9f909c993 libc.so.6`__GI___pthread_kill 
[inlined] __pthread_kill_internal(signo=6, threadid=) at 
pthread_kill.c:46:37
...
frame #4: 0x0020c46a 
llvmcache-18224759554195557949`c4crasher::Crasher::crasher(unsigned long) at 
crasher.h:428
...
```
After this change:
```
# no warnings
(lldb) bt
* thread #1, name = 'crasher', stop reason = signal SIGABRT
  * frame #0: 0x7fe9f909c993 libc.so.6`__GI___pthread_kill 
[inlined] __pthread_kill_internal(signo=6, threadid=) at 
pthread_kill.c:46:37
...
frame #4: 0x0020c46a 
llvmcache-18224759554195557949`c4crasher::Crasher::crasher(this=0x7fff44c51998,
 depth=0) at crasher.h:428
```

   

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


1 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+1) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index a40d6d9e37978b..811f41702fb63a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4357,6 +4357,7 @@ const std::shared_ptr 
&SymbolFileDWARF::GetDwpSymbolFile() {
 FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
 ModuleSpec module_spec;
 module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
+module_spec.GetUUID() = m_objfile_sp->GetUUID();
 FileSpec dwp_filespec;
 for (const auto &symfile : symfiles.files()) {
   module_spec.GetSymbolFileSpec() =

``




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


[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

2024-09-25 Thread Adrian Prantl via lldb-commits


@@ -254,14 +256,46 @@ class ClangDiagnosticManagerAdapter : public 
clang::DiagnosticConsumer {
   std::string stripped_output =
   std::string(llvm::StringRef(m_output).trim());
 
-  auto new_diagnostic = std::make_unique(
-  stripped_output, severity, Info.getID());
+  // Translate the source location.
+  if (Info.hasSourceManager()) {
+DiagnosticDetail::SourceLocation loc;
+clang::SourceManager &sm = Info.getSourceManager();
+const clang::SourceLocation sloc = Info.getLocation();
+if (sloc.isValid()) {
+  const clang::FullSourceLoc fsloc(sloc, sm);
+  clang::PresumedLoc PLoc = fsloc.getPresumedLoc(true);
+  StringRef filename =
+  PLoc.isValid() ? PLoc.getFilename() : StringRef{};

adrian-prantl wrote:

That would not generally be correct. The expression source code can have source 
locations that are in any of a number of files, for example, you could trigger 
an error diagnostic in an STL header, or, a compiler-builtin with no source 
location.

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


[Lldb-commits] [lldb] [llvm] [lldb][DWARF] Replace lldb's DWARFDebugArangeSet with llvm's (PR #110058)

2024-09-25 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 df4f828938db807fc7c4611896fe9659af482e81 
8e1c59729905fb89a8764ace3dfa0d04119d273e --extensions h,cpp -- 
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp 
lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp 
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugArangeSet.h 
llvm/lib/DebugInfo/DWARF/DWARFDebugArangeSet.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
index 03ab45ac98..062fcc6dc5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
@@ -7,11 +7,11 @@
 
//===--===//
 
 #include "DWARFDebugAranges.h"
-#include "llvm/DebugInfo/DWARF/DWARFDebugArangeSet.h"
 #include "DWARFUnit.h"
 #include "LogChannelDWARF.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Timer.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugArangeSet.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -23,7 +23,8 @@ DWARFDebugAranges::DWARFDebugAranges() : m_aranges() {}
 
 // Extract
 void DWARFDebugAranges::extract(const DWARFDataExtractor &debug_aranges_data) {
-  llvm::DWARFDataExtractor llvm_dwarf_data = 
debug_aranges_data.GetAsLLVMDWARF();
+  llvm::DWARFDataExtractor llvm_dwarf_data =
+  debug_aranges_data.GetAsLLVMDWARF();
   lldb::offset_t offset = 0;
 
   DWARFDebugArangeSet set;
diff --git a/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp 
b/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
index 254f3ac8aa..d77f0a5074 100644
--- a/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -155,12 +155,12 @@ TEST_F(SymbolFileDWARFTests, ParseArangesIgnoreEmpty) {
   // and we ensure that these are ignored by our DWARFDebugArangeSet parser
   // and not included in the descriptors that are returned.
   unsigned char binary_data[] = {
-  0, 0, 0, 0, // unit_length that will be set correctly after this
-  0, 2,   // DWARF version number (uint16_t)
+  0, 0, 0, 0,   // unit_length that will be set correctly after this
+  0, 2, // DWARF version number (uint16_t)
   0, 0, 0, 255, // CU offset
-  4,  // address size
-  0,  // segment size
-  0, 0, 0, 0, // alignment for the first tuple
+  4,// address size
+  0,// segment size
+  0, 0, 0, 0,   // alignment for the first tuple
   // BEGIN TUPLES
   0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x01, 0x00, // [0x1000-0x1100)
   0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x00, 0x00, // [0x1100-0x1100)

``




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


[Lldb-commits] [lldb] [llvm] [lldb][DWARF] Replace lldb's DWARFDebugArangeSet with llvm's (PR #110058)

2024-09-25 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu created 
https://github.com/llvm/llvm-project/pull/110058

They are close enough to swap lldb's `DWARFDebugArangeSet` with the llvm one.

The difference is that llvm's `DWARFDebugArangeSet` add empty ranges when 
extracting. To accommodate this, `DWARFDebugAranges` in lldb filters out empty 
ranges when extracting.

>From 8e1c59729905fb89a8764ace3dfa0d04119d273e Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Wed, 25 Sep 2024 15:59:29 -0700
Subject: [PATCH] [lldb][DWARF] Replace lldb's DWARFDebugArangeSet with llvm's

---
 .../Plugins/SymbolFile/DWARF/CMakeLists.txt   |   1 -
 .../SymbolFile/DWARF/DWARFDebugArangeSet.cpp  | 176 --
 .../SymbolFile/DWARF/DWARFDebugArangeSet.h|  70 ---
 .../SymbolFile/DWARF/DWARFDebugAranges.cpp|  47 ++---
 .../SymbolFile/DWARF/SymbolFileDWARFTests.cpp |  56 +++---
 .../DebugInfo/DWARF/DWARFDebugArangeSet.h |   6 +
 .../DebugInfo/DWARF/DWARFDebugArangeSet.cpp   |  12 +-
 7 files changed, 55 insertions(+), 313 deletions(-)
 delete mode 100644 lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
 delete mode 100644 lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt 
b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
index 0e4fd5b995d1ba..83c4f1a4cf892c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
@@ -18,7 +18,6 @@ add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
   DWARFContext.cpp
   DWARFDataExtractor.cpp
   DWARFDebugAranges.cpp
-  DWARFDebugArangeSet.cpp
   DWARFDebugInfo.cpp
   DWARFDebugInfoEntry.cpp
   DWARFDebugMacro.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
deleted file mode 100644
index 8461b94abca630..00
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ /dev/null
@@ -1,176 +0,0 @@
-//===-- DWARFDebugArangeSet.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 "DWARFDebugArangeSet.h"
-#include "DWARFDataExtractor.h"
-#include "LogChannelDWARF.h"
-#include "llvm/Object/Error.h"
-#include 
-
-using namespace lldb_private;
-using namespace lldb_private::plugin::dwarf;
-
-DWARFDebugArangeSet::DWARFDebugArangeSet()
-: m_offset(DW_INVALID_OFFSET), m_next_offset(DW_INVALID_OFFSET) {}
-
-void DWARFDebugArangeSet::Clear() {
-  m_offset = DW_INVALID_OFFSET;
-  m_next_offset = DW_INVALID_OFFSET;
-  m_header.length = 0;
-  m_header.version = 0;
-  m_header.cu_offset = 0;
-  m_header.addr_size = 0;
-  m_header.seg_size = 0;
-  m_arange_descriptors.clear();
-}
-
-llvm::Error DWARFDebugArangeSet::extract(const DWARFDataExtractor &data,
- lldb::offset_t *offset_ptr) {
-  assert(data.ValidOffset(*offset_ptr));
-
-  m_arange_descriptors.clear();
-  m_offset = *offset_ptr;
-
-  // 7.20 Address Range Table
-  //
-  // Each set of entries in the table of address ranges contained in the
-  // .debug_aranges section begins with a header consisting of: a 4-byte
-  // length containing the length of the set of entries for this compilation
-  // unit, not including the length field itself; a 2-byte version identifier
-  // containing the value 2 for DWARF Version 2; a 4-byte offset into
-  // the.debug_infosection; a 1-byte unsigned integer containing the size in
-  // bytes of an address (or the offset portion of an address for segmented
-  // addressing) on the target system; and a 1-byte unsigned integer
-  // containing the size in bytes of a segment descriptor on the target
-  // system. This header is followed by a series of tuples. Each tuple
-  // consists of an address and a length, each in the size appropriate for an
-  // address on the target architecture.
-  m_header.length = data.GetDWARFInitialLength(offset_ptr);
-  // The length could be 4 bytes or 12 bytes, so use the current offset to
-  // determine the next offset correctly.
-  if (m_header.length > 0)
-m_next_offset = *offset_ptr + m_header.length;
-  else
-m_next_offset = DW_INVALID_OFFSET;
-  m_header.version = data.GetU16(offset_ptr);
-  m_header.cu_offset = data.GetDWARFOffset(offset_ptr);
-  m_header.addr_size = data.GetU8(offset_ptr);
-  m_header.seg_size = data.GetU8(offset_ptr);
-
-  // Try to avoid reading invalid arange sets by making sure:
-  // 1 - the version looks good
-  // 2 - the address byte size looks plausible
-  // 3 - the length seems to make sense
-  // 4 - size looks plausible
-  // 5 - the arange tuples do not contain a segment field
-  if (m_header.version < 2 || m_header.version > 5)
-r

[Lldb-commits] [lldb] [llvm] [lldb][DWARF] Replace lldb's DWARFDebugArangeSet with llvm's (PR #110058)

2024-09-25 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Zequan Wu (ZequanWu)


Changes

They are close enough to swap lldb's `DWARFDebugArangeSet` with the llvm one.

The difference is that llvm's `DWARFDebugArangeSet` add empty ranges when 
extracting. To accommodate this, `DWARFDebugAranges` in lldb filters out empty 
ranges when extracting.

---

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


7 Files Affected:

- (modified) lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt (-1) 
- (removed) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp (-176) 
- (removed) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h (-70) 
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp 
(+13-34) 
- (modified) lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp (+29-27) 
- (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFDebugArangeSet.h (+6) 
- (modified) llvm/lib/DebugInfo/DWARF/DWARFDebugArangeSet.cpp (+7-5) 


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt 
b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
index 0e4fd5b995d1ba..83c4f1a4cf892c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
@@ -18,7 +18,6 @@ add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
   DWARFContext.cpp
   DWARFDataExtractor.cpp
   DWARFDebugAranges.cpp
-  DWARFDebugArangeSet.cpp
   DWARFDebugInfo.cpp
   DWARFDebugInfoEntry.cpp
   DWARFDebugMacro.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
deleted file mode 100644
index 8461b94abca630..00
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ /dev/null
@@ -1,176 +0,0 @@
-//===-- DWARFDebugArangeSet.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 "DWARFDebugArangeSet.h"
-#include "DWARFDataExtractor.h"
-#include "LogChannelDWARF.h"
-#include "llvm/Object/Error.h"
-#include 
-
-using namespace lldb_private;
-using namespace lldb_private::plugin::dwarf;
-
-DWARFDebugArangeSet::DWARFDebugArangeSet()
-: m_offset(DW_INVALID_OFFSET), m_next_offset(DW_INVALID_OFFSET) {}
-
-void DWARFDebugArangeSet::Clear() {
-  m_offset = DW_INVALID_OFFSET;
-  m_next_offset = DW_INVALID_OFFSET;
-  m_header.length = 0;
-  m_header.version = 0;
-  m_header.cu_offset = 0;
-  m_header.addr_size = 0;
-  m_header.seg_size = 0;
-  m_arange_descriptors.clear();
-}
-
-llvm::Error DWARFDebugArangeSet::extract(const DWARFDataExtractor &data,
- lldb::offset_t *offset_ptr) {
-  assert(data.ValidOffset(*offset_ptr));
-
-  m_arange_descriptors.clear();
-  m_offset = *offset_ptr;
-
-  // 7.20 Address Range Table
-  //
-  // Each set of entries in the table of address ranges contained in the
-  // .debug_aranges section begins with a header consisting of: a 4-byte
-  // length containing the length of the set of entries for this compilation
-  // unit, not including the length field itself; a 2-byte version identifier
-  // containing the value 2 for DWARF Version 2; a 4-byte offset into
-  // the.debug_infosection; a 1-byte unsigned integer containing the size in
-  // bytes of an address (or the offset portion of an address for segmented
-  // addressing) on the target system; and a 1-byte unsigned integer
-  // containing the size in bytes of a segment descriptor on the target
-  // system. This header is followed by a series of tuples. Each tuple
-  // consists of an address and a length, each in the size appropriate for an
-  // address on the target architecture.
-  m_header.length = data.GetDWARFInitialLength(offset_ptr);
-  // The length could be 4 bytes or 12 bytes, so use the current offset to
-  // determine the next offset correctly.
-  if (m_header.length > 0)
-m_next_offset = *offset_ptr + m_header.length;
-  else
-m_next_offset = DW_INVALID_OFFSET;
-  m_header.version = data.GetU16(offset_ptr);
-  m_header.cu_offset = data.GetDWARFOffset(offset_ptr);
-  m_header.addr_size = data.GetU8(offset_ptr);
-  m_header.seg_size = data.GetU8(offset_ptr);
-
-  // Try to avoid reading invalid arange sets by making sure:
-  // 1 - the version looks good
-  // 2 - the address byte size looks plausible
-  // 3 - the length seems to make sense
-  // 4 - size looks plausible
-  // 5 - the arange tuples do not contain a segment field
-  if (m_header.version < 2 || m_header.version > 5)
-return llvm::make_error(
-"Invalid arange header version");
-
-  if (m_header.addr_size != 4 && m_header.addr_size != 8)
-return llv

[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

2024-09-25 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/106442

>From d6eec42d53f17bfa34fec7623d1616fbca0a0140 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Wed, 28 Aug 2024 10:04:33 -0700
Subject: [PATCH] [lldb] Store expression evaluator diagnostics in an
 llvm::Error (NFC)

This patch is a reworking of Pete Lawrence's (@PortalPete) proposal
for better expression evaluator error messages:
https://github.com/llvm/llvm-project/pull/80938

This patch is preparatory patch for improving the rendering of
expression evaluator diagnostics. Currently diagnostics are rendered
into a string and the command interpreter layer then textually parses
words like "error:" to (sometimes) color the output accordingly. In
order to enable user interfaces to do better with diagnostics, we need
to store them in a machine-readable fromat. This patch does this by
adding a new llvm::Error kind wrapping a DiagnosticDetail struct that
is used when the error type is eErrorTypeExpression. Multiple
diagnostics are modeled using llvm::ErrorList.

Right now the extra information is not used by the CommandInterpreter,
this will be added in a follow-up patch!
---
 .../lldb/Expression/DiagnosticManager.h   |  89 +++
 lldb/include/lldb/Utility/Status.h|  23 ++--
 lldb/source/Breakpoint/BreakpointLocation.cpp |  11 +-
 lldb/source/Expression/DiagnosticManager.cpp  | 103 +++---
 lldb/source/Expression/ExpressionParser.cpp   |   5 +-
 lldb/source/Expression/UserExpression.cpp |  49 +
 lldb/source/Expression/UtilityFunction.cpp|  18 +--
 .../ExpressionParser/Clang/ClangDiagnostic.h  |   5 +-
 .../Clang/ClangExpressionParser.cpp   |  69 
 .../Clang/ClangUserExpression.h   |   2 +
 .../Plugins/Platform/POSIX/PlatformPOSIX.cpp  |  12 +-
 .../Platform/Windows/PlatformWindows.cpp  |  12 +-
 lldb/source/Target/Target.cpp |   3 +-
 lldb/source/Utility/Status.cpp|  29 +
 .../TestModulesCompileError.py|   2 +-
 .../Expression/DiagnosticManagerTest.cpp  |  22 +++-
 lldb/unittests/Utility/StatusTest.cpp |   2 +-
 17 files changed, 293 insertions(+), 163 deletions(-)

diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h 
b/lldb/include/lldb/Expression/DiagnosticManager.h
index d49b7c99b114fb..3e363ee5c0139c 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -12,6 +12,9 @@
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Status.h"
+
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -20,6 +23,53 @@
 
 namespace lldb_private {
 
+/// A compiler-independent representation of a Diagnostic. Expression
+/// evaluation failures often have more than one diagnostic that a UI
+/// layer might want to render differently, for example to colorize
+/// it.
+///
+/// Running example:
+///   (lldb) expr 1+foo
+///   error: :1:3: use of undeclared identifier 'foo'
+///   1+foo
+/// ^
+struct DiagnosticDetail {
+  struct SourceLocation {
+FileSpec file;
+unsigned line = 0;
+uint16_t column = 0;
+uint16_t length = 0;
+bool in_user_input = false;
+  };
+  /// Contains {{}, 1, 3, 3, true} in the example above.
+  std::optional source_location;
+  /// Contains eSeverityError in the example above.
+  lldb::Severity severity = lldb::eSeverityInfo;
+  /// Contains "use of undeclared identifier 'x'" in the example above.
+  std::string message;
+  /// Contains the fully rendered error message.
+  std::string rendered;
+};
+
+/// An llvm::Error used to communicate diagnostics in Status. Multiple
+/// diagnostics may be chained in an llvm::ErrorList.
+class ExpressionError
+: public llvm::ErrorInfo {
+  std::string m_message;
+  std::vector m_details;
+
+public:
+  static char ID;
+  using llvm::ErrorInfo::ErrorInfo;
+  ExpressionError(lldb::ExpressionResults result, std::string msg,
+  std::vector details = {});
+  std::string message() const override;
+  llvm::ArrayRef GetDetail() const { return m_details; }
+  std::error_code convertToErrorCode() const override;
+  void log(llvm::raw_ostream &OS) const override;
+  std::unique_ptr Clone() const override;
+};
+
 enum DiagnosticOrigin {
   eDiagnosticOriginUnknown = 0,
   eDiagnosticOriginLLDB,
@@ -49,37 +99,28 @@ class Diagnostic {
 }
   }
 
-  Diagnostic(llvm::StringRef message, lldb::Severity severity,
- DiagnosticOrigin origin, uint32_t compiler_id)
-  : m_message(message), m_severity(severity), m_origin(origin),
-m_compiler_id(compiler_id) {}
-
-  Diagnostic(const Diagnostic &rhs)
-  : m_message(rhs.m_message), m_severity(rhs.m_severity),
-m_origin(rhs.m_origin), m_compiler_id(rhs.m_compiler_id) {}
+  Diagnostic(DiagnosticOrigin origin, uint32_t compiler_id,
+ DiagnosticDetail 

[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

2024-09-25 Thread Adrian Prantl via lldb-commits


@@ -61,8 +65,50 @@ std::string DiagnosticManager::GetString(char separator) {
   stream << message.drop_front(severity_pos + severity.size());
 stream << separator;
   }
+  return str;
+}
 
-  return ret;
+void DiagnosticManager::Dump(Log *log) {
+  if (!log)
+return;
+
+  std::string str = GetString();
+
+  // We want to remove the last '\n' because log->PutCString will add
+  // one for us.
+
+  if (str.size() && str.back() == '\n')
+str.pop_back();
+
+  log->PutString(str);
+}
+
+llvm::Error Diagnostic::GetAsError() const {
+  return llvm::make_error(m_detail);
+}
+
+llvm::Error
+DiagnosticManager::GetAsError(lldb::ExpressionResults result) const {
+  llvm::Error diags = Status::FromExpressionError(result, "").takeError();
+  for (const auto &diagnostic : m_diagnostics)
+diags = llvm::joinErrors(std::move(diags), diagnostic->GetAsError());
+  return diags;
+}
+
+llvm::Error DiagnosticManager::GetAsError(llvm::Twine msg) const {
+  llvm::Error diags = llvm::createStringError(msg);
+  for (const auto &diagnostic : m_diagnostics)
+diags = llvm::joinErrors(std::move(diags), diagnostic->GetAsError());
+  return diags;
+}
+
+void DiagnosticManager::AddDiagnostic(llvm::StringRef message,
+  lldb::Severity severity,
+  DiagnosticOrigin origin,
+  uint32_t compiler_id) {
+  m_diagnostics.emplace_back(std::make_unique(
+  origin, compiler_id,
+  DiagnosticDetail{{}, severity, message.str(), message.str()}));

adrian-prantl wrote:

This API is only used to add higher-level LLDB-specific "diagnostics". They 
don't have a source location.

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


[Lldb-commits] [lldb] [llvm] [lldb][DWARF] Replace lldb's DWARFDebugArangeSet with llvm's (PR #110058)

2024-09-25 Thread Zequan Wu via lldb-commits

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/110058

>From 8e1c59729905fb89a8764ace3dfa0d04119d273e Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Wed, 25 Sep 2024 15:59:29 -0700
Subject: [PATCH 1/2] [lldb][DWARF] Replace lldb's DWARFDebugArangeSet with
 llvm's

---
 .../Plugins/SymbolFile/DWARF/CMakeLists.txt   |   1 -
 .../SymbolFile/DWARF/DWARFDebugArangeSet.cpp  | 176 --
 .../SymbolFile/DWARF/DWARFDebugArangeSet.h|  70 ---
 .../SymbolFile/DWARF/DWARFDebugAranges.cpp|  47 ++---
 .../SymbolFile/DWARF/SymbolFileDWARFTests.cpp |  56 +++---
 .../DebugInfo/DWARF/DWARFDebugArangeSet.h |   6 +
 .../DebugInfo/DWARF/DWARFDebugArangeSet.cpp   |  12 +-
 7 files changed, 55 insertions(+), 313 deletions(-)
 delete mode 100644 lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
 delete mode 100644 lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt 
b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
index 0e4fd5b995d1ba..83c4f1a4cf892c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
@@ -18,7 +18,6 @@ add_lldb_library(lldbPluginSymbolFileDWARF PLUGIN
   DWARFContext.cpp
   DWARFDataExtractor.cpp
   DWARFDebugAranges.cpp
-  DWARFDebugArangeSet.cpp
   DWARFDebugInfo.cpp
   DWARFDebugInfoEntry.cpp
   DWARFDebugMacro.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
deleted file mode 100644
index 8461b94abca630..00
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.cpp
+++ /dev/null
@@ -1,176 +0,0 @@
-//===-- DWARFDebugArangeSet.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 "DWARFDebugArangeSet.h"
-#include "DWARFDataExtractor.h"
-#include "LogChannelDWARF.h"
-#include "llvm/Object/Error.h"
-#include 
-
-using namespace lldb_private;
-using namespace lldb_private::plugin::dwarf;
-
-DWARFDebugArangeSet::DWARFDebugArangeSet()
-: m_offset(DW_INVALID_OFFSET), m_next_offset(DW_INVALID_OFFSET) {}
-
-void DWARFDebugArangeSet::Clear() {
-  m_offset = DW_INVALID_OFFSET;
-  m_next_offset = DW_INVALID_OFFSET;
-  m_header.length = 0;
-  m_header.version = 0;
-  m_header.cu_offset = 0;
-  m_header.addr_size = 0;
-  m_header.seg_size = 0;
-  m_arange_descriptors.clear();
-}
-
-llvm::Error DWARFDebugArangeSet::extract(const DWARFDataExtractor &data,
- lldb::offset_t *offset_ptr) {
-  assert(data.ValidOffset(*offset_ptr));
-
-  m_arange_descriptors.clear();
-  m_offset = *offset_ptr;
-
-  // 7.20 Address Range Table
-  //
-  // Each set of entries in the table of address ranges contained in the
-  // .debug_aranges section begins with a header consisting of: a 4-byte
-  // length containing the length of the set of entries for this compilation
-  // unit, not including the length field itself; a 2-byte version identifier
-  // containing the value 2 for DWARF Version 2; a 4-byte offset into
-  // the.debug_infosection; a 1-byte unsigned integer containing the size in
-  // bytes of an address (or the offset portion of an address for segmented
-  // addressing) on the target system; and a 1-byte unsigned integer
-  // containing the size in bytes of a segment descriptor on the target
-  // system. This header is followed by a series of tuples. Each tuple
-  // consists of an address and a length, each in the size appropriate for an
-  // address on the target architecture.
-  m_header.length = data.GetDWARFInitialLength(offset_ptr);
-  // The length could be 4 bytes or 12 bytes, so use the current offset to
-  // determine the next offset correctly.
-  if (m_header.length > 0)
-m_next_offset = *offset_ptr + m_header.length;
-  else
-m_next_offset = DW_INVALID_OFFSET;
-  m_header.version = data.GetU16(offset_ptr);
-  m_header.cu_offset = data.GetDWARFOffset(offset_ptr);
-  m_header.addr_size = data.GetU8(offset_ptr);
-  m_header.seg_size = data.GetU8(offset_ptr);
-
-  // Try to avoid reading invalid arange sets by making sure:
-  // 1 - the version looks good
-  // 2 - the address byte size looks plausible
-  // 3 - the length seems to make sense
-  // 4 - size looks plausible
-  // 5 - the arange tuples do not contain a segment field
-  if (m_header.version < 2 || m_header.version > 5)
-return llvm::make_error(
-"Invalid arange header version");
-
-  if (m_header.addr_size != 4 && m_header.addr_size != 8)
-return llvm::make_error(
-"Invalid arange header address size");
-
-  if (m_header.length == 0)
-return llvm:

[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

2024-09-25 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/106442

>From 9f4ba3fdb8b144736e51134ced3019a2c57ca861 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Wed, 28 Aug 2024 10:04:33 -0700
Subject: [PATCH] [lldb] Store expression evaluator diagnostics in an
 llvm::Error (NFC)

This patch is a reworking of Pete Lawrence's (@PortalPete) proposal
for better expression evaluator error messages:
https://github.com/llvm/llvm-project/pull/80938

This patch is preparatory patch for improving the rendering of
expression evaluator diagnostics. Currently diagnostics are rendered
into a string and the command interpreter layer then textually parses
words like "error:" to (sometimes) color the output accordingly. In
order to enable user interfaces to do better with diagnostics, we need
to store them in a machine-readable fromat. This patch does this by
adding a new llvm::Error kind wrapping a DiagnosticDetail struct that
is used when the error type is eErrorTypeExpression. Multiple
diagnostics are modeled using llvm::ErrorList.

Right now the extra information is not used by the CommandInterpreter,
this will be added in a follow-up patch!
---
 .../lldb/Expression/DiagnosticManager.h   |  89 +++
 lldb/include/lldb/Utility/Status.h|  23 ++--
 lldb/source/Breakpoint/BreakpointLocation.cpp |  11 +-
 lldb/source/Expression/DiagnosticManager.cpp  | 103 +++---
 lldb/source/Expression/ExpressionParser.cpp   |   5 +-
 lldb/source/Expression/UserExpression.cpp |  49 +
 lldb/source/Expression/UtilityFunction.cpp|  18 +--
 .../ExpressionParser/Clang/ClangDiagnostic.h  |   5 +-
 .../Clang/ClangExpressionParser.cpp   |  69 
 .../Clang/ClangUserExpression.h   |   2 +
 .../Plugins/Platform/POSIX/PlatformPOSIX.cpp  |  12 +-
 .../Platform/Windows/PlatformWindows.cpp  |  12 +-
 lldb/source/Target/Target.cpp |   3 +-
 lldb/source/Utility/Status.cpp|  29 +
 .../TestModulesCompileError.py|   2 +-
 .../Expression/DiagnosticManagerTest.cpp  |  22 +++-
 16 files changed, 292 insertions(+), 162 deletions(-)

diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h 
b/lldb/include/lldb/Expression/DiagnosticManager.h
index d49b7c99b114fb..3e363ee5c0139c 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -12,6 +12,9 @@
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
+#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/Status.h"
+
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -20,6 +23,53 @@
 
 namespace lldb_private {
 
+/// A compiler-independent representation of a Diagnostic. Expression
+/// evaluation failures often have more than one diagnostic that a UI
+/// layer might want to render differently, for example to colorize
+/// it.
+///
+/// Running example:
+///   (lldb) expr 1+foo
+///   error: :1:3: use of undeclared identifier 'foo'
+///   1+foo
+/// ^
+struct DiagnosticDetail {
+  struct SourceLocation {
+FileSpec file;
+unsigned line = 0;
+uint16_t column = 0;
+uint16_t length = 0;
+bool in_user_input = false;
+  };
+  /// Contains {{}, 1, 3, 3, true} in the example above.
+  std::optional source_location;
+  /// Contains eSeverityError in the example above.
+  lldb::Severity severity = lldb::eSeverityInfo;
+  /// Contains "use of undeclared identifier 'x'" in the example above.
+  std::string message;
+  /// Contains the fully rendered error message.
+  std::string rendered;
+};
+
+/// An llvm::Error used to communicate diagnostics in Status. Multiple
+/// diagnostics may be chained in an llvm::ErrorList.
+class ExpressionError
+: public llvm::ErrorInfo {
+  std::string m_message;
+  std::vector m_details;
+
+public:
+  static char ID;
+  using llvm::ErrorInfo::ErrorInfo;
+  ExpressionError(lldb::ExpressionResults result, std::string msg,
+  std::vector details = {});
+  std::string message() const override;
+  llvm::ArrayRef GetDetail() const { return m_details; }
+  std::error_code convertToErrorCode() const override;
+  void log(llvm::raw_ostream &OS) const override;
+  std::unique_ptr Clone() const override;
+};
+
 enum DiagnosticOrigin {
   eDiagnosticOriginUnknown = 0,
   eDiagnosticOriginLLDB,
@@ -49,37 +99,28 @@ class Diagnostic {
 }
   }
 
-  Diagnostic(llvm::StringRef message, lldb::Severity severity,
- DiagnosticOrigin origin, uint32_t compiler_id)
-  : m_message(message), m_severity(severity), m_origin(origin),
-m_compiler_id(compiler_id) {}
-
-  Diagnostic(const Diagnostic &rhs)
-  : m_message(rhs.m_message), m_severity(rhs.m_severity),
-m_origin(rhs.m_origin), m_compiler_id(rhs.m_compiler_id) {}
+  Diagnostic(DiagnosticOrigin origin, uint32_t compiler_id,
+ DiagnosticDetail detail)
+  : m_origin(origin), m_compiler_id(compile

[Lldb-commits] [lldb] [lldb] fix vFile:open, vFile:unlink error codes (PR #106950)

2024-09-25 Thread David Spickett via lldb-commits

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

LGTM

In theory we could test this but it'd be platform specific and error injection 
is always tricky, the change is straightforward, so let's not worry about it.

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


[Lldb-commits] [lldb] c93e294 - [lldb] fix vFile:open, vFile:unlink error codes (#106950)

2024-09-25 Thread via lldb-commits

Author: dlav-sc
Date: 2024-09-25T10:13:40+01:00
New Revision: c93e29439b1ab8ef6873c385f152a06e3395cb59

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

LOG: [lldb] fix vFile:open, vFile:unlink error codes (#106950)

This patch makes gdb-server sends only GDB RSP supported error codes
during vFile:open and vFile:unlink handling.

Added: 


Modified: 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index f9d37490e16aec..324db3db7eb4c7 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -496,6 +496,17 @@ GDBRemoteCommunicationServerCommon::Handle_qSpeedTest(
   return SendErrorResponse(7);
 }
 
+static GDBErrno system_errno_to_gdb(int err) {
+  switch (err) {
+#define HANDLE_ERRNO(name, value)  
\
+  case name:   
\
+return GDB_##name;
+#include "Plugins/Process/gdb-remote/GDBRemoteErrno.def"
+  default:
+return GDB_EUNKNOWN;
+  }
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Open(
 StringExtractorGDBRemote &packet) {
@@ -522,9 +533,7 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Open(
 } else {
   response.PutCString("-1");
   std::error_code code = errorToErrorCode(file.takeError());
-  if (code.category() == std::system_category()) {
-response.Printf(",%x", code.value());
-  }
+  response.Printf(",%x", system_errno_to_gdb(code.value()));
 }
 
 return SendPacketNoLock(response.GetString());
@@ -534,17 +543,6 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Open(
   return SendErrorResponse(18);
 }
 
-static GDBErrno system_errno_to_gdb(int err) {
-  switch (err) {
-#define HANDLE_ERRNO(name, value)  
\
-  case name:   
\
-return GDB_##name;
-#include "Plugins/Process/gdb-remote/GDBRemoteErrno.def"
-  default:
-return GDB_EUNKNOWN;
-  }
-}
-
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Close(
 StringExtractorGDBRemote &packet) {
@@ -727,7 +725,8 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_unlink(
   packet.GetHexByteString(path);
   Status error(llvm::sys::fs::remove(path));
   StreamString response;
-  response.Printf("F%x,%x", error.GetError(), error.GetError());
+  response.Printf("F%x,%x", error.GetError(),
+  system_errno_to_gdb(error.GetError()));
   return SendPacketNoLock(response.GetString());
 }
 



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


[Lldb-commits] [lldb] [lldb] fix vFile:open, vFile:unlink error codes (PR #106950)

2024-09-25 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [lldb] fix vFile:open, vFile:unlink error codes (PR #106950)

2024-09-25 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Feels like we diverted you from your original problem though, which was the 
remote taking the informative errno and changing it to some meaningless value.

If you've solved your problem then no obligation to do any more but if you do 
want to look into the other reporting methods that would be welcome.

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


[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add Floating Point Mode Register (PR #106695)

2024-09-25 Thread David Spickett via lldb-commits

DavidSpickett wrote:

We're almost there for the save all / restore all anyway. We'll need special 
cases for SME of course but SME is weird enough to justify that.

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


[Lldb-commits] [lldb] 5ee2dea - [lldb][AArch64][Linux] Add Floating Point Mode Register (#106695)

2024-09-25 Thread via lldb-commits

Author: David Spickett
Date: 2024-09-25T10:27:57+01:00
New Revision: 5ee2deac0c3b85deaeb0031b4030db99d266abdc

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

LOG: [lldb][AArch64][Linux] Add Floating Point Mode Register (#106695)

Otherwise known as FEAT_FPMR. This register controls the behaviour of
floating point operations.

https://developer.arm.com/documentation/ddi0601/2024-06/AArch64-Registers/FPMR--Floating-point-Mode-Register

As the current floating point register contexts are fixed size, this has
been placed in a new set. Linux kernel patches have landed already, so
you can cross check with those.

To simplify testing we're not going to do any floating point operations,
just read and write from the program and debugger to make sure each sees
the other's values correctly.

Added: 
lldb/test/API/linux/aarch64/fpmr/Makefile
lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py
lldb/test/API/linux/aarch64/fpmr/main.c

Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index df5a110cb5b309..c6b7ce84109c09 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1370,6 +1370,9 @@ def isAArch64PAuth(self):
 return True
 return self.isAArch64() and "paca" in self.getCPUInfo()
 
+def isAArch64FPMR(self):
+return self.isAArch64() and "fpmr" in self.getCPUInfo()
+
 def isAArch64Windows(self):
 """Returns true if the architecture is AArch64 and platform windows."""
 if self.getPlatform() == "windows":

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 1dd4fd41351333..6056f3001fed6e 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -60,10 +60,16 @@
 #define NT_ARM_TAGGED_ADDR_CTRL 0x409 /* Tagged address control register */
 #endif
 
+#ifndef NT_ARM_FPMR
+#define NT_ARM_FPMR 0x40e /* Floating point mode register */
+#endif
+
 #define HWCAP_PACA (1 << 30)
 
 #define HWCAP2_MTE (1 << 18)
 
+#define HWCAP2_FPMR (1UL << 48)
+
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
@@ -139,8 +145,12 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 
 std::optional auxv_at_hwcap2 =
 process.GetAuxValue(AuxVector::AUXV_AT_HWCAP2);
-if (auxv_at_hwcap2 && (*auxv_at_hwcap2 & HWCAP2_MTE))
-  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
+if (auxv_at_hwcap2) {
+  if (*auxv_at_hwcap2 & HWCAP2_MTE)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
+  if (*auxv_at_hwcap2 & HWCAP2_FPMR)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+}
 
 opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
 
@@ -186,6 +196,7 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
 
   m_mte_ctrl_reg = 0;
+  m_fpmr_reg = 0;
 
   // 16 is just a maximum value, query hardware for actual watchpoint count
   m_max_hwp_supported = 16;
@@ -201,6 +212,7 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_mte_ctrl_is_valid = false;
   m_tls_is_valid = false;
   m_zt_buffer_is_valid = false;
+  m_fpmr_is_valid = false;
 
   // SME adds the tpidr2 register
   m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -413,6 +425,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
   assert(offset < GetSMEPseudoBufferSize());
   src = (uint8_t *)GetSMEPseudoBuffer() + offset;
 }
+  } else if (IsFPMR(reg)) {
+error = ReadFPMR();
+if (error.Fail())
+  return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
+assert(offset < GetFPMRBufferSize());
+src = (uint8_t *)GetFPMRBuffer() + offset;
   } else
 return Status::FromErrorString(
 "failed - register wasn't recognized to be a GPR or an FPR, "
@@ -626,6 +646,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegist

[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add Floating Point Mode Register (PR #106695)

2024-09-25 Thread David Spickett via lldb-commits

DavidSpickett wrote:

> There will be more features in future meaning more read/ write routines doing 
> almost similar stuff.
IMO We may later consider refactoring the code to provide a top level interface 
for feature registers read and write.

Yes I would like a "data driven" model for it. I got caught out because I 
forgot to clear one of the "is valid" bools, which is time not well spent.

I'll certainly look into that. Depending on how long Guarded Control Stack 
takes to land in the kernel, it may be before or after that work.

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


[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add Floating Point Mode Register (PR #106695)

2024-09-25 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] 706821b - [lldb][test] Skip TestConcurrentVFork on all Linux

2024-09-25 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-09-25T10:08:10+01:00
New Revision: 706821ba8ff9db829252581dd12d8c5ee2e7b3f0

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

LOG: [lldb][test] Skip TestConcurrentVFork on all Linux

And given that it is only for Linux - effectively skip it,
but in a way where we don't forget that it's Linux only.

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

This test times out on occasion on Arm, AArch64 and X86 Linux,
which I saw just today in a buildkite build. Causing a failure that
is 1. confusing because the PR wasn't for LLDB and 2. annoying
to find in the giant log file (which isn't the test's fault,
but it adds to the overhead).

It's probably important to have this test running somewhere but
right now it's causing too much noise to do so.

Added: 


Modified: 
lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py 
b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
index dd9500c186b2c8..3b5efb834b1626 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
@@ -49,7 +49,7 @@ def follow_child_helper(self, use_fork, call_exec):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_parent_vfork_no_exec(self):
 """
 Make sure that debugging concurrent vfork() from multiple threads 
won't crash lldb during follow-parent.
@@ -59,7 +59,7 @@ def test_follow_parent_vfork_no_exec(self):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_parent_fork_no_exec(self):
 """
 Make sure that debugging concurrent fork() from multiple threads won't 
crash lldb during follow-parent.
@@ -69,7 +69,7 @@ def test_follow_parent_fork_no_exec(self):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_parent_vfork_call_exec(self):
 """
 Make sure that debugging concurrent vfork() from multiple threads 
won't crash lldb during follow-parent.
@@ -79,7 +79,7 @@ def test_follow_parent_vfork_call_exec(self):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_parent_fork_call_exec(self):
 """
 Make sure that debugging concurrent vfork() from multiple threads 
won't crash lldb during follow-parent.
@@ -89,7 +89,7 @@ def test_follow_parent_fork_call_exec(self):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_child_vfork_no_exec(self):
 """
 Make sure that debugging concurrent vfork() from multiple threads 
won't crash lldb during follow-child.
@@ -99,7 +99,7 @@ def test_follow_child_vfork_no_exec(self):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_child_fork_no_exec(self):
 """
 Make sure that debugging concurrent fork() from multiple threads won't 
crash lldb during follow-child.
@@ -109,7 +109,7 @@ def test_follow_child_fork_no_exec(self):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_child_vfork_call_exec(self):
 """
 Make sure that debugging concurrent vfork() from multiple threads 
won't crash lldb during follow-child.
@@ -119,7 +119,7 @@ def test_follow_child_vfork_call_exec(self):
 
 @skipUnlessPlatform(["linux"])
 # https://github.com/llvm/llvm-project/issues/85084.
-@skipIf(oslist=["linux"], archs=["aarch64", "arm"])
+@skipIf(oslist=["linux"])
 def test_follow_child_fork_call_exec(self):
 """
 Make sure that debugging concurrent fork() from multiple threads won't 
crash lldb during follow-child.



___
lldb-commits mailing list
lldb-commits@lists

[Lldb-commits] [lldb] [lldb] Fix assert frame recognizer for Ubuntu 22.04 (PR #109594)

2024-09-25 Thread Pavel Labath via lldb-commits

labath wrote:

> Is the following correct: Even if the user does not have debug symbols 
> installed, it would always be sufficient to match the **first** named frame.

That *might* work. The complication is that the name of the "first named frame" 
will different depending on whether you have debug symbols, how your libc was 
compiled, and its version, which sounds a bit whack-a-mole-ish, but hopefully 
the list does not end up too long.

> 
> If so, we could change `StackFrameList::SelectMostRelevantFrame` to match 
> against the first frame which has a name available. Although, that might 
> already cause a lot of unwinding, in case no debug info is available at all...

Yes, that sounds like it could be a problem (maybe, I really don't know). I 
think it might be nicer if we were able to put this unwinding into the hands of 
the individual recognizer -- so that it could do some checks (and bail out 
early) on the intermediate frames as well. For example, even though we don't 
know the names of these "unnamed" frames, we can be sure that all of them are 
coming from the c library, so we can bail out as soon as we encounter a 
non-libc frame (*)

(*) I'm ignoring the fact that on linux, none of the libc frames actually have 
to be in a library called `libc.so.6`. If you're linking your binary 
statically, the name of the module can be completely arbitrary.

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


[Lldb-commits] [lldb] [lldb][AArch64][Linux] Add Floating Point Mode Register (PR #106695)

2024-09-25 Thread Omair Javaid via lldb-commits

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

This looks good to me. I haven't tested it myself though.

Just one thing that's bothering me is that we are now gradually implementing a 
number of similar functions to read/write feature registers like ZT ZA or FPMR 
etc. 

There will be more features in future meaning more read/ write routines doing 
almost similar stuff. 
IMO We may later consider refactoring the code to provide a top level interface 
for feature registers read and write.

Also we can avoid caching these single registers at all. We don't have much of 
performance benefit in this day n age to store them in temporary 
NativeRegisterContext variables. Instead read from ptrace every time we need 
them.

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


[Lldb-commits] [lldb] [LLDB][Minidump] Have Minidumps save off and properly read TLS data (PR #109477)

2024-09-25 Thread Pavel Labath via lldb-commits

labath wrote:

Yeah, the thing which makes the core files different from live processes is 
that core files (at least minidumps, the situation is a bit fuzzier for elf 
cores) often have a better idea of where the files are located. OTOH, the 
dynamic loader plugin is a (I'm deliberately not using "the" because I could 
think of other places as well) for the thread-local storage lookup code, so we 
do the two to work together.

Regarding the "allow the DYLD to do it's own book keeping but just not handle 
loading/unloading modules owned by the CoreProcess" idea, how exactly would 
that differ from what your change in `DynamicLoaderPOSIXDYLD::DidAttach` does? 
In that the dynamic loader would not attempt to relocate the module if it has 
already been loaded by the process plugin, where as right now it does? I think 
that would make sense, though I also think that the current behavior 
(attempting to relocate it also does) -- my core question is: how likely is it 
that the dynamic loader will find the right thread-local for a module which was 
loaded by the process class, if the two classes can't even agree on its load 
address?

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


[Lldb-commits] [lldb] [lldb] Fix assert frame recognizer for Ubuntu 22.04 (PR #109594)

2024-09-25 Thread via lldb-commits

jimingham wrote:



> On Sep 24, 2024, at 10:25 AM, Adrian Vogelsgesang ***@***.***> wrote:
> 
> 
> However, with these new "hiding frame" recognizers, we are using the frame 
> hiding recognizers to handle "step out past hidden frames". So if - as was 
> done in the original implementation - we're determining this during execution 
> control, its hard to see how we can avoid running these recognizers all the 
> time. That argues for not allowing unbounded backtracing in recognizers.
> 
> Good point, that's something which probably should be changed... CC 
> @adrian-prantl 
> [...] So it would be better to make this an explicit gesture somehow.
> 
> StackFrameList::GetSelectedFrameIndex already takes a parameter 
> select_most_relevant which enables the frame recognizers only for 
> user-visible stops. At least from a cursory look over all its callers, it 
> looks like what you are proposing?
> 
The execution control part of lldb doesn't care about selected frames, and 
doesn't ever call that API.  We're always just doing "GetStackFrameAtIndex".  I 
doubt it's going to be hard to make sure that never triggers running the 
recognizers, it may even do that already, I'd have to go back and check.

But if we're going to allow unbounded search recognizers, I think we do have to 
insist that they can't take part in execution control once it's running, though 
they certainly could augment a user level command like StepOut with further 
instructions (like step out to frame 7) before setting the ThreadPlan going.

Jim
> (Not sure if I missed anything, though. Still new to the lldb code base...)
> 
> —
> Reply to this email directly, view it on GitHub 
> , 
> or unsubscribe 
> .
> You are receiving this because you were mentioned.
> 



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


[Lldb-commits] [lldb] [lldb] Fix assert frame recognizer for Ubuntu 22.04 (PR #109594)

2024-09-25 Thread via lldb-commits

jimingham wrote:



> On Sep 25, 2024, at 10:33 AM, Jim Ingham ***@***.***> wrote:
> 
>> 
>> 
>> 
>>> On Sep 24, 2024, at 10:25 AM, Adrian Vogelsgesang ***@***.***> wrote:
>>> 
>>> 
>>> However, with these new "hiding frame" recognizers, we are using the frame 
>>> hiding recognizers to handle "step out past hidden frames". So if - as was 
>>> done in the original implementation - we're determining this during 
>>> execution control, its hard to see how we can avoid running these 
>>> recognizers all the time. That argues for not allowing unbounded 
>>> backtracing in recognizers.
>>> 
>>> Good point, that's something which probably should be changed... CC 
>>> @adrian-prantl 
>>> [...] So it would be better to make this an explicit gesture somehow.
>>> 
>>> StackFrameList::GetSelectedFrameIndex already takes a parameter 
>>> select_most_relevant which enables the frame recognizers only for 
>>> user-visible stops. At least from a cursory look over all its callers, it 
>>> looks like what you are proposing?
>>> 
>> The execution control part of lldb doesn't care about selected frames, and 
>> doesn't ever call that API.  We're always just doing "GetStackFrameAtIndex". 
>>  I doubt it's going to be hard to make sure that never triggers running the 
>> recognizers, it may even do that already, I'd have to go back and check.
>> 
>> But if we're going to allow unbounded search recognizers, I think we do have 
>> to insist that they can't take part in execution control once it's running, 
>> though they certainly could augment a user level command like StepOut with 
>> further instructions (like step out to frame 7) before setting the 
>> ThreadPlan going.

Actually, this may not be as bad as that.  If the way to ask "make a 
RecognizedStackFrame for THIS FRAME and ask is the frame hidden" that is 
separate from "run the recognizer on all frames", then provided recognizing 
this frame is not too slow, the execution control could ask that safely when it 
stops.  We really only need to trigger the full stack scan when we calculate 
the Selected frame.  So these two parts of the functionality should be easy to 
cordon off from one another.

Jim

>> 
>> Jim
>>> (Not sure if I missed anything, though. Still new to the lldb code base...)
>>> 
>>> —
>>> Reply to this email directly, view it on GitHub 
>>> , 
>>> or unsubscribe 
>>> .
>>> You are receiving this because you were mentioned.
>>> 
>> 
>> 



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


[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

2024-09-25 Thread Adrian Prantl via lldb-commits


@@ -224,6 +224,14 @@ const char *Status::AsCString(const char 
*default_error_str) const {
 if (!m_string.empty() && m_string[m_string.size() - 1] == '\n')
   m_string.pop_back();
 
+  // FIXME: Workaround for ErrorList[ExpressionError, ...].
+  if (m_error.isA()) {
+while (!m_string.empty() && m_string[0] == '\n')
+  m_string = std::string(m_string.data() + 1, m_string.size() - 1);
+if (!m_string.empty() && m_string[m_string.size() - 1] != '\n')
+  m_string += '\n';
+  }

adrian-prantl wrote:

That's what I had originally, I think I might have misunderstood an earlier 
comment from you suggesting we represent the expression failure as ErrorInfo to 
mean that you preferred having them be an ErrorList. I can definitely change 
that back. It would make iterating over the details a tiny bit less cumbersome.

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


[Lldb-commits] [lldb] b3b6141 - [lldb] Fix two formatv issues in LDB_LOG (NFC)

2024-09-25 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2024-09-25T10:17:09-07:00
New Revision: b3b6141ba1105ad5b9712a9c93891003170c32ac

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

LOG: [lldb] Fix two formatv issues in LDB_LOG (NFC)

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index b0f49ebf2d2cbb..264b2e84114077 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -9702,7 +9702,7 @@ ScratchTypeSystemClang::GetForTarget(Target &target,
   lldb::eLanguageTypeC, create_on_demand);
   if (auto err = type_system_or_err.takeError()) {
 LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
-   "Couldn't get scratch TypeSystemClang");
+   "Couldn't get scratch TypeSystemClang: {0}");
 return nullptr;
   }
   auto ts_sp = *type_system_or_err;

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 29e9efb83efeb6..6123e5b9c20902 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3617,7 +3617,7 @@ void Target::FinalizeFileActions(ProcessLaunchInfo &info) 
{
   if (info.GetFileActionForFD(STDERR_FILENO) == nullptr)
 err_file_spec = GetStandardErrorPath();
 
-  LLDB_LOG(log, "target stdin='{0}', target stdout='{1}', stderr='{1}'",
+  LLDB_LOG(log, "target stdin='{0}', target stdout='{1}', stderr='{2}'",
in_file_spec, out_file_spec, err_file_spec);
 
   if (in_file_spec) {



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


[Lldb-commits] [lldb] [lldb] fix vFile:open, vFile:unlink error codes (PR #106950)

2024-09-25 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

> Feels like we diverted you from your original problem though, which was the 
> remote taking the informative errno and changing it to some meaningless value.

It should do that though, shouldn't it?  We look up the ERRNO constant we got 
in the GDBRemoteErrno table, sending back the constant values that the gdb RSP 
docs define, and if it's an errno not included in that table, we send back 
GDB_EUNKNOWN.

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


[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-25 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/108495

>From 2bf9eaf7e01293e6c1f388daf14cb8099314a5cf Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 25 Sep 2024 11:04:56 -0700
Subject: [PATCH] [lldb] Introduce an always-on system log category/channel

Add an "always on" log category and channel. Unlike other, existing log
channels, it is not exposed to users. The channel is meant to be used
sparsely and deliberately for logging high-value information to the
system log.

We have a similar concept in the downstream Swift fork and this has
proven to be extremely valuable. This is especially true on macOS where
system log messages are automatically captured as part of a sysdiagnose.

This patch revives https://reviews.llvm.org/D135621 although the purpose
is slightly different (logging to the system log rather than to a ring
buffer dumped as part of the diagnostics). This patch addresses all the
remaining oustanding comments.
---
 lldb/include/lldb/Host/Host.h | 16 
 lldb/include/lldb/Utility/Log.h   | 11 ++-
 lldb/source/API/SystemInitializerFull.cpp |  5 +
 lldb/source/Host/common/Host.cpp  | 16 
 lldb/source/Host/common/HostInfoBase.cpp  |  2 ++
 lldb/test/Shell/Host/TestSytemLogChannel.test |  3 +++
 6 files changed, 48 insertions(+), 5 deletions(-)
 create mode 100644 lldb/test/Shell/Host/TestSytemLogChannel.test

diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 9d0994978402f7..1ca89fa056aa93 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -31,6 +31,22 @@ class ProcessInstanceInfo;
 class ProcessInstanceInfoMatch;
 typedef std::vector ProcessInstanceInfoList;
 
+// Always on system log category and channel.
+enum class SystemLog : Log::MaskType {
+  System = Log::ChannelFlag<0>,
+  LLVM_MARK_AS_BITMASK_ENUM(System)
+};
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+class LogChannelSystem {
+public:
+  static void Initialize();
+  static void Terminate();
+};
+
+template <> Log::Channel &LogChannelFor();
+
 // Exit Type for inferior processes
 struct WaitStatus {
   enum Type : uint8_t {
diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 27707c17f9b824..d709e8c4f04c6e 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -272,6 +272,12 @@ class Log final {
   void VAFormatf(llvm::StringRef file, llvm::StringRef function,
  const char *format, va_list args);
 
+  void Enable(const std::shared_ptr &handler_sp,
+  uint32_t options = 0,
+  MaskType flags = std::numeric_limits::max());
+
+  void Disable(MaskType flags = 0);
+
 private:
   Channel &m_channel;
 
@@ -297,11 +303,6 @@ class Log final {
 return m_handler;
   }
 
-  void Enable(const std::shared_ptr &handler_sp, uint32_t options,
-  MaskType flags);
-
-  void Disable(MaskType flags);
-
   bool Dump(llvm::raw_ostream &stream);
 
   typedef llvm::StringMap ChannelMap;
diff --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index 995d14f7c1fa1e..1958db7291fb64 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -20,6 +20,8 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetSelect.h"
 
+#include "lldb/Version/Version.h"
+
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wglobal-constructors"
 #include "llvm/ExecutionEngine/MCJIT.h"
@@ -83,6 +85,9 @@ llvm::Error SystemInitializerFull::Initialize() {
   // Use the Debugger's LLDBAssert callback.
   SetLLDBAssertCallback(Debugger::AssertCallback);
 
+  // Log the LLDB version to the system log.
+  LLDB_LOG(GetLog(SystemLog::System), "{0}", GetVersion());
+
   return llvm::Error::success();
 }
 
diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index f08adea6546ae1..abca6068d3604a 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -125,6 +125,22 @@ void Host::SystemLog(Severity severity, llvm::StringRef 
message) {
 #endif
 #endif
 
+static constexpr Log::Category g_categories[] = {
+{{"system"}, {"system log"}, SystemLog::System}};
+
+static Log::Channel g_system_channel(g_categories, SystemLog::System);
+static Log g_system_log(g_system_channel);
+
+template <> Log::Channel &lldb_private::LogChannelFor() {
+  return g_system_channel;
+}
+
+void LogChannelSystem::Initialize() {
+  g_system_log.Enable(std::make_shared());
+}
+
+void LogChannelSystem::Terminate() { g_system_log.Disable(); }
+
 #if !defined(__APPLE__) && !defined(_WIN32)
 static thread_result_t
 MonitorChildProcessThreadFunction(::pid_t pid,
diff --git a/lldb/source/Host/common/HostInfoBase.cpp 
b/lldb/source/Host/common/HostInfoBase.cpp
index 5c44c2f38b2879..89dfe4a9e9baa3 100644
--- a/lldb/source/Host/common/Ho

[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-25 Thread Jonas Devlieghere via lldb-commits

JDevlieghere wrote:

Rebased + addressed @labath and @adrian-prantl's feedback. 

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


[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -224,6 +224,14 @@ const char *Status::AsCString(const char 
*default_error_str) const {
 if (!m_string.empty() && m_string[m_string.size() - 1] == '\n')
   m_string.pop_back();
 
+  // FIXME: Workaround for ErrorList[ExpressionError, ...].
+  if (m_error.isA()) {
+while (!m_string.empty() && m_string[0] == '\n')
+  m_string = std::string(m_string.data() + 1, m_string.size() - 1);
+if (!m_string.empty() && m_string[m_string.size() - 1] != '\n')
+  m_string += '\n';
+  }

labath wrote:

Also, stepping back a bit, I'm wondering if using an ErrorList is the best way 
to represent these errors. The alternative I'm considering is having a single 
ExpressionError object which contains a vector of "diagnostics" as a regular 
member. If nothing else, that would give you more control over how these get 
serialized into a string (but maybe it would also be easier to print them out 
using the fancy ascii art?)

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


[Lldb-commits] [lldb] [lldb] Store expression evaluator diagnostics in an llvm::Error (NFC) (PR #106442)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -61,8 +65,50 @@ std::string DiagnosticManager::GetString(char separator) {
   stream << message.drop_front(severity_pos + severity.size());
 stream << separator;
   }
+  return str;
+}
 
-  return ret;
+void DiagnosticManager::Dump(Log *log) {
+  if (!log)
+return;
+
+  std::string str = GetString();
+
+  // We want to remove the last '\n' because log->PutCString will add
+  // one for us.
+
+  if (str.size() && str.back() == '\n')

labath wrote:

```suggestion
  if (!str.empty() && str.back() == '\n')
```

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


[Lldb-commits] [libcxx] [lldb] [lldb][libc++] Hide all libc++ implementation details from stacktraces (PR #108870)

2024-09-25 Thread Michael Buch via lldb-commits


@@ -90,9 +79,28 @@ class LibCXXFrameRecognizer : public StackFrameRecognizer {
 if (!sc.function)
   return {};
 
-for (RegularExpression &r : m_hidden_regex)
-  if (r.Execute(sc.function->GetNameNoArguments()))
+// Check if we have a regex match
+for (RegularExpression &r : m_hidden_regex) {
+  if (!r.Execute(sc.function->GetNameNoArguments()))
+continue;
+
+  // Only hide this frame if the immediate caller is also within libc++.
+  lldb::ThreadSP thread_sp = frame_sp->GetThread();
+  if (!thread_sp)
+return {};
+  lldb::StackFrameSP parent_frame_sp =
+  thread_sp->GetStackFrameAtIndex(frame_sp->GetFrameIndex() + 1);
+  if (!parent_frame_sp)
+return {};
+  const auto &parent_sc =
+  parent_frame_sp->GetSymbolContext(lldb::eSymbolContextFunction);
+  if (!parent_sc.function)
+return {};
+  if (parent_sc.function->GetNameNoArguments().GetStringRef().starts_with(
+  "std::")) {
 return m_hidden_frame;
+  }

Michael137 wrote:

```suggestion
  if (parent_sc.function->GetNameNoArguments().GetStringRef().starts_with(
  "std::"))
return m_hidden_frame;
```

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


[Lldb-commits] [libcxx] [lldb] [lldb][libc++] Hide all libc++ implementation details from stacktraces (PR #108870)

2024-09-25 Thread Michael Buch via lldb-commits

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


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


[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for the fpmr register (PR #109934)

2024-09-25 Thread David Spickett via lldb-commits

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

The FP8 formats have a "_" in the name so that they are:
1. Easier to read.
2. Possible to use in register expressions if/when they are supported.

Some other bits do have defined meanings but they are not simple to name. 
Better that folks read the manual for those.

See this page for the full details:
https://developer.arm.com/documentation/ddi0601/2024-06/AArch64-Registers/FPMR--Floating-point-Mode-Register

>From 2d46b8537041f42ca57f480dc3185d4b27571c6f Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Thu, 29 Aug 2024 15:52:55 +0100
Subject: [PATCH] [lldb][AArch64] Add register fields for the fpmr register

The FP8 formats have a "_" in the name so that they are:
1. Easier to read.
2. Possible to use in register expressions if/when they are supported.

Some other bits do have defined meanings but they are not
simple to name. Better that folks read the manual for those.

See this page for the full details:
https://developer.arm.com/documentation/ddi0601/2024-06/AArch64-Registers/FPMR--Floating-point-Mode-Register
---
 .../Utility/RegisterFlagsDetector_arm64.cpp   | 24 +++
 .../Utility/RegisterFlagsDetector_arm64.h |  4 +++-
 .../aarch64/fpmr/TestAArch64LinuxFPMR.py  |  5 
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git 
a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp
index 7c8dba3680938d..72ced42a158233 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp
@@ -23,9 +23,33 @@
 #define HWCAP2_AFP (1ULL << 20)
 #define HWCAP2_SME (1ULL << 23)
 #define HWCAP2_EBF16 (1ULL << 32)
+#define HWCAP2_FPMR (1UL << 48)
 
 using namespace lldb_private;
 
+Arm64RegisterFlagsDetector::Fields
+Arm64RegisterFlagsDetector::DetectFPMRFields(uint64_t hwcap, uint64_t hwcap2) {
+  (void)hwcap;
+
+  if (!(hwcap2 & HWCAP2_FPMR))
+return {};
+
+  static const FieldEnum fp8_format_enum("fp8_format_enum", {
+{0, 
"FP8_E5M2"},
+{1, 
"FP8_E4M3"},
+});
+  return {
+  {"LSCALE2", 32, 37},
+  {"NSCALE", 24, 31},
+  {"LSCALE", 16, 22},
+  {"OSC", 15},
+  {"OSM", 14},
+  {"F8D", 6, 8, &fp8_format_enum},
+  {"F8S2", 3, 5, &fp8_format_enum},
+  {"F8S1", 0, 2, &fp8_format_enum},
+  };
+}
+
 Arm64RegisterFlagsDetector::Fields
 Arm64RegisterFlagsDetector::DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2) {
   (void)hwcap;
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h
index a5bb38670b9cdf..0f3d53d93892bd 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h
@@ -60,6 +60,7 @@ class Arm64RegisterFlagsDetector {
   static Fields DetectFPCRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2);
+  static Fields DetectFPMRFields(uint64_t hwcap, uint64_t hwcap2);
 
   struct RegisterEntry {
 RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
@@ -69,12 +70,13 @@ class Arm64RegisterFlagsDetector {
 llvm::StringRef m_name;
 RegisterFlags m_flags;
 DetectorFn m_detector;
-  } m_registers[5] = {
+  } m_registers[6] = {
   RegisterEntry("cpsr", 4, DetectCPSRFields),
   RegisterEntry("fpsr", 4, DetectFPSRFields),
   RegisterEntry("fpcr", 4, DetectFPCRFields),
   RegisterEntry("mte_ctrl", 8, DetectMTECtrlFields),
   RegisterEntry("svcr", 8, DetectSVCRFields),
+  RegisterEntry("fpmr", 8, DetectFPMRFields),
   };
 
   // Becomes true once field detection has been run for all registers.
diff --git a/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py 
b/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py
index 5a3b8f501095e9..d022c8eb3d6cc4 100644
--- a/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py
+++ b/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py
@@ -45,6 +45,11 @@ def test_fpmr_register(self):
 substrs=["Floating Point Mode Register", f"fpmr = 
{expected_fpmr:#018x}"],
 )
 
+if self.hasXMLSupport():
+self.expect(
+"register read fpmr", substrs=["LSCALE2 = 42", "F8S1 = 
FP8_E4M3 | 0x4"]
+)
+
 # Write a value for the program to find. Same fields but with bit 
values
 # inverted.
 new_fpmr = (0b010101 << 32) | 0b010

___
lldb-commits mailing list
lldb-commits@lists.llvm.o

[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for the fpmr register (PR #109934)

2024-09-25 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

The FP8 formats have a "_" in the name so that they are:
1. Easier to read.
2. Possible to use in register expressions if/when they are supported.

Some other bits do have defined meanings but they are not simple to name. 
Better that folks read the manual for those.

See this page for the full details:
https://developer.arm.com/documentation/ddi0601/2024-06/AArch64-Registers/FPMR--Floating-point-Mode-Register

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


3 Files Affected:

- (modified) 
lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp (+24) 
- (modified) lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h 
(+3-1) 
- (modified) lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py (+5) 


``diff
diff --git 
a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp
index 7c8dba3680938d..72ced42a158233 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp
@@ -23,9 +23,33 @@
 #define HWCAP2_AFP (1ULL << 20)
 #define HWCAP2_SME (1ULL << 23)
 #define HWCAP2_EBF16 (1ULL << 32)
+#define HWCAP2_FPMR (1UL << 48)
 
 using namespace lldb_private;
 
+Arm64RegisterFlagsDetector::Fields
+Arm64RegisterFlagsDetector::DetectFPMRFields(uint64_t hwcap, uint64_t hwcap2) {
+  (void)hwcap;
+
+  if (!(hwcap2 & HWCAP2_FPMR))
+return {};
+
+  static const FieldEnum fp8_format_enum("fp8_format_enum", {
+{0, 
"FP8_E5M2"},
+{1, 
"FP8_E4M3"},
+});
+  return {
+  {"LSCALE2", 32, 37},
+  {"NSCALE", 24, 31},
+  {"LSCALE", 16, 22},
+  {"OSC", 15},
+  {"OSM", 14},
+  {"F8D", 6, 8, &fp8_format_enum},
+  {"F8S2", 3, 5, &fp8_format_enum},
+  {"F8S1", 0, 2, &fp8_format_enum},
+  };
+}
+
 Arm64RegisterFlagsDetector::Fields
 Arm64RegisterFlagsDetector::DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2) {
   (void)hwcap;
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h
index a5bb38670b9cdf..0f3d53d93892bd 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h
@@ -60,6 +60,7 @@ class Arm64RegisterFlagsDetector {
   static Fields DetectFPCRFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2);
   static Fields DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2);
+  static Fields DetectFPMRFields(uint64_t hwcap, uint64_t hwcap2);
 
   struct RegisterEntry {
 RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
@@ -69,12 +70,13 @@ class Arm64RegisterFlagsDetector {
 llvm::StringRef m_name;
 RegisterFlags m_flags;
 DetectorFn m_detector;
-  } m_registers[5] = {
+  } m_registers[6] = {
   RegisterEntry("cpsr", 4, DetectCPSRFields),
   RegisterEntry("fpsr", 4, DetectFPSRFields),
   RegisterEntry("fpcr", 4, DetectFPCRFields),
   RegisterEntry("mte_ctrl", 8, DetectMTECtrlFields),
   RegisterEntry("svcr", 8, DetectSVCRFields),
+  RegisterEntry("fpmr", 8, DetectFPMRFields),
   };
 
   // Becomes true once field detection has been run for all registers.
diff --git a/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py 
b/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py
index 5a3b8f501095e9..d022c8eb3d6cc4 100644
--- a/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py
+++ b/lldb/test/API/linux/aarch64/fpmr/TestAArch64LinuxFPMR.py
@@ -45,6 +45,11 @@ def test_fpmr_register(self):
 substrs=["Floating Point Mode Register", f"fpmr = 
{expected_fpmr:#018x}"],
 )
 
+if self.hasXMLSupport():
+self.expect(
+"register read fpmr", substrs=["LSCALE2 = 42", "F8S1 = 
FP8_E4M3 | 0x4"]
+)
+
 # Write a value for the program to find. Same fields but with bit 
values
 # inverted.
 new_fpmr = (0b010101 << 32) | 0b010

``




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


[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for the fpmr register (PR #109934)

2024-09-25 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Core files will be the next PR and a test will be added to ensure fields show 
up when using a core file too.

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


[Lldb-commits] [lldb] Improve type lookup using .debug_names parent chain (PR #108907)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -374,25 +377,40 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
   m_fallback.GetFullyQualifiedType(context, callback);
 }
 
+bool DebugNamesDWARFIndex::SameAsEntryATName(
+llvm::StringRef query_name, const DebugNames::Entry &entry) const {
+  auto maybe_dieoffset = entry.getDIEUnitOffset();
+  if (!maybe_dieoffset)
+return false;
+
+  // [Optimization] instead of parsing the entry from dwo file, we simply
+  // check if the query_name can point to an entry of the same DIE offset.
+  // This greatly reduced number of dwo file parsed and thus improved the
+  // performance.
+  for (const DebugNames::Entry &query_entry :
+   entry.getNameIndex()->equal_range(query_name)) {
+auto query_dieoffset = query_entry.getDIEUnitOffset();
+if (!query_dieoffset)
+  continue;
+
+if (*query_dieoffset == *maybe_dieoffset) {
+  return true;
+} else if (*query_dieoffset > *maybe_dieoffset) {
+  // The pool entries of the same name are sequentially cluttered together
+  // so if the query name from `query_name` is after the target entry, this
+  // is definitely not the correct one, we can stop searching.

labath wrote:

Okay, I think I've finally understood what you mean now. You want to 
reverse-engineer the name of an debug_names entry (check whether it matches the 
expected string) by looking at whether its contained in the range of entries 
belonging to that name.

That should be doable (and it's quite clever actually), but we don't currently 
have an API which would make that possible. If necessary, I think we can add 
one, though.

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits


@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)

dzhidzhoev wrote:

I thought it failed because of the missing tool, but it unexpectedly passed on 
my machine on mainline.

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits


@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)

dzhidzhoev wrote:

https://lab.llvm.org/staging/#/builders/195/builds/2710/steps/16/logs/XPASS__lldb-api__TestBSDArchives_py

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits


@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)

DavidSpickett wrote:

Ok fair enough, maybe it's because the `llvm-whatever` is better than the 
ancient distro tool.

But if this is already UPASS-ing without this PR then it should be put in its 
own commit (if you can push direct) or PR (if not).

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits

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

Decide what to do with the remote test change but otherwise this LGTM.

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

https://github.com/dzhidzhoev updated 
https://github.com/llvm/llvm-project/pull/109961

>From 025276a52939c4091181dea5f633e443c908c4ef Mon Sep 17 00:00:00 2001
From: Vladislav Dzhidzhoev 
Date: Mon, 16 Sep 2024 17:48:15 +0200
Subject: [PATCH 1/3] [lldb][test] Use tools from llvm instead of
 compiler-based tools

In #102185, toolchain detection for API tests has been rewritten to Python.
However, tools paths for tests are determined from compiler path.

Here tools are taken from `--llvm-tools-dir` dotest.py argument, which
by default refers to the LLVM build directory, unless they are explicitly
redefined in environment variables. It helps to minimize external
dependencies and to maximize the reproducibility of the build.
---
 .../Python/lldbsuite/test/builders/builder.py   | 13 +++--
 .../packages/Python/lldbsuite/test/configuration.py |  3 +++
 lldb/packages/Python/lldbsuite/test/dotest.py   |  1 +
 lldb/test/API/functionalities/archives/Makefile |  2 --
 .../API/functionalities/archives/TestBSDArchives.py |  1 -
 5 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 564918c58b6dd2..90f11fffbb4928 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -110,6 +110,10 @@ def getToolchainSpec(self, compiler):
 if not cc:
 return []
 
+exe_ext = ""
+if lldbplatformutil.getHostPlatform() == "windows":
+exe_ext = ".exe"
+
 cc = cc.strip()
 cc_path = pathlib.Path(cc)
 
@@ -149,9 +153,9 @@ def getToolchainSpec(self, compiler):
 cc_dir = cc_path.parent
 
 def getToolchainUtil(util_name):
-return cc_dir / (cc_prefix + util_name + cc_ext)
+return os.path.join(configuration.llvm_tools_dir, util_name + 
exe_ext)
 
-cxx = getToolchainUtil(cxx_type)
+cxx = cc_dir / (cc_prefix + cxx_type + cc_ext)
 
 util_names = {
 "OBJCOPY": "objcopy",
@@ -161,6 +165,11 @@ def getToolchainUtil(util_name):
 }
 utils = []
 
+# Required by API TestBSDArchives.py tests.
+# TODO don't forget to fix the test's Makefile when porting to mainline
+if not os.getenv("LLVM_AR"):
+utils.extend(["LLVM_AR=%s" % getToolchainUtil("llvm-ar")])
+
 if not lldbplatformutil.platformIsDarwin():
 if cc_type in ["clang", "cc", "gcc"]:
 util_paths = {}
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 27eef040497d13..1bacd74a968c31 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -118,6 +118,9 @@
 # same base name.
 all_tests = set()
 
+# Path to LLVM tools to be used by tests.
+llvm_tools_dir = None
+
 # LLDB library directory.
 lldb_libs_dir = None
 lldb_obj_root = None
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index f14a00a2394b0b..b1ae896d3fd3b4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,6 +280,7 @@ def parseOptionsAndInitTestdirs():
 "xcrun -find -toolchain default dsymutil"
 )
 if args.llvm_tools_dir:
+configuration.llvm_tools_dir = args.llvm_tools_dir
 configuration.filecheck = shutil.which("FileCheck", 
path=args.llvm_tools_dir)
 configuration.yaml2obj = shutil.which("yaml2obj", 
path=args.llvm_tools_dir)
 
diff --git a/lldb/test/API/functionalities/archives/Makefile 
b/lldb/test/API/functionalities/archives/Makefile
index c4c593e6db0519..4b9696e26b5752 100644
--- a/lldb/test/API/functionalities/archives/Makefile
+++ b/lldb/test/API/functionalities/archives/Makefile
@@ -12,12 +12,10 @@ libfoo.a: a.o b.o
 
 # This tests whether lldb can load a thin archive
 libbar.a: c.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsDT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
 libfoo-thin.a: a.o b.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsUT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
diff --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py 
b/lldb/test/API/functionalities/archives/TestBSDArchives.py
index 1bef8e896e0be7..928e9508617ad6 100644
--- a/lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ b/lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)
 def test(self):
 """Break inside a() and b() defined within libfoo.a."""
 s

[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits


@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)

dzhidzhoev wrote:

Fixed

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

dzhidzhoev wrote:

> Decide what to do with the remote test change but otherwise this LGTM.

Some other tests are unexpectedly passing on remote config, so they'll need 
extra attention.

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


[Lldb-commits] [lldb] aea0668 - [lldb][test] Use tools from llvm instead of compiler tools (#109961)

2024-09-25 Thread via lldb-commits

Author: Vladislav Dzhidzhoev
Date: 2024-09-25T16:19:02+02:00
New Revision: aea06684992873f70c5834e2f455f913e5b8d671

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

LOG: [lldb][test] Use tools from llvm instead of compiler tools (#109961)

In #102185, toolchain detection for API tests has been rewritten in
Python. Tools paths for tests there are determined from compiler path.

Here tools are taken from `--llvm-tools-dir` dotest.py argument, which
by default refers to the LLVM build directory, unless they are
explicitly redefined in environment variables. It helps to minimize
external dependencies and to maximize the reproducibility of the build.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/builder.py
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/test/API/functionalities/archives/Makefile

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 564918c58b6dd2..e3099219e437e3 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -110,6 +110,10 @@ def getToolchainSpec(self, compiler):
 if not cc:
 return []
 
+exe_ext = ""
+if lldbplatformutil.getHostPlatform() == "windows":
+exe_ext = ".exe"
+
 cc = cc.strip()
 cc_path = pathlib.Path(cc)
 
@@ -149,9 +153,9 @@ def getToolchainSpec(self, compiler):
 cc_dir = cc_path.parent
 
 def getToolchainUtil(util_name):
-return cc_dir / (cc_prefix + util_name + cc_ext)
+return os.path.join(configuration.llvm_tools_dir, util_name + 
exe_ext)
 
-cxx = getToolchainUtil(cxx_type)
+cxx = cc_dir / (cc_prefix + cxx_type + cc_ext)
 
 util_names = {
 "OBJCOPY": "objcopy",
@@ -161,6 +165,10 @@ def getToolchainUtil(util_name):
 }
 utils = []
 
+# Required by API TestBSDArchives.py tests.
+if not os.getenv("LLVM_AR"):
+utils.extend(["LLVM_AR=%s" % getToolchainUtil("llvm-ar")])
+
 if not lldbplatformutil.platformIsDarwin():
 if cc_type in ["clang", "cc", "gcc"]:
 util_paths = {}

diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 27eef040497d13..1bacd74a968c31 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -118,6 +118,9 @@
 # same base name.
 all_tests = set()
 
+# Path to LLVM tools to be used by tests.
+llvm_tools_dir = None
+
 # LLDB library directory.
 lldb_libs_dir = None
 lldb_obj_root = None

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index f14a00a2394b0b..b1ae896d3fd3b4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,6 +280,7 @@ def parseOptionsAndInitTestdirs():
 "xcrun -find -toolchain default dsymutil"
 )
 if args.llvm_tools_dir:
+configuration.llvm_tools_dir = args.llvm_tools_dir
 configuration.filecheck = shutil.which("FileCheck", 
path=args.llvm_tools_dir)
 configuration.yaml2obj = shutil.which("yaml2obj", 
path=args.llvm_tools_dir)
 

diff  --git a/lldb/test/API/functionalities/archives/Makefile 
b/lldb/test/API/functionalities/archives/Makefile
index c4c593e6db0519..4b9696e26b5752 100644
--- a/lldb/test/API/functionalities/archives/Makefile
+++ b/lldb/test/API/functionalities/archives/Makefile
@@ -12,12 +12,10 @@ libfoo.a: a.o b.o
 
 # This tests whether lldb can load a thin archive
 libbar.a: c.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsDT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
 libfoo-thin.a: a.o b.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsUT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 



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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

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


[Lldb-commits] [lldb] 26e0b50 - [lldb][lldb-dap] Fix compilation error on 32 bit platforms

2024-09-25 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2024-09-25T14:31:57Z
New Revision: 26e0b5077236064d9ab0548e049dffce4d476c06

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

LOG: [lldb][lldb-dap] Fix compilation error on 32 bit platforms

https://github.com/llvm/llvm-project/pull/109485 tried to std::min
between size_t and uint64_t. size_t on 32 bit is 32 bits.

https://lab.llvm.org/buildbot/#/builders/18/builds/4430/steps/4/logs/stdio

Explicitly select the size_t template to fix this.

This will truncate one of the arguments but that's the count_requested.
If you're debugging from a 32 bit host and you asked it to read
> 32 bit range of memory from a 64 bit target, you weren't going
to have any success anyway.

The final result needs to be size_t to resize the vector with.

Added: 


Modified: 
lldb/tools/lldb-dap/lldb-dap.cpp

Removed: 




diff  --git a/lldb/tools/lldb-dap/lldb-dap.cpp 
b/lldb/tools/lldb-dap/lldb-dap.cpp
index f692d77347038c..db4dbbd6f6200a 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4451,7 +4451,7 @@ void request_readMemory(const llvm::json::Object 
&request) {
 g_dap.SendJSON(llvm::json::Value(std::move(response)));
 return;
   }
-  buf.resize(std::min(count_result, count_requested));
+  buf.resize(std::min(count_result, count_requested));
 
   llvm::json::Object body;
   std::string formatted_addr = "0x" + llvm::utohexstr(addr_int);



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


[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -393,125 +362,222 @@ int main_platform(int argc, char *argv[]) {
   lldb_private::Args inferior_arguments;
   inferior_arguments.SetArguments(argc, const_cast(argv));
 
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
   if (fd != SharedSocket::kInvalidFD) {
 // Child process will handle the connection and exit.
+if (gdbserver_port)
+  protocol = Socket::ProtocolTcp;
+
 Log *log = GetLog(LLDBLog::Platform);
-if (!listen_host_port.empty()) {
-  LLDB_LOGF(log, "lldb-platform child: "
- "ambiguous parameters --listen and --child-platform-fd");
-  return socket_error;
-}
 
-NativeSocket socket;
-error = SharedSocket::GetNativeSocket(fd, socket);
+NativeSocket sockfd;
+error = SharedSocket::GetNativeSocket(fd, sockfd);
 if (error.Fail()) {
   LLDB_LOGF(log, "lldb-platform child: %s", error.AsCString());
   return socket_error;
 }
 
-GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, "tcp");
-if (port_offset > 0)
-  platform.SetPortOffset(port_offset);
-platform.SetPortMap(std::move(gdbserver_portmap));
+GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
+Socket *socket;
+if (protocol == Socket::ProtocolTcp)
+  socket = new TCPSocket(sockfd, /*should_close=*/true,
+ /*child_processes_inherit=*/false);
+else {
+#if LLDB_ENABLE_POSIX
+  socket = new DomainSocket(sockfd, /*should_close=*/true,
+/*child_processes_inherit=*/false);
+#else
+  LLDB_LOGF(log,
+"lldb-platform child: Unix domain sockets are not supported on 
"
+"this platform.");
+  return socket_error;
+#endif
+}
 platform.SetConnection(
-std::unique_ptr(new ConnectionFileDescriptor(
-new TCPSocket(socket, /*should_close=*/true,
-  /*child_processes_inherit=*/false;
+std::unique_ptr(new ConnectionFileDescriptor(socket)));
 client_handle(platform, inferior_arguments);
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
-  // the test suite makes many connections in parallel, let's not miss any.
-  // The highest this should get reasonably is a function of the number
-  // of target CPUs. For now, let's just use 100.
-  const int backlog = 100;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::string hostname;
+  uint16_t platform_port = 0;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+  } else {
+if (gdbserver_port) {
+  fprintf(stderr,
+  "--gdbserver-port %u is redundant for non-tcp protocol %s.",
+  gdbserver_port, Socket::FindSchemeByProtocol(protocol));
+  return socket_error;
+}
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s.\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+  }
+
+  std::unique_ptr sock_platform =
+  Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+  if (error.Fail()) {
+printf("Failed to create platform socket: %s\n", err

[Lldb-commits] [lldb] [lldb-dap] Simplify `readMemory` (PR #109485)

2024-09-25 Thread Adrian Vogelsgesang via lldb-commits

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


[Lldb-commits] [lldb] 786dc5a - [lldb-dap] Simplify `readMemory` (#109485)

2024-09-25 Thread via lldb-commits

Author: Adrian Vogelsgesang
Date: 2024-09-25T13:49:42+02:00
New Revision: 786dc5a2da9bb55d98c65d018de25d9bd31485ff

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

LOG: [lldb-dap] Simplify `readMemory` (#109485)

The `readMemory` request used the `MemoryRegionInfo` so it could also
support short reads. Since #106532, this is no longer necessary, as
mentioned by @labath in a comment on #104317.

With this commit, we no longer set the `unreadableBytes` in the result.
But this is optional, anyway, according to the spec, and afaik the
VS Code UI does not make good use of `unreadableBytes`, anyway.

We prefer `SBTarget::ReadMemory` over `SBProcess::ReadMemory`, because
the `memory read` command also reads memory through the target instead
of the process, and because users would expect the UI view and the
results from memory read to be in-sync.

Added: 


Modified: 
lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
lldb/tools/lldb-dap/lldb-dap.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py 
b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
index 1082541aebcf7c..ea43fccf016a7f 100644
--- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -93,15 +93,18 @@ def test_readMemory(self):
 
 # We can read the complete string
 mem = self.dap_server.request_readMemory(memref, 0, 5)["body"]
-self.assertEqual(mem["unreadableBytes"], 0)
 self.assertEqual(b64decode(mem["data"]), b"dead\0")
 
+# We can read large chunks, potentially returning partial results
+mem = self.dap_server.request_readMemory(memref, 0, 4096)["body"]
+self.assertEqual(b64decode(mem["data"])[0:5], b"dead\0")
+
 # Use an offset
 mem = self.dap_server.request_readMemory(memref, 2, 3)["body"]
 self.assertEqual(b64decode(mem["data"]), b"ad\0")
 
 # Reads of size 0 are successful
-# VS-Code sends those in order to check if a `memoryReference` can 
actually be dereferenced.
+# VS Code sends those in order to check if a `memoryReference` can 
actually be dereferenced.
 mem = self.dap_server.request_readMemory(memref, 0, 0)
 self.assertEqual(mem["success"], True)
 self.assertEqual(mem["body"]["data"], "")
@@ -109,4 +112,3 @@ def test_readMemory(self):
 # Reads at offset 0x0 fail
 mem = self.dap_server.request_readMemory("0x0", 0, 6)
 self.assertEqual(mem["success"], False)
-self.assertEqual(mem["message"], "Memory region is not readable")

diff  --git a/lldb/tools/lldb-dap/lldb-dap.cpp 
b/lldb/tools/lldb-dap/lldb-dap.cpp
index c7653fed2def4e..f692d77347038c 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4422,14 +4422,6 @@ void request_readMemory(const llvm::json::Object 
&request) {
   FillResponse(request, response);
   auto *arguments = request.getObject("arguments");
 
-  lldb::SBProcess process = g_dap.target.GetProcess();
-  if (!process.IsValid()) {
-response["success"] = false;
-response["message"] = "No process running";
-g_dap.SendJSON(llvm::json::Value(std::move(response)));
-return;
-  }
-
   llvm::StringRef memoryReference = GetString(arguments, "memoryReference");
   auto addr_opt = DecodeMemoryReference(memoryReference);
   if (!addr_opt.has_value()) {
@@ -4439,57 +4431,32 @@ void request_readMemory(const llvm::json::Object 
&request) {
 g_dap.SendJSON(llvm::json::Value(std::move(response)));
 return;
   }
-  lldb::addr_t addr = *addr_opt;
-
-  addr += GetSigned(arguments, "offset", 0);
-  const uint64_t requested_count = GetUnsigned(arguments, "count", 0);
-  lldb::SBMemoryRegionInfo region_info;
-  lldb::SBError memreg_error = process.GetMemoryRegionInfo(addr, region_info);
-  if (memreg_error.Fail()) {
-response["success"] = false;
-EmplaceSafeString(response, "message",
-  "Unable to find memory region: " +
-  std::string(memreg_error.GetCString()));
-g_dap.SendJSON(llvm::json::Value(std::move(response)));
-return;
-  }
-  if (!region_info.IsReadable()) {
+  lldb::addr_t addr_int = *addr_opt;
+  addr_int += GetSigned(arguments, "offset", 0);
+  const uint64_t count_requested = GetUnsigned(arguments, "count", 0);
+
+  // We also need support reading 0 bytes
+  // VS Code sends those requests to check if a `memoryReference`
+  // can be dereferenced.
+  const uint64_t count_read = std::max(count_requested, 1);
+  std::vector buf;
+  buf.resize(count_read);
+  lldb::SBError error;
+  lldb::SBAddress addr{addr_int, g_dap.target};
+  size_t count_result =
+  g_dap.target.ReadMemory(addr, buf.data(), count_read, error

[Lldb-commits] [libcxx] [lldb] [lldb][libc++] Hide all libc++ implementation details from stacktraces (PR #108870)

2024-09-25 Thread Adrian Vogelsgesang via lldb-commits


@@ -90,9 +79,28 @@ class LibCXXFrameRecognizer : public StackFrameRecognizer {
 if (!sc.function)
   return {};
 
-for (RegularExpression &r : m_hidden_regex)
-  if (r.Execute(sc.function->GetNameNoArguments()))
+// Check if we have a regex match
+for (RegularExpression &r : m_hidden_regex) {
+  if (!r.Execute(sc.function->GetNameNoArguments()))
+continue;
+
+  // Only hide this frame if the immediate caller is also within libc++.
+  lldb::ThreadSP thread_sp = frame_sp->GetThread();
+  if (!thread_sp)
+return {};
+  lldb::StackFrameSP parent_frame_sp =
+  thread_sp->GetStackFrameAtIndex(frame_sp->GetFrameIndex() + 1);
+  if (!parent_frame_sp)
+return {};
+  const auto &parent_sc =
+  parent_frame_sp->GetSymbolContext(lldb::eSymbolContextFunction);
+  if (!parent_sc.function)
+return {};
+  if (parent_sc.function->GetNameNoArguments().GetStringRef().starts_with(
+  "std::")) {
 return m_hidden_frame;
+  }

vogelsgesang wrote:

Argh. There really should be a clang-tidy check or something similar for this...

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits


@@ -161,6 +165,11 @@ def getToolchainUtil(util_name):
 }
 utils = []
 
+# Required by API TestBSDArchives.py tests.
+# TODO don't forget to fix the test's Makefile when porting to mainline

DavidSpickett wrote:

TODO left in

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits


@@ -149,9 +153,9 @@ def getToolchainSpec(self, compiler):
 cc_dir = cc_path.parent
 
 def getToolchainUtil(util_name):
-return cc_dir / (cc_prefix + util_name + cc_ext)
+return os.path.join(configuration.llvm_tools_dir, util_name + 
exe_ext)
 
-cxx = getToolchainUtil(cxx_type)
+cxx = cc_dir / (cc_prefix + cxx_type + cc_ext)

DavidSpickett wrote:

TIL that this magical mystery use of divide exists.

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits


@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)

DavidSpickett wrote:

And this works now because...? Because it in a remote context it finds the tool 
in the llvm build dir where before it wouldn't?

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits

https://github.com/DavidSpickett commented:

My only question here is, is there some tool that every bot is going to have to 
add an override for because it's not something in `/bin`?

If there is such a tool, that doesn't make this change bad, but we may want to 
delay while that's accounted for.

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

https://github.com/dzhidzhoev updated 
https://github.com/llvm/llvm-project/pull/109961

>From 025276a52939c4091181dea5f633e443c908c4ef Mon Sep 17 00:00:00 2001
From: Vladislav Dzhidzhoev 
Date: Mon, 16 Sep 2024 17:48:15 +0200
Subject: [PATCH 1/2] [lldb][test] Use tools from llvm instead of
 compiler-based tools

In #102185, toolchain detection for API tests has been rewritten to Python.
However, tools paths for tests are determined from compiler path.

Here tools are taken from `--llvm-tools-dir` dotest.py argument, which
by default refers to the LLVM build directory, unless they are explicitly
redefined in environment variables. It helps to minimize external
dependencies and to maximize the reproducibility of the build.
---
 .../Python/lldbsuite/test/builders/builder.py   | 13 +++--
 .../packages/Python/lldbsuite/test/configuration.py |  3 +++
 lldb/packages/Python/lldbsuite/test/dotest.py   |  1 +
 lldb/test/API/functionalities/archives/Makefile |  2 --
 .../API/functionalities/archives/TestBSDArchives.py |  1 -
 5 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 564918c58b6dd2..90f11fffbb4928 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -110,6 +110,10 @@ def getToolchainSpec(self, compiler):
 if not cc:
 return []
 
+exe_ext = ""
+if lldbplatformutil.getHostPlatform() == "windows":
+exe_ext = ".exe"
+
 cc = cc.strip()
 cc_path = pathlib.Path(cc)
 
@@ -149,9 +153,9 @@ def getToolchainSpec(self, compiler):
 cc_dir = cc_path.parent
 
 def getToolchainUtil(util_name):
-return cc_dir / (cc_prefix + util_name + cc_ext)
+return os.path.join(configuration.llvm_tools_dir, util_name + 
exe_ext)
 
-cxx = getToolchainUtil(cxx_type)
+cxx = cc_dir / (cc_prefix + cxx_type + cc_ext)
 
 util_names = {
 "OBJCOPY": "objcopy",
@@ -161,6 +165,11 @@ def getToolchainUtil(util_name):
 }
 utils = []
 
+# Required by API TestBSDArchives.py tests.
+# TODO don't forget to fix the test's Makefile when porting to mainline
+if not os.getenv("LLVM_AR"):
+utils.extend(["LLVM_AR=%s" % getToolchainUtil("llvm-ar")])
+
 if not lldbplatformutil.platformIsDarwin():
 if cc_type in ["clang", "cc", "gcc"]:
 util_paths = {}
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 27eef040497d13..1bacd74a968c31 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -118,6 +118,9 @@
 # same base name.
 all_tests = set()
 
+# Path to LLVM tools to be used by tests.
+llvm_tools_dir = None
+
 # LLDB library directory.
 lldb_libs_dir = None
 lldb_obj_root = None
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index f14a00a2394b0b..b1ae896d3fd3b4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,6 +280,7 @@ def parseOptionsAndInitTestdirs():
 "xcrun -find -toolchain default dsymutil"
 )
 if args.llvm_tools_dir:
+configuration.llvm_tools_dir = args.llvm_tools_dir
 configuration.filecheck = shutil.which("FileCheck", 
path=args.llvm_tools_dir)
 configuration.yaml2obj = shutil.which("yaml2obj", 
path=args.llvm_tools_dir)
 
diff --git a/lldb/test/API/functionalities/archives/Makefile 
b/lldb/test/API/functionalities/archives/Makefile
index c4c593e6db0519..4b9696e26b5752 100644
--- a/lldb/test/API/functionalities/archives/Makefile
+++ b/lldb/test/API/functionalities/archives/Makefile
@@ -12,12 +12,10 @@ libfoo.a: a.o b.o
 
 # This tests whether lldb can load a thin archive
 libbar.a: c.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsDT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
 libfoo-thin.a: a.o b.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsUT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
diff --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py 
b/lldb/test/API/functionalities/archives/TestBSDArchives.py
index 1bef8e896e0be7..928e9508617ad6 100644
--- a/lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ b/lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)
 def test(self):
 """Break inside a() and b() defined within libfoo.a."""
 s

[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits

DavidSpickett wrote:

A better version of my question, do you have a list of tools this effects, is 
it just the ones where you have modified lines in this PR?

If so we probably have nothing to worry about.

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

dzhidzhoev wrote:

> A better version of my question, do you have a list of tools this effects, is 
> it just the ones where you have modified lines in this PR?
> 
> If so we probably have nothing to worry about.

These are the tools listed in util_names dictionary 
https://github.com/llvm/llvm-project/blob/025276a52939c4091181dea5f633e443c908c4ef/lldb/packages/Python/lldbsuite/test/builders/builder.py#L160.

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


[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

2024-09-25 Thread via lldb-commits

dlav-sc wrote:

@lhames could you take a look, please?

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


[Lldb-commits] [libcxx] [lldb] [lldb][libc++] Hide all libc++ implementation details from stacktraces (PR #108870)

2024-09-25 Thread Adrian Vogelsgesang via lldb-commits

https://github.com/vogelsgesang updated 
https://github.com/llvm/llvm-project/pull/108870

>From 04daaac0eade25a439856bbb287e15860aba1dfd Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang 
Date: Tue, 27 Aug 2024 17:34:11 +
Subject: [PATCH 1/9] [lldb][libc++] Hide all libc++ implementation details
 from stacktraces

This commit changes the libc++ frame recognizer to hide implementation
details of libc++ more aggressively. The applied heuristic is rather
straightforward: We consider every function name starting with `__` as
an implementation detail.

This works pretty neatly for `std::invoke`, `std::function`,
`std::sort`, `std::map::emplace` and many others. Also, this should
align quite nicely with libc++'s general coding convention of using the
`__` for their implementation details, thereby keeping the future
maintenance effort low.

However, it is noteworthy, that this does not work 100% in all cases:
E.g., for `std::ranges::sort`, itself is not really a function call, but
an object with an overloaded `operator()`, which means that there is no
actual call `std::ranges::sort` in the call stack.
---
 libcxx/docs/UserDocumentation.rst | 26 ++
 .../CPlusPlus/CPPLanguageRuntime.cpp  | 27 ++
 .../cpp/libcxx-internals-recognizer/Makefile  |  5 ++
 .../TestLibcxxInternalsRecognizer.py  | 56 
 .../cpp/libcxx-internals-recognizer/main.cpp  | 86 +++
 5 files changed, 181 insertions(+), 19 deletions(-)
 create mode 100644 lldb/test/API/lang/cpp/libcxx-internals-recognizer/Makefile
 create mode 100644 
lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py
 create mode 100644 lldb/test/API/lang/cpp/libcxx-internals-recognizer/main.cpp

diff --git a/libcxx/docs/UserDocumentation.rst 
b/libcxx/docs/UserDocumentation.rst
index 6659fa54f49df5..8999b4f23e91d2 100644
--- a/libcxx/docs/UserDocumentation.rst
+++ b/libcxx/docs/UserDocumentation.rst
@@ -346,6 +346,32 @@ Third-party Integrations
 
 Libc++ provides integration with a few third-party tools.
 
+Debugging libc++ internals in LLDB
+--
+
+LLDB hides the implementation details of libc++ by default.
+
+E.g., when setting a breakpoint in a comparator passed to ``std::sort``, the
+backtrace will read as
+
+.. code-block::
+
+  (lldb) thread backtrace
+  * thread #1, name = 'a.out', stop reason = breakpoint 3.1
+* frame #0: 0x520e a.out`my_comparator(a=1, b=8) at 
test-std-sort.cpp:6:3
+  frame #7: 0x5615 a.out`void 
std::__1::sort[abi:ne20], bool (*)(int, 
int)>(__first=(item = 8), __last=(item = 0), __comp=(a.out`my_less(int, int) at 
test-std-sort.cpp:5)) at sort.h:1003:3
+  frame #8: 0x531a a.out`main at test-std-sort.cpp:24:3
+
+Note how the caller of ``my_comparator`` is shown as ``std::sort``. Looking at
+the frame numbers, we can see that frames #1 until #6 were hidden. Those frames
+represent internal implementation details such as ``__sort4`` and similar
+utility functions.
+
+To also show those implementation details, use ``thread backtrace -u``.
+Alternatively, to disable those compact backtraces for good, use
+``frame recognizer list`` and ``frame recognizer delete`` to delete the libc++
+frame recognizer.
+
 GDB Pretty printers for libc++
 --
 
diff --git 
a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp 
b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index faa05e8f834ea1..d0e84bdeb94f01 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -45,7 +45,7 @@ char CPPLanguageRuntime::ID = 0;
 /// A frame recognizer that is installed to hide libc++ implementation
 /// details from the backtrace.
 class LibCXXFrameRecognizer : public StackFrameRecognizer {
-  std::array m_hidden_regex;
+  std::array m_hidden_regex;
   RecognizedStackFrameSP m_hidden_frame;
 
   struct LibCXXHiddenFrame : public RecognizedStackFrame {
@@ -55,28 +55,17 @@ class LibCXXFrameRecognizer : public StackFrameRecognizer {
 public:
   LibCXXFrameRecognizer()
   : m_hidden_regex{
-// internal implementation details of std::function
+// internal implementation details in the `std::` namespace
 //std::__1::__function::__alloc_func, void ()>::operator()[abi:ne20]
 //std::__1::__function::__func, void ()>::operator()
 //std::__1::__function::__value_func::operator()[abi:ne20]() const
-RegularExpression{""
-  R"(^std::__[^:]*::)" // Namespace.
-  R"(__function::.*::operator\(\))"},
-// internal implementation details of std::function in ABI v2
 //std::__2::__function::__policy_invoker::__call_impl[abi:ne20]>
-RegularExpression{""
-  R"(^std::__[^:]*::)" // Namespace.
-

[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits


@@ -161,6 +165,11 @@ def getToolchainUtil(util_name):
 }
 utils = []
 
+# Required by API TestBSDArchives.py tests.
+# TODO don't forget to fix the test's Makefile when porting to mainline

dzhidzhoev wrote:

Thanks!

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


[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -8,6 +8,7 @@
 
 #include "lldb/Utility/Status.h"
 
+#include "lldb/Expression/DiagnosticManager.h"

labath wrote:

Definitely not a good idea.

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


[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -486,19 +603,37 @@ bool 
CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
 
 result.SetStatus(eReturnStatusSuccessFinishResult);
   } else {
-const char *error_cstr = result_valobj_sp->GetError().AsCString();
-if (error_cstr && error_cstr[0]) {
-  const size_t error_cstr_len = strlen(error_cstr);
-  const bool ends_with_newline = error_cstr[error_cstr_len - 1] == 
'\n';
-  if (strstr(error_cstr, "error:") != error_cstr)
-error_stream.PutCString("error: ");
-  error_stream.Write(error_cstr, error_cstr_len);
-  if (!ends_with_newline)
-error_stream.EOL();
+// Retrieve the diagnostics.
+std::vector details;
+llvm::consumeError(
+llvm::handleErrors(result_valobj_sp->GetError().ToError(),
+   [&](DetailedExpressionError &error) {
+ details.push_back(error.GetDetail());
+   }));
+// Find the position of the expression in the command.
+std::optional expr_pos;
+size_t nchar = m_original_command.find(expr);
+if (nchar != std::string::npos)
+  expr_pos = nchar + GetDebugger().GetPrompt().size();
+
+if (!details.empty()) {
+  bool multiline = expr.contains('\n');
+  RenderDiagnosticDetails(error_stream, expr_pos, multiline, details);
 } else {
-  error_stream.PutCString("error: unknown error\n");
+  const char *error_cstr = result_valobj_sp->GetError().AsCString();
+  if (error_cstr && error_cstr[0]) {
+const size_t error_cstr_len = strlen(error_cstr);
+const bool ends_with_newline =
+error_cstr[error_cstr_len - 1] == '\n';
+if (strstr(error_cstr, "error:") != error_cstr)

labath wrote:

Yes, please.

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


[Lldb-commits] [lldb] [lldb] Inline expression evaluator error visualization (PR #106470)

2024-09-25 Thread Pavel Labath via lldb-commits

labath wrote:

> > This seems like it could be problematic for IDEs, if they don't print the 
> > error in the same window as the expression being evaluated. The arrows 
> > could end up pointing to nowhere, or to the wrong place in the expression 
> > (if we don't get the right offset).
> 
> Eventually I want IDEs to get access to the same kind of machine-readable 
> diagnostics, so they can format errors however they like (or, if they aren't 
> interested, dump the pre-rendered textual error that is still available).

I'm still worried about this. Yes, the IDEs can dump the pre-rendered error 
(and right now, that's all they can do), but this rendering assumes that the 
error message will be printed in a particular way (right under the original 
command, which will include the prompt and everything). I think that's fine for 
the commands entered through the LLDB command line, but I don't think it's 
reasonable to expect that every caller of `SBCommandInterpreter::HandleCommand` 
will do the same thing. I've looked at what lldb-dap that, and it looks like it 
will mostly be okay, because it explicitly echoes the command to be executed 
(although it [hardcodes the prompt 
string](https://github.com/llvm/llvm-project/blob/60ed2361c0917b4f8d54cb85935cfbf8904aa51d/lldb/tools/lldb-dap/LLDBUtils.cpp#L61)
 instead of getting it from lldb), but if you're looking for a example, you 
don't need to look further than your test case. Even though you've formatted 
the test input nicely, this is how its trace looks like when you run it:

```
Ran command:
"expr -i 0 -o 0 -- a"

Got output:
 ^
 ╰─ error: use of undeclared identifier 'a'
```

(sure we can fix this so that the output makes sense, but I wonder how many 
other such callers are out there)

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler-based tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

https://github.com/dzhidzhoev created 
https://github.com/llvm/llvm-project/pull/109961

In #102185, toolchain detection for API tests has been rewritten to Python. 
However, tools paths for tests are determined from compiler path.

Here tools are taken from `--llvm-tools-dir` dotest.py argument, which by 
default refers to the LLVM build directory, unless they are explicitly 
redefined in environment variables. It helps to minimize external dependencies 
and to maximize the reproducibility of the build.

>From 025276a52939c4091181dea5f633e443c908c4ef Mon Sep 17 00:00:00 2001
From: Vladislav Dzhidzhoev 
Date: Mon, 16 Sep 2024 17:48:15 +0200
Subject: [PATCH] [lldb][test] Use tools from llvm instead of compiler-based
 tools

In #102185, toolchain detection for API tests has been rewritten to Python.
However, tools paths for tests are determined from compiler path.

Here tools are taken from `--llvm-tools-dir` dotest.py argument, which
by default refers to the LLVM build directory, unless they are explicitly
redefined in environment variables. It helps to minimize external
dependencies and to maximize the reproducibility of the build.
---
 .../Python/lldbsuite/test/builders/builder.py   | 13 +++--
 .../packages/Python/lldbsuite/test/configuration.py |  3 +++
 lldb/packages/Python/lldbsuite/test/dotest.py   |  1 +
 lldb/test/API/functionalities/archives/Makefile |  2 --
 .../API/functionalities/archives/TestBSDArchives.py |  1 -
 5 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 564918c58b6dd2..90f11fffbb4928 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -110,6 +110,10 @@ def getToolchainSpec(self, compiler):
 if not cc:
 return []
 
+exe_ext = ""
+if lldbplatformutil.getHostPlatform() == "windows":
+exe_ext = ".exe"
+
 cc = cc.strip()
 cc_path = pathlib.Path(cc)
 
@@ -149,9 +153,9 @@ def getToolchainSpec(self, compiler):
 cc_dir = cc_path.parent
 
 def getToolchainUtil(util_name):
-return cc_dir / (cc_prefix + util_name + cc_ext)
+return os.path.join(configuration.llvm_tools_dir, util_name + 
exe_ext)
 
-cxx = getToolchainUtil(cxx_type)
+cxx = cc_dir / (cc_prefix + cxx_type + cc_ext)
 
 util_names = {
 "OBJCOPY": "objcopy",
@@ -161,6 +165,11 @@ def getToolchainUtil(util_name):
 }
 utils = []
 
+# Required by API TestBSDArchives.py tests.
+# TODO don't forget to fix the test's Makefile when porting to mainline
+if not os.getenv("LLVM_AR"):
+utils.extend(["LLVM_AR=%s" % getToolchainUtil("llvm-ar")])
+
 if not lldbplatformutil.platformIsDarwin():
 if cc_type in ["clang", "cc", "gcc"]:
 util_paths = {}
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 27eef040497d13..1bacd74a968c31 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -118,6 +118,9 @@
 # same base name.
 all_tests = set()
 
+# Path to LLVM tools to be used by tests.
+llvm_tools_dir = None
+
 # LLDB library directory.
 lldb_libs_dir = None
 lldb_obj_root = None
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index f14a00a2394b0b..b1ae896d3fd3b4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,6 +280,7 @@ def parseOptionsAndInitTestdirs():
 "xcrun -find -toolchain default dsymutil"
 )
 if args.llvm_tools_dir:
+configuration.llvm_tools_dir = args.llvm_tools_dir
 configuration.filecheck = shutil.which("FileCheck", 
path=args.llvm_tools_dir)
 configuration.yaml2obj = shutil.which("yaml2obj", 
path=args.llvm_tools_dir)
 
diff --git a/lldb/test/API/functionalities/archives/Makefile 
b/lldb/test/API/functionalities/archives/Makefile
index c4c593e6db0519..4b9696e26b5752 100644
--- a/lldb/test/API/functionalities/archives/Makefile
+++ b/lldb/test/API/functionalities/archives/Makefile
@@ -12,12 +12,10 @@ libfoo.a: a.o b.o
 
 # This tests whether lldb can load a thin archive
 libbar.a: c.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsDT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
 libfoo-thin.a: a.o b.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsUT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
diff --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py 
b/lldb/test/API/functionalities/archives/TestBSDArchives.py
index 1bef8e896e0be7..928e9508617ad6 100644
--- a/lldb/test/API/f

[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler-based tools (PR #109961)

2024-09-25 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)


Changes

In #102185, toolchain detection for API tests has been rewritten in 
Python. However, tools paths for tests are determined from compiler path.

Here tools are taken from `--llvm-tools-dir` dotest.py argument, which by 
default refers to the LLVM build directory, unless they are explicitly 
redefined in environment variables. It helps to minimize external dependencies 
and to maximize the reproducibility of the build.

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


5 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/builders/builder.py (+11-2) 
- (modified) lldb/packages/Python/lldbsuite/test/configuration.py (+3) 
- (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+1) 
- (modified) lldb/test/API/functionalities/archives/Makefile (-2) 
- (modified) lldb/test/API/functionalities/archives/TestBSDArchives.py (-1) 


``diff
diff --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index 564918c58b6dd2..90f11fffbb4928 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -110,6 +110,10 @@ def getToolchainSpec(self, compiler):
 if not cc:
 return []
 
+exe_ext = ""
+if lldbplatformutil.getHostPlatform() == "windows":
+exe_ext = ".exe"
+
 cc = cc.strip()
 cc_path = pathlib.Path(cc)
 
@@ -149,9 +153,9 @@ def getToolchainSpec(self, compiler):
 cc_dir = cc_path.parent
 
 def getToolchainUtil(util_name):
-return cc_dir / (cc_prefix + util_name + cc_ext)
+return os.path.join(configuration.llvm_tools_dir, util_name + 
exe_ext)
 
-cxx = getToolchainUtil(cxx_type)
+cxx = cc_dir / (cc_prefix + cxx_type + cc_ext)
 
 util_names = {
 "OBJCOPY": "objcopy",
@@ -161,6 +165,11 @@ def getToolchainUtil(util_name):
 }
 utils = []
 
+# Required by API TestBSDArchives.py tests.
+# TODO don't forget to fix the test's Makefile when porting to mainline
+if not os.getenv("LLVM_AR"):
+utils.extend(["LLVM_AR=%s" % getToolchainUtil("llvm-ar")])
+
 if not lldbplatformutil.platformIsDarwin():
 if cc_type in ["clang", "cc", "gcc"]:
 util_paths = {}
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index 27eef040497d13..1bacd74a968c31 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -118,6 +118,9 @@
 # same base name.
 all_tests = set()
 
+# Path to LLVM tools to be used by tests.
+llvm_tools_dir = None
+
 # LLDB library directory.
 lldb_libs_dir = None
 lldb_obj_root = None
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index f14a00a2394b0b..b1ae896d3fd3b4 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -280,6 +280,7 @@ def parseOptionsAndInitTestdirs():
 "xcrun -find -toolchain default dsymutil"
 )
 if args.llvm_tools_dir:
+configuration.llvm_tools_dir = args.llvm_tools_dir
 configuration.filecheck = shutil.which("FileCheck", 
path=args.llvm_tools_dir)
 configuration.yaml2obj = shutil.which("yaml2obj", 
path=args.llvm_tools_dir)
 
diff --git a/lldb/test/API/functionalities/archives/Makefile 
b/lldb/test/API/functionalities/archives/Makefile
index c4c593e6db0519..4b9696e26b5752 100644
--- a/lldb/test/API/functionalities/archives/Makefile
+++ b/lldb/test/API/functionalities/archives/Makefile
@@ -12,12 +12,10 @@ libfoo.a: a.o b.o
 
 # This tests whether lldb can load a thin archive
 libbar.a: c.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsDT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
 libfoo-thin.a: a.o b.o
-   $(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
$(eval LLVM_ARFLAGS := -rcsUT)
$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
 
diff --git a/lldb/test/API/functionalities/archives/TestBSDArchives.py 
b/lldb/test/API/functionalities/archives/TestBSDArchives.py
index 1bef8e896e0be7..928e9508617ad6 100644
--- a/lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ b/lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)
 def test(self):
 """Break inside a() and b() defined within libfoo.a."""
 self.build()

``




https://github.com/llvm/llvm-project/pull/109961
___

[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler-based tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler-based tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread Vladislav Dzhidzhoev via lldb-commits

dzhidzhoev wrote:

> Is there some tool that every bot is going to have to add an override for 
> because it's not something in `/bin`?

AFAIK no. All these tools have llvm alternatives.

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


[Lldb-commits] [lldb] [lldb][test] Use tools from llvm instead of compiler tools (PR #109961)

2024-09-25 Thread David Spickett via lldb-commits


@@ -25,7 +25,6 @@ def setUp(self):
 oslist=["windows"],
 bugnumber="llvm.org/pr24527.  Makefile.rules doesn't know how to build 
static libs on Windows",
 )
-@expectedFailureAll(remote=True)

DavidSpickett wrote:

Just need a basic explanation of this then I'll approve the PR.

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


[Lldb-commits] [clang] [compiler-rt] [lldb] [llvm] [Support] Remove terminfo dependency (PR #92865)

2024-09-25 Thread Bernhard Kaindl via lldb-commits

bernhardkaindl wrote:

@davide-q: The patch linked from this comment avoids this issue 
https://github.com/llvm/llvm-project/issues/101368:
https://github.com/spack/spack/pull/46504#issuecomment-2372520318

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


[Lldb-commits] [lldb] [llvm] [lldb]Implement LLDB Telemetry (PR #98528)

2024-09-25 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] [lldb]Implement LLDB Telemetry (PR #98528)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,273 @@
+
+//===-- 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 "lldb/API/SBDebugger.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/TelemetryVendor.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/LLDBLog.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/JSON.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+namespace lldb_private {
+
+using ::llvm::telemetry::Destination;
+using ::llvm::telemetry::EventStats;
+using ::llvm::telemetry::ExitDescription;
+using ::llvm::telemetry::SteadyTimePoint;
+using ::llvm::telemetry::TelemetryInfo;
+
+static std::string
+ExitDescToString(const llvm::telemetry::ExitDescription *desc) {
+  return ("ExitCode:" + desc->ExitCode) +
+ (" ExixitDescription: " + desc->Description);
+}
+
+static std::string GetDuration(const EventStats &stats) {
+  if (stats.End.has_value())
+return std::to_string((stats.End.value() - stats.Start).count()) +
+   "(nanosec)";

labath wrote:

```suggestion
return llvm::formatv("{0:ns}", stats.End.value() - stats.Start).str();
```

(or maybe some other unit (ms/us) -- how small do we expect these to get)?

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] [lldb]Implement LLDB Telemetry (PR #98528)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,273 @@
+
+//===-- 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 "lldb/API/SBDebugger.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/TelemetryVendor.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/LLDBLog.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/JSON.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+namespace lldb_private {
+
+using ::llvm::telemetry::Destination;
+using ::llvm::telemetry::EventStats;
+using ::llvm::telemetry::ExitDescription;
+using ::llvm::telemetry::SteadyTimePoint;
+using ::llvm::telemetry::TelemetryInfo;
+
+static std::string
+ExitDescToString(const llvm::telemetry::ExitDescription *desc) {
+  return ("ExitCode:" + desc->ExitCode) +
+ (" ExixitDescription: " + desc->Description);
+}
+
+static std::string GetDuration(const EventStats &stats) {
+  if (stats.End.has_value())
+return std::to_string((stats.End.value() - stats.Start).count()) +
+   "(nanosec)";
+  return "";
+}
+
+std::string LldbBaseTelemetryInfo::ToString() const {
+  return ("[LldbBaseTelemetryInfo]\n") + (" SessionId: " + SessionId + "\n");
+}
+
+std::string DebuggerTelemetryInfo::ToString() const {
+  std::string duration_desc =
+  (ExitDesc.has_value() ? "  lldb session duration: "
+: "  lldb startup duration: ") +
+  std::to_string((Stats.End.value() - Stats.Start).count()) + 
"(nanosec)\n";
+
+  return LldbBaseTelemetryInfo::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";
+}
+
+static size_t ToNanosecOrZero(const std::optional &Point) {
+  if (!Point.has_value())
+return 0;
+
+  return Point.value().time_since_epoch().count();
+}
+
+llvm::json::Object DebuggerTelemetryInfo::serializeToJson() const {
+  return llvm::json::Object{
+  {"DebuggerInfo",
+   {
+   {"SessionId", SessionId},
+   {"username", username},
+   {"lldb_git_sha", lldb_git_sha},
+   {"lldb_path", lldb_path},
+   {"cwd", cwd},
+   {
+   "EventStats",
+   {
+   {"Start", Stats.Start.time_since_epoch().count()},
+   {"End", ToNanosecOrZero(Stats.End)},
+   },
+   },
+   // TODO: fill in more?
+   }}};
+}
+
+std::string ClientTelemetryInfo::ToString() const {
+  return LldbBaseTelemetryInfo::ToString() + "\n" +
+ ("[DapRequestInfoEntry]\n") +

labath wrote:

I don't think we should assume this request comes from lldb-dap.

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] [lldb]Implement LLDB Telemetry (PR #98528)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -0,0 +1,273 @@
+
+//===-- 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 "lldb/API/SBDebugger.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/TelemetryVendor.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/LLDBLog.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/JSON.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+namespace lldb_private {
+
+using ::llvm::telemetry::Destination;
+using ::llvm::telemetry::EventStats;
+using ::llvm::telemetry::ExitDescription;
+using ::llvm::telemetry::SteadyTimePoint;
+using ::llvm::telemetry::TelemetryInfo;
+
+static std::string
+ExitDescToString(const llvm::telemetry::ExitDescription *desc) {
+  return ("ExitCode:" + desc->ExitCode) +
+ (" ExixitDescription: " + desc->Description);

labath wrote:

```suggestion
 (" ExitDescription: " + desc->Description);
```

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] [lldb]Implement LLDB Telemetry (PR #98528)

2024-09-25 Thread Pavel Labath via lldb-commits


@@ -257,8 +257,8 @@ enum StopReason {
 };
 
 /// Command Return Status Types.
-enum ReturnStatus {
-  eReturnStatusInvalid,
+enum ReturnStatus : int {

labath wrote:

How does this relate to the rest of the patch?

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] [lldb]Implement LLDB Telemetry (PR #98528)

2024-09-25 Thread Pavel Labath via lldb-commits

https://github.com/labath commented:

I think it'd be good to separate the general mechanics of creating and managing 
a telemetry (telemeter) instance from the details of logging individual 
telemetry entries. I.e. one PR for the general infrastructure (maybe include 
one simple (startup?) telemetry entry, if you think it's necessary to make a 
point), and then one PR for each of the entries.

The plugin infrastructure seems very.. baroque. Why do we need the intermediate 
TelemetryVendor class? And why a whole list of them?

The reason we support registering more than one e.g. SymbolFile plugin is that 
we can choose which one to use at runtime (based on which file the user opens, 
etc.). Do we want to do something like that here? What would that even mean?

To create a basic indirection (so that the vendor implementation can be 
decoupled from the rest of lldb), it sounds to me like all one need is one 
function pointer (the factory function for the "telemeter"), a way to set it 
from inside the plugin (`PluginManager::SetTelemetryPlugin` ?) and then a way 
to call it (another function?)

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][Minidump] Have Minidumps save off and properly read TLS data (PR #109477)

2024-09-25 Thread Jacob Lalonde via lldb-commits

Jlalond wrote:

> I think that would make sense, though I also think that the current behavior 
> (attempting to relocate it also does) -- my core question is: how likely is 
> it that the dynamic loader will find the right thread-local for a module 
> which was loaded by the process class, if the two classes can't even agree on 
> its load address?

I'm mentally struggling with this as there is a distinction I think between 
loading in a core file and actually loading of a module. ProcessMinidump 
handles loading modules defined in the module list, but the DYLD will need to 
relocate them. I want to support this workflow but not force it for all Cores 
(Which I am doing). I wonder if we can force have the process force relocation 
which is what I'm doing with the `IsCoreFile()` check in DidAttach

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


[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-25 Thread Jonas Devlieghere via lldb-commits

https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/108495

>From c2d2758c68c661153bae0cda857b4c0cb7eddafe Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Wed, 25 Sep 2024 11:04:56 -0700
Subject: [PATCH] [lldb] Introduce an always-on system log category/channel

Add an "always on" log category and channel. Unlike other, existing log
channels, it is not exposed to users. The channel is meant to be used
sparsely and deliberately for logging high-value information to the
system log.

We have a similar concept in the downstream Swift fork and this has
proven to be extremely valuable. This is especially true on macOS where
system log messages are automatically captured as part of a sysdiagnose.

This patch revives https://reviews.llvm.org/D135621 although the purpose
is slightly different (logging to the system log rather than to a ring
buffer dumped as part of the diagnostics). This patch addresses all the
remaining oustanding comments.
---
 lldb/include/lldb/Host/Host.h | 19 +++
 lldb/include/lldb/Utility/Log.h   | 11 ++-
 lldb/source/API/SystemInitializerFull.cpp |  5 +
 lldb/source/Host/common/Host.cpp  | 16 
 lldb/source/Host/common/HostInfoBase.cpp  |  2 ++
 lldb/test/Shell/Host/TestSytemLogChannel.test |  3 +++
 6 files changed, 51 insertions(+), 5 deletions(-)
 create mode 100644 lldb/test/Shell/Host/TestSytemLogChannel.test

diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 9d0994978402f7..d8113a5fceeada 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -31,6 +31,25 @@ class ProcessInstanceInfo;
 class ProcessInstanceInfoMatch;
 typedef std::vector ProcessInstanceInfoList;
 
+// System log category and channel. This log channel is always enabled and
+// therefore is supposed to be used sparsely. Use this log channel to log
+// critical information that is expected to be relevant to the majority of bug
+// reports.
+enum class SystemLog : Log::MaskType {
+  System = Log::ChannelFlag<0>,
+  LLVM_MARK_AS_BITMASK_ENUM(System)
+};
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+class LogChannelSystem {
+public:
+  static void Initialize();
+  static void Terminate();
+};
+
+template <> Log::Channel &LogChannelFor();
+
 // Exit Type for inferior processes
 struct WaitStatus {
   enum Type : uint8_t {
diff --git a/lldb/include/lldb/Utility/Log.h b/lldb/include/lldb/Utility/Log.h
index 27707c17f9b824..d709e8c4f04c6e 100644
--- a/lldb/include/lldb/Utility/Log.h
+++ b/lldb/include/lldb/Utility/Log.h
@@ -272,6 +272,12 @@ class Log final {
   void VAFormatf(llvm::StringRef file, llvm::StringRef function,
  const char *format, va_list args);
 
+  void Enable(const std::shared_ptr &handler_sp,
+  uint32_t options = 0,
+  MaskType flags = std::numeric_limits::max());
+
+  void Disable(MaskType flags = 0);
+
 private:
   Channel &m_channel;
 
@@ -297,11 +303,6 @@ class Log final {
 return m_handler;
   }
 
-  void Enable(const std::shared_ptr &handler_sp, uint32_t options,
-  MaskType flags);
-
-  void Disable(MaskType flags);
-
   bool Dump(llvm::raw_ostream &stream);
 
   typedef llvm::StringMap ChannelMap;
diff --git a/lldb/source/API/SystemInitializerFull.cpp 
b/lldb/source/API/SystemInitializerFull.cpp
index 995d14f7c1fa1e..1958db7291fb64 100644
--- a/lldb/source/API/SystemInitializerFull.cpp
+++ b/lldb/source/API/SystemInitializerFull.cpp
@@ -20,6 +20,8 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/TargetSelect.h"
 
+#include "lldb/Version/Version.h"
+
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wglobal-constructors"
 #include "llvm/ExecutionEngine/MCJIT.h"
@@ -83,6 +85,9 @@ llvm::Error SystemInitializerFull::Initialize() {
   // Use the Debugger's LLDBAssert callback.
   SetLLDBAssertCallback(Debugger::AssertCallback);
 
+  // Log the LLDB version to the system log.
+  LLDB_LOG(GetLog(SystemLog::System), "{0}", GetVersion());
+
   return llvm::Error::success();
 }
 
diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index f08adea6546ae1..abca6068d3604a 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -125,6 +125,22 @@ void Host::SystemLog(Severity severity, llvm::StringRef 
message) {
 #endif
 #endif
 
+static constexpr Log::Category g_categories[] = {
+{{"system"}, {"system log"}, SystemLog::System}};
+
+static Log::Channel g_system_channel(g_categories, SystemLog::System);
+static Log g_system_log(g_system_channel);
+
+template <> Log::Channel &lldb_private::LogChannelFor() {
+  return g_system_channel;
+}
+
+void LogChannelSystem::Initialize() {
+  g_system_log.Enable(std::make_shared());
+}
+
+void LogChannelSystem::Terminate() { g_system_log.Disable(); }
+
 #if !defined(__APPLE__) && !defined(_WIN32)
 static thread_result_t
 MonitorChildProcessTh

[Lldb-commits] [lldb] [LLDB][Minidump] Have Minidumps save off and properly read TLS data (PR #109477)

2024-09-25 Thread Pavel Labath via lldb-commits

labath wrote:

The way I see it, there should be no distinction between "loading in a core 
file" and "actually loading" a module. "Loading of a module", in the sense that 
I'm using it here, consists of two steps:
- adding the module to the target's list of modules (usually via 
target.GetOrCreateModule)
- setting the load address of the module (loading in the narrow sense) -- done 
by calling module.SetLoadAddress

Both of these steps are necessary for a module to work properly, but it 
shouldn't matter who's doing the loading (except that probably thread local 
sections will not work for a module which was not loaded by the dynamic loader).

I think there's nothing wrong with giving the dynamic loader a chance to load 
any additional modules (for all processes even, not just core files). Normally, 
it should compute the same address (if any) that was produced by the process 
class, but if it happens to compute a different one, so be it (it probably 
means one of them has a bug).

Your mention of a relocation makes it sound like this is already happening 
though (process computing one load address and then the DYLD "relocating" it). 
Is that true? If so, then I think we ought to figure out what's causing the 
difference (and which one is correct).

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


[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

2024-09-25 Thread Dmitry Vasilyev via lldb-commits


@@ -393,125 +362,222 @@ int main_platform(int argc, char *argv[]) {
   lldb_private::Args inferior_arguments;
   inferior_arguments.SetArguments(argc, const_cast(argv));
 
+  Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain;
+
   if (fd != SharedSocket::kInvalidFD) {
 // Child process will handle the connection and exit.
+if (gdbserver_port)
+  protocol = Socket::ProtocolTcp;
+
 Log *log = GetLog(LLDBLog::Platform);
-if (!listen_host_port.empty()) {
-  LLDB_LOGF(log, "lldb-platform child: "
- "ambiguous parameters --listen and --child-platform-fd");
-  return socket_error;
-}
 
-NativeSocket socket;
-error = SharedSocket::GetNativeSocket(fd, socket);
+NativeSocket sockfd;
+error = SharedSocket::GetNativeSocket(fd, sockfd);
 if (error.Fail()) {
   LLDB_LOGF(log, "lldb-platform child: %s", error.AsCString());
   return socket_error;
 }
 
-GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, "tcp");
-if (port_offset > 0)
-  platform.SetPortOffset(port_offset);
-platform.SetPortMap(std::move(gdbserver_portmap));
+GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port);
+Socket *socket;
+if (protocol == Socket::ProtocolTcp)
+  socket = new TCPSocket(sockfd, /*should_close=*/true,
+ /*child_processes_inherit=*/false);
+else {
+#if LLDB_ENABLE_POSIX
+  socket = new DomainSocket(sockfd, /*should_close=*/true,
+/*child_processes_inherit=*/false);
+#else
+  LLDB_LOGF(log,
+"lldb-platform child: Unix domain sockets are not supported on 
"
+"this platform.");
+  return socket_error;
+#endif
+}
 platform.SetConnection(
-std::unique_ptr(new ConnectionFileDescriptor(
-new TCPSocket(socket, /*should_close=*/true,
-  /*child_processes_inherit=*/false;
+std::unique_ptr(new ConnectionFileDescriptor(socket)));
 client_handle(platform, inferior_arguments);
 return 0;
   }
 
-  const bool children_inherit_listen_socket = false;
-  // the test suite makes many connections in parallel, let's not miss any.
-  // The highest this should get reasonably is a function of the number
-  // of target CPUs. For now, let's just use 100.
-  const int backlog = 100;
+  if (gdbserver_port != 0 &&
+  (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) {
+WithColor::error() << llvm::formatv("Port number {0} is not in the "
+"valid user port range of {1} - {2}\n",
+gdbserver_port, LOW_PORT, HIGH_PORT);
+return 1;
+  }
 
-  std::unique_ptr acceptor_up(Acceptor::Create(
-  listen_host_port, children_inherit_listen_socket, error));
-  if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+  std::string hostname;
+  uint16_t platform_port = 0;
+
+  // Try to match socket name as URL - e.g., tcp://localhost:
+  if (std::optional uri = URI::Parse(listen_host_port)) {
+if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) {
+  fprintf(stderr, "Unknown protocol scheme \"%s\".",
+  uri->scheme.str().c_str());
+  return socket_error;
+}
+if (protocol == Socket::ProtocolTcp) {
+  hostname = uri->hostname;
+  if (uri->port) {
+platform_port = *(uri->port);
+  }
+} else
+  hostname = listen_host_port.substr(uri->scheme.size() + strlen("://"));
+  } else {
+// Try to match socket name as $host:port - e.g., localhost:
+llvm::Expected host_port =
+Socket::DecodeHostAndPort(listen_host_port);
+if (!llvm::errorToBool(host_port.takeError())) {
+  protocol = Socket::ProtocolTcp;
+  hostname = host_port->hostname;
+  platform_port = host_port->port;
+} else
+  hostname = listen_host_port;
   }
 
-  error = acceptor_up->Listen(backlog);
+  if (protocol == Socket::ProtocolTcp) {
+if (platform_port != 0 && platform_port == gdbserver_port) {
+  fprintf(stderr, "The same platform and gdb ports %u.", platform_port);
+  return socket_error;
+}
+  } else {
+if (gdbserver_port) {
+  fprintf(stderr,
+  "--gdbserver-port %u is redundant for non-tcp protocol %s.",
+  gdbserver_port, Socket::FindSchemeByProtocol(protocol));
+  return socket_error;
+}
+if (g_server) {
+  fprintf(stderr,
+  "Ambiguous parameters --server --listen %s.\n"
+  "The protocol must be tcp for the server mode.",
+  listen_host_port.c_str());
+  return socket_error;
+}
+  }
+
+  std::unique_ptr sock_platform =
+  Socket::Create(protocol, /*child_processes_inherit=*/false, error);
+  if (error.Fail()) {
+printf("Failed to create platform socket: %s\n", err