[Lldb-commits] [PATCH] D103391: [lldb] Add missing reproducer instrumentation to some SB classes

[Bruce Mitchener via Phabricator via lldb-commits]

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

The change to `SBDebugger::GetProgressFromEvent` needs to be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103391

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

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

[Bruce Mitchener via Phabricator via lldb-commits]

brucem added a comment.

It failed because on macOS, the underlying args contains argv0 while on Linux 
it does not. So my test that checked that `GetNumArguments` is 1 fails on 
macOS, since there are 2.

Ideally, each platform would agree on whether or not the argv0 was included in 
the args ... or I can modify the test to check that the last argument is 
`--abc`.

(I've just been busy this week, so I hadn't gotten back to this.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103675

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

[Lldb-commits] [lldb] 8fb53dc - Improve materializer error messages to include type names.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2021-06-11T17:59:00-07:00
New Revision: 8fb53dca2f61d2d15204ae0046339ee60b7f3b54

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

LOG: Improve materializer error messages to include type names.

rdar://79201552

Added: 


Modified: 
lldb/source/Expression/Materializer.cpp

Removed: 




diff  --git a/lldb/source/Expression/Materializer.cpp 
b/lldb/source/Expression/Materializer.cpp
index a93c127dd0d0..3945f3a70f75 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -794,13 +794,15 @@ class EntityResultVariable : public Materializer::Entity {
 
   llvm::Optional byte_size = m_type.GetByteSize(exe_scope);
   if (!byte_size) {
-err.SetErrorString("can't get size of type");
+err.SetErrorStringWithFormat("can't get size of type \"%s\"",
+ m_type.GetTypeName().AsCString());
 return;
   }
 
   llvm::Optional opt_bit_align = m_type.GetTypeBitAlign(exe_scope);
   if (!opt_bit_align) {
-err.SetErrorString("can't get the type alignment");
+err.SetErrorStringWithFormat("can't get the alignment of type  \"%s\"",
+ m_type.GetTypeName().AsCString());
 return;
   }
 



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

[Lldb-commits] [PATCH] D104162: NFC Fix the handling of BreakpointOptions - return references to make it clear when you will get a valid object

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: aprantl, shafik.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

  Many of the API's that returned BreakpointOptions always returned valid ones.
  However, internally the BreakpointLocations usually have null 
BreakpointOptions, since they
  use their owner's options until an option is set specifically on the location.
  So the original code used pointers & unique_ptr everywhere for consistency.
  But that made the code hard to reason about from the outside, since it wasn't 
clear 
  when you did and didn't have to validate the pointers.
  
  This patch changes the code so that everywhere an API is guaranteed to
  return a non-null BreakpointOption, it returns it as a reference to make
  that clear.
  
  It also changes the Breakpoint to hold a BreakpointOption
  member where it previously had a UP.  Since we were always filling the UP
  in the Breakpoint constructor, having the UP wasn't helping anything.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104162

Files:
  lldb/include/lldb/Breakpoint/Breakpoint.h
  lldb/include/lldb/Breakpoint/BreakpointLocation.h
  lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
  lldb/include/lldb/Breakpoint/BreakpointSite.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBBreakpoint.cpp
  lldb/source/API/SBBreakpointLocation.cpp
  lldb/source/API/SBBreakpointName.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointLocation.cpp
  lldb/source/Breakpoint/BreakpointLocationCollection.cpp
  lldb/source/Breakpoint/BreakpointName.cpp
  lldb/source/Breakpoint/BreakpointOptions.cpp
  lldb/source/Breakpoint/BreakpointSite.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/StopInfo.cpp

Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -125,7 +125,7 @@
   BreakpointSiteSP bp_site_sp(
   process_sp->GetBreakpointSiteList().FindByID(m_value));
   if (bp_site_sp)
-return bp_site_sp->ValidForThisThread();
+return bp_site_sp->ValidForThisThread(thread);
 }
 return false;
   }
@@ -413,7 +413,7 @@
 // The breakpoint site may have many locations associated with it,
 // not all of them valid for this thread.  Skip the ones that
 // aren't:
-if (!bp_loc_sp->ValidForThisThread(thread_sp.get())) {
+if (!bp_loc_sp->ValidForThisThread(*thread_sp)) {
   if (log) {
 LLDB_LOGF(log,
   "Breakpoint %s hit on thread 0x%llx but it was not "
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -244,7 +244,7 @@
   std::unique_ptr AcquireInterpreterLock() override;
 
   void CollectDataForBreakpointCommandCallback(
-  std::vector _options_vec,
+  std::vector> _options_vec,
   CommandReturnObject ) override;
 
   void
@@ -252,20 +252,19 @@
   CommandReturnObject ) override;
 
   /// Set the callback body text into the callback for the breakpoint.
-  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  Status SetBreakpointCommandCallback(BreakpointOptions _options,
   const char *callback_body) override;
 
   Status SetBreakpointCommandCallbackFunction(
-  BreakpointOptions *bp_options,
-  const char *function_name,
+  BreakpointOptions _options, const char *function_name,
   StructuredData::ObjectSP extra_args_sp) override;
 
   /// This one is for deserialization:
   Status SetBreakpointCommandCallback(
-  BreakpointOptions *bp_options,
+  BreakpointOptions _options,
   std::unique_ptr _up) override;
 
-  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  Status SetBreakpointCommandCallback(BreakpointOptions _options,
   const char *command_body_text,
   StructuredData::ObjectSP 

[Lldb-commits] [lldb] 4fc93a3 - Allow signposts to take advantage of deferred string substitution

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2021-06-11T16:52:34-07:00
New Revision: 4fc93a3a1f95ef5a0a57750fc621f2411ea445a8

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

LOG: Allow signposts to take advantage of deferred string substitution

One nice feature of the os_signpost API is that format string
substitutions happen in the consumer, not the logging
application. LLVM's current Signpost class doesn't take advantage of
this though and instead always uses a static "Begin/End %s" format
string.

This patch uses variadic macros to allow the API to be used as
intended. Unfortunately, the primary use-case I had in mind (the
LLDB_SCOPED_TIMER() macro) does not get much better from this, because
__PRETTY_FUNCTION__ is *not* a macro, but a static string, so
signposts created by LLDB_SCOPED_TIMER() still use a static "%s"
format string. At least LLDB_SCOPED_TIMERF() works as intended.

This reapplies the previsously reverted patch with support for
platforms where signposts are unavailable.

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

Added: 


Modified: 
lldb/include/lldb/Utility/Timer.h
lldb/source/Utility/Timer.cpp
llvm/include/llvm/Support/Signposts.h
llvm/lib/Support/Signposts.cpp
llvm/lib/Support/Timer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Timer.h 
b/lldb/include/lldb/Utility/Timer.h
index 2b39881de89e..1e47ac7afc82 100644
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -10,10 +10,16 @@
 #define LLDB_UTILITY_TIMER_H
 
 #include "lldb/lldb-defines.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Signposts.h"
 #include 
 #include 
 
+namespace llvm {
+  class SignpostEmitter;
+}
+
 namespace lldb_private {
 class Stream;
 
@@ -72,15 +78,28 @@ class Timer {
   const Timer =(const Timer &) = delete;
 };
 
+llvm::SignpostEmitter ();
+
 } // namespace lldb_private
 
 // Use a format string because LLVM_PRETTY_FUNCTION might not be a string
 // literal.
 #define LLDB_SCOPED_TIMER()
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION)
-#define LLDB_SCOPED_TIMERF(...)
\
+  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION);   
\
+  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),  
\
+  &_scoped_timer, "%s", LLVM_PRETTY_FUNCTION); 
\
+  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {   
\
+::lldb_private::GetSignposts().endInterval(&_scoped_timer);
\
+  })
+
+#define LLDB_SCOPED_TIMERF(FMT, ...)   
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, __VA_ARGS__)
+  ::lldb_private::Timer _scoped_timer(_cat, FMT, __VA_ARGS__); 
\
+  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),  
\
+  &_scoped_timer, FMT, __VA_ARGS__);   
\
+  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {   
\
+::lldb_private::GetSignposts().endInterval(&_scoped_timer);
\
+  })
 
 #endif // LLDB_UTILITY_TIMER_H

diff  --git a/lldb/source/Utility/Timer.cpp b/lldb/source/Utility/Timer.cpp
index 2f3afe4c8703..b59ce3b9f556 100644
--- a/lldb/source/Utility/Timer.cpp
+++ b/lldb/source/Utility/Timer.cpp
@@ -33,6 +33,8 @@ static std::atomic g_categories;
 /// Allows llvm::Timer to emit signposts when supported.
 static llvm::ManagedStatic Signposts;
 
+llvm::SignpostEmitter _private::GetSignposts() { return *Signposts; }
+
 std::atomic Timer::g_quiet(true);
 std::atomic Timer::g_display_depth(0);
 static std::mutex () {
@@ -59,7 +61,6 @@ void Timer::SetQuiet(bool value) { g_quiet = value; }
 
 Timer::Timer(Timer::Category , const char *format, ...)
 : m_category(category), m_total_start(std::chrono::steady_clock::now()) {
-  Signposts->startInterval(this, m_category.GetName());
   TimerStack  = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
@@ -86,8 +87,6 @@ Timer::~Timer() {
   auto total_dur = stop_time - m_total_start;
   auto timer_dur = total_dur - m_child_duration;
 
-  Signposts->endInterval(this, m_category.GetName());
-
   TimerStack  = GetTimerStackForCurrentThread();
   if (g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());

diff  --git a/llvm/include/llvm/Support/Signposts.h 
b/llvm/include/llvm/Support/Signposts.h
index d31f3c1e6eb5..be01eb7854e5 100644
--- 

[Lldb-commits] [lldb] b90f9be - Revert "Allow signposts to take advantage of deferred string substitution"

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2021-06-11T16:46:34-07:00
New Revision: b90f9bea9673d07f2764006eb15fc4ea5e23be6d

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

LOG: Revert "Allow signposts to take advantage of deferred string substitution"

I forgot to make the LLDB macro conditional on Linux.

This reverts commit 541ccd1c1bb23e1e20a382844b35312c0caffd79.

Added: 


Modified: 
lldb/include/lldb/Utility/Timer.h
lldb/source/Utility/Timer.cpp
llvm/include/llvm/Support/Signposts.h
llvm/lib/Support/Signposts.cpp
llvm/lib/Support/Timer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Timer.h 
b/lldb/include/lldb/Utility/Timer.h
index 1e47ac7afc82..2b39881de89e 100644
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -10,16 +10,10 @@
 #define LLDB_UTILITY_TIMER_H
 
 #include "lldb/lldb-defines.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Chrono.h"
-#include "llvm/Support/Signposts.h"
 #include 
 #include 
 
-namespace llvm {
-  class SignpostEmitter;
-}
-
 namespace lldb_private {
 class Stream;
 
@@ -78,28 +72,15 @@ class Timer {
   const Timer =(const Timer &) = delete;
 };
 
-llvm::SignpostEmitter ();
-
 } // namespace lldb_private
 
 // Use a format string because LLVM_PRETTY_FUNCTION might not be a string
 // literal.
 #define LLDB_SCOPED_TIMER()
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION);   
\
-  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),  
\
-  &_scoped_timer, "%s", LLVM_PRETTY_FUNCTION); 
\
-  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {   
\
-::lldb_private::GetSignposts().endInterval(&_scoped_timer);
\
-  })
-
-#define LLDB_SCOPED_TIMERF(FMT, ...)   
\
+  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION)
+#define LLDB_SCOPED_TIMERF(...)
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, FMT, __VA_ARGS__); 
\
-  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),  
\
-  &_scoped_timer, FMT, __VA_ARGS__);   
\
-  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {   
\
-::lldb_private::GetSignposts().endInterval(&_scoped_timer);
\
-  })
+  ::lldb_private::Timer _scoped_timer(_cat, __VA_ARGS__)
 
 #endif // LLDB_UTILITY_TIMER_H

diff  --git a/lldb/source/Utility/Timer.cpp b/lldb/source/Utility/Timer.cpp
index b59ce3b9f556..2f3afe4c8703 100644
--- a/lldb/source/Utility/Timer.cpp
+++ b/lldb/source/Utility/Timer.cpp
@@ -33,8 +33,6 @@ static std::atomic g_categories;
 /// Allows llvm::Timer to emit signposts when supported.
 static llvm::ManagedStatic Signposts;
 
-llvm::SignpostEmitter _private::GetSignposts() { return *Signposts; }
-
 std::atomic Timer::g_quiet(true);
 std::atomic Timer::g_display_depth(0);
 static std::mutex () {
@@ -61,6 +59,7 @@ void Timer::SetQuiet(bool value) { g_quiet = value; }
 
 Timer::Timer(Timer::Category , const char *format, ...)
 : m_category(category), m_total_start(std::chrono::steady_clock::now()) {
+  Signposts->startInterval(this, m_category.GetName());
   TimerStack  = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
@@ -87,6 +86,8 @@ Timer::~Timer() {
   auto total_dur = stop_time - m_total_start;
   auto timer_dur = total_dur - m_child_duration;
 
+  Signposts->endInterval(this, m_category.GetName());
+
   TimerStack  = GetTimerStackForCurrentThread();
   if (g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());

diff  --git a/llvm/include/llvm/Support/Signposts.h 
b/llvm/include/llvm/Support/Signposts.h
index 66675313c083..d31f3c1e6eb5 100644
--- a/llvm/include/llvm/Support/Signposts.h
+++ b/llvm/include/llvm/Support/Signposts.h
@@ -18,17 +18,8 @@
 #define LLVM_SUPPORT_SIGNPOSTS_H
 
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Config/config.h"
 #include 
 
-#if LLVM_SUPPORT_XCODE_SIGNPOSTS
-#include 
-#include 
-#endif
-
-#define SIGNPOSTS_AVAILABLE()  
\
-  __builtin_available(macos 10.14, iOS 12, tvOS 12, watchOS 5, *)
-
 namespace llvm {
 class SignpostEmitterImpl;
 
@@ -45,27 +36,8 @@ class SignpostEmitter {
 
   /// Begin a signposted interval for a given object.
   void startInterval(const void *O, StringRef Name);
-
-#if LLVM_SUPPORT_XCODE_SIGNPOSTS
-  os_log_t () const;
-  

[Lldb-commits] [PATCH] D103575: Allow signposts to take advantage of deferred string substitution

[Adrian Prantl 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 rG541ccd1c1bb2: Allow signposts to take advantage of deferred 
string substitution (authored by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103575

Files:
  lldb/include/lldb/Utility/Timer.h
  lldb/source/Utility/Timer.cpp
  llvm/include/llvm/Support/Signposts.h
  llvm/lib/Support/Signposts.cpp
  llvm/lib/Support/Timer.cpp

Index: llvm/lib/Support/Timer.cpp
===
--- llvm/lib/Support/Timer.cpp
+++ llvm/lib/Support/Timer.cpp
@@ -174,7 +174,7 @@
   Running = false;
   Time += TimeRecord::getCurrentTime(false);
   Time -= StartTime;
-  Signposts->endInterval(this, getName());
+  Signposts->endInterval(this);
 }
 
 void Timer::clear() {
Index: llvm/lib/Support/Signposts.cpp
===
--- llvm/lib/Support/Signposts.cpp
+++ llvm/lib/Support/Signposts.cpp
@@ -10,19 +10,14 @@
 #include "llvm/Support/Signposts.h"
 #include "llvm/Support/Timer.h"
 
-#include "llvm/Config/config.h"
 #if LLVM_SUPPORT_XCODE_SIGNPOSTS
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Mutex.h"
-#include 
-#include 
 #endif // if LLVM_SUPPORT_XCODE_SIGNPOSTS
 
 using namespace llvm;
 
 #if LLVM_SUPPORT_XCODE_SIGNPOSTS
-#define SIGNPOSTS_AVAILABLE()  \
-  __builtin_available(macos 10.14, iOS 12, tvOS 12, watchOS 5, *)
 namespace {
 os_log_t *LogCreator() {
   os_log_t *X = new os_log_t;
@@ -40,13 +35,13 @@
 namespace llvm {
 class SignpostEmitterImpl {
   using LogPtrTy = std::unique_ptr;
-  using LogTy = LogPtrTy::element_type;
 
   LogPtrTy SignpostLog;
   DenseMap Signposts;
   sys::SmartMutex Mutex;
 
-  LogTy () const { return *SignpostLog; }
+public:
+  os_log_t () const { return *SignpostLog; }
   os_signpost_id_t getSignpostForObject(const void *O) {
 sys::SmartScopedLock Lock(Mutex);
 const auto  = Signposts.find(O);
@@ -60,7 +55,6 @@
 return Inserted.first->second;
   }
 
-public:
   SignpostEmitterImpl() : SignpostLog(LogCreator()) {}
 
   bool isEnabled() const {
@@ -79,7 +73,7 @@
 }
   }
 
-  void endInterval(const void *O, llvm::StringRef Name) {
+  void endInterval(const void *O) {
 if (isEnabled()) {
   if (SIGNPOSTS_AVAILABLE()) {
 // Both strings used here are required to be constant literal strings.
@@ -125,10 +119,17 @@
 #endif // if !HAVE_ANY_SIGNPOST_IMPL
 }
 
-void SignpostEmitter::endInterval(const void *O, StringRef Name) {
+#if HAVE_ANY_SIGNPOST_IMPL
+os_log_t ::getLogger() const { return Impl->getLogger(); }
+os_signpost_id_t SignpostEmitter::getSignpostForObject(const void *O) {
+  return Impl->getSignpostForObject(O);
+}
+#endif
+
+void SignpostEmitter::endInterval(const void *O) {
 #if HAVE_ANY_SIGNPOST_IMPL
   if (Impl == nullptr)
 return;
-  Impl->endInterval(O, Name);
+  Impl->endInterval(O);
 #endif // if !HAVE_ANY_SIGNPOST_IMPL
 }
Index: llvm/include/llvm/Support/Signposts.h
===
--- llvm/include/llvm/Support/Signposts.h
+++ llvm/include/llvm/Support/Signposts.h
@@ -18,8 +18,17 @@
 #define LLVM_SUPPORT_SIGNPOSTS_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Config/config.h"
 #include 
 
+#if LLVM_SUPPORT_XCODE_SIGNPOSTS
+#include 
+#include 
+#endif
+
+#define SIGNPOSTS_AVAILABLE()  \
+  __builtin_available(macos 10.14, iOS 12, tvOS 12, watchOS 5, *)
+
 namespace llvm {
 class SignpostEmitterImpl;
 
@@ -36,8 +45,27 @@
 
   /// Begin a signposted interval for a given object.
   void startInterval(const void *O, StringRef Name);
+
+#if LLVM_SUPPORT_XCODE_SIGNPOSTS
+  os_log_t () const;
+  os_signpost_id_t getSignpostForObject(const void *O);
+#endif
+
+  /// A macro to take advantage of the special format string handling
+  /// in the os_signpost API. The format string substitution is
+  /// deferred to the log consumer and done outside of the
+  /// application.
+#define SIGNPOST_EMITTER_START_INTERVAL(SIGNPOST_EMITTER, O, ...)  \
+  do { \
+if ((SIGNPOST_EMITTER).isEnabled())\
+  if (SIGNPOSTS_AVAILABLE())   \
+os_signpost_interval_begin((SIGNPOST_EMITTER).getLogger(), \
+   (SIGNPOST_EMITTER).getSignpostForObject(O), \
+   "LLVM Timers", __VA_ARGS__);\
+  } while (0)
+
   /// End a signposted interval for a given object.
-  void endInterval(const void *O, StringRef Name);
+  void endInterval(const void *O);
 };
 
 } // end namespace 

[Lldb-commits] [lldb] 635b721 - Disambiguate usage of struct mach_header and other MachO definitions.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2021-06-11T16:35:43-07:00
New Revision: 635b72136e95173e74d45ae66eca714d7e4e4f87

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

LOG: Disambiguate usage of struct mach_header and other MachO definitions.

Unfortunately the Darwin signpost header also pulls in the system
MachO header and so we need to make sure to use the LLVM versions of
those definitions.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 0285e336df80..e015de5ba117 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -64,6 +64,68 @@
 
 #include 
 
+// Unfortunately the signpost header pulls in the system MachO header, too.
+#ifdef CPU_TYPE_ARM
+#undef CPU_TYPE_ARM
+#endif
+#ifdef CPU_TYPE_ARM64
+#undef CPU_TYPE_ARM64
+#endif
+#ifdef CPU_TYPE_ARM64_32
+#undef CPU_TYPE_ARM64_32
+#endif
+#ifdef CPU_TYPE_I386
+#undef CPU_TYPE_I386
+#endif
+#ifdef CPU_TYPE_X86_64
+#undef CPU_TYPE_X86_64
+#endif
+#ifdef MH_DYLINKER
+#undef MH_DYLINKER
+#endif
+#ifdef MH_OBJECT
+#undef MH_OBJECT
+#endif
+#ifdef LC_VERSION_MIN_MACOSX
+#undef LC_VERSION_MIN_MACOSX
+#endif
+#ifdef LC_VERSION_MIN_IPHONEOS
+#undef LC_VERSION_MIN_IPHONEOS
+#endif
+#ifdef LC_VERSION_MIN_TVOS
+#undef LC_VERSION_MIN_TVOS
+#endif
+#ifdef LC_VERSION_MIN_WATCHOS
+#undef LC_VERSION_MIN_WATCHOS
+#endif
+#ifdef LC_BUILD_VERSION
+#undef LC_BUILD_VERSION
+#endif
+#ifdef PLATFORM_MACOS
+#undef PLATFORM_MACOS
+#endif
+#ifdef PLATFORM_MACCATALYST
+#undef PLATFORM_MACCATALYST
+#endif
+#ifdef PLATFORM_IOS
+#undef PLATFORM_IOS
+#endif
+#ifdef PLATFORM_IOSSIMULATOR
+#undef PLATFORM_IOSSIMULATOR
+#endif
+#ifdef PLATFORM_TVOS
+#undef PLATFORM_TVOS
+#endif
+#ifdef PLATFORM_TVOSSIMULATOR
+#undef PLATFORM_TVOSSIMULATOR
+#endif
+#ifdef PLATFORM_WATCHOS
+#undef PLATFORM_WATCHOS
+#endif
+#ifdef PLATFORM_WATCHOSSIMULATOR
+#undef PLATFORM_WATCHOSSIMULATOR
+#endif
+
 #define THUMB_ADDRESS_BIT_MASK 0xfffeull
 using namespace lldb;
 using namespace lldb_private;
@@ -741,11 +803,11 @@ static uint32_t MachHeaderSizeFromMagic(uint32_t magic) {
   switch (magic) {
   case MH_MAGIC:
   case MH_CIGAM:
-return sizeof(struct mach_header);
+return sizeof(struct llvm::MachO::mach_header);
 
   case MH_MAGIC_64:
   case MH_CIGAM_64:
-return sizeof(struct mach_header_64);
+return sizeof(struct llvm::MachO::mach_header_64);
 break;
 
   default:
@@ -1064,7 +1126,7 @@ bool ObjectFileMachO::ParseHeader() {
 // None found.
 return false;
   } else {
-memset(_header, 0, sizeof(struct mach_header));
+memset(_header, 0, sizeof(struct llvm::MachO::mach_header));
   }
   return false;
 }
@@ -1274,7 +1336,7 @@ bool ObjectFileMachO::IsStripped() {
   for (uint32_t i = 0; i < m_header.ncmds; ++i) {
 const lldb::offset_t load_cmd_offset = offset;
 
-load_command lc;
+llvm::MachO::load_command lc;
 if (m_data.GetU32(, , 2) == nullptr)
   break;
 if (lc.cmd == LC_DYSYMTAB) {
@@ -1301,7 +1363,7 @@ ObjectFileMachO::EncryptedFileRanges 
ObjectFileMachO::GetEncryptedFileRanges() {
   EncryptedFileRanges result;
   lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
 
-  encryption_info_command encryption_cmd;
+  llvm::MachO::encryption_info_command encryption_cmd;
   for (uint32_t i = 0; i < m_header.ncmds; ++i) {
 const lldb::offset_t load_cmd_offset = offset;
 if (m_data.GetU32(, _cmd, 2) == nullptr)
@@ -1326,8 +1388,8 @@ ObjectFileMachO::EncryptedFileRanges 
ObjectFileMachO::GetEncryptedFileRanges() {
   return result;
 }
 
-void ObjectFileMachO::SanitizeSegmentCommand(segment_command_64 _cmd,
- uint32_t cmd_idx) {
+void ObjectFileMachO::SanitizeSegmentCommand(
+llvm::MachO::segment_command_64 _cmd, uint32_t cmd_idx) {
   if (m_length == 0 || seg_cmd.filesize == 0)
 return;
 
@@ -1383,7 +1445,8 @@ void 
ObjectFileMachO::SanitizeSegmentCommand(segment_command_64 _cmd,
   }
 }
 
-static uint32_t GetSegmentPermissions(const segment_command_64 _cmd) {
+static uint32_t
+GetSegmentPermissions(const llvm::MachO::segment_command_64 _cmd) {
   uint32_t result = 0;
   if (seg_cmd.initprot & VM_PROT_READ)
 result |= ePermissionsReadable;
@@ -1551,11 +1614,10 @@ struct ObjectFileMachO::SegmentParsingContext {
   : EncryptedRanges(std::move(EncryptedRanges)), UnifiedList(UnifiedList) 
{}
 };
 
-void ObjectFileMachO::ProcessSegmentCommand(const load_command _cmd_,
-lldb::offset_t offset,
-  

[Lldb-commits] [lldb] 541ccd1 - Allow signposts to take advantage of deferred string substitution

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2021-06-11T16:35:43-07:00
New Revision: 541ccd1c1bb23e1e20a382844b35312c0caffd79

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

LOG: Allow signposts to take advantage of deferred string substitution

One nice feature of the os_signpost API is that format string
substitutions happen in the consumer, not the logging
application. LLVM's current Signpost class doesn't take advantage of
this though and instead always uses a static "Begin/End %s" format
string.

This patch uses variadic macros to allow the API to be used as
intended. Unfortunately, the primary use-case I had in mind (the
LLDB_SCOPED_TIMER() macro) does not get much better from this, because
__PRETTY_FUNCTION__ is *not* a macro, but a static string, so
signposts created by LLDB_SCOPED_TIMER() still use a static "%s"
format string. At least LLDB_SCOPED_TIMERF() works as intended.

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

Added: 


Modified: 
lldb/include/lldb/Utility/Timer.h
lldb/source/Utility/Timer.cpp
llvm/include/llvm/Support/Signposts.h
llvm/lib/Support/Signposts.cpp
llvm/lib/Support/Timer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Timer.h 
b/lldb/include/lldb/Utility/Timer.h
index 2b39881de89e..1e47ac7afc82 100644
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -10,10 +10,16 @@
 #define LLDB_UTILITY_TIMER_H
 
 #include "lldb/lldb-defines.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Signposts.h"
 #include 
 #include 
 
+namespace llvm {
+  class SignpostEmitter;
+}
+
 namespace lldb_private {
 class Stream;
 
@@ -72,15 +78,28 @@ class Timer {
   const Timer =(const Timer &) = delete;
 };
 
+llvm::SignpostEmitter ();
+
 } // namespace lldb_private
 
 // Use a format string because LLVM_PRETTY_FUNCTION might not be a string
 // literal.
 #define LLDB_SCOPED_TIMER()
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION)
-#define LLDB_SCOPED_TIMERF(...)
\
+  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION);   
\
+  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),  
\
+  &_scoped_timer, "%s", LLVM_PRETTY_FUNCTION); 
\
+  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {   
\
+::lldb_private::GetSignposts().endInterval(&_scoped_timer);
\
+  })
+
+#define LLDB_SCOPED_TIMERF(FMT, ...)   
\
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);   
\
-  ::lldb_private::Timer _scoped_timer(_cat, __VA_ARGS__)
+  ::lldb_private::Timer _scoped_timer(_cat, FMT, __VA_ARGS__); 
\
+  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),  
\
+  &_scoped_timer, FMT, __VA_ARGS__);   
\
+  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {   
\
+::lldb_private::GetSignposts().endInterval(&_scoped_timer);
\
+  })
 
 #endif // LLDB_UTILITY_TIMER_H

diff  --git a/lldb/source/Utility/Timer.cpp b/lldb/source/Utility/Timer.cpp
index 2f3afe4c8703..b59ce3b9f556 100644
--- a/lldb/source/Utility/Timer.cpp
+++ b/lldb/source/Utility/Timer.cpp
@@ -33,6 +33,8 @@ static std::atomic g_categories;
 /// Allows llvm::Timer to emit signposts when supported.
 static llvm::ManagedStatic Signposts;
 
+llvm::SignpostEmitter _private::GetSignposts() { return *Signposts; }
+
 std::atomic Timer::g_quiet(true);
 std::atomic Timer::g_display_depth(0);
 static std::mutex () {
@@ -59,7 +61,6 @@ void Timer::SetQuiet(bool value) { g_quiet = value; }
 
 Timer::Timer(Timer::Category , const char *format, ...)
 : m_category(category), m_total_start(std::chrono::steady_clock::now()) {
-  Signposts->startInterval(this, m_category.GetName());
   TimerStack  = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
@@ -86,8 +87,6 @@ Timer::~Timer() {
   auto total_dur = stop_time - m_total_start;
   auto timer_dur = total_dur - m_child_duration;
 
-  Signposts->endInterval(this, m_category.GetName());
-
   TimerStack  = GetTimerStackForCurrentThread();
   if (g_quiet && stack.size() <= g_display_depth) {
 std::lock_guard lock(GetFileMutex());

diff  --git a/llvm/include/llvm/Support/Signposts.h 
b/llvm/include/llvm/Support/Signposts.h
index d31f3c1e6eb5..66675313c083 100644
--- a/llvm/include/llvm/Support/Signposts.h
+++ b/llvm/include/llvm/Support/Signposts.h
@@ -18,8 +18,17 @@
 #define 

[Lldb-commits] [PATCH] D103588: [trace] Create a top level ThreadTrace class

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I mostly commented on ThreadTrace.h and we should resolve the questions in 
there before reviewing the rest of this patch.




Comment at: lldb/include/lldb/Target/ThreadTrace.h:44-45
+  /// \param[in] start_position
+  /// The instruction position to start iterating on, where 0 corresponds 
to
+  /// the oldest instruction chronologically.
+  ///

Should zero mean oldest when using TraceDirection::Forwards and newest when 
using TraceDirection::Backwards? Otherwise how would a person easily go 
backwards from the end of the thread trace?



Comment at: lldb/include/lldb/Target/ThreadTrace.h:61
+  virtual void TraverseInstructions(
+  size_t position, TraceDirection direction,
+  std::function `GetInstructionCount` requires decoding the whole ThreadTrace (if not for PT, 
> then for other tracing technologies). I don't think we can take the O(n) hit 
> up front.
This could be very expensive to calculate. Do we want to omit this to avoid 
having to traverse all of the data and also disassemble everything to find out 
instruction counts? Since our trace might have 1000 trace branches, but 
millions of instructions that are traversed between each branch



Comment at: lldb/include/lldb/Target/ThreadTrace.h:77
+  ///  The instruction type.
+  virtual lldb::TraceInstructionType GetInstructionType(size_t index) = 0;
+

vsk wrote:
> How does a a ThreadTrace client query what the valid range for indices is?
Why not just put this information in the callback? It seems like a storage 
issue or performance issue if you can only find the address out at callback 
time.



Comment at: lldb/include/lldb/Target/ThreadTrace.h:83
+  ///  The index of the instruction in question. It must be valid.
+  virtual size_t GetInstructionSize(size_t index) = 0;
+};

Can we put this info in the callback? Ditto from above comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103588

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

[Lldb-commits] [PATCH] D103588: [trace] Create a top level ThreadTrace class

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

I think this is moving in the right direction! However, the way ThreadTrace is 
specified still raises the same scalability concerns (see inline). The 
"instruction index" concept doesn't seem sufficient. Decoding needs to be done 
in two stages: a separate "trace index" concept would help with that. In the 
first stage, lldb identifies which subset of the trace it needs decoded by 
specifying start/stop trace indices. This should be fast, as counting the 
number of available trace bytes/"chunks" is O(1). Nothing needs to be decoded 
at this point. The second stage starts after a trace subset is decoded, and is 
where instruction indices are useful.

(I'll be OOO next week, but hope to pick up review on 6/21.)




Comment at: lldb/include/lldb/Target/ThreadTrace.h:68
+  ///   The number of available instructions in the trace.
+  virtual size_t GetInstructionCount() = 0;
+

`GetInstructionCount` requires decoding the whole ThreadTrace (if not for PT, 
then for other tracing technologies). I don't think we can take the O(n) hit up 
front.



Comment at: lldb/include/lldb/Target/ThreadTrace.h:77
+  ///  The instruction type.
+  virtual lldb::TraceInstructionType GetInstructionType(size_t index) = 0;
+

How does a a ThreadTrace client query what the valid range for indices is?



Comment at: lldb/include/lldb/lldb-enumerations.h:791
+  /// The instruction is a conditional jump
+  eTraceInstructionCondJump,
+  /// The instruction writes custom data to the trace, e.g. Intel's PTWRITE.

Do we have an example of a generic trace analysis that needs to differentiate 
between conditional & unconditional branches?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103588

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

[Lldb-commits] [lldb] 60fde95 - [lldb] Remove GCC XFAIL for TestCPPAuto and TestClassTemplateParameterPack

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-06-11T23:22:26+02:00
New Revision: 60fde9542aac57d4b7b98bed0be746a3a82bd0eb

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

LOG: [lldb] Remove GCC XFAIL for TestCPPAuto and TestClassTemplateParameterPack

Both tests are passing for GCC>8 on Linux so let's mark them as passing.

TestCPPAuto was originally disabled due to "an problem with debug info 
generation"
in ea35dbeff29f3095df3ad1d77cce3d9e5b197e7c .

TestClassTemplateParameterPack was disabled without explanation in
0f01fb39e3fe3d8e99df1dd185e75ad584b777b3 .

Added: 


Modified: 
lldb/test/API/lang/cpp/auto/TestCPPAuto.py

lldb/test/API/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/auto/TestCPPAuto.py 
b/lldb/test/API/lang/cpp/auto/TestCPPAuto.py
index c30de747e0896..c5b656b9f1b4b 100644
--- a/lldb/test/API/lang/cpp/auto/TestCPPAuto.py
+++ b/lldb/test/API/lang/cpp/auto/TestCPPAuto.py
@@ -11,9 +11,6 @@ class CPPAutoTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(
-compiler="gcc",
-bugnumber="GCC generates incomplete debug info")
 def test_with_run_command(self):
 """Test that auto types work in the expression parser"""
 self.build()

diff  --git 
a/lldb/test/API/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
 
b/lldb/test/API/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
index e0497b62f55cb..d452ae6a4272d 100644
--- 
a/lldb/test/API/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
+++ 
b/lldb/test/API/lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
@@ -7,7 +7,6 @@ class TestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(compiler="gcc")
 def test(self):
 self.build()
 lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))



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

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

[Greg Clayton via Phabricator via lldb-commits]

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

Looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

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

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev updated this revision to Diff 351547.
OmarEmaraDev added a comment.

- Handle review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/test/API/commands/gui/expand-threads-tree/Makefile
  lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
  lldb/test/API/commands/gui/expand-threads-tree/main.c

Index: lldb/test/API/commands/gui/expand-threads-tree/main.c
===
--- /dev/null
+++ lldb/test/API/commands/gui/expand-threads-tree/main.c
@@ -0,0 +1,10 @@
+#include 
+
+void *thread_start_routine(void *arg) { return NULL; }
+
+int main() {
+  pthread_t thread;
+  pthread_create(, NULL, thread_start_routine, NULL);
+  pthread_join(thread, NULL);
+  return 0;
+}
Index: lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
===
--- /dev/null
+++ lldb/test/API/commands/gui/expand-threads-tree/TestGuiExpandThreadsTree.py
@@ -0,0 +1,55 @@
+"""
+Test the 'gui' default thread tree expansion.
+The root process tree item and the tree item corresponding to the selected
+thread should be expanded by default.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestGuiExpandThreadsTree(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact("a.out"), dimensions=(100,500))
+self.expect("breakpoint set -r thread_start_routine", substrs=["Breakpoint 1", "address ="])
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI and close the welcome window.
+self.child.sendline("gui")
+self.child.send(escape_key)
+self.child.expect_exact("Threads")
+
+# The thread running thread_start_routine should be expanded.
+self.child.expect_exact("frame #0: thread_start_routine")
+
+# Exit GUI.
+self.child.send(escape_key)
+self.expect_prompt()
+
+# Select the main thread.
+self.child.sendline("thread select 1")
+
+# Start the GUI.
+self.child.sendline("gui")
+self.child.expect_exact("Threads")
+
+# The main thread should be expanded.
+self.child.expect("frame #\d+: main")
+
+# Quit the GUI
+self.child.send(escape_key)
+
+self.expect_prompt()
+self.quit()
Index: lldb/test/API/commands/gui/expand-threads-tree/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/gui/expand-threads-tree/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+ENABLE_THREADS := YES
+include Makefile.rules
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1608,8 +1608,13 @@
 
   virtual void TreeDelegateDrawTreeItem(TreeItem , Window ) = 0;
   virtual void TreeDelegateGenerateChildren(TreeItem ) = 0;
+  virtual void TreeDelegateUpdateSelection(TreeItem , int _index,
+   TreeItem *_item) {
+return;
+  }
   virtual bool TreeDelegateItemSelected(
   TreeItem ) = 0; // Return true if we need to update views
+  virtual bool TreeDelegateExpandRootByDefault() { return false; }
 };
 
 typedef std::shared_ptr TreeDelegateSP;
@@ -1619,7 +1624,10 @@
   TreeItem(TreeItem *parent, TreeDelegate , bool might_have_children)
   : m_parent(parent), m_delegate(delegate), m_user_data(nullptr),
 m_identifier(0), m_row_idx(-1), m_children(),
-m_might_have_children(might_have_children), m_is_expanded(false) {}
+m_might_have_children(might_have_children), m_is_expanded(false) {
+if (m_parent == nullptr)
+  m_is_expanded = m_delegate.TreeDelegateExpandRootByDefault();
+  }
 
   TreeItem =(const TreeItem ) {
 if (this != ) {
@@ -1848,6 +1856,8 @@
   const int num_visible_rows = NumVisibleRows();
   m_num_rows = 0;
   m_root.CalculateRowIndexes(m_num_rows);
+  m_delegate_sp->TreeDelegateUpdateSelection(m_root, m_selected_row_idx,
+ m_selected_item);
 
   // If we unexpanded while having something selected our total number of
   // rows is less than the num visible rows, then make sure we show all the
@@ -2149,7 +2159,7 @@
 public:
   ThreadsTreeDelegate(Debugger )
   : TreeDelegate(), m_thread_delegate_sp(), 

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

[Greg Clayton via Phabricator via lldb-commits]

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

Many minor things, but looking good! I look forward to seeing this get checked 
in




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1611-1612
   virtual void TreeDelegateGenerateChildren(TreeItem ) = 0;
+  virtual void SetDefaultSelectionIfNeeded(TreeItem , int 
*selection_index,
+   TreeItem *selected_item) = 0;
   virtual bool TreeDelegateItemSelected(

Don't make this pure virtual unless all subclasses must implement this. I see a 
few implementations below that do nothing, so just make a default 
implementation so classes that don't need this don't have to implement it. Also 
renamed to TreeDelegateUpdateSelection to stay consistent with the other 
delegate methods.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1626-1627
+// Expand root tree items by default.
+if (m_parent == nullptr)
+  m_is_expanded = true;
+  }

We might want to opt into this? We don't want all variables in a variable view 
to be expanded by default. But we do want the process tree to always expand, so 
we need to be able to opt in in the tree delegates. 



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1857-1858
   m_root.CalculateRowIndexes(m_num_rows);
+  m_delegate_sp->SetDefaultSelectionIfNeeded(m_root, _selected_row_idx,
+ m_selected_item);
 

I assume we want this method to update both m_selected_row_idx and 
m_selected_item right? If so, make sure we switch to references on line 1611 
above so both do get updated.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2042-2046
+  void SetDefaultSelectionIfNeeded(TreeItem , int *selection_index,
+   TreeItem *selected_item) override {
+return;
+  }
+

Remove and make a default implementation in the base class and don't make it 
pure virtual



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2133-2137
+  void SetDefaultSelectionIfNeeded(TreeItem , int *selection_index,
+   TreeItem *selected_item) override {
+return;
+  }
+

Remove and make a default implementation in the base class and don't make it 
pure virtual



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2197
   void TreeDelegateGenerateChildren(TreeItem ) override {
 ProcessSP process_sp = GetProcess();
 if (process_sp && process_sp->IsAlive()) {

See comment below where you used to set m_need_to_set_default_selection to 
false.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2254
+  if (selected_thread->GetID() == thread->GetID()) {
+selected_item = [i][thread->GetSelectedFrameIndex()];
+*selection_index = selected_item->GetRowIndex();

This just sets the local variable "selected_item" to a value. Make sure we 
switch to using references on 1611 as mentioned before.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2256
+*selection_index = selected_item->GetRowIndex();
+m_need_to_set_default_selection = false;
+return;

You can remove this and just always set this in the TreeDelegateGenerateChildren



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2268
   uint32_t m_stop_id;
+  bool m_need_to_set_default_selection;
   FormatEntity::Entry m_format;

Maybe renamed to m_update_selection so this variable is a bit shorter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

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

[Lldb-commits] [PATCH] D104067: [lldb] Decouple ObjCLanguage from Symtab

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:104
+  // We also return the FunctionNameType of each possible name.
+  std::vector>
   GetMethodNameVariants(ConstString method_name) const override;

teemperor wrote:
> Could we make this a custom struct? `first` `second` are always so 
> non-descriptive and we might want to extend what we return from this function 
> in a future patch.
> 
> ```
> lang=c++
> class MethodNameVariant {
>   ConstString m_name;
>   lldb::FunctionNameType m_type;
> public:
>   [...]
> }```
I second (pun intended?) this, if we can avoid using `std::pair` and instead 
use a custom struct we should.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104067

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

[Lldb-commits] [PATCH] D103391: [lldb] Add missing reproducer instrumentation to some SB classes

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103391

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

[Lldb-commits] [PATCH] D103575: Allow signposts to take advantage of deferred string substitution

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Looks good to me!


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

https://reviews.llvm.org/D103575

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

[Lldb-commits] [lldb] fc71a5c - [lldb] Support new objective-c hash table layout

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-06-11T10:26:16-07:00
New Revision: fc71a5c6e8e8bd28e43cf58faabad6e7e2fcc652

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

LOG: [lldb] Support new objective-c hash table layout

Update LLDB for thew new Objective-C hash table layout in the dyld
shared cache found in macOS Monterey.

rdar://72863911

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
index c1082100f2746..405b8a6f16b77 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -252,6 +252,7 @@ bool ClassDescriptorV2::method_list_t::Read(Process 
*process,
 }
 
 bool ClassDescriptorV2::method_t::Read(Process *process, lldb::addr_t addr,
+   lldb::addr_t 
relative_selector_base_addr,
bool is_small, bool has_direct_sel) {
   size_t ptr_size = process->GetAddressByteSize();
   size_t size = GetSize(process, is_small);
@@ -281,6 +282,8 @@ bool ClassDescriptorV2::method_t::Read(Process *process, 
lldb::addr_t addr,
   0, error);
   if (!error.Success())
 return false;
+} else if (relative_selector_base_addr != LLDB_INVALID_ADDRESS) {
+  m_name_ptr = relative_selector_base_addr + nameref_offset;
 }
 m_types_ptr = addr + 4 + types_offset;
 m_imp_ptr = addr + 8 + imp_offset;
@@ -389,14 +392,14 @@ bool ClassDescriptorV2::Describe(
 if (base_method_list->m_entsize != method_t::GetSize(process, is_small))
   return false;
 
-std::unique_ptr method;
-method = std::make_unique();
-
+std::unique_ptr method = std::make_unique();
+lldb::addr_t relative_selector_base_addr =
+m_runtime.GetRelativeSelectorBaseAddr();
 for (uint32_t i = 0, e = base_method_list->m_count; i < e; ++i) {
   method->Read(process,
base_method_list->m_first_ptr +
(i * base_method_list->m_entsize),
-   is_small, has_direct_selector);
+   relative_selector_base_addr, is_small, has_direct_selector);
 
   if (instance_method_func(method->m_name.c_str(), 
method->m_types.c_str()))
 break;

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
index 16e94f6790841..60374498c8965 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -166,7 +166,8 @@ class ClassDescriptorV2 : public 
ObjCLanguageRuntime::ClassDescriptor {
  + field_size; // IMP imp;
 }
 
-bool Read(Process *process, lldb::addr_t addr, bool, bool);
+bool Read(Process *process, lldb::addr_t addr,
+  lldb::addr_t relative_method_lists_base_addr, bool, bool);
   };
 
   struct ivar_list_t {

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index 83f4356c3fb37..069368c8e058b 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -263,6 +263,12 @@ struct objc_classheader_t {
 int32_t hiOffset;
 };
 
+struct objc_classheader_v16_t {
+uint64_t isDuplicate   : 1,
+ objectCacheOffset : 47, // Offset from the shared cache base
+ dylibObjCIndex: 16;
+};
+
 struct objc_clsopt_t {
 uint32_t capacity;
 uint32_t occupied;
@@ -280,6 +286,22 @@ struct objc_clsopt_t {
 //  objc_classheader_t duplicateOffsets[duplicateCount];
 };
 
+struct objc_clsopt_v16_t {
+   uint32_t version;
+   uint32_t capacity;
+   uint32_t occupied;
+   uint32_t shift;
+   uint32_t mask;
+   uint64_t salt;
+   uint32_t scramble[256];
+   uint8_t  tab[0]; // tab[mask+1]
+   //  uint8_t checkbytes[capacity];
+   //  int32_t 

[Lldb-commits] [PATCH] D104054: [lldb] Enable Rust v0 symbol demangling

[Alexander via Phabricator via lldb-commits]

asm added a comment.

And thank you for the quick review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104054

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

[Lldb-commits] [lldb] c7dee6a - [lldb] Enable TestRuntimeTypes on Apple Silicon

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-06-11T10:24:41-07:00
New Revision: c7dee6aefd5b5f1d92d279ddd40b81217b496d50

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

LOG: [lldb] Enable TestRuntimeTypes on Apple Silicon

Added: 


Modified: 
lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py

Removed: 




diff  --git a/lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py 
b/lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
index 901965005e1af..0bd9fbe50a386 100644
--- a/lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
+++ b/lldb/test/API/lang/objc/foundation/TestRuntimeTypes.py
@@ -21,9 +21,6 @@ class RuntimeTypesTestCase(TestBase):
 @skipIfReproducer # FIXME: Unexpected packet during (active) replay
 def test_break(self):
 """Test setting objc breakpoints using '_regexp-break' and 'breakpoint 
set'."""
-if self.getArchitecture() != 'x86_64':
-self.skipTest("This only applies to the v2 runtime")
-
 self.build()
 exe = self.getBuildArtifact("a.out")
 self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)



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

[Lldb-commits] [PATCH] D103675: [LLDB/API] Expose args and env from SBProcessInfo.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D103675#2800653 , @brucem wrote:

> Not sure why this failed. Is it perhaps a difference where some processes 
> have args including the executable and some do not?

Do you have the bot failures around for that? (Or, the platform at least so I 
could reproduce it locally?)




Comment at: lldb/bindings/interface/SBProcessInfo.i:78
+
+%feature("docstring",
+"Return the specified argument given to the described process."

brucem wrote:
> teemperor wrote:
> > Can you add this line here?
> > 
> > ```
> > %feature("autodoc", "GetArgumentAtIndex(int index) -> string") 
> > GetArgumentAtIndex;
> > ```
> > 
> > Otherwise the Python docs will mention that this returns `const char *` 
> > which is always kinda weird for users to see.
> I changed this to have the whole thing be an autodoc rather than both an 
> autodoc and a docstring. This isn't consistently done throughout the code 
> though and in this case, the resulting syntax help is a bit different.
Sure, let's see how it looks on the website and then we can decide what works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103675

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

[Lldb-commits] [PATCH] D104054: [lldb] Enable Rust v0 symbol demangling

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, thanks! (And congrats on your first patch)

I'll land this for you next week just to give the others another chance to take 
a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104054

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

[Lldb-commits] [PATCH] D104054: [lldb] Enable Rust v0 symbol demangling

[Alexander via Phabricator via lldb-commits]

asm updated this revision to Diff 351481.
asm marked an inline comment as done.
asm added a comment.

code style fixes, use modern log


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104054

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Symbol/Symtab.cpp
  lldb/unittests/Core/MangledTest.cpp

Index: lldb/unittests/Core/MangledTest.cpp
===
--- lldb/unittests/Core/MangledTest.cpp
+++ lldb/unittests/Core/MangledTest.cpp
@@ -55,6 +55,23 @@
   EXPECT_STREQ("", TheDemangled.GetCString());
 }
 
+TEST(MangledTest, ResultForValidRustV0Name) {
+  ConstString mangled_name("_RNvC1a4main");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  ConstString expected_result("a::main");
+  EXPECT_STREQ(expected_result.GetCString(), the_demangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidRustV0Name) {
+  ConstString mangled_name("_RRR");
+  Mangled the_mangled(mangled_name);
+  ConstString the_demangled = the_mangled.GetDemangledName();
+
+  EXPECT_STREQ("", the_demangled.GetCString());
+}
+
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   SubsystemRAII
   subsystems;
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -240,6 +240,10 @@
   case Mangled::eManglingSchemeMSVC:
 return false;
 
+  // No filters for this scheme yet. Include all names in indexing.
+  case Mangled::eManglingSchemeRustV0:
+return false;
+
   // Don't try and demangle things we can't categorize.
   case Mangled::eManglingSchemeNone:
 return true;
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -72,6 +72,9 @@
   if (name.startswith("?"))
 return Mangled::eManglingSchemeMSVC;
 
+  if (name.startswith("_R"))
+return Mangled::eManglingSchemeRustV0;
+
   if (name.startswith("_Z"))
 return Mangled::eManglingSchemeItanium;
 
@@ -199,6 +202,19 @@
   return demangled_cstr;
 }
 
+static char *GetRustV0DemangledStr(const char *M) {
+  char *demangled_cstr = llvm::rustDemangle(M, nullptr, nullptr, nullptr);
+
+  if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) {
+if (demangled_cstr && demangled_cstr[0])
+  LLDB_LOG(log, "demangled rustv0: {0} -> \"{1}\"", M, demangled_cstr);
+else
+  LLDB_LOG(log, "demangled rustv0: {0} -> error: failed to demangle", M);
+  }
+
+  return demangled_cstr;
+}
+
 // Explicit demangling for scheduled requests during batch processing. This
 // makes use of ItaniumPartialDemangler's rich demangle info
 bool Mangled::DemangleWithRichManglingInfo(
@@ -256,6 +272,10 @@
   return context.FromCxxMethodName(m_demangled);
 }
   }
+
+  case eManglingSchemeRustV0:
+// Rich demangling scheme is not supported for Rust
+return false;
   }
   llvm_unreachable("Fully covered switch above!");
 }
@@ -284,6 +304,9 @@
 demangled_name = GetItaniumDemangledStr(mangled_name);
 break;
   }
+  case eManglingSchemeRustV0:
+demangled_name = GetRustV0DemangledStr(mangled_name);
+break;
   case eManglingSchemeNone:
 llvm_unreachable("eManglingSchemeNone was handled already");
   }
Index: lldb/include/lldb/Core/Mangled.h
===
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -43,7 +43,8 @@
   enum ManglingScheme {
 eManglingSchemeNone = 0,
 eManglingSchemeMSVC,
-eManglingSchemeItanium
+eManglingSchemeItanium,
+eManglingSchemeRustV0
   };
 
   /// Default constructor.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D103609: [lldb-vscode] Synchronize calls to SendTerminatedEvent

[Ayush Sahay 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 rG5ef5177145b4: [lldb-vscode] Synchronize calls to 
SendTerminatedEvent (authored by asahay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103609

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -179,6 +179,19 @@
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
+  // If an inferior exits prior to the processing of a disconnect request, then
+  // the threads executing EventThreadFunction and request_discontinue
+  // respectively may call SendTerminatedEvent simultaneously. Without any
+  // synchronization, the thread executing EventThreadFunction may set
+  // g_vsc.sent_terminated_event before the thread executing
+  // request_discontinue has had a chance to test it, in which case the latter
+  // would move ahead to issue a response to the disconnect request. Said
+  // response may get dispatched ahead of the terminated event compelling the
+  // client to terminate the debug session without consuming any console output
+  // that might've been generated by the execution of terminateCommands. So,
+  // synchronize simultaneous calls to SendTerminatedEvent.
+  static std::mutex mutex;
+  std::lock_guard locker(mutex);
   if (!g_vsc.sent_terminated_event) {
 g_vsc.sent_terminated_event = true;
 g_vsc.RunTerminateCommands();


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -179,6 +179,19 @@
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
+  // If an inferior exits prior to the processing of a disconnect request, then
+  // the threads executing EventThreadFunction and request_discontinue
+  // respectively may call SendTerminatedEvent simultaneously. Without any
+  // synchronization, the thread executing EventThreadFunction may set
+  // g_vsc.sent_terminated_event before the thread executing
+  // request_discontinue has had a chance to test it, in which case the latter
+  // would move ahead to issue a response to the disconnect request. Said
+  // response may get dispatched ahead of the terminated event compelling the
+  // client to terminate the debug session without consuming any console output
+  // that might've been generated by the execution of terminateCommands. So,
+  // synchronize simultaneous calls to SendTerminatedEvent.
+  static std::mutex mutex;
+  std::lock_guard locker(mutex);
   if (!g_vsc.sent_terminated_event) {
 g_vsc.sent_terminated_event = true;
 g_vsc.RunTerminateCommands();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5ef5177 - [lldb-vscode] Synchronize calls to SendTerminatedEvent

[Ayush Sahay via lldb-commits]

Author: Ayush Sahay
Date: 2021-06-11T21:37:19+05:30
New Revision: 5ef5177145b48e6379fe3a6434d3ff593fe7202a

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

LOG: [lldb-vscode] Synchronize calls to SendTerminatedEvent

If an inferior exits prior to the processing of a disconnect request,
then the threads executing EventThreadFunction and request_discontinue
respectively may call SendTerminatedEvent simultaneously, in turn,
testing and/or setting g_vsc.sent_terminated_event without any
synchronization. In case the thread executing EventThreadFunction sets
it before the thread executing request_discontinue has had a chance to
test it, the latter would move ahead to issue a response to the
disconnect request. Said response may be dispatched ahead of the
terminated event compelling the client to terminate the debug session
without consuming any console output that might've been generated by
the execution of terminateCommands.

Reviewed By: clayborg, wallace

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

Added: 


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

Removed: 




diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 29b4019379d83..9619572ef4889 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -179,6 +179,19 @@ void SendThreadExitedEvent(lldb::tid_t tid) {
 // Send a "terminated" event to indicate the process is done being
 // debugged.
 void SendTerminatedEvent() {
+  // If an inferior exits prior to the processing of a disconnect request, then
+  // the threads executing EventThreadFunction and request_discontinue
+  // respectively may call SendTerminatedEvent simultaneously. Without any
+  // synchronization, the thread executing EventThreadFunction may set
+  // g_vsc.sent_terminated_event before the thread executing
+  // request_discontinue has had a chance to test it, in which case the latter
+  // would move ahead to issue a response to the disconnect request. Said
+  // response may get dispatched ahead of the terminated event compelling the
+  // client to terminate the debug session without consuming any console output
+  // that might've been generated by the execution of terminateCommands. So,
+  // synchronize simultaneous calls to SendTerminatedEvent.
+  static std::mutex mutex;
+  std::lock_guard locker(mutex);
   if (!g_vsc.sent_terminated_event) {
 g_vsc.sent_terminated_event = true;
 g_vsc.RunTerminateCommands();



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

[Lldb-commits] [PATCH] D104041: [lldb] Replace default bodies of special member functions with = default;

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Expression/ExpressionParser.h:45
+  virtual ~ExpressionParser() = default;
+  ;
 

teemperor wrote:
> Please drop that when you land it.
Dropped.



Comment at: lldb/include/lldb/Target/Process.h:227
 
-  {}
+  = default;
 

teemperor wrote:
> format
formatted



Comment at: lldb/include/lldb/Target/StackFrameRecognizer.h:41
+  virtual ~RecognizedStackFrame() = default;
+  ;
 

teemperor wrote:
> please remove that pointy boy
pointy boy gone



Comment at: lldb/include/lldb/Target/StackFrameRecognizer.h:68
+  virtual ~StackFrameRecognizer() = default;
+  ;
 };

teemperor wrote:
> and that one too
gone too



Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:26
 
-{}
+= default;
 

teemperor wrote:
> formaat
doone



Comment at: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.h:219
 
-{}
+= default;
 

teemperor wrote:
> formato
hecho



Comment at: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h:111
 
-{}
+= default;
 

teemperor wrote:
> formatione
factum



Comment at: 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:50
+  RSCoordinate() = default;
+  ;
 

teemperor wrote:
> goodbye
ciao



Comment at: 
lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetPendingItemsHandler.h:60
 
-{}
+= default;
   };

teemperor wrote:
> フォーマット
完了



Comment at: lldb/source/Plugins/SystemRuntime/MacOSX/AppleGetQueuesHandler.h:57
 
-{}
+= default;
   };

teemperor wrote:
> хэлбэр хэмжээ
хэлбэртэй, хэмжээтэй



Comment at: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.h:195
 
-{}
+= default;
 

teemperor wrote:
> formoup
doune



Comment at: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.h:219
 
-{}
+= default;
 

teemperor wrote:
> isimo
isimo sixazululiwe


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

https://reviews.llvm.org/D104041

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

[Lldb-commits] [lldb] ab46490 - [lldb] Remove GCC XFAIL for TestTypedefArray

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-06-11T16:24:59+02:00
New Revision: ab46490dfc5cded4b2873da75eb47e150d1ad92c

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

LOG: [lldb] Remove GCC XFAIL for TestTypedefArray

This passes with GCC>8.5 and ToT LLDB.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/typedef_array/TestTypedefArray.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/typedef_array/TestTypedefArray.py
 
b/lldb/test/API/functionalities/data-formatter/typedef_array/TestTypedefArray.py
index 7e67f73b70925..f08c0dcbda983 100644
--- 
a/lldb/test/API/functionalities/data-formatter/typedef_array/TestTypedefArray.py
+++ 
b/lldb/test/API/functionalities/data-formatter/typedef_array/TestTypedefArray.py
@@ -1,7 +1,4 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-lldbinline.MakeInlineTest(
-__file__, globals(), [
-decorators.expectedFailureAll(
-compiler="gcc")])
+lldbinline.MakeInlineTest(__file__, globals(), [])



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

[Lldb-commits] [lldb] b1603cb - [lldb] Fix compilation after removal of APInt::toString

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-06-11T16:20:57+02:00
New Revision: b1603cb66fc68eb74c21041456c9e36466603244

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

LOG: [lldb] Fix compilation after removal of APInt::toString

Added: 


Modified: 
lldb/source/Plugins/Language/ObjC/Cocoa.cpp
lldb/source/Utility/Scalar.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp 
b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
index a0b1b01359ccd..1479f4f0c1510 100644
--- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -383,7 +383,7 @@ static void NSNumber_FormatInt128(ValueObject , 
Stream ,
   stream.PutCString(prefix.c_str());
   const int radix = 10;
   const bool isSigned = true;
-  std::string str = value.toString(radix, isSigned);
+  std::string str = llvm::toString(value, radix, isSigned);
   stream.PutCString(str.c_str());
   stream.PutCString(suffix.c_str());
 }

diff  --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 9bf633d0c4e0f..e0b26e89f3c1f 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -160,7 +160,7 @@ void Scalar::GetValue(Stream *s, bool show_type) const {
   case e_void:
 break;
   case e_int:
-s->PutCString(m_integer.toString(10));
+s->PutCString(llvm::toString(m_integer, 10));
 break;
   case e_float:
 llvm::SmallString<24> string;



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

[Lldb-commits] [lldb] e2d0798 - [lldb] Unmark TestInlines as XFAIL for gcc

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-06-11T16:06:39+02:00
New Revision: e2d0798bc3e462738c557270528e8a983df0cf02

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

LOG: [lldb] Unmark TestInlines as XFAIL for gcc

This works for GCC>8.5 and ToT LLDB.

Added: 


Modified: 
lldb/test/API/lang/cpp/inlines/TestInlines.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/inlines/TestInlines.py 
b/lldb/test/API/lang/cpp/inlines/TestInlines.py
index 5f77d8f5963b0..2b03e87330345 100644
--- a/lldb/test/API/lang/cpp/inlines/TestInlines.py
+++ b/lldb/test/API/lang/cpp/inlines/TestInlines.py
@@ -19,7 +19,6 @@ def setUp(self):
 'inlines.cpp',
 '// Set break point at this line.')
 
-@expectedFailureAll("llvm.org/pr26710", oslist=["linux"], compiler="gcc")
 def test(self):
 """Test that local variables are visible in expressions."""
 self.build()



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

[Lldb-commits] [lldb] 307cfad - Fix extraneous ')' error.

[Simon Pilgrim via lldb-commits]

Author: Simon Pilgrim
Date: 2021-06-11T14:50:22+01:00
New Revision: 307cfad0d639b0397033069473e84e6c7f249056

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

LOG: Fix extraneous ')' error.

Added: 


Modified: 
lldb/source/Core/DumpDataExtractor.cpp

Removed: 




diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index 93aaae8a1040..175ffef04a81 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -116,7 +116,7 @@ static lldb::offset_t DumpAPInt(Stream *s, const 
DataExtractor ,
 bool is_signed, unsigned radix) {
   llvm::Optional apint = GetAPInt(data, , byte_size);
   if (apint.hasValue()) {
-std::string apint_str = toString(apint.getValue(), radix, is_signed));
+std::string apint_str = toString(apint.getValue(), radix, is_signed);
 switch (radix) {
 case 2:
   s->Write("0b", 2);



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

[Lldb-commits] [PATCH] D97285: [lldb][AArch64] Add "memory tag read" command

[David Spickett via Phabricator via lldb-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97285

Files:
  lldb/source/Commands/CMakeLists.txt
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectMemoryTag.h
  lldb/test/API/functionalities/memory/tag/Makefile
  lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
  lldb/test/API/functionalities/memory/tag/main.cpp
  lldb/test/API/linux/aarch64/mte_tag_read/Makefile
  lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
  lldb/test/API/linux/aarch64/mte_tag_read/main.c

Index: lldb/test/API/linux/aarch64/mte_tag_read/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/main.c
@@ -0,0 +1,77 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  // We assume that the test runner has checked we're on an MTE system
+
+  if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+return 1;
+  }
+
+  size_t page_size = sysconf(_SC_PAGESIZE);
+
+  // Allocate memory with MTE
+  // We ask for two pages. One is read only so that we get
+  // 2 mappings in /proc/.../smaps so we can check reading
+  // a range across mappings.
+  // The first allocation will start at the highest address,
+  // so we allocate buf2 first to get:
+  //  | buf | buf2 | 
+  int prot = PROT_READ | PROT_MTE;
+  int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+
+  char *buf2 = mmap(0, page_size, prot, flags, -1, 0);
+  if (buf2 == MAP_FAILED)
+return 1;
+
+  // Writeable so we can set tags on it later
+  char *buf = mmap(0, page_size, prot | PROT_WRITE, flags, -1, 0);
+  if (buf == MAP_FAILED)
+return 1;
+
+  // We expect the mappings to be next to each other
+  if (buf2 - buf != page_size)
+return 1;
+
+  // And without MTE
+  char *non_mte_buf = mmap(0, page_size, PROT_READ | PROT_WRITE, flags, -1, 0);
+  if (non_mte_buf == MAP_FAILED)
+return 1;
+
+  // Set incrementing tags until end of the first page
+  char *tagged_ptr = buf;
+  // This ignores tag bits when subtracting the addresses
+  while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+// Set the allocation tag for this location
+__arm_mte_set_tag(tagged_ptr);
+// + 16 for 16 byte granules
+// Earlier we allowed all tag values, so this will give us an
+// incrementing pattern 0-0xF wrapping back to 0.
+tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1);
+  }
+
+  // Tag the original pointer with 9
+  buf = __arm_mte_create_random_tag(buf, ~(1 << 9));
+  // A different tag so that buf_alt_tag > buf if you don't handle the tag
+  char *buf_alt_tag = __arm_mte_create_random_tag(buf, ~(1 << 10));
+
+  // lldb should be removing the whole top byte, not just the tags.
+  // So fill 63-60 with something non zero so we'll fail if we only remove tags.
+#define SET_TOP_NIBBLE(ptr) (char *)((size_t)(ptr) | (0xA << 60))
+  buf = SET_TOP_NIBBLE(buf);
+  buf_alt_tag = SET_TOP_NIBBLE(buf_alt_tag);
+  buf2 = SET_TOP_NIBBLE(buf2);
+
+  // Breakpoint here
+  return 0;
+}
Index: lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
@@ -0,0 +1,126 @@
+"""
+Test "memory tag read" command on AArch64 Linux with MTE.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxMTEMemoryTagReadTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_tag_read(self):
+if not self.isAArch64MTE():
+self.skipTest('Target must support MTE.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Breakpoint here'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# 

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading to lldb

[David Spickett via Phabricator via lldb-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95602

Files:
  lldb/include/lldb/Core/Architecture.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
  lldb/source/Plugins/Architecture/AArch64/CMakeLists.txt
  lldb/source/Plugins/Architecture/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -466,3 +466,68 @@
   EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=0x1234"));
   EXPECT_EQ(llvm::None, GetQOffsets("TextSeg=12345678123456789"));
 }
+
+static void
+check_qmemtags(TestClient , MockServer , size_t read_len,
+   const char *packet, llvm::StringRef response,
+   llvm::Optional> expected_tag_data) {
+  const auto  = [&](size_t len, const char *packet,
+   llvm::StringRef response) {
+std::future result = std::async(std::launch::async, [&] {
+  return client.ReadMemoryTags(0xDEF0, read_len, 1);
+});
+
+HandlePacket(server, packet, response);
+return result.get();
+  };
+
+  auto result = ReadMemoryTags(0, packet, response);
+  if (expected_tag_data) {
+ASSERT_TRUE(result);
+llvm::ArrayRef expected(*expected_tag_data);
+llvm::ArrayRef got = result->GetData();
+ASSERT_THAT(expected, testing::ContainerEq(got));
+  } else {
+ASSERT_FALSE(result);
+  }
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, ReadMemoryTags) {
+  // Zero length reads are valid
+  check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+ std::vector{});
+
+  // The client layer does not check the length of the received data.
+  // All we need is the "m" and for the decode to use all of the chars
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09",
+ std::vector{0x9});
+
+  // Zero length response is fine as long as the "m" is present
+  check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+ std::vector{});
+
+  // Normal responses
+  check_qmemtags(client, server, 16, "qMemTags:def0,10:1", "m66",
+ std::vector{0x66});
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m0102",
+ std::vector{0x1, 0x2});
+
+  // Empty response is an error
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "", llvm::None);
+  // Usual error response
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "E01", llvm::None);
+  // Leading m missing
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "01", llvm::None);
+  // Anything other than m is an error
+  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "z01", llvm::None);
+  // Decoding tag data doesn't use all the chars in the packet
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09zz", llvm::None);
+  // Data that is not hex bytes
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "mhello",
+ llvm::None);
+  // Data is not a complete hex char
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m9", llvm::None);
+  // Data has a trailing hex char
+  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m01020",
+ llvm::None);
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -6065,3 +6065,84 @@
 
   return false;
 }
+
+llvm::Expected
+Process::GetMemoryTagManager(lldb::addr_t addr, lldb::addr_t end_addr) {
+  Architecture *arch = GetTarget().GetArchitecturePlugin();
+  const MemoryTagManager *tag_manager =
+  arch ? arch->GetMemoryTagManager() : nullptr;
+  if (!arch || !tag_manager) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"This architecture does not support memory tagging",
+GetPluginName().GetCString());
+  }
+
+  if (!SupportsMemoryTagging()) {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Process does not support memory tagging");
+  }
+
+  ptrdiff_t len = tag_manager->AddressDiff(end_addr, addr);
+  if (len <= 0) {
+return 

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add memory tag reading to lldb-server

[David Spickett via Phabricator via lldb-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Host/linux/Ptrace.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/memory-tagging/Makefile
  lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
  lldb/test/API/tools/lldb-server/memory-tagging/main.c

Index: lldb/test/API/tools/lldb-server/memory-tagging/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-tagging/main.c
@@ -0,0 +1,55 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int print_result(char *ptr) {
+  // Page size allows the test to try reading off of the end of the page
+  printf("buffer: %p page_size: 0x%x\n", ptr, sysconf(_SC_PAGESIZE));
+
+  // Exit after some time, so we don't leave a zombie process
+  // if the test framework lost track of us.
+  sleep(60);
+  return 0;
+}
+
+int main(int argc, char const *argv[]) {
+  if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC |
+// Allow all tags to be generated by the addg
+// instruction __arm_mte_increment_tag produces.
+(0x << PR_MTE_TAG_SHIFT),
+0, 0, 0)) {
+return print_result(NULL);
+  }
+
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE | PROT_MTE,
+   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (buf == MAP_FAILED)
+return print_result(NULL);
+
+  // Set incrementing tags until end of the page
+  char *tagged_ptr = buf;
+  // This intrinsic treats the addresses as if they were untagged
+  while (__arm_mte_ptrdiff(tagged_ptr, buf) < page_size) {
+// This sets the allocation tag
+__arm_mte_set_tag(tagged_ptr);
+// Set the tag of the next granule (hence +16) to the next
+// tag value. Returns a new pointer with the new logical tag.
+// Tag values wrap at 0xF so it'll cycle.
+tagged_ptr = __arm_mte_increment_tag(tagged_ptr + 16, 1);
+  }
+
+  // lldb-server should be removing the top byte from addresses passed
+  // to ptrace. So put some random bits in there.
+  // ptrace expects you to remove them but it can still succeed if you
+  // don't. So this isn't proof that we're removing them, it's just a
+  // smoke test in case something didn't account for them.
+  buf = (char *)((size_t)buf | ((size_t)0xAA << 56));
+  return print_result(buf);
+}
Index: lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
@@ -0,0 +1,116 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemoteMemoryTagging(gdbremote_testcase.GdbRemoteTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def check_qmemtags_response(self, body, expected):
+self.test_sequence.add_log_lines(["read packet: $qMemTags:{}#00".format(body),
+  "send packet: ${}#00".format(expected),
+  ],
+ True)
+self.expect_gdbremote_sequence()
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_qmemtags_packets(self):
+""" Test that qMemTags packets are parsed correctly and/or rejected. """
+
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior()
+
+# Run the process
+self.test_sequence.add_log_lines(
+[
+# Start running after initial stop
+"read packet: $c#63",
+		# Match the address of the MTE page
+{"type": "output_match",
+ "regex": self.maybe_strict_output_regex(r"buffer: (.+) page_size: (.+)\r\n"),
+ "capture": {1: "buffer", 2: 

[Lldb-commits] [PATCH] D97282: [lldb][AArch64] Add memory-tagging qSupported feature

[David Spickett via Phabricator via lldb-commits]

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

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97282

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Target/Process.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1025,6 +1025,14 @@
 self.assertEqual(supported_dict.get('fork-events', '-'), '-')
 self.assertEqual(supported_dict.get('vfork-events', '-'), '-')
 
+# We need to be able to self.runCmd to get cpuinfo,
+# which is not possible when using a remote platform.
+@skipIfRemote
+def test_qSupported_memory_tagging(self):
+supported_dict = self.get_qSupported_dict()
+self.assertEqual(supported_dict.get("memory-tagging", '-'),
+ '+' if self.isAArch64MTE() else '-')
+
 @skipIfWindows # No pty support to test any inferior output
 def test_written_M_content_reads_back_correctly(self):
 self.build()
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -235,6 +235,8 @@
   friend class GDBRemoteCommunicationClient;
   friend class GDBRemoteRegisterContext;
 
+  bool SupportsMemoryTagging() override;
+
   /// Broadcaster event bits definitions.
   enum {
 eBroadcastBitAsyncContinue = (1 << 0),
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2768,6 +2768,10 @@
   return 0;
 }
 
+bool ProcessGDBRemote::SupportsMemoryTagging() {
+  return m_gdb_comm.GetMemoryTaggingSupported();
+}
+
 Status ProcessGDBRemote::WriteObjectFile(
 std::vector entries) {
   Status error;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3608,6 +3608,8 @@
 ret.push_back("qXfer:auxv:read+");
   if (bool(plugin_features & Extension::libraries_svr4))
 ret.push_back("qXfer:libraries-svr4:read+");
+  if (bool(plugin_features & Extension::memory_tagging))
+ret.push_back("memory-tagging+");
 
   // check for client features
   m_extensions_supported = {};
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -451,6 +451,8 @@
 
   bool GetSharedCacheInfoSupported();
 
+  bool GetMemoryTaggingSupported();
+
   /// Use qOffsets to query the offset used when relocating the target
   /// executable. If successful, the returned structure will contain at least
   /// one value in the offsets field.
@@ -558,6 +560,7 @@
   LazyBool m_supports_QPassSignals = eLazyBoolCalculate;
   LazyBool m_supports_error_string_reply = eLazyBoolCalculate;
   LazyBool m_supports_multiprocess = eLazyBoolCalculate;
+  LazyBool m_supports_memory_tagging = eLazyBoolCalculate;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
   m_supports_qUserName : 1, m_supports_qGroupName : 1,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -309,6 +309,7 @@
   m_supports_multiprocess = eLazyBoolNo;
   m_supports_qEcho = eLazyBoolNo;
   m_supports_QPassSignals = eLazyBoolNo;
+  m_supports_memory_tagging = eLazyBoolNo;
 
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
 

[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags

[David Spickett via Phabricator via lldb-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97281

Files:
  lldb/include/lldb/Target/MemoryTagManager.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp

Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
@@ -0,0 +1,120 @@
+//===-- MemoryTagManagerAArch64MTETest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(MemoryTagManagerAArch64MTETest, UnpackTagsData) {
+  MemoryTagManagerAArch64MTE manager;
+
+  // Error for insufficient tag data
+  std::vector input;
+  ASSERT_THAT_EXPECTED(
+  manager.UnpackTagsData(input, 2),
+  llvm::FailedWithMessage(
+  "Packed tag data size does not match expected number of tags. "
+  "Expected 2 tag(s) for 2 granules, got 0 tag(s)."));
+
+  // This is out of the valid tag range
+  input.push_back(0x1f);
+  ASSERT_THAT_EXPECTED(
+  manager.UnpackTagsData(input, 1),
+  llvm::FailedWithMessage(
+  "Found tag 0x1f which is > max MTE tag value of 0xf."));
+
+  // MTE tags are 1 per byte
+  input.pop_back();
+  input.push_back(0xe);
+  input.push_back(0xf);
+
+  std::vector expected{0xe, 0xf};
+
+  llvm::Expected> got =
+  manager.UnpackTagsData(input, 2);
+  ASSERT_THAT_EXPECTED(got, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*got));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) {
+  MemoryTagManagerAArch64MTE manager;
+
+  // Set surrounding bits to check shift is correct
+  ASSERT_EQ((lldb::addr_t)0, manager.GetLogicalTag(0xe0e0));
+  // Max tag value
+  ASSERT_EQ((lldb::addr_t)0xf, manager.GetLogicalTag(0x0f00));
+  ASSERT_EQ((lldb::addr_t)2, manager.GetLogicalTag(0x0200));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, ExpandToGranule) {
+  MemoryTagManagerAArch64MTE manager;
+  // Reading nothing, no alignment needed
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0, 0),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0, 0)));
+
+  // Ranges with 0 size are unchanged even if address is non 0
+  // (normally 0x1234 would be aligned to 0x1230)
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0x1234, 0),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0x1234, 0)));
+
+  // Ranges already aligned don't change
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0x100, 64),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0x100, 64)));
+
+  // Any read of less than 1 granule is rounded up to reading 1 granule
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0, 16),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(0, 1)));
+
+  // Start address is aligned down, and length modified accordingly
+  // Here bytes 8 through 24 straddle 2 granules. So the resulting range starts
+  // at 0 and covers 32 bytes.
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(0, 32),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(8, 16)));
+
+  // Here only the size of the range needs aligning
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(16, 32),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(16, 24)));
+
+  // Start and size need aligning here but we only need 1 granule to cover it
+  ASSERT_EQ(
+  MemoryTagManagerAArch64MTE::TagRange(16, 16),
+  manager.ExpandToGranule(MemoryTagManagerAArch64MTE::TagRange(18, 4)));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, RemoveNonAddressBits) {
+  MemoryTagManagerAArch64MTE manager;
+
+  ASSERT_EQ(0, 0);
+  ASSERT_EQ((lldb::addr_t)0x00ffeedd11223344,
+manager.RemoveNonAddressBits(0x00ffeedd11223344));
+  ASSERT_EQ((lldb::addr_t)0x,
+manager.RemoveNonAddressBits(0xFF00));
+  ASSERT_EQ((lldb::addr_t)0x0055,
+manager.RemoveNonAddressBits(0xee55));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, AddressDiff) {
+  MemoryTagManagerAArch64MTE manager;

[Lldb-commits] [PATCH] D81550: [lldb] Add support for evaluating expressions in static member functions

[Raphael Isemann via Phabricator via lldb-commits]

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

Reopen because of the revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81550

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

[Lldb-commits] [lldb] 4125b46 - Revert "[lldb] Add support for evaluating expressions in static member functions"

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-06-11T15:00:38+02:00
New Revision: 4125b46232caf6a558470670f87b56ccb46d61aa

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

LOG: Revert "[lldb] Add support for evaluating expressions in static member 
functions"

This reverts commit 00764c36edf88ae9806e8d57a6addb782e6ceae8 and the
follow up d2223c7a49973a61cc2de62992662afa8d19065a.

The original patch broke that one could use static member variables while
inside a static member functions without having a running target. It seems that
LLDB currently requires that static variables are only found via the global
variable lookup so that they can get materialized and mapped to the argument
struct of the expression.

After 00764c36edf88ae9806e8d57a6addb782e6ceae8 static variables of the current
class could be found via Clang's lookup which LLDB isn't observing. This
resulting in expressions actually containing these variables as normal
globals that can't be rewritten to a member of the argument struct.

More specifically, in the test TestCPPThis, the expression
`expr --j false -- s_a` is now only passing if we have a runnable target.

I'll revert the patch as the possible fixes aren't trivial and it degrades
the debugging experience more than the issue that the revert patch addressed.

The underlying bug can be reproduced before/after this patch by stopping
in `TestCPPThis` main function and running: `e -j false -- my_a; A::s_a`.
The `my_a` will pull in the `A` class and the second expression will
be resolved by Clang on its own (which causes LLDB to not materialize the
static variable).

Note: A workaround is to just do `::s_a` which will force LLDB to take the 
global
variable lookup.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

Removed: 
lldb/test/API/lang/cpp/stopped_in_static_member_function/Makefile

lldb/test/API/lang/cpp/stopped_in_static_member_function/TestStoppedInStaticMemberFunction.py
lldb/test/API/lang/cpp/stopped_in_static_member_function/main.cpp



diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index 761e6aa273f76..731b81c61a6f8 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -810,7 +810,7 @@ void 
ClangExpressionDeclMap::LookUpLldbClass(NameSearchContext ) {
 LLDB_LOG(log, "  CEDM::FEVD Adding type for $__lldb_class: {1}",
  class_qual_type.getAsString());
 
-AddContextClassType(context, class_user_type, method_decl);
+AddContextClassType(context, class_user_type);
 
 if (method_decl->isInstance()) {
   // self is a pointer to the object
@@ -1890,9 +1890,8 @@ void 
ClangExpressionDeclMap::AddOneFunction(NameSearchContext ,
   }
 }
 
-void ClangExpressionDeclMap::AddContextClassType(
-NameSearchContext , const TypeFromUser ,
-CXXMethodDecl *context_method) {
+void ClangExpressionDeclMap::AddContextClassType(NameSearchContext ,
+ const TypeFromUser ) {
   CompilerType copied_clang_type = GuardedCopyType(ut);
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
@@ -1914,12 +1913,7 @@ void ClangExpressionDeclMap::AddContextClassType(
 void_clang_type, _ptr_clang_type, 1, false, 0);
 
 const bool is_virtual = false;
-// If we evaluate an expression inside a static method, we also need to
-// make our lldb_expr method static so that Clang denies access to
-// non-static members.
-// If we don't have a context_method we are evaluating within a context
-// object and we can allow access to non-static members.
-const bool is_static = context_method ? context_method->isStatic() : false;
+const bool is_static = false;
 const bool is_inline = false;
 const bool is_explicit = false;
 const bool is_attr_used = true;

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
index 1af5df95821e5..93060bd6b587e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -607,13 +607,8 @@ class 

[Lldb-commits] [lldb] cd2e500 - [lldb] DumpDataExtractor.cpp - replace APInt::toString() with llvm::toString(APInt)

[Simon Pilgrim via lldb-commits]

Author: Simon Pilgrim
Date: 2021-06-11T13:39:14+01:00
New Revision: cd2e500e555e134ffc19d07f26b17d0b0c71efd9

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

LOG: [lldb] DumpDataExtractor.cpp - replace APInt::toString() with 
llvm::toString(APInt)

APInt::toString() was removed rG61cdaf66fe22be2b5942ddee4f46a998b4f3ee29

Added: 


Modified: 
lldb/source/Core/DumpDataExtractor.cpp

Removed: 




diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index 66fcfbd43bd29..93aaae8a10408 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -116,7 +116,7 @@ static lldb::offset_t DumpAPInt(Stream *s, const 
DataExtractor ,
 bool is_signed, unsigned radix) {
   llvm::Optional apint = GetAPInt(data, , byte_size);
   if (apint.hasValue()) {
-std::string apint_str(apint.getValue().toString(radix, is_signed));
+std::string apint_str = toString(apint.getValue(), radix, is_signed));
 switch (radix) {
 case 2:
   s->Write("0b", 2);



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

[Lldb-commits] [PATCH] D104067: [lldb] Decouple ObjCLanguage from Symtab

[Raphael Isemann via Phabricator via lldb-commits]

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

In D104067#2811834 , @jingham wrote:

> This looks pretty good to me.
>
> It's a little awkward in InitNameIndexes that we look up the various 
> NameToSymbolIndex maps by eFunctionNameType, use the function name type again 
> to sort the names & index pairs into the bucket we looked up before.  I 
> wonder if that could be made cleaner by having an
>
> AddToSymbolNameToIndexMap(symbol_name, index, func_name_type)
>
> interface, which would just sort the symbol names into the right map.  Not 
> sure that's worth the bother, however.

That sounds good to me as a follow-up refactoring.

Only some small complains but otherwise this seems pretty good. Someone (*puts 
finger on nose*) should maybe do some stats whether




Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:225
+  auto variant_name = variant_name_and_type.first;
   Module::LookupInfo variant_lookup(name, name_type_mask,
 lang->GetLanguageType());

Shouldn't that use the type form the variant instead of the `name_type_mask`? 
FWIW, I would prefer if we keep this code unchanged as right now we introduce 
the selectors to this list of lookups.

So what about adding filter here for  `eFunctionNameTypeFull` to keep this 
patch NFC? And then maybe a `FIXME: ` to figure out if we should add variants 
that aren't the full name.



Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:104
+  // We also return the FunctionNameType of each possible name.
+  std::vector>
   GetMethodNameVariants(ConstString method_name) const override;

Could we make this a custom struct? `first` `second` are always so 
non-descriptive and we might want to extend what we return from this function 
in a future patch.

```
lang=c++
class MethodNameVariant {
  ConstString m_name;
  lldb::FunctionNameType m_type;
public:
  [...]
}```



Comment at: lldb/source/Symbol/Symtab.cpp:332
 // name, add the version without categories to the index too.
-ObjCLanguage::MethodName objc_method(name.GetStringRef(), true);
-if (objc_method.IsValid(true)) {
-  selector_to_index.Append(objc_method.GetSelector(), value);
-
-  if (ConstString objc_method_no_category =
-  objc_method.GetFullNameWithoutCategory(true))
-name_to_index.Append(objc_method_no_category, value);
+if (auto *objc_lang = Language::FindPlugin(lldb::eLanguageTypeObjC)) {
+  for (auto variant_name_and_type :

jingham wrote:
> Shouldn't this be in a loop over the supported languages?
+1

I also wonder if this might become quite expensive in the future if we end up 
with more language plugins, but I guess in that case we can make some kind of 
initial query pass where plugins can register whether they care about this 
indexing stuff here. Anyway, I don't think that's a real concern at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104067

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

[Lldb-commits] [lldb] f3f9045 - [lldb] Fix leak in test

[Vitaly Buka via lldb-commits]

Author: Vitaly Buka
Date: 2021-06-11T00:20:35-07:00
New Revision: f3f904563ec9ce8c7bfda83bbca19790cc4d9afc

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

LOG: [lldb] Fix leak in test

Test leaks if we run
tools/lldb/unittests/Host/HostTests without --gtest_filter

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/include/lldb/Host/linux/HostInfoLinux.h
lldb/source/Host/linux/HostInfoLinux.cpp
lldb/unittests/Host/HostInfoTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/linux/HostInfoLinux.h 
b/lldb/include/lldb/Host/linux/HostInfoLinux.h
index e8080033cd8f8..4a7bb6cc11fbb 100644
--- a/lldb/include/lldb/Host/linux/HostInfoLinux.h
+++ b/lldb/include/lldb/Host/linux/HostInfoLinux.h
@@ -28,6 +28,7 @@ class HostInfoLinux : public HostInfoPosix {
 
 public:
   static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+  static void Terminate();
 
   static llvm::VersionTuple GetOSVersion();
   static bool GetOSBuildString(std::string );

diff  --git a/lldb/source/Host/linux/HostInfoLinux.cpp 
b/lldb/source/Host/linux/HostInfoLinux.cpp
index 36ac0ec6c3c37..6c37b97d7ccfb 100644
--- a/lldb/source/Host/linux/HostInfoLinux.cpp
+++ b/lldb/source/Host/linux/HostInfoLinux.cpp
@@ -41,6 +41,13 @@ void HostInfoLinux::Initialize(SharedLibraryDirectoryHelper 
*helper) {
   g_fields = new HostInfoLinuxFields();
 }
 
+void HostInfoLinux::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
   assert(g_fields && "Missing call to Initialize?");
   llvm::call_once(g_fields->m_os_version_once_flag, []() {

diff  --git a/lldb/unittests/Host/HostInfoTest.cpp 
b/lldb/unittests/Host/HostInfoTest.cpp
index 96d47d75794a3..0accdd8dbcdbf 100644
--- a/lldb/unittests/Host/HostInfoTest.cpp
+++ b/lldb/unittests/Host/HostInfoTest.cpp
@@ -60,3 +60,16 @@ TEST_F(HostInfoTest, GetXcodeSDK) {
   
EXPECT_TRUE(HostInfo::GetXcodeSDKPath(XcodeSDK("CeciNestPasUnOS.sdk")).empty());
 }
 #endif
+
+TEST(HostInfoTestInitialization, InitTwice) {
+  llvm::VersionTuple Version;
+  {
+SubsystemRAII subsystems;
+Version = HostInfo::GetOSVersion();
+  }
+
+  {
+SubsystemRAII subsystems;
+EXPECT_EQ(Version, HostInfo::GetOSVersion());
+  }
+}



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

[Lldb-commits] [PATCH] D104091: [lldb] Fix leak in test

[Vitaly Buka 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 rGf3f904563ec9: [lldb] Fix leak in test (authored by 
vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104091

Files:
  lldb/include/lldb/Host/linux/HostInfoLinux.h
  lldb/source/Host/linux/HostInfoLinux.cpp
  lldb/unittests/Host/HostInfoTest.cpp


Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -60,3 +60,16 @@
   
EXPECT_TRUE(HostInfo::GetXcodeSDKPath(XcodeSDK("CeciNestPasUnOS.sdk")).empty());
 }
 #endif
+
+TEST(HostInfoTestInitialization, InitTwice) {
+  llvm::VersionTuple Version;
+  {
+SubsystemRAII subsystems;
+Version = HostInfo::GetOSVersion();
+  }
+
+  {
+SubsystemRAII subsystems;
+EXPECT_EQ(Version, HostInfo::GetOSVersion());
+  }
+}
Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -41,6 +41,13 @@
   g_fields = new HostInfoLinuxFields();
 }
 
+void HostInfoLinux::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
   assert(g_fields && "Missing call to Initialize?");
   llvm::call_once(g_fields->m_os_version_once_flag, []() {
Index: lldb/include/lldb/Host/linux/HostInfoLinux.h
===
--- lldb/include/lldb/Host/linux/HostInfoLinux.h
+++ lldb/include/lldb/Host/linux/HostInfoLinux.h
@@ -28,6 +28,7 @@
 
 public:
   static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+  static void Terminate();
 
   static llvm::VersionTuple GetOSVersion();
   static bool GetOSBuildString(std::string );


Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -60,3 +60,16 @@
   EXPECT_TRUE(HostInfo::GetXcodeSDKPath(XcodeSDK("CeciNestPasUnOS.sdk")).empty());
 }
 #endif
+
+TEST(HostInfoTestInitialization, InitTwice) {
+  llvm::VersionTuple Version;
+  {
+SubsystemRAII subsystems;
+Version = HostInfo::GetOSVersion();
+  }
+
+  {
+SubsystemRAII subsystems;
+EXPECT_EQ(Version, HostInfo::GetOSVersion());
+  }
+}
Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -41,6 +41,13 @@
   g_fields = new HostInfoLinuxFields();
 }
 
+void HostInfoLinux::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
   assert(g_fields && "Missing call to Initialize?");
   llvm::call_once(g_fields->m_os_version_once_flag, []() {
Index: lldb/include/lldb/Host/linux/HostInfoLinux.h
===
--- lldb/include/lldb/Host/linux/HostInfoLinux.h
+++ lldb/include/lldb/Host/linux/HostInfoLinux.h
@@ -28,6 +28,7 @@
 
 public:
   static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+  static void Terminate();
 
   static llvm::VersionTuple GetOSVersion();
   static bool GetOSBuildString(std::string );
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104091: [lldb] Fix leak in test

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, big thanks for the patch & quick turnaround!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104091

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

[Lldb-commits] [PATCH] D104091: [lldb] Fix leak in test

[Vitaly Buka via Phabricator via lldb-commits]

vitalybuka updated this revision to Diff 351347.
vitalybuka marked 2 inline comments as done.
vitalybuka edited the summary of this revision.
vitalybuka added a comment.

::Terminate


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104091

Files:
  lldb/include/lldb/Host/linux/HostInfoLinux.h
  lldb/source/Host/linux/HostInfoLinux.cpp
  lldb/unittests/Host/HostInfoTest.cpp


Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -60,3 +60,16 @@
   
EXPECT_TRUE(HostInfo::GetXcodeSDKPath(XcodeSDK("CeciNestPasUnOS.sdk")).empty());
 }
 #endif
+
+TEST(HostInfoTestInitialization, InitTwice) {
+  llvm::VersionTuple Version;
+  {
+SubsystemRAII subsystems;
+Version = HostInfo::GetOSVersion();
+  }
+
+  {
+SubsystemRAII subsystems;
+EXPECT_EQ(Version, HostInfo::GetOSVersion());
+  }
+}
Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -41,6 +41,13 @@
   g_fields = new HostInfoLinuxFields();
 }
 
+void HostInfoLinux::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
   assert(g_fields && "Missing call to Initialize?");
   llvm::call_once(g_fields->m_os_version_once_flag, []() {
Index: lldb/include/lldb/Host/linux/HostInfoLinux.h
===
--- lldb/include/lldb/Host/linux/HostInfoLinux.h
+++ lldb/include/lldb/Host/linux/HostInfoLinux.h
@@ -28,6 +28,7 @@
 
 public:
   static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+  static void Terminate();
 
   static llvm::VersionTuple GetOSVersion();
   static bool GetOSBuildString(std::string );


Index: lldb/unittests/Host/HostInfoTest.cpp
===
--- lldb/unittests/Host/HostInfoTest.cpp
+++ lldb/unittests/Host/HostInfoTest.cpp
@@ -60,3 +60,16 @@
   EXPECT_TRUE(HostInfo::GetXcodeSDKPath(XcodeSDK("CeciNestPasUnOS.sdk")).empty());
 }
 #endif
+
+TEST(HostInfoTestInitialization, InitTwice) {
+  llvm::VersionTuple Version;
+  {
+SubsystemRAII subsystems;
+Version = HostInfo::GetOSVersion();
+  }
+
+  {
+SubsystemRAII subsystems;
+EXPECT_EQ(Version, HostInfo::GetOSVersion());
+  }
+}
Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -41,6 +41,13 @@
   g_fields = new HostInfoLinuxFields();
 }
 
+void HostInfoLinux::Terminate() {
+  assert(g_fields && "Missing call to Initialize?");
+  delete g_fields;
+  g_fields = nullptr;
+  HostInfoBase::Terminate();
+}
+
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
   assert(g_fields && "Missing call to Initialize?");
   llvm::call_once(g_fields->m_os_version_once_flag, []() {
Index: lldb/include/lldb/Host/linux/HostInfoLinux.h
===
--- lldb/include/lldb/Host/linux/HostInfoLinux.h
+++ lldb/include/lldb/Host/linux/HostInfoLinux.h
@@ -28,6 +28,7 @@
 
 public:
   static void Initialize(SharedLibraryDirectoryHelper *helper = nullptr);
+  static void Terminate();
 
   static llvm::VersionTuple GetOSVersion();
   static bool GetOSBuildString(std::string );
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104093: [lldb] Move once_flags in HostInfoLinux so the internal state struct

[Raphael Isemann 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 rG632cbcac7906: [lldb] Move once_flags in HostInfoLinux so the 
internal state struct (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104093

Files:
  lldb/source/Host/linux/HostInfoLinux.cpp


Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -26,7 +26,9 @@
 
 namespace {
 struct HostInfoLinuxFields {
+  llvm::once_flag m_distribution_once_flag;
   std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
   llvm::VersionTuple m_os_version;
 };
 
@@ -40,8 +42,8 @@
 }
 
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
 struct utsname un;
 if (uname() != 0)
   return;
@@ -82,10 +84,10 @@
 }
 
 llvm::StringRef HostInfoLinux::GetDistributionId() {
+  assert(g_fields && "Missing call to Initialize?");
   // Try to run 'lbs_release -i', and use that response for the distribution
   // id.
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  llvm::call_once(g_fields->m_distribution_once_flag, []() {
 
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST));
 LLDB_LOGF(log, "attempting to determine Linux distribution...");


Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -26,7 +26,9 @@
 
 namespace {
 struct HostInfoLinuxFields {
+  llvm::once_flag m_distribution_once_flag;
   std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
   llvm::VersionTuple m_os_version;
 };
 
@@ -40,8 +42,8 @@
 }
 
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
 struct utsname un;
 if (uname() != 0)
   return;
@@ -82,10 +84,10 @@
 }
 
 llvm::StringRef HostInfoLinux::GetDistributionId() {
+  assert(g_fields && "Missing call to Initialize?");
   // Try to run 'lbs_release -i', and use that response for the distribution
   // id.
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  llvm::call_once(g_fields->m_distribution_once_flag, []() {
 
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST));
 LLDB_LOGF(log, "attempting to determine Linux distribution...");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 632cbca - [lldb] Move once_flags in HostInfoLinux so the internal state struct

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-06-11T08:53:38+02:00
New Revision: 632cbcac79065a62a306dbda7b3a6e1f315e3260

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

LOG: [lldb] Move once_flags in HostInfoLinux so the internal state struct

The HostInfoLinuxFields struct is supposed to be set up/torn down on
Initialize/Terminate and should contain all the state of the plugin.
`once_flags` are part of this state and should also be reset on `Terminate` so
we can re-initialize these lazy values after the next `Initialize` call.

This itself is NFC as the HostInfoLinux was broken before this patch and is
still broken afterwards. D104091 will be the proper fix.

Reviewed By: vitalybuka

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

Added: 


Modified: 
lldb/source/Host/linux/HostInfoLinux.cpp

Removed: 




diff  --git a/lldb/source/Host/linux/HostInfoLinux.cpp 
b/lldb/source/Host/linux/HostInfoLinux.cpp
index 5f314cc7b50d9..36ac0ec6c3c37 100644
--- a/lldb/source/Host/linux/HostInfoLinux.cpp
+++ b/lldb/source/Host/linux/HostInfoLinux.cpp
@@ -26,7 +26,9 @@ using namespace lldb_private;
 
 namespace {
 struct HostInfoLinuxFields {
+  llvm::once_flag m_distribution_once_flag;
   std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
   llvm::VersionTuple m_os_version;
 };
 
@@ -40,8 +42,8 @@ void HostInfoLinux::Initialize(SharedLibraryDirectoryHelper 
*helper) {
 }
 
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
 struct utsname un;
 if (uname() != 0)
   return;
@@ -82,10 +84,10 @@ bool HostInfoLinux::GetOSKernelDescription(std::string ) {
 }
 
 llvm::StringRef HostInfoLinux::GetDistributionId() {
+  assert(g_fields && "Missing call to Initialize?");
   // Try to run 'lbs_release -i', and use that response for the distribution
   // id.
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  llvm::call_once(g_fields->m_distribution_once_flag, []() {
 
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST));
 LLDB_LOGF(log, "attempting to determine Linux distribution...");



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

[Lldb-commits] [PATCH] D104093: [lldb] Move once_flags in HostInfoLinux so the internal state struct

[Vitaly Buka via Phabricator via lldb-commits]

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

I see comment on the D104091 


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

https://reviews.llvm.org/D104093

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

[Lldb-commits] [PATCH] D104093: [lldb] Move once_flags in HostInfoLinux so the internal state struct

[Vitaly Buka via Phabricator via lldb-commits]

vitalybuka added inline comments.



Comment at: lldb/source/Host/linux/HostInfoLinux.cpp:41
 
   g_fields = new HostInfoLinuxFields();
 }

this still does not solve memory leak if HostInfoLinux::Initialize called twice


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

https://reviews.llvm.org/D104093

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

[Lldb-commits] [PATCH] D104091: [NFC][lldb] Fix leak in test

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.



Comment at: lldb/unittests/Host/HostInfoTest.cpp:64
+
+TEST(HostInfoTest2, InitTwice) {
+  llvm::VersionTuple Version;

`HostInfoTestInitialization` maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104091

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

[Lldb-commits] [PATCH] D104091: [NFC][lldb] Fix leak in test

[Raphael Isemann via Phabricator via lldb-commits]

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

I think we should instead implement the `Terminate` function that the plugin 
system provides to tear down our state. I made a patch in D104093 
 that
moves the `once_flags` to the internal state struct, so with a proper 
`Terminate` implementation + my patch this should all work properly.

  HostInfoLinux::Terminate() {
assert(g_fields && "Missing call to Initialize?");
delete g_fields;
g_fields = nullptr;
HostInfoBase::Terminate();
  }




Comment at: lldb/source/Host/linux/HostInfoLinux.cpp:38
   HostInfoPosix::Initialize(helper);
-
-  g_fields = new HostInfoLinuxFields();
+  if (!g_fields)
+g_fields = new HostInfoLinuxFields();

Please instead add a `HostInfoLinux::Terminate()` function that deletes and 
zeroes `g_fields` (see `HostInfoBase::Terminate`). Initialize -> Terminate 
should set up and tear down the data structures if possible (not saying that 
LLDB is doing this consistently at the moment, but that's the idea at least).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104091

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

[Lldb-commits] [PATCH] D104093: [lldb] Move once_flags in HostInfoLinux so the internal state struct

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: vitalybuka, LLDB.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.

The HostInfoLinuxFields struct is supposed to be set up/torn down on 
Initialize/Terminate
and should contain all the state of the plugin. `once_flags` are part of this 
state and should
also be reset on `Terminate` so we can re-initialize these lazy values after 
the next
`Initialize` call.

This itself is NFC as the HostInfoLinux was broken before this patch and is 
still broken
afterwards. D104091  will be the proper fix.


https://reviews.llvm.org/D104093

Files:
  lldb/source/Host/linux/HostInfoLinux.cpp


Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -26,7 +26,9 @@
 
 namespace {
 struct HostInfoLinuxFields {
+  llvm::once_flag m_distribution_once_flag;
   std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
   llvm::VersionTuple m_os_version;
 };
 
@@ -40,8 +42,8 @@
 }
 
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
 struct utsname un;
 if (uname() != 0)
   return;
@@ -82,10 +84,10 @@
 }
 
 llvm::StringRef HostInfoLinux::GetDistributionId() {
+  assert(g_fields && "Missing call to Initialize?");
   // Try to run 'lbs_release -i', and use that response for the distribution
   // id.
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  llvm::call_once(g_fields->m_distribution_once_flag, []() {
 
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST));
 LLDB_LOGF(log, "attempting to determine Linux distribution...");


Index: lldb/source/Host/linux/HostInfoLinux.cpp
===
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -26,7 +26,9 @@
 
 namespace {
 struct HostInfoLinuxFields {
+  llvm::once_flag m_distribution_once_flag;
   std::string m_distribution_id;
+  llvm::once_flag m_os_version_once_flag;
   llvm::VersionTuple m_os_version;
 };
 
@@ -40,8 +42,8 @@
 }
 
 llvm::VersionTuple HostInfoLinux::GetOSVersion() {
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  assert(g_fields && "Missing call to Initialize?");
+  llvm::call_once(g_fields->m_os_version_once_flag, []() {
 struct utsname un;
 if (uname() != 0)
   return;
@@ -82,10 +84,10 @@
 }
 
 llvm::StringRef HostInfoLinux::GetDistributionId() {
+  assert(g_fields && "Missing call to Initialize?");
   // Try to run 'lbs_release -i', and use that response for the distribution
   // id.
-  static llvm::once_flag g_once_flag;
-  llvm::call_once(g_once_flag, []() {
+  llvm::call_once(g_fields->m_distribution_once_flag, []() {
 
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST));
 LLDB_LOGF(log, "attempting to determine Linux distribution...");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D103588: [trace] Create a top level ThreadTrace class

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 351344.
wallace edited the summary of this revision.
wallace added a comment.

nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103588

Files:
  lldb/include/lldb/Target/ThreadTrace.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadTraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py

Index: lldb/test/API/commands/trace/TestTraceDumpInstructions.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInstructions.py
+++ lldb/test/API/commands/trace/TestTraceDumpInstructions.py
@@ -160,8 +160,7 @@
 self.expect("trace load " +
 os.path.join(self.getSourceDir(), "intelpt-trace", "trace_wrong_cpu.json"))
 self.expect("thread trace dump instructions",
-substrs=['''thread #1: tid = 3842849, total instructions = 1
-[0] error: unknown cpu'''])
+substrs=['''thread #1: tid = 3842849, error: unknown cpu'''])
 
 def testMultiFileTraceWithMissingModule(self):
 self.expect("trace load " +
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -141,8 +141,8 @@
 /// If \b true, then the \a InstructionSymbolInfo will have the
 /// \a disassembler and \a instruction objects calculated.
 static void TraverseInstructionsWithSymbolInfo(
-Trace , Thread , size_t position,
-Trace::TraceDirection direction, SymbolContextItem symbol_scope,
+ThreadTraceSP thread_trace_sp, Thread , size_t position,
+TraceDirection direction, SymbolContextItem symbol_scope,
 bool include_disassembler,
 std::function insn)>
 callback) {
@@ -197,8 +197,8 @@
 : InstructionSP());
   };
 
-  trace.TraverseInstructions(
-  thread, position, direction,
+  thread_trace_sp->TraverseInstructions(
+  position, direction,
   [&](size_t index, Expected load_address) -> bool {
 if (!load_address)
   return callback(index, load_address.takeError());
@@ -301,59 +301,59 @@
 
 void Trace::DumpTraceInstructions(Thread , Stream , size_t count,
   size_t end_position, bool raw) {
-  Optional instructions_count = GetInstructionCount(thread);
-  if (!instructions_count) {
-s.Printf("thread #%u: tid = %" PRIu64 ", not traced\n", thread.GetIndexID(),
- thread.GetID());
-return;
+  s.Printf("thread #%u: tid = %" PRIu64, thread.GetIndexID(), thread.GetID());
+
+  if (Expected thread_trace_sp = GetThreadTrace(thread)) {
+size_t instructions_count = thread_trace_sp.get()->GetInstructionCount();
+s.Printf(", total instructions = %zu\n", instructions_count);
+
+if (count == 0 || end_position >= instructions_count)
+  return;
+
+int digits_count = GetNumberOfDigits(end_position);
+size_t start_position =
+end_position + 1 < count ? 0 : end_position + 1 - count;
+auto printInstructionIndex = [&](size_t index) {
+  s.Printf("[%*zu] ", digits_count, index);
+};
+
+bool was_prev_instruction_an_error = false;
+Optional prev_insn;
+
+TraverseInstructionsWithSymbolInfo(
+*thread_trace_sp, thread, start_position, TraceDirection::Forwards,
+eSymbolContextEverything, /*disassembler*/ true,
+[&](size_t index, Expected insn) -> bool {
+  if (!insn) {
+printInstructionIndex(index);
+s << toString(insn.takeError());
+
+prev_insn = None;
+was_prev_instruction_an_error = true;
+  } else {
+if (was_prev_instruction_an_error)
+  s.Printf("...missing instructions\n");
+
+if (!raw)
+  DumpInstructionSymbolContext(s, prev_insn, *insn);
+
+printInstructionIndex(index);
+s.Printf("0x%016" PRIx64, insn->load_address);
+
+if (!raw)
+  DumpInstructionDisassembly(s, *insn);
+
+prev_insn = *insn;
+was_prev_instruction_an_error = false;
+  }
+
+  s.Printf("\n");
+  return index < end_position;
+});
+
+  } else {
+s.Printf(", %s\n", 

[Lldb-commits] [PATCH] D104054: [lldb] Enable Rust v0 symbol demangling

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Once teemperor's issues are resolved this looks good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104054

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