date:20230705

[Lldb-commits] [PATCH] D154505: [lldb][NFC] Remove code duplication in InitOSO

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve created this revision.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Two identical loops were iterating over different ranges, leading to code
duplication. We replace this by a loop over the concatenation of the ranges.

We also use early returns to avoid deeply nested code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154505

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -32,6 +32,7 @@
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Symbol/VariableList.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "lldb/Target/StackFrame.h"
@@ -286,112 +287,105 @@
   // we return the abilities of the first N_OSO's DWARF.
 
   Symtab *symtab = m_objfile_sp->GetSymtab();
-  if (symtab) {
-Log *log = GetLog(DWARFLog::DebugMap);
-
-std::vector oso_indexes;
-// When a mach-o symbol is encoded, the n_type field is encoded in bits
-// 23:16, and the n_desc field is encoded in bits 15:0.
-//
-// To find all N_OSO entries that are part of the DWARF + debug map we find
-// only object file symbols with the flags value as follows: bits 23:16 ==
-// 0x66 (N_OSO) bits 15: 0 == 0x0001 (specifies this is a debug map object
-// file)
-const uint32_t k_oso_symbol_flags_value = 0x660001u;
-
-const uint32_t oso_index_count =
-symtab->AppendSymbolIndexesWithTypeAndFlagsValue(
-eSymbolTypeObjectFile, k_oso_symbol_flags_value, oso_indexes);
-
-if (oso_index_count > 0) {
-  symtab->AppendSymbolIndexesWithType(eSymbolTypeCode, Symtab::eDebugYes,
-  Symtab::eVisibilityAny,
-  m_func_indexes);
-  symtab->AppendSymbolIndexesWithType(eSymbolTypeData, Symtab::eDebugYes,
-  Symtab::eVisibilityAny,
-  m_glob_indexes);
-
-  symtab->SortSymbolIndexesByValue(m_func_indexes, true);
-  symtab->SortSymbolIndexesByValue(m_glob_indexes, true);
-
-  for (uint32_t sym_idx : m_func_indexes) {
-const Symbol *symbol = symtab->SymbolAtIndex(sym_idx);
-lldb::addr_t file_addr = symbol->GetAddressRef().GetFileAddress();
-lldb::addr_t byte_size = symbol->GetByteSize();
-DebugMap::Entry debug_map_entry(
-file_addr, byte_size, OSOEntry(sym_idx, LLDB_INVALID_ADDRESS));
-m_debug_map.Append(debug_map_entry);
-  }
-  for (uint32_t sym_idx : m_glob_indexes) {
-const Symbol *symbol = symtab->SymbolAtIndex(sym_idx);
-lldb::addr_t file_addr = symbol->GetAddressRef().GetFileAddress();
-lldb::addr_t byte_size = symbol->GetByteSize();
-DebugMap::Entry debug_map_entry(
-file_addr, byte_size, OSOEntry(sym_idx, LLDB_INVALID_ADDRESS));
-m_debug_map.Append(debug_map_entry);
-  }
-  m_debug_map.Sort();
-
-  m_compile_unit_infos.resize(oso_index_count);
-
-  for (uint32_t i = 0; i < oso_index_count; ++i) {
-const uint32_t so_idx = oso_indexes[i] - 1;
-const uint32_t oso_idx = oso_indexes[i];
-const Symbol *so_symbol = symtab->SymbolAtIndex(so_idx);
-const Symbol *oso_symbol = symtab->SymbolAtIndex(oso_idx);
-if (so_symbol && oso_symbol &&
-so_symbol->GetType() == eSymbolTypeSourceFile &&
-oso_symbol->GetType() == eSymbolTypeObjectFile) {
-  m_compile_unit_infos[i].so_file.SetFile(
-  so_symbol->GetName().AsCString(), FileSpec::Style::native);
-  m_compile_unit_infos[i].oso_path = oso_symbol->GetName();
-  m_compile_unit_infos[i].oso_mod_time =
-  llvm::sys::toTimePoint(oso_symbol->GetIntegerValue(0));
-  uint32_t sibling_idx = so_symbol->GetSiblingIndex();
-  // The sibling index can't be less that or equal to the current index
-  // "i"
-  if (sibling_idx == UINT32_MAX) {
-m_objfile_sp->GetModule()->ReportError(
-"N_SO in symbol with UID {0} has invalid sibling in debug "
-"map, "
-"please file a bug and attach the binary listed in this error",
-so_symbol->GetID());
-  } else {
-const Symbol *last_symbol = symtab->SymbolAtIndex(sibling_idx - 1);
-m_compile_unit_infos[i].first_symbol_index = so_idx;
-m_compile_unit_infos[i].last_symbol_index = sibling_idx - 1;
-m_compile_unit_infos[i].first_symbol_id = so_symbol->GetID();
-m

[Lldb-commits] [PATCH] D154513: [lldb][NFC] Factor out code from SymbolFileDWARF::ParseVariableDIE

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve created this revision.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This function does a _lot_ of different things:

1. Parses a DIE,
2. Builds an ExpressionList
3. Figures out lifetime of variable
4. Remaps addresses for debug maps
5. Handles external variables
6. Figures out scope of variables

A lot of this functionality is coded in a complex nest of conditions, variables
that are declared and then initialized much later, variables that are updated in
multiple code paths. All of this makes the code really hard to follow.

This commit attempts to improve the state of things by factoring out (3), adding
documentation on how the expression list is built, and by reducing the scope of
variables.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154513

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3269,6 +3269,60 @@
   return var_sp;
 }
 
+/// Creates a DWARFExpressionList from an DW_AT_location form_value.
+static DWARFExpressionList GetExprListFromAtLocation(DWARFFormValue form_value,
+ ModuleSP module,
+ const DWARFDIE &die,
+ const addr_t func_low_pc) {
+  if (DWARFFormValue::IsBlockForm(form_value.Form())) {
+const DWARFDataExtractor &data = die.GetData();
+
+uint32_t block_offset = form_value.BlockData() - data.GetDataStart();
+uint32_t block_length = form_value.Unsigned();
+return DWARFExpressionList(
+module, DataExtractor(data, block_offset, block_length), die.GetCU());
+  }
+
+  DWARFExpressionList location_list(module, DWARFExpression(), die.GetCU());
+  DataExtractor data = die.GetCU()->GetLocationData();
+  dw_offset_t offset = form_value.Unsigned();
+  if (form_value.Form() == DW_FORM_loclistx)
+offset = die.GetCU()->GetLoclistOffset(offset).value_or(-1);
+  if (data.ValidOffset(offset)) {
+data = DataExtractor(data, offset, data.GetByteSize() - offset);
+const DWARFUnit *dwarf_cu = form_value.GetUnit();
+if (DWARFExpression::ParseDWARFLocationList(dwarf_cu, data, &location_list))
+  location_list.SetFuncFileAddress(func_low_pc);
+  }
+
+  return location_list;
+}
+
+/// Creates a DWARFExpressionList from an DW_AT_const_value. This is either a
+/// block form, or a string, or a data form. For data forms, this returns an
+/// empty list, as we cannot initialize it properly without a SymbolFileType.
+static DWARFExpressionList
+GetExprListFromAtConstValue(DWARFFormValue form_value, ModuleSP module,
+const DWARFDIE &die) {
+  const DWARFDataExtractor &debug_info_data = die.GetData();
+  if (DWARFFormValue::IsBlockForm(form_value.Form())) {
+// Retrieve the value as a block expression.
+uint32_t block_offset =
+form_value.BlockData() - debug_info_data.GetDataStart();
+uint32_t block_length = form_value.Unsigned();
+return DWARFExpressionList(
+module, DataExtractor(debug_info_data, block_offset, block_length),
+die.GetCU());
+  }
+  if (const char *str = form_value.AsCString())
+return DWARFExpressionList(module,
+   DataExtractor(str, strlen(str) + 1,
+ die.GetCU()->GetByteOrder(),
+ die.GetCU()->GetAddressByteSize()),
+   die.GetCU());
+  return DWARFExpressionList(module, DWARFExpression(), die.GetCU());
+}
+
 VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
  const DWARFDIE &die,
  const lldb::addr_t func_low_pc) {
@@ -3290,7 +3344,6 @@
   const char *mangled = nullptr;
   Declaration decl;
   DWARFFormValue type_die_form;
-  DWARFExpressionList location_list(module, DWARFExpression(), die.GetCU());
   bool is_external = false;
   bool is_artificial = false;
   DWARFFormValue const_value_form, location_form;
@@ -3355,57 +3408,16 @@
   // for static constexpr member variables -- DW_AT_const_value will be
   // present in the class declaration and DW_AT_location in the DIE defining
   // the member.
-  bool location_is_const_value_data = false;
-  bool has_explicit_location = location_form.IsValid();
-  bool use_type_size_for_value = false;
-  if (location_form.IsValid()) {
-if (DWARFFormValue::IsBlockForm(location_form.Form())) {
-  const DWARFDataExtractor &data = die.GetData();
-
-  uint32_t block_offset = location_form.BlockData() - data.GetDataStart();
-  uint32_t

[Lldb-commits] [PATCH] D154513: [lldb][NFC] Factor out code from SymbolFileDWARF::ParseVariableDIE

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3272
 
+/// Creates a DWARFExpressionList from an DW_AT_location form_value.
+static DWARFExpressionList GetExprListFromAtLocation(DWARFFormValue form_value,

FWIW all this code was just moved outside the main function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154513/new/

https://reviews.llvm.org/D154513

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

[Lldb-commits] [PATCH] D154505: [lldb][NFC] Remove code duplication in InitOSO

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

I'm a big fan of refactoring deeply-nested code to be flatter, so I'm happy to 
see this work being done! I wasn't aware of `llvm::concat` before, I probably 
would have used a lambda or something to do this, so it was interesting 
learning about that. 😄




Comment at: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:347-348
+  uint32_t sibling_idx = so_symbol->GetSiblingIndex();
+  // The sibling index can't be less that or equal to the current index
+  // "i"
+  if (sibling_idx == UINT32_MAX) {

Besides the typo, is this comment correct? We're not verifying the claim in the 
comment AFAICT.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154505/new/

https://reviews.llvm.org/D154505

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

[Lldb-commits] [PATCH] D154029: [lldb-vscode] Adding support for column break points.

2023-07-05 Thread walter erquinigo via Phabricator via lldb-commits

wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.

Nice!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154029/new/

https://reviews.llvm.org/D154029

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

[Lldb-commits] [lldb] 1c7c997 - [lldb] Deprecate SBHostOS threading functionality

2023-07-05 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-07-05T08:46:48-07:00
New Revision: 1c7c9970379e1949a0b338eba2746dbf84b0bda4

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

LOG: [lldb] Deprecate SBHostOS threading functionality

For some context, Raphael tried to this before: https://reviews.llvm.org/D104231

These methods are not tested at all, and in some cases, are not even fully
implemented (e.g. SBHostOS::ThreadCreated). I'm not convinced it's
possible to use these correctly from Python, and I'm not aware of any
users of these methods. It's difficult to remove these methods
wholesale, but we can start with deprecating them.

A possible follow-up to this change (which may require an RFC to get
more buy in from the community) is to gut these functions entirely. That
is, remove the implementations and replace them either with nothing or
have them dump out a message to stderr saying not to use these.

Differential Revision: https://reviews.llvm.org/D153900

Added: 


Modified: 
lldb/include/lldb/API/SBHostOS.h
lldb/tools/driver/Driver.cpp

Removed: 




diff  --git a/lldb/include/lldb/API/SBHostOS.h 
b/lldb/include/lldb/API/SBHostOS.h
index b170f8d6581038..ad57a9ec409f61 100644
--- a/lldb/include/lldb/API/SBHostOS.h
+++ b/lldb/include/lldb/API/SBHostOS.h
@@ -24,15 +24,26 @@ class LLDB_API SBHostOS {
 
   static lldb::SBFileSpec GetUserHomeDirectory();
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is 
diff icult to use from Python.")
   static void ThreadCreated(const char *name);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is 
diff icult to use from Python.")
   static lldb::thread_t ThreadCreate(const char *name,
  lldb::thread_func_t thread_function,
  void *thread_arg, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is 
diff icult to use from Python.")
   static bool ThreadCancel(lldb::thread_t thread, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is 
diff icult to use from Python.")
   static bool ThreadDetach(lldb::thread_t thread, lldb::SBError *err);
+
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is 
diff icult to use from Python.")
   static bool ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
  lldb::SBError *err);
 

diff  --git a/lldb/tools/driver/Driver.cpp b/lldb/tools/driver/Driver.cpp
index d463267aeef353..b14d15602aba31 100644
--- a/lldb/tools/driver/Driver.cpp
+++ b/lldb/tools/driver/Driver.cpp
@@ -795,8 +795,6 @@ int main(int argc, char const *argv[]) {
   // Setup LLDB signal handlers once the debugger has been initialized.
   SBDebugger::PrintDiagnosticsOnError();
 
-  SBHostOS::ThreadCreated("");
-
   signal(SIGINT, sigint_handler);
 #if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);



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

[Lldb-commits] [PATCH] D153900: [lldb] Deprecate SBHostOS threading functionality

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c7c9970379e: [lldb] Deprecate SBHostOS threading 
functionality (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153900/new/

https://reviews.llvm.org/D153900

Files:
  lldb/include/lldb/API/SBHostOS.h
  lldb/tools/driver/Driver.cpp


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -795,8 +795,6 @@
   // Setup LLDB signal handlers once the debugger has been initialized.
   SBDebugger::PrintDiagnosticsOnError();
 
-  SBHostOS::ThreadCreated("");
-
   signal(SIGINT, sigint_handler);
 #if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
Index: lldb/include/lldb/API/SBHostOS.h
===
--- lldb/include/lldb/API/SBHostOS.h
+++ lldb/include/lldb/API/SBHostOS.h
@@ -24,15 +24,26 @@
 
   static lldb::SBFileSpec GetUserHomeDirectory();
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static void ThreadCreated(const char *name);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static lldb::thread_t ThreadCreate(const char *name,
  lldb::thread_func_t thread_function,
  void *thread_arg, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadCancel(lldb::thread_t thread, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadDetach(lldb::thread_t thread, lldb::SBError *err);
+
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
  lldb::SBError *err);
 


Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -795,8 +795,6 @@
   // Setup LLDB signal handlers once the debugger has been initialized.
   SBDebugger::PrintDiagnosticsOnError();
 
-  SBHostOS::ThreadCreated("");
-
   signal(SIGINT, sigint_handler);
 #if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
Index: lldb/include/lldb/API/SBHostOS.h
===
--- lldb/include/lldb/API/SBHostOS.h
+++ lldb/include/lldb/API/SBHostOS.h
@@ -24,15 +24,26 @@
 
   static lldb::SBFileSpec GetUserHomeDirectory();
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static void ThreadCreated(const char *name);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static lldb::thread_t ThreadCreate(const char *name,
  lldb::thread_func_t thread_function,
  void *thread_arg, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadCancel(lldb::thread_t thread, lldb::SBError *err);
 
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadDetach(lldb::thread_t thread, lldb::SBError *err);
+
+  LLDB_DEPRECATED("Threading functionality in SBHostOS is not well supported, "
+  "not portable, and is difficult to use from Python.")
   static bool ThreadJoin(lldb::thread_t thread, lldb::thread_result_t *result,
  lldb::SBError *err);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104231: [lldb] Deprecate the threading functionality in SBHostOS

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord commandeered this revision.
bulbazord added a reviewer: teemperor.
bulbazord added a comment.
Herald added a project: All.

Commandeering this change in order to close it. I've effectively done this 
already with D153900 .


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104231/new/

https://reviews.llvm.org/D104231

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

[Lldb-commits] [PATCH] D153900: [lldb] Deprecate SBHostOS threading functionality

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

In D153900#4469649 , @mib wrote:

> LGTM! @bulbazord when you land this, make sure to close @teemperor's diff 
> (D104231 ). Thanks!

👍


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153900/new/

https://reviews.llvm.org/D153900

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

[Lldb-commits] [lldb] 20f9927 - [lldb][NFCI] Deprecate SBValue::GetOpaqueType

2023-07-05 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-07-05T08:52:32-07:00
New Revision: 20f99278dbb81e880c23c65688e9752ce7ad019a

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

LOG: [lldb][NFCI] Deprecate SBValue::GetOpaqueType

This method, as far as I can ascertain, is non-trivial to actually use
to work with (if not impossible). It doesn't make sense to use from
Python and you do not have access to the accompanying TypeSystem, so it
doesn't really do anything useful.

A possible follow-up is to gut the implementation and have it return `nullptr`.

Differential Revision: https://reviews.llvm.org/D153918

Added: 


Modified: 
lldb/include/lldb/API/SBValue.h

Removed: 




diff  --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index e288dbf4c298ae..b66c2d5642b6f9 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -283,6 +283,7 @@ class LLDB_API SBValue {
 
   uint32_t GetNumChildren(uint32_t max);
 
+  LLDB_DEPRECATED("SBValue::GetOpaqueType() is deprecated.")
   void *GetOpaqueType();
 
   lldb::SBTarget GetTarget();



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

[Lldb-commits] [PATCH] D153918: [lldb][NFCI] Deprecate SBValue::GetOpaqueType

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20f99278dbb8: [lldb][NFCI] Deprecate SBValue::GetOpaqueType 
(authored by bulbazord).
Herald added a subscriber: wangpc.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153918/new/

https://reviews.llvm.org/D153918

Files:
  lldb/include/lldb/API/SBValue.h


Index: lldb/include/lldb/API/SBValue.h
===
--- lldb/include/lldb/API/SBValue.h
+++ lldb/include/lldb/API/SBValue.h
@@ -283,6 +283,7 @@
 
   uint32_t GetNumChildren(uint32_t max);
 
+  LLDB_DEPRECATED("SBValue::GetOpaqueType() is deprecated.")
   void *GetOpaqueType();
 
   lldb::SBTarget GetTarget();


Index: lldb/include/lldb/API/SBValue.h
===
--- lldb/include/lldb/API/SBValue.h
+++ lldb/include/lldb/API/SBValue.h
@@ -283,6 +283,7 @@
 
   uint32_t GetNumChildren(uint32_t max);
 
+  LLDB_DEPRECATED("SBValue::GetOpaqueType() is deprecated.")
   void *GetOpaqueType();
 
   lldb::SBTarget GetTarget();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] fd5748c - [lldb][NFCI] Minor cleanup of default OptionValue::GetSubValue implementation

2023-07-05 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-07-05T09:00:59-07:00
New Revision: fd5748cb5a45c4178b84eb329bea9055f8ee485d

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

LOG: [lldb][NFCI] Minor cleanup of default OptionValue::GetSubValue 
implementation

This does 2 things:
- Corrects a minor typo (`value subvalue` -> `valid subvalue`)
- Removes an unnecessary instance of `str().c_str()` (creating a
  temporary std::string from a StringRef just to get a valid
  null-terminated string).

Differential Revision: https://reviews.llvm.org/D154387

Added: 


Modified: 
lldb/include/lldb/Interpreter/OptionValue.h

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValue.h 
b/lldb/include/lldb/Interpreter/OptionValue.h
index b43715357a01c3..4fa0b23042669d 100644
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -114,8 +114,7 @@ class OptionValue {
   virtual lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
   llvm::StringRef name,
   Status &error) const {
-error.SetErrorStringWithFormat("'%s' is not a value subvalue",
-   name.str().c_str());
+error.SetErrorStringWithFormatv("'{0}' is not a valid subvalue", name);
 return lldb::OptionValueSP();
   }
 



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

[Lldb-commits] [PATCH] D154387: [lldb][NFCI] Minor cleanup of default OptionValue::GetSubValue implementation

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd5748cb5a45: [lldb][NFCI] Minor cleanup of default 
OptionValue::GetSubValue implementation (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154387/new/

https://reviews.llvm.org/D154387

Files:
  lldb/include/lldb/Interpreter/OptionValue.h


Index: lldb/include/lldb/Interpreter/OptionValue.h
===
--- lldb/include/lldb/Interpreter/OptionValue.h
+++ lldb/include/lldb/Interpreter/OptionValue.h
@@ -114,8 +114,7 @@
   virtual lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
   llvm::StringRef name,
   Status &error) const {
-error.SetErrorStringWithFormat("'%s' is not a value subvalue",
-   name.str().c_str());
+error.SetErrorStringWithFormatv("'{0}' is not a valid subvalue", name);
 return lldb::OptionValueSP();
   }
 


Index: lldb/include/lldb/Interpreter/OptionValue.h
===
--- lldb/include/lldb/Interpreter/OptionValue.h
+++ lldb/include/lldb/Interpreter/OptionValue.h
@@ -114,8 +114,7 @@
   virtual lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
   llvm::StringRef name,
   Status &error) const {
-error.SetErrorStringWithFormat("'%s' is not a value subvalue",
-   name.str().c_str());
+error.SetErrorStringWithFormatv("'{0}' is not a valid subvalue", name);
 return lldb::OptionValueSP();
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154505: [lldb][NFC] Remove code duplication in InitOSO

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

In D154505#4473986 , @bulbazord wrote:

> I'm a big fan of refactoring deeply-nested code to be flatter, so I'm happy 
> to see this work being done! I wasn't aware of `llvm::concat` before, I 
> probably would have used a lambda or something to do this, so it was 
> interesting learning about that. 😄

+1 on both accounts. LGTM modulo the comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154505/new/

https://reviews.llvm.org/D154505

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

[Lldb-commits] [PATCH] D153840: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate.

2023-07-05 Thread Caroline Tice via Phabricator via lldb-commits

cmtice marked an inline comment as done.
cmtice added a comment.

Hi Jason,

I had been talking more with David, and yes, I had come to the conclusion that 
you are both right and that this was not the right fix.  I am planning on 
reverting this, but I am trying to figure out the right fix to replace it with. 
 I can't share the source that was causing the bug to manifest, because it's in 
proprietary code, but David is looking at it and I believe he has come to the 
conclusion that there is a bug in the DWARF code generation -- we were getting 
a size of 16, which is absolutely not right.  The question is, in the case of 
bad DWARF being generated, what (if anything) should the LLDB code here be 
doing? Should we check the size as soon as we read it in, and assert that  it 
must be <= 8?  Or something else?  Or just leave the LLDB code entirely alone?

What do you (and other reviewers) think is the right thing to do here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153840/new/

https://reviews.llvm.org/D153840

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

[Lldb-commits] [PATCH] D154505: [lldb][NFC] Remove code duplication in InitOSO

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve added inline comments.



Comment at: 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:347-348
+  uint32_t sibling_idx = so_symbol->GetSiblingIndex();
+  // The sibling index can't be less that or equal to the current index
+  // "i"
+  if (sibling_idx == UINT32_MAX) {

bulbazord wrote:
> Besides the typo, is this comment correct? We're not verifying the claim in 
> the comment AFAICT.
I think this is one of those situations where "if this is not true, the linker 
did not do its job".
That said, we should not crash, so I will change the condition to: `sibling_idx 
<= i || sibling_idx == UINT32_MAX`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154505/new/

https://reviews.llvm.org/D154505

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

[Lldb-commits] [PATCH] D154530: [lldb] Fix incorrect uses of LLDB_LOG_ERROR

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, fdeazeve.
Herald added a project: All.
JDevlieghere requested review of this revision.

Fix incorrect uses of LLDB_LOG_ERROR. The macro doesn't automatically inject 
the error in the log message: it merely passes the error as the first argument 
to `formatv` and therefore must be referenced with `{0}`.

Thanks to Nicholas Allegra for collecting a list of places where the macro was 
misused.

rdar://111581655


https://reviews.llvm.org/D154530

Files:
  lldb/source/Breakpoint/Watchpoint.cpp
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -56,7 +56,7 @@
 Thread &ThreadPlanTracer::GetThread() {
   if (m_thread)
 return *m_thread;
-
+
   ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
   m_thread = thread_sp.get();
   return *m_thread;
@@ -107,8 +107,9 @@
   auto type_system_or_err =
   target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
   if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(GetLog(LLDBLog::Types), std::move(err),
-   "Unable to get integer pointer type from TypeSystem");
+LLDB_LOG_ERROR(
+GetLog(LLDBLog::Types), std::move(err),
+"Unable to get integer pointer type from TypeSystem: {0}");
   } else {
 if (auto ts = *type_system_or_err)
   m_intptr_type = TypeFromUser(ts->GetBuiltinTypeForEncodingAndBitSize(
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2387,10 +2387,11 @@
 auto type_system_or_err =
 GetScratchTypeSystemForLanguage(language, create_on_demand);
 if (!type_system_or_err)
-  LLDB_LOG_ERROR(GetLog(LLDBLog::Target), type_system_or_err.takeError(),
- "Language '{}' has expression support but no scratch type "
- "system available",
- Language::GetNameForLanguageType(language));
+  LLDB_LOG_ERROR(
+  GetLog(LLDBLog::Target), type_system_or_err.takeError(),
+  "Language '{1}' has expression support but no scratch type "
+  "system available: {0}",
+  Language::GetNameForLanguageType(language));
 else
   if (auto ts = *type_system_or_err)
 scratch_type_systems.push_back(ts);
@@ -2409,7 +2410,7 @@
 
   if (auto err = type_system_or_err.takeError()) {
 LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
-   "Unable to get persistent expression state for language {}",
+   "Unable to get persistent expression state for language {1}",
Language::GetNameForLanguageType(language));
 return nullptr;
   }
@@ -2418,7 +2419,7 @@
 return ts->GetPersistentExpressionState();
 
   LLDB_LOG(GetLog(LLDBLog::Target),
-   "Unable to get persistent expression state for language {}",
+   "Unable to get persistent expression state for language {0}",
Language::GetNameForLanguageType(language));
   return nullptr;
 }
@@ -2617,7 +2618,7 @@
   auto ts = *type_system_or_err;
   if (!ts)
 LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
-   "Scratch type system is no longer live");
+   "Scratch type system is no longer live: {0}");
   else
 persistent_var_sp =
 ts->GetPersistentExpressionState()->GetVariable(expr);
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -1360,7 +1360,7 @@
 target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
 if (auto err = c_type_system_or_err.takeError()) {
   LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(err),
- "Unable to guess value for given address");
+ "Unable to guess value for given address: {0}");

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, fdeazeve, mib.
Herald added a project: All.
JDevlieghere requested review of this revision.

Fix incorrect uses of formatv specifiers in LLDB_LOG. Unlike Python, arguments 
must be numbered.


https://reviews.llvm.org/D154532

Files:
  lldb/source/API/SBTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,7 +3600,7 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
  llvm::toString(private_state_thread.takeError()));
 return false;
   }
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -182,7 +182,7 @@
 llvm::Expected maybe_thread =
 Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
 if (!maybe_thread)
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
llvm::toString(maybe_thread.takeError()));
 return true;
   }
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -177,7 +177,7 @@
 if (error_ptr)
   *error_ptr = Status(maybe_thread.takeError());
 else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
llvm::toString(maybe_thread.takeError()));
 }
   }
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1915,7 +1915,7 @@
 if (event_handler_thread) {
   m_event_handler_thread = *event_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
llvm::toString(event_handler_thread.takeError()));
 }
 
@@ -2056,7 +2056,7 @@
 if (io_handler_thread) {
   m_io_handler_thread = *io_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
llvm::toString(io_handler_thread.takeError()));
 }
   }
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1117,7 +1117,7 @@
 llvm::Expected> expected_vector =
 target_sp->GetBreakpointList().FindBreakpointsByName(name);
 if (!expected_vector) {
-  LLDB_LOG(GetLog(LLDBLog::Breakpoints), "invalid breakpoint name: {}",
+  LLDB_LOG(GetLog(LLDBLog::Breakpoints), "invalid breakpoint name: {0}",
llvm::toString(expected_vector.takeError()));
   return false;
 }
@@ -1591,7 +1591,7 @@
 
 const char *SBTarget::GetABIName() {
   LLDB_INSTRUMENT_VA(this);
-  
+
   TargetSP target_sp(GetSP());
   if (!target_sp)
 return nullptr;


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,7 +3600,7 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
  llvm::toString(private_state_thread.takeError()));
 return false;
   }
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -182,7 +182,7 @@
 llvm::Expected maybe_thread =
 Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
 if (!maybe_thread)
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
llvm::toString(maybe_thread.takeError()));
 return true;
   }
Index: lldb/source/Core/ThreadedCommunication.cpp

[Lldb-commits] [PATCH] D154413: [lldb] Fix crash when completing register names after program exit

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154413/new/

https://reviews.llvm.org/D154413

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

[Lldb-commits] [PATCH] D154530: [lldb] Fix incorrect uses of LLDB_LOG_ERROR

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

I was actually looking at this today independently, guess you beat me to the 
punch 😛 . LGTM modulo one comment.




Comment at: lldb/source/Target/Target.cpp:2413-2414
 LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
-   "Unable to get persistent expression state for language {}",
+   "Unable to get persistent expression state for language 
{1}",
Language::GetNameForLanguageType(language));
 return nullptr;

No use of `{0}` here, so we're dropping the error message from the look of it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154530/new/

https://reviews.llvm.org/D154530

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

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

These uses of `LLDB_LOG` are taking errors, consuming them while turning them 
into strings, and logging them. Unless I'm missing something, I think it would 
make more sense to convert these to use `LLDB_LOG_ERROR`.




Comment at: lldb/source/Core/Debugger.cpp:1918-1919
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
llvm::toString(event_handler_thread.takeError()));
 }

Same here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

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

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:1918-1919
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
+  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
llvm::toString(event_handler_thread.takeError()));
 }

bulbazord wrote:
> Same here.
Sorry, ignore this comment, I deleted a previous inline comment and forgot to 
delete this one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

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

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D154532#4474534 , @bulbazord wrote:

> These uses of `LLDB_LOG` are taking errors, consuming them while turning them 
> into strings, and logging them. Unless I'm missing something, I think it 
> would make more sense to convert these to use `LLDB_LOG_ERROR`.

Good point, I blindly fixed the format specifiers without lookin what was being 
passed in.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

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

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 537435.
JDevlieghere edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

Files:
  lldb/source/API/SBTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,8 +3600,8 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
- llvm::toString(private_state_thread.takeError()));
+LLDB_LOG_ERROR(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
+   private_state_thread.takeError());
 return false;
   }
 
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -182,8 +182,8 @@
 llvm::Expected maybe_thread =
 Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
 if (!maybe_thread)
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), "failed to launch host thread: 
{0}",
+ maybe_thread.takeError());
 return true;
   }
   return false;
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -177,8 +177,8 @@
 if (error_ptr)
   *error_ptr = Status(maybe_thread.takeError());
 else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), "failed to launch host thread: 
{0}",
+ maybe_thread.takeError());
 }
   }
 
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1915,8 +1915,8 @@
 if (event_handler_thread) {
   m_event_handler_thread = *event_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(event_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), "failed to launch host thread: 
{0}",
+ event_handler_thread.takeError());
 }
 
 // Make sure DefaultEventHandler() is running and listening to events
@@ -2056,8 +2056,8 @@
 if (io_handler_thread) {
   m_io_handler_thread = *io_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(io_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), "failed to launch host thread: 
{0}",
+ io_handler_thread.takeError());
 }
   }
   return m_io_handler_thread.IsJoinable();
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1117,8 +1117,9 @@
 llvm::Expected> expected_vector =
 target_sp->GetBreakpointList().FindBreakpointsByName(name);
 if (!expected_vector) {
-  LLDB_LOG(GetLog(LLDBLog::Breakpoints), "invalid breakpoint name: {}",
-   llvm::toString(expected_vector.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Breakpoints),
+ "invalid breakpoint name: {0}",
+ expected_vector.takeError());
   return false;
 }
 for (BreakpointSP bkpt_sp : *expected_vector) {
@@ -1591,7 +1592,7 @@
 
 const char *SBTarget::GetABIName() {
   LLDB_INSTRUMENT_VA(this);
-  
+
   TargetSP target_sp(GetSP());
   if (!target_sp)
 return nullptr;


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,8 +3600,8 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
- llvm::toString(private_state_thread.takeError()));
+LLDB_LOG_ERROR(GetLog(LLDBLog::Host), "failed to launch host thread: {0}",
+   private_state_thread.takeError());
 return false;
   }
 
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/so

[Lldb-commits] [PATCH] D154530: [lldb] Fix incorrect uses of LLDB_LOG_ERROR

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve accepted this revision.
fdeazeve added a comment.

Nice catch!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154530/new/

https://reviews.llvm.org/D154530

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

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, fdeazeve.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This accomplishes a few minor things:

- Removed unnecessary uses of `this->`
- Removed an unnecessary std::string allocation.
- Removed some nesting to improve readability using early returns where it 
makes sense.
- Replaced `strtoul` with `llvm::to_integer` which avoids another std::string 
allocation.
- Removed braces from single statement conditions, removed else-after-returns.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154534

Files:
  lldb/source/Utility/StructuredData.cpp


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -104,33 +104,29 @@
 
 StructuredData::ObjectSP
 StructuredData::Object::GetObjectForDotSeparatedPath(llvm::StringRef path) {
-  if (this->GetType() == lldb::eStructuredDataTypeDictionary) {
+  if (GetType() == lldb::eStructuredDataTypeDictionary) {
 std::pair match = path.split('.');
-std::string key = match.first.str();
-ObjectSP value = this->GetAsDictionary()->GetValueForKey(key);
-if (value.get()) {
-  // Do we have additional words to descend?  If not, return the value
-  // we're at right now.
-  if (match.second.empty()) {
-return value;
-  } else {
-return value->GetObjectForDotSeparatedPath(match.second);
-  }
-}
-return ObjectSP();
-  }
-
-  if (this->GetType() == lldb::eStructuredDataTypeArray) {
+llvm::StringRef key = match.first;
+ObjectSP value = GetAsDictionary()->GetValueForKey(key);
+if (!value)
+  return {};
+
+// Do we have additional words to descend?  If not, return the value
+// we're at right now.
+if (match.second.empty())
+  return value;
+
+return value->GetObjectForDotSeparatedPath(match.second);
+  } else if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');
-if (match.second.empty()) {
+if (match.second.empty())
   return this->shared_from_this();
-}
-errno = 0;
-uint64_t val = strtoul(match.second.str().c_str(), nullptr, 10);
-if (errno == 0) {
-  return this->GetAsArray()->GetItemAtIndex(val);
-}
-return ObjectSP();
+
+uint64_t val = 0;
+if (!llvm::to_integer(match.second, val, /* Base = */ 10))
+  return {};
+
+return GetAsArray()->GetItemAtIndex(val);
   }
 
   return this->shared_from_this();


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -104,33 +104,29 @@
 
 StructuredData::ObjectSP
 StructuredData::Object::GetObjectForDotSeparatedPath(llvm::StringRef path) {
-  if (this->GetType() == lldb::eStructuredDataTypeDictionary) {
+  if (GetType() == lldb::eStructuredDataTypeDictionary) {
 std::pair match = path.split('.');
-std::string key = match.first.str();
-ObjectSP value = this->GetAsDictionary()->GetValueForKey(key);
-if (value.get()) {
-  // Do we have additional words to descend?  If not, return the value
-  // we're at right now.
-  if (match.second.empty()) {
-return value;
-  } else {
-return value->GetObjectForDotSeparatedPath(match.second);
-  }
-}
-return ObjectSP();
-  }
-
-  if (this->GetType() == lldb::eStructuredDataTypeArray) {
+llvm::StringRef key = match.first;
+ObjectSP value = GetAsDictionary()->GetValueForKey(key);
+if (!value)
+  return {};
+
+// Do we have additional words to descend?  If not, return the value
+// we're at right now.
+if (match.second.empty())
+  return value;
+
+return value->GetObjectForDotSeparatedPath(match.second);
+  } else if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');
-if (match.second.empty()) {
+if (match.second.empty())
   return this->shared_from_this();
-}
-errno = 0;
-uint64_t val = strtoul(match.second.str().c_str(), nullptr, 10);
-if (errno == 0) {
-  return this->GetAsArray()->GetItemAtIndex(val);
-}
-return ObjectSP();
+
+uint64_t val = 0;
+if (!llvm::to_integer(match.second, val, /* Base = */ 10))
+  return {};
+
+return GetAsArray()->GetItemAtIndex(val);
   }
 
   return this->shared_from_this();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 537442.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

Files:
  lldb/source/API/SBTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,8 +3600,8 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
- llvm::toString(private_state_thread.takeError()));
+LLDB_LOG_ERROR(GetLog(LLDBLog::Host), private_state_thread.takeError(),
+   "failed to launch host thread: {0}");
 return false;
   }
 
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -182,8 +182,8 @@
 llvm::Expected maybe_thread =
 Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
 if (!maybe_thread)
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+ "failed to launch host thread: {0}");
 return true;
   }
   return false;
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -177,8 +177,8 @@
 if (error_ptr)
   *error_ptr = Status(maybe_thread.takeError());
 else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
   }
 
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1915,8 +1915,8 @@
 if (event_handler_thread) {
   m_event_handler_thread = *event_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(event_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), event_handler_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
 
 // Make sure DefaultEventHandler() is running and listening to events
@@ -2056,8 +2056,8 @@
 if (io_handler_thread) {
   m_io_handler_thread = *io_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(io_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), io_handler_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
   }
   return m_io_handler_thread.IsJoinable();
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1117,8 +1117,8 @@
 llvm::Expected> expected_vector =
 target_sp->GetBreakpointList().FindBreakpointsByName(name);
 if (!expected_vector) {
-  LLDB_LOG(GetLog(LLDBLog::Breakpoints), "invalid breakpoint name: {}",
-   llvm::toString(expected_vector.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Breakpoints), expected_vector.takeError(),
+ "invalid breakpoint name: {0}");
   return false;
 }
 for (BreakpointSP bkpt_sp : *expected_vector) {
@@ -1591,7 +1591,7 @@
 
 const char *SBTarget::GetABIName() {
   LLDB_INSTRUMENT_VA(this);
-  
+
   TargetSP target_sp(GetSP());
   if (!target_sp)
 return nullptr;


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,8 +3600,8 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
- llvm::toString(private_state_thread.takeError()));
+LLDB_LOG_ERROR(GetLog(LLDBLog::Host), private_state_thread.takeError(),
+   "failed to launch host thread: {0}");
 return false;
   }
 
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -182,8 +182,8 @@
 llvm::Expect

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve added a comment.

In https://reviews.llvm.org/D154532, the string argument comes after the error. 
Are these interchangeable?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

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

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve accepted this revision.
fdeazeve added a comment.
This revision is now accepted and ready to land.

Ah you updated the review as I typed the question! LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

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

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord requested changes to this revision.
bulbazord added a comment.
This revision now requires changes to proceed.

The 2nd argument to `LLDB_LOG_ERROR` is the error itself, so these should be 
something like `LLDB_LOG_ERROR(GetLog(whatever), std::move(err), "message: 
{0}");` or something to this effect.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

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

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

I see you updated it while I typed it up. LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

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

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve added inline comments.



Comment at: lldb/source/Utility/StructuredData.cpp:120
+return value->GetObjectForDotSeparatedPath(match.second);
+  } else if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');

Shouldn't we remove this `else`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154534/new/

https://reviews.llvm.org/D154534

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

[Lldb-commits] [lldb] e0e36e3 - [lldb] Fix incorrect uses of LLDB_LOG_ERROR

2023-07-05 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-07-05T11:27:52-07:00
New Revision: e0e36e3725b50ac690d1839f0e9476e93ff7988d

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

LOG: [lldb] Fix incorrect uses of LLDB_LOG_ERROR

Fix incorrect uses of LLDB_LOG_ERROR. The macro doesn't automatically
inject the error in the log message: it merely passes the error as the
first argument to formatv and therefore must be referenced with {0}.

Thanks to Nicholas Allegra for collecting a list of places where the
macro was misused.

rdar://111581655

Differential revision: https://reviews.llvm.org/D154530

Added: 


Modified: 
lldb/source/Breakpoint/Watchpoint.cpp
lldb/source/Core/ValueObjectRegister.cpp
lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/ThreadPlanTracer.cpp

Removed: 




diff  --git a/lldb/source/Breakpoint/Watchpoint.cpp 
b/lldb/source/Breakpoint/Watchpoint.cpp
index 597e696c71276c..b6e6d4a5a32d5e 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -41,14 +41,14 @@ Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, 
uint32_t size,
 target.GetScratchTypeSystemForLanguage(eLanguageTypeC);
 if (auto err = type_system_or_err.takeError()) {
   LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
- "Failed to set type.");
+ "Failed to set type: {0}");
 } else {
   if (auto ts = *type_system_or_err)
 m_type =
 ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8 * size);
   else
 LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
-   "Failed to set type. Typesystem is no longer live.");
+   "Failed to set type: Typesystem is no longer live: 
{0}");
 }
   }
 

diff  --git a/lldb/source/Core/ValueObjectRegister.cpp 
b/lldb/source/Core/ValueObjectRegister.cpp
index 798868997a05c1..c2b84c11347359 100644
--- a/lldb/source/Core/ValueObjectRegister.cpp
+++ b/lldb/source/Core/ValueObjectRegister.cpp
@@ -202,7 +202,7 @@ CompilerType ValueObjectRegister::GetCompilerTypeImpl() {
 exe_module->GetTypeSystemForLanguage(eLanguageTypeC);
 if (auto err = type_system_or_err.takeError()) {
   LLDB_LOG_ERROR(GetLog(LLDBLog::Types), std::move(err),
- "Unable to get CompilerType from TypeSystem");
+ "Unable to get CompilerType from TypeSystem: {0}");
 } else {
   if (auto ts = *type_system_or_err)
 m_compiler_type = ts->GetBuiltinTypeForEncodingAndBitSize(

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
index c92ac4bdec1480..e780cd8285c455 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
@@ -45,7 +45,7 @@ class BlockPointerSyntheticFrontEnd : public 
SyntheticChildrenFrontEnd {
 lldb::eLanguageTypeC_plus_plus);
 if (auto err = type_system_or_err.takeError()) {
   LLDB_LOG_ERROR(GetLog(LLDBLog::DataFormatters), std::move(err),
- "Failed to get scratch TypeSystemClang");
+ "Failed to get scratch TypeSystemClang: {0}");
   return;
 }
 

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index ef54c2b5044b0b..a2e1c8e54dd4e5 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1685,7 +1685,7 @@ 
AppleObjCRuntimeV2::SharedCacheImageHeaders::CreateSharedCacheImageHeaders(
   entsize));
   if (auto Err = shared_cache_image_headers->UpdateIfNeeded()) {
 LLDB_LOG_ERROR(log, std::move(Err),
-   "Failed to update SharedCacheImageHeaders");
+   "Failed to update SharedCacheImageHeaders: {0}");
 ret

[Lldb-commits] [lldb] 520681e - [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-07-05T11:27:52-07:00
New Revision: 520681e56d3ab9a9f187a1f9c805ff281b815d55

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

LOG: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

Fix incorrect uses of formatv specifiers in LLDB_LOG. Unlike Python,
arguments must be numbered. All the affected log statements take
llvm:Errors so use the LLDB_LOG_ERROR macro instead.

Differential revision: https://reviews.llvm.org/D154532

Added: 


Modified: 
lldb/source/API/SBTarget.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/ThreadedCommunication.cpp
lldb/source/Host/common/ProcessLaunchInfo.cpp
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 53b1bd6a535d62..3fef7428abe24b 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1117,8 +1117,8 @@ bool SBTarget::FindBreakpointsByName(const char *name,
 llvm::Expected> expected_vector =
 target_sp->GetBreakpointList().FindBreakpointsByName(name);
 if (!expected_vector) {
-  LLDB_LOG(GetLog(LLDBLog::Breakpoints), "invalid breakpoint name: {}",
-   llvm::toString(expected_vector.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Breakpoints), expected_vector.takeError(),
+ "invalid breakpoint name: {0}");
   return false;
 }
 for (BreakpointSP bkpt_sp : *expected_vector) {
@@ -1591,7 +1591,7 @@ const char *SBTarget::GetTriple() {
 
 const char *SBTarget::GetABIName() {
   LLDB_INSTRUMENT_VA(this);
-  
+
   TargetSP target_sp(GetSP());
   if (!target_sp)
 return nullptr;

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index bf672fe013847f..196038baa6d80a 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1915,8 +1915,8 @@ bool Debugger::StartEventHandlerThread() {
 if (event_handler_thread) {
   m_event_handler_thread = *event_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(event_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), event_handler_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
 
 // Make sure DefaultEventHandler() is running and listening to events
@@ -2056,8 +2056,8 @@ bool Debugger::StartIOHandlerThread() {
 if (io_handler_thread) {
   m_io_handler_thread = *io_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(io_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), io_handler_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
   }
   return m_io_handler_thread.IsJoinable();

diff  --git a/lldb/source/Core/ThreadedCommunication.cpp 
b/lldb/source/Core/ThreadedCommunication.cpp
index ec4b0806a80e43..755a158a5359e9 100644
--- a/lldb/source/Core/ThreadedCommunication.cpp
+++ b/lldb/source/Core/ThreadedCommunication.cpp
@@ -177,8 +177,8 @@ bool ThreadedCommunication::StartReadThread(Status 
*error_ptr) {
 if (error_ptr)
   *error_ptr = Status(maybe_thread.takeError());
 else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
   }
 

diff  --git a/lldb/source/Host/common/ProcessLaunchInfo.cpp 
b/lldb/source/Host/common/ProcessLaunchInfo.cpp
index 0e2c3da11ba9fb..a1866b2a99fd87 100644
--- a/lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ b/lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -182,8 +182,8 @@ bool ProcessLaunchInfo::MonitorProcess() const {
 llvm::Expected maybe_thread =
 Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
 if (!maybe_thread)
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+ "failed to launch host thread: {0}");
 return true;
   }
   return false;

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index ca2eac7d8e5b04..05ddbbd146a20f 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3600,8 +3600,8 @@ bool Process::StartPrivateStateThread(bool 
is_secondary_thread) {
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch hos

[Lldb-commits] [PATCH] D154530: [lldb] Fix incorrect uses of LLDB_LOG_ERROR

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe0e36e3725b5: [lldb] Fix incorrect uses of LLDB_LOG_ERROR 
(authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D154530?vs=537425&id=537447#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154530/new/

https://reviews.llvm.org/D154530

Files:
  lldb/source/Breakpoint/Watchpoint.cpp
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetItemInfoHandler.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -56,7 +56,7 @@
 Thread &ThreadPlanTracer::GetThread() {
   if (m_thread)
 return *m_thread;
-
+
   ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
   m_thread = thread_sp.get();
   return *m_thread;
@@ -107,8 +107,9 @@
   auto type_system_or_err =
   target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
   if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(GetLog(LLDBLog::Types), std::move(err),
-   "Unable to get integer pointer type from TypeSystem");
+LLDB_LOG_ERROR(
+GetLog(LLDBLog::Types), std::move(err),
+"Unable to get integer pointer type from TypeSystem: {0}");
   } else {
 if (auto ts = *type_system_or_err)
   m_intptr_type = TypeFromUser(ts->GetBuiltinTypeForEncodingAndBitSize(
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2387,10 +2387,11 @@
 auto type_system_or_err =
 GetScratchTypeSystemForLanguage(language, create_on_demand);
 if (!type_system_or_err)
-  LLDB_LOG_ERROR(GetLog(LLDBLog::Target), type_system_or_err.takeError(),
- "Language '{}' has expression support but no scratch type "
- "system available",
- Language::GetNameForLanguageType(language));
+  LLDB_LOG_ERROR(
+  GetLog(LLDBLog::Target), type_system_or_err.takeError(),
+  "Language '{1}' has expression support but no scratch type "
+  "system available: {0}",
+  Language::GetNameForLanguageType(language));
 else
   if (auto ts = *type_system_or_err)
 scratch_type_systems.push_back(ts);
@@ -2408,9 +2409,10 @@
   auto type_system_or_err = GetScratchTypeSystemForLanguage(language, true);
 
   if (auto err = type_system_or_err.takeError()) {
-LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
-   "Unable to get persistent expression state for language {}",
-   Language::GetNameForLanguageType(language));
+LLDB_LOG_ERROR(
+GetLog(LLDBLog::Target), std::move(err),
+"Unable to get persistent expression state for language {1}: {0}",
+Language::GetNameForLanguageType(language));
 return nullptr;
   }
 
@@ -2418,7 +2420,7 @@
 return ts->GetPersistentExpressionState();
 
   LLDB_LOG(GetLog(LLDBLog::Target),
-   "Unable to get persistent expression state for language {}",
+   "Unable to get persistent expression state for language {1}: {0}",
Language::GetNameForLanguageType(language));
   return nullptr;
 }
@@ -2617,7 +2619,7 @@
   auto ts = *type_system_or_err;
   if (!ts)
 LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
-   "Scratch type system is no longer live");
+   "Scratch type system is no longer live: {0}");
   else
 persistent_var_sp =
 ts->GetPersistentExpressionState()->GetVariable(expr);
Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -1360,7 +1360,7 @@
 target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
 if (auto err = c_type_system_or_err.takeError()) {
   LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(err),
- "Unab

[Lldb-commits] [PATCH] D154532: [lldb] Fix incorrect uses of formatv specifiers in LLDB_LOG

2023-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG520681e56d3a: [lldb] Fix incorrect uses of formatv 
specifiers in LLDB_LOG (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154532/new/

https://reviews.llvm.org/D154532

Files:
  lldb/source/API/SBTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,8 +3600,8 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
- llvm::toString(private_state_thread.takeError()));
+LLDB_LOG_ERROR(GetLog(LLDBLog::Host), private_state_thread.takeError(),
+   "failed to launch host thread: {0}");
 return false;
   }
 
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -182,8 +182,8 @@
 llvm::Expected maybe_thread =
 Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID());
 if (!maybe_thread)
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+ "failed to launch host thread: {0}");
 return true;
   }
   return false;
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -177,8 +177,8 @@
 if (error_ptr)
   *error_ptr = Status(maybe_thread.takeError());
 else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(maybe_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), maybe_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
   }
 
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1915,8 +1915,8 @@
 if (event_handler_thread) {
   m_event_handler_thread = *event_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(event_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), event_handler_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
 
 // Make sure DefaultEventHandler() is running and listening to events
@@ -2056,8 +2056,8 @@
 if (io_handler_thread) {
   m_io_handler_thread = *io_handler_thread;
 } else {
-  LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
-   llvm::toString(io_handler_thread.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), io_handler_thread.takeError(),
+ "failed to launch host thread: {0}");
 }
   }
   return m_io_handler_thread.IsJoinable();
Index: lldb/source/API/SBTarget.cpp
===
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -1117,8 +1117,8 @@
 llvm::Expected> expected_vector =
 target_sp->GetBreakpointList().FindBreakpointsByName(name);
 if (!expected_vector) {
-  LLDB_LOG(GetLog(LLDBLog::Breakpoints), "invalid breakpoint name: {}",
-   llvm::toString(expected_vector.takeError()));
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Breakpoints), expected_vector.takeError(),
+ "invalid breakpoint name: {0}");
   return false;
 }
 for (BreakpointSP bkpt_sp : *expected_vector) {
@@ -1591,7 +1591,7 @@
 
 const char *SBTarget::GetABIName() {
   LLDB_INSTRUMENT_VA(this);
-  
+
   TargetSP target_sp(GetSP());
   if (!target_sp)
 return nullptr;


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -3600,8 +3600,8 @@
   },
   8 * 1024 * 1024);
   if (!private_state_thread) {
-LLDB_LOG(GetLog(LLDBLog::Host), "failed to launch host thread: {}",
- llvm::toString(private_state_thread.takeError()));
+LLDB_LOG_ERROR(GetLog(LLDBLog::Host), private_state_thread.takeError(),
+   "failed to launch host thread: {0}");
 return false;
   }
 
Index: lldb/source/Host/common/ProcessLaunch

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Utility/StructuredData.cpp:120
+return value->GetObjectForDotSeparatedPath(match.second);
+  } else if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');

fdeazeve wrote:
> Shouldn't we remove this `else`?
Ah, the first `if` does return no matter what, so yeah this `else` isn't 
necessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154534/new/

https://reviews.llvm.org/D154534

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

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord updated this revision to Diff 537451.
bulbazord added a comment.

Remove unneeded `else`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154534/new/

https://reviews.llvm.org/D154534

Files:
  lldb/source/Utility/StructuredData.cpp


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -104,33 +104,31 @@
 
 StructuredData::ObjectSP
 StructuredData::Object::GetObjectForDotSeparatedPath(llvm::StringRef path) {
-  if (this->GetType() == lldb::eStructuredDataTypeDictionary) {
+  if (GetType() == lldb::eStructuredDataTypeDictionary) {
 std::pair match = path.split('.');
-std::string key = match.first.str();
-ObjectSP value = this->GetAsDictionary()->GetValueForKey(key);
-if (value.get()) {
-  // Do we have additional words to descend?  If not, return the value
-  // we're at right now.
-  if (match.second.empty()) {
-return value;
-  } else {
-return value->GetObjectForDotSeparatedPath(match.second);
-  }
-}
-return ObjectSP();
+llvm::StringRef key = match.first;
+ObjectSP value = GetAsDictionary()->GetValueForKey(key);
+if (!value)
+  return {};
+
+// Do we have additional words to descend?  If not, return the value
+// we're at right now.
+if (match.second.empty())
+  return value;
+
+return value->GetObjectForDotSeparatedPath(match.second);
   }
 
-  if (this->GetType() == lldb::eStructuredDataTypeArray) {
+  if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');
-if (match.second.empty()) {
+if (match.second.empty())
   return this->shared_from_this();
-}
-errno = 0;
-uint64_t val = strtoul(match.second.str().c_str(), nullptr, 10);
-if (errno == 0) {
-  return this->GetAsArray()->GetItemAtIndex(val);
-}
-return ObjectSP();
+
+uint64_t val = 0;
+if (!llvm::to_integer(match.second, val, /* Base = */ 10))
+  return {};
+
+return GetAsArray()->GetItemAtIndex(val);
   }
 
   return this->shared_from_this();


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -104,33 +104,31 @@
 
 StructuredData::ObjectSP
 StructuredData::Object::GetObjectForDotSeparatedPath(llvm::StringRef path) {
-  if (this->GetType() == lldb::eStructuredDataTypeDictionary) {
+  if (GetType() == lldb::eStructuredDataTypeDictionary) {
 std::pair match = path.split('.');
-std::string key = match.first.str();
-ObjectSP value = this->GetAsDictionary()->GetValueForKey(key);
-if (value.get()) {
-  // Do we have additional words to descend?  If not, return the value
-  // we're at right now.
-  if (match.second.empty()) {
-return value;
-  } else {
-return value->GetObjectForDotSeparatedPath(match.second);
-  }
-}
-return ObjectSP();
+llvm::StringRef key = match.first;
+ObjectSP value = GetAsDictionary()->GetValueForKey(key);
+if (!value)
+  return {};
+
+// Do we have additional words to descend?  If not, return the value
+// we're at right now.
+if (match.second.empty())
+  return value;
+
+return value->GetObjectForDotSeparatedPath(match.second);
   }
 
-  if (this->GetType() == lldb::eStructuredDataTypeArray) {
+  if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');
-if (match.second.empty()) {
+if (match.second.empty())
   return this->shared_from_this();
-}
-errno = 0;
-uint64_t val = strtoul(match.second.str().c_str(), nullptr, 10);
-if (errno == 0) {
-  return this->GetAsArray()->GetItemAtIndex(val);
-}
-return ObjectSP();
+
+uint64_t val = 0;
+if (!llvm::to_integer(match.second, val, /* Base = */ 10))
+  return {};
+
+return GetAsArray()->GetItemAtIndex(val);
   }
 
   return this->shared_from_this();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve accepted this revision.
fdeazeve added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks for improving this


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154534/new/

https://reviews.llvm.org/D154534

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

[Lldb-commits] [lldb] 58370ee - Revert "Change the dyld notification function that lldb puts a breakpoint in"

2023-07-05 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2023-07-05T12:52:21-07:00
New Revision: 58370eef673e80f3aea1bb431a291d1564e0f9f4

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

LOG: Revert "Change the dyld notification function that lldb puts a breakpoint 
in"

We're seeing a lot of test failures on the lldb incremental x86 CI bot
since I landed https://reviews.llvm.org/D139453 - revert it while I
investigate.

This reverts commit 624813a4f41c5945dc8f8d998173960ad75db731.

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index f439fa88fc7345..67e79fdcec8f56 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -235,15 +235,10 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
  lldb::user_id_t break_loc_id) {
   // Let the event know that the images have changed
   // DYLD passes three arguments to the notification breakpoint.
-  //
-  // Arg1: enum dyld_notify_mode mode
-  // 0 = adding, 1 = removing, 2 = remove all, 3 = dyld moved
-  //
-  // Arg2: unsigned long count
-  // Number of shared libraries added/removed
-  //
-  // Arg3: struct dyld_image_info mach_headers[]
-  // Array of load addresses of binaries added/removed
+  // Arg1: enum dyld_notify_mode mode - 0 = adding, 1 = removing, 2 = remove
+  // all Arg2: unsigned long icount- Number of shared libraries
+  // added/removed Arg3: uint64_t mach_headers[] - Array of load addresses
+  // of binaries added/removed
 
   DynamicLoaderMacOS *dyld_instance = (DynamicLoaderMacOS *)baton;
 
@@ -273,10 +268,9 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
 ValueList argument_values;
 
 Value mode_value;// enum dyld_notify_mode { dyld_notify_adding=0,
- // dyld_notify_removing=1, dyld_notify_remove_all=2,
- // dyld_notify_dyld_moved=3 };
+ // dyld_notify_removing=1, dyld_notify_remove_all=2 };
 Value count_value;   // unsigned long count
-Value headers_value; // struct dyld_image_info machHeaders[]
+Value headers_value; // uint64_t machHeaders[] (aka void*)
 
 CompilerType clang_void_ptr_type =
 scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
@@ -305,9 +299,6 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
 argument_values.PushValue(count_value);
 argument_values.PushValue(headers_value);
 
-// void lldb_image_notifier(enum dyld_image_mode mode, uint32_t infoCount,
-// const dyld_image_info info[])
-
 if (abi->GetArgumentValues(exe_ctx.GetThreadRef(), argument_values)) {
   uint32_t dyld_mode =
   argument_values.GetValueAtIndex(0)->GetScalar().UInt(-1);
@@ -321,32 +312,12 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
   argument_values.GetValueAtIndex(2)->GetScalar().ULongLong(-1);
   if (header_array != static_cast(-1)) {
 std::vector image_load_addresses;
-
-// struct dyld_image_info_32 {
-// uint32_timageLoadAddress;
-// uint32_timageFilePath;
-// uint32_timageFileModDate;
-// };
-// struct dyld_image_info_64 {
-// uint64_timageLoadAddress;
-// uint64_timageFilePath;
-// uint64_timageFileModDate;
-// };
-
-uint32_t addr_size =
-process->GetTarget().GetArchitecture().GetAddressByteSize();
 for (uint64_t i = 0; i < image_infos_count; i++) {
   Status error;
-  addr_t dyld_image_info = header_array + (3 * addr_size * i);
-  addr_t addr =
-  process->ReadPointerFromMemory(dyld_image_info, error);
-  if (error.Success()) {
+  addr_t addr = process->ReadUnsignedIntegerFromMemory(
+  header_array + (8 * i), 8, LLDB_INVALID_ADDRESS, error);
+  if (addr != LLDB_INVALID_ADDRESS) {
 image_load_addresses.push_back(addr);
-  } else {
-Debugger::ReportWarning(
-"DynamicLoaderMacOS::NotifyBreakpointHit unable "
-"to read binary mach-o load address at 0x%" PRIx64,
-addr);
   }
 }
 if (dyld_mode == 0) {
@@ -391,18 +362,1

[Lldb-commits] [PATCH] D153840: [LLDB] Fix buffer overflow problem in DWARFExpression::Evaluate.

2023-07-05 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added a comment.

In D153840#4474213 , @cmtice wrote:

> Hi Jason,
>
> I had been talking more with David, and yes, I had come to the conclusion 
> that you are both right and that this was not the right fix.  I am planning 
> on reverting this, but I am trying to figure out the right fix to replace it 
> with.  I can't share the source that was causing the bug to manifest, because 
> it's in proprietary code, but David is looking at it and I believe he has 
> come to the conclusion that there is a bug in the DWARF code generation -- we 
> were getting a size of 16, which is absolutely not right.  The question is, 
> in the case of bad DWARF being generated, what (if anything) should the LLDB 
> code here be doing? Should we check the size as soon as we read it in, and 
> assert that  it must be <= 8?  Or something else?  Or just leave the LLDB 
> code entirely alone?
>
> What do you (and other reviewers) think is the right thing to do here?

While it's likely generally under-tested/under-covered, debuggers shouldn't 
crash on invalid/problematic DWARF. So this code should probably abort parsing 
an invalid expression like this - there's probably some codepaths through here 
that do some kind of error handling like the "Failed to dereference pointer 
from" a few lines later. I'd expect a similar error handling should be 
introduced if `size` is invalid (not sure if "invalid" should be `> 
sizeof(lldb::addr_t)` or maybe something more specific (like it should check 
the address size in the DWARF, perhaps - I don't know/recall the /exact/ DWARF 
spec wording about the size limitations)).

Comment at: lldb/source/Expression/DWARFExpression.cpp:1088
 intptr_t ptr;
 ::memcpy(&ptr, src, sizeof(void *));
 // I can't decide whether the size operand should apply to the bytes in

FWIW, I think this code is differently invalid, but figured I'd mention it 
while I'm here - this is probably illegal load widening. If we're processing 
`DW_OP_deref_size(1)` maybe that 1 byte is at the end of a page - reading 
`sizeof(void*)` would read more data than would be valid, and be a problem?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153840/new/

https://reviews.llvm.org/D153840

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

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib accepted this revision.
mib added a comment.

LGTM with nits.




Comment at: lldb/source/Utility/StructuredData.cpp:125
+if (match.second.empty())
   return this->shared_from_this();
+





Comment at: lldb/source/Utility/StructuredData.cpp:134
 
   return this->shared_from_this();
 }




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154534/new/

https://reviews.llvm.org/D154534

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

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Utility/StructuredData.cpp:125
+if (match.second.empty())
   return this->shared_from_this();
+

mib wrote:
> 
Good catch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154534/new/

https://reviews.llvm.org/D154534

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

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Jim Ingham via Phabricator via lldb-commits

jingham created this revision.
jingham added reviewers: JDevlieghere, mib, bulbazord, labath.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This  patch enhances the interruption features I added a little while back.  
There are two main changes.  The main one here is that I made a nicer interface 
to reporting interruption events.  I also added a way to ask a debugger if it 
was using a Module, so that we can query for interruption in Module code where 
we don't have access to a debugger.  I used that to support interrupting debug 
info reading on a per-module basis - since that's a long-running but - at least 
on a module boundary - resumable operation.

The current method of checking "Does this module one belong to a debugger 
requesting interruption" is quick and dirty here.  IMO, a better solution would 
be to have Modules keep a list of the Debuggers (or maybe Targets) using them.  
But we should figure out all the ways we want to use that and design something 
nice for the purpose.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154542

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
 return error;
   }
   target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+  debugger.GetTargetList().RegisterInProcessTarget(target_sp.get());
   target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
   if (user_exe_path_is_bundle)
 exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
 // No file was specified, just create an empty target with any arch if a
 // valid arch was specified
 target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp.get());
   }
 
   if (!target_sp)
@@ -513,6 +515,7 @@
 void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
   lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
  "target already exists it the list");
+  UnregisterInProcessTarget(target_sp.get());
   m_target_list.push_back(std::move(target_sp));
   if (do_select)
 SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
 m_selected_target_idx = 0;
   return GetTargetAtIndex(m_selected_target_idx);
 }
+
+bool TargetList::AnyTargetContainsModule(Module *module) {
+  std::lock_guard guard(m_target_list_mutex);
+  for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(module))
+  return true;
+  }
+  for (const auto target: m_in_process_target_list) {
+if (target->GetImages().FindModule(module))
+  return true;
+  }
+  return false;
+}
+
+  void TargetList::RegisterInProcessTarget(Target *target) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target);
+assert(was_added && "Target pointer was left in the in-process map");
+  }
+  
+  void TargetList::UnregisterInProcessTarget(Target *target) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target);
+assert(was_present && "Target pointer being removed was not registered");
+  }
+  
+  bool TargetList::IsTargetInProcess(Target *target) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target) == 1; 
+  }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
 // each library in parallel.
 if (GetPreloadSymbols())
   module_sp->PreloadSymbols();
-
 llvm::SmallVector replaced_modules;
 for (ModuleSP &old_module_sp : old_modules) {
   if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
 }
 
 bool TargetProperties::GetPreloadSymbols() const {
+  if (INTERRUPT_REQUESTED(m_target->GetDebugger(), 
+  "Interrupted checking preload symbols")) {
+return false;
+  }
   const uint32_t idx = ePropertyPreloadSymbols;
   return GetPropertyAtIndexAs(
   idx, g_tar

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

This looks very cool! I left "a few" comments on how I think we can simplify 
this patch, and also had some remarks regarding the `in_process_target` sanity 
checks.




Comment at: lldb/include/lldb/Core/Debugger.h:435-436
+  ReportInterruption(InterruptionReport(cur_func, 
+llvm::formatv(formatv, 
+std::forward(args)...)));
+}

If you use `LLVM_PRETTY_FUNCTION` you won't need this.



Comment at: lldb/include/lldb/Core/Debugger.h:446
+#define INTERRUPT_REQUESTED(debugger, ...) \
+(debugger).InterruptRequested(__func__, __VA_ARGS__)
+

You could use `LLVM_PRETTY_FUNCTION` which will give you both the function 
signature and the argument well formatted.



Comment at: lldb/include/lldb/Core/Debugger.h:454
+  public:
+InterruptionReport(std::string function_name, std::string description) :
+m_function_name(function_name), 





Comment at: lldb/include/lldb/Core/Debugger.h:460-468
+InterruptionReport(std::string function_name, 
+const llvm::formatv_object_base &payload);
+
+  template 
+  InterruptionReport(std::string function_name,
+  const char *format, Args &&... args) :
+InterruptionReport(function_name, llvm::formatv(format, 
std::forward(args)...)) {}

Using `LLVM_PRETTY_FUNCTION` we can simplify the class by removing the variadic 
constructors and replace to `std::string` with a `llvm::StringLiteral`



Comment at: lldb/include/lldb/Target/TargetList.h:192
+  ///  need to ask whetehr this target contains this module. 
+  bool AnyTargetContainsModule(Module *module);
 

Can the module be null ? If not, can we pass a reference instead of a raw 
pointer here ?



Comment at: lldb/include/lldb/Target/TargetList.h:200
   collection m_target_list;
+  std::unordered_set m_in_process_target_list;
   mutable std::recursive_mutex m_target_list_mutex;

Same question as above, can any of the `in_process_target` be null ? If not, 
lets make it a `Target&`



Comment at: lldb/include/lldb/Target/TargetList.h:216
 
+  void RegisterInProcessTarget(Target *target);
+  

ditto



Comment at: lldb/include/lldb/Target/TargetList.h:218
+  
+  void UnregisterInProcessTarget(Target *target);
+  

ditto



Comment at: lldb/include/lldb/Target/TargetList.h:220
+  
+  bool IsTargetInProcess(Target *target);
+  

ditto



Comment at: lldb/source/API/SBFrame.cpp:57-58
 
+#include 
+
 using namespace lldb;

Is this still necessary ?



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:67-68
 
+#include 
+
 

Is this still necessary ?



Comment at: lldb/source/Core/Debugger.cpp:1279-1287
+Debugger::InterruptionReport::InterruptionReport(std::string function_name, 
+const llvm::formatv_object_base &payload) :  
+m_function_name(function_name), 
+m_interrupt_time(std::chrono::system_clock::now()),
+m_thread_id(llvm::get_threadid())
+{
+  llvm::raw_string_ostream desc(m_description);

You can get rid of this is you use `LLVM_PRETTY_FUNCTION`.



Comment at: lldb/source/Core/Debugger.cpp:1289-1293
+void Debugger::ReportInterruption(const InterruptionReport &report) {
+// For now, just log the description:
+  Log *log = GetLog(LLDBLog::Host);
+  LLDB_LOG(log, "Interruption: {0}", report.m_description);
+}

It would be nice if `InterruptionReport` had a `Dump` method.



Comment at: lldb/source/Core/Module.cpp:1071-1072
 if (!m_did_load_symfile.load() && can_create) {
+  Debugger::DebuggerList interruptors 
+  = DebuggersOwningModuleRequestingInterruption(this);
+  if (!interruptors.empty()) {

You can get the Module reference by dereferencing `this`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154542/new/

https://reviews.llvm.org/D154542

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

[Lldb-commits] [PATCH] D154030: [lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands.

2023-07-05 Thread John Harrison via Phabricator via lldb-commits

ashgti updated this revision to Diff 537496.
ashgti marked 4 inline comments as done.
ashgti added a comment.

Updating the behavior of auto mode to try to evalute local variables over lldb 
commands.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154030/new/

https://reviews.llvm.org/D154030

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/Options.td
  lldb/tools/lldb-vscode/README.md
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -252,7 +252,7 @@
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // We will have cleared g_vsc.focus_tid if the focus thread doesn't have
   // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
   // then set the focus thread to the first thread with a stop reason.
   if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
@@ -1065,50 +1065,62 @@
   FillResponse(request, response);
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
+
+  // If we have a frame, try to set the context for variable completions.
+  lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
+  if (frame.IsValid()) {
+frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
+frame.GetThread().SetSelectedFrame(frame.GetFrameID());
+  }
+
   std::string text = std::string(GetString(arguments, "text"));
   auto original_column = GetSigned(arguments, "column", text.size());
-  auto actual_column = original_column - 1;
+  auto original_line = GetSigned(arguments, "line", 1);
+  auto offset = original_column - 1;
+  if (original_line > 1) {
+llvm::StringRef text_ref{text};
+::llvm::SmallVector<::llvm::StringRef, 2> lines;
+text_ref.split(lines, '\n');
+for (int i = 0; i < original_line - 1; i++) {
+  offset += lines[i].size();
+}
+  }
   llvm::json::Array targets;
-  // NOTE: the 'line' argument is not needed, as multiline expressions
-  // work well already
-  // TODO: support frameID. Currently
-  // g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions
-  // is frame-unaware.
-
-  if (!text.empty() && text[0] == '`') {
-text = text.substr(1);
-actual_column--;
-  } else {
-char command[] = "expression -- ";
+
+  if (g_vsc.DetectExpressionContext(text) == ExpressionContext::Variable) {
+char command[] = "frame variable ";
 text = command + text;
-actual_column += strlen(command);
+offset += strlen(command);
   }
   lldb::SBStringList matches;
   lldb::SBStringList descriptions;
-  g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions(
-  text.c_str(), actual_column, 0, -1, matches, descriptions);
-  size_t count = std::min((uint32_t)100, matches.GetSize());
-  targets.reserve(count);
-  for (size_t i = 0; i < count; i++) {
-std::string match = matches.GetStringAtIndex(i);
-std::string description = descriptions.GetStringAtIndex(i);
-
-llvm::json::Object item;
-
-llvm::StringRef match_ref = match;
-for (llvm::StringRef commit_point : {".", "->"}) {
-  if (match_ref.contains(commit_point)) {
-match_ref = match_ref.rsplit(commit_point).second;
+
+  if (g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions(
+  text.c_str(), offset, 0, 100, matches, descriptions)) {
+// The first element is the common substring after the cursor position for
+// all the matches. The rest of the elements are the matches.
+targets.reserve(matches.GetSize() - 1);
+std::string common_pattern = matches.GetStringAtIndex(0);
+for (size_t i = 1; i < matches.GetSize(); i++) {
+  std::string match = matches.GetStringAtIndex(i);
+  std::string description = descriptions.GetStringAtIndex(i);
+
+  llvm::json::Object item;
+  llvm::StringRef match_ref = match;
+  for (llvm::StringRef commit_point : {".", "->"}) {
+if (match_ref.contains(commit_point)) {
+  match_ref = match_ref.rsplit(commit_point).second;
+}
   }
-}
-EmplaceSafeString(item, "text", match_ref);
+  EmplaceSafeString(item, "text", match_ref);
 
-if (description.empty())
-  EmplaceSafeString(item, "label", match);
-else
-  EmplaceSafeString(item, "label", match + " -- " + description);
+  if (description.empty())
+EmplaceSafeString(item, "label", match);
+  else
+EmplaceSafeString(item, "label", match + " -- " + description);
 
-targets.emplace_back(std::move(item));
+  targets.emplace_back(std::move(item));
+}
   }
 
   body.try_emplace("targets", std::move(targets));
@@ -1223,12 +1235,17 @@
   llvm::json::Object body;

[Lldb-commits] [PATCH] D154030: [lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands.

2023-07-05 Thread John Harrison via Phabricator via lldb-commits

ashgti added inline comments.



Comment at: lldb/tools/lldb-vscode/JSONUtils.h:239
+/// useful to ensure the same column provided by the setBreakpoints request
+/// are returned to the IDE as a fallback.
 ///

DavidSpickett wrote:
> Appears that this got clang-formatted accidentally. Formatting it is fine but 
> put it in an NFC change if you're gonna do that.
> 
> If you aren't already using it, the clang-format-diff script will help you 
> limit formatting to only what you've edited (with occasional extra bits): 
> https://clang.llvm.org/docs/ClangFormat.html
I had another commit (https://reviews.llvm.org/D154029) that touched this area 
of the code so I think thats how this snuck in. Reverted this change.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:797
+g_vsc.repl_mode = ReplMode::Variable;
+result.AppendMessage("lldb-vscode repl-mode variable set.");
+  } else if (new_mode == "command") {

DavidSpickett wrote:
> There is AppendMessageWithFormat that you could use to avoid repeating the 
> string here.
Do you mean Printf? Switched to that instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154030/new/

https://reviews.llvm.org/D154030

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

[Lldb-commits] [PATCH] D153489: [lldb] Print hint if object description is requested but not implemented

2023-07-05 Thread Augusto Noronha via Phabricator via lldb-commits

augusto2112 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153489/new/

https://reviews.llvm.org/D153489

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

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

I really like the change to the interface, I especially like that it can report 
what was interrupted and how much work actually done. I had a lot of the same 
feedback as Ismail but also had some questions to help me understand all the 
details.




Comment at: lldb/include/lldb/Core/Debugger.h:430-431
+  template 
+  bool InterruptRequested(const char *cur_func, 
+  const char *formatv, Args &&... args) {
+bool ret_val = InterruptRequested();

I think it would make more sense to have `cur_func` and `formatv` be of type 
`llvm::StringRef`. Concretely, if you construct a `std::string` from `nullptr` 
(like you're doing below when you make an InterruptionReport object), you will 
crash.

I know the recommendation is to use `INTERRUPT_REQUESTED` instead of filling 
this manually, but inevitably somebody will go against the advice and make a 
mistake.



Comment at: lldb/include/lldb/Core/Debugger.h:446
+#define INTERRUPT_REQUESTED(debugger, ...) \
+(debugger).InterruptRequested(__func__, __VA_ARGS__)
+

mib wrote:
> You could use `LLVM_PRETTY_FUNCTION` which will give you both the function 
> signature and the argument well formatted.
+1



Comment at: lldb/include/lldb/Core/Debugger.h:455-456
+InterruptionReport(std::string function_name, std::string description) :
+m_function_name(function_name), 
+m_description(description),
+m_interrupt_time(std::chrono::system_clock::now()),

To avoid some unnecessary copies

Could also do what Ismail is suggesting.



Comment at: lldb/include/lldb/Core/Debugger.h:465
+  InterruptionReport(std::string function_name,
+  const char *format, Args &&... args) :
+InterruptionReport(function_name, llvm::formatv(format, 
std::forward(args)...)) {}

since we're passing `format` to `formatv`, I think it would make sense for its 
type to be `llvm::StringRef` up-front here.



Comment at: lldb/include/lldb/Target/TargetList.h:191
+  ///  these not yet added target, but for interruption purposes, we might
+  ///  need to ask whetehr this target contains this module. 
+  bool AnyTargetContainsModule(Module *module);





Comment at: lldb/include/lldb/Target/TargetList.h:192
+  ///  need to ask whetehr this target contains this module. 
+  bool AnyTargetContainsModule(Module *module);
 

mib wrote:
> Can the module be null ? If not, can we pass a reference instead of a raw 
> pointer here ?
+1

Alternatively, if these are being pulled out of shared pointers, pass the 
shared pointer directly so we can avoid the issue of dangling raw pointers if 
the module ever is freed by the shared pointer.



Comment at: lldb/include/lldb/Target/TargetList.h:200
   collection m_target_list;
+  std::unordered_set m_in_process_target_list;
   mutable std::recursive_mutex m_target_list_mutex;

mib wrote:
> Same question as above, can any of the `in_process_target` be null ? If not, 
> lets make it a `Target&`
+1

Same as above, let's not circumvent shared pointer semantics since we're 
storing references/pointers to these objects.



Comment at: lldb/include/lldb/Target/TargetList.h:216-221
+  void RegisterInProcessTarget(Target *target);
+  
+  void UnregisterInProcessTarget(Target *target);
+  
+  bool IsTargetInProcess(Target *target);
+  

mib wrote:
> ditto
Same as my comment above.



Comment at: lldb/source/API/SBFrame.cpp:57
 
+#include 
+

What is this used for?



Comment at: lldb/source/Commands/CommandObjectTarget.cpp:67-68
 
+#include 
+
 

mib wrote:
> Is this still necessary ?
Where is this used?



Comment at: lldb/source/Core/Debugger.cpp:1266-1267
 
-bool Debugger::InterruptRequested() {
+bool Debugger::InterruptRequested() 
+{
   // This is the one we should call internally.  This will return true either

Did clang-format give this to you?



Comment at: lldb/source/Core/Debugger.cpp:1291-1292
+// For now, just log the description:
+  Log *log = GetLog(LLDBLog::Host);
+  LLDB_LOG(log, "Interruption: {0}", report.m_description);
+}





Comment at: lldb/source/Core/Module.cpp:1075
+for (auto debugger_sp : interruptors) {
+  REPORT_INTERRUPTION(*(debugger_sp.get()), 
+  "Interrupted fetching symbols for module {0}", 





Comment at: lldb/source/Core/Module.cpp:1077
+  "Interrupted fetching symbols for module {0}", 
+  this->GetFileSpec());
+}

I don't think you need to specify `this->`?



Comment at: lldb/source/Ta

[Lldb-commits] [PATCH] D154549: [lldb][NFCI] Remove use of Stream * from TypeSystem

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, fdeazeve, kastiglione.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We always assume these streams are valid, might as well take references
instead of raw pointers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154549

Files:
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -830,7 +830,7 @@
  bool show_summary, bool verbose, uint32_t depth) {
   if (!IsValid())
 if (auto type_system_sp = GetTypeSystem())
-  type_system_sp->DumpValue(m_type, exe_ctx, s, format, data,
+  type_system_sp->DumpValue(m_type, exe_ctx, *s, format, data,
 data_byte_offset, data_byte_size,
 bitfield_bit_size, bitfield_bit_offset,
 show_types, show_summary, verbose, depth);
@@ -844,9 +844,9 @@
  ExecutionContextScope *exe_scope) {
   if (IsValid())
 if (auto type_system_sp = GetTypeSystem())
-  return type_system_sp->DumpTypeValue(m_type, s, format, data, byte_offset,
-   byte_size, bitfield_bit_size,
-   bitfield_bit_offset, exe_scope);
+  return type_system_sp->DumpTypeValue(
+  m_type, *s, format, data, byte_offset, byte_size, bitfield_bit_size,
+  bitfield_bit_offset, exe_scope);
   return false;
 }
 
@@ -856,7 +856,7 @@
size_t data_byte_size) {
   if (IsValid())
 if (auto type_system_sp = GetTypeSystem())
-  type_system_sp->DumpSummary(m_type, exe_ctx, s, data, data_byte_offset,
+  type_system_sp->DumpSummary(m_type, exe_ctx, *s, data, data_byte_offset,
   data_byte_size);
 }
 
@@ -870,7 +870,7 @@
lldb::DescriptionLevel level) const {
   if (IsValid())
 if (auto type_system_sp = GetTypeSystem())
-  type_system_sp->DumpTypeDescription(m_type, s, level);
+  type_system_sp->DumpTypeDescription(m_type, *s, level);
 }
 
 #ifndef NDEBUG
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1030,20 +1030,20 @@
   void DumpFromSymbolFile(Stream &s, llvm::StringRef symbol_name);
 
   void DumpValue(lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx,
- Stream *s, lldb::Format format, const DataExtractor &data,
+ Stream &s, lldb::Format format, const DataExtractor &data,
  lldb::offset_t data_offset, size_t data_byte_size,
  uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
  bool show_types, bool show_summary, bool verbose,
  uint32_t depth) override;
 
-  bool DumpTypeValue(lldb::opaque_compiler_type_t type, Stream *s,
+  bool DumpTypeValue(lldb::opaque_compiler_type_t type, Stream &s,
  lldb::Format format, const DataExtractor &data,
  lldb::offset_t data_offset, size_t data_byte_size,
  uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
  ExecutionContextScope *exe_scope) override;
 
   void DumpSummary(lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx,
-   Stream *s, const DataExtractor &data,
+   Stream &s, const DataExtractor &data,
lldb::offset_t data_offset, size_t data_byte_size) override;
 
   void DumpTypeDescription(
@@ -1051,7 +1051,7 @@
   lldb::DescriptionLevel level = lldb::eDescriptionLevelFull) override;
 
   void DumpTypeDescription(
-  lldb::opaque_compiler_type_t type, Stream *s,
+  lldb::opaque_compiler_type_t type, Stream &s,
   lldb::DescriptionLevel level = lldb::eDescriptionLevelFull) override;
 
   static void DumpTypeName(const CompilerType &type);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -4750,7 +4750,7 @@
 static bool g_printed = false;
 if (!g_printed) {
   StreamString s;
-  DumpTypeDescription(type, &s);
+  DumpTypeDescription(type, s);
 
   llvm::outs() <<

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

I had an offline chat with Jim and I misunderstood originally, what he was 
trying to achieve with the `InterruptionReport:: m_function_name` so my 
suggestion of using `LLVM_PRETTY_FUNCTION` doesn't actually work here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154542/new/

https://reviews.llvm.org/D154542

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

[Lldb-commits] [PATCH] D154534: [lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath

2023-07-05 Thread Alex Langford via Phabricator via lldb-commits

bulbazord updated this revision to Diff 537523.
bulbazord added a comment.

Address feedback from @mib


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154534/new/

https://reviews.llvm.org/D154534

Files:
  lldb/source/Utility/StructuredData.cpp


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -104,36 +104,34 @@
 
 StructuredData::ObjectSP
 StructuredData::Object::GetObjectForDotSeparatedPath(llvm::StringRef path) {
-  if (this->GetType() == lldb::eStructuredDataTypeDictionary) {
+  if (GetType() == lldb::eStructuredDataTypeDictionary) {
 std::pair match = path.split('.');
-std::string key = match.first.str();
-ObjectSP value = this->GetAsDictionary()->GetValueForKey(key);
-if (value.get()) {
-  // Do we have additional words to descend?  If not, return the value
-  // we're at right now.
-  if (match.second.empty()) {
-return value;
-  } else {
-return value->GetObjectForDotSeparatedPath(match.second);
-  }
-}
-return ObjectSP();
+llvm::StringRef key = match.first;
+ObjectSP value = GetAsDictionary()->GetValueForKey(key);
+if (!value)
+  return {};
+
+// Do we have additional words to descend?  If not, return the value
+// we're at right now.
+if (match.second.empty())
+  return value;
+
+return value->GetObjectForDotSeparatedPath(match.second);
   }
 
-  if (this->GetType() == lldb::eStructuredDataTypeArray) {
+  if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');
-if (match.second.empty()) {
-  return this->shared_from_this();
-}
-errno = 0;
-uint64_t val = strtoul(match.second.str().c_str(), nullptr, 10);
-if (errno == 0) {
-  return this->GetAsArray()->GetItemAtIndex(val);
-}
-return ObjectSP();
+if (match.second.empty())
+  return shared_from_this();
+
+uint64_t val = 0;
+if (!llvm::to_integer(match.second, val, /* Base = */ 10))
+  return {};
+
+return GetAsArray()->GetItemAtIndex(val);
   }
 
-  return this->shared_from_this();
+  return shared_from_this();
 }
 
 void StructuredData::Object::DumpToStdout(bool pretty_print) const {


Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -104,36 +104,34 @@
 
 StructuredData::ObjectSP
 StructuredData::Object::GetObjectForDotSeparatedPath(llvm::StringRef path) {
-  if (this->GetType() == lldb::eStructuredDataTypeDictionary) {
+  if (GetType() == lldb::eStructuredDataTypeDictionary) {
 std::pair match = path.split('.');
-std::string key = match.first.str();
-ObjectSP value = this->GetAsDictionary()->GetValueForKey(key);
-if (value.get()) {
-  // Do we have additional words to descend?  If not, return the value
-  // we're at right now.
-  if (match.second.empty()) {
-return value;
-  } else {
-return value->GetObjectForDotSeparatedPath(match.second);
-  }
-}
-return ObjectSP();
+llvm::StringRef key = match.first;
+ObjectSP value = GetAsDictionary()->GetValueForKey(key);
+if (!value)
+  return {};
+
+// Do we have additional words to descend?  If not, return the value
+// we're at right now.
+if (match.second.empty())
+  return value;
+
+return value->GetObjectForDotSeparatedPath(match.second);
   }
 
-  if (this->GetType() == lldb::eStructuredDataTypeArray) {
+  if (GetType() == lldb::eStructuredDataTypeArray) {
 std::pair match = path.split('[');
-if (match.second.empty()) {
-  return this->shared_from_this();
-}
-errno = 0;
-uint64_t val = strtoul(match.second.str().c_str(), nullptr, 10);
-if (errno == 0) {
-  return this->GetAsArray()->GetItemAtIndex(val);
-}
-return ObjectSP();
+if (match.second.empty())
+  return shared_from_this();
+
+uint64_t val = 0;
+if (!llvm::to_integer(match.second, val, /* Base = */ 10))
+  return {};
+
+return GetAsArray()->GetItemAtIndex(val);
   }
 
-  return this->shared_from_this();
+  return shared_from_this();
 }
 
 void StructuredData::Object::DumpToStdout(bool pretty_print) const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154505: [lldb][NFC] Remove code duplication in InitOSO

2023-07-05 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve updated this revision to Diff 537524.
fdeazeve added a comment.

Address review comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154505/new/

https://reviews.llvm.org/D154505

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -32,6 +32,7 @@
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Symbol/VariableList.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "lldb/Target/StackFrame.h"
@@ -286,112 +287,105 @@
   // we return the abilities of the first N_OSO's DWARF.
 
   Symtab *symtab = m_objfile_sp->GetSymtab();
-  if (symtab) {
-Log *log = GetLog(DWARFLog::DebugMap);
-
-std::vector oso_indexes;
-// When a mach-o symbol is encoded, the n_type field is encoded in bits
-// 23:16, and the n_desc field is encoded in bits 15:0.
-//
-// To find all N_OSO entries that are part of the DWARF + debug map we find
-// only object file symbols with the flags value as follows: bits 23:16 ==
-// 0x66 (N_OSO) bits 15: 0 == 0x0001 (specifies this is a debug map object
-// file)
-const uint32_t k_oso_symbol_flags_value = 0x660001u;
-
-const uint32_t oso_index_count =
-symtab->AppendSymbolIndexesWithTypeAndFlagsValue(
-eSymbolTypeObjectFile, k_oso_symbol_flags_value, oso_indexes);
-
-if (oso_index_count > 0) {
-  symtab->AppendSymbolIndexesWithType(eSymbolTypeCode, Symtab::eDebugYes,
-  Symtab::eVisibilityAny,
-  m_func_indexes);
-  symtab->AppendSymbolIndexesWithType(eSymbolTypeData, Symtab::eDebugYes,
-  Symtab::eVisibilityAny,
-  m_glob_indexes);
-
-  symtab->SortSymbolIndexesByValue(m_func_indexes, true);
-  symtab->SortSymbolIndexesByValue(m_glob_indexes, true);
-
-  for (uint32_t sym_idx : m_func_indexes) {
-const Symbol *symbol = symtab->SymbolAtIndex(sym_idx);
-lldb::addr_t file_addr = symbol->GetAddressRef().GetFileAddress();
-lldb::addr_t byte_size = symbol->GetByteSize();
-DebugMap::Entry debug_map_entry(
-file_addr, byte_size, OSOEntry(sym_idx, LLDB_INVALID_ADDRESS));
-m_debug_map.Append(debug_map_entry);
-  }
-  for (uint32_t sym_idx : m_glob_indexes) {
-const Symbol *symbol = symtab->SymbolAtIndex(sym_idx);
-lldb::addr_t file_addr = symbol->GetAddressRef().GetFileAddress();
-lldb::addr_t byte_size = symbol->GetByteSize();
-DebugMap::Entry debug_map_entry(
-file_addr, byte_size, OSOEntry(sym_idx, LLDB_INVALID_ADDRESS));
-m_debug_map.Append(debug_map_entry);
-  }
-  m_debug_map.Sort();
-
-  m_compile_unit_infos.resize(oso_index_count);
-
-  for (uint32_t i = 0; i < oso_index_count; ++i) {
-const uint32_t so_idx = oso_indexes[i] - 1;
-const uint32_t oso_idx = oso_indexes[i];
-const Symbol *so_symbol = symtab->SymbolAtIndex(so_idx);
-const Symbol *oso_symbol = symtab->SymbolAtIndex(oso_idx);
-if (so_symbol && oso_symbol &&
-so_symbol->GetType() == eSymbolTypeSourceFile &&
-oso_symbol->GetType() == eSymbolTypeObjectFile) {
-  m_compile_unit_infos[i].so_file.SetFile(
-  so_symbol->GetName().AsCString(), FileSpec::Style::native);
-  m_compile_unit_infos[i].oso_path = oso_symbol->GetName();
-  m_compile_unit_infos[i].oso_mod_time =
-  llvm::sys::toTimePoint(oso_symbol->GetIntegerValue(0));
-  uint32_t sibling_idx = so_symbol->GetSiblingIndex();
-  // The sibling index can't be less that or equal to the current index
-  // "i"
-  if (sibling_idx == UINT32_MAX) {
-m_objfile_sp->GetModule()->ReportError(
-"N_SO in symbol with UID {0} has invalid sibling in debug "
-"map, "
-"please file a bug and attach the binary listed in this error",
-so_symbol->GetID());
-  } else {
-const Symbol *last_symbol = symtab->SymbolAtIndex(sibling_idx - 1);
-m_compile_unit_infos[i].first_symbol_index = so_idx;
-m_compile_unit_infos[i].last_symbol_index = sibling_idx - 1;
-m_compile_unit_infos[i].first_symbol_id = so_symbol->GetID();
-m_compile_unit_infos[i].last_symbol_id = last_symbol->GetID();
-
-LLDB_LOGF(log, "Initialized OSO 0x%8.8x: file=%s", i,
-  oso_symbol->GetName().GetCString());
-  }
-} else

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



Comment at: lldb/include/lldb/Core/Debugger.h:435-436
+  ReportInterruption(InterruptionReport(cur_func, 
+llvm::formatv(formatv, 
+std::forward(args)...)));
+}

mib wrote:
> If you use `LLVM_PRETTY_FUNCTION` you won't need this.
I'm not sure I understand how you want to use LLVM_PRETTY_FUNCTION.  From what 
I can tell, LLVM_PRETTY_FUNCTION prints something that looks like the function 
invocation as written.  But ReportInterruption and the associated macros 
provide a formatv API.



Comment at: lldb/source/Target/TargetList.cpp:518
  "target already exists it the list");
+  UnregisterInProcessTarget(target_sp.get());
   m_target_list.push_back(std::move(target_sp));

bulbazord wrote:
> I'm somewhat puzzled by this. Why are we unregistering the target in 
> `AddTargetInternal` when we're registering it in `CreateTargetInternal`? 
> Maybe I don't understand the intent behind 
> `{Register,Unregister}InProcessTarget`.
The point behind this is someone says "make me a target" and then in the 
process of making the target, the Target code does something slow (e.g. look 
for external debug files) that we want to interrupt.  So we need a way for the 
debugger to find the "in the process of being made" Targets.  

 "AddTargetInternal" is the API where the target gets added to the Debugger's 
TargetList, so after that method has run we will find the target in the regular 
TargetList.  But between CreateTargetInternal and AddTargetInternal, that 
target isn't stored anywhere that the Debugger knows how to query.  I added 
this list to hold the targets that are in the process of getting made, before 
they get added to the Debugger's TargetList.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154542/new/

https://reviews.llvm.org/D154542

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

[Lldb-commits] [PATCH] D154030: [lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands.

2023-07-05 Thread John Harrison via Phabricator via lldb-commits

ashgti updated this revision to Diff 537536.
ashgti added a comment.

Uploading latest diff


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154030/new/

https://reviews.llvm.org/D154030

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/Options.td
  lldb/tools/lldb-vscode/README.md
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -191,6 +191,27 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// Send a "continued" event to indicate the process is in the running state.
+void SendContinuedEvent() {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  if (!process.IsValid()) {
+return;
+  }
+
+  // If the focus thread is not set then we haven't reported any thread status
+  // to the client, so nothing to report.
+  if (g_vsc.focus_tid == LLDB_INVALID_THREAD_ID) {
+return;
+  }
+
+  llvm::json::Object event(CreateEventObject("continued"));
+  llvm::json::Object body;
+  body.try_emplace("threadId", (int64_t)g_vsc.focus_tid);  
+  body.try_emplace("allThreadsContinued", true);
+  event.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(event)));
+}
+
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
@@ -252,7 +273,7 @@
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // We will have cleared g_vsc.focus_tid if the focus thread doesn't have
   // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
   // then set the focus thread to the first thread with a stop reason.
   if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
@@ -468,6 +489,7 @@
 break;
   case lldb::eStateRunning:
 g_vsc.WillContinue();
+SendContinuedEvent();
 break;
   case lldb::eStateExited:
 // When restarting, we can get an "exited" event for the process we
@@ -767,9 +789,10 @@
   FillResponse(request, response);
   lldb::SBProcess process = g_vsc.target.GetProcess();
   auto arguments = request.getObject("arguments");
-  // Remember the thread ID that caused the resume so we can set the
-  // "threadCausedFocus" boolean value in the "stopped" events.
-  g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+  // Invalidate the focused, continuing the process may land on any thread, not
+  // just the current thread in which case we want the client to update focus to
+  // the new stopped thread.
+  g_vsc.focus_tid = LLDB_INVALID_THREAD_ID;
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
@@ -1065,50 +1088,62 @@
   FillResponse(request, response);
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
+
+  // If we have a frame, try to set the context for variable completions.
+  lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
+  if (frame.IsValid()) {
+frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
+frame.GetThread().SetSelectedFrame(frame.GetFrameID());
+  }
+
   std::string text = std::string(GetString(arguments, "text"));
   auto original_column = GetSigned(arguments, "column", text.size());
-  auto actual_column = original_column - 1;
+  auto original_line = GetSigned(arguments, "line", 1);
+  auto offset = original_column - 1;
+  if (original_line > 1) {
+llvm::StringRef text_ref{text};
+::llvm::SmallVector<::llvm::StringRef, 2> lines;
+text_ref.split(lines, '\n');
+for (int i = 0; i < original_line - 1; i++) {
+  offset += lines[i].size();
+}
+  }
   llvm::json::Array targets;
-  // NOTE: the 'line' argument is not needed, as multiline expressions
-  // work well already
-  // TODO: support frameID. Currently
-  // g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions
-  // is frame-unaware.
-
-  if (!text.empty() && text[0] == '`') {
-text = text.substr(1);
-actual_column--;
-  } else {
-char command[] = "expression -- ";
+
+  if (g_vsc.DetectExpressionContext(frame, text) == ExpressionContext::Variable) {
+char command[] = "frame variable ";
 text = command + text;
-actual_column += strlen(command);
+offset += strlen(command);
   }
   lldb::SBStringList matches;
   lldb::SBStringList descriptions;
-  g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions(
-  text.c_str(), actual_column, 0, -1, matches, descriptions);
-  size_t count = std::min((uint32_t)100, matches.GetSize());
-  targets.reserve(count);
-  for (size_t i = 0; i < count; i++) {
-std::strin

[Lldb-commits] [PATCH] D154030: [lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands.

2023-07-05 Thread John Harrison via Phabricator via lldb-commits

ashgti updated this revision to Diff 537538.
ashgti added a comment.

Removing a log statement that is not needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154030/new/

https://reviews.llvm.org/D154030

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/Options.td
  lldb/tools/lldb-vscode/README.md
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -191,6 +191,27 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+// Send a "continued" event to indicate the process is in the running state.
+void SendContinuedEvent() {
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  if (!process.IsValid()) {
+return;
+  }
+
+  // If the focus thread is not set then we haven't reported any thread status
+  // to the client, so nothing to report.
+  if (g_vsc.focus_tid == LLDB_INVALID_THREAD_ID) {
+return;
+  }
+
+  llvm::json::Object event(CreateEventObject("continued"));
+  llvm::json::Object body;
+  body.try_emplace("threadId", (int64_t)g_vsc.focus_tid);  
+  body.try_emplace("allThreadsContinued", true);
+  event.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(event)));
+}
+
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
@@ -252,7 +273,7 @@
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // We will have cleared g_vsc.focus_tid if the focus thread doesn't have
   // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
   // then set the focus thread to the first thread with a stop reason.
   if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
@@ -468,6 +489,7 @@
 break;
   case lldb::eStateRunning:
 g_vsc.WillContinue();
+SendContinuedEvent();
 break;
   case lldb::eStateExited:
 // When restarting, we can get an "exited" event for the process we
@@ -767,9 +789,10 @@
   FillResponse(request, response);
   lldb::SBProcess process = g_vsc.target.GetProcess();
   auto arguments = request.getObject("arguments");
-  // Remember the thread ID that caused the resume so we can set the
-  // "threadCausedFocus" boolean value in the "stopped" events.
-  g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+  // Invalidate the focused, continuing the process may land on any thread, not
+  // just the current thread in which case we want the client to update focus to
+  // the new stopped thread.
+  g_vsc.focus_tid = LLDB_INVALID_THREAD_ID;
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
@@ -1065,50 +1088,62 @@
   FillResponse(request, response);
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
+
+  // If we have a frame, try to set the context for variable completions.
+  lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
+  if (frame.IsValid()) {
+frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
+frame.GetThread().SetSelectedFrame(frame.GetFrameID());
+  }
+
   std::string text = std::string(GetString(arguments, "text"));
   auto original_column = GetSigned(arguments, "column", text.size());
-  auto actual_column = original_column - 1;
+  auto original_line = GetSigned(arguments, "line", 1);
+  auto offset = original_column - 1;
+  if (original_line > 1) {
+llvm::StringRef text_ref{text};
+::llvm::SmallVector<::llvm::StringRef, 2> lines;
+text_ref.split(lines, '\n');
+for (int i = 0; i < original_line - 1; i++) {
+  offset += lines[i].size();
+}
+  }
   llvm::json::Array targets;
-  // NOTE: the 'line' argument is not needed, as multiline expressions
-  // work well already
-  // TODO: support frameID. Currently
-  // g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions
-  // is frame-unaware.
-
-  if (!text.empty() && text[0] == '`') {
-text = text.substr(1);
-actual_column--;
-  } else {
-char command[] = "expression -- ";
+
+  if (g_vsc.DetectExpressionContext(frame, text) == ExpressionContext::Variable) {
+char command[] = "frame variable ";
 text = command + text;
-actual_column += strlen(command);
+offset += strlen(command);
   }
   lldb::SBStringList matches;
   lldb::SBStringList descriptions;
-  g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions(
-  text.c_str(), actual_column, 0, -1, matches, descriptions);
-  size_t count = std::min((uint32_t)100, matches.GetSize());
-  targets.reserve(count);
-  for (size_t i = 0; i < count;

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Jim Ingham via Phabricator via lldb-commits

jingham updated this revision to Diff 537544.
jingham added a comment.

Address review comments:

Made the in_process target list shared pointers.

Removed sstream includes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154542/new/

https://reviews.llvm.org/D154542

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
 return error;
   }
   target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+  debugger.GetTargetList().RegisterInProcessTarget(target_sp);
   target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
   if (user_exe_path_is_bundle)
 exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
 // No file was specified, just create an empty target with any arch if a
 // valid arch was specified
 target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp);
   }
 
   if (!target_sp)
@@ -513,6 +515,7 @@
 void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
   lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
  "target already exists it the list");
+  UnregisterInProcessTarget(target_sp);
   m_target_list.push_back(std::move(target_sp));
   if (do_select)
 SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
 m_selected_target_idx = 0;
   return GetTargetAtIndex(m_selected_target_idx);
 }
+
+bool TargetList::AnyTargetContainsModule(Module &module) {
+  std::lock_guard guard(m_target_list_mutex);
+  for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+  return true;
+  }
+  for (const auto &target_sp: m_in_process_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+  return true;
+  }
+  return false;
+}
+
+  void TargetList::RegisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target_sp);
+assert(was_added && "Target pointer was left in the in-process map");
+  }
+  
+  void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target_sp);
+assert(was_present && "Target pointer being removed was not registered");
+  }
+  
+  bool TargetList::IsTargetInProcess(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target_sp) == 1; 
+  }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
 // each library in parallel.
 if (GetPreloadSymbols())
   module_sp->PreloadSymbols();
-
 llvm::SmallVector replaced_modules;
 for (ModuleSP &old_module_sp : old_modules) {
   if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
 }
 
 bool TargetProperties::GetPreloadSymbols() const {
+  if (INTERRUPT_REQUESTED(m_target->GetDebugger(), 
+  "Interrupted checking preload symbols")) {
+return false;
+  }
   const uint32_t idx = ePropertyPreloadSymbols;
   return GetPropertyAtIndexAs(
   idx, g_target_properties[idx].default_uint_value != 0);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -509,11 +509,11 @@
 } else {
   // Check for interruption when building the frames.
   // Do the check in idx > 0 so that we'll always create a 0th frame.
-  if (allow_interrupt && dbg.InterruptRequested()) {
-Log *log = GetLog(LLDBLog::Host);
-LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
-was_interrupted = true;
-break;
+  if (allow_interrupt 
+  && INTERRUPT_REQUESTED(dbg, "Interrupted having fetched {0} frames",
+ m_frames.size())) {
+  was_interrupted = true;
+  break;
   }

[Lldb-commits] [PATCH] D154386: [lldb][NFCI] Remove use of ConstString from OptionValue

2023-07-05 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154386/new/

https://reviews.llvm.org/D154386

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

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



Comment at: lldb/include/lldb/Core/Debugger.h:430-431
+  template 
+  bool InterruptRequested(const char *cur_func, 
+  const char *formatv, Args &&... args) {
+bool ret_val = InterruptRequested();

bulbazord wrote:
> I think it would make more sense to have `cur_func` and `formatv` be of type 
> `llvm::StringRef`. Concretely, if you construct a `std::string` from 
> `nullptr` (like you're doing below when you make an InterruptionReport 
> object), you will crash.
> 
> I know the recommendation is to use `INTERRUPT_REQUESTED` instead of filling 
> this manually, but inevitably somebody will go against the advice and make a 
> mistake.
> I think it would make more sense to have `cur_func` and `formatv` be of type 
> `llvm::StringRef`. Concretely, if you construct a `std::string` from 
> `nullptr` (like you're doing below when you make an InterruptionReport 
> object), you will crash.
> 
> I know the recommendation is to use `INTERRUPT_REQUESTED` instead of filling 
> this manually, but inevitably somebody will go against the advice and make a 
> mistake.

I don't see how I can make the formatv option a StringRef.  The llvm::formatv 
API only offers a version that takes a `const char *`.  Anyway, these are 
formatv strings, they are almost universally going to be const strings.  
Turning them into llvm::StringRef's and back out again to use in llvm::formatv 
seems odd.

But you are right, we should protect against someone passing in a null pointer 
for the function or format to InterruptRequested.   Since this is just logging, 
an assert seems overkill, I'll just add null pointer checks here and turn them 
into "UNKNOWN FUNCTION" and "Unknown message".



Comment at: lldb/source/API/SBFrame.cpp:57-58
 
+#include 
+
 using namespace lldb;

mib wrote:
> bulbazord wrote:
> > What is this used for?
> Is this still necessary ?
Not sure what you are asking here.  We use StackFrameSP w/o saying 
lldb::StackFrameSP and we use VariableList for instance rather than 
lldb_private::VariableList.  We could remove the using statements here but 
that's not how we do it in general.  In some cases we don't do `using namespace 
lldb` but that's mostly in files that use clang API's heavily since those 
conflict with some clang type names and it was good to be explicit there.  But 
I don't think we want to be verbose like that in the SB files.



Comment at: lldb/source/Target/StackFrameList.cpp:31
 #include 
+#include 
 

bulbazord wrote:
> Where is this used?
Dunno, but this seems more like an orthogonal cleanup unrelated to the current 
patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154542/new/

https://reviews.llvm.org/D154542

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

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Jim Ingham via Phabricator via lldb-commits

jingham updated this revision to Diff 537551.
jingham added a comment.

Protect InterruptRequested from null function & format strings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154542/new/

https://reviews.llvm.org/D154542

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -325,6 +325,7 @@
 return error;
   }
   target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+  debugger.GetTargetList().RegisterInProcessTarget(target_sp);
   target_sp->SetExecutableModule(exe_module_sp, load_dependent_files);
   if (user_exe_path_is_bundle)
 exe_module_sp->GetFileSpec().GetPath(resolved_bundle_exe_path,
@@ -336,6 +337,7 @@
 // No file was specified, just create an empty target with any arch if a
 // valid arch was specified
 target_sp.reset(new Target(debugger, arch, platform_sp, is_dummy_target));
+debugger.GetTargetList().RegisterInProcessTarget(target_sp);
   }
 
   if (!target_sp)
@@ -513,6 +515,7 @@
 void TargetList::AddTargetInternal(TargetSP target_sp, bool do_select) {
   lldbassert(!llvm::is_contained(m_target_list, target_sp) &&
  "target already exists it the list");
+  UnregisterInProcessTarget(target_sp);
   m_target_list.push_back(std::move(target_sp));
   if (do_select)
 SetSelectedTargetInternal(m_target_list.size() - 1);
@@ -540,3 +543,35 @@
 m_selected_target_idx = 0;
   return GetTargetAtIndex(m_selected_target_idx);
 }
+
+bool TargetList::AnyTargetContainsModule(Module &module) {
+  std::lock_guard guard(m_target_list_mutex);
+  for (const auto &target_sp : m_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+  return true;
+  }
+  for (const auto &target_sp: m_in_process_target_list) {
+if (target_sp->GetImages().FindModule(&module))
+  return true;
+  }
+  return false;
+}
+
+  void TargetList::RegisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+std::unordered_set::iterator iter;
+bool was_added;
+std::tie(iter, was_added) = m_in_process_target_list.insert(target_sp);
+assert(was_added && "Target pointer was left in the in-process map");
+  }
+  
+  void TargetList::UnregisterInProcessTarget(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+bool was_present = m_in_process_target_list.erase(target_sp);
+assert(was_present && "Target pointer being removed was not registered");
+  }
+  
+  bool TargetList::IsTargetInProcess(TargetSP target_sp) {
+std::lock_guard guard(m_target_list_mutex);
+return m_in_process_target_list.count(target_sp) == 1; 
+  }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2236,7 +2236,6 @@
 // each library in parallel.
 if (GetPreloadSymbols())
   module_sp->PreloadSymbols();
-
 llvm::SmallVector replaced_modules;
 for (ModuleSP &old_module_sp : old_modules) {
   if (m_images.GetIndexForModule(old_module_sp.get()) !=
@@ -4205,6 +4204,10 @@
 }
 
 bool TargetProperties::GetPreloadSymbols() const {
+  if (INTERRUPT_REQUESTED(m_target->GetDebugger(), 
+  "Interrupted checking preload symbols")) {
+return false;
+  }
   const uint32_t idx = ePropertyPreloadSymbols;
   return GetPropertyAtIndexAs(
   idx, g_target_properties[idx].default_uint_value != 0);
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -509,11 +509,11 @@
 } else {
   // Check for interruption when building the frames.
   // Do the check in idx > 0 so that we'll always create a 0th frame.
-  if (allow_interrupt && dbg.InterruptRequested()) {
-Log *log = GetLog(LLDBLog::Host);
-LLDB_LOG(log, "Interrupted %s", __FUNCTION__);
-was_interrupted = true;
-break;
+  if (allow_interrupt 
+  && INTERRUPT_REQUESTED(dbg, "Interrupted having fetched {0} frames",
+ m_frames.size())) {
+  was_interrupted = true;
+  break;
   }
 
   const bool success =
@@ -965,11 +9

[Lldb-commits] [PATCH] D154542: Further refinements on reporting interruption in lldb

2023-07-05 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



Comment at: lldb/include/lldb/Core/Debugger.h:455-456
+InterruptionReport(std::string function_name, std::string description) :
+m_function_name(function_name), 
+m_description(description),
+m_interrupt_time(std::chrono::system_clock::now()),

bulbazord wrote:
> To avoid some unnecessary copies
> 
> Could also do what Ismail is suggesting.
This is a local that is copied to an ivar and never used again.  Do I really 
have to put move in there explicitly for the optimizer to know it can reuse the 
value?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154542/new/

https://reviews.llvm.org/D154542

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

61 matches

Mail list logo