date:20230630

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

2023-06-30 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added a comment.

Making `m_lock_count`'s type into `std::atomic` makes sense to me, 
but I'm a little confused about why `Process::LoadOperatingSystemPlugin` is 
guarded by acquiring `m_thread_mutex`. My (admittedly limited) understanding of 
that is that it's a mutex that the Process holds for the ThreadList to manage 
concurrent modifications to the thread list. Is loading an Operating System 
plugin related to modifying the ThreadList? If not, perhaps it would be better 
served by having its own mutex?


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

https://reviews.llvm.org/D154271

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

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

2023-06-30 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 536514.
mib edited the summary of this revision.
mib added a comment.

Address @JDevlieghere comment.


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

https://reviews.llvm.org/D154271

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -421,7 +421,7 @@
   bool m_session_is_active;
   bool m_pty_secondary_is_open;
   bool m_valid_session;
-  uint32_t m_lock_count;
+  std::atomic m_lock_count;
   PyThreadState *m_command_thread_state;
 };
 


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -421,7 +421,7 @@
   bool m_session_is_active;
   bool m_pty_secondary_is_open;
   bool m_valid_session;
-  uint32_t m_lock_count;
+  std::atomic m_lock_count;
   PyThreadState *m_command_thread_state;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

2023-06-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Does this have to be a recursive mutex? Can we simplify this by using a 
`std::atomic` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154271

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

[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld

2023-06-30 Thread Jason Molenda via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG624813a4f41c: Change the dyld notification function that 
lldb puts a breakpoint in (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139453

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

Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -235,10 +235,15 @@
  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 Arg2: unsigned long icount- Number of shared libraries
-  // added/removed Arg3: uint64_t mach_headers[] - Array of load addresses
-  // of binaries added/removed
+  //
+  // 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
 
   DynamicLoaderMacOS *dyld_instance = (DynamicLoaderMacOS *)baton;
 
@@ -268,9 +273,10 @@
 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_removing=1, dyld_notify_remove_all=2,
+ // dyld_notify_dyld_moved=3 };
 Value count_value;   // unsigned long count
-Value headers_value; // uint64_t machHeaders[] (aka void*)
+Value headers_value; // struct dyld_image_info machHeaders[]
 
 CompilerType clang_void_ptr_type =
 scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
@@ -299,6 +305,9 @@
 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);
@@ -312,12 +321,32 @@
   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 addr = process->ReadUnsignedIntegerFromMemory(
-  header_array + (8 * i), 8, LLDB_INVALID_ADDRESS, error);
-  if (addr != LLDB_INVALID_ADDRESS) {
+  addr_t dyld_image_info = header_array + (3 * addr_size * i);
+  addr_t addr =
+  process->ReadPointerFromMemory(dyld_image_info, error);
+  if (error.Success()) {
 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) {
@@ -362,10 +391,18 @@
   Status error;
   addr_t notification_addr =
   process->ReadPointerFromMemory(notification_location, error);
-  if (ABISP abi_sp = process->GetABI())
-notification_addr = abi_sp->FixCodeAddress(notification_addr);
+  if (!error.Success()) {
+Debugger::ReportWarning(
+"DynamicLoaderMacOS::NotifyBreakpointHit unable "
+"to read address of dyld-handover notification function at "
+"0x%" PRIx64,
+notification_location);
+  } else {
+if (ABISP abi_sp = p

[Lldb-commits] [lldb] 624813a - Change the dyld notification function that lldb puts a breakpoint in

2023-06-30 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2023-06-30T18:29:46-07:00
New Revision: 624813a4f41c5945dc8f8d998173960ad75db731

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

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

On Darwin systems, the dynamic linker dyld has an empty function
it calls when binaries are added/removed from the process.  lldb puts
a breakpoint on this dyld function to catch the notifications.  The
function arguments are used by lldb to tell what is happening.

The linker has a natural representation when the addresses of
binaries being added/removed are in the pointer size of the process.
There is then a second function where the addresses of the binaries
are in a uint64_t array, which the debugger has been using before -
dyld allocates memory for the array, copies the values in to it,
and calls it for lldb's benefit.

This changes to using the native notifier function, with pointer-sized
addresses.

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

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 67e79fdcec8f56..f439fa88fc7345 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -235,10 +235,15 @@ 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 Arg2: unsigned long icount- Number of shared libraries
-  // added/removed Arg3: uint64_t mach_headers[] - Array of load addresses
-  // of binaries added/removed
+  //
+  // 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
 
   DynamicLoaderMacOS *dyld_instance = (DynamicLoaderMacOS *)baton;
 
@@ -268,9 +273,10 @@ 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_removing=1, dyld_notify_remove_all=2,
+ // dyld_notify_dyld_moved=3 };
 Value count_value;   // unsigned long count
-Value headers_value; // uint64_t machHeaders[] (aka void*)
+Value headers_value; // struct dyld_image_info machHeaders[]
 
 CompilerType clang_void_ptr_type =
 scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
@@ -299,6 +305,9 @@ 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);
@@ -312,12 +321,32 @@ 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 addr = process->ReadUnsignedIntegerFromMemory(
-  header_array + (8 * i), 8, LLDB_INVALID_ADDRESS, error);
-  if (addr != LLDB_INVALID_ADDRESS) {
+  addr_t dyld_image_info = header_a

[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld

2023-06-30 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda updated this revision to Diff 536492.
jasonmolenda added a comment.

Update patch to address Jim and Jonas' feedback, most importantly report a 
warning if we can't read one of the mach-o binary addresses that dyld is 
telling us are being added/removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139453

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

Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -235,10 +235,15 @@
  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 Arg2: unsigned long icount- Number of shared libraries
-  // added/removed Arg3: uint64_t mach_headers[] - Array of load addresses
-  // of binaries added/removed
+  //
+  // 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
 
   DynamicLoaderMacOS *dyld_instance = (DynamicLoaderMacOS *)baton;
 
@@ -268,9 +273,10 @@
 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_removing=1, dyld_notify_remove_all=2,
+ // dyld_notify_dyld_moved=3 };
 Value count_value;   // unsigned long count
-Value headers_value; // uint64_t machHeaders[] (aka void*)
+Value headers_value; // struct dyld_image_info machHeaders[]
 
 CompilerType clang_void_ptr_type =
 scratch_ts_sp->GetBasicType(eBasicTypeVoid).GetPointerType();
@@ -299,6 +305,9 @@
 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);
@@ -312,12 +321,32 @@
   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 addr = process->ReadUnsignedIntegerFromMemory(
-  header_array + (8 * i), 8, LLDB_INVALID_ADDRESS, error);
-  if (addr != LLDB_INVALID_ADDRESS) {
+  addr_t dyld_image_info = header_array + (3 * addr_size * i);
+  addr_t addr =
+  process->ReadPointerFromMemory(dyld_image_info, error);
+  if (error.Success()) {
 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) {
@@ -362,10 +391,18 @@
   Status error;
   addr_t notification_addr =
   process->ReadPointerFromMemory(notification_location, error);
-  if (ABISP abi_sp = process->GetABI())
-notification_addr = abi_sp->FixCodeAddress(notification_addr);
+  if (!error.Success()) {
+Debugger::ReportWarning(
+"DynamicLoaderMacOS::NotifyBreakpointHit unable "
+"to read address of dyld-handover notification function at "
+"0x%" PRIx64,
+notification_location

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

2023-06-30 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

I have to agree with David, I don't see how the old code could overflow if 
DW_OP_deref_size's maximum size is the pointer size in the target, and we are 
reading it in to an 8-byte buffer, unless the target had addresses larger than 
8 bytes, or the dwarf was malformed. The DWARF5 doc says

"In the DW_OP_deref_size operation, however, the size in bytes of the data 
retrieved from the dereferenced address is specified by
the single operand. This operand is a 1-byte unsigned integral constant whose 
value may not be larger than the size of the generic type. The data retrieved 
is zero extended to the size of an address on the target machine before being 
pushed onto the expression stack"

If there was an invalid dwarf input file that had a DW_OP_deref_size with a 
size of 16, we will only read the first 8 bytes of it into our buffer, and then 
try to extract 16 bytes with a call to `DerefSizeExtractDataHelper`, which will 
only read an Address sized object out of the buffer so it won't exceed the 
length.

I think this change may be hiding a real bug in the DWARF @cmtice ?  Do you 
have a record of what the input file that caused the address sanitizer to trip 
was?


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] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Dave Lee via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb43375bb8195: [lldb] Add log indicating which kind of data 
formatter (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154128

Files:
  lldb/source/DataFormatters/FormatManager.cpp


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,15 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message) "[%s] " Message, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +614,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", 
__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Language search success. Returning."));
   return retval_sp;
 }
 }
   }
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving hardcoded a chance."));
   return GetHardcoded(match_data);
 }
 
@@ -628,24 +635,23 @@
   ImplSP retval_sp;
   Log *log = GetLog(LLDBLog::DataFormatters);
   if (match_data.GetTypeForCache()) {
-LLDB_LOGF(log, "\n\n[%s] Looking into cache for type %s", __FUNCTION__,
+LLDB_LOGF(log, "\n\n" FORMAT_LOG("Looking into cache for type %s"),
   match_data.GetTypeForCache().AsCString(""));
 if (m_format_cache.Get(match_data.GetTypeForCache(), retval_sp)) {
   if (log) {
-LLDB_LOGF(log, "[%s] Cache search success. Returning.", __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search success. Returning."));
 LLDB_LOGV(log, "Cache hits: {0} - Cache Misses: {1}",
   m_format_cache.GetCacheHits(),
   m_format_cache.GetCacheMisses());
   }
   return retval_sp;
 }
-LLDB_LOGF(log, "[%s] Cache search failed. Going normal route",
-  __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search failed. Going normal route"));
   }
 
   m_categories_map.Get(match_data, retval_sp);
   if (match_data.GetTypeForCache() && (!retval_sp || 
!retval_sp->NonCacheable())) {
-LLDB_LOGF(log, "[%s] Caching %p for type %s", __FUNCTION__,
+LLDB_LOGF(log, FORMAT_LOG("Caching %p for type %s"),
   static_cast(retval_sp.get()),
   match_data.GetTypeForCache().AsCString(""));
 m_format_cache.Set(match_data.GetTypeForCache(), retval_sp);
@@ -655,6 +661,8 @@
   return retval_sp;
 }
 
+#undef FORMAT_LOG
+
 lldb::TypeFormatImplSP
 FormatManager::GetFormat(ValueObject &valobj,
  lldb::DynamicValueType use_dynamic) {


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,15 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message) "[%s] " Message, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +614,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", __FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Language search success. Returning."));
   return retval_sp;

[Lldb-commits] [lldb] b43375b - [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Dave Lee via lldb-commits


Author: Dave Lee
Date: 2023-06-30T16:51:14-07:00
New Revision: b43375bb8195bd451850f42b7b99548aa1ba43fd

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

LOG: [lldb] Add log indicating which kind of data formatter

The `formatter` logs include a function name, but these functions are mostly 
templates
and the template type parameter is not printed, which is useful context.

This change adds a new log which is printed upon entry of `FormatManager::Get`, 
which
shows the formatter context as either `format`, `summary`, or `synthetic`.

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

Added: 


Modified: 
lldb/source/DataFormatters/FormatManager.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/FormatManager.cpp 
b/lldb/source/DataFormatters/FormatManager.cpp
index 4dedf7ea73e160..f1f135de32ca87 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,15 @@ ImplSP FormatManager::GetHardcoded(FormattersMatchData 
&match_data) {
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message) "[%s] " Message, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +614,19 @@ ImplSP FormatManager::Get(ValueObject &valobj,
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", 
__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Language search success. Returning."));
   return retval_sp;
 }
 }
   }
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving hardcoded a chance."));
   return GetHardcoded(match_data);
 }
 
@@ -628,24 +635,23 @@ ImplSP FormatManager::GetCached(FormattersMatchData 
&match_data) {
   ImplSP retval_sp;
   Log *log = GetLog(LLDBLog::DataFormatters);
   if (match_data.GetTypeForCache()) {
-LLDB_LOGF(log, "\n\n[%s] Looking into cache for type %s", __FUNCTION__,
+LLDB_LOGF(log, "\n\n" FORMAT_LOG("Looking into cache for type %s"),
   match_data.GetTypeForCache().AsCString(""));
 if (m_format_cache.Get(match_data.GetTypeForCache(), retval_sp)) {
   if (log) {
-LLDB_LOGF(log, "[%s] Cache search success. Returning.", __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search success. Returning."));
 LLDB_LOGV(log, "Cache hits: {0} - Cache Misses: {1}",
   m_format_cache.GetCacheHits(),
   m_format_cache.GetCacheMisses());
   }
   return retval_sp;
 }
-LLDB_LOGF(log, "[%s] Cache search failed. Going normal route",
-  __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search failed. Going normal route"));
   }
 
   m_categories_map.Get(match_data, retval_sp);
   if (match_data.GetTypeForCache() && (!retval_sp || 
!retval_sp->NonCacheable())) {
-LLDB_LOGF(log, "[%s] Caching %p for type %s", __FUNCTION__,
+LLDB_LOGF(log, FORMAT_LOG("Caching %p for type %s"),
   static_cast(retval_sp.get()),
   match_data.GetTypeForCache().AsCString(""));
 m_format_cache.Set(match_data.GetTypeForCache(), retval_sp);
@@ -655,6 +661,8 @@ ImplSP FormatManager::GetCached(FormattersMatchData 
&match_data) {
   return retval_sp;
 }
 
+#undef FORMAT_LOG
+
 lldb::TypeFormatImplSP
 FormatManager::GetFormat(ValueObject &valobj,
  lldb::DynamicValueType use_dynamic) {



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

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

2023-06-30 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

This patch should fix some data races when a python script (i.e. a
Scripted Process) has an indirect nested call to another python script (i.e. a
OperatingSystem Plugin), which can cause concurrent writes to the python
lock count.

This patch addresses that issue by guarding the accesses by a recursive mutex.

rdar://109413039

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154271

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -380,9 +380,13 @@
 
   uint32_t IsExecutingPython() const { return m_lock_count > 0; }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +426,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::recursive_mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2467,6 +2467,7 @@
 }
 
 void Process::LoadOperatingSystemPlugin(bool flush) {
+  std::lock_guard guard(m_thread_mutex);
   if (flush)
 m_thread_list.Clear();
   m_os_up.reset(OperatingSystem::FindPlugin(this, nullptr));
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -380,9 +380,13 @@
 
   uint32_t IsExecutingPython() const { return m_lock_count > 0; }
 
-  uint32_t IncrementLockCount() { return ++m_lock_count; }
+  uint32_t IncrementLockCount() {
+std::lock_guard guard(m_mutex);
+return ++m_lock_count;
+  }
 
   uint32_t DecrementLockCount() {
+std::lock_guard guard(m_mutex);
 if (m_lock_count > 0)
   --m_lock_count;
 return m_lock_count;
@@ -422,6 +426,7 @@
   bool m_pty_secondary_is_open;
   bool m_valid_session;
   uint32_t m_lock_count;
+  std::recursive_mutex m_mutex;
   PyThreadState *m_command_thread_state;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154201: [lldb][AArch64] Fix tagged watch test on Graviton 3

2023-06-30 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

Interesting.  I remember reading in the recent arm arm docs about how there may 
be false watchpoint notifications when a processor is in SVE Streaming Mode and 
a write happens near a watchpoint, but I didn't realize that could happen 
outside of that context.  I'm working on using power-of-2 mask watchpoints on 
Darwin and one of the problems I'll need to handle is similar, where a user 
watches a 48 byte object, I need to watch 64 bytes and ignore writes to the 
other 16.  My current thinking is to add a new type of watchpoint in addition 
to "read" and "write" -- "modify", where lldb will compare the bytes of the 
watched object and silently continue if they are unmodified.  I suspect that's 
what most people using write watchpoints would actually want, outside of this 
issue of false watchpoint hits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154201

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

[Lldb-commits] [PATCH] D154204: [lldb][AArch64] Account for extra libc frames in PAC unwind test

2023-06-30 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

Interesting, libc.so must have stripped the names of non-externally-visible 
functions, and libc_start_main calls one of them which calls main in turn.  
This change to the test case looks correct to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154204

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

[Lldb-commits] [PATCH] D154265: [lldb][NFC] Simplify GetLocation_DW_OP_addr function

2023-06-30 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.

this looks good to me.  I'm a little overly paranoid about specifying when 
things are file addresses versus load addresses -- I'm sure that anyone working 
in this context would know these are file addresses being returned, but I would 
probably say that in the prototype doc strings e.g. "Return the file address 
specified".  That's more of a personal style thing than anything important tho.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154265

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

[Lldb-commits] [PATCH] D154268: [lldb] Skip apple accelerator table test in DWARF 5 mode

2023-06-30 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/D154268/new/

https://reviews.llvm.org/D154268

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

[Lldb-commits] [PATCH] D153834: [lldb] Add two-level caching in the source manager

2023-06-30 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

This looks like a good change to me.  I don't have any comments over Alex's 
feedback.


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

https://reviews.llvm.org/D153834

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

[Lldb-commits] [PATCH] D154268: [lldb] Skip apple accelerator table test in DWARF 5 mode

2023-06-30 Thread Felipe de Azevedo Piovezan via Phabricator via lldb-commits

fdeazeve updated this revision to Diff 536450.
fdeazeve added a comment.
Herald added a subscriber: JDevlieghere.

fix typo in commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154268

Files:
  lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py


Index: lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
===
--- lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -7,6 +7,7 @@
 class CPPAcceleratorTableTestCase(TestBase):
 @skipUnlessDarwin
 @skipIf(debug_info=no_match(["dwarf"]))
+@skipIf(dwarf_version=[">=", "5"])
 def test(self):
 """Test that type lookups fail early (performance)"""
 self.build()


Index: lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
===
--- lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -7,6 +7,7 @@
 class CPPAcceleratorTableTestCase(TestBase):
 @skipUnlessDarwin
 @skipIf(debug_info=no_match(["dwarf"]))
+@skipIf(dwarf_version=[">=", "5"])
 def test(self):
 """Test that type lookups fail early (performance)"""
 self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154268: [lldb] Skip apple accelerator table test in DWARF 5 mode

2023-06-30 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.

D68678  added a test that ensure an Apple 
accelerator lookup is done efficiently.
Since these tables are not used for DWARF 5, we should decorate the test
appropriately.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154268

Files:
  lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py


Index: lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
===
--- lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -7,6 +7,7 @@
 class CPPAcceleratorTableTestCase(TestBase):
 @skipUnlessDarwin
 @skipIf(debug_info=no_match(["dwarf"]))
+@skipIf(dwarf_version=[">=", "5"])
 def test(self):
 """Test that type lookups fail early (performance)"""
 self.build()


Index: lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
===
--- lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
+++ lldb/test/API/lang/cpp/accelerator-table/TestCPPAccelerator.py
@@ -7,6 +7,7 @@
 class CPPAcceleratorTableTestCase(TestBase):
 @skipUnlessDarwin
 @skipIf(debug_info=no_match(["dwarf"]))
+@skipIf(dwarf_version=[">=", "5"])
 def test(self):
 """Test that type lookups fail early (performance)"""
 self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 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/D154128/new/

https://reviews.llvm.org/D154128

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

[Lldb-commits] [PATCH] D154265: [lldb][NFC] Simplify GetLocation_DW_OP_addr function

2023-06-30 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.

A very old commit (9422dd64f870dd33) changed the signature of this function in a
number of ways. This patch aims to improve it:

1. Reword the documentation, which still mentions old parameters that no longer

exist, and to elaborate upon the behavior of this function.

2. Remove the unnecessary parameter `op_addr_idx`. This parameter is odd in a

couple of ways: we never use it with a value that is non-zero, and the matching
`Update_DW_OP_addr` function doesn't use a similar parameter. We also document
that this new behavior. If we ever decide to handle multiple "DW_OP_addr", we
can introduce the complexity again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154265

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/source/Expression/DWARFExpression.cpp
  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
@@ -3457,8 +3457,8 @@
   bool op_error = false;
   const DWARFExpression* location = location_list.GetAlwaysValidExpr();
   if (location)
-location_DW_OP_addr = location->GetLocation_DW_OP_addr(
-location_form.GetUnit(), 0, op_error);
+location_DW_OP_addr =
+location->GetLocation_DW_OP_addr(location_form.GetUnit(), op_error);
   if (op_error) {
 StreamString strm;
 location->DumpLocation(&strm, eDescriptionLevelFull, nullptr);
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -355,39 +355,28 @@
 }
 
 lldb::addr_t DWARFExpression::GetLocation_DW_OP_addr(const DWARFUnit *dwarf_cu,
- uint32_t op_addr_idx,
  bool &error) const {
   error = false;
   lldb::offset_t offset = 0;
-  uint32_t curr_op_addr_idx = 0;
   while (m_data.ValidOffset(offset)) {
 const uint8_t op = m_data.GetU8(&offset);
 
-if (op == DW_OP_addr) {
-  const lldb::addr_t op_file_addr = m_data.GetAddress(&offset);
-  if (curr_op_addr_idx == op_addr_idx)
-return op_file_addr;
-  ++curr_op_addr_idx;
-} else if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
+if (op == DW_OP_addr)
+  return m_data.GetAddress(&offset);
+if (op == DW_OP_GNU_addr_index || op == DW_OP_addrx) {
   uint64_t index = m_data.GetULEB128(&offset);
-  if (curr_op_addr_idx == op_addr_idx) {
-if (!dwarf_cu) {
-  error = true;
-  break;
-}
-
+  if (dwarf_cu)
 return dwarf_cu->ReadAddressFromDebugAddrSection(index);
-  }
-  ++curr_op_addr_idx;
-} else {
-  const offset_t op_arg_size =
-  GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
-  if (op_arg_size == LLDB_INVALID_OFFSET) {
-error = true;
-break;
-  }
-  offset += op_arg_size;
+  error = true;
+  break;
+}
+const offset_t op_arg_size =
+GetOpcodeDataSize(m_data, offset, op, dwarf_cu);
+if (op_arg_size == LLDB_INVALID_OFFSET) {
+  error = true;
+  break;
 }
+offset += op_arg_size;
   }
   return LLDB_INVALID_ADDRESS;
 }
Index: lldb/include/lldb/Expression/DWARFExpression.h
===
--- lldb/include/lldb/Expression/DWARFExpression.h
+++ lldb/include/lldb/Expression/DWARFExpression.h
@@ -50,30 +50,22 @@
   /// Return true if the location expression contains data
   bool IsValid() const;
 
-  /// If a location is not a location list, return true if the location
-  /// contains a DW_OP_addr () opcode in the stream that matches \a file_addr.
-  /// If file_addr is LLDB_INVALID_ADDRESS, the this function will return true
-  /// if the variable there is any DW_OP_addr in a location that (yet still is
-  /// NOT a location list). This helps us detect if a variable is a global or
-  /// static variable since there is no other indication from DWARF debug
-  /// info.
+  /// Return the address specified by the first
+  /// DW_OP_{addr, addrx, GNU_addr_index} in the operation stream.
   ///
   /// \param[in] dwarf_cu
-  /// The dwarf unit this expression belongs to.
-  ///
-  /// \param[in] op_addr_idx
-  /// The DW_OP_addr index to retrieve in case there is more than
-  /// one DW_OP_addr opcode in the location byte stream.
+  /// The dwarf unit this expression belongs to. Only required to resolve
+  /// DW_OP{addrx, GNU_addr_index}.
   ///
   /// \param[out] error

[Lldb-commits] [PATCH] D153802: [lldb] Delete RewriteObjCClassReferences (NFC)

2023-06-30 Thread Dave Lee via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c37cbef5821: [lldb] Delete RewriteObjCClassReferences (NFC) 
(authored by kastiglione).

Changed prior to commit:
  https://reviews.llvm.org/D153802?vs=534693&id=536439#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153802

Files:
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
  lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h

Index: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
===
--- lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
+++ lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
@@ -252,28 +252,6 @@
   /// True on success; false otherwise
   bool RewriteObjCSelectors(llvm::BasicBlock &basic_block);
 
-  /// A basic block-level pass to find all Objective-C class references that
-  /// use the old-style Objective-C runtime and rewrite them to use
-  /// class_getClass instead of statically allocated class references.
-
-  /// Replace a single old-style class reference
-  ///
-  /// \param[in] class_load
-  /// The load of the statically-allocated selector.
-  ///
-  /// \return
-  /// True on success; false otherwise
-  bool RewriteObjCClassReference(llvm::Instruction *class_load);
-
-  /// The top-level pass implementation
-  ///
-  /// \param[in] basic_block
-  /// The basic block currently being processed.
-  ///
-  /// \return
-  /// True on success; false otherwise
-  bool RewriteObjCClassReferences(llvm::BasicBlock &basic_block);
-
   /// A basic block-level pass to find all newly-declared persistent
   /// variables and register them with the ClangExprDeclMap.  This allows them
   /// to be materialized and dematerialized like normal external variables.
@@ -425,9 +403,6 @@
   /// The address of the function sel_registerName, cast to the appropriate
   /// function pointer type.
   llvm::FunctionCallee m_sel_registerName;
-  /// The address of the function objc_getClass, cast to the appropriate
-  /// function pointer type.
-  llvm::FunctionCallee m_objc_getClass;
   /// The type of an integer large enough to hold a pointer.
   llvm::IntegerType *m_intptr_ty = nullptr;
   /// The stream on which errors should be printed.
Index: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -859,132 +859,6 @@
   return true;
 }
 
-static bool IsObjCClassReference(Value *value) {
-  GlobalVariable *global_variable = dyn_cast(value);
-
-  return !(!global_variable || !global_variable->hasName() ||
-   !global_variable->getName().startswith("OBJC_CLASS_REFERENCES_"));
-}
-
-// This function does not report errors; its callers are responsible.
-bool IRForTarget::RewriteObjCClassReference(Instruction *class_load) {
-  lldb_private::Log *log(GetLog(LLDBLog::Expressions));
-
-  LoadInst *load = dyn_cast(class_load);
-
-  if (!load)
-return false;
-
-  // Unpack the class name from the reference.  In LLVM IR, a reference to an
-  // Objective-C class gets represented as
-  //
-  // %tmp = load ptr, ptr @OBJC_CLASS_REFERENCES_, align 4
-  //
-  // @OBJC_CLASS_REFERENCES_ is a reference to a character array called
-  // @OBJC_CLASS_NAME_. @OBJC_CLASS_NAME contains the string.
-
-  // Find the pointer's initializer and get the string from its target
-
-  GlobalVariable *_objc_class_references_ =
-  dyn_cast(load->getPointerOperand());
-
-  if (!_objc_class_references_ ||
-  !_objc_class_references_->hasInitializer())
-return false;
-
-  // Find the string's initializer (a ConstantArray) and get the string from it
-
-  GlobalVariable *_objc_class_name_ =
-  dyn_cast(_objc_class_references_->getInitializer());
-
-  if (!_objc_class_name_ || !_objc_class_name_->hasInitializer())
-return false;
-
-  Constant *ocn_initializer = _objc_class_name_->getInitializer();
-
-  ConstantDataArray *ocn_initializer_array =
-  dyn_cast(ocn_initializer);
-
-  if (!ocn_initializer_array->isString())
-return false;
-
-  std::string ocn_initializer_string =
-  std::string(ocn_initializer_array->getAsString());
-
-  LLDB_LOG(log, "Found Objective-C class reference \"{0}\"",
-   ocn_initializer_string);
-
-  // Construct a call to objc_getClass
-
-  if (!m_objc_getClass) {
-lldb::addr_t objc_getClass_addr;
-
-bool missing_weak = false;
-static lldb_private::ConstString g_objc_getClass_str("objc_getClass");
-objc_getClass_addr = m_execution_unit.FindSymbol(g_objc_getClass_str,
- missing_weak);
-if (objc_getClass_addr == LLDB_INVALID_ADDRESS || missing_weak)
-  return false;
-
-LLDB_LOG(log, "Foun

[Lldb-commits] [lldb] 2c37cbe - [lldb] Delete RewriteObjCClassReferences (NFC)

2023-06-30 Thread Dave Lee via lldb-commits


Author: Dave Lee
Date: 2023-06-30T14:39:20-07:00
New Revision: 2c37cbef5821ae80a62c9eb7ecebd9ec25801c38

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

LOG: [lldb] Delete RewriteObjCClassReferences (NFC)

The `RewriteObjCClassReferences` pass was applicable only to the code generated 
for the
fragile ObjC ABI (v1). That ABI is no longer active (last used for i386 macOS), 
which
means this pass has no effect.

Sources: `OBJC_CLASS_REFERENCES_` is emitted only by `CGObjCMac`, and not by
`CGObjCNonFragileABIMac`.

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index bb7d860d61a2a0..1b7e86bb187f23 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -859,132 +859,6 @@ bool IRForTarget::RewriteObjCSelectors(BasicBlock 
&basic_block) {
   return true;
 }
 
-static bool IsObjCClassReference(Value *value) {
-  GlobalVariable *global_variable = dyn_cast(value);
-
-  return !(!global_variable || !global_variable->hasName() ||
-   !global_variable->getName().startswith("OBJC_CLASS_REFERENCES_"));
-}
-
-// This function does not report errors; its callers are responsible.
-bool IRForTarget::RewriteObjCClassReference(Instruction *class_load) {
-  lldb_private::Log *log(GetLog(LLDBLog::Expressions));
-
-  LoadInst *load = dyn_cast(class_load);
-
-  if (!load)
-return false;
-
-  // Unpack the class name from the reference.  In LLVM IR, a reference to an
-  // Objective-C class gets represented as
-  //
-  // %tmp = load ptr, ptr @OBJC_CLASS_REFERENCES_, align 4
-  //
-  // @OBJC_CLASS_REFERENCES_ is a reference to a character array called
-  // @OBJC_CLASS_NAME_. @OBJC_CLASS_NAME contains the string.
-
-  // Find the pointer's initializer and get the string from its target
-
-  GlobalVariable *_objc_class_references_ =
-  dyn_cast(load->getPointerOperand());
-
-  if (!_objc_class_references_ ||
-  !_objc_class_references_->hasInitializer())
-return false;
-
-  // Find the string's initializer (a ConstantArray) and get the string from it
-
-  GlobalVariable *_objc_class_name_ =
-  dyn_cast(_objc_class_references_->getInitializer());
-
-  if (!_objc_class_name_ || !_objc_class_name_->hasInitializer())
-return false;
-
-  Constant *ocn_initializer = _objc_class_name_->getInitializer();
-
-  ConstantDataArray *ocn_initializer_array =
-  dyn_cast(ocn_initializer);
-
-  if (!ocn_initializer_array->isString())
-return false;
-
-  std::string ocn_initializer_string =
-  std::string(ocn_initializer_array->getAsString());
-
-  LLDB_LOG(log, "Found Objective-C class reference \"{0}\"",
-   ocn_initializer_string);
-
-  // Construct a call to objc_getClass
-
-  if (!m_objc_getClass) {
-lldb::addr_t objc_getClass_addr;
-
-bool missing_weak = false;
-static lldb_private::ConstString g_objc_getClass_str("objc_getClass");
-objc_getClass_addr = m_execution_unit.FindSymbol(g_objc_getClass_str,
- missing_weak);
-if (objc_getClass_addr == LLDB_INVALID_ADDRESS || missing_weak)
-  return false;
-
-LLDB_LOG(log, "Found objc_getClass at {0}", objc_getClass_addr);
-
-// Build the function type: %struct._objc_class *objc_getClass(i8*)
-
-Type *class_type = load->getType();
-Type *type_array[1];
-type_array[0] = llvm::Type::getInt8PtrTy(m_module->getContext());
-
-ArrayRef ogC_arg_types(type_array, 1);
-
-llvm::FunctionType *ogC_type =
-FunctionType::get(class_type, ogC_arg_types, false);
-
-// Build the constant containing the pointer to the function
-PointerType *ogC_ptr_ty = PointerType::getUnqual(ogC_type);
-Constant *ogC_addr_int =
-ConstantInt::get(m_intptr_ty, objc_getClass_addr, false);
-m_objc_getClass = {ogC_type,
-   ConstantExpr::getIntToPtr(ogC_addr_int, ogC_ptr_ty)};
-  }
-
-  CallInst *ogC_call = CallInst::Create(m_objc_getClass, _objc_class_name_,
-"objc_getClass", class_load);
-
-  // Replace the load with the call in all users
-
-  class_load->replaceAllUsesWith(ogC_call);
-
-  class_load->eraseFromParent();
-
-  return true;
-}
-
-bool IRForTarget::RewriteObjCClassReferences(BasicBlock &basic_block) {
-  lldb_private::Log *log(GetLog(LLDBLog::Expressions));
-
-  InstrList class_loads;
-
-  for (Instruction &inst : basic_block) {
-if (LoadInst *load = dyn_cast(&inst)

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione updated this revision to Diff 536348.
kastiglione added a comment.

Remove __FUNCTION__ from FormatManager logs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154128

Files:
  lldb/source/DataFormatters/FormatManager.cpp


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,15 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message) "[%s] " Message, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +614,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", 
__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Language search success. Returning."));
   return retval_sp;
 }
 }
   }
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving hardcoded a chance."));
   return GetHardcoded(match_data);
 }
 
@@ -628,24 +635,23 @@
   ImplSP retval_sp;
   Log *log = GetLog(LLDBLog::DataFormatters);
   if (match_data.GetTypeForCache()) {
-LLDB_LOGF(log, "\n\n[%s] Looking into cache for type %s", __FUNCTION__,
+LLDB_LOGF(log, "\n\n" FORMAT_LOG("Looking into cache for type %s"),
   match_data.GetTypeForCache().AsCString(""));
 if (m_format_cache.Get(match_data.GetTypeForCache(), retval_sp)) {
   if (log) {
-LLDB_LOGF(log, "[%s] Cache search success. Returning.", __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search success. Returning."));
 LLDB_LOGV(log, "Cache hits: {0} - Cache Misses: {1}",
   m_format_cache.GetCacheHits(),
   m_format_cache.GetCacheMisses());
   }
   return retval_sp;
 }
-LLDB_LOGF(log, "[%s] Cache search failed. Going normal route",
-  __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search failed. Going normal route"));
   }
 
   m_categories_map.Get(match_data, retval_sp);
   if (match_data.GetTypeForCache() && (!retval_sp || 
!retval_sp->NonCacheable())) {
-LLDB_LOGF(log, "[%s] Caching %p for type %s", __FUNCTION__,
+LLDB_LOGF(log, FORMAT_LOG("Caching %p for type %s"),
   static_cast(retval_sp.get()),
   match_data.GetTypeForCache().AsCString(""));
 m_format_cache.Set(match_data.GetTypeForCache(), retval_sp);
@@ -655,6 +661,8 @@
   return retval_sp;
 }
 
+#undef FORMAT_LOG
+
 lldb::TypeFormatImplSP
 FormatManager::GetFormat(ValueObject &valobj,
  lldb::DynamicValueType use_dynamic) {


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,15 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message) "[%s] " Message, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +614,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", __FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Language search success. Returning."));
   return retval_sp;
 }
 }
   }
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving h

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

We should still remove the redundant function name from the log messages 
themselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154128

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

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Our logging infrastructure already supports including the function name in the 
log message. That has always been the case for `LLDB_LOG` but `LLDB_LOGF` 
wasn't updated until 6099d519bb83 
 because 
the idea was that everyone would move to formatv-style logging `LLDB_LOG` but 
that never gained consensus.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154128

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

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D154128#4461033 , @augusto2112 
wrote:

> I've bumped into this problem before, so really like this change. It's a bit 
> sad that you'll only print the kind of data formatter once though, so someone 
> reading the logs might have to jump back and forth to understand what kind of 
> formatter they're dealing with. There's also `__PRETTY_FUNCTION__`, which 
> shows the generic type parameters by default, but I think it's not a standard 
> macro (although it's implemented by both clang and gcc). What do you think of 
> having a macro like:
>
>   #ifndef __PRETTY_FUNCTION__
>  #define __PRETTY_FUNCTION__ __FUNCTION__
>   #endif

FWIW that's exactly what `LLVM_PRETTY_FUNCTION` does. It's implemented as you 
would expect (and you described): 
https://github.com/llvm/llvm-project/blob/5e2f0947c586042083cc1643f2aac90f2492bacb/llvm/include/llvm/Support/Compiler.h#L497


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154128

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

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione updated this revision to Diff 536305.
kastiglione added a comment.

Restore `static_cast()` because of `-Wformat-pedantic`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154128

Files:
  lldb/source/DataFormatters/FormatManager.cpp


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,16 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message)
\
+  "[FormatManager::%s][%s] " Message, __FUNCTION__, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +615,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", 
__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Language search success. Returning."));
   return retval_sp;
 }
 }
   }
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving hardcoded a chance."));
   return GetHardcoded(match_data);
 }
 
@@ -628,24 +636,23 @@
   ImplSP retval_sp;
   Log *log = GetLog(LLDBLog::DataFormatters);
   if (match_data.GetTypeForCache()) {
-LLDB_LOGF(log, "\n\n[%s] Looking into cache for type %s", __FUNCTION__,
+LLDB_LOGF(log, "\n\n" FORMAT_LOG("Looking into cache for type %s"),
   match_data.GetTypeForCache().AsCString(""));
 if (m_format_cache.Get(match_data.GetTypeForCache(), retval_sp)) {
   if (log) {
-LLDB_LOGF(log, "[%s] Cache search success. Returning.", __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search success. Returning."));
 LLDB_LOGV(log, "Cache hits: {0} - Cache Misses: {1}",
   m_format_cache.GetCacheHits(),
   m_format_cache.GetCacheMisses());
   }
   return retval_sp;
 }
-LLDB_LOGF(log, "[%s] Cache search failed. Going normal route",
-  __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search failed. Going normal route"));
   }
 
   m_categories_map.Get(match_data, retval_sp);
   if (match_data.GetTypeForCache() && (!retval_sp || 
!retval_sp->NonCacheable())) {
-LLDB_LOGF(log, "[%s] Caching %p for type %s", __FUNCTION__,
+LLDB_LOGF(log, FORMAT_LOG("Caching %p for type %s"),
   static_cast(retval_sp.get()),
   match_data.GetTypeForCache().AsCString(""));
 m_format_cache.Set(match_data.GetTypeForCache(), retval_sp);
@@ -655,6 +662,8 @@
   return retval_sp;
 }
 
+#undef FORMAT_LOG
+
 lldb::TypeFormatImplSP
 FormatManager::GetFormat(ValueObject &valobj,
  lldb::DynamicValueType use_dynamic) {


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,16 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message)\
+  "[FormatManager::%s][%s] " Message, __FUNCTION__, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +615,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", __FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FU

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

2023-06-30 Thread Dave Lee via Phabricator via lldb-commits

kastiglione updated this revision to Diff 536300.
kastiglione added a comment.

Include formatter kind ("format", "summary", "synthetic") to FormatManager logs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154128

Files:
  lldb/source/DataFormatters/FormatManager.cpp


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,16 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message)
\
+  "[FormatManager::%s][%s] " Message, __FUNCTION__, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +615,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", 
__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search success. Returning.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Language search success. Returning."));
   return retval_sp;
 }
 }
   }
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.",
-__FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving hardcoded a chance."));
   return GetHardcoded(match_data);
 }
 
@@ -628,25 +636,23 @@
   ImplSP retval_sp;
   Log *log = GetLog(LLDBLog::DataFormatters);
   if (match_data.GetTypeForCache()) {
-LLDB_LOGF(log, "\n\n[%s] Looking into cache for type %s", __FUNCTION__,
+LLDB_LOGF(log, "\n\n" FORMAT_LOG("Looking into cache for type %s"),
   match_data.GetTypeForCache().AsCString(""));
 if (m_format_cache.Get(match_data.GetTypeForCache(), retval_sp)) {
   if (log) {
-LLDB_LOGF(log, "[%s] Cache search success. Returning.", __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search success. Returning."));
 LLDB_LOGV(log, "Cache hits: {0} - Cache Misses: {1}",
   m_format_cache.GetCacheHits(),
   m_format_cache.GetCacheMisses());
   }
   return retval_sp;
 }
-LLDB_LOGF(log, "[%s] Cache search failed. Going normal route",
-  __FUNCTION__);
+LLDB_LOGF(log, FORMAT_LOG("Cache search failed. Going normal route"));
   }
 
   m_categories_map.Get(match_data, retval_sp);
   if (match_data.GetTypeForCache() && (!retval_sp || 
!retval_sp->NonCacheable())) {
-LLDB_LOGF(log, "[%s] Caching %p for type %s", __FUNCTION__,
-  static_cast(retval_sp.get()),
+LLDB_LOGF(log, FORMAT_LOG("Caching %p for type %s"), retval_sp.get(),
   match_data.GetTypeForCache().AsCString(""));
 m_format_cache.Set(match_data.GetTypeForCache(), retval_sp);
   }
@@ -655,6 +661,8 @@
   return retval_sp;
 }
 
+#undef FORMAT_LOG
+
 lldb::TypeFormatImplSP
 FormatManager::GetFormat(ValueObject &valobj,
  lldb::DynamicValueType use_dynamic) {


Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -596,6 +596,16 @@
   return retval_sp;
 }
 
+namespace {
+template  const char *FormatterKind;
+template <> const char *FormatterKind = "format";
+template <> const char *FormatterKind = "summary";
+template <> const char *FormatterKind = "synthetic";
+} // namespace
+
+#define FORMAT_LOG(Message)\
+  "[FormatManager::%s][%s] " Message, __FUNCTION__, FormatterKind
+
 template 
 ImplSP FormatManager::Get(ValueObject &valobj,
   lldb::DynamicValueType use_dynamic) {
@@ -605,21 +615,19 @@
 
   Log *log = GetLog(LLDBLog::DataFormatters);
 
-  LLDB_LOGF(log, "[%s] Search failed. Giving language a chance.", __FUNCTION__);
+  LLDB_LOGF(log, FORMAT_LOG("Search failed. Giving language a chance."));
   for (lldb::LanguageType lang_type : match_data.GetCandidateLanguages()) {
 if (LanguageCategory *lang_category = GetCategoryForLanguage(lang_type)) {
   ImplSP retval_sp;
   if (lang_category->Get(match_data, retval_sp))
 if (retval_sp) {
-  LLDB_LOGF(log, "[%s] Language search

[Lldb-commits] [PATCH] D154208: [lldb][AArch64] Handle different default vector length in SVE testing

2023-06-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This test previously ran on QEMU or A64FX both of which can/do have
512 bit SVE by default.

Graviton 3 has 256 bit SVE so the first part of the test failed.

To fix this, probe the supported vector lengths before starting
the test. The first check will use the default vector length and
the rest use either 256 or 128 bit.

Therefore this test will be skipped on a machine with only 128 bit SVE.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154208

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py


Index: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -1,5 +1,10 @@
 """
 Test the AArch64 SVE registers dynamic resize with multiple threads.
+
+This test assumes a minimum supported vector length (VL) of 256 bits
+and will test 512 bits if possible. We refer to "vg" which is the
+register shown in lldb. This is in units of 64 bits. 256 bit VL is
+the same as a vg of 4.
 """
 
 import lldb
@@ -9,6 +14,39 @@
 
 
 class RegisterCommandsTestCase(TestBase):
+def get_supported_vg(self):
+# Changing VL trashes the register state, so we need to run the program
+# just to test this. Then run it again for the test.
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+main_thread_stop_line = line_number("main.c", "// Break in main 
thread")
+lldbutil.run_break_set_by_file_and_line(self, "main.c", 
main_thread_stop_line)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"thread info 1",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint"],
+)
+
+# Write back the current vg to confirm read/write works at all.
+current_vg = self.match("register read vg", ["(0x[0-9]+)"])
+self.assertTrue(current_vg is not None)
+self.expect("register write vg {}".format(current_vg.group()))
+
+# Aka 128, 256 and 512 bit.
+supported_vg = []
+for vg in [2, 4, 8]:
+# This could mask other errors but writing vg is tested elsewhere
+# so we assume the hardware rejected the value.
+self.runCmd("register write vg {}".format(vg), check=False)
+if not self.res.GetError():
+supported_vg.append(vg)
+
+return supported_vg
+
 def check_sve_registers(self, vg_test_value):
 z_reg_size = vg_test_value * 8
 p_reg_size = int(z_reg_size / 8)
@@ -56,13 +94,18 @@
 def test_sve_registers_dynamic_config(self):
 """Test AArch64 SVE registers multi-threaded dynamic resize."""
 
+if not self.isAArch64SVE():
+self.skipTest("SVE registers must be supported.")
+
 self.build()
+supported_vg = self.get_supported_vg()
+
+if not (2 in supported_vg and 4 in supported_vg):
+self.skipTest("Not all required SVE vector lengths are supported.")
+
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
-if not self.isAArch64SVE():
-self.skipTest("SVE registers must be supported.")
-
 main_thread_stop_line = line_number("main.c", "// Break in main 
thread")
 lldbutil.run_break_set_by_file_and_line(self, "main.c", 
main_thread_stop_line)
 
@@ -90,7 +133,10 @@
 substrs=["stop reason = breakpoint"],
 )
 
-self.check_sve_registers(8)
+if 8 in supported_vg:
+self.check_sve_registers(8)
+else:
+self.check_sve_registers(4)
 
 self.runCmd("process continue", RUN_SUCCEEDED)
 


Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -1,5 +1,10 @@
 """
 Test the AArch64 SVE registers dynamic resize with multiple threads.
+
+This test assumes a minimum supported vector length (VL) of 256 bits
+and will test 512 bits if possible. We refer to "vg" which is the
+register shown in lldb. This is in

[Lldb-commits] [PATCH] D154204: [lldb][AArch64] Account for extra libc frames in PAC unwind test

2023-06-30 Thread David Spickett via Phabricator via lldb-commits

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

Running this on Amazon Ubuntu the final backtrace is:

  (lldb) thread backtrace
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
* frame #0: 0x07d0 a.out`func_c at main.c:10:3
  frame #1: 0x07c4 a.out`func_b at main.c:14:3
  frame #2: 0x07b4 a.out`func_a at main.c:18:3
  frame #3: 0x07a4 a.out`main(argc=, 
argv=) at main.c:22:3
  frame #4: 0xf7b373fc libc.so.6`___lldb_unnamed_symbol2962 + 108
  frame #5: 0xf7b374cc libc.so.6`__libc_start_main + 152
  frame #6: 0x06b0 a.out`_start + 48

This causes the test to fail because of the extra ___lldb_unnamed_symbol2962 
frame
(an inlined function?).

To fix this, strictly check all the frames in main.c then for the rest
just check we find __libc_start_main and _start in that order regardless
of other frames in between.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154204

Files:
  
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py


Index: 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -42,17 +42,31 @@
 "func_b",
 "func_a",
 "main",
+]
+
+libc_backtrace = [
 "__libc_start_main",
 "_start",
 ]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
-for frame_idx, frame in enumerate(thread.frames):
-frame = thread.GetFrameAtIndex(frame_idx)
+
+self.assertTrue(thread.GetNumFrames() >= (len(backtrace) + 
len(libc_backtrace)))
+
+# Strictly check frames that are in the test program's source.
+for frame_idx, frame in enumerate(thread.frames[:len(backtrace)]):
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
-# Check line number for functions in main.c
-if frame_idx < 4:
-self.assertEqual(
-frame.GetLineEntry().GetLine(),
-line_number("main.c", "Frame " + backtrace[frame_idx]),
-)
+self.assertEqual(
+frame.GetLineEntry().GetLine(),
+line_number("main.c", "Frame " + backtrace[frame_idx]),
+)
+
+# After the program comes some libc frames. The number varies by
+# system, so ensure we have at least these two in this order,
+# skipping frames in between.
+start_idx = frame_idx + 1
+for frame_idx, frame in enumerate(thread.frames[start_idx:], 
start=start_idx):
+self.assertTrue(frame)
+if libc_backtrace[0] == frame.GetFunctionName():
+libc_backtrace.pop(0)
+
+self.assertFalse(libc_backtrace, "Did not find expected libc frames.")


Index: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
===
--- lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
+++ lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py
@@ -42,17 +42,31 @@
 "func_b",
 "func_a",
 "main",
+]
+
+libc_backtrace = [
 "__libc_start_main",
 "_start",
 ]
-self.assertEqual(thread.GetNumFrames(), len(backtrace))
-for frame_idx, frame in enumerate(thread.frames):
-frame = thread.GetFrameAtIndex(frame_idx)
+
+self.assertTrue(thread.GetNumFrames() >= (len(backtrace) + len(libc_backtrace)))
+
+# Strictly check frames that are in the test program's source.
+for frame_idx, frame in enumerate(thread.frames[:len(backtrace)]):
 self.assertTrue(frame)
 self.assertEqual(frame.GetFunctionName(), backtrace[frame_idx])
-# Check line number for functions in main.c
-if frame_idx < 4:
-self.assertEqual(
-frame.GetLineEntry().GetLine(),
-line_number("main.c", "Frame " + backtrace[frame_idx]),
-)
+self.assertEqual(
+frame.GetLineEntry().GetLine(),
+line_number("main.c", "Frame " + backtrace[frame_idx]),
+)
+
+# After the program comes some libc frames. The number varies by
+# system, so ensure we have at least these two in this order,
+# skipping frames in between.
+

[Lldb-commits] [PATCH] D154201: [lldb][AArch64] Fix tagged watch test on Graviton 3

2023-06-30 Thread David Spickett via Phabricator via lldb-commits

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

During __do_global_dtors_aux glibc sets a flag that is right
next to the global variable. This is done using a store byte.

On QEMU the watchpoints are handled with a finer granularity
than real hardware, so this wasn't a problem. On Graviton 3
(and Mountain Jade, though this test won't run there) watchpoints
look at larger chunks of memory.

This means that the final continue actually stops in  __do_global_dtors_aux
instead of exiting.

We could fix this by padding the global to be away from the flag,
but that is fiddly and it is easier just to remove the watchpoint
before the final continue. We have already verified it worked by that
point.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154201

Files:
  lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py


Index: 
lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
===
--- 
lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
+++ 
lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
@@ -129,12 +129,15 @@
 substrs=["stop reason = watchpoint"],
 )
 
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should now be 2.
+self.expect("watchpoint list -v", substrs=["hit_count = 2"])
+
+# On some hardware, during __do_global_dtors_aux a flag is set near
+# the global which can trigger the watchpoint. So we must remove it.
+self.runCmd("watchpoint delete 1")
 self.runCmd("process continue")
 
 # There should be no more watchpoint hit and the process status should
 # be 'exited'.
 self.expect("process status", substrs=["exited"])
-
-# Use the '-v' option to do verbose listing of the watchpoint.
-# The hit count should now be 2.
-self.expect("watchpoint list -v", substrs=["hit_count = 2"])


Index: lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
===
--- lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
+++ lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
@@ -129,12 +129,15 @@
 substrs=["stop reason = watchpoint"],
 )
 
+# Use the '-v' option to do verbose listing of the watchpoint.
+# The hit count should now be 2.
+self.expect("watchpoint list -v", substrs=["hit_count = 2"])
+
+# On some hardware, during __do_global_dtors_aux a flag is set near
+# the global which can trigger the watchpoint. So we must remove it.
+self.runCmd("watchpoint delete 1")
 self.runCmd("process continue")
 
 # There should be no more watchpoint hit and the process status should
 # be 'exited'.
 self.expect("process status", substrs=["exited"])
-
-# Use the '-v' option to do verbose listing of the watchpoint.
-# The hit count should now be 2.
-self.expect("watchpoint list -v", substrs=["hit_count = 2"])
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D153513: [lldb] Check that qLaunchGDBServer packet does not return an error

2023-06-30 Thread David Spickett via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdfbe3a79e20f: [lldb] Check that qLaunchGDBServer packet does 
not return an error (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153513

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/test/API/commands/platform/launchgdbserver/Makefile
  lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
  lldb/test/API/commands/platform/launchgdbserver/main.c


Index: lldb/test/API/commands/platform/launchgdbserver/main.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/main.c
@@ -0,0 +1 @@
+int main() { return 0; }
Index: 
lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
===
--- /dev/null
+++ 
lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -0,0 +1,58 @@
+""" Check that errors while handling qLaunchGDBServer are reported to the user.
+Though this isn't a platform command in itself, the best way to test it is
+from Python because we can juggle multiple processes more easily.
+"""
+
+import os
+import socket
+import shutil
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPlatformProcessLaunchGDBServer(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfRemote
+# Windows doesn't use lldb-server and on Darwin we may be using 
debugserver.
+@skipUnlessPlatform(["linux"])
+@add_test_categories(["lldb-server"])
+def test_platform_process_launch_gdb_server(self):
+self.build()
+
+hostname = socket.getaddrinfo("localhost", 0, 
proto=socket.IPPROTO_TCP)[0][4][0]
+listen_url = "[%s]:0" % hostname
+
+port_file = self.getBuildArtifact("port")
+commandline_args = [
+"platform",
+"--listen",
+listen_url,
+"--socket-file",
+port_file,
+"--",
+self.getBuildArtifact("a.out"),
+"foo",
+]
+
+# Run lldb-server from a new location.
+new_lldb_server = self.getBuildArtifact("lldb-server")
+shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+
+self.spawnSubprocess(new_lldb_server, commandline_args)
+socket_id = lldbutil.wait_for_file_on_target(self, port_file)
+
+# Remove our new lldb-server so that when it tries to invoke itself as 
a
+# gdbserver, it fails.
+os.remove(new_lldb_server)
+
+new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
+self.dbg.SetSelectedPlatform(new_platform)
+
+connect_url = "connect://[%s]:%s" % (hostname, socket_id)
+self.runCmd("platform connect %s" % connect_url)
+
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.expect("run", substrs=["unable to launch a GDB server on"], 
error=True)
Index: lldb/test/API/commands/platform/launchgdbserver/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2590,6 +2590,9 @@
 
   if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
   PacketResult::Success) {
+if (response.IsErrorResponse())
+  return false;
+
 llvm::StringRef name;
 llvm::StringRef value;
 while (response.GetNameColonValue(name, value)) {


Index: lldb/test/API/commands/platform/launchgdbserver/main.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/main.c
@@ -0,0 +1 @@
+int main() { return 0; }
Index: lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -0,0 +1,58 @@
+""" Check that errors while handling qLaunchGDBServer are reported to the user.
+Though this isn't a platform command in itself, the best way to test it is
+from Python because we can juggle multiple processes more easily.
+"""
+
+import os
+import socket
+import shutil
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldb

[Lldb-commits] [lldb] dfbe3a7 - [lldb] Check that qLaunchGDBServer packet does not return an error

2023-06-30 Thread David Spickett via lldb-commits


Author: David Spickett
Date: 2023-06-30T09:12:30Z
New Revision: dfbe3a79e20f1bc51a59ee858fabce792d59c9ae

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

LOG: [lldb] Check that qLaunchGDBServer packet does not return an error

While looking at https://github.com/llvm/llvm-project/issues/61955
I noticed that when we send qLaunchGDBServer we check that we got a response
but not what kind of response it was.

I think this was why the bug reporter saw:
(lldb) run
error: invalid host:port specification: '[192.168.64.2]'

The missing port is because we went down a path we only should have
chosen if the operation succeeded. Since we didn't check, we went ahead
with an empty port number.

To test this I've done the following:
* Make a temporary copy of lldb-server.
* Run that as a platform.
* Remove the copy.
* Attempt to create and run a target.

This fails because the running lldb-server will try to invoke itself
and it no longer exists.

Reviewed By: jasonmolenda

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

Added: 
lldb/test/API/commands/platform/launchgdbserver/Makefile

lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
lldb/test/API/commands/platform/launchgdbserver/main.c

Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 18aa98bc046495..36e046d7466ebb 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2590,6 +2590,9 @@ bool GDBRemoteCommunicationClient::LaunchGDBServer(
 
   if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
   PacketResult::Success) {
+if (response.IsErrorResponse())
+  return false;
+
 llvm::StringRef name;
 llvm::StringRef value;
 while (response.GetNameColonValue(name, value)) {

diff  --git a/lldb/test/API/commands/platform/launchgdbserver/Makefile 
b/lldb/test/API/commands/platform/launchgdbserver/Makefile
new file mode 100644
index 00..10495940055b63
--- /dev/null
+++ b/lldb/test/API/commands/platform/launchgdbserver/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
 
b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
new file mode 100644
index 00..ea849ab1fa236d
--- /dev/null
+++ 
b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -0,0 +1,58 @@
+""" Check that errors while handling qLaunchGDBServer are reported to the user.
+Though this isn't a platform command in itself, the best way to test it is
+from Python because we can juggle multiple processes more easily.
+"""
+
+import os
+import socket
+import shutil
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPlatformProcessLaunchGDBServer(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfRemote
+# Windows doesn't use lldb-server and on Darwin we may be using 
debugserver.
+@skipUnlessPlatform(["linux"])
+@add_test_categories(["lldb-server"])
+def test_platform_process_launch_gdb_server(self):
+self.build()
+
+hostname = socket.getaddrinfo("localhost", 0, 
proto=socket.IPPROTO_TCP)[0][4][0]
+listen_url = "[%s]:0" % hostname
+
+port_file = self.getBuildArtifact("port")
+commandline_args = [
+"platform",
+"--listen",
+listen_url,
+"--socket-file",
+port_file,
+"--",
+self.getBuildArtifact("a.out"),
+"foo",
+]
+
+# Run lldb-server from a new location.
+new_lldb_server = self.getBuildArtifact("lldb-server")
+shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+
+self.spawnSubprocess(new_lldb_server, commandline_args)
+socket_id = lldbutil.wait_for_file_on_target(self, port_file)
+
+# Remove our new lldb-server so that when it tries to invoke itself as 
a
+# gdbserver, it fails.
+os.remove(new_lldb_server)
+
+new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
+self.dbg.SetSelectedPlatform(new_platform)
+
+connect_url = "connect://[%s]:%s" % (hostname, socket_id)
+self.runCmd("platform connect %s" % connect_url)
+
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out"

[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-06-30 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

Some random comments, I'll leave the real review to the VSCode experts.




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.
 ///

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



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") {

There is AppendMessageWithFormat that you could use to avoid repeating the 
string here.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:805
+  } else {
+std::string error = "Unknown repl-mode '" + new_mode.str() + "'.";
+result.SetError(error.c_str());

Would be useful if it also said: "repl-mode "foo". Expected one of "auto", 
.".



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:806
+std::string error = "Unknown repl-mode '" + new_mode.str() + "'.";
+result.SetError(error.c_str());
+result.SetStatus(lldb::eReturnStatusFailed);

SetError ends up calling CommandReturnObject::AppendError which will set the 
status to failed for you (I changed this a while back because I kept forgetting 
to set it).


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] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-06-30 Thread Corentin Jabot via Phabricator via lldb-commits

cor3ntin added a comment.

It would be great if we could make progress on this for clang 17, I was told 
it's forcing some people to use other compilers.
Are the recent changes to the itanium abi enough to complete this work? Thanks!


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

https://reviews.llvm.org/D140996

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

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld

[Lldb-commits] [lldb] 624813a - Change the dyld notification function that lldb puts a breakpoint in

[Lldb-commits] [PATCH] D139453: Switch to a different library-loaded notification function breakpoint in Darwin dyld

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

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [lldb] b43375b - [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154271: [lldb] Fix data race when interacting with python scripts

[Lldb-commits] [PATCH] D154201: [lldb][AArch64] Fix tagged watch test on Graviton 3

[Lldb-commits] [PATCH] D154204: [lldb][AArch64] Account for extra libc frames in PAC unwind test

[Lldb-commits] [PATCH] D154265: [lldb][NFC] Simplify GetLocation_DW_OP_addr function

[Lldb-commits] [PATCH] D154268: [lldb] Skip apple accelerator table test in DWARF 5 mode

[Lldb-commits] [PATCH] D153834: [lldb] Add two-level caching in the source manager

[Lldb-commits] [PATCH] D154268: [lldb] Skip apple accelerator table test in DWARF 5 mode

[Lldb-commits] [PATCH] D154268: [lldb] Skip apple accelerator table test in DWARF 5 mode

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154265: [lldb][NFC] Simplify GetLocation_DW_OP_addr function

[Lldb-commits] [PATCH] D153802: [lldb] Delete RewriteObjCClassReferences (NFC)

[Lldb-commits] [lldb] 2c37cbe - [lldb] Delete RewriteObjCClassReferences (NFC)

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154128: [lldb] Add log indicating which kind of data formatter

[Lldb-commits] [PATCH] D154208: [lldb][AArch64] Handle different default vector length in SVE testing

[Lldb-commits] [PATCH] D154204: [lldb][AArch64] Account for extra libc frames in PAC unwind test

[Lldb-commits] [PATCH] D154201: [lldb][AArch64] Fix tagged watch test on Graviton 3

[Lldb-commits] [PATCH] D153513: [lldb] Check that qLaunchGDBServer packet does not return an error

[Lldb-commits] [lldb] dfbe3a7 - [lldb] Check that qLaunchGDBServer packet does not return an error

[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.

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

34 matches

Site Navigation

Mail list logo

Footer information