[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in shadow mode to event listeners

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 515960.
mib added a comment.

fix typo


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

https://reviews.llvm.org/D148397

Files:
  lldb/include/lldb/API/SBAttachInfo.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/include/lldb/Utility/Listener.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Listener.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -23,13 +23,13 @@
 
 ProcessInfo::ProcessInfo()
 : m_executable(), m_arguments(), m_environment(), m_arch(), m_listener_sp(),
-  m_hijack_listener_sp(), m_passthrough_listener_sp() {}
+  m_hijack_listener_sp(), m_shadow_listener_sp() {}
 
 ProcessInfo::ProcessInfo(const char *name, const ArchSpec ,
  lldb::pid_t pid)
 : m_executable(name), m_arguments(), m_environment(), m_arch(arch),
   m_pid(pid), m_listener_sp(), m_hijack_listener_sp(),
-  m_passthrough_listener_sp() {}
+  m_shadow_listener_sp() {}
 
 void ProcessInfo::Clear() {
   m_executable.Clear();
Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -35,7 +35,7 @@
 
 Listener::Listener(const char *name)
 : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex() {
+  m_events_mutex(), m_is_shadow() {
   Log *log = GetLog(LLDBLog::Object);
   if (log != nullptr)
 LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
@@ -302,7 +302,8 @@
   // to return it so it should be okay to get the next event off the queue
   // here - and it might be useful to do that in the "DoOnRemoval".
   lock.unlock();
-  event_sp->DoOnRemoval();
+  if (!m_is_shadow)
+event_sp->DoOnRemoval();
 }
 return true;
   }
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -228,6 +228,8 @@
   _broadcaster, event_type))
   return;
 hijacking_listener_sp->AddEvent(event_sp);
+if (m_shadow_listener)
+  m_shadow_listener->AddEvent(event_sp);
   } else {
 for (auto  : GetListeners()) {
   if (!(pair.second & event_type))
@@ -237,6 +239,8 @@
 continue;
 
   pair.first->AddEvent(event_sp);
+  if (m_shadow_listener)
+m_shadow_listener->AddEvent(event_sp);
 }
   }
 }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3193,6 +3193,7 @@
 // Since we didn't have a platform launch the process, launch it here.
 if (m_process_sp) {
   m_process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  m_process_sp->SetShadowListener(launch_info.GetShadowListener());
   error = m_process_sp->Launch(launch_info);
 }
   }
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -423,6 +423,7 @@
 
 if (process_sp) {
   process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  process_sp->SetShadowListener(launch_info.GetShadowListener());
 
   error = process_sp->ConnectRemote(connect_url.c_str());
   // Retry the connect remote one time...
@@ -515,6 +516,7 @@
   ListenerSP listener_sp = attach_info.GetHijackListener();
   if (listener_sp)
 process_sp->HijackProcessEvents(listener_sp);
+  process_sp->SetShadowListener(attach_info.GetShadowListener());
   error = process_sp->Attach(attach_info);
 }
 
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -401,6 +401,7 @@
   attach_info.SetHijackListener(listener_sp);
 }
 process_sp->HijackProcessEvents(listener_sp);
+process_sp->SetShadowListener(attach_info.GetShadowListener());
 error = 

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in shadow mode to event listeners

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 515959.
mib added a comment.

More renaming and reformat


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

https://reviews.llvm.org/D148397

Files:
  lldb/include/lldb/API/SBAttachInfo.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/include/lldb/Utility/Listener.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Listener.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -23,13 +23,13 @@
 
 ProcessInfo::ProcessInfo()
 : m_executable(), m_arguments(), m_environment(), m_arch(), m_listener_sp(),
-  m_hijack_listener_sp(), m_passthrough_listener_sp() {}
+  m_hijack_listener_sp(), m_shadow_listener_sp() {}
 
 ProcessInfo::ProcessInfo(const char *name, const ArchSpec ,
  lldb::pid_t pid)
 : m_executable(name), m_arguments(), m_environment(), m_arch(arch),
   m_pid(pid), m_listener_sp(), m_hijack_listener_sp(),
-  m_passthrough_listener_sp() {}
+  m_shadow_listener_sp() {}
 
 void ProcessInfo::Clear() {
   m_executable.Clear();
Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -35,7 +35,7 @@
 
 Listener::Listener(const char *name)
 : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex() {
+  m_events_mutex(), m_is_shadow() {
   Log *log = GetLog(LLDBLog::Object);
   if (log != nullptr)
 LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
@@ -302,7 +302,8 @@
   // to return it so it should be okay to get the next event off the queue
   // here - and it might be useful to do that in the "DoOnRemoval".
   lock.unlock();
-  event_sp->DoOnRemoval();
+  if (!m_is_shadow)
+event_sp->DoOnRemoval();
 }
 return true;
   }
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -228,6 +228,8 @@
   _broadcaster, event_type))
   return;
 hijacking_listener_sp->AddEvent(event_sp);
+if (m_shadow_listener)
+  m_shadow_listener->AddEvent(event_sp);
   } else {
 for (auto  : GetListeners()) {
   if (!(pair.second & event_type))
@@ -237,6 +239,8 @@
 continue;
 
   pair.first->AddEvent(event_sp);
+  if (m_shadow_listener)
+m_shadow_listener->AddEvent(event_sp);
 }
   }
 }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3193,6 +3193,7 @@
 // Since we didn't have a platform launch the process, launch it here.
 if (m_process_sp) {
   m_process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  m_process_sp->SetShadowListener(launch_info.GetShadowListener());
   error = m_process_sp->Launch(launch_info);
 }
   }
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -423,6 +423,7 @@
 
 if (process_sp) {
   process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  process_sp->SetShadowListener(launch_info.GetShadowListener());
 
   error = process_sp->ConnectRemote(connect_url.c_str());
   // Retry the connect remote one time...
@@ -515,6 +516,7 @@
   ListenerSP listener_sp = attach_info.GetHijackListener();
   if (listener_sp)
 process_sp->HijackProcessEvents(listener_sp);
+  process_sp->SetShadowListener(attach_info.GetShadowListener());
   error = process_sp->Attach(attach_info);
 }
 
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -401,6 +401,7 @@
   attach_info.SetHijackListener(listener_sp);
 }
 process_sp->HijackProcessEvents(listener_sp);
+process_sp->SetShadowListener(attach_info.GetShadowListener());
 

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in shadow mode to event listeners

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 515956.
mib marked an inline comment as done.
mib retitled this revision from "[lldb/Utility] Add opt-in passthrough mode to 
event listeners" to "[lldb/Utility] Add opt-in shadow mode to event listeners".
mib edited the summary of this revision.
mib added a comment.

"Shadow Listener" it is @JDevlieghere


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

https://reviews.llvm.org/D148397

Files:
  lldb/include/lldb/API/SBAttachInfo.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/include/lldb/Utility/Listener.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Listener.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -23,13 +23,13 @@
 
 ProcessInfo::ProcessInfo()
 : m_executable(), m_arguments(), m_environment(), m_arch(), m_listener_sp(),
-  m_hijack_listener_sp(), m_passthrough_listener_sp() {}
+  m_hijack_listener_sp(), m_shadow_listener_sp() {}
 
 ProcessInfo::ProcessInfo(const char *name, const ArchSpec ,
  lldb::pid_t pid)
 : m_executable(name), m_arguments(), m_environment(), m_arch(arch),
   m_pid(pid), m_listener_sp(), m_hijack_listener_sp(),
-  m_passthrough_listener_sp() {}
+  m_shadow_listener_sp() {}
 
 void ProcessInfo::Clear() {
   m_executable.Clear();
Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -35,7 +35,7 @@
 
 Listener::Listener(const char *name)
 : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex() {
+  m_events_mutex(), m_is_passthrough() {
   Log *log = GetLog(LLDBLog::Object);
   if (log != nullptr)
 LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
@@ -302,7 +302,8 @@
   // to return it so it should be okay to get the next event off the queue
   // here - and it might be useful to do that in the "DoOnRemoval".
   lock.unlock();
-  event_sp->DoOnRemoval();
+  if (!m_is_passthrough)
+event_sp->DoOnRemoval();
 }
 return true;
   }
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -228,6 +228,8 @@
   _broadcaster, event_type))
   return;
 hijacking_listener_sp->AddEvent(event_sp);
+if (m_shadow_listener)
+  m_shadow_listener->AddEvent(event_sp);
   } else {
 for (auto  : GetListeners()) {
   if (!(pair.second & event_type))
@@ -237,6 +239,8 @@
 continue;
 
   pair.first->AddEvent(event_sp);
+  if (m_shadow_listener)
+m_shadow_listener->AddEvent(event_sp);
 }
   }
 }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3193,6 +3193,8 @@
 // Since we didn't have a platform launch the process, launch it here.
 if (m_process_sp) {
   m_process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  m_process_sp->SetShadowListener(
+  launch_info.GetShadowListener());
   error = m_process_sp->Launch(launch_info);
 }
   }
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -423,6 +423,8 @@
 
 if (process_sp) {
   process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  process_sp->SetShadowListener(
+  launch_info.GetShadowListener());
 
   error = process_sp->ConnectRemote(connect_url.c_str());
   // Retry the connect remote one time...
@@ -515,6 +517,8 @@
   ListenerSP listener_sp = attach_info.GetHijackListener();
   if (listener_sp)
 process_sp->HijackProcessEvents(listener_sp);
+  process_sp->SetShadowListener(
+  attach_info.GetShadowListener());
   error = process_sp->Attach(attach_info);
 }
 
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- 

[Lldb-commits] [PATCH] D148940: [Demangle] remove unused params of microsoftDemangle

[Nick Desaulniers via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc2709fcb0a5b: [Demangle] remove unused params of 
microsoftDemangle (authored by nickdesaulniers).
Herald added a subscriber: JDevlieghere.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148940

Files:
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
  llvm/tools/llvm-objdump/COFFDump.cpp
  llvm/tools/llvm-undname/llvm-undname.cpp

Index: llvm/tools/llvm-undname/llvm-undname.cpp
===
--- llvm/tools/llvm-undname/llvm-undname.cpp
+++ llvm/tools/llvm-undname/llvm-undname.cpp
@@ -75,8 +75,7 @@
 Flags = MSDemangleFlags(Flags | MSDF_NoVariableType);
 
   size_t NRead;
-  char *ResultBuf =
-  microsoftDemangle(S.c_str(), , nullptr, nullptr, , Flags);
+  char *ResultBuf = microsoftDemangle(S.c_str(), , , Flags);
   if (Status == llvm::demangle_success) {
 outs() << ResultBuf << "\n";
 outs().flush();
Index: llvm/tools/llvm-objdump/COFFDump.cpp
===
--- llvm/tools/llvm-objdump/COFFDump.cpp
+++ llvm/tools/llvm-objdump/COFFDump.cpp
@@ -849,8 +849,7 @@
<< Name;
 if (Demangle && Name.startswith("?")) {
   int Status = -1;
-  char *DemangledSymbol =
-  microsoftDemangle(Name.data(), nullptr, nullptr, nullptr, );
+  char *DemangledSymbol = microsoftDemangle(Name.data(), nullptr, );
 
   if (Status == 0 && DemangledSymbol) {
 outs() << " (" << StringRef(DemangledSymbol) << ")";
Index: llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
+++ llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
@@ -14,7 +14,6 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  free(llvm::microsoftDemangle(NullTerminatedString.c_str(), nullptr, nullptr,
-   nullptr, nullptr));
+  free(llvm::microsoftDemangle(NullTerminatedString.c_str(), nullptr, nullptr));
   return 0;
 }
Index: llvm/lib/Demangle/MicrosoftDemangle.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -2398,7 +2398,6 @@
 }
 
 char *llvm::microsoftDemangle(const char *MangledName, size_t *NMangled,
-  char *Buf, size_t *N,
   int *Status, MSDemangleFlags Flags) {
   Demangler D;
 
@@ -2423,14 +2422,13 @@
 OF = OutputFlags(OF | OF_NoVariableType);
 
   int InternalStatus = demangle_success;
+  char *Buf;
   if (D.Error)
 InternalStatus = demangle_invalid_mangled_name;
   else {
-OutputBuffer OB(Buf, N);
+OutputBuffer OB;
 AST->output(OB, OF);
 OB += '\0';
-if (N != nullptr)
-  *N = OB.getCurrentPosition();
 Buf = OB.getBuffer();
   }
 
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -36,8 +36,7 @@
   if (S[0] == '_' && nonMicrosoftDemangle(S + 1, Result))
 return Result;
 
-  if (char *Demangled =
-  microsoftDemangle(S, nullptr, nullptr, nullptr, nullptr)) {
+  if (char *Demangled = microsoftDemangle(S, nullptr, nullptr)) {
 Result = Demangled;
 std::free(Demangled);
 return Result;
Index: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
===
--- llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
+++ llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
@@ -687,7 +687,7 @@
 // Only do MSVC C++ demangling on symbols starting with '?'.
 int status = 0;
 char *DemangledName = microsoftDemangle(
-Name.c_str(), nullptr, nullptr, nullptr, ,
+Name.c_str(), nullptr, ,
 MSDemangleFlags(MSDF_NoAccessSpecifier | MSDF_NoCallingConvention |
 MSDF_NoMemberType | MSDF_NoReturnType));
 if (status != 0)
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -46,15 +46,9 @@
 /// success, or nullptr on error.
 /// If n_read is non-null and demangling was successful, it receives how many
 /// bytes of the input string were consumed.
-/// buf can point to a 

[Lldb-commits] [lldb] c2709fc - [Demangle] remove unused params of microsoftDemangle

[Nick Desaulniers via lldb-commits]

Author: Nick Desaulniers
Date: 2023-04-21T15:37:00-07:00
New Revision: c2709fcb0a5b32e42a35661fab4d5744e11d04c0

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

LOG: [Demangle] remove unused params of microsoftDemangle

No call sites use these parameters, so drop them.

Reviewed By: MaskRay

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

Added: 


Modified: 
lldb/source/Core/Mangled.cpp
llvm/include/llvm/Demangle/Demangle.h
llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
llvm/lib/Demangle/Demangle.cpp
llvm/lib/Demangle/MicrosoftDemangle.cpp
llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
llvm/tools/llvm-objdump/COFFDump.cpp
llvm/tools/llvm-undname/llvm-undname.cpp

Removed: 




diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index 29fb35980036a..30c8e1a3330d2 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -108,7 +108,7 @@ void Mangled::SetValue(ConstString name) {
 // Local helpers for 
diff erent demangling implementations.
 static char *GetMSVCDemangledStr(const char *M) {
   char *demangled_cstr = llvm::microsoftDemangle(
-  M, nullptr, nullptr, nullptr, nullptr,
+  M, nullptr, nullptr,
   llvm::MSDemangleFlags(
   llvm::MSDF_NoAccessSpecifier | llvm::MSDF_NoCallingConvention |
   llvm::MSDF_NoMemberType | llvm::MSDF_NoVariableType));

diff  --git a/llvm/include/llvm/Demangle/Demangle.h 
b/llvm/include/llvm/Demangle/Demangle.h
index 6133d0b95bbfc..855ab1f57512f 100644
--- a/llvm/include/llvm/Demangle/Demangle.h
+++ b/llvm/include/llvm/Demangle/Demangle.h
@@ -46,15 +46,9 @@ enum MSDemangleFlags {
 /// success, or nullptr on error.
 /// If n_read is non-null and demangling was successful, it receives how many
 /// bytes of the input string were consumed.
-/// buf can point to a *n_buf bytes large buffer where the demangled name is
-/// stored. If the buffer is too small, it is grown with realloc(). If buf is
-/// nullptr, then this malloc()s memory for the result.
-/// *n_buf stores the size of buf on input if buf is non-nullptr, and it
-/// receives the size of the demangled string on output if n_buf is not 
nullptr.
 /// status receives one of the demangle_ enum entries above if it's not 
nullptr.
 /// Flags controls various details of the demangled representation.
-char *microsoftDemangle(const char *mangled_name, size_t *n_read, char *buf,
-size_t *n_buf, int *status,
+char *microsoftDemangle(const char *mangled_name, size_t *n_read, int *status,
 MSDemangleFlags Flags = MSDF_None);
 
 // Demangles a Rust v0 mangled symbol.

diff  --git a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp 
b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
index 499ceae88d563..26e56af402234 100644
--- a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
+++ b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
@@ -687,7 +687,7 @@ LLVMSymbolizer::DemangleName(const std::string ,
 // Only do MSVC C++ demangling on symbols starting with '?'.
 int status = 0;
 char *DemangledName = microsoftDemangle(
-Name.c_str(), nullptr, nullptr, nullptr, ,
+Name.c_str(), nullptr, ,
 MSDemangleFlags(MSDF_NoAccessSpecifier | MSDF_NoCallingConvention |
 MSDF_NoMemberType | MSDF_NoReturnType));
 if (status != 0)

diff  --git a/llvm/lib/Demangle/Demangle.cpp b/llvm/lib/Demangle/Demangle.cpp
index 9d128424cabf4..68eddde056021 100644
--- a/llvm/lib/Demangle/Demangle.cpp
+++ b/llvm/lib/Demangle/Demangle.cpp
@@ -36,8 +36,7 @@ std::string llvm::demangle(const std::string ) {
   if (S[0] == '_' && nonMicrosoftDemangle(S + 1, Result))
 return Result;
 
-  if (char *Demangled =
-  microsoftDemangle(S, nullptr, nullptr, nullptr, nullptr)) {
+  if (char *Demangled = microsoftDemangle(S, nullptr, nullptr)) {
 Result = Demangled;
 std::free(Demangled);
 return Result;

diff  --git a/llvm/lib/Demangle/MicrosoftDemangle.cpp 
b/llvm/lib/Demangle/MicrosoftDemangle.cpp
index 828350076ba5e..c3713a7dbd3ad 100644
--- a/llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ b/llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -2398,7 +2398,6 @@ void Demangler::dumpBackReferences() {
 }
 
 char *llvm::microsoftDemangle(const char *MangledName, size_t *NMangled,
-  char *Buf, size_t *N,
   int *Status, MSDemangleFlags Flags) {
   Demangler D;
 
@@ -2423,14 +2422,13 @@ char *llvm::microsoftDemangle(const char *MangledName, 
size_t *NMangled,
 OF = OutputFlags(OF | OF_NoVariableType);
 
   int InternalStatus = demangle_success;
+  char *Buf;
   if (D.Error)
 InternalStatus = demangle_invalid_mangled_name;
   else {
-OutputBuffer 

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Alex Langford via Phabricator via lldb-commits]

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

I don't have an issue here then. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148395

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

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/include/lldb/Target/Process.h:122
 m_async(false) {
 ProcessInfo::operator=(launch_info);
 SetProcessPluginName(launch_info.GetProcessPluginName());

bulbazord wrote:
> mib wrote:
> > Because we moved `m_listener_sp` and `m_hijack_listener_sp` to 
> > `ProcessInfo` and since both `Process{Attach,Launch}Info` are derived from 
> > that class, this should also copy the listeners.
> I don't think that's true? We're taking a different subclass and initializing 
> from that, we have to be explicit about which fields are copied over, no? 
> Maybe I'm missing something here.
> 
> Either way, I'm not sure it really matters unless the `ProcessLaunchInfo` and 
> `ProcessAttachInfo` necessarily need to be in sync. Do they?
Wait, I'm being silly, you're doing `operator=` there. Okay, sorry for all the 
noise, I think I was just confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148395

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

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/include/lldb/Target/Process.h:122
 m_async(false) {
 ProcessInfo::operator=(launch_info);
 SetProcessPluginName(launch_info.GetProcessPluginName());

mib wrote:
> Because we moved `m_listener_sp` and `m_hijack_listener_sp` to `ProcessInfo` 
> and since both `Process{Attach,Launch}Info` are derived from that class, this 
> should also copy the listeners.
I don't think that's true? We're taking a different subclass and initializing 
from that, we have to be explicit about which fields are copied over, no? Maybe 
I'm missing something here.

Either way, I'm not sure it really matters unless the `ProcessLaunchInfo` and 
`ProcessAttachInfo` necessarily need to be in sync. Do they?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148395

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

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Target/PathMappingList.cpp:72
 void PathMappingList::Append(const PathMappingList , bool notify) {
+  std::scoped_lock locks(m_mutex, rhs.m_mutex);
   ++m_mod_id;

nickdesaulniers wrote:
> ```
> /android0/llvm-project/lldb/source/Target/PathMappingList.cpp:72:3: warning: 
> 'scoped_lock' may not intend to support class template argument deduction 
> [-Wctad-maybe-unsupported]
>   std::scoped_lock locks(m_mutex, rhs.m_mutex);
>   ^
> /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/mutex:692:11: 
> note: add a deduction guide to suppress this warning
> class scoped_lock
>   ^
> ```
Fixed in `c5fc7809e05940674424aaed7dd06c6be0639864`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [PATCH] D148940: [Demangle] remove unused params of microsoftDemangle

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers updated this revision to Diff 515935.
nickdesaulniers added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- fix lldb, test building lldb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148940

Files:
  lldb/source/Core/Mangled.cpp
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
  llvm/tools/llvm-objdump/COFFDump.cpp
  llvm/tools/llvm-undname/llvm-undname.cpp

Index: llvm/tools/llvm-undname/llvm-undname.cpp
===
--- llvm/tools/llvm-undname/llvm-undname.cpp
+++ llvm/tools/llvm-undname/llvm-undname.cpp
@@ -75,8 +75,7 @@
 Flags = MSDemangleFlags(Flags | MSDF_NoVariableType);
 
   size_t NRead;
-  char *ResultBuf =
-  microsoftDemangle(S.c_str(), , nullptr, nullptr, , Flags);
+  char *ResultBuf = microsoftDemangle(S.c_str(), , , Flags);
   if (Status == llvm::demangle_success) {
 outs() << ResultBuf << "\n";
 outs().flush();
Index: llvm/tools/llvm-objdump/COFFDump.cpp
===
--- llvm/tools/llvm-objdump/COFFDump.cpp
+++ llvm/tools/llvm-objdump/COFFDump.cpp
@@ -849,8 +849,7 @@
<< Name;
 if (Demangle && Name.startswith("?")) {
   int Status = -1;
-  char *DemangledSymbol =
-  microsoftDemangle(Name.data(), nullptr, nullptr, nullptr, );
+  char *DemangledSymbol = microsoftDemangle(Name.data(), nullptr, );
 
   if (Status == 0 && DemangledSymbol) {
 outs() << " (" << StringRef(DemangledSymbol) << ")";
Index: llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
===
--- llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
+++ llvm/tools/llvm-microsoft-demangle-fuzzer/llvm-microsoft-demangle-fuzzer.cpp
@@ -14,7 +14,6 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
   std::string NullTerminatedString((const char *)Data, Size);
-  free(llvm::microsoftDemangle(NullTerminatedString.c_str(), nullptr, nullptr,
-   nullptr, nullptr));
+  free(llvm::microsoftDemangle(NullTerminatedString.c_str(), nullptr, nullptr));
   return 0;
 }
Index: llvm/lib/Demangle/MicrosoftDemangle.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -2398,7 +2398,6 @@
 }
 
 char *llvm::microsoftDemangle(const char *MangledName, size_t *NMangled,
-  char *Buf, size_t *N,
   int *Status, MSDemangleFlags Flags) {
   Demangler D;
 
@@ -2423,14 +2422,13 @@
 OF = OutputFlags(OF | OF_NoVariableType);
 
   int InternalStatus = demangle_success;
+  char *Buf;
   if (D.Error)
 InternalStatus = demangle_invalid_mangled_name;
   else {
-OutputBuffer OB(Buf, N);
+OutputBuffer OB;
 AST->output(OB, OF);
 OB += '\0';
-if (N != nullptr)
-  *N = OB.getCurrentPosition();
 Buf = OB.getBuffer();
   }
 
Index: llvm/lib/Demangle/Demangle.cpp
===
--- llvm/lib/Demangle/Demangle.cpp
+++ llvm/lib/Demangle/Demangle.cpp
@@ -36,8 +36,7 @@
   if (S[0] == '_' && nonMicrosoftDemangle(S + 1, Result))
 return Result;
 
-  if (char *Demangled =
-  microsoftDemangle(S, nullptr, nullptr, nullptr, nullptr)) {
+  if (char *Demangled = microsoftDemangle(S, nullptr, nullptr)) {
 Result = Demangled;
 std::free(Demangled);
 return Result;
Index: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
===
--- llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
+++ llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
@@ -687,7 +687,7 @@
 // Only do MSVC C++ demangling on symbols starting with '?'.
 int status = 0;
 char *DemangledName = microsoftDemangle(
-Name.c_str(), nullptr, nullptr, nullptr, ,
+Name.c_str(), nullptr, ,
 MSDemangleFlags(MSDF_NoAccessSpecifier | MSDF_NoCallingConvention |
 MSDF_NoMemberType | MSDF_NoReturnType));
 if (status != 0)
Index: llvm/include/llvm/Demangle/Demangle.h
===
--- llvm/include/llvm/Demangle/Demangle.h
+++ llvm/include/llvm/Demangle/Demangle.h
@@ -46,15 +46,9 @@
 /// success, or nullptr on error.
 /// If n_read is non-null and demangling was successful, it receives how many
 /// bytes of the input string were consumed.
-/// buf can point to a *n_buf bytes large buffer where the demangled name is
-/// stored. If the buffer is too small, it 

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers added inline comments.



Comment at: lldb/source/Target/PathMappingList.cpp:51
   if (this != ) {
+std::scoped_lock locks(m_mutex, rhs.m_mutex);
 m_pairs = rhs.m_pairs;

```
[412/1011] Building CXX object 
tools/lldb/sourc...CMakeFiles/lldbTarget.dir/PathMappingList.cpp.o
/android0/llvm-project/lldb/source/Target/PathMappingList.cpp:51:5: warning: 
'scoped_lock' may not intend to support class template argument deduction 
[-Wctad-maybe-unsupported]
std::scoped_lock locks(m_mutex, rhs.m_mutex);
^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/mutex:692:11: note: 
add a deduction guide to suppress this warning
class scoped_lock
  ^
```



Comment at: lldb/source/Target/PathMappingList.cpp:72
 void PathMappingList::Append(const PathMappingList , bool notify) {
+  std::scoped_lock locks(m_mutex, rhs.m_mutex);
   ++m_mod_id;

```
/android0/llvm-project/lldb/source/Target/PathMappingList.cpp:72:3: warning: 
'scoped_lock' may not intend to support class template argument deduction 
[-Wctad-maybe-unsupported]
  std::scoped_lock locks(m_mutex, rhs.m_mutex);
  ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/mutex:692:11: note: 
add a deduction guide to suppress this warning
class scoped_lock
  ^
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG076341d1088b: Make sure SelectMostRelevantFrame happens only 
when returning to the user. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/StackFrameList.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/API/SBThread.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -263,10 +263,11 @@
new ThreadEventData(this->shared_from_this(), new_frame_id));
 }
 
-lldb::StackFrameSP Thread::GetSelectedFrame() {
+lldb::StackFrameSP
+Thread::GetSelectedFrame(SelectMostRelevant select_most_relevant) {
   StackFrameListSP stack_frame_list_sp(GetStackFrameList());
   StackFrameSP frame_sp = stack_frame_list_sp->GetFrameAtIndex(
-  stack_frame_list_sp->GetSelectedFrameIndex());
+  stack_frame_list_sp->GetSelectedFrameIndex(select_most_relevant));
   FrameSelectedCallback(frame_sp.get());
   return frame_sp;
 }
@@ -297,7 +298,7 @@
   const bool broadcast = true;
   bool success = SetSelectedFrameByIndex(frame_idx, broadcast);
   if (success) {
-StackFrameSP frame_sp = GetSelectedFrame();
+StackFrameSP frame_sp = GetSelectedFrame(DoNoSelectMostRelevantFrame);
 if (frame_sp) {
   bool already_shown = false;
   SymbolContext frame_sc(
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3367,9 +3367,10 @@
 if (async) {
   process_sp->RestoreProcessEvents();
 } else {
-  state = process_sp->WaitForProcessToStop(std::nullopt, nullptr, false,
-   attach_info.GetHijackListener(),
-   stream);
+  // We are stopping all the way out to the user, so update selected frames.
+  state = process_sp->WaitForProcessToStop(
+  std::nullopt, nullptr, false, attach_info.GetHijackListener(), stream,
+  true, SelectMostRelevantFrame);
   process_sp->RestoreProcessEvents();
 
   if (state != eStateStopped) {
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -1456,7 +1456,8 @@
 return ValueObjectSP();
   }
 
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+  thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
 
   if (!frame_sp) {
 return ValueObjectSP();
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -815,12 +815,18 @@
   }
 }
 
-uint32_t StackFrameList::GetSelectedFrameIndex() {
+uint32_t StackFrameList::GetSelectedFrameIndex(
+SelectMostRelevant select_most_relevant) {
   std::lock_guard guard(m_mutex);
-  if (!m_selected_frame_idx)
+  if (!m_selected_frame_idx && select_most_relevant)
 SelectMostRelevantFrame();
-  if (!m_selected_frame_idx)
+  if (!m_selected_frame_idx) {
+// If we aren't selecting the most relevant frame, and the selected frame
+// isn't set, then don't force a selection here, just return 0.
+if (!select_most_relevant)
+  return 0;
 m_selected_frame_idx = 0;
+  }
   return *m_selected_frame_idx;
 }
 
@@ -858,7 +864,8 @@
 void StackFrameList::SetDefaultFileAndLineToSelectedFrame() {
   if (m_thread.GetID() ==
   m_thread.GetProcess()->GetThreadList().GetSelectedThread()->GetID()) {
-StackFrameSP 

[Lldb-commits] [lldb] 076341d - Make sure SelectMostRelevantFrame happens only when returning to the user.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-04-21T14:21:25-07:00
New Revision: 076341d1088b0b3e0140178760dc45ac5162cd65

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

LOG: Make sure SelectMostRelevantFrame happens only when returning to the user.

This is a user facing action, it is meant to focus the user's attention on
something other than the 0th frame when you stop somewhere where that's
helpful. For instance, stopping in pthread_kill after an assert will select
the assert frame.

This is not something you want to have happen internally in lldb, both
because internally you really don't want the selected frame changing out
from under you, and because the recognizers can do arbitrary work, and that
can cause deadlocks or other unexpected behavior.

However, it's not something that the current code does
explicitly after a stop has been delivered, it's expected to happen implicitly
as part of stopping. I changing this to call SMRF explicitly after a user
stop, but that got pretty ugly quickly.

So I added a bool to control whether to run this and audited all the current
uses to determine whether we're returning to the user or not.

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

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/StackFrameList.h
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/lldb-private-enumerations.h
lldb/source/API/SBThread.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/ValueObject.cpp
lldb/source/Expression/REPL.cpp

lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp

lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp

lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
lldb/source/Target/ExecutionContext.cpp
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/StopInfo.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/Thread.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 0c90078accfc..808226067d5a 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -832,6 +832,7 @@ class Process : public 
std::enable_shared_from_this,
   /// \see Thread:Suspend()
   Status Resume();
 
+  /// Resume a process, and wait for it to stop.
   Status ResumeSynchronous(Stream *stream);
 
   /// Halts a running process.
@@ -2167,12 +2168,17 @@ class Process : public 
std::enable_shared_from_this,
   // process is hijacked and use_run_lock is true (the default), then this
   // function releases the run lock after the stop. Setting use_run_lock to
   // false will avoid this behavior.
+  // If we are waiting to stop that will return control to the user,
+  // then we also want to run SelectMostRelevantFrame, which is controlled
+  // by "select_most_relevant".
   lldb::StateType
   WaitForProcessToStop(const Timeout ,
lldb::EventSP *event_sp_ptr = nullptr,
bool wait_always = true,
lldb::ListenerSP hijack_listener = lldb::ListenerSP(),
-   Stream *stream = nullptr, bool use_run_lock = true);
+   Stream *stream = nullptr, bool use_run_lock = true,
+   SelectMostRelevant select_most_relevant =
+   DoNoSelectMostRelevantFrame);
 
   uint32_t GetIOHandlerID() const { return m_iohandler_sync.GetValue(); }
 
@@ -2210,9 +2216,10 @@ class Process : public 
std::enable_shared_from_this,
   /// \return
   /// \b true if the event describes a process state changed event, \b 
false
   /// otherwise.
-  static bool HandleProcessStateChangedEvent(const lldb::EventSP _sp,
- Stream *stream,
- bool _process_io_handler);
+  static bool
+  HandleProcessStateChangedEvent(const lldb::EventSP _sp, Stream *stream,
+ SelectMostRelevant select_most_relevant,
+ bool _process_io_handler);
 
   Event *PeekAtStateChangedEvents();
 

diff  --git a/lldb/include/lldb/Target/StackFrameList.h 

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[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/D148863/new/

https://reviews.llvm.org/D148863

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Jim Ingham via Phabricator via lldb-commits]

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

Adopted the "enum SelectMostRelevant" as Jonas suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/StackFrameList.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/API/SBThread.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -263,10 +263,11 @@
new ThreadEventData(this->shared_from_this(), new_frame_id));
 }
 
-lldb::StackFrameSP Thread::GetSelectedFrame() {
+lldb::StackFrameSP
+Thread::GetSelectedFrame(SelectMostRelevant select_most_relevant) {
   StackFrameListSP stack_frame_list_sp(GetStackFrameList());
   StackFrameSP frame_sp = stack_frame_list_sp->GetFrameAtIndex(
-  stack_frame_list_sp->GetSelectedFrameIndex());
+  stack_frame_list_sp->GetSelectedFrameIndex(select_most_relevant));
   FrameSelectedCallback(frame_sp.get());
   return frame_sp;
 }
@@ -297,7 +298,7 @@
   const bool broadcast = true;
   bool success = SetSelectedFrameByIndex(frame_idx, broadcast);
   if (success) {
-StackFrameSP frame_sp = GetSelectedFrame();
+StackFrameSP frame_sp = GetSelectedFrame(DoNoSelectMostRelevantFrame);
 if (frame_sp) {
   bool already_shown = false;
   SymbolContext frame_sc(
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3367,9 +3367,10 @@
 if (async) {
   process_sp->RestoreProcessEvents();
 } else {
-  state = process_sp->WaitForProcessToStop(std::nullopt, nullptr, false,
-   attach_info.GetHijackListener(),
-   stream);
+  // We are stopping all the way out to the user, so update selected frames.
+  state = process_sp->WaitForProcessToStop(
+  std::nullopt, nullptr, false, attach_info.GetHijackListener(), stream,
+  true, SelectMostRelevantFrame);
   process_sp->RestoreProcessEvents();
 
   if (state != eStateStopped) {
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -1456,7 +1456,8 @@
 return ValueObjectSP();
   }
 
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+  thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
 
   if (!frame_sp) {
 return ValueObjectSP();
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -815,12 +815,18 @@
   }
 }
 
-uint32_t StackFrameList::GetSelectedFrameIndex() {
+uint32_t StackFrameList::GetSelectedFrameIndex(
+SelectMostRelevant select_most_relevant) {
   std::lock_guard guard(m_mutex);
-  if (!m_selected_frame_idx)
+  if (!m_selected_frame_idx && select_most_relevant)
 SelectMostRelevantFrame();
-  if (!m_selected_frame_idx)
+  if (!m_selected_frame_idx) {
+// If we aren't selecting the most relevant frame, and the selected frame
+// isn't set, then don't force a selection here, just return 0.
+if (!select_most_relevant)
+  return 0;
 m_selected_frame_idx = 0;
+  }
   return *m_selected_frame_idx;
 }
 
@@ -858,7 +864,8 @@
 void StackFrameList::SetDefaultFileAndLineToSelectedFrame() {
   if (m_thread.GetID() ==
   m_thread.GetProcess()->GetThreadList().GetSelectedThread()->GetID()) {
-StackFrameSP frame_sp(GetFrameAtIndex(GetSelectedFrameIndex()));
+StackFrameSP 

[Lldb-commits] [PATCH] D148548: [lldb] Improve breakpoint management for interactive scripted process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 515902.
mib marked 2 inline comments as done.
mib added a comment.

Re-format python code with `black`


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

https://reviews.llvm.org/D148548

Files:
  lldb/bindings/interface/SBTargetExtensions.i
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/API/SBBreakpoint.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -139,6 +139,10 @@
   return nullptr;
 }
 
+void *lldb_private::LLDBSWIGPython_CastPyObjectToSBBreakpoint(PyObject *data) {
+  return nullptr;
+}
+
 void *lldb_private::LLDBSWIGPython_CastPyObjectToSBAttachInfo(PyObject *data) {
   return nullptr;
 }
Index: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
===
--- lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
+++ lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
@@ -6,7 +6,7 @@
 #   -o "create_sub" \
 #   -o "br set -p 'also break here'" -o 'continue'
 
-import os, json, struct, signal
+import os, json, struct, signal, tempfile
 
 from threading import Thread
 from typing import Any, Dict
@@ -152,6 +152,11 @@
 )
 )
 
+def create_breakpoint(self, addr, error, pid=None):
+if not self.multiplexer:
+error.SetErrorString("Multiplexer is not set.")
+return self.multiplexer.create_breakpoint(addr, error, self.get_process_id())
+
 def get_scripted_thread_plugin(self) -> str:
 return f"{MultiplexedScriptedThread.__module__}.{MultiplexedScriptedThread.__name__}"
 
@@ -300,6 +305,40 @@
 )
 self.multiplexed_processes = {}
 
+# Copy breakpoints from real target to passthrough
+with tempfile.NamedTemporaryFile() as tf:
+bkpt_file = lldb.SBFileSpec(tf.name)
+error = self.driving_target.BreakpointsWriteToFile(bkpt_file)
+if error.Fail():
+log(
+"Failed to save breakpoints from driving target (%s)"
+% error.GetCString()
+)
+bkpts_list = lldb.SBBreakpointList(self.target)
+error = self.target.BreakpointsCreateFromFile(bkpt_file, bkpts_list)
+if error.Fail():
+log(
+"Failed create breakpoints from driving target \
+(bkpt file: %s)"
+% tf.name
+)
+
+# Copy breakpoint from passthrough to real target
+if error.Success():
+self.driving_target.DeleteAllBreakpoints()
+for bkpt in self.target.breakpoints:
+if bkpt.IsValid():
+for bl in bkpt:
+real_bpkt = self.driving_target.BreakpointCreateBySBAddress(
+bl.GetAddress()
+)
+if not real_bpkt.IsValid():
+log(
+"Failed to set breakpoint at address %s in \
+driving target"
+% hex(bl.GetLoadAddress())
+)
+
 self.listener_thread = Thread(
 target=self.wait_for_driving_process_to_stop, daemon=True
 )
@@ -364,6 +403,47 @@
 parity = pid % 2
 return dict(filter(lambda pair: pair[0] % 2 == parity, self.threads.items()))
 
+def create_breakpoint(self, addr, error, pid=None):
+if not self.driving_target:
+error.SetErrorString("%s has no driving target." % self.__class__.__name__)

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 515899.
mib marked 2 inline comments as done.
mib added a comment.

Re-format python code with `black`


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

https://reviews.llvm.org/D145297

Files:
  lldb/test/API/functionalities/interactive_scripted_process/Makefile
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
  lldb/test/API/functionalities/interactive_scripted_process/main.cpp

Index: lldb/test/API/functionalities/interactive_scripted_process/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/main.cpp
@@ -0,0 +1,35 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void spawn_thread(int index) {
+  std::string name = "I'm thread " + std::to_string(index) + " !";
+  bool done = false;
+  std::string state = "Started execution!";
+  while (true) {
+if (done) // also break here
+  break;
+  }
+
+  state = "Stopped execution!";
+}
+
+int main() {
+  constexpr size_t num_threads = 10;
+  std::vector threads;
+
+  for (size_t i = 0; i < num_threads; i++) {
+threads.push_back(std::thread(spawn_thread, i));
+  }
+
+  std::cout << "Spawned " << threads.size() << " threads!" << std::endl; // Break here
+
+  for (auto  : threads) {
+if (t.joinable())
+  t.join();
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
@@ -0,0 +1,494 @@
+# Usage:
+# ./bin/lldb $LLVM/lldb/test/API/functionalities/interactive_scripted_process/main \
+#   -o "br set -p 'Break here'" \
+#   -o "command script import $LLVM/lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py" \
+#   -o "create_mux" \
+#   -o "create_sub" \
+#   -o "br set -p 'also break here'" -o 'continue'
+
+import os, json, struct, signal
+
+from threading import Thread
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+
+class PassthruScriptedProcess(ScriptedProcess):
+driving_target = None
+driving_process = None
+
+def __init__(
+self,
+exe_ctx: lldb.SBExecutionContext,
+args: lldb.SBStructuredData,
+launched_driving_process: bool = True,
+):
+super().__init__(exe_ctx, args)
+
+self.driving_target = None
+self.driving_process = None
+
+self.driving_target_idx = args.GetValueForKey("driving_target_idx")
+if self.driving_target_idx and self.driving_target_idx.IsValid():
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
+idx = self.driving_target_idx.GetIntegerValue(42)
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeString:
+idx = int(self.driving_target_idx.GetStringValue(100))
+self.driving_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+
+if launched_driving_process:
+self.driving_process = self.driving_target.GetProcess()
+for driving_thread in self.driving_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(
+json.dumps(
+{
+"driving_target_idx": idx,
+"thread_idx": driving_thread.GetIndexID(),
+}
+)
+)
+
+self.threads[driving_thread.GetThreadID()] = PassthruScriptedThread(
+self, structured_data
+)
+
+for module in self.driving_target.modules:
+path = module.file.fullpath
+load_addr = module.GetObjectFileHeaderAddress().GetLoadAddress(
+self.driving_target
+)
+self.loaded_images.append({"path": path, "load_addr": load_addr})
+
+def get_memory_region_containing_address(
+self, addr: int
+) -> lldb.SBMemoryRegionInfo:
+mem_region = lldb.SBMemoryRegionInfo()
+error = self.driving_process.GetMemoryRegionInfo(addr, mem_region)
+if error.Fail():
+return None
+return mem_region
+
+def read_memory_at_address(
+self, addr: int, size: int, error: lldb.SBError
+) -> lldb.SBData:
+data = lldb.SBData()
+bytes_read = self.driving_process.ReadMemory(addr, size, error)
+
+if error.Fail():
+  

[Lldb-commits] [lldb] d9bc7f7 - TestSTL was marked as skipped unconditionally in 2018, undo

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2023-04-21T13:17:43-07:00
New Revision: d9bc7f7844e803ea622f29e7367c8ba6dccde442

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

LOG: TestSTL was marked as skipped unconditionally in 2018, undo

It was avoiding a crash at the time on macOS, apparently, and
it skipped the test on all platforms.  This test passes for me
now on macOS, let's remove the skip and see how the bots go.

Added: 


Modified: 
lldb/test/API/lang/cpp/stl/TestSTL.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/stl/TestSTL.py 
b/lldb/test/API/lang/cpp/stl/TestSTL.py
index 357144cdc5bf8..004dc8002ece0 100644
--- a/lldb/test/API/lang/cpp/stl/TestSTL.py
+++ b/lldb/test/API/lang/cpp/stl/TestSTL.py
@@ -12,7 +12,6 @@
 
 class STLTestCase(TestBase):
 
-@skipIf
 @expectedFailureAll(bugnumber="llvm.org/PR36713")
 def test(self):
 """Test some expressions involving STL data types."""



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

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py:9
+
+import os,json,struct,signal
+import time

bulbazord wrote:
> mib wrote:
> > bulbazord wrote:
> > > nit: import these all on different lines
> > why :p ?
> I don't actually think it matters a ton, which is why it was a nit, but I 
> think there's something to be said for consistency with the rest of lldb.
Given the discussion in 
https://discourse.llvm.org/t/rfc-document-and-standardize-python-code-style, 
let's format new tests with `black`. It doesn't look like the tool has a 
preference between those two so I would vote for consistency. 


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

https://reviews.llvm.org/D145297

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D148863#4288186 , @mib wrote:

> It's unfortunate that we're not able to determine easily if the stop is 
> public or not, so I guess this is the best we could do for now.
>
> We could however greatly reduce the amount of changes in the patch by having 
> `select_most_relevant` default to `false` (or the `true` either way). Then, 
> to improve readability, we would only need to add an inline comment before 
> the argument (`GetSelectedFrame(/*select_most_relevant=*/true)`) like Jonas 
> suggested.
>
> Other than that, this looks good to me :)

I thought about that (it would have made my job easier) but I don't want to 
have a default argument.  I want people to have to think about who they are 
returning this too, and if it had a default argument it would be too easy to 
add it somewhere w/o thinking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/include/lldb/Target/Thread.h:433
+  // call it internally.
+  uint32_t GetSelectedFrameIndex(bool select_most_relevant) {
+return GetStackFrameList()

ditto



Comment at: lldb/include/lldb/Target/Thread.h:438
 
-  lldb::StackFrameSP GetSelectedFrame();
+  lldb::StackFrameSP GetSelectedFrame(bool select_most_relevant);
 

ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/include/lldb/Target/StackFrameList.h:56
+  /// pass false here.
+  uint32_t GetSelectedFrameIndex(bool select_most_relevant_frame);
 

I'd suggested making `select_most_relevant_frame = false` by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

It's unfortunate that we're not able to determine easily if the stop is public 
or not, so I guess this is the best we could do for now.

We could however greatly reduce the amount of changes in the patch by having 
`select_most_relevant` default to `false` (or the `true` either way). Then, to 
improve readability, we would only need to add an inline comment before the 
argument (`GetSelectedFrame(/*select_most_relevant=*/true)`) like Jonas 
suggested.

Other than that, this looks good to me :)




Comment at: lldb/include/lldb/Target/Process.h:2164
   // false will avoid this behavior.
+  // If we are waiting to stop that will return control to the user, 
+  // then we also want to run SelectMostRelevantFrame, which is controlled

typo I guess


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

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

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D148395#4287673 , @bulbazord wrote:

> In D148395#4285017 , @mib wrote:
>
>> In D148395#4270508 , @bulbazord 
>> wrote:
>>
>>> Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change 
>>> means they'll have different listeners. Is that ProcessLaunchInfo kept 
>>> around for any other reason? Or is it just made to create the 
>>> ProcessAttachInfo? This seems like a reasonable move to me, but I'm not 
>>> sure how LaunchInfo and AttachInfo might interact.
>>
>> @bulbazord Not sure what you mean ... We need to convert to the 
>> `ProcessLaunchInfo` into a `ProcessAttachInfo` when the user ask use to 
>> launch a process but we end up asking the platform to do the launch after 
>> which we attach to the process. In both cases, we use the default listener 
>> (the debugger listener if the user didn't provide a custom listener).
>
> What I'm not sure about is that the `ProcessAttachInfo` constructor we're 
> using takes a `ProcessLaunchInfo` as an argument and fills in its fields with 
> it. It used to be that `ProcessAttachInfo` would get its Listener and 
> HijackListener from the `ProcessLaunchInfo` passed in. Now, they could have 
> different Listeners and HijackListeners. When we create a `ProcessAttachInfo` 
> from a `ProcessLaunchInfo`, is it expected that these 2 things will diverge 
> in that way? Do we use the `ProcessLaunchInfo` that we build the 
> `ProcessAttachInfo` with in any other way?

We had to do that previously because `ProcessLaunchInfo` and 
`ProcessAttachInfo` were not related (they were not derived from the same base 
class). With this patch, I hoisted the the `m_listener_sp` and 
`m_hijack_listener_sp` with their getters and setters into the `ProcessInfo` 
class from which both the `ProcessAttachInfo` and `ProcessLaunchInfo` are 
derived. So by calling the copy constructor for ProcessInfo (see inline 
comment), that should also copy the listener and hijack listener from the 
process launch info into the process attach info.




Comment at: lldb/include/lldb/Target/Process.h:122
 m_async(false) {
 ProcessInfo::operator=(launch_info);
 SetProcessPluginName(launch_info.GetProcessPluginName());

Because we moved `m_listener_sp` and `m_hijack_listener_sp` to `ProcessInfo` 
and since both `Process{Attach,Launch}Info` are derived from that class, this 
should also copy the listeners.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148395

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

[Lldb-commits] [PATCH] D148752: lldb: Fix usage of sve functions on arm64

[Manoj Gupta via Phabricator via lldb-commits]

manojgupta added a comment.

Are there any objecttions? This patch is only making a consistent use of 
pre-existing and already in-use APIs/struct defintions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148752

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

[Lldb-commits] [PATCH] D147831: [lldb-vscode] Implement RestartRequest

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added a comment.

Ping.


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

https://reviews.llvm.org/D147831

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

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Closed by commit rGc1d55d26d373: [lldb] Let Mangled decide whether a name is 
mangled or not (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D148846?vs=515522=515821#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148846

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/test/API/macosx/format/Makefile
  lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
  lldb/test/API/macosx/format/main.c

Index: lldb/test/API/macosx/format/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/format/main.c
@@ -0,0 +1,6 @@
+#include 
+
+int main(int argc, char **argv) {
+  assert(argc != 1);
+  return 0;
+}
Index: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
===
--- /dev/null
+++ lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestFunctionNameWithoutArgs(TestBase):
+@skipUnlessDarwin
+@no_debug_info_test
+def test_function_name_without_args(self):
+self.build()
+target = self.createTestTarget()
+target.LaunchSimple(None, None, self.get_process_working_directory())
+
+self.runCmd("run", RUN_SUCCEEDED)
+self.expect(
+"bt",
+substrs=[
+"stop reason = hit program assert",
+"libsystem_kernel.dylib`__pthread_kill",
+],
+)
+self.runCmd(
+'settings set frame-format "frame #${frame.index}: ${function.name-without-args}\n"'
+)
+self.expect(
+"bt",
+substrs=["stop reason = hit program assert", "frame #0: __pthread_kill"],
+)
Index: lldb/test/API/macosx/format/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/format/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1980,7 +1980,7 @@
   } else if (!func_undecorated_name.empty()) {
 mangled.SetDemangledName(ConstString(func_undecorated_name));
   } else if (!func_name.empty())
-mangled.SetValue(ConstString(func_name), false);
+mangled.SetValue(ConstString(func_name));
 
   return mangled;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2417,7 +2417,7 @@
_base)) {
 Mangled func_name;
 if (mangled)
-  func_name.SetValue(ConstString(mangled), true);
+  func_name.SetValue(ConstString(mangled));
 else if ((die.GetParent().Tag() == DW_TAG_compile_unit ||
   die.GetParent().Tag() == DW_TAG_partial_unit) &&
  Language::LanguageIsCPlusPlus(
@@ -2428,9 +2428,9 @@
   // If the mangled name is not present in the DWARF, generate the
   // demangled name using the decl context. We skip if the function is
   // "main" as its name is never mangled.
-  func_name.SetValue(ConstructDemangledNameFromDWARF(die), false);
+  func_name.SetValue(ConstructDemangledNameFromDWARF(die));
 } else
-  func_name.SetValue(ConstString(name), false);
+  func_name.SetValue(ConstString(name));
 
 FunctionSP func_sp;
 std::unique_ptr decl_up;
Index: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===
--- lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -246,7 +246,7 @@
 
   if (auto record = FuncRecord::parse(*It)) {
 Mangled func_name;
-func_name.SetValue(ConstString(record->Name), false);
+func_name.SetValue(ConstString(record->Name));
 addr_t address = record->Address + base;
 SectionSP section_sp = list->FindSectionContainingFileAddress(address);
 if (section_sp) {
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

[Lldb-commits] [lldb] c1d55d2 - [lldb] Let Mangled decide whether a name is mangled or not

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-04-21T10:23:24-07:00
New Revision: c1d55d26d373e8804aba92f7d0f07de353ea93c9

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

LOG: [lldb] Let Mangled decide whether a name is mangled or not

We have a handful of places in LLDB where we try to outsmart the logic
in Mangled to determine whether a string is mangled or not. There's at
least one place (*) where we are getting this wrong and causes a subtle
bug. The `cstring_is_mangled` is cheap enough that we should always rely
on it to determine whether a string is mangled or not.

(*) `ObjectFileMachO` assumes that a symbol that starts with a double
underscore (such as `__pthread_kill`) is mangled. That's mostly
harmless, until you use `function.name-without-args` in the frame
format. The formatter calls `Symbol::GetNameNoArguments()` which is a
wrapper around `Mangled::GetName(ePreferDemangledWithoutArguments)`. The
latter will first try using the appropriate language plugin to get the
demangled name without arguments, and if that fails, falls back to
returning the demangled name. Because we forced Mangled to treat the
symbol as a mangled name (even though it's not) there's no demangled
name. The result is that frames don't show any symbol at all.

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

Added: 
lldb/test/API/macosx/format/Makefile
lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
lldb/test/API/macosx/format/main.c

Modified: 
lldb/include/lldb/Core/Mangled.h
lldb/source/Core/Mangled.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Mangled.h 
b/lldb/include/lldb/Core/Mangled.h
index dcaa7a8cda6cc..b7813313d4597 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -185,19 +185,6 @@ class Mangled {
   /// The number of bytes that this object occupies in memory.
   size_t MemorySize() const;
 
-  /// Set the string value in this object.
-  ///
-  /// If \a is_mangled is \b true, then the mangled named is set to \a name,
-  /// else the demangled name is set to \a name.
-  ///
-  /// \param[in] name
-  /// The already const version of the name for this object.
-  ///
-  /// \param[in] is_mangled
-  /// If \b true then \a name is a mangled name, if \b false then
-  /// \a name is demangled.
-  void SetValue(ConstString name, bool is_mangled);
-
   /// Set the string value in this object.
   ///
   /// This version auto detects if the string is mangled by inspecting the

diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index 0c4d9f78c4402..29fb35980036a 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -90,23 +90,6 @@ int Mangled::Compare(const Mangled , const Mangled ) {
   b.GetName(ePreferMangled));
 }
 
-// Set the string value in this objects. If "mangled" is true, then the mangled
-// named is set with the new value in "s", else the demangled name is set.
-void Mangled::SetValue(ConstString s, bool mangled) {
-  if (s) {
-if (mangled) {
-  m_demangled.Clear();
-  m_mangled = s;
-} else {
-  m_demangled = s;
-  m_mangled.Clear();
-}
-  } else {
-m_demangled.Clear();
-m_mangled.Clear();
-  }
-}
-
 void Mangled::SetValue(ConstString name) {
   if (name) {
 if (cstring_is_mangled(name.GetStringRef())) {

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 1f89db0cb5ca0..621df79e52e56 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -3030,7 +3030,7 @@ void ObjectFileMachO::ParseSymtab(Symtab ) {
   // the first contains a directory and the
   // second contains a full path.
   sym[sym_idx - 1].GetMangled().SetValue(
-  ConstString(symbol_name), false);
+  ConstString(symbol_name));
   m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
   add_nlist = false;
 } else {
@@ -3076,7 +3076,7 @@ void ObjectFileMachO::ParseSymtab(Symtab ) {
 full_so_path += '/';
   full_so_path += symbol_name;
   sym[sym_idx - 1].GetMangled().SetValue(
- 

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 3 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py:1
+import os
+import lldb

aprantl wrote:
> why?
I assume the question is rhetoric, but on the off-chance it's not: I 
copy-pasted it from another test. Same for the other "why?".


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

https://reviews.llvm.org/D148846

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

[Lldb-commits] [PATCH] D148863: Make sure SelectMostRelevantFrame happens only when returning to the user

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D148863#4285797 , @JDevlieghere 
wrote:

> I looked at all the call sites and they all seemed reasonable in terms of 
> doing work on behalf of the user or not and selecting the most relevant frame 
> respectively. My only concern is that we now have a bunch of places where 
> we're passing a blind `true` or `false`. While it's pretty obvious in this 
> patch, it won't be so obvious when you're reading the same code in the 
> future, let alone when someone inevitably copy-pastes it. I can think of two 
> easy ways to improve readability:
>
> 1. Add an inline comment:
>
>   StackFrameSP frame_sp = GetSelectedFrame(/*select_most_relevant=*/false);
>
>
>
> 2. Add an enum to `Frame`:
>
>   enum SelectMostRelevant : bool {
> SelectMostRelevantFrame = true,
> DoNoSelectMostRelevantFrame = false,
>   };
>
>
>
>   StackFrameSP frame_sp = GetSelectedFrame(SelectMostRelevantFrame);
>
> This is a trick that @dexonsmith thought me a long time ago. I personally 
> like it the best because it makes things very explicit without the hassle of 
> having to actually treat the value like an enum.

Oh, that's cute, I'll try that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148863

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

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

Looks good to me. Might want to let @jingham or @JDevlieghere give it a 
look-over though.




Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py:9
+
+import os,json,struct,signal
+import time

mib wrote:
> bulbazord wrote:
> > nit: import these all on different lines
> why :p ?
I don't actually think it matters a ton, which is why it was a nit, but I think 
there's something to be said for consistency with the rest of lldb.


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

https://reviews.llvm.org/D145297

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

[Lldb-commits] [PATCH] D148548: [lldb] Improve breakpoint management for interactive scripted process

[Alex Langford via Phabricator via lldb-commits]

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

one nit but otherwise LGTM.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:124
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
+return {};
+

JDevlieghere wrote:
> return false
+1


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

https://reviews.llvm.org/D148548

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

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D148395#4285017 , @mib wrote:

> In D148395#4270508 , @bulbazord 
> wrote:
>
>> Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change means 
>> they'll have different listeners. Is that ProcessLaunchInfo kept around for 
>> any other reason? Or is it just made to create the ProcessAttachInfo? This 
>> seems like a reasonable move to me, but I'm not sure how LaunchInfo and 
>> AttachInfo might interact.
>
> @bulbazord Not sure what you mean ... We need to convert to the 
> `ProcessLaunchInfo` into a `ProcessAttachInfo` when the user ask use to 
> launch a process but we end up asking the platform to do the launch after 
> which we attach to the process. In both cases, we use the default listener 
> (the debugger listener if the user didn't provide a custom listener).

What I'm not sure about is that the `ProcessAttachInfo` constructor we're using 
takes a `ProcessLaunchInfo` as an argument and fills in its fields with it. It 
used to be that `ProcessAttachInfo` would get its Listener and HijackListener 
from the `ProcessLaunchInfo` passed in. Now, they could have different 
Listeners and HijackListeners. When we create a `ProcessAttachInfo` from a 
`ProcessLaunchInfo`, is it expected that these 2 things will diverge in that 
way? Do we use the `ProcessLaunchInfo` that we build the `ProcessAttachInfo` 
with in any other way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148395

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

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py:7
+
+
+class TestFunctionNameWithoutArgs(TestBase):

I guess this is a no debug info test?


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

https://reviews.llvm.org/D148846

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

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py:1
+import os
+import lldb

why?



Comment at: lldb/test/API/macosx/format/main.c:2
+#include 
+#include 
+

why?


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

https://reviews.llvm.org/D148846

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

[Lldb-commits] [PATCH] D148846: [lldb] Let Mangled decide whether a name is mangled or not

[Adrian Prantl via Phabricator via lldb-commits]

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

It seems like we can quickly and unambiguously decide whether a name is mangled 
or not by looking at the first characters so that seems fine to me. Does 
Mangled try all Language plugins to decide?
This seems like a risky change, but I think it's going into the right direction.


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

https://reviews.llvm.org/D148846

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

[Lldb-commits] [PATCH] D148823: Make diagnostics API safer to use

[Adrian Prantl via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG737820e6d6e2: Make diagnostics API safer to use (authored by 
aprantl).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148823

Files:
  lldb/include/lldb/Expression/DiagnosticManager.h
  lldb/unittests/Expression/DiagnosticManagerTest.cpp


Index: lldb/unittests/Expression/DiagnosticManagerTest.cpp
===
--- lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -75,6 +75,9 @@
 TEST(DiagnosticManagerTest, GetStringNoDiags) {
   DiagnosticManager mgr;
   EXPECT_EQ("", mgr.GetString());
+  std::unique_ptr empty;
+  mgr.AddDiagnostic(std::move(empty));
+  EXPECT_EQ("", mgr.GetString());
 }
 
 TEST(DiagnosticManagerTest, GetStringBasic) {
Index: lldb/include/lldb/Expression/DiagnosticManager.h
===
--- lldb/include/lldb/Expression/DiagnosticManager.h
+++ lldb/include/lldb/Expression/DiagnosticManager.h
@@ -114,7 +114,8 @@
   }
 
   void AddDiagnostic(std::unique_ptr diagnostic) {
-m_diagnostics.push_back(std::move(diagnostic));
+if (diagnostic)
+  m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)


Index: lldb/unittests/Expression/DiagnosticManagerTest.cpp
===
--- lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -75,6 +75,9 @@
 TEST(DiagnosticManagerTest, GetStringNoDiags) {
   DiagnosticManager mgr;
   EXPECT_EQ("", mgr.GetString());
+  std::unique_ptr empty;
+  mgr.AddDiagnostic(std::move(empty));
+  EXPECT_EQ("", mgr.GetString());
 }
 
 TEST(DiagnosticManagerTest, GetStringBasic) {
Index: lldb/include/lldb/Expression/DiagnosticManager.h
===
--- lldb/include/lldb/Expression/DiagnosticManager.h
+++ lldb/include/lldb/Expression/DiagnosticManager.h
@@ -114,7 +114,8 @@
   }
 
   void AddDiagnostic(std::unique_ptr diagnostic) {
-m_diagnostics.push_back(std::move(diagnostic));
+if (diagnostic)
+  m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 737820e - Make diagnostics API safer to use

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2023-04-21T08:21:56-07:00
New Revision: 737820e6d6e2df50f2ddf522a0db9ffb794ff749

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

LOG: Make diagnostics API safer to use

I received a crash report in DiagnosticManager that was caused by a
nullptr diagnostic having been added. The API allows passing in a null
unique_ptr, but all the methods are written assuming that all pointers
a dereferencable. This patch makes it impossible to add a null
diagnostic.

rdar://107633615

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

Added: 


Modified: 
lldb/include/lldb/Expression/DiagnosticManager.h
lldb/unittests/Expression/DiagnosticManagerTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Expression/DiagnosticManager.h 
b/lldb/include/lldb/Expression/DiagnosticManager.h
index c0271c954ba3f..df9ba3b245f51 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -114,7 +114,8 @@ class DiagnosticManager {
   }
 
   void AddDiagnostic(std::unique_ptr diagnostic) {
-m_diagnostics.push_back(std::move(diagnostic));
+if (diagnostic)
+  m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)

diff  --git a/lldb/unittests/Expression/DiagnosticManagerTest.cpp 
b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
index 3cfb5ed0b66bb..cab26debedb14 100644
--- a/lldb/unittests/Expression/DiagnosticManagerTest.cpp
+++ b/lldb/unittests/Expression/DiagnosticManagerTest.cpp
@@ -75,6 +75,9 @@ TEST(DiagnosticManagerTest, HasFixits) {
 TEST(DiagnosticManagerTest, GetStringNoDiags) {
   DiagnosticManager mgr;
   EXPECT_EQ("", mgr.GetString());
+  std::unique_ptr empty;
+  mgr.AddDiagnostic(std::move(empty));
+  EXPECT_EQ("", mgr.GetString());
 }
 
 TEST(DiagnosticManagerTest, GetStringBasic) {



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

[Lldb-commits] [lldb] 70d8c01 - [lldb] Update QEMU git URL in setup.sh

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-04-21T14:31:39Z
New Revision: 70d8c01e45741ca70f1da16f1ad61112d90b6474

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

LOG: [lldb] Update QEMU git URL in setup.sh

The old name was maintained for a while but now it has been changed
to gitlab.

Added: 


Modified: 
lldb/scripts/lldb-test-qemu/setup.sh

Removed: 




diff  --git a/lldb/scripts/lldb-test-qemu/setup.sh 
b/lldb/scripts/lldb-test-qemu/setup.sh
index c89dc16820007..c2389b35756cb 100644
--- a/lldb/scripts/lldb-test-qemu/setup.sh
+++ b/lldb/scripts/lldb-test-qemu/setup.sh
@@ -45,7 +45,7 @@ build_qemu() {
   # Checkout source code
   check_dir_exists "qemu.git"
   if [ ! -d "qemu.git" ]; then
-git clone --depth 1 git://git.qemu.org/qemu.git qemu.git
+git clone --depth 1 https://gitlab.com/qemu-project/qemu.git qemu.git
   fi
 
   cd qemu.git



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

[Lldb-commits] [lldb] c9083be - [LLDB] Don't print register fields when asked for a specific format

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-04-21T11:47:05Z
New Revision: c9083bea168699b4078b27ce88fd35562751f46f

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

LOG: [LLDB] Don't print register fields when asked for a specific format

Previously if a register had fields we would always print them after the
value if the register was asked for by name.
```
(lldb) register read MDCR_EL3
MDCR_EL3 = 0x
 = {
 ETBAD = 0
<...>
 RLTE = 0
   }
```
This can be quite annoying if there are a whole lot of fields but you
want to see the register in a specific format.
```
(lldb) register read MDCR_EL3 -f i
MDCR_EL3 = 0x   unknown udf#0x0
 = {
 ETBAD = 0
<...lots of fields...>
```
Since it pushes the interesting bit far up the terminal. To solve this,
don't print fields if the user passes --format. If they're doing that
then I think it's reasonable to assume they know what they want and only
want to see that output.

This also gives users a way to silence fields, but not change the format.
By doing `register read foo -f x`. In case they are not useful or perhaps
they are trying to work around a crash.

I have customised the help text for --format for register read to explain this:
```
-f  ( --format  )
 Specify a format to be used for display. If this is set, register fields 
will not be dispayed.
```

Reviewed By: jasonmolenda

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectRegister.cpp
lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectRegister.cpp 
b/lldb/source/Commands/CommandObjectRegister.cpp
index 8784b8b9977a8..a6ea64229eccb 100644
--- a/lldb/source/Commands/CommandObjectRegister.cpp
+++ b/lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@ class CommandObjectRegisterRead : public CommandObjectParsed 
{
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be displayed."}}) 
{
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,12 @@ class CommandObjectRegisterRead : public 
CommandObjectParsed {
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags =
+!m_format_options.GetFormatValue().OptionWasSet();
 if (!DumpRegister(m_exe_ctx, strm, *reg_ctx, *reg_info,
-  /*print_flags=*/true))
+  print_flags))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);
   } else {
 result.AppendErrorWithFormat("Invalid register name '%s'.\n",

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
index 3ccf3ccae6e9b..e6a80f3c79253 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@ def test_flags_child_limit(self):
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 
1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 
0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], 
matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):



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

[Lldb-commits] [PATCH] D148790: [LLDB] Don't print register fields when asked for a specific format

[David Spickett via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc9083bea1686: [LLDB] Dont print register fields when 
asked for a specific format (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148790

Files:
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 
1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 
0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], 
matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be displayed."}}) 
{
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,12 @@
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags =
+!m_format_options.GetFormatValue().OptionWasSet();
 if (!DumpRegister(m_exe_ctx, strm, *reg_ctx, *reg_info,
-  /*print_flags=*/true))
+  print_flags))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);
   } else {
 result.AppendErrorWithFormat("Invalid register name '%s'.\n",


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be displayed."}}) {
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,12 @@
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags =
+!m_format_options.GetFormatValue().OptionWasSet();
 if 

[Lldb-commits] [PATCH] D148790: [LLDB] Don't print register fields when asked for a specific format

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 515669.
DavidSpickett added a comment.

dispayed -> displayed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148790

Files:
  lldb/source/Commands/CommandObjectRegister.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 
1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 
0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], 
matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be displayed."}}) 
{
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,12 @@
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags =
+!m_format_options.GetFormatValue().OptionWasSet();
 if (!DumpRegister(m_exe_ctx, strm, *reg_ctx, *reg_info,
-  /*print_flags=*/true))
+  print_flags))
   strm.Printf("%-12s = error: unavailable\n", reg_info->name);
   } else {
 result.AppendErrorWithFormat("Invalid register name '%s'.\n",


Index: lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py
@@ -410,6 +410,15 @@
 
 self.expect("register read cpsr", substrs=["= (field_2 = 1, field_1 = 1, ...)"])
 
+@skipIfXmlSupportMissing
+@skipIfRemote
+def test_format_disables_flags(self):
+# If asked for a specific format, don't print flags after it.
+self.setup_flags_test('')
+
+self.expect("register read cpsr --format X", substrs=["cpsr = 0x"])
+self.expect("register read cpsr --format X", substrs=["field_0"], matching=False)
+
 @skipIfXmlSupportMissing
 @skipIfRemote
 def test_xml_includes(self):
Index: lldb/source/Commands/CommandObjectRegister.cpp
===
--- lldb/source/Commands/CommandObjectRegister.cpp
+++ lldb/source/Commands/CommandObjectRegister.cpp
@@ -44,7 +44,10 @@
 nullptr,
 eCommandRequiresFrame | eCommandRequiresRegContext |
 eCommandProcessMustBeLaunched | eCommandProcessMustBePaused),
-m_format_options(eFormatDefault) {
+m_format_options(eFormatDefault, UINT64_MAX, UINT64_MAX,
+ {{CommandArgumentType::eArgTypeFormat,
+   "Specify a format to be used for display. If this "
+   "is set, register fields will not be displayed."}}) {
 CommandArgumentEntry arg;
 CommandArgumentData register_arg;
 
@@ -215,8 +218,12 @@
 
   if (const RegisterInfo *reg_info =
   reg_ctx->GetRegisterInfoByName(arg_str)) {
+// If they have asked for a specific format don't obscure that by
+// printing flags afterwards.
+bool print_flags =
+!m_format_options.GetFormatValue().OptionWasSet();
 if (!DumpRegister(m_exe_ctx, strm, *reg_ctx, *reg_info,
-  /*print_flags=*/true))
+  print_flags))