[Lldb-commits] [PATCH] D124760: [lldb] Fix ppc64 detection in lldb

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Sure, that's great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124760

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

[Lldb-commits] [lldb] fa593b0 - Revert "[lldb] parallelize calling of Module::PreloadSymbols()"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-05-09T11:11:01+02:00
New Revision: fa593b079b76f1c30d684cfe42a662fed157e2e5

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

LOG: Revert "[lldb] parallelize calling of Module::PreloadSymbols()"

This reverts commit b7d807dbcff0d9df466e0312b4fef57178d207be -- it
breaks TestMultipleDebuggers.py.

Added: 


Modified: 
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index d6ad333957e11..ed733f2645c3a 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -62,7 +62,6 @@
 
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SetVector.h"
-#include "llvm/Support/ThreadPool.h"
 
 #include 
 #include 
@@ -1624,17 +1623,6 @@ void 
Target::NotifyModulesRemoved(lldb_private::ModuleList &module_list) {
 void Target::ModulesDidLoad(ModuleList &module_list) {
   const size_t num_images = module_list.GetSize();
   if (m_valid && num_images) {
-if (GetPreloadSymbols()) {
-  // Try to preload symbols in parallel.
-  llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
-  auto preload_symbols_fn = [&](size_t idx) {
-ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
-module_sp->PreloadSymbols();
-  };
-  for (size_t idx = 0; idx < num_images; ++idx)
-task_group.async(preload_symbols_fn, idx);
-  task_group.wait();
-}
 for (size_t idx = 0; idx < num_images; ++idx) {
   ModuleSP module_sp(module_list.GetModuleAtIndex(idx));
   LoadScriptingResourceForModule(module_sp, this);
@@ -2182,6 +2170,11 @@ ModuleSP Target::GetOrCreateModule(const ModuleSpec 
&module_spec, bool notify,
   });
 }
 
+// Preload symbols outside of any lock, so hopefully we can do this for
+// each library in parallel.
+if (GetPreloadSymbols())
+  module_sp->PreloadSymbols();
+
 llvm::SmallVector replaced_modules;
 for (ModuleSP &old_module_sp : old_modules) {
   if (m_images.GetIndexForModule(old_module_sp.get()) !=



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

[Lldb-commits] [PATCH] D122975: parallelize calling of Module::PreloadSymbols()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm afraid I had to revert this, as it was causing hangs in 
TestMultipleDebuggers.py.

I haven't fully debugged this, but what I think is happening is this:

- the test debug multiple (identical) inferiors in parallel
- as a result the thread pool gets hit with many preload-symbols tasks for the 
same set of modules. As these tasks are taking the respective module mutexes, 
they cannot all make progress and a large part of them gets stuck.
- those that to manage to make progress (take the module mutex) get to the 
dwarf indexing stage. while waiting for the indexing tasks to complete, they 
start to perform some other tasks
- if one of those tasks is another preload-symbols task the can get stuck 
acquiring the module mutex. However, they will still be holding the mutex for 
the module that they are indexing, preventing the other threads from making 
progress
- if this happens to multiple threads, they can form a loop in the module mutex 
waits, thereby hanging forever

I'm not entirely sure what would be the right way to fix this, but it seems 
that we either need to make sure the threads don't hold the mutexes while 
performing other tasks (not sure if that's possible), or rethink the 
do-tasks-while-you-wait idea.

It should be fairly easy to reproduce this by running TestMultipleDebuggers a 
couple of times, but if you run into problems doing that, I can provide you 
with some backtraces, logs, or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122975

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

[Lldb-commits] [PATCH] D125154: [lldb] Specify aguments of `image list`

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Can you add a test for the help output to 
`lldb/test/API/commands/help/TestHelp.py` ? Just the one line that includes the 
argument list is fine.

Also I see that we don't test this part of the command at all. Not an issue for 
this change (I assume it is working for you right now otherwise you wouldn't be 
doing this) but coverage would be welcome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125154

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

[Lldb-commits] [PATCH] D122975: parallelize calling of Module::PreloadSymbols()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D122975#3500176 , @labath wrote:

> or rethink the do-tasks-while-you-wait idea.

I suppose one way to fix this would be to ensure that the waiting thread only 
picks up its own subtasks while its waiting. That would avoid these deadlocks, 
and also ensure that the wait is not unnecessarily delayed by a completely 
unrelated long-running task.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122975

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

[Lldb-commits] [PATCH] D124370: [lldb/DWARF] Fix linking direction in CopyUniqueClassMethodTypes

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGae7fe65cf65d: [lldb/DWARF] Fix linking direction in 
CopyUniqueClassMethodTypes (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124370

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/incomplete-types/members/Makefile
  
lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
  lldb/test/API/lang/cpp/incomplete-types/members/a.h
  lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
  lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
  lldb/test/API/lang/cpp/incomplete-types/members/main.cpp

Index: lldb/test/API/lang/cpp/incomplete-types/members/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/main.cpp
@@ -0,0 +1,9 @@
+#include "a.h"
+
+A::A() = default;
+void A::anchor() {}
+
+int main() {
+  A().f();
+  A().g();
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
@@ -0,0 +1,8 @@
+#include "a.h"
+
+int A::g() {
+  struct Ag {
+int a, b;
+  } ag{47, 42};
+  return ag.a + ag.b; // break here
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
@@ -0,0 +1,8 @@
+#include "a.h"
+
+int A::f() {
+  struct Af {
+int x, y;
+  } af{42, 47};
+  return af.x + af.y; // break here
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/a.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/a.h
@@ -0,0 +1,14 @@
+#ifndef A_H
+#define A_H
+
+class A {
+public:
+  A();
+  virtual void anchor();
+  int f();
+  int g();
+
+  int member = 47;
+};
+
+#endif
Index: lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
@@ -0,0 +1,32 @@
+"""
+Test situations where we don't have a definition for a type, but we have (some)
+of its member functions.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCppIncompleteTypeMembers(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("f.cpp"))
+
+# Sanity check that we really have to debug info for this type.
+this = self.expect_var_path("this", type="A *")
+self.assertEquals(this.GetType().GetPointeeType().GetNumberOfFields(),
+0, str(this))
+
+self.expect_var_path("af.x", value='42')
+
+lldbutil.run_break_set_by_source_regexp(self, "// break here",
+extra_options="-f g.cpp")
+self.runCmd("continue")
+
+self.expect_var_path("ag.a", value='47')
Index: lldb/test/API/lang/cpp/incomplete-types/members/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-types/members/Makefile
@@ -0,0 +1,10 @@
+CXX_SOURCES := main.cpp f.cpp g.cpp
+
+include Makefile.rules
+
+# Force main.cpp to be built with no debug information
+main.o: CFLAGS = $(CFLAGS_NO_DEBUG)
+
+# And force -flimit-debug-info on the rest.
+f.o: CFLAGS_EXTRAS += $(LIMIT_DEBUG_INFO_FLAGS)
+g.o: CFLAGS_EXTRAS += $(LIMIT_DEBUG_INFO_FLAGS)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1040,10 +1040,8 @@
 // struct and see if this is actually a C++ method
 Type *class_type = dwarf->ResolveType(decl_ctx_die);
 if (class_type) {
-  bool alternate_defn = false;
   if (class_type->GetID() != decl_ctx_die.GetID() ||
   IsClangModuleFwdDecl(decl_ctx_die)) {
-alternate_defn = true;
 
 // We uniqued the parent class of this function to another
 // class so we now need to associate all dies under
@@ -1112,7 +1110,7 @@
 CompilerType class_opaque_type =
 class_type->GetForwardCompilerType();
 if (TypeSystemClang::IsCXXClassType(class_opaque_type)) {
-  if (class_opaque_type.IsBeingDefined() || alternate_defn) {
+  if (class_opaque_type.IsB

[Lldb-commits] [lldb] ae7fe65 - [lldb/DWARF] Fix linking direction in CopyUniqueClassMethodTypes

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-05-09T11:47:55+02:00
New Revision: ae7fe65cf65dd4f71e117dee868965c152d27542

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

LOG: [lldb/DWARF] Fix linking direction in CopyUniqueClassMethodTypes

IIUC, the purpose of CopyUniqueClassMethodTypes is to link together
class definitions in two compile units so that we only have a single
definition of a class. It does this by adding entries to the die_to_type
and die_to_decl_ctx maps.

However, the direction of the linking seems to be reversed. It is taking
entries from the class that has not yet been parsed, and copying them to
the class which has been parsed already -- i.e., it is a very
complicated no-op.

Changing the linking order allows us to revert the changes in D13224
(while keeping the associated test case passing), and is sufficient to
fix PR54761, which was caused by an undesired interaction with that
patch.

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

Added: 
lldb/test/API/lang/cpp/incomplete-types/members/Makefile

lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
lldb/test/API/lang/cpp/incomplete-types/members/a.h
lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
lldb/test/API/lang/cpp/incomplete-types/members/main.cpp

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8662578336bd3..4ec5013b135a0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1040,10 +1040,8 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const 
DWARFDIE &die,
 // struct and see if this is actually a C++ method
 Type *class_type = dwarf->ResolveType(decl_ctx_die);
 if (class_type) {
-  bool alternate_defn = false;
   if (class_type->GetID() != decl_ctx_die.GetID() ||
   IsClangModuleFwdDecl(decl_ctx_die)) {
-alternate_defn = true;
 
 // We uniqued the parent class of this function to another
 // class so we now need to associate all dies under
@@ -1112,7 +1110,7 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const 
DWARFDIE &die,
 CompilerType class_opaque_type =
 class_type->GetForwardCompilerType();
 if (TypeSystemClang::IsCXXClassType(class_opaque_type)) {
-  if (class_opaque_type.IsBeingDefined() || alternate_defn) {
+  if (class_opaque_type.IsBeingDefined()) {
 if (!is_static && !die.HasChildren()) {
   // We have a C++ member function with no children (this
   // pointer!) and clang will get mad if we try and make
@@ -1120,84 +1118,50 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const 
DWARFDIE &die,
   // we will just skip it...
   type_handled = true;
 } else {
-  bool add_method = true;
-  if (alternate_defn) {
-// If an alternate definition for the class exists,
-// then add the method only if an equivalent is not
-// already present.
-clang::CXXRecordDecl *record_decl =
-m_ast.GetAsCXXRecordDecl(
-class_opaque_type.GetOpaqueQualType());
-if (record_decl) {
-  for (auto method_iter = record_decl->method_begin();
-   method_iter != record_decl->method_end();
-   method_iter++) {
-clang::CXXMethodDecl *method_decl = *method_iter;
-if (method_decl->getNameInfo().getAsString() ==
-attrs.name.GetStringRef()) {
-  if (method_decl->getType() ==
-  ClangUtil::GetQualType(clang_type)) {
-add_method = false;
-LinkDeclContextToDIE(method_decl, die);
-type_handled = true;
-
-break;
-  }
-}
-  }
-}
-  }
-
-  if (add_method) {
-llvm::PrettyStackTraceFormat stack_trace(
-"SymbolFileDWARF::ParseType() is adding a method "
-"%s to class %s in DIE 0x%8.8" PRIx64 " from %s",
-attrs.name.GetCString(),

[Lldb-commits] [lldb] fc440f2 - Filter non-external static members from SBType::GetFieldAtIndex.

[Pavel Labath via lldb-commits]

Author: Sigurur sgeirsson
Date: 2022-05-09T12:34:13+02:00
New Revision: fc440f27cd50e48e1f9ebe6e56febe2823e59de4

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

LOG: Filter non-external static members from SBType::GetFieldAtIndex.

See [[ https://github.com/llvm/llvm-project/issues/55040 | issue 55040 ]] where 
static members of classes declared in the anonymous namespace are incorrectly 
returned as member fields from lldb::SBType::GetFieldAtIndex(). It appears that 
attrs.member_byte_offset contains a sentinel value for members that don't have 
a DW_AT_data_member_location.

Reviewed By: labath

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

Added: 

lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4ec5013b135a0..f7164d0f370d9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2408,8 +2408,6 @@ struct MemberAttributes {
   /// structure.
   uint32_t member_byte_offset;
   bool is_artificial = false;
-  /// On DW_TAG_members, this means the member is static.
-  bool is_external = false;
 };
 
 /// Parsed form of all attributes that are relevant for parsing Objective-C
@@ -2485,9 +2483,6 @@ MemberAttributes::MemberAttributes(const DWARFDIE &die,
   case DW_AT_artificial:
 is_artificial = form_value.Boolean();
 break;
-  case DW_AT_external:
-is_external = form_value.Boolean();
-break;
   default:
 break;
   }
@@ -2632,8 +2627,10 @@ void DWARFASTParserClang::ParseSingleMember(
   if (class_is_objc_object_or_interface)
 attrs.accessibility = eAccessNone;
 
-  // Handle static members
-  if (attrs.is_external && attrs.member_byte_offset == UINT32_MAX) {
+  // Handle static members, which is any member that doesn't have a bit or a
+  // byte member offset.
+  if (attrs.member_byte_offset == UINT32_MAX &&
+  attrs.data_bit_offset == UINT64_MAX) {
 Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
 
 if (var_type) {

diff  --git 
a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s
 
b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s
new file mode 100644
index 0..bbe36ebfec5a8
--- /dev/null
+++ 
b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s
@@ -0,0 +1,173 @@
+# RUN: llvm-mc --triple=x86_64-pc-linux --filetype=obj %s -o %t
+# RUN: %lldb -o "target variable ug U::s" -b %t | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = (m = 14159265)
+# CHECK: (int) U::s = 65295141
+
+# This tests that a static member in a class declared in the anonymous 
namespace
+# does not appear as a field of the class. There is a 
diff erence between the
+# debug info generated by gcc and clang, where clang flags the static member
+# with DW_AT_external, but gcc does not.
+#
+# Roughly corresponds to this source code:
+#
+# namespace {
+# struct U {
+#   static int s;
+#   int m = 14159265;
+# };
+# int U::s = 65295141;
+# }
+#
+# U ug;
+
+.file   "test.cpp"
+.data
+.quad 0
+ug:
+.long 14159265
+.Lug_s:
+.long 65295141
+
+.section.debug_info,"",@progbits
+.Ldebug_info0:
+.long   .Lcu_end-.Lcu_begin
+.Lcu_begin:
+.value  0x4
+.long   .Ldebug_abbrev0
+.byte   0x8
+.uleb128 0x1
+.asciz  "GCC DWARF reduced by hand"
+.byte   0x4
+.asciz  "test.cpp"
+.uleb128 0x2
+.LU:
+.uleb128 0x3
+.string "U"
+.byte   0x4
+.LU_s:
+.uleb128 0x4
+.string "s"
+.long   .Lint-.Ldebug_info0
+.uleb128 0x5
+.string "m"
+.long   .Lint-.Ldebug_info0
+.byte   0
+.byte   0
+.byte   0
+.uleb128 0x6
+.long   0x2d
+.Lint:
+.uleb128 0x7
+.byte   0x4
+.byte   0x5
+.string "int"
+.uleb128 0x9
+.string "ug"
+.long   .LU-.Ldebug_info0
+.uleb128 0x9
+.byte   0x3
+.quad   ug
+.uleb128 0xa
+.long   .LU_s-.Ldebug_info0
+.uleb128 0x9
+.byte   0x3
+.quad .Lug_s
+.byte   0
+.Lcu_end:
+.section.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+.uleb128 0x1
+.uleb128 0x11
+.byte   0x1
+.uleb128 0x25
+.uleb128 0x8
+.uleb128 0x13
+.uleb128 0xb
+.uleb

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc440f27cd50: Filter non-external static members from 
SBType::GetFieldAtIndex. (authored by Sigurur sgeirsson 
, committed by labath).

Changed prior to commit:
  https://reviews.llvm.org/D124409?vs=426452&id=428023#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124409

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s
@@ -0,0 +1,173 @@
+# RUN: llvm-mc --triple=x86_64-pc-linux --filetype=obj %s -o %t
+# RUN: %lldb -o "target variable ug U::s" -b %t | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = (m = 14159265)
+# CHECK: (int) U::s = 65295141
+
+# This tests that a static member in a class declared in the anonymous namespace
+# does not appear as a field of the class. There is a difference between the
+# debug info generated by gcc and clang, where clang flags the static member
+# with DW_AT_external, but gcc does not.
+#
+# Roughly corresponds to this source code:
+#
+# namespace {
+# struct U {
+#   static int s;
+#   int m = 14159265;
+# };
+# int U::s = 65295141;
+# }
+#
+# U ug;
+
+.file   "test.cpp"
+.data
+.quad 0
+ug:
+.long 14159265
+.Lug_s:
+.long 65295141
+
+.section.debug_info,"",@progbits
+.Ldebug_info0:
+.long   .Lcu_end-.Lcu_begin
+.Lcu_begin:
+.value  0x4
+.long   .Ldebug_abbrev0
+.byte   0x8
+.uleb128 0x1
+.asciz  "GCC DWARF reduced by hand"
+.byte   0x4
+.asciz  "test.cpp"
+.uleb128 0x2
+.LU:
+.uleb128 0x3
+.string "U"
+.byte   0x4
+.LU_s:
+.uleb128 0x4
+.string "s"
+.long   .Lint-.Ldebug_info0
+.uleb128 0x5
+.string "m"
+.long   .Lint-.Ldebug_info0
+.byte   0
+.byte   0
+.byte   0
+.uleb128 0x6
+.long   0x2d
+.Lint:
+.uleb128 0x7
+.byte   0x4
+.byte   0x5
+.string "int"
+.uleb128 0x9
+.string "ug"
+.long   .LU-.Ldebug_info0
+.uleb128 0x9
+.byte   0x3
+.quad   ug
+.uleb128 0xa
+.long   .LU_s-.Ldebug_info0
+.uleb128 0x9
+.byte   0x3
+.quad .Lug_s
+.byte   0
+.Lcu_end:
+.section.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+.uleb128 0x1
+.uleb128 0x11
+.byte   0x1
+.uleb128 0x25
+.uleb128 0x8
+.uleb128 0x13
+.uleb128 0xb
+.uleb128 0x3
+.uleb128 0x8
+.byte   0
+.byte   0
+.uleb128 0x2
+.uleb128 0x39
+.byte   0x1
+.byte   0
+.byte   0
+.uleb128 0x3
+.uleb128 0x13
+.byte   0x1
+.uleb128 0x3
+.uleb128 0x8
+.uleb128 0xb
+.uleb128 0xb
+.byte   0
+.byte   0
+.uleb128 0x4
+.uleb128 0xd
+.byte   0
+.uleb128 0x3
+.uleb128 0x8
+.uleb128 0x49
+.uleb128 0x13
+.uleb128 0x3c
+.uleb128 0x19
+.byte   0
+.byte   0
+.uleb128 0x5
+.uleb128 0xd
+.byte   0
+.uleb128 0x3
+.uleb128 0x8
+.uleb128 0x49
+.uleb128 0x13
+.uleb128 0x38
+.uleb128 0xb
+.byte   0
+.byte   0
+.uleb128 0x6
+.uleb128 0x3a
+.byte   0
+.uleb128 0x18
+.uleb128 0x13
+.byte   0
+.byte   0
+.uleb128 0x7
+.uleb128 0x24
+.byte   0
+.uleb128 0xb
+.uleb128 0xb
+.uleb128 0x3e
+.uleb128 0xb
+.uleb128 0x3
+.uleb128 0x8
+.byte   0
+.byte   0
+.uleb128 0x8
+.uleb128 0x26
+.byte   0
+.uleb128 0x49
+.uleb128 0x13
+.byte   0
+.byte   0
+.uleb128 0x9
+.uleb128 0x34
+.byte   0
+.uleb128 0x3
+.uleb128 0x8
+.uleb128 0x49
+.uleb128 0x13
+.uleb128 0x2
+.uleb128 0x18
+.byte   0
+.byte   0
+	.uleb128 0xa
+	.uleb128 0x34
+	.byte	0
+	.uleb128 0x47
+	.uleb128 0x13
+	.uleb128 0x2
+	.uleb128 0x18
+	.byte	0
+	.byte	0
+.byte   0
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFA

[Lldb-commits] [PATCH] D125218: [lldb][NFC] Make cmd a reference in GenerateOptionUsage

[David Spickett via Phabricator via lldb-commits]

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

Nowhere in lldb do we call this with a null pointer.
If we did, the first line of the function would fault anyway.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125218

Files:
  lldb/include/lldb/Interpreter/Options.h
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/Options.cpp

Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -388,21 +388,15 @@
   return true;
 }
 
-void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
+void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd,
   uint32_t screen_width) {
-  const bool only_print_args = cmd->IsDashDashCommand();
+  const bool only_print_args = cmd.IsDashDashCommand();
 
   auto opt_defs = GetDefinitions();
   const uint32_t save_indent_level = strm.GetIndentLevel();
-  llvm::StringRef name;
-
+  llvm::StringRef name = cmd.GetCommandName();
   StreamString arguments_str;
-
-  if (cmd) {
-name = cmd->GetCommandName();
-cmd->GetFormattedCommandArguments(arguments_str);
-  } else
-name = "";
+  cmd.GetFormattedCommandArguments(arguments_str);
 
   const uint32_t num_options = NumCommandOptions();
   if (num_options == 0)
@@ -432,8 +426,7 @@
 
   // Different option sets may require different args.
   StreamString args_str;
-  if (cmd)
-cmd->GetFormattedCommandArguments(args_str, opt_set_mask);
+  cmd.GetFormattedCommandArguments(args_str, opt_set_mask);
 
   // First go through and print all options that take no arguments as a
   // single string. If a command has "-a" "-b" and "-c", this will show up
@@ -482,7 +475,7 @@
   }
 
   if (args_str.GetSize() > 0) {
-if (cmd->WantsRawCommandString() && !only_print_args)
+if (cmd.WantsRawCommandString() && !only_print_args)
   strm.Printf(" --");
 
 strm << " " << args_str.GetString();
@@ -492,7 +485,7 @@
 }
   }
 
-  if (cmd && (only_print_args || cmd->WantsRawCommandString()) &&
+  if ((only_print_args || cmd.WantsRawCommandString()) &&
   arguments_str.GetSize() > 0) {
 if (!only_print_args)
   strm.PutChar('\n');
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -132,7 +132,7 @@
   } else {
 // No error string, output the usage information into result
 options->GenerateOptionUsage(
-result.GetErrorStream(), this,
+result.GetErrorStream(), *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   }
 }
@@ -326,7 +326,7 @@
   if (!found_word && search_options && GetOptions() != nullptr) {
 StreamString usage_help;
 GetOptions()->GenerateOptionUsage(
-usage_help, this,
+usage_help, *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
 if (!usage_help.Empty()) {
   llvm::StringRef usage_text = usage_help.GetString();
@@ -863,7 +863,7 @@
   Options *options = GetOptions();
   if (options != nullptr) {
 options->GenerateOptionUsage(
-output_strm, this,
+output_strm, *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   }
   llvm::StringRef long_help = GetHelpLong();
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -3796,7 +3796,7 @@
 
 default:
   m_options.GenerateOptionUsage(
-  result.GetErrorStream(), this,
+  result.GetErrorStream(), *this,
   GetCommandInterpreter().GetDebugger().GetTerminalWidth());
   syntax_error = true;
   break;
Index: lldb/source/Commands/CommandObjectFrame.cpp
===
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -348,7 +348,7 @@
 "too many arguments; expected frame-index, saw '%s'.\n",
 command[0].c_str());
 m_options.GenerateOptionUsage(
-result.GetErrorStream(), this,
+result.GetErrorStream(), *this,
 GetCommandInterpreter().GetDebugger().GetTerminalWidth());
 return false;
   }
Index: lldb/source/Commands/CommandObjectDisassemble.cpp
=

[Lldb-commits] [PATCH] D125219: [lldb][NFC] Simplify GenerateOptionUsage

[David Spickett via Phabricator via lldb-commits]

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

Once we get into the if block we know the value of only_print_args.
Move some variables closer to point of use.

Depends on D125218 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125219

Files:
  lldb/source/Interpreter/Options.cpp


Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -390,8 +390,6 @@
 
 void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd,
   uint32_t screen_width) {
-  const bool only_print_args = cmd.IsDashDashCommand();
-
   auto opt_defs = GetDefinitions();
   const uint32_t save_indent_level = strm.GetIndentLevel();
   llvm::StringRef name = cmd.GetCommandName();
@@ -402,6 +400,7 @@
   if (num_options == 0)
 return;
 
+  const bool only_print_args = cmd.IsDashDashCommand();
   if (!only_print_args)
 strm.PutCString("\nCommand Options Usage:\n");
 
@@ -413,19 +412,16 @@
   //   [options-for-level-1]
   //   etc.
 
-  uint32_t num_option_sets = GetRequiredOptions().size();
-
   if (!only_print_args) {
+uint32_t num_option_sets = GetRequiredOptions().size();
 for (uint32_t opt_set = 0; opt_set < num_option_sets; ++opt_set) {
-  uint32_t opt_set_mask;
-
-  opt_set_mask = 1 << opt_set;
   if (opt_set > 0)
 strm.Printf("\n");
   strm.Indent(name);
 
   // Different option sets may require different args.
   StreamString args_str;
+  uint32_t opt_set_mask = 1 << opt_set;
   cmd.GetFormattedCommandArguments(args_str, opt_set_mask);
 
   // First go through and print all options that take no arguments as a
@@ -475,12 +471,9 @@
   }
 
   if (args_str.GetSize() > 0) {
-if (cmd.WantsRawCommandString() && !only_print_args)
+if (cmd.WantsRawCommandString())
   strm.Printf(" --");
-
 strm << " " << args_str.GetString();
-if (only_print_args)
-  break;
   }
 }
   }


Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -390,8 +390,6 @@
 
 void Options::GenerateOptionUsage(Stream &strm, CommandObject &cmd,
   uint32_t screen_width) {
-  const bool only_print_args = cmd.IsDashDashCommand();
-
   auto opt_defs = GetDefinitions();
   const uint32_t save_indent_level = strm.GetIndentLevel();
   llvm::StringRef name = cmd.GetCommandName();
@@ -402,6 +400,7 @@
   if (num_options == 0)
 return;
 
+  const bool only_print_args = cmd.IsDashDashCommand();
   if (!only_print_args)
 strm.PutCString("\nCommand Options Usage:\n");
 
@@ -413,19 +412,16 @@
   //   [options-for-level-1]
   //   etc.
 
-  uint32_t num_option_sets = GetRequiredOptions().size();
-
   if (!only_print_args) {
+uint32_t num_option_sets = GetRequiredOptions().size();
 for (uint32_t opt_set = 0; opt_set < num_option_sets; ++opt_set) {
-  uint32_t opt_set_mask;
-
-  opt_set_mask = 1 << opt_set;
   if (opt_set > 0)
 strm.Printf("\n");
   strm.Indent(name);
 
   // Different option sets may require different args.
   StreamString args_str;
+  uint32_t opt_set_mask = 1 << opt_set;
   cmd.GetFormattedCommandArguments(args_str, opt_set_mask);
 
   // First go through and print all options that take no arguments as a
@@ -475,12 +471,9 @@
   }
 
   if (args_str.GetSize() > 0) {
-if (cmd.WantsRawCommandString() && !only_print_args)
+if (cmd.WantsRawCommandString())
   strm.Printf(" --");
-
 strm << " " << args_str.GetString();
-if (only_print_args)
-  break;
   }
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D124409: Filter non-external static members from SBType::GetFieldAtIndex.

[Sigurður Ásgeirsson via Phabricator via lldb-commits]

siggi-alpheus added a comment.

Thanks!




Comment at: 
lldb/test/Shell/SymbolFile/DWARF/x86/debug_static-member-anonymous-namespace.s:18
+#   static int s;
+#   int m = 14159265;
+# };

I wonder why I couldn't get the query to work against an initialized global 
without the main function. Is the assembly directly out of gcc, or did you need 
to tweak it?
PS: Now I fancy some pie.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124409

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

[Lldb-commits] [lldb] 8abfa51 - [lldb/test] Fix TestCppIncompleteTypeMembers.py

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-05-09T17:11:57+02:00
New Revision: 8abfa5119addc97e881025a819b8d643d53dda14

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

LOG: [lldb/test] Fix TestCppIncompleteTypeMembers.py

modify the Makefile.rules line which was interfering with the
target-specific variable values.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/make/Makefile.rules

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index f4aa6d646711f..3f9c2abecddbc 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -367,7 +367,7 @@ ifeq "$(OS)" "Windows_NT"
# MSVC 2015 or higher is required, which depends on c++14, so
# append these values unconditionally.
CXXFLAGS += -fms-compatibility-version=19.0
-   override CXXFLAGS := $(subst -std=c++11,-std=c++14,$(CXXFLAGS))
+   CXXFLAGS += -std=c++14
 
# The MSVC linker doesn't understand long section names
# generated by the clang compiler.



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

[Lldb-commits] [lldb] ac7747e - [lldb/test] Append CXXFLAGS_EXTRAS last in Makefile.rules

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-05-09T19:26:29+02:00
New Revision: ac7747ef281c3be4cfefa5adb5d0395cefc10223

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

LOG: [lldb/test] Append CXXFLAGS_EXTRAS last in Makefile.rules

This matches what we do with CFLAGS, and it started to matter
8abfa5119ad, which added some -std=-apending code.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/make/Makefile.rules

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules 
b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 3f9c2abecddbc..6917d73130c4a 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -275,7 +275,7 @@ ifeq "$(MAKE_GMODULES)" "YES"
 endif
 
 CFLAGS += $(CFLAGS_EXTRAS)
-CXXFLAGS += -std=c++11 $(CFLAGS) $(ARCH_CXXFLAGS) $(CXXFLAGS_EXTRAS)
+CXXFLAGS += -std=c++11 $(CFLAGS) $(ARCH_CXXFLAGS)
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
@@ -411,6 +411,8 @@ ifeq (1,$(USE_LIBDL))
endif
 endif
 
+CXXFLAGS += $(CXXFLAGS_EXTRAS)
+
 #--
 # dylib settings
 #--



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

[Lldb-commits] [lldb] a49d5e9 - [lldb/test] Remove superfluous -std=c++11 from tests

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-05-09T20:04:14+02:00
New Revision: a49d5e976e6d49a5a182a394c4a2a04395159b13

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

LOG: [lldb/test] Remove superfluous -std=c++11 from tests

We default to that anyway. It does not work on windows, and since
ac7747e, the flag actually takes effect.

Added: 


Modified: 
lldb/test/API/commands/expression/no-deadlock/Makefile
lldb/test/API/lang/c/step_over_no_deadlock/Makefile

Removed: 




diff  --git a/lldb/test/API/commands/expression/no-deadlock/Makefile 
b/lldb/test/API/commands/expression/no-deadlock/Makefile
index 4b3467bc4e82d..ee0d4690d8347 100644
--- a/lldb/test/API/commands/expression/no-deadlock/Makefile
+++ b/lldb/test/API/commands/expression/no-deadlock/Makefile
@@ -1,5 +1,4 @@
 CXX_SOURCES := locking.cpp
-CXXFLAGS_EXTRAS := -std=c++11
 ENABLE_THREADS := YES
 
 include Makefile.rules

diff  --git a/lldb/test/API/lang/c/step_over_no_deadlock/Makefile 
b/lldb/test/API/lang/c/step_over_no_deadlock/Makefile
index 4b3467bc4e82d..ee0d4690d8347 100644
--- a/lldb/test/API/lang/c/step_over_no_deadlock/Makefile
+++ b/lldb/test/API/lang/c/step_over_no_deadlock/Makefile
@@ -1,5 +1,4 @@
 CXX_SOURCES := locking.cpp
-CXXFLAGS_EXTRAS := -std=c++11
 ENABLE_THREADS := YES
 
 include Makefile.rules



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

[Lldb-commits] [PATCH] D125253: Add the ability to debug through an exec into ld

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, wallace, yinghuitan, rdhindsa.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

A previous commit enabled LLDB to be able to debug a program launched via ld: 
https://reviews.llvm.org/D108061.

This commit adds the ability to debug a program launched via ld when it happens 
during an exec into the dynamic loader. There was an issue where after the exec 
we would locate the rendezvous structure right away but it didn't contain any 
valid values and we would try to set the dyanamic loader breakpoint at address 
zero. This patch fixes that and adds a test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125253

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/dyld-exec-linux/Makefile
  lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
  lldb/test/API/functionalities/dyld-exec-linux/main.cpp

Index: lldb/test/API/functionalities/dyld-exec-linux/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-exec-linux/main.cpp
@@ -0,0 +1,16 @@
+#include 
+
+int main(int argc, const char *argv[], const char *envp[]) {
+  if (argc == 2) {
+// If we have two arguments the first is the path to this executable,
+// the second is the path to the linux dynamic loader that we should
+// exec with. We want to re-run this problem under the dynamic loader
+// and make sure we can hit the breakpoint in the "else".
+const char *interpreter = argv[1];
+const char *this_program = argv[0];
+const char *exec_argv[3] = {interpreter, this_program, nullptr};
+execve(interpreter, (char *const *)exec_argv, (char *const *)envp);
+  }
+  // Break here
+  return 0;
+}
Index: lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
@@ -0,0 +1,61 @@
+"""
+Test that LLDB can launch a linux executable and then execs into the dynamic
+loader into this program again.
+"""
+
+import lldb
+import os
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestLinux64ExecViaDynamicLoader(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(oslist=no_match(['linux']))
+@no_debug_info_test
+@skipIf(oslist=["linux"], archs=["arm"])
+def test(self):
+self.build()
+
+# Extracts path of the interpreter.
+exe = self.getBuildArtifact("a.out")
+
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(exe))
+interp_section = lldb.SBModule(spec).FindSection(".interp")
+if not interp_section:
+  return
+section_data = interp_section.GetSectionData()
+error = lldb.SBError()
+dyld_path = section_data.GetString(error,0)
+if error.Fail():
+  return
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set a breakpoint in the main function that will get hit after the
+# program exec's via the dynamic loader. The breakpoint will only get
+# hit if we can successfully read the shared library lists in the
+# DynamicLoaderPOSIXDYLD.cpp when we exec into the dynamic loader.
+breakpoint_main = target.BreakpointCreateBySourceRegex("// Break here", lldb.SBFileSpec("main.cpp"))
+# Setup our launch info to supply the dynamic loader path to the
+# program so it gets two args:
+# - path to a.out
+# - path to dynamic loader
+launch_info = lldb.SBLaunchInfo([dyld_path])
+error = lldb.SBError()
+process = target.Launch(launch_info, error)
+self.assertSuccess(error)
+
+threads = lldbutil.get_stopped_threads(process, lldb.eStopReasonExec)
+self.assertEqual(len(threads), 1, "We got a thread stopped for exec.")
+
+process.Continue();
+
+# Stopped on main here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+thread = process.GetSelectedThread()
+self.assertIn("main", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
Index: lldb/test/API/functionalities/dyld-exec-linux/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-exec-linux/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD

[Lldb-commits] [PATCH] D125253: Add the ability to debug through an exec into ld

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:312
   BreakpointSP dyld_break;
-  if (m_rendezvous.IsValid()) {
+  if (m_rendezvous.IsValid() && m_rendezvous.GetBreakAddress() != 0) {
 break_addr = m_rendezvous.GetBreakAddress();

Since zero break address is considered invalid, maybe move 
`m_rendezvous.GetBreakAddress() != 0` check into `DYLDRendezvous::IsValid()` 
method? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125253

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

[Lldb-commits] [PATCH] D125253: Add the ability to debug through an exec into ld

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:312
   BreakpointSP dyld_break;
-  if (m_rendezvous.IsValid()) {
+  if (m_rendezvous.IsValid() && m_rendezvous.GetBreakAddress() != 0) {
 break_addr = m_rendezvous.GetBreakAddress();

yinghuitan wrote:
> Since zero break address is considered invalid, maybe move 
> `m_rendezvous.GetBreakAddress() != 0` check into `DYLDRendezvous::IsValid()` 
> method? 
I wanted to be safe and not mess up anyone else that might be using 
m_rendezvous.IsValid() as it isn't super clear what IsValid() should return. So 
to be super safe and not affect any other code, I would prefer to leave it this 
way unless one of the original authors of this file has a different opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125253

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

[Lldb-commits] [PATCH] D125253: Add the ability to debug through an exec into ld

[walter erquinigo via Phabricator via lldb-commits]

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

nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125253

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

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h:41-48
+  /// \param[in] tid
+  /// The tid of the thread to be traced.
+  ///
+  /// \return
+  ///   A \a IntelPTSingleBufferTrace instance if tracing was successful, or
+  ///   an \a llvm::Error otherwise.
+  static llvm::Expected

jj10306 wrote:
> wallace wrote:
> > jj10306 wrote:
> > > Shouldn't this structure be general and have no notion of a tid since it 
> > > could represent the buffer of a single thread or the buffer of a single 
> > > CPU?
> > > The way I see it, this structure simply wraps the buffer of a specific 
> > > perf_event but has no notion of if it's for a specific tid or cpu.
> > > Then you could have two subclasses, one for thread one for cpu, that 
> > > inherit from this and have the additional context about the buffer. The 
> > > inheritance may be overkill, but point I'm trying to make is that this 
> > > structure should be agnostic to what "unit's" (thread or cpu) trace data 
> > > it contains
> > what I was thinking is to change this in a later diff to be like 
> >   Start(const TraceIntelPTStartRequest &request, Optional tid, 
> > Optional core_id);
> > 
> > which seems to me generic enough for all possible use cases. LLDB couldn't 
> > support cgroups, so we are limited to tids and core_ids. With this, it 
> > seems to me that having two child classes it's a little bit too much 
> > because the only difference are just two lines of code where we specify the 
> > perf_event configuration for tid and core_id.
> > 
> > But if you insist in the clarify of the subclasses, I for sure can do it 
> > that way.
> What you suggested makes sense, do you plan make this change in this diff or 
> is this in one of the future ones?
it's in a later diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124648

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

[Lldb-commits] [PATCH] D125148: Add an example command that runs to one of a set of breakpoints

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw added a comment.

Just trying to be helpful!




Comment at: lldb/examples/python/cont_to_bkpt.py:26
+target = exe_ctx.target
+if  not exe_ctx.target.IsValid():
+result.SetError("Need a valid target")

Reuse `target` here rather than `exe_ctx.target`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125148

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

[Lldb-commits] [lldb] b8d1776 - [trace][intelpt] Support system-wide tracing [2] - Add a dummy --per-core-tracing option

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-05-09T16:05:26-07:00
New Revision: b8d1776fc58d56af30d446386788e377d25dd512

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

LOG: [trace][intelpt] Support system-wide tracing [2] - Add a dummy 
--per-core-tracing option

This updates the documentation of the gdb-remote protocol, as well as the help 
messages, to include the new --per-core-tracing option.

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

Added: 


Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/test/API/commands/trace/TestTraceStartStop.py

lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 545fbb46ae242..50a47ccf50bfb 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -283,7 +283,7 @@ read packet: {"name":, 
"description":}/E;
 //Tracing technology name, e.g. intel-pt, arm-coresight.
 //
 ///* thread tracing only */
-//"tids": [],
+//"tids"?: [],
 //Individual threads to trace.
 //
 //... other parameters specific to the provided tracing type
@@ -298,16 +298,25 @@ read packet: {"name":, 
"description":}/E;
 // INTEL-PT
 //  intel-pt supports both "thread tracing" and "process tracing".
 //
-//  "Process tracing" is implemented by tracing each thread individually, but
-//  managed by the same "process trace" instance.
-//  Each actual thread trace, either from "process tracing" or "thread 
tracing",
+//  "Process tracing" is implemented in two 
diff erent ways. If the
+//  "perCoreTracing" option is false, then each thread is traced individually
+//  but managed by the same "process trace" instance. This means that the
+//  amount of trace buffers used is proportional to the number of running
+//  threads. This is the recommended option unless the number of threads is
+//  huge. If "perCoreTracing" is true, then each cpu core is traced invidually
+//  instead of each thread, which uses a fixed number of trace buffers, but
+//  might result in less data available for less frequent threads. See
+//  "perCoreTracing" below for more information.
+//
+//  Each actual trace buffer, either from "process tracing" or "thread 
tracing",
 //  is stored in an in-memory circular buffer, which keeps the most recent 
data.
 //
 //  Additional params in the input schema:
 //   {
-// "threadBufferSize": ,
-// Trace buffer size per thread in bytes. It must be a power of 2
-// greater than or equal to 4096 (2^12) bytes.
+// "traceBufferSize": ,
+// Size in bytes used by each individual per-thread or per-core trace
+// buffer. It must be a power of 2 greater than or equal to 4096 (2^12)
+// bytes.
 //
 // "enableTsc": ,
 // Whether to enable TSC timestamps or not. This is supported on
@@ -342,15 +351,35 @@ read packet: {"name":, 
"description":}/E;
 //  0 if supported.
 //
 // /* process tracing only */
+// "perCoreTracing": 
+// Instead of having an individual trace buffer per thread, this option
+// triggers the collection on a per cpu core basis. This effectively
+// traces the entire activity on all cores. At decoding time, in order
+// to correctly associate a decoded instruction with a thread, the
+// context switch trace of each core is needed, as well as a record per
+// cpu indicating which thread was running on each core when tracing
+// started. These secondary traces are correlated with the intel-pt
+// trace by comparing TSC timestamps.
+//
+// This option forces the capture of TSC timestamps (see "enableTsc").
+//
+// Note: This option can't be used simulatenously with any other trace
+// sessions because of its system-wide nature.
+//
+// /* process tracing only */
 // "processBufferSizeLimit": ,
 // Maximum total buffer size per process in bytes.
 // This limit applies to the sum of the sizes of all trace buffers for
 // the current process, 

[Lldb-commits] [lldb] 7b73de9 - [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-05-09T16:05:26-07:00
New Revision: 7b73de9ec2b19df040c919d3004dfbead9b6ac59

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

LOG: [trace][intelpt] Support system-wide tracing [3] - Refactor 
IntelPTThreadTrace

I'm refactoring IntelPTThreadTrace into IntelPTSingleBufferTrace so that it can
both single threads or single cores. In this diff I'm basically renaming the
class, moving it to its own file, and removing all the pieces that are not used
along with some basic cleanup.

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

Added: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h

Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/CMakeLists.txt
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Process/Linux/IntelPTCollector.h
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/Perf.cpp
lldb/source/Plugins/Process/Linux/Perf.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/unittests/Process/Linux/CMakeLists.txt
lldb/unittests/Process/Linux/PerfTests.cpp

Removed: 
lldb/unittests/Process/Linux/IntelPTCollectorTests.cpp



diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 50a47ccf50bfb..17e85a6d0df3f 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -495,7 +495,7 @@ read packet: OK/E;A
 // INTEL PT
 //
 //  Binary data kinds:
-//- threadTraceBuffer: trace buffer for a thread.
+//- traceBuffer: trace buffer for a thread or a core.
 //- procfsCpuInfo: contents of the /proc/cpuinfo file.
 //
 //  Counter info kinds:
@@ -550,7 +550,7 @@ read packet: {...object}/E;A
 // INTEL PT
 //
 //  Binary data kinds:
-//- threadTraceBuffer: trace buffer for a thread.
+//- traceBuffer: trace buffer for a thread or a core.
 //- procfsCpuInfo: contents of the /proc/cpuinfo file.
 //--
 

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index d331b18c4ae70..ddb758fe95e98 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -21,7 +21,7 @@ namespace lldb_private {
 // List of data kinds used by jLLDBGetState and jLLDBGetBinaryData.
 struct IntelPTDataKinds {
   static const char *kProcFsCpuInfo;
-  static const char *kThreadTraceBuffer;
+  static const char *kTraceBuffer;
 };
 
 /// jLLDBTraceStart gdb-remote packet

diff  --git a/lldb/source/Plugins/Process/Linux/CMakeLists.txt 
b/lldb/source/Plugins/Process/Linux/CMakeLists.txt
index 125cc0e38ca21..fb249008146b2 100644
--- a/lldb/source/Plugins/Process/Linux/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Linux/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_library(lldbPluginProcessLinux
   IntelPTCollector.cpp
+  IntelPTSingleBufferTrace.cpp
   NativeProcessLinux.cpp
   NativeRegisterContextLinux.cpp
   NativeRegisterContextLinux_arm.cpp

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index bcb270c9fae74..bf0b77b39f9b8 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -32,394 +32,6 @@ using namespace lldb_private;
 using namespace process_linux;
 using namespace llvm;
 
-const char *kOSEventIntelPTTypeFile =
-"/sys/bus/event_source/devices/intel_pt/type";
-
-const char *kPSBPeriodCapFile =
-"/sys/bus/event_source/devices/intel_pt/caps/psb_cyc";
-
-const char *kPSBPeriodValidValuesFile =
-"/sys/bus/event_source/devices/intel_pt/caps/psb_periods";
-
-const char *kTSCBitOffsetFile =
-"/sys/bus/event_source/devices/intel_pt/format/tsc";
-
-const char *kPSBPeriodBitOffsetFile =
-"/sys/bus/event_source/devices/intel_pt/format/psb_period";
-
-enum IntelPTConfigFileType {
-  Hex = 0,
-  // 0 or 1
-  ZeroOne,
-  Decimal,
-  // a bit index file always starts with the prefix config: following by an 
int,
-  // which represents the offset of the perf_event_attr.config value where to
-  // store a given configuration.
-  BitOffset
-};
-
-static Expected ReadIntelPTConfigFile(const char *file,
-IntelPTConfigFileType type) {
-  ErrorOr> stream =
-  MemoryBuffer::getFileAsSt

[Lldb-commits] [PATCH] D124640: [trace][intelpt] Support system-wide tracing [2] - Add a dummy --per-core-tracing option

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb8d1776fc58d: [trace][intelpt] Support system-wide tracing 
[2] - Add a dummy --per-core… (authored by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124640

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/test/API/commands/trace/TestTraceStartStop.py
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

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
@@ -152,3 +152,17 @@
 self.expect("c", substrs=['Thread', "can't be traced"])
 
 self.traceStopProcess()
+
+@skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+@testSBAPIAndCommands
+def testStartPerCoreSession(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.dbg.CreateTarget(exe)
+
+self.expect("b main")
+self.expect("r")
+
+self.traceStartProcess(
+error=True, perCoreTracing=True,
+substrs=["Per-core tracing is not supported"])
Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- lldb/test/API/commands/trace/TestTraceStartStop.py
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -35,18 +35,18 @@
 self.expect("r")
 
 self.traceStartThread(
-error=True, threadBufferSize=2000,
+error=True, traceBufferSize=2000,
 substrs=["The trace buffer size must be a power of 2", "It was 2000"])
 
 self.traceStartThread(
-error=True, threadBufferSize=5000,
+error=True, traceBufferSize=5000,
 substrs=["The trace buffer size must be a power of 2", "It was 5000"])
 
 self.traceStartThread(
-error=True, threadBufferSize=0,
+error=True, traceBufferSize=0,
 substrs=["The trace buffer size must be a power of 2", "It was 0"])
 
-self.traceStartThread(threadBufferSize=1048576)
+self.traceStartThread(traceBufferSize=1048576)
 
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
 def testSBAPIHelp(self):
@@ -55,7 +55,7 @@
 self.expect("r")
 
 help = self.getTraceOrCreate().GetStartConfigurationHelp()
-self.assertIn("threadBufferSize", help)
+self.assertIn("traceBufferSize", help)
 self.assertIn("processBufferSizeLimit", help)
 
 @skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -20,29 +20,27 @@
   Path path) {
   ObjectMapper o(value, path);
   if (!o || !fromJSON(value, (TraceStartRequest &)packet, path) ||
-  !o.map("enableTsc", packet.enableTsc) ||
-  !o.map("psbPeriod", packet.psbPeriod) ||
-  !o.map("threadBufferSize", packet.threadBufferSize) ||
-  !o.map("processBufferSizeLimit", packet.processBufferSizeLimit))
-return false;
-  if (packet.tids && packet.processBufferSizeLimit) {
-path.report("processBufferSizeLimit must be provided");
-return false;
-  }
-  if (!packet.tids && !packet.processBufferSizeLimit) {
-path.report("processBufferSizeLimit must not be provided");
+  !o.map("enableTsc", packet.enable_tsc) ||
+  !o.map("psbPeriod", packet.psb_period) ||
+  !o.map("traceBufferSize", packet.trace_buffer_size))
 return false;
+
+  if (packet.IsProcessTracing()) {
+if (!o.map("processBufferSizeLimit", packet.process_buffer_size_limit) ||
+!o.map("perCoreTracing", packet.per_core_tracing))
+  return false;
   }
   return true;
 }
 
 json::Value toJSON(const TraceIntelPTStartRequest &packet) {
   json::Value base = toJSON((const TraceStartRequest &)packet);
-  base.getAsObject()

[Lldb-commits] [PATCH] D124648: [trace][intelpt] Support system-wide tracing [3] - Refactor IntelPTThreadTrace

[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 rG7b73de9ec2b1: [trace][intelpt] Support system-wide tracing 
[3] - Refactor IntelPTThreadTrace (authored by Walter Erquinigo 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124648

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/unittests/Process/Linux/CMakeLists.txt
  lldb/unittests/Process/Linux/IntelPTCollectorTests.cpp
  lldb/unittests/Process/Linux/PerfTests.cpp

Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -86,4 +86,135 @@
 (SLEEP_NANOS + acceptable_overhead).count());
 }
 
+size_t ReadCylicBufferWrapper(void *buf, size_t buf_size, void *cyc_buf,
+  size_t cyc_buf_size, size_t cyc_start,
+  size_t offset) {
+  llvm::MutableArrayRef dst(reinterpret_cast(buf),
+ buf_size);
+  llvm::ArrayRef src(reinterpret_cast(cyc_buf),
+  cyc_buf_size);
+  ReadCyclicBuffer(dst, src, cyc_start, offset);
+  return dst.size();
+}
+
+TEST(CyclicBuffer, EdgeCases) {
+  size_t bytes_read;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  // We will always leave the last bytes untouched
+  // so that string comparisons work.
+  char smaller_buffer[4] = {};
+
+  // empty buffer to read into
+  bytes_read = ReadCylicBufferWrapper(smaller_buffer, 0, cyclic_buffer,
+  sizeof(cyclic_buffer), 3, 0);
+  ASSERT_EQ(0u, bytes_read);
+
+  // empty cyclic buffer
+  bytes_read = ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+  cyclic_buffer, 0, 3, 0);
+  ASSERT_EQ(0u, bytes_read);
+
+  // bigger offset
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 6);
+  ASSERT_EQ(0u, bytes_read);
+
+  // wrong offset
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 7);
+  ASSERT_EQ(0u, bytes_read);
+
+  // wrong start
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, sizeof(smaller_buffer),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 7);
+  ASSERT_EQ(0u, bytes_read);
+}
+
+TEST(CyclicBuffer, EqualSizeBuffer) {
+  size_t bytes_read = 0;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  char cyclic[] = "cyclic";
+  for (size_t i = 0; i < sizeof(cyclic); i++) {
+// We will always leave the last bytes untouched
+// so that string comparisons work.
+char equal_size_buffer[7] = {};
+bytes_read =
+ReadCylicBufferWrapper(equal_size_buffer, sizeof(cyclic_buffer),
+   cyclic_buffer, sizeof(cyclic_buffer), 3, i);
+ASSERT_EQ((sizeof(cyclic) - i - 1), bytes_read);
+ASSERT_STREQ(equal_size_buffer, (cyclic + i));
+  }
+}
+
+TEST(CyclicBuffer, SmallerSizeBuffer) {
+  size_t bytes_read;
+  uint8_t cyclic_buffer[6] = {'l', 'i', 'c', 'c', 'y', 'c'};
+
+  // We will always leave the last bytes untouched
+  // so that string comparisons work.
+  char smaller_buffer[4] = {};
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 0);
+  ASSERT_EQ(3u, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "cyc");
+
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 1);
+  ASSERT_EQ(3u, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "ycl");
+
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (sizeof(smaller_buffer) - 1),
+ cyclic_buffer, sizeof(cyclic_buffer), 3, 2);
+  ASSERT_EQ(3u, bytes_read);
+  ASSERT_STREQ(smaller_buffer, "cli");
+
+  bytes_read =
+  ReadCylicBufferWrapper(smaller_buffer, (

[Lldb-commits] [lldb] 879a47a - Add the ability to debug through an exec into ld

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2022-05-09T16:07:40-07:00
New Revision: 879a47a55ffb94976cbac1d191ef53be135d86a7

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

LOG: Add the ability to debug through an exec into ld

A previous commit enabled LLDB to be able to debug a program launched via ld: 
https://reviews.llvm.org/D108061.

This commit adds the ability to debug a program launched via ld when it happens 
during an exec into the dynamic loader. There was an issue where after the exec 
we would locate the rendezvous structure right away but it didn't contain any 
valid values and we would try to set the dyanamic loader breakpoint at address 
zero. This patch fixes that and adds a test.

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

Added: 
lldb/test/API/functionalities/dyld-exec-linux/Makefile
lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
lldb/test/API/functionalities/dyld-exec-linux/main.cpp

Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index 7993c5906aa35..b3360dbf6158a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -309,7 +309,7 @@ bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
   addr_t break_addr;
   Target &target = m_process->GetTarget();
   BreakpointSP dyld_break;
-  if (m_rendezvous.IsValid()) {
+  if (m_rendezvous.IsValid() && m_rendezvous.GetBreakAddress() != 0) {
 break_addr = m_rendezvous.GetBreakAddress();
 LLDB_LOG(log, "Setting rendezvous break address for pid {0} at {1:x}",
  m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID,
@@ -565,7 +565,7 @@ ModuleSP DynamicLoaderPOSIXDYLD::LoadInterpreterModule() {
   FileSpec file(info.GetName().GetCString());
   ModuleSpec module_spec(file, target.GetArchitecture());
 
-  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec, 
+  if (ModuleSP module_sp = target.GetOrCreateModule(module_spec,
 true /* notify */)) {
 UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_interpreter_base,
  false);

diff  --git a/lldb/test/API/functionalities/dyld-exec-linux/Makefile 
b/lldb/test/API/functionalities/dyld-exec-linux/Makefile
new file mode 100644
index 0..3d0b98f13f3d7
--- /dev/null
+++ b/lldb/test/API/functionalities/dyld-exec-linux/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules

diff  --git 
a/lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py 
b/lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
new file mode 100644
index 0..2696ae1838249
--- /dev/null
+++ b/lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
@@ -0,0 +1,61 @@
+"""
+Test that LLDB can launch a linux executable and then execs into the dynamic
+loader into this program again.
+"""
+
+import lldb
+import os
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestLinux64ExecViaDynamicLoader(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(oslist=no_match(['linux']))
+@no_debug_info_test
+@skipIf(oslist=["linux"], archs=["arm"])
+def test(self):
+self.build()
+
+# Extracts path of the interpreter.
+exe = self.getBuildArtifact("a.out")
+
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(exe))
+interp_section = lldb.SBModule(spec).FindSection(".interp")
+if not interp_section:
+  return
+section_data = interp_section.GetSectionData()
+error = lldb.SBError()
+dyld_path = section_data.GetString(error,0)
+if error.Fail():
+  return
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set a breakpoint in the main function that will get hit after the
+# program exec's via the dynamic loader. The breakpoint will only get
+# hit if we can successfully read the shared library lists in the
+# DynamicLoaderPOSIXDYLD.cpp when we exec into the dynamic loader.
+breakpoint_main = target.BreakpointCreateBySourceRegex("// Break 
here", lldb.SBFileSpec("main.cpp"))
+# Setup our launch info to supply the dynamic loader path to the
+# program so it gets two args:
+# - path to a.out
+# - path to dynamic loader
+launch_info = lldb.SBLaunchInfo([dyld_pa

[Lldb-commits] [PATCH] D125253: Add the ability to debug through an exec into ld

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG879a47a55ffb: Add the ability to debug through an exec into 
ld (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125253

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/dyld-exec-linux/Makefile
  lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
  lldb/test/API/functionalities/dyld-exec-linux/main.cpp

Index: lldb/test/API/functionalities/dyld-exec-linux/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-exec-linux/main.cpp
@@ -0,0 +1,16 @@
+#include 
+
+int main(int argc, const char *argv[], const char *envp[]) {
+  if (argc == 2) {
+// If we have two arguments the first is the path to this executable,
+// the second is the path to the linux dynamic loader that we should
+// exec with. We want to re-run this problem under the dynamic loader
+// and make sure we can hit the breakpoint in the "else".
+const char *interpreter = argv[1];
+const char *this_program = argv[0];
+const char *exec_argv[3] = {interpreter, this_program, nullptr};
+execve(interpreter, (char *const *)exec_argv, (char *const *)envp);
+  }
+  // Break here
+  return 0;
+}
Index: lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-exec-linux/TestDyldExecLinux.py
@@ -0,0 +1,61 @@
+"""
+Test that LLDB can launch a linux executable and then execs into the dynamic
+loader into this program again.
+"""
+
+import lldb
+import os
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestLinux64ExecViaDynamicLoader(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(oslist=no_match(['linux']))
+@no_debug_info_test
+@skipIf(oslist=["linux"], archs=["arm"])
+def test(self):
+self.build()
+
+# Extracts path of the interpreter.
+exe = self.getBuildArtifact("a.out")
+
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(exe))
+interp_section = lldb.SBModule(spec).FindSection(".interp")
+if not interp_section:
+  return
+section_data = interp_section.GetSectionData()
+error = lldb.SBError()
+dyld_path = section_data.GetString(error,0)
+if error.Fail():
+  return
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set a breakpoint in the main function that will get hit after the
+# program exec's via the dynamic loader. The breakpoint will only get
+# hit if we can successfully read the shared library lists in the
+# DynamicLoaderPOSIXDYLD.cpp when we exec into the dynamic loader.
+breakpoint_main = target.BreakpointCreateBySourceRegex("// Break here", lldb.SBFileSpec("main.cpp"))
+# Setup our launch info to supply the dynamic loader path to the
+# program so it gets two args:
+# - path to a.out
+# - path to dynamic loader
+launch_info = lldb.SBLaunchInfo([dyld_path])
+error = lldb.SBError()
+process = target.Launch(launch_info, error)
+self.assertSuccess(error)
+
+threads = lldbutil.get_stopped_threads(process, lldb.eStopReasonExec)
+self.assertEqual(len(threads), 1, "We got a thread stopped for exec.")
+
+process.Continue();
+
+# Stopped on main here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+thread = process.GetSelectedThread()
+self.assertIn("main", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
Index: lldb/test/API/functionalities/dyld-exec-linux/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-exec-linux/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -309,7 +309,7 @@
   addr_t break_addr;
   Target &target = m_process->GetTarget();
   BreakpointSP dyld_break;
-  if (m_rendezvous.IsValid()) {
+  if (m_rendezvous.IsValid() && m_rendezvous.GetBreakAddress() != 0) {
 break_addr = m_rendezvous.GetBreakAddress();
 LLDB_LOG(log, "Setting rendezvous break address for pid {0} at {1:x}",
  m_process ? m_process->GetID() : LLDB_INVALID_PROCESS_ID,
@@ -565,7 +565,7 @@
   FileSpe

[Lldb-commits] [lldb] 909a2e3 - [lldb] Fix 7b73de9ec2b19df040c919d3004dfbead9b6ac59

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-05-09T21:02:40-07:00
New Revision: 909a2e3c8822f0826234aa320794003c7066fada

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

LOG: [lldb] Fix 7b73de9ec2b19df040c919d3004dfbead9b6ac59

This commit causes
https://lab.llvm.org/buildbot/#/builders/17/builds/21743 to fail
seemingly because of bad handling of the PERF_ATTR_SIZE_VER5 symbol.

This patch tries to handle better the absence of this symbol.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
index 47d587741dbc..30c30eb6cfff 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -146,6 +146,7 @@ static Error CheckPsbPeriod(size_t psb_period) {
   return createStringError(inconvertibleErrorCode(), error.str().c_str());
 }
 
+#ifdef PERF_ATTR_SIZE_VER5
 static Expected
 GeneratePerfEventConfigValue(bool enable_tsc, Optional psb_period) {
   uint64_t config = 0;
@@ -179,9 +180,8 @@ GeneratePerfEventConfigValue(bool enable_tsc, 
Optional psb_period) {
 static Expected
 CreateIntelPTPerfEventConfiguration(bool enable_tsc,
 llvm::Optional psb_period) {
-#ifndef PERF_ATTR_SIZE_VER5
-  return llvm_unreachable("Intel PT Linux perf event not supported");
-#else
+  return createStringError(inconvertibleErrorCode(),
+   "Intel PT Linux perf event not supported");
   perf_event_attr attr;
   memset(&attr, 0, sizeof(attr));
   attr.size = sizeof(attr);
@@ -204,8 +204,8 @@ CreateIntelPTPerfEventConfiguration(bool enable_tsc,
 return intel_pt_type.takeError();
 
   return attr;
-#endif
 }
+#endif
 
 size_t IntelPTSingleBufferTrace::GetTraceBufferSize() const {
   return m_perf_event.GetAuxBuffer().size();
@@ -263,6 +263,10 @@ IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, 
size_t size) const {
 Expected
 IntelPTSingleBufferTrace::Start(const TraceIntelPTStartRequest &request,
 lldb::tid_t tid) {
+#ifndef PERF_ATTR_SIZE_VER5
+  return createStringError(inconvertibleErrorCode(),
+   "Intel PT Linux perf event not supported");
+#else
   Log *log = GetLog(POSIXLog::Trace);
 
   LLDB_LOG(log, "Will start tracing thread id {0}", tid);
@@ -299,4 +303,5 @@ IntelPTSingleBufferTrace::Start(const 
TraceIntelPTStartRequest &request,
   } else {
 return perf_event.takeError();
   }
+#endif
 }



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

[Lldb-commits] [lldb] c4172c7 - [lldb] Fix 7b73de9ec2b19df040c919d3004dfbead9b6ac59

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-05-09T21:12:11-07:00
New Revision: c4172c751a39313efd9c7588a103482c3648987e

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

LOG: [lldb] Fix 7b73de9ec2b19df040c919d3004dfbead9b6ac59

It turns out that the issue in
https://lab.llvm.org/buildbot/#/builders/17/builds/21754 is that a
size_t is attempted to be used interchangeably with uint64_t.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
index 30c30eb6cfff..a1cd30b2fb5a 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -179,9 +179,7 @@ GeneratePerfEventConfigValue(bool enable_tsc, 
Optional psb_period) {
 ///   or an \a llvm::Error otherwise.
 static Expected
 CreateIntelPTPerfEventConfiguration(bool enable_tsc,
-llvm::Optional psb_period) {
-  return createStringError(inconvertibleErrorCode(),
-   "Intel PT Linux perf event not supported");
+llvm::Optional psb_period) {
   perf_event_attr attr;
   memset(&attr, 0, sizeof(attr));
   attr.size = sizeof(attr);



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

[Lldb-commits] [lldb] b6bb9e7 - [lldb] Fix 7b73de9ec2b19df040c919d3004dfbead9b6ac59

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-05-09T21:29:00-07:00
New Revision: b6bb9e7d61fd84caf1221eeb09a743e8393fb43f

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

LOG: [lldb] Fix 7b73de9ec2b19df040c919d3004dfbead9b6ac59

It turns out that the issue in
https://lab.llvm.org/buildbot/#/builders/17/builds/21754 is that a
size_t is attempted to be used interchangeably with uint64_t.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
index a1cd30b2fb5a..18c69d13700d 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
@@ -148,7 +148,7 @@ static Error CheckPsbPeriod(size_t psb_period) {
 
 #ifdef PERF_ATTR_SIZE_VER5
 static Expected
-GeneratePerfEventConfigValue(bool enable_tsc, Optional psb_period) {
+GeneratePerfEventConfigValue(bool enable_tsc, Optional psb_period) {
   uint64_t config = 0;
   // tsc is always supported
   if (enable_tsc) {



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

[Lldb-commits] [lldb] 9d2dd6d - [NFC][lldb][trace] Use uint64_t when decoding and enconding json

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-05-09T21:55:43-07:00
New Revision: 9d2dd6d7622335ba9c19b55ac7d463cf662cab0d

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

LOG: [NFC][lldb][trace] Use uint64_t when decoding and enconding json

llvm's json parser supports uint64_t, so let's better use it for the
packets being sent between lldb and lldb-server instead of using int64_t
as an intermediate type, which might be error-prone.

Added: 


Modified: 
lldb/include/lldb/Utility/TraceGDBRemotePackets.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceGDBRemotePackets.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
index b2669ee3d813d..8e8919c4d8d87 100644
--- a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
@@ -45,7 +45,7 @@ struct TraceStartRequest {
 
   /// If \a llvm::None, then this starts tracing the whole process. Otherwise,
   /// only tracing for the specified threads is enabled.
-  llvm::Optional> tids;
+  llvm::Optional> tids;
 
   /// \return
   /// \b true if \a tids is \a None, i.e. whole process tracing.
@@ -73,7 +73,7 @@ struct TraceStopRequest {
   std::string type;
   /// If \a llvm::None, then this stops tracing the whole process. Otherwise,
   /// only tracing for the specified threads is stopped.
-  llvm::Optional> tids;
+  llvm::Optional> tids;
 };
 
 bool fromJSON(const llvm::json::Value &value, TraceStopRequest &packet,
@@ -98,7 +98,7 @@ struct TraceBinaryData {
   /// Identifier of data to fetch with jLLDBTraceGetBinaryData.
   std::string kind;
   /// Size in bytes for this data.
-  int64_t size;
+  uint64_t size;
 };
 
 bool fromJSON(const llvm::json::Value &value, TraceBinaryData &packet,
@@ -107,7 +107,7 @@ bool fromJSON(const llvm::json::Value &value, 
TraceBinaryData &packet,
 llvm::json::Value toJSON(const TraceBinaryData &packet);
 
 struct TraceThreadState {
-  int64_t tid;
+  lldb::tid_t tid;
   /// List of binary data objects for this thread.
   std::vector binaryData;
 };
@@ -161,11 +161,11 @@ struct TraceGetBinaryDataRequest {
   /// Identifier for the data.
   std::string kind;
   /// Optional tid if the data is related to a thread.
-  llvm::Optional tid;
+  llvm::Optional tid;
   /// Offset in bytes from where to start reading the data.
-  int64_t offset;
+  uint64_t offset;
   /// Number of bytes to read.
-  int64_t size;
+  uint64_t size;
 };
 
 bool fromJSON(const llvm::json::Value &value,

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index ddb758fe95e98..cb79fb351a438 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -16,6 +16,9 @@
 #include 
 
 /// See docs/lldb-gdb-remote.txt for more information.
+///
+/// Do not use system-dependent types, like size_t, because they might cause
+/// issues when compiling on arm.
 namespace lldb_private {
 
 // List of data kinds used by jLLDBGetState and jLLDBGetBinaryData.
@@ -28,20 +31,20 @@ struct IntelPTDataKinds {
 /// \{
 struct TraceIntelPTStartRequest : TraceStartRequest {
   /// Size in bytes to use for each thread's trace buffer.
-  int64_t trace_buffer_size;
+  uint64_t trace_buffer_size;
 
   /// Whether to enable TSC
   bool enable_tsc;
 
   /// PSB packet period
-  llvm::Optional psb_period;
+  llvm::Optional psb_period;
 
   /// Required when doing "process tracing".
   ///
   /// Limit in bytes on all the thread traces started by this "process trace"
   /// instance. When a thread is about to be traced and the limit would be hit,
   /// then a "tracing" stop event is triggered.
-  llvm::Optional process_buffer_size_limit;
+  llvm::Optional process_buffer_size_limit;
 
   /// Whether to have a trace buffer per thread or per cpu core.
   llvm::Optional per_core_tracing;

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index bf0b77b39f9b8..a1544680043f5 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -72,10 +72,9 @@ std::vector
 IntelPTThreadTraceCollection::GetThreadStates() const {
   std::vector states;
   for (const auto &it : m_thread_traces)
-states.push_back({static_cast(it.first),
+states.push_back({it.first,
   {TraceBinaryData{IntelPTData

[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/docs/lldb-gdb-remote.txt:372-374
+// This limit applies to the sum of the sizes of all trace and core
+// buffers for the current process, excluding the ones started with
+// "thread tracing".

jj10306 wrote:
> Not sure what is trying to be communicated hear, so maybe my suggestion 
> doesn't make sense, but I was confused by the wording here
you are right!



Comment at: lldb/docs/lldb-gdb-remote.txt:465-474
 //  "tid": ,
 //  "binaryData": [
 //{
 //  "kind": ,
 //  Identifier for some binary data related to this thread to
 //  fetch with the jLLDBTraceGetBinaryData packet.
 //  "size": ,

jj10306 wrote:
> Should the thread info be optional now that we have an optional `cores`? If 
> not, can you explain how this output works in the case that you are doing per 
> core tracing? Will both the tid binary data and the cores binary data section 
> be populated?
I'll make this field optional and include more documentation below



Comment at: lldb/include/lldb/lldb-types.h:92
 typedef uint64_t queue_id_t;
+typedef int core_id_t; // CPU core id
 } // namespace lldb

jj10306 wrote:
> nit: this should be an unsigned int of some sort?
I wasn't sure if int would be correct for all platforms, but in any case, I 
should go for unsigned, as you suggest, which is a more restrictive type, and 
if in the future someone needs a signed int, they could change this type



Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp:228-234
+  return m_per_thread_process_trace_up->TraceStart(tid);
 }
 
 Error IntelPTCollector::OnThreadDestroyed(lldb::tid_t tid) {
-  if (IsProcessTracingEnabled() && m_process_trace->TracesThread(tid))
-return m_process_trace->TraceStop(tid);
+  if (IsProcessTracingEnabled() &&
+  m_per_thread_process_trace_up->TracesThread(tid))
+return m_per_thread_process_trace_up->TraceStop(tid);

jj10306 wrote:
> Do we not need to check if `m_per_thread_process_trace_up` is null here (aka 
> per core tracing is enabled)?
I need to check that m_per_thread_process_trace_up exists



Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp:260
+  }
+  if (m_per_core_process_trace_up) {
+state.cores.emplace();

jj10306 wrote:
> Should this be an "else if" if these two are mutually exclusive?
good idea



Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:63-64
+
+/// Manages a "process trace" instance by tracing each thread individually.
+class IntelPTPerThreadProcessTrace {
 public:

jj10306 wrote:
> Why is this named "PerThread" if it handles both the per thread and per core 
> cases for "process trace"? Perhaps I'm missing something
this doesn't handle the per core case. This is handling exclusively the case of 
"process trace" without the "per-cpu" option. This class effectively creates a 
perf event per thread. Even its Start method asks for tids



Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.h:152-156
+  IntelPTPerThreadProcessTraceUP m_per_thread_process_trace_up;
+  /// Cores traced due to per-core "process tracing". Only one active
+  /// "process tracing" instance is allowed for a single process.
+  /// This might be \b nullptr.
+  IntelPTMultiCoreTraceUP m_per_core_process_trace_up;

jj10306 wrote:
> So these are mutually exclusive tight?
yes, I'll leave a comment about that here



Comment at: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp:20-22
+  int64_t required = cores.size() * request.trace_buffer_size;
+  int64_t limit = request.process_buffer_size_limit.getValueOr(
+  std::numeric_limits::max());

jj10306 wrote:
> nit: why are we using signed integers here?
yes, this is bad, i'm changing to unsigned



Comment at: lldb/source/Plugins/Process/Linux/Perf.h:111-115
   /// \param[in] pid
-  /// The process to be monitored by the event.
+  /// The process or thread to be monitored by the event.
   ///
   /// \param[in] cpu
   /// The cpu to be monitored by the event.

jj10306 wrote:
> Maybe add explanation of what is passed to the syscall for pid and cpu when 
> they are `None` here?
makes sense


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124858

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

[Lldb-commits] [lldb] 285b39a - Revert "[NFC][lldb][trace] Use uint64_t when decoding and enconding json"

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-05-09T22:47:05-07:00
New Revision: 285b39a31ec63a0253fa88c3c61f447712e2f131

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

LOG: Revert "[NFC][lldb][trace] Use uint64_t when decoding and enconding json"

This reverts commit 9d2dd6d7622335ba9c19b55ac7d463cf662cab0d.

Reverting because this exposes an issue in the uint64_t json parser.

Added: 


Modified: 
lldb/include/lldb/Utility/TraceGDBRemotePackets.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceGDBRemotePackets.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
index 8e8919c4d8d87..b2669ee3d813d 100644
--- a/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceGDBRemotePackets.h
@@ -45,7 +45,7 @@ struct TraceStartRequest {
 
   /// If \a llvm::None, then this starts tracing the whole process. Otherwise,
   /// only tracing for the specified threads is enabled.
-  llvm::Optional> tids;
+  llvm::Optional> tids;
 
   /// \return
   /// \b true if \a tids is \a None, i.e. whole process tracing.
@@ -73,7 +73,7 @@ struct TraceStopRequest {
   std::string type;
   /// If \a llvm::None, then this stops tracing the whole process. Otherwise,
   /// only tracing for the specified threads is stopped.
-  llvm::Optional> tids;
+  llvm::Optional> tids;
 };
 
 bool fromJSON(const llvm::json::Value &value, TraceStopRequest &packet,
@@ -98,7 +98,7 @@ struct TraceBinaryData {
   /// Identifier of data to fetch with jLLDBTraceGetBinaryData.
   std::string kind;
   /// Size in bytes for this data.
-  uint64_t size;
+  int64_t size;
 };
 
 bool fromJSON(const llvm::json::Value &value, TraceBinaryData &packet,
@@ -107,7 +107,7 @@ bool fromJSON(const llvm::json::Value &value, 
TraceBinaryData &packet,
 llvm::json::Value toJSON(const TraceBinaryData &packet);
 
 struct TraceThreadState {
-  lldb::tid_t tid;
+  int64_t tid;
   /// List of binary data objects for this thread.
   std::vector binaryData;
 };
@@ -161,11 +161,11 @@ struct TraceGetBinaryDataRequest {
   /// Identifier for the data.
   std::string kind;
   /// Optional tid if the data is related to a thread.
-  llvm::Optional tid;
+  llvm::Optional tid;
   /// Offset in bytes from where to start reading the data.
-  uint64_t offset;
+  int64_t offset;
   /// Number of bytes to read.
-  uint64_t size;
+  int64_t size;
 };
 
 bool fromJSON(const llvm::json::Value &value,

diff  --git a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h 
b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
index cb79fb351a438..ddb758fe95e98 100644
--- a/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
+++ b/lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
@@ -16,9 +16,6 @@
 #include 
 
 /// See docs/lldb-gdb-remote.txt for more information.
-///
-/// Do not use system-dependent types, like size_t, because they might cause
-/// issues when compiling on arm.
 namespace lldb_private {
 
 // List of data kinds used by jLLDBGetState and jLLDBGetBinaryData.
@@ -31,20 +28,20 @@ struct IntelPTDataKinds {
 /// \{
 struct TraceIntelPTStartRequest : TraceStartRequest {
   /// Size in bytes to use for each thread's trace buffer.
-  uint64_t trace_buffer_size;
+  int64_t trace_buffer_size;
 
   /// Whether to enable TSC
   bool enable_tsc;
 
   /// PSB packet period
-  llvm::Optional psb_period;
+  llvm::Optional psb_period;
 
   /// Required when doing "process tracing".
   ///
   /// Limit in bytes on all the thread traces started by this "process trace"
   /// instance. When a thread is about to be traced and the limit would be hit,
   /// then a "tracing" stop event is triggered.
-  llvm::Optional process_buffer_size_limit;
+  llvm::Optional process_buffer_size_limit;
 
   /// Whether to have a trace buffer per thread or per cpu core.
   llvm::Optional per_core_tracing;

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index a1544680043f5..bf0b77b39f9b8 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -72,9 +72,10 @@ std::vector
 IntelPTThreadTraceCollection::GetThreadStates() const {
   std::vector states;
   for (const auto &it : m_thread_traces)
-states.push_back({it.first,
+states.push_back({static_cast(it.first),
   {TraceBinaryData{IntelPTDataKinds::kTraceBuffer,
-