date:20210601

[Lldb-commits] [PATCH] D103439: [lldb] Print the backtrace for all threads if the test breakpoint can't be hit

2021-06-01 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: JDevlieghere.
teemperor added a project: LLDB.
teemperor requested review of this revision.
Herald added a subscriber: lldb-commits.

At the moment if a test fails to hits the breakpoint we set in 
`run_to_source_breakpoint`
the test suite just prints:

  AssertionError: 0 != 1 : Expected 1 thread to stop at breakpoint, 0 did

Often these errors happen because the debuggee crashed before that breakpoint 
is being
hit or the process failed to launch but it's hard to tell what happened from 
the current error.

This patch adds that we now print the backtrace of all threads in case the test 
failed
because we didn't hit the right breakpoint


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103439

Files:
  lldb/packages/Python/lldbsuite/test/lldbutil.py


Index: lldb/packages/Python/lldbsuite/test/lldbutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -840,6 +840,12 @@
 threads.append(thread)
 return threads
 
+def get_thread_overview(test):
+"""Returns a string providing an overview over all current threads.
+Should be used for making test errors more expressive."""
+test.runCmd("thread backtrace all")
+return "Current threads:\n" + test.res.GetOutput()
+
 # Helper functions for run_to_{source,name}_breakpoint:
 
 def run_to_breakpoint_make_target(test, exe_name = "a.out", in_cwd = True):
@@ -897,9 +903,13 @@
 
 num_threads = len(threads)
 if only_one_thread:
-test.assertEqual(num_threads, 1, "Expected 1 thread to stop at 
breakpoint, %d did."%(num_threads))
+test.assertEqual(num_threads, 1,
+ "Expected 1 thread to stop at breakpoint, %d did.\n%s"
+ % (num_threads, get_thread_overview(test)))
 else:
-test.assertGreater(num_threads, 0, "No threads stopped at breakpoint")
+test.assertGreater(num_threads, 0,
+   "No threads stopped at breakpoint.\n%s" %
+   (get_thread_overview(test)))
 
 thread = threads[0]
 return (target, process, thread, bkpt)


Index: lldb/packages/Python/lldbsuite/test/lldbutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -840,6 +840,12 @@
 threads.append(thread)
 return threads
 
+def get_thread_overview(test):
+"""Returns a string providing an overview over all current threads.
+Should be used for making test errors more expressive."""
+test.runCmd("thread backtrace all")
+return "Current threads:\n" + test.res.GetOutput()
+
 # Helper functions for run_to_{source,name}_breakpoint:
 
 def run_to_breakpoint_make_target(test, exe_name = "a.out", in_cwd = True):
@@ -897,9 +903,13 @@
 
 num_threads = len(threads)
 if only_one_thread:
-test.assertEqual(num_threads, 1, "Expected 1 thread to stop at breakpoint, %d did."%(num_threads))
+test.assertEqual(num_threads, 1,
+ "Expected 1 thread to stop at breakpoint, %d did.\n%s"
+ % (num_threads, get_thread_overview(test)))
 else:
-test.assertGreater(num_threads, 0, "No threads stopped at breakpoint")
+test.assertGreater(num_threads, 0,
+   "No threads stopped at breakpoint.\n%s" %
+   (get_thread_overview(test)))
 
 thread = threads[0]
 return (target, process, thread, bkpt)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D103442: [lldb][NFC] Remove a redundant call to weak_ptr::expired

2021-06-01 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.
Herald added a subscriber: lldb-commits.

The `lock` call directly will check for us if the `weak_ptr` is expired and
returns an invalid `shared_ptr` (which we correctly handle), so this check is 
redundant.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103442

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


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3792,7 +3792,7 @@
 }
 
 SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
-  if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
+  if (m_debug_map_symfile == nullptr) {
 lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
 if (module_sp) {
   m_debug_map_symfile =


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3792,7 +3792,7 @@
 }
 
 SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
-  if (m_debug_map_symfile == nullptr && !m_debug_map_module_wp.expired()) {
+  if (m_debug_map_symfile == nullptr) {
 lldb::ModuleSP module_sp(m_debug_map_module_wp.lock());
 if (module_sp) {
   m_debug_map_symfile =
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

2021-06-01 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits

PatriosTheGreat updated this revision to Diff 348919.
PatriosTheGreat added a comment.

Select most related frame only in threads which were stopped with reason.
This diff is for further discussion.


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

https://reviews.llvm.org/D103271

Files:
  lldb/source/Target/Thread.cpp


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -606,7 +606,9 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
+  lldb::StopReason stop_reason = GetStopReason();
+  if (stop_reason != lldb::StopReason::eStopReasonNone)
+SelectMostRelevantFrame();
 
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -606,7 +606,9 @@
 void Thread::WillStop() {
   ThreadPlan *current_plan = GetCurrentPlan();
 
-  SelectMostRelevantFrame();
+  lldb::StopReason stop_reason = GetStopReason();
+  if (stop_reason != lldb::StopReason::eStopReasonNone)
+SelectMostRelevantFrame();
 
   // FIXME: I may decide to disallow threads with no plans.  In which
   // case this should go to an assert.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D103454: [lldb][docs] Document SBType

2021-06-01 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.
Herald added a subscriber: lldb-commits.

This documents the behaviour of the different SBType functions with notes for 
the
language-specific behaviour for C/C++/Objective-C. All of this reflects the 
current
behaviour of LLDB (even though that also means some functions behave kinda weird
but at least they are now documented to be weird)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103454

Files:
  lldb/bindings/interface/SBType.i

Index: lldb/bindings/interface/SBType.i
===
--- lldb/bindings/interface/SBType.i
+++ lldb/bindings/interface/SBType.i
@@ -111,8 +111,24 @@
 };
 
 %feature("docstring",
-"Represents a data type in lldb.  The FindFirstType() method of SBTarget/SBModule
-returns a SBType.
+"Represents a data type in lldb.
+
+The actual characteristics of each type are defined by the semantics of the
+programming language and the specific language implementation that was used
+to compile the target program. See the language-specific notes in the
+documentation of each method.
+
+SBType instances can be obtained by a variety of methods.
+`SBTarget.FindFirstType` and `SBModule.FindFirstType` can be used to create
+`SBType` representations of types in executables/libraries with debug
+information. For some languages such as C, C++ and Objective-C it is possible
+to create new types by evaluating expressions that define a new type.
+
+Note that most `SBType` properties are computed independently of any runtime
+information so for dynamic languages the functionality can be very limited.
+`SBValue` can be used to represent runtime values which then can be more
+accurately queried for certain information such as byte size.
+
 
 SBType supports the eq/ne operator. For example,::
 
@@ -181,13 +197,13 @@
 # id_type and int_type should be the same type!
 self.assertTrue(id_type == int_type)
 
-...") SBType;
+") SBType;
 class SBType
 {
 public:
-SBType ();
+`SBType` ();
 
-SBType (const lldb::SBType &rhs);
+`SBType` (const lldb::SBType &rhs);
 
 ~SBType ();
 
@@ -196,132 +212,637 @@
 
 explicit operator bool() const;
 
+
+%feature("docstring",
+"The number of bytes a variable with the given types occupies in memory.
+
+Returns ``0`` if the size can't be determined.
+
+If a type occupies ``N`` bytes + ``M`` bits in memory, this function returns
+the rounded up amount of bytes (i.e., if ``M`` is ``0``,
+this function returns ``N`` and otherwise ``N + 1``).
+
+Language-specific behaviour:
+
+* C: The output is expected to match the value of ``sizeof(Type)``. If
+  ``sizeof(Type)`` is not a valid expression for the given type, the
+  function returns ``0``.
+* C++: Same as in C.
+* Objective-C: Same as in C. For Objective-C classes this always returns
+  `0`` as the actual size depends on runtime information.
+") GetByteSize;
 uint64_t
 GetByteSize();
 
+
+%feature("docstring",
+"Returns true if this type is a pointer type.
+
+Language-specific behaviour:
+
+* C: Returns true for C pointer types (or typedefs of these types).
+* C++: Pointer types include the C pointer types as well as pointers to data
+  mebers or member functions.
+* Objective-C: Pointer types include the C pointer types. ``id``, ``Class``
+  and pointers to blocks are also considered pointer types.
+") IsPointerType;
 bool
 IsPointerType();
 
+%feature("docstring",
+"Returns true if this type is a reference type.
+
+Language-specific behaviour:
+
+* C: Returns false for all types.
+* C++: Both l-value and r-value references are considered reference types.
+* Objective-C: Returns false for all types.
+") IsReferenceType;
 bool
 IsReferenceType();
 
+%feature("docstring",
+"Returns true if this type is a function type.
+
+Language-specific behaviour:
+
+* C: Returns true for types that represent functions. Note that function
+  pointers are not function types (but their `GetPointeeType()` are function
+  types).
+* C++: Same as in C.
+* Objective-C: Returns false for all types.
+") IsPolymorphicClass;
 bool
 IsFunctionType ();
 
+%feature("docstring",
+"Returns true if this type is a polymorphic type.
+
+Language-specific behaviour:
+
+* C: Returns false for all types.
+* C++: Returns true if the type is a class type that contains at least one
+  virtual member function or if at least one of its base classes is
+  considered a polymorphic type.
+* Objective-C: Returns false for all types.
+") IsPolymorphicClass;
 bool
 IsPolymorphicClass ();
 
+%feature("docstring",
+"Returns true if this type is

[Lldb-commits] [PATCH] D103454: [lldb][docs] Document SBType

2021-06-01 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

The language here might shift between functions and needs to be synced up (that 
patch was written over a few weeks during some build/test idle time). But the 
patch seems good enough for a draft.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103454

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

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

2021-06-01 Thread Sam Parker via Phabricator via lldb-commits

samparker added inline comments.



Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1250
   else
-return false;
+return Changed;
 

dblaikie wrote:
> Also might be worth reaching out to authors to check that this change is 
> intended & possibly tested.
Ah, yes this should be fine. The Changed value would only be used by the 
pipeline manager and this pass is the last transform in the Arm pipeline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102942

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

[Lldb-commits] [PATCH] D103439: [lldb] Print the backtrace for all threads if the test breakpoint can't be hit

2021-06-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103439

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

[Lldb-commits] [lldb] 01fb14e - [lldb] Remove SBCommandReturnObject::ref

2021-06-01 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-06-01T17:57:21+02:00
New Revision: 01fb14e17763269779f2c03b772db960540f47ed

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

LOG: [lldb] Remove SBCommandReturnObject::ref

This function was added in D67589 and returns an internal CommandReturnObject
which isn't allowed in the SB API. This patch just makes it private as all uses
of this function are inside SBCommandReturnObject.

Reviewed By: jankratochvil

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

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/API/SBCommandReturnObject.h 
b/lldb/include/lldb/API/SBCommandReturnObject.h
index 6e1e7adb82caf..5738c911ae125 100644
--- a/lldb/include/lldb/API/SBCommandReturnObject.h
+++ b/lldb/include/lldb/API/SBCommandReturnObject.h
@@ -105,9 +105,6 @@ class LLDB_API SBCommandReturnObject {
 
   void SetError(const char *error_cstr);
 
-  // ref() is internal for LLDB only.
-  lldb_private::CommandReturnObject &ref() const;
-
 protected:
   friend class SBCommandInterpreter;
   friend class SBOptions;
@@ -119,6 +116,8 @@ class LLDB_API SBCommandReturnObject {
   lldb_private::CommandReturnObject &operator*() const;
 
 private:
+  lldb_private::CommandReturnObject &ref() const;
+
   std::unique_ptr m_opaque_up;
 };
 



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

[Lldb-commits] [PATCH] D103390: [lldb] Remove SBCommandReturnObject::ref

2021-06-01 Thread 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 rG01fb14e17763: [lldb] Remove SBCommandReturnObject::ref 
(authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103390

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


Index: lldb/include/lldb/API/SBCommandReturnObject.h
===
--- lldb/include/lldb/API/SBCommandReturnObject.h
+++ lldb/include/lldb/API/SBCommandReturnObject.h
@@ -105,9 +105,6 @@
 
   void SetError(const char *error_cstr);
 
-  // ref() is internal for LLDB only.
-  lldb_private::CommandReturnObject &ref() const;
-
 protected:
   friend class SBCommandInterpreter;
   friend class SBOptions;
@@ -119,6 +116,8 @@
   lldb_private::CommandReturnObject &operator*() const;
 
 private:
+  lldb_private::CommandReturnObject &ref() const;
+
   std::unique_ptr m_opaque_up;
 };
 


Index: lldb/include/lldb/API/SBCommandReturnObject.h
===
--- lldb/include/lldb/API/SBCommandReturnObject.h
+++ lldb/include/lldb/API/SBCommandReturnObject.h
@@ -105,9 +105,6 @@
 
   void SetError(const char *error_cstr);
 
-  // ref() is internal for LLDB only.
-  lldb_private::CommandReturnObject &ref() const;
-
 protected:
   friend class SBCommandInterpreter;
   friend class SBOptions;
@@ -119,6 +116,8 @@
   lldb_private::CommandReturnObject &operator*() const;
 
 private:
+  lldb_private::CommandReturnObject &ref() const;
+
   std::unique_ptr m_opaque_up;
 };
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

2021-06-01 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added inline comments.



Comment at: lldb/include/lldb/Interpreter/CommandReturnObject.h:169
+  /// as a temporary.
+  bool m_hermetic;
 };

bool m_hermetic = false;



Comment at: lldb/source/Interpreter/CommandReturnObject.cpp:46
   m_status(eReturnStatusStarted), m_did_change_process_state(false),
-  m_interactive(true) {}
+  m_interactive(true), m_hermetic(false) {}
 

then we don't need this



Comment at: lldb/source/Interpreter/CommandReturnObject.cpp:155
   m_interactive = true;
+  m_hermetic = false;
 }

or this


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

https://reviews.llvm.org/D103349

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

[Lldb-commits] [PATCH] D103158: [lldb][NFC] Use Language plugins in Mangled::GuessLanguage

2021-06-01 Thread 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 rGecfca427f960: [lldb][NFC] Use Language plugins in 
Mangled::GuessLanguage (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103158

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Core/Mangled.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -105,6 +105,8 @@
   std::vector
   GetMethodNameVariants(ConstString method_name) const override;
 
+  bool SymbolNameFitsToLanguage(Mangled mangled) const override;
+
   lldb::TypeCategoryImplSP GetFormatters() override;
 
   std::vector
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -267,6 +267,13 @@
   return variant_names;
 }
 
+bool ObjCLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
+  ConstString demangled_name = mangled.GetDemangledName();
+  if (!demangled_name)
+return false;
+  return ObjCLanguage::IsPossibleObjCMethodName(demangled_name.GetCString());
+}
+
 static void LoadObjCFormatters(TypeCategoryImplSP objc_category_sp) {
   if (!objc_category_sp)
 return;
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -105,6 +105,8 @@
 
   static lldb_private::ConstString GetPluginNameStatic();
 
+  bool SymbolNameFitsToLanguage(Mangled mangled) const override;
+
   static bool IsCPPMangledName(llvm::StringRef name);
 
   // Extract C++ context and identifier from a string using heuristic matching
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -59,6 +59,11 @@
   return g_name;
 }
 
+bool CPlusPlusLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
+  const char *mangled_name = mangled.GetMangledName().GetCString();
+  return mangled_name && CPlusPlusLanguage::IsCPPMangledName(mangled_name);
+}
+
 // PluginInterface protocol
 
 lldb_private::ConstString CPlusPlusLanguage::GetPluginName() {
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -17,7 +17,6 @@
 #include "lldb/lldb-enumerations.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Demangle/Demangle.h"
@@ -373,22 +372,16 @@
 // of mangling names from a given language, likewise the compilation units
 // within those targets.
 lldb::LanguageType Mangled::GuessLanguage() const {
-  ConstString mangled = GetMangledName();
-
-  if (mangled) {
-const char *mangled_name = mangled.GetCString();
-if (CPlusPlusLanguage::IsCPPMangledName(mangled_name))
-  return lldb::eLanguageTypeC_plus_plus;
-  } else {
-// ObjC names aren't really mangled, so they won't necessarily be in the
-// mangled name slot.
-ConstString demangled_name = GetDemangledName();
-if (demangled_name 
-&& ObjCLanguage::IsPossibleObjCMethodName(demangled_name.GetCString()))
-  return lldb::eLanguageTypeObjC;
-  
-  }
-  return lldb::eLanguageTypeUnknown;
+  lldb::LanguageType result = lldb::eLanguageTypeUnknown;
+  // Ask each language plugin to check if the mangled name belongs to it.
+  Language::ForEach([this, &result](Language *l) {
+if (l->SymbolNameFitsToLanguage(*this)) {
+  result = l->GetLanguageType();
+  return false;
+}
+return true;
+  });
+  return result;
 }
 
 // Dump OBJ to the supplied stream S.
Index: lldb/include/lldb/Target/Language.h
===
--- lldb/include/lldb/Target/Language.h
+++ lldb/include/lldb/Target/Language.h
@@ -192,6 +192,13 @@
 return std::vector();
   };
 
+  /// Returns true iff the given symbol name is compatible with the mangling
+  /// scheme of this language.
+  ///
+  /// This function should only return t

[Lldb-commits] [lldb] ecfca42 - [lldb][NFC] Use Language plugins in Mangled::GuessLanguage

2021-06-01 Thread Raphael Isemann via lldb-commits


Author: Raphael Isemann
Date: 2021-06-01T18:02:07+02:00
New Revision: ecfca427f9601a7789c0703582cff92e7a3277c0

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

LOG: [lldb][NFC] Use Language plugins in Mangled::GuessLanguage

This removes the direct dependency to the ObjC and C++ plugins.

Reviewed By: bulbazord

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

Added: 


Modified: 
lldb/include/lldb/Target/Language.h
lldb/source/Core/Mangled.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.h

Removed: 




diff  --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 6368828e36daf..0639a40020dbe 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -192,6 +192,13 @@ class Language : public PluginInterface {
 return std::vector();
   };
 
+  /// Returns true iff the given symbol name is compatible with the mangling
+  /// scheme of this language.
+  ///
+  /// This function should only return true if there is a high confidence
+  /// that the name actually belongs to this language.
+  virtual bool SymbolNameFitsToLanguage(Mangled name) const { return false; }
+
   // if an individual data formatter can apply to several types and cross a
   // language boundary it makes sense for individual languages to want to
   // customize the printing of values of that type by appending proper

diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index 82cabc0300113..f82c4c5b164f3 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -17,7 +17,6 @@
 #include "lldb/lldb-enumerations.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Demangle/Demangle.h"
@@ -373,22 +372,16 @@ size_t Mangled::MemorySize() const {
 // of mangling names from a given language, likewise the compilation units
 // within those targets.
 lldb::LanguageType Mangled::GuessLanguage() const {
-  ConstString mangled = GetMangledName();
-
-  if (mangled) {
-const char *mangled_name = mangled.GetCString();
-if (CPlusPlusLanguage::IsCPPMangledName(mangled_name))
-  return lldb::eLanguageTypeC_plus_plus;
-  } else {
-// ObjC names aren't really mangled, so they won't necessarily be in the
-// mangled name slot.
-ConstString demangled_name = GetDemangledName();
-if (demangled_name 
-&& ObjCLanguage::IsPossibleObjCMethodName(demangled_name.GetCString()))
-  return lldb::eLanguageTypeObjC;
-  
-  }
-  return lldb::eLanguageTypeUnknown;
+  lldb::LanguageType result = lldb::eLanguageTypeUnknown;
+  // Ask each language plugin to check if the mangled name belongs to it.
+  Language::ForEach([this, &result](Language *l) {
+if (l->SymbolNameFitsToLanguage(*this)) {
+  result = l->GetLanguageType();
+  return false;
+}
+return true;
+  });
+  return result;
 }
 
 // Dump OBJ to the supplied stream S.

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 714c5ccb824ca..d94651784e4e5 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -59,6 +59,11 @@ lldb_private::ConstString 
CPlusPlusLanguage::GetPluginNameStatic() {
   return g_name;
 }
 
+bool CPlusPlusLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
+  const char *mangled_name = mangled.GetMangledName().GetCString();
+  return mangled_name && CPlusPlusLanguage::IsCPPMangledName(mangled_name);
+}
+
 // PluginInterface protocol
 
 lldb_private::ConstString CPlusPlusLanguage::GetPluginName() {

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
index e2b5d29187539..d2a239da9b08a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -105,6 +105,8 @@ class CPlusPlusLanguage : public Language {
 
   static lldb_private::ConstString GetPluginNameStatic();
 
+  bool SymbolNameFitsToLanguage(Mangled mangled) const override;
+
   static bool IsCPPMangledName(llvm::StringRef name);
 
   // Extract C++ context and identifier from a string using heuristic matching

diff  --git a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp 
b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
index 345789ba92a66..785b1fd3b546f 100644
--- a/lldb/source/Plugins/

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

2021-06-01 Thread Michael Benfield via Phabricator via lldb-commits

mbenfield added a comment.

I also heard via email from Amara Emerson that the change is fine for similar 
reasons.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102942

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

[Lldb-commits] [PATCH] D103439: [lldb] Print the backtrace for all threads if the test breakpoint can't be hit

2021-06-01 Thread Augusto Noronha via Phabricator via lldb-commits

augusto2112 accepted this revision.
augusto2112 added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103439

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

[Lldb-commits] [PATCH] D103454: [lldb][docs] Document SBType

2021-06-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

Left few comments regarding the phrasing but LGTM! Thanks for doing it!




Comment at: lldb/bindings/interface/SBType.i:372
+%feature("docstring",
+"Returns the underlying pointer type.
+

Could you keep the same style as the previous function docstring ?



Comment at: lldb/bindings/interface/SBType.i:411
+%feature("docstring",
+"Returns the underlying type of a typedef.
+

Same



Comment at: lldb/bindings/interface/SBType.i:427
+%feature("docstring",
+"Returns the underlying type of a reference type.
+If this type is a reference as designated by `IsReferenceType`, then the

Same + newline



Comment at: lldb/bindings/interface/SBType.i:443
+%feature("docstring",
+"Returns the unqualified version of this type.
+

Same



Comment at: lldb/bindings/interface/SBType.i:458
+%feature("docstring",
+"Returns the underlying integer type if this is an enumeration type.
+

Same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103454

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

[Lldb-commits] [PATCH] D103439: [lldb] Print the backtrace for all threads if the test breakpoint can't be hit

2021-06-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM. Are there other places where we check this, either in `lldbutil` or maybe 
more generally a pattern in the tests that could be extracted into a helper?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103439

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

[Lldb-commits] [PATCH] D103442: [lldb][NFC] Remove a redundant call to weak_ptr::expired

2021-06-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103442

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

[Lldb-commits] [PATCH] D103439: [lldb] Print the backtrace for all threads if the test breakpoint can't be hit

2021-06-01 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

In D103439#2791298 , @JDevlieghere 
wrote:

> LGTM. Are there other places where we check this, either in `lldbutil` or 
> maybe more generally a pattern in the tests that could be extracted into a 
> helper?

From what I can see all our `lldbutil` functions that run to a breakpoint end 
up calling this function so this should cover everything beside the tests that 
manually do stuff like "continue" and so on. But there might be some utility 
function for going to the next breakpoint that are missing this. I'll check, 
thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103439

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

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

2021-06-01 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added a comment.

In D102942#2791204 , @mbenfield wrote:

> I also heard via email from Amara Emerson that the change is fine for similar 
> reasons.

Great, thanks for checking!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102942

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

[Lldb-commits] [PATCH] D103158: [lldb][NFC] Use Language plugins in Mangled::GuessLanguage

2021-06-01 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

I am curious how we would use this, for example `extern "C"` names won't fall 
under these categories, which may be fine depending on how we are using it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103158

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

[Lldb-commits] [PATCH] D103454: [lldb][docs] Document SBType

2021-06-01 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: lldb/bindings/interface/SBType.i:338
+* C: Returns true for anonymous struct and unions.
+* C++: Same as in C.
+* Objective-C: Same as in C.

Worth noting that anonymous structs are a GNU extension in C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103454

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

[Lldb-commits] [PATCH] D103439: [lldb] Print the backtrace for all threads if the test breakpoint can't be hit

2021-06-01 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D103439#2791324 , @teemperor wrote:

> In D103439#2791298 , @JDevlieghere 
> wrote:
>
>> LGTM. Are there other places where we check this, either in `lldbutil` or 
>> maybe more generally a pattern in the tests that could be extracted into a 
>> helper?
>
> From what I can see all our `lldbutil` functions that run to a breakpoint end 
> up calling this function so this should cover everything beside the tests 
> that manually do stuff like "continue" and so on. But there might be some 
> utility function for going to the next breakpoint that are missing this. I'll 
> check, thanks!

You probably also want to do this to "lldbutil.continue_to_breakpoint", except 
that doesn't actually do any tests.  It should really be rewritten to take a 
TestBase and do the assert's in the function, it is inconvenient to use as 
currently written.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103439

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

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

2021-06-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

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



Comment at: lldb/include/lldb/Interpreter/CommandReturnObject.h:169
+  /// as a temporary.
+  bool m_hermetic;
 };

aprantl wrote:
> bool m_hermetic = false;
This is the second time someone has suggested to use in-class member 
initializers. It's not what we've been doing in LLDB so far, but I am a fan of 
it personally, so I'm going to interpret this as what we want to do moving 
forward. I believe it's even a C++ core guideline. Maybe we can have a clang 
tidy check to enforce this. 



Comment at: lldb/source/Interpreter/CommandReturnObject.cpp:155
   m_interactive = true;
+  m_hermetic = false;
 }

aprantl wrote:
> or this
We would still need this :-) 


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

https://reviews.llvm.org/D103349

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

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

2021-06-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 349033.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.

Use in-class initializers


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

https://reviews.llvm.org/D103349

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/test/Shell/Breakpoint/breakpoint-command.test
  lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test

Index: lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test
@@ -7,6 +7,5 @@
 # CHECK-NEXT: foo foo
 # CHECK-NEXT: foo bar
 # CHECK-NEXT: foo bar
-# CHECK-NEXT: foo bar
 # CHECK: script
 # CHECK-NEXT: bar bar
Index: lldb/test/Shell/Breakpoint/breakpoint-command.test
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/breakpoint-command.test
@@ -0,0 +1,5 @@
+# RUN: %build %p/Inputs/dummy-target.c -o %t.out
+# RUN: %lldb %t.out -o 'b main' -o 'break command add 1 -o "script print(95000 + 126)"' -o 'r'
+
+# CHECK: 95125
+# CHECK-NOT: 95126
Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -41,9 +41,7 @@
 }
 
 CommandReturnObject::CommandReturnObject(bool colors)
-: m_out_stream(colors), m_err_stream(colors),
-  m_status(eReturnStatusStarted), m_did_change_process_state(false),
-  m_interactive(true) {}
+: m_out_stream(colors), m_err_stream(colors) {}
 
 void CommandReturnObject::AppendErrorWithFormat(const char *format, ...) {
   if (!format)
@@ -152,6 +150,7 @@
   m_status = eReturnStatusStarted;
   m_did_change_process_state = false;
   m_interactive = true;
+  m_hermetic = false;
 }
 
 bool CommandReturnObject::GetDidChangeProcessState() {
@@ -165,3 +164,7 @@
 bool CommandReturnObject::GetInteractive() const { return m_interactive; }
 
 void CommandReturnObject::SetInteractive(bool b) { m_interactive = b; }
+
+bool CommandReturnObject::GetHermetic() const { return m_hermetic; }
+
+void CommandReturnObject::SetHermetic(bool b) { m_hermetic = b; }
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2306,6 +2306,7 @@
 
 CommandReturnObject tmp_result(m_debugger.GetUseColor());
 tmp_result.SetInteractive(result.GetInteractive());
+tmp_result.SetHermetic(true);
 
 // We might call into a regex or alias command, in which case the
 // add_to_history will get lost.  This m_command_source_depth dingus is the
Index: lldb/include/lldb/Interpreter/CommandReturnObject.h
===
--- lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -63,20 +63,28 @@
   }
 
   void SetImmediateOutputFile(lldb::FileSP file_sp) {
+if (m_hermetic)
+  return;
 lldb::StreamSP stream_sp(new StreamFile(file_sp));
 m_out_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp);
   }
 
   void SetImmediateErrorFile(lldb::FileSP file_sp) {
+if (m_hermetic)
+  return;
 lldb::StreamSP stream_sp(new StreamFile(file_sp));
 m_err_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp);
   }
 
   void SetImmediateOutputStream(const lldb::StreamSP &stream_sp) {
+if (m_hermetic)
+  return;
 m_out_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp);
   }
 
   void SetImmediateErrorStream(const lldb::StreamSP &stream_sp) {
+if (m_hermetic)
+  return;
 m_err_stream.SetStreamAtIndex(eImmediateStreamIndex, stream_sp);
   }
 
@@ -142,16 +150,26 @@
 
   void SetInteractive(bool b);
 
+  bool GetHermetic() const;
+
+  void SetHermetic(bool b);
+
 private:
   enum { eStreamStringIndex = 0, eImmediateStreamIndex = 1 };
 
   StreamTee m_out_stream;
   StreamTee m_err_stream;
 
-  lldb::ReturnStatus m_status;
-  bool m_did_change_process_state;
-  bool m_interactive; // If true, then the input handle from the debugger will
-  // be hooked up
+  lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
+
+  bool m_did_change_process_state = false;
+
+  /// If true, then the input handle from the debugger will be hooked up.
+  bool m_interactive = true;
+
+  /// If true, disallow immediate streams. Useful when using this return object
+  /// as a temporary.
+  bool m_hermetic = false;
 };
 
 } // namespace lldb_private
___
lldb-commits mailing list
lldb-commits@l

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

2021-06-01 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In D103349#2791614 , @JDevlieghere 
wrote:

> Use in-class initializers

I'm all for in-class initializers, but I think it is confusing to use them 
piecemeal, where some ivars in a class have initializers and some are 
initialized in the constructor's definition.


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

https://reviews.llvm.org/D103349

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

[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

2021-06-01 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added inline comments.



Comment at: lldb/include/lldb/Interpreter/CommandReturnObject.h:169
+  /// as a temporary.
+  bool m_hermetic;
 };

JDevlieghere wrote:
> aprantl wrote:
> > bool m_hermetic = false;
> This is the second time someone has suggested to use in-class member 
> initializers. It's not what we've been doing in LLDB so far, but I am a fan 
> of it personally, so I'm going to interpret this as what we want to do moving 
> forward. I believe it's even a C++ core guideline. Maybe we can have a clang 
> tidy check to enforce this. 
This is idiomatic C++ and we should be using this feature going forward. It is 
more readable then a crowded `mem-init-list` and really makes a big difference 
when there are multiple constructors. 




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

https://reviews.llvm.org/D103349

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

[Lldb-commits] [PATCH] D103483: [lldb] Convert the default constructor’s member initializers into default member initializers

2021-06-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

FWIW this is more of an RFC and as always there's some manual fixing up to be 
done and formatting to be fixed. I wanted to put this up for review before 
sinking time in that.


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

https://reviews.llvm.org/D103483

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

[Lldb-commits] [PATCH] D103483: [lldb] Convert the default constructor’s member initializers into default member initializers

2021-06-01 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

+1. I'm anyway doing the same whenever I have to touching constructors, so we 
might as well pull out the big hammer.

I'm also glad you volunteer to resolve all the downstream conflicts :)




Comment at: lldb/include/lldb/Breakpoint/BreakpointOptions.h:47
 CommandData()
-: user_source(), script_source(),
-  interpreter(lldb::eScriptLanguageNone), stop_on_error(true) {}
+: user_source(), script_source() {}
 

If we anyway touch this, could we also delete these default member initializers?


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

https://reviews.llvm.org/D103483

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

[Lldb-commits] [PATCH] D103483: [lldb] Convert the default constructor’s member initializers into default member initializers

2021-06-01 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

Thanks for taking this on.  The in-class initialization is so much less 
boiler-plate and easier to read!


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

https://reviews.llvm.org/D103483

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

[Lldb-commits] [lldb] 00d19c6 - [various] Remove or use variables which are unused but set.

2021-06-01 Thread George Burgess IV via lldb-commits


Author: Michael Benfield
Date: 2021-06-01T15:38:48-07:00
New Revision: 00d19c6704f421157ae3de3623aca5f58f6c366d

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

LOG: [various] Remove or use variables which are unused but set.

This is in preparation for the -Wunused-but-set-variable warning.

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

Added: 


Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
llvm/lib/Target/ARM/ARMISelLowering.cpp
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
llvm/lib/Target/X86/X86FloatingPoint.cpp
llvm/utils/benchmark/src/complexity.cc

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 3398ec2595d67..75c26c42e18d1 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3101,7 +3101,6 @@ CommandInterpreter::ResolveCommandImpl(std::string 
&command_line,
   CommandObject *cmd_obj = nullptr;
   StreamString revised_command_line;
   bool wants_raw_input = false;
-  size_t actual_cmd_name_len = 0;
   std::string next_word;
   StringList matches;
   bool done = false;
@@ -3123,12 +3122,10 @@ CommandInterpreter::ResolveCommandImpl(std::string 
&command_line,
 revised_command_line.Printf("%s", alias_result.c_str());
 if (cmd_obj) {
   wants_raw_input = cmd_obj->WantsRawCommandString();
-  actual_cmd_name_len = cmd_obj->GetCommandName().size();
 }
   } else {
 if (cmd_obj) {
   llvm::StringRef cmd_name = cmd_obj->GetCommandName();
-  actual_cmd_name_len += cmd_name.size();
   revised_command_line.Printf("%s", cmd_name.str().c_str());
   wants_raw_input = cmd_obj->WantsRawCommandString();
 } else {
@@ -3143,7 +3140,6 @@ CommandInterpreter::ResolveCommandImpl(std::string 
&command_line,
   // The subcommand's name includes the parent command's name, so
   // restart rather than append to the revised_command_line.
   llvm::StringRef sub_cmd_name = sub_cmd_obj->GetCommandName();
-  actual_cmd_name_len = sub_cmd_name.size() + 1;
   revised_command_line.Clear();
   revised_command_line.Printf("%s", sub_cmd_name.str().c_str());
   cmd_obj = sub_cmd_obj;

diff  --git a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp 
b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
index cfecf35da1ed6..98a14b50cbf3b 100644
--- a/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
+++ b/lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
@@ -683,8 +683,6 @@ ValueObjectSP ABISysV_ppc::GetReturnValueObjectImpl(
   r3_value.GetData(r3_data);
   rdx_value.GetData(rdx_data);
 
-  uint32_t fp_bytes =
-  0; // Tracks how much of the xmm registers we've consumed so far
   uint32_t integer_bytes =
   0; // Tracks how much of the r3/rds registers we've consumed so far
 
@@ -751,7 +749,6 @@ ValueObjectSP ABISysV_ppc::GetReturnValueObjectImpl(
 break;
   } else if (*field_bit_width == 64) {
 copy_from_offset = 0;
-fp_bytes += field_byte_width;
   } else if (*field_bit_width == 32) {
 // This one is kind of complicated.  If we are in an "eightbyte"
 // with another float, we'll be stuffed into an xmm register with
@@ -815,8 +812,6 @@ ValueObjectSP ABISysV_ppc::GetReturnValueObjectImpl(
 copy_from_offset = integer_bytes - 8;
 integer_bytes += field_byte_width;
   }
-} else {
-  fp_bytes += field_byte_width;
 }
   }
 }

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
index a49655f56ed0d..98d0e9cf991b0 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
@@ -339,8 +339,6 @@ void AppleObjCRuntimeV1::UpdateISAToDescriptorMapIfNeeded() 
{
 if (!objc_module_sp)
   return;
 
-uint32_t isa_count = 0;
-
 lldb::addr_t hash_table_ptr = GetISAHashTablePointer();
 if (hash_table_ptr != LLDB_INVALID_ADDRESS) {
   // Read the NXHashTable struct:
@@ -383,8 +381,6 @@ void AppleObjCRuntimeV1::UpdateISAToDescriptorMapIfNeeded() 
{
   if (bucket_isa_count == 0)

[Lldb-commits] [PATCH] D102942: Remove or use variables which are unused but set.

2021-06-01 Thread George Burgess IV via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG00d19c6704f4: [various] Remove or use variables which are 
unused but set. (authored by mbenfield, committed by george.burgess.iv).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102942

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
  llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
  llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
  llvm/lib/Target/X86/X86FloatingPoint.cpp
  llvm/utils/benchmark/src/complexity.cc

Index: llvm/utils/benchmark/src/complexity.cc
===
--- llvm/utils/benchmark/src/complexity.cc
+++ llvm/utils/benchmark/src/complexity.cc
@@ -76,7 +76,6 @@
 LeastSq MinimalLeastSq(const std::vector& n,
const std::vector& time,
BigOFunc* fitting_curve) {
-  double sigma_gn = 0.0;
   double sigma_gn_squared = 0.0;
   double sigma_time = 0.0;
   double sigma_time_gn = 0.0;
@@ -84,7 +83,6 @@
   // Calculate least square fitting parameter
   for (size_t i = 0; i < n.size(); ++i) {
 double gn_i = fitting_curve(n[i]);
-sigma_gn += gn_i;
 sigma_gn_squared += gn_i * gn_i;
 sigma_time += time[i];
 sigma_time_gn += time[i] * gn_i;
Index: llvm/lib/Target/X86/X86FloatingPoint.cpp
===
--- llvm/lib/Target/X86/X86FloatingPoint.cpp
+++ llvm/lib/Target/X86/X86FloatingPoint.cpp
@@ -1526,7 +1526,7 @@
 
 // Scan the assembly for ST registers used, defined and clobbered. We can
 // only tell clobbers from defs by looking at the asm descriptor.
-unsigned STUses = 0, STDefs = 0, STClobbers = 0, STDeadDefs = 0;
+unsigned STUses = 0, STDefs = 0, STClobbers = 0;
 unsigned NumOps = 0;
 SmallSet FRegIdx;
 unsigned RCID;
@@ -1559,8 +1559,6 @@
   case InlineAsm::Kind_RegDef:
   case InlineAsm::Kind_RegDefEarlyClobber:
 STDefs |= (1u << STReg);
-if (MO.isDead())
-  STDeadDefs |= (1u << STReg);
 break;
   case InlineAsm::Kind_Clobber:
 STClobbers |= (1u << STReg);
Index: llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
===
--- llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
+++ llvm/lib/Target/PowerPC/PPCBranchSelector.cpp
@@ -415,5 +415,5 @@
   }
 
   BlockSizes.clear();
-  return true;
+  return EverMadeChange;
 }
Index: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
===
--- llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
@@ -1227,7 +1227,7 @@
   if (LoLoop.Preheader)
 LoLoop.Start = SearchForStart(LoLoop.Preheader);
   else
-return false;
+return Changed;
 
   // Find the low-overhead loop components and decide whether or not to fall
   // back to a normal loop. Also look for a vctp instructions and decide
@@ -1261,7 +1261,7 @@
   LLVM_DEBUG(LoLoop.dump());
   if (!LoLoop.FoundAllComponents()) {
 LLVM_DEBUG(dbgs() << "ARM Loops: Didn't find loop start, update, end\n");
-return false;
+return Changed;
   }
 
   assert(LoLoop.Start->getOpcode() != ARM::t2WhileLoopStart &&
Index: llvm/lib/Target/ARM/ARMISelLowering.cpp
===
--- llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -6861,12 +6861,10 @@
   return SDValue();
 // NEON has a 64-bit VMOV splat where each byte is either 0 or 0xff.
 uint64_t BitMask = 0xff;
-uint64_t Val = 0;
 unsigned ImmMask = 1;
 Imm = 0;
 for (int ByteNum = 0; ByteNum < 8; ++ByteNum) {
   if (((SplatBits | SplatUndef) & BitMask) == BitMask) {
-Val |= BitMask;
 Imm |= ImmMask;
   } else if ((SplatBits & BitMask) != 0) {
 return SDValue();
Index: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
===
--- llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
+++ llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp
@@ -180,7 +180,7 @@
   bool Changed = false;
   for (auto &BB : MF)
 Changed |= optimizeNZCVDefs(BB);
-  return true;
+  return Changed;
 }
 
 char AArch64PostSelectOptimize::ID = 0;
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/Appl

[Lldb-commits] [PATCH] D103375: [lldb/API] Expose triple for SBProcessInfo.

2021-06-01 Thread Greg Clayton via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103375

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

[Lldb-commits] [PATCH] D103271: [lldb/Target] Select most relevant frame only in case of signal

2021-06-01 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

One of the "frame recognizers" - probably the one that's causing this 
particular problem - is the "assert" recognizer.  The idea is that when you 
call "assert" in user code, you pretty much always end up with a 4 or 5 frames 
deep stack that wanders it's way from assert to __pthread_kill.  But you pretty 
much never care about any of those stack frames.  The stack frame you really 
want to see is the caller of the assert.  So the assert recognizer detects a 
thread in that situation and resets the selected frame to the assert caller.

So if you only applied recognizers on frames that stopped "with a reason", you 
would get into the situation where one thread stops because of a breakpoint, 
say, and gets a cooked presentation because it triggered a recognizer, but then 
you continue and another thread stops - say for another breakpoint.  While 
you're stopped if you go back to look at the first thread you might see that, 
even though it had not made any progress, it's presentation was different.  
That's because it didn't have a stop reason and so didn't trigger the 
recognizer, but to a user, that's just going to be confusing.

More generally, frame recognizers are about presenting stack frames in some way 
regardless of the reason why the thread stops.  I don't want users to have to 
guess when they do and don't apply.

So, first off, in situations which are highly sensitive to doing general work 
on "the stacks of all threads", you probably just want to turn frame 
recognizers off.  To do that, call `frame recognizer clear`.  Then none of this 
work will be done.

But there are two other things to try if you want to keep these recognizers on.

The first is to see if the recognizers can be run lazily when the user prints a 
thread backtrace.  If you stop in the debugger because of a breakpoint, say, 
and the user never does a "thread list" and only backtraces the stopped thread, 
we should not pay the cost of running the recognizers on the other threads.  It 
might take a little work to find/make a hook for "thread about to be presented 
to the user", but that would be a better place to put this computation.

The other thing is that the way the assert frame recognizer is currently 
written it will always unwind 5 frames looking for the signature of an assert 
-> __pthread_kill (or whatever it is on other systems...).  You could probably 
rewrite this to be more careful, and only continue unwinding if the first frame 
or two looked likely to lead to an assert.  You're going to have to do this 
somewhat carefully, however, since you don't want to miss actual asserts.


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

https://reviews.llvm.org/D103271

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

[Lldb-commits] [lldb] 658f6ed - Make ignore counts work as "after stop" modifiers so they play nicely with conditions

2021-06-01 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2021-06-01T18:22:27-07:00
New Revision: 658f6ed1523b0e61ddee494ce1691f29a701c317

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

LOG: Make ignore counts work as "after stop" modifiers so they play nicely with 
conditions

Previously ignore counts were checked when we stopped to do the sync callback 
in Breakpoint::ShouldStop. That meant we would do all the ignore count work 
even when
there is also a condition says the breakpoint should not stop.

That's wrong, lldb treats breakpoint hits that fail the thread or condition 
checks as "not having hit the breakpoint". So the ignore count check should 
happen after
the condition and thread checks in StopInfoBreakpoint::PerformAction.

The one side-effect of doing this is that if you have a breakpoint with a 
synchronous callback, it will run the synchronous callback before checking the 
ignore count.
That is probably a good thing, since this was already true of the condition and 
thread checks, so this removes an odd asymmetry. And breakpoints with sync 
callbacks
are all internal lldb breakpoints and there's not a really good reason why you 
would want one of these to use an ignore count (but not a condition or thread 
check...)

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

Added: 


Modified: 
lldb/include/lldb/Breakpoint/Breakpoint.h
lldb/include/lldb/Breakpoint/BreakpointLocation.h
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Breakpoint/BreakpointLocation.cpp
lldb/source/Target/StopInfo.cpp

lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/TestBreakpointIgnoreCount.py
lldb/test/API/functionalities/breakpoint/breakpoint_ignore_count/main.c

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/Breakpoint.h 
b/lldb/include/lldb/Breakpoint/Breakpoint.h
index ddb70b719d6ed..37710bb33118f 100644
--- a/lldb/include/lldb/Breakpoint/Breakpoint.h
+++ b/lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -617,14 +617,6 @@ class Breakpoint : public 
std::enable_shared_from_this,
 
   void DecrementIgnoreCount();
 
-  // BreakpointLocation::IgnoreCountShouldStop &
-  // Breakpoint::IgnoreCountShouldStop can only be called once per stop, and
-  // BreakpointLocation::IgnoreCountShouldStop should be tested first, and if
-  // it returns false we should continue, otherwise we should test
-  // Breakpoint::IgnoreCountShouldStop.
-
-  bool IgnoreCountShouldStop();
-
 private:
   // To call from CopyFromBreakpoint.
   Breakpoint(Target &new_target, const Breakpoint &bp_to_copy_from);

diff  --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h 
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index a12696be91e38..ca1739161c0cc 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -297,6 +297,10 @@ class BreakpointLocation
 
   void DecrementIgnoreCount();
 
+  /// BreakpointLocation::IgnoreCountShouldStop  can only be called once
+  /// per stop.  This method checks first against the loc and then the owner.
+  /// It also takes care of decrementing the ignore counters.
+  /// If it returns false we should continue, otherwise stop.
   bool IgnoreCountShouldStop();
 
 private:

diff  --git a/lldb/source/Breakpoint/Breakpoint.cpp 
b/lldb/source/Breakpoint/Breakpoint.cpp
index a09efe79c4781..45e23f65aea62 100644
--- a/lldb/source/Breakpoint/Breakpoint.cpp
+++ b/lldb/source/Breakpoint/Breakpoint.cpp
@@ -326,18 +326,6 @@ uint32_t Breakpoint::GetIgnoreCount() const {
   return m_options_up->GetIgnoreCount();
 }
 
-bool Breakpoint::IgnoreCountShouldStop() {
-  uint32_t ignore = GetIgnoreCount();
-  if (ignore != 0) {
-// When we get here we know the location that caused the stop doesn't have
-// an ignore count, since by contract we call it first...  So we don't have
-// to find & decrement it, we only have to decrement our own ignore count.
-DecrementIgnoreCount();
-return false;
-  } else
-return true;
-}
-
 uint32_t Breakpoint::GetHitCount() const { return m_hit_counter.GetValue(); }
 
 bool Breakpoint::IsOneShot() const { return m_options_up->IsOneShot(); }

diff  --git a/lldb/source/Breakpoint/BreakpointLocation.cpp 
b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 617b89bf19643..acdffa12369cf 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -357,15 +357,16 @@ void BreakpointLocation::DecrementIgnoreCount() {
 }
 
 bool BreakpointLocation::IgnoreCountShouldStop() {
-  if (m_options_up != nullptr) {
-uint32_t loc_ignore = m_options_up->GetIgnoreCount();
-if (loc_ignore != 0) {
-  m_owner.DecrementIgnoreCount();
-  DecrementIgnoreCount(); // Have to decrement our owners' ignore count,
-

[Lldb-commits] [PATCH] D103500: [trace][intel-pt] Create basic SB API

2021-06-01 Thread walter erquinigo via Phabricator via lldb-commits

wallace created this revision.
wallace added a reviewer: clayborg.
Herald added a subscriber: dang.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This adds a basic SB API for creating and stopping traces.
Note: This doesn't add any APIs for inspecting individual instructions. That'd 
be a more complicated change and it might be better to enhande the dump 
functionality to output the data in binary format. I'll leave that for a later 
diff.

This also enhances the existing tests so that they test the same flow using 
both the command interface and the SB API.

I also did some cleanup of legacy code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103500

Files:
  lldb/bindings/interface/SBStructuredData.i
  lldb/bindings/interface/SBTarget.i
  lldb/bindings/interface/SBTrace.i
  lldb/bindings/interface/SBTraceOptions.i
  lldb/docs/design/overview.rst
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/API/SBTarget.h
  lldb/include/lldb/API/SBThread.h
  lldb/include/lldb/API/SBTrace.h
  lldb/include/lldb/API/SBTraceOptions.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceOptions.h
  lldb/include/lldb/lldb-forward.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
  lldb/source/API/SBProcess.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBStructuredData.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/API/SBTrace.cpp
  lldb/source/API/SBTraceOptions.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/constants.h
  lldb/source/Target/Trace.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/TestTraceSchema.py
  lldb/test/API/commands/trace/TestTraceStartStop.py
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
  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
@@ -12,7 +12,6 @@
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/StructuredData.h"
-#include "lldb/Utility/TraceOptions.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Testing/Support/Error.h"
Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -1,53 +1,49 @@
 import lldb
+from intelpt_testcase import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
-ADDRESS_REGEX = '0x[0-9a-fA-F]*'
-
-class TestTraceStartStopMultipleThreads(TestBase):
+class TestTraceStartStopMultipleThreads(TraceIntelPTTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
-NO_DEBUG_INFO_TESTCASE = True
-
-def setUp(self):
-TestBase.setUp(self)
-if 'intel-pt' not in configuration.enabled_plugins:
-self.skipTest("The intel-pt test plugin is not enabled")
 
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+@testSBAPIAndCommands
 def testStartMultipleLiveThreads(self):
 self.build()
-target = self.createTestTarget()
+exe = self.getBuildArtifact("a.out")
+
+self.dbg.CreateTarget(exe)
 
 self.expect("b main")
 self.expect("b 6")
 self.expect("b 11")
 
 self.expect("r")
-self.expect("proce trace start")
-
+self.traceStartProcess()
 
-# We'll see here the first thread
 self.expect("continue")
 self.expect("thread trace dump instructions", substrs=['main.cpp:9'])
-
+
 # We'll see here the second thread
 self.expect("continue")
 self.expect("thread trace dump instructions", substrs=['main.cpp:4'])
 
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+@testSBAPIAndCommands
 def testStartMultipleLiveThreadsWithStops(self):
 self.build()
-tar

[Lldb-commits] [lldb] 251a5d9 - [lldb/API] Expose triple for SBProcessInfo.

2021-06-01 Thread Bruce Mitchener via lldb-commits


Author: Bruce Mitchener
Date: 2021-06-02T11:35:11+07:00
New Revision: 251a5d9d5239c0402e0ab68718aa194c2b4f04bb

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

LOG: [lldb/API] Expose triple for SBProcessInfo.

This is present when doing a `platform process list` and is
tracked by the underlying code. To do something like the
process list via the SB API in the future, this must be
exposed.

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

Added: 


Modified: 
lldb/bindings/interface/SBProcessInfo.i
lldb/include/lldb/API/SBProcessInfo.h
lldb/source/API/SBProcessInfo.cpp
lldb/test/API/python_api/process/TestProcessAPI.py

Removed: 




diff  --git a/lldb/bindings/interface/SBProcessInfo.i 
b/lldb/bindings/interface/SBProcessInfo.i
index 009842599bf84..17b2761a344e7 100644
--- a/lldb/bindings/interface/SBProcessInfo.i
+++ b/lldb/bindings/interface/SBProcessInfo.i
@@ -62,6 +62,12 @@ public:
 
 lldb::pid_t
 GetParentProcessID ();
+
+%feature("docstring",
+"Return the target triple (arch-vendor-os) for the described process."
+) GetTriple;
+const char *
+GetTriple ();
 };
 
 } // namespace lldb

diff  --git a/lldb/include/lldb/API/SBProcessInfo.h 
b/lldb/include/lldb/API/SBProcessInfo.h
index 0cc5f6a2f9f6c..36fae9e842a61 100644
--- a/lldb/include/lldb/API/SBProcessInfo.h
+++ b/lldb/include/lldb/API/SBProcessInfo.h
@@ -50,6 +50,9 @@ class LLDB_API SBProcessInfo {
 
   lldb::pid_t GetParentProcessID();
 
+  /// Return the target triple (arch-vendor-os) for the described process.
+  const char *GetTriple();
+
 private:
   friend class SBProcess;
 

diff  --git a/lldb/source/API/SBProcessInfo.cpp 
b/lldb/source/API/SBProcessInfo.cpp
index 29a9c7b24b5a7..cba3bdc179f30 100644
--- a/lldb/source/API/SBProcessInfo.cpp
+++ b/lldb/source/API/SBProcessInfo.cpp
@@ -179,6 +179,21 @@ lldb::pid_t SBProcessInfo::GetParentProcessID() {
   return proc_id;
 }
 
+const char *SBProcessInfo::GetTriple() {
+  LLDB_RECORD_METHOD_NO_ARGS(const char *, SBProcessInfo, GetTriple);
+
+  const char *triple = nullptr;
+  if (m_opaque_up) {
+const auto &arch = m_opaque_up->GetArchitecture();
+if (arch.IsValid()) {
+  // Const-ify the string so we don't need to worry about the lifetime of
+  // the string
+  triple = ConstString(arch.GetTriple().getTriple().c_str()).GetCString();
+}
+  }
+  return triple;
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -204,6 +219,7 @@ void RegisterMethods(Registry &R) {
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveUserIDIsValid, ());
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveGroupIDIsValid, ());
   LLDB_REGISTER_METHOD(lldb::pid_t, SBProcessInfo, GetParentProcessID, ());
+  LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetTriple, ());
 }
 
 }

diff  --git a/lldb/test/API/python_api/process/TestProcessAPI.py 
b/lldb/test/API/python_api/process/TestProcessAPI.py
index 942e2616c4095..b7efc7e4affa6 100644
--- a/lldb/test/API/python_api/process/TestProcessAPI.py
+++ b/lldb/test/API/python_api/process/TestProcessAPI.py
@@ -356,6 +356,8 @@ def test_get_process_info(self):
 self.assertNotEqual(
 process_info.GetProcessID(), lldb.LLDB_INVALID_PROCESS_ID,
 "Process ID is valid")
+triple = process_info.GetTriple()
+self.assertIsNotNone(triple, "Process has a triple")
 
 # Additional process info varies by platform, so just check that
 # whatever info was retrieved is consistent and nothing blows up.



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

[Lldb-commits] [PATCH] D103375: [lldb/API] Expose triple for SBProcessInfo.

2021-06-01 Thread Bruce Mitchener 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 rG251a5d9d5239: [lldb/API] Expose triple for SBProcessInfo. 
(authored by brucem).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103375

Files:
  lldb/bindings/interface/SBProcessInfo.i
  lldb/include/lldb/API/SBProcessInfo.h
  lldb/source/API/SBProcessInfo.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py


Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -356,6 +356,8 @@
 self.assertNotEqual(
 process_info.GetProcessID(), lldb.LLDB_INVALID_PROCESS_ID,
 "Process ID is valid")
+triple = process_info.GetTriple()
+self.assertIsNotNone(triple, "Process has a triple")
 
 # Additional process info varies by platform, so just check that
 # whatever info was retrieved is consistent and nothing blows up.
Index: lldb/source/API/SBProcessInfo.cpp
===
--- lldb/source/API/SBProcessInfo.cpp
+++ lldb/source/API/SBProcessInfo.cpp
@@ -179,6 +179,21 @@
   return proc_id;
 }
 
+const char *SBProcessInfo::GetTriple() {
+  LLDB_RECORD_METHOD_NO_ARGS(const char *, SBProcessInfo, GetTriple);
+
+  const char *triple = nullptr;
+  if (m_opaque_up) {
+const auto &arch = m_opaque_up->GetArchitecture();
+if (arch.IsValid()) {
+  // Const-ify the string so we don't need to worry about the lifetime of
+  // the string
+  triple = ConstString(arch.GetTriple().getTriple().c_str()).GetCString();
+}
+  }
+  return triple;
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -204,6 +219,7 @@
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveUserIDIsValid, ());
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveGroupIDIsValid, ());
   LLDB_REGISTER_METHOD(lldb::pid_t, SBProcessInfo, GetParentProcessID, ());
+  LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetTriple, ());
 }
 
 }
Index: lldb/include/lldb/API/SBProcessInfo.h
===
--- lldb/include/lldb/API/SBProcessInfo.h
+++ lldb/include/lldb/API/SBProcessInfo.h
@@ -50,6 +50,9 @@
 
   lldb::pid_t GetParentProcessID();
 
+  /// Return the target triple (arch-vendor-os) for the described process.
+  const char *GetTriple();
+
 private:
   friend class SBProcess;
 
Index: lldb/bindings/interface/SBProcessInfo.i
===
--- lldb/bindings/interface/SBProcessInfo.i
+++ lldb/bindings/interface/SBProcessInfo.i
@@ -62,6 +62,12 @@
 
 lldb::pid_t
 GetParentProcessID ();
+
+%feature("docstring",
+"Return the target triple (arch-vendor-os) for the described process."
+) GetTriple;
+const char *
+GetTriple ();
 };
 
 } // namespace lldb


Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -356,6 +356,8 @@
 self.assertNotEqual(
 process_info.GetProcessID(), lldb.LLDB_INVALID_PROCESS_ID,
 "Process ID is valid")
+triple = process_info.GetTriple()
+self.assertIsNotNone(triple, "Process has a triple")
 
 # Additional process info varies by platform, so just check that
 # whatever info was retrieved is consistent and nothing blows up.
Index: lldb/source/API/SBProcessInfo.cpp
===
--- lldb/source/API/SBProcessInfo.cpp
+++ lldb/source/API/SBProcessInfo.cpp
@@ -179,6 +179,21 @@
   return proc_id;
 }
 
+const char *SBProcessInfo::GetTriple() {
+  LLDB_RECORD_METHOD_NO_ARGS(const char *, SBProcessInfo, GetTriple);
+
+  const char *triple = nullptr;
+  if (m_opaque_up) {
+const auto &arch = m_opaque_up->GetArchitecture();
+if (arch.IsValid()) {
+  // Const-ify the string so we don't need to worry about the lifetime of
+  // the string
+  triple = ConstString(arch.GetTriple().getTriple().c_str()).GetCString();
+}
+  }
+  return triple;
+}
+
 namespace lldb_private {
 namespace repro {
 
@@ -204,6 +219,7 @@
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveUserIDIsValid, ());
   LLDB_REGISTER_METHOD(bool, SBProcessInfo, EffectiveGroupIDIsValid, ());
   LLDB_REGISTER_METHOD(lldb::pid_t, SBProcessInfo, GetParentProcessID, ());
+  LLDB_REGISTER_METHOD(const char *, SBProcessInfo, GetTriple, ());
 }
 
 }
Index: lldb/include/lldb/API/SBProcessInfo.h
===
--- lldb/include/lldb/API/SBProcessInfo.

[Lldb-commits] [PATCH] D103504: Improve performance when parsing symbol tables in mach-o files.

2021-06-01 Thread Greg Clayton via Phabricator via lldb-commits

clayborg created this revision.
clayborg added reviewers: labath, aprantl, JDevlieghere, jingham.
Herald added a subscriber: kristof.beyls.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Some larger projects were loading quite slowly with the current LLDB on macOS 
and macOS simulator builds. I did some instrument traces and found 3 main 
culprits:

- a LLDB timer that was put into a function that was called too often
- a std::set that was keeping track of the address of symbols that were already 
added
- a unnamed function generator in ObjectFile that was going slow due to 
allocations

In order to see this in action I ran the latest LLDB on a large application 
with many frameworks using the following method:

(lldb) script import time; start_time = time.perf_counter()
(lldb) file Large.app
(lldb) script print(time.perf_counter() - start_time)

I first range "sudo purge" to clear the system file caches to simulate a cold 
startup of the debugger, followed by two iterations with warm file caches.

Prior to this fix I was seeing the following timings:

17.68 (cold)
14.56 (warm 1)
14.52 (warm 2)

After this fix I was seeing:

11.32 (cold)
8.43 (warm 1)
8.49 (warm 2)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103504

Files:
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Symbol/ObjectFile.cpp

Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -617,11 +617,12 @@
 }
 
 ConstString ObjectFile::GetNextSyntheticSymbolName() {
-  StreamString ss;
+  llvm::SmallString<256> name;
+  llvm::raw_svector_ostream os(name);
   ConstString file_name = GetModule()->GetFileSpec().GetFilename();
-  ss.Printf("___lldb_unnamed_symbol%u$$%s", ++m_synthetic_symbol_idx,
-file_name.GetCString());
-  return ConstString(ss.GetString());
+  os << "___lldb_unnamed_symbol" << ++m_synthetic_symbol_idx << "$$"
+ << file_name.GetStringRef();
+  return ConstString(os.str());
 }
 
 std::vector
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -44,6 +44,7 @@
 
 #include "lldb/Host/SafeMachO.h"
 
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MemoryBuffer.h"
 
@@ -1895,9 +1896,9 @@
 filename = first_section_sp->GetObjectFile()->GetFileSpec().GetPath();
 
   Host::SystemLog(Host::eSystemLogError,
-  "error: unable to find section %d for a symbol in %s, corrupt file?\n",
-  n_sect, 
-  filename.c_str());
+  "error: unable to find section %d for a symbol in "
+  "%s, corrupt file?\n",
+  n_sect, filename.c_str());
 }
   }
   if (m_section_infos[n_sect].vm_range.Contains(file_addr)) {
@@ -2183,7 +2184,16 @@
   // We add symbols to the table in the order of most information (nlist
   // records) to least (function starts), and avoid duplicating symbols
   // via this set.
-  std::set symbols_added;
+  llvm::DenseSet symbols_added;
+
+  // We are using a llvm::DenseSet for "symbols_added" so we must be sure we
+  // do not add the tombstone or empty keys to the set.
+  auto add_symbol_addr = [&symbols_added](lldb::addr_t file_addr) {
+// Don't add the tombstone or empty keys.
+if (file_addr == UINT64_MAX || file_addr == UINT64_MAX - 1)
+  return;
+symbols_added.insert(file_addr);
+  };
   FunctionStarts function_starts;
   lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
   uint32_t i;
@@ -3640,9 +3650,9 @@
 symbol_section);
 sym[GSYM_sym_idx].GetAddressRef().SetOffset(
 symbol_value);
-symbols_added.insert(sym[GSYM_sym_idx]
- .GetAddress()
- .GetFileAddress());
+add_symbol_addr(sym[GSYM_sym_idx]
+.GetAddress()
+.GetFileAddress());
 // We just need the flags from the linker
 // symbol, so put these flags
 // into the N_GSYM flags to avoid duplicate
@@ -3662,7 +3672,7 @@
   if (set_value) {
 sym[sym_idx].GetAddressRef().SetSection(symbol_section);
 sym

38 matches

Mail list logo