[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

2022-10-13 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Looks good. I suggested some changes, which I hope will reduce duplication and 
better empasize the differences between the various branches in the code. I 
think I understand the algorithm now, and I believe it is ok, though I can't 
escape the feeling that this could have been done in a simpler way.




Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:459-469
+  if (parent->kind == Member::Struct) {
+parent->fields.push_back(std::move(fields.back()));
+end_offset_map[end_offset].push_back(parent);
+  } else {
+lldbassert(parent == &record &&
+   "If parent is union, it must be the top level record.");
+MemberUP anonymous_struct = std::make_unique(Member::Struct);

The current version will create a needless nested struct in case of a union 
with a single member. I think it should be possible to just insert a single 
field first, and let it be converted to a struct later -- if it is necessary.



Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:471-491
+  if (parent->kind == Member::Struct) {
+MemberUP anonymous_union = std::make_unique(Member::Union);
+for (auto &field : fields) {
+  int64_t bit_size = field->bit_size;
+  anonymous_union->fields.push_back(std::move(field));
+  end_offset_map[offset + bit_size].push_back(
+  anonymous_union->fields.back().get());

This should be equivalent but shorter, and --  I hope -- more obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-13 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

Would a `lit.local.cfg` work in this folder (`lldb/test/Shell/lldb-server/` 
that is)? That would automatically apply it to all existing and future tests.

`llvm/test/MC/AArch64/lit.local.cfg` is one example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135825

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

[Lldb-commits] [lldb] 32cb683 - [lldb] Place PlatformQemu Properties into anonymous namespace

2022-10-13 Thread Pavel Labath via lldb-commits 

Author: Pavel Labath
Date: 2022-10-13T15:23:58+02:00
New Revision: 32cb683d2d3aa9c8fe0f8b24bd3ad1a5ea53bdcc

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

LOG: [lldb] Place PlatformQemu Properties into anonymous namespace

It's fine right now, but will break as soon as someone else declares a
PluginProperties class in the same way.

Also tighten up the scope of the anonymous namespaces surrounding the
other PluginProperties classes.

Added: 


Modified: 
lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp 
b/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
index 26e73a7aeaede..3825c0bc09c77 100644
--- a/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
+++ b/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
@@ -53,7 +53,6 @@ template  struct jit_descriptor {
 };
 
 namespace {
-
 enum EnableJITLoaderGDB {
   eEnableJITLoaderGDBDefault,
   eEnableJITLoaderGDBOn,
@@ -105,6 +104,7 @@ class PluginProperties : public Properties {
 g_jitloadergdb_properties[ePropertyEnable].default_uint_value);
   }
 };
+} // namespace
 
 static PluginProperties &GetGlobalPluginProperties() {
   static PluginProperties g_settings;
@@ -112,8 +112,8 @@ static PluginProperties &GetGlobalPluginProperties() {
 }
 
 template 
-bool ReadJITEntry(const addr_t from_addr, Process *process,
-  jit_code_entry *entry) {
+static bool ReadJITEntry(const addr_t from_addr, Process *process,
+ jit_code_entry *entry) {
   lldbassert(from_addr % sizeof(ptr_t) == 0);
 
   ArchSpec::Core core = process->GetTarget().GetArchitecture().GetCore();
@@ -142,8 +142,6 @@ bool ReadJITEntry(const addr_t from_addr, Process *process,
   return true;
 }
 
-} // anonymous namespace end
-
 JITLoaderGDB::JITLoaderGDB(lldb_private::Process *process)
 : JITLoader(process), m_jit_objects(),
   m_jit_break_id(LLDB_INVALID_BREAK_ID),

diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 61c9b9f84272d..72fc82c3cd804 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -100,13 +100,13 @@ class PluginProperties : public Properties {
   }
 };
 
+} // namespace
+
 static PluginProperties &GetGlobalPluginProperties() {
   static PluginProperties g_settings;
   return g_settings;
 }
 
-} // namespace
-
 static bool GetDebugLinkContents(const llvm::object::COFFObjectFile &coff_obj,
  std::string &gnu_debuglink_file,
  uint32_t &gnu_debuglink_crc) {

diff  --git a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp 
b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
index 7ee92ef76c9c7..4ba20117cdb56 100644
--- a/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
+++ b/lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp
@@ -23,6 +23,7 @@ using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE(PlatformQemuUser)
 
+namespace {
 #define LLDB_PROPERTIES_platformqemuuser
 #include "PlatformQemuUserProperties.inc"
 
@@ -71,6 +72,8 @@ class PluginProperties : public Properties {
   }
 };
 
+} // namespace
+
 static PluginProperties &GetGlobalProperties() {
   static PluginProperties g_settings;
   return g_settings;

diff  --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp 
b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index 6b9be1e55d4f2..88971e8336a17 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -83,13 +83,13 @@ class PluginProperties : public Properties {
   }
 };
 
+} // namespace
+
 static PluginProperties &GetGlobalPluginProperties() {
   static PluginProperties g_settings;
   return g_settings;
 }
 
-} // anonymous namespace end
-
 static const lldb::tid_t g_kernel_tid = 1;
 
 llvm::StringRef ProcessKDP::GetPluginDescriptionStatic() {

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index ecd9606106ba6..ba180cc821e29 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -166,13 +166,13 @@ class PluginProperties : public Properties {
   }
 };
 
+} // names

[Lldb-commits] [PATCH] D134878: Update developer policy on potentially breaking changes

2022-10-13 Thread Aaron Ballman via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8673444598be: Update developer policy on potentially 
breaking changes (authored by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134878

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's 
code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading 
a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is to give vendors early notice that potentially disruptive changes
+  are being considered but have not yet been accepted. Vendors can give early
+  testing feedback on the changes to alert us to unacceptable breakages. The
+  current list of vendor groups is:
+
+  * `Clang vendors `_
+  * `libc++ vendors `_
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.
+
+* When committing the change to the repository, add appropriate information
+  about the potentially breaking changes to the ``Potentially Breaking 
Changes``
+  section of the project's release notes. The release note should have
+  information about what the change is, what is potentially disruptive about
+  it, as well as any code examples, links, and motivation that is appropriate
+  to share with users. This helps users to learn about potential issues with
+  upgrading to that release.
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the
+  `Announcements `_ channel on
+  Discourse. The post should be tagged with the ``potentially-breaking`` label
+  and a label specific to the project (such as ``clang``, ``llvm``, etc). This
+  is another mechanism by which we can give pre-release notice to users about
+  potentially disruptive changes. It is a lower-traffic alternative to the
+  joining "vendors" group. To automatically be notified of new announcements
+  with the ``potentially-breaking`` label, go to your user preferences page in
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``.
+
 .. _code owners:
 
 Code Owners
@@ -181,7 +226,12 @@
   programming paradigms.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
-  a future release, such as removal of a deprecated feature.
+  a future release, such as removal of a deprecated feature. In this case, the
+  release note should be added to a ``Potentially Breaking Changes`` section of
+  the notes with sufficient information and examples to demonstrate the
+  potential disruption. Additionally, any new entries to this section should be
+  announced in the `Announcements `_
+  channel on Discourse. See :ref:`breaking` for more details.
 
 Code reviewers are encouraged to request a release note if they think one is
 warranted when performing a code review.


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" grou

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-13 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added a comment.

In D135825#3855539 , @DavidSpickett 
wrote:

> Would a `lit.local.cfg` work in this folder (`lldb/test/Shell/lldb-server/` 
> that is)? That would automatically apply it to all existing and future tests.
>
> `llvm/test/MC/AArch64/lit.local.cfg` is one example.

Just tried this out, seems like it will work for the purposes of this patch. 
I'll update this patch when I have a moment. Thanks for reviewing! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135825

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

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

2022-10-13 Thread Henrique Bucher via Phabricator via lldb-commits 
HenriqueBucher added a comment.

Is there anything to be done at this point? Can I reset my branch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

2022-10-13 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu updated this revision to Diff 467573.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
  lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt
  lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

Index: lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
===
--- /dev/null
+++ lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -0,0 +1,244 @@
+//===-- UdtRecordCompleterTests.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private::npdb;
+using namespace llvm;
+
+namespace {
+using Member = UdtRecordCompleter::Member;
+using MemberUP = std::unique_ptr;
+using Record = UdtRecordCompleter::Record;
+
+class WrappedMember {
+public:
+  WrappedMember(const Member &obj) : m_obj(obj) {}
+
+private:
+  const Member &m_obj;
+
+  friend bool operator==(const WrappedMember &lhs, const WrappedMember &rhs) {
+return lhs.m_obj.kind == rhs.m_obj.kind &&
+   lhs.m_obj.name == rhs.m_obj.name &&
+   lhs.m_obj.bit_offset == rhs.m_obj.bit_offset &&
+   lhs.m_obj.bit_size == rhs.m_obj.bit_size &&
+   lhs.m_obj.base_offset == rhs.m_obj.base_offset &&
+   std::equal(lhs.m_obj.fields.begin(), lhs.m_obj.fields.end(),
+  rhs.m_obj.fields.begin(), rhs.m_obj.fields.end(),
+  [](const MemberUP &lhs, const MemberUP &rhs) {
+return WrappedMember(*lhs) == WrappedMember(*rhs);
+  });
+  }
+
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
+   const WrappedMember &w) {
+os << llvm::formatv("Member{.kind={0}, .name=\"{1}\", .bit_offset={2}, "
+".bit_size={3}, .base_offset={4}, .fields=[",
+w.m_obj.kind, w.m_obj.name, w.m_obj.bit_offset,
+w.m_obj.bit_size, w.m_obj.base_offset);
+llvm::ListSeparator sep;
+for (auto &f : w.m_obj.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
+
+class WrappedRecord {
+public:
+  WrappedRecord(const Record &obj) : m_obj(obj) {}
+
+private:
+  const Record &m_obj;
+
+  friend bool operator==(const WrappedRecord &lhs, const WrappedRecord &rhs) {
+return lhs.m_obj.start_offset == rhs.m_obj.start_offset &&
+   std::equal(
+   lhs.m_obj.record.fields.begin(), lhs.m_obj.record.fields.end(),
+   rhs.m_obj.record.fields.begin(), rhs.m_obj.record.fields.end(),
+   [](const MemberUP &lhs, const MemberUP &rhs) {
+ return WrappedMember(*lhs) == WrappedMember(*rhs);
+   });
+  }
+
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
+   const WrappedRecord &w) {
+os << llvm::formatv("Record{.start_offset={0}, .record.fields=[",
+w.m_obj.start_offset);
+llvm::ListSeparator sep;
+for (const MemberUP &f : w.m_obj.record.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
+
+class UdtRecordCompleterRecordTests : public testing::Test {
+protected:
+  Record record;
+
+public:
+  void SetKind(Member::Kind kind) { record.record.kind = kind; }
+  void CollectMember(StringRef name, uint64_t byte_offset, uint64_t byte_size) {
+record.CollectMember(name, byte_offset * 8, byte_size * 8,
+ clang::QualType(), lldb::eAccessPublic, 0);
+  }
+  void ConstructRecord() { record.ConstructRecord(); }
+};
+Member *AddField(Member *member, StringRef name, uint64_t byte_offset,
+ uint64_t byte_size, Member::Kind kind,
+ uint64_t base_offset = 0) {
+  auto field =
+  std::make_unique(name, byte_offset

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

2022-10-13 Thread Zequan Wu via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd3492ed01667: [LLDB][NativePDB] Fix struct layout when it 
has anonymous unions. (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
  lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt
  lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

Index: lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
===
--- /dev/null
+++ lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -0,0 +1,244 @@
+//===-- UdtRecordCompleterTests.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private::npdb;
+using namespace llvm;
+
+namespace {
+using Member = UdtRecordCompleter::Member;
+using MemberUP = std::unique_ptr;
+using Record = UdtRecordCompleter::Record;
+
+class WrappedMember {
+public:
+  WrappedMember(const Member &obj) : m_obj(obj) {}
+
+private:
+  const Member &m_obj;
+
+  friend bool operator==(const WrappedMember &lhs, const WrappedMember &rhs) {
+return lhs.m_obj.kind == rhs.m_obj.kind &&
+   lhs.m_obj.name == rhs.m_obj.name &&
+   lhs.m_obj.bit_offset == rhs.m_obj.bit_offset &&
+   lhs.m_obj.bit_size == rhs.m_obj.bit_size &&
+   lhs.m_obj.base_offset == rhs.m_obj.base_offset &&
+   std::equal(lhs.m_obj.fields.begin(), lhs.m_obj.fields.end(),
+  rhs.m_obj.fields.begin(), rhs.m_obj.fields.end(),
+  [](const MemberUP &lhs, const MemberUP &rhs) {
+return WrappedMember(*lhs) == WrappedMember(*rhs);
+  });
+  }
+
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
+   const WrappedMember &w) {
+os << llvm::formatv("Member{.kind={0}, .name=\"{1}\", .bit_offset={2}, "
+".bit_size={3}, .base_offset={4}, .fields=[",
+w.m_obj.kind, w.m_obj.name, w.m_obj.bit_offset,
+w.m_obj.bit_size, w.m_obj.base_offset);
+llvm::ListSeparator sep;
+for (auto &f : w.m_obj.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
+
+class WrappedRecord {
+public:
+  WrappedRecord(const Record &obj) : m_obj(obj) {}
+
+private:
+  const Record &m_obj;
+
+  friend bool operator==(const WrappedRecord &lhs, const WrappedRecord &rhs) {
+return lhs.m_obj.start_offset == rhs.m_obj.start_offset &&
+   std::equal(
+   lhs.m_obj.record.fields.begin(), lhs.m_obj.record.fields.end(),
+   rhs.m_obj.record.fields.begin(), rhs.m_obj.record.fields.end(),
+   [](const MemberUP &lhs, const MemberUP &rhs) {
+ return WrappedMember(*lhs) == WrappedMember(*rhs);
+   });
+  }
+
+  friend llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
+   const WrappedRecord &w) {
+os << llvm::formatv("Record{.start_offset={0}, .record.fields=[",
+w.m_obj.start_offset);
+llvm::ListSeparator sep;
+for (const MemberUP &f : w.m_obj.record.fields)
+  os << sep << WrappedMember(*f);
+return os << "]}";
+  }
+};
+
+class UdtRecordCompleterRecordTests : public testing::Test {
+protected:
+  Record record;
+
+public:
+  void SetKind(Member::Kind kind) { record.record.kind = kind; }
+  void CollectMember(StringRef name, uint64_t byte_offset, uint64_t byte_size) {
+record.CollectMember(name, byte_offset * 8, byte_size * 8,
+ clang::QualType(), lldb::eAccessPublic, 0);
+  }
+  void ConstructRecord() { record.ConstructRecord(); }
+};
+Member *AddField(Member *member, StringRef name, uint64_t byte_offset,
+ uint64_t byte_size, Mem

[Lldb-commits] [lldb] d3492ed - [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

2022-10-13 Thread Zequan Wu via lldb-commits 

Author: Zequan Wu
Date: 2022-10-13T12:43:45-07:00
New Revision: d3492ed01667a1fdebff4421a83b6c0500f07348

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

LOG: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

Previously, lldb mistook fields in anonymous union in a struct as the direct
field of the struct, which causes lldb crashes due to multiple fields sharing
the same offset in a struct. This patch fixes it.

MSVC generated pdb doesn't have the debug info entity representing a anonymous
union in a struct. It looks like the following:
```
struct S {
union {
  char c;
  int i;
};
};

0x1004 | LF_FIELDLIST [size = 40]
 - LF_MEMBER [name = `c`, Type = 0x0070 (char), offset = 0, attrs = 
public]
 - LF_MEMBER [name = `i`, Type = 0x0074 (int), offset = 0, attrs = 
public]
0x1005 | LF_STRUCTURE [size = 32] `S`
 unique name: `.?AUS@@`
 vtable: , base list: , field list: 0x1004
```
Clang generated pdb is similar, though due to the [[ 
https://github.com/llvm/llvm-project/issues/57999 | bug ]],
it's not more useful than the debug info above. But that's not very relavent,
lldb should still be able to understand MSVC geneerated pdb.
```
0x1003 | LF_UNION [size = 60] `S::`
 unique name: `.?AT@S@@`
 field list: 
 options: forward ref (= 0x1003) | has unique name | is nested, sizeof 0
0x1004 | LF_FIELDLIST [size = 40]
 - LF_MEMBER [name = `c`, Type = 0x0070 (char), offset = 0, attrs = 
public]
 - LF_MEMBER [name = `i`, Type = 0x0074 (int), offset = 0, attrs = 
public]
 - LF_NESTTYPE [name = ``, parent = 0x1003]
0x1005 | LF_STRUCTURE [size = 32] `S`
 unique name: `.?AUS@@`
 vtable: , base list: , field list: 0x1004
 options: contains nested class | has unique name, sizeof 4
0x1006 | LF_FIELDLIST [size = 28]
 - LF_MEMBER [name = `c`, Type = 0x0070 (char), offset = 0, attrs = 
public]
 - LF_MEMBER [name = `i`, Type = 0x0074 (int), offset = 0, attrs = 
public]
0x1007 | LF_UNION [size = 60] `S::`
 unique name: `.?AT@S@@`
 field list: 0x1006
 options: has unique name | is nested | sealed, sizeof
```
This patch delays the FieldDecl creation when travesing LF_FIELDLIST so we know
if there are multiple fields are in the same offsets and are able to group them
into different anonymous unions based on offsets. Nested anonymous union will
be flatten into one anonymous union, because we simply don't have that info, but
they are equivalent in terms of union layout.

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

Added: 
lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit
lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp
lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt

Removed: 
lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp



diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 2e17aa1260a83..fb27d0ae05c33 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -477,7 +477,7 @@ bool PdbAstBuilder::CompleteTagDecl(clang::TagDecl &tag) {
   // Visit all members of this class, then perform any finalization necessary
   // to complete the class.
   CompilerType ct = ToCompilerType(tag_qt);
-  UdtRecordCompleter completer(best_ti, ct, tag, *this, index,
+  UdtRecordCompleter completer(best_ti, ct, tag, *this, index, 
m_decl_to_status,
m_cxx_record_map);
   llvm::Error error =
   llvm::codeview::visitMemberRecordStream(field_list.Data, completer);

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
index 695bb85e2b464..164bfa8b3726a 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
@@ -1104,6 +1104,11 @@ size_t lldb_private::npdb::GetSizeOfType(PdbTypeSymId id,
 return GetSizeOfTypeInternal(cvt);
   case LF_UNION:
 return GetSizeOfTypeInternal(cvt);
+  case LF_BITFIELD: {
+BitFieldRecord record;
+llvm::cantFail(TypeDeserializer::dese

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

2022-10-13 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

Looking pretty good to me FWIW




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1561
+std::string
+DWARFASTParserClang::GetCPlusPlusQualifiedName(const DWARFDIE &die) {
+  if (!die.IsValid())

Sorry, when I gave feedback on some pieces of this that were just moved around 
rather than new code written in this review - I don't mind the code moving 
around without changes (and optionally before/after the move making 
improvements, but not necessary).

If possible, might simplify the code review to move the code first/separately 
from this change, if the move isn't too meaningless independent of the rest of 
the changes?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2498-2501
+llvm::StringRef qualNameTemplateParams =
+qualName.slice(it, qualName.size());
+if (templateParams != qualNameTemplateParams)
+  return true;

aeubanks wrote:
> dblaikie wrote:
> > And this checks the stringified template params match exactly - so there's 
> > opportunity for improvement in the future to compare with more fuzzy 
> > matching (I guess we can't use clang's AST because that'd involve making 
> > two instances of the type somehow which doesn't make a lot of sense) so 
> > that users don't have to spell the template spec exactly the same way clang 
> > does (eg: `x` versus `x` - or other more complicated 
> > situations with function pointers, parentheses, casts, etc.
> lldb already requires exact name matching when looking up classes
> 
> e.g.
> ```
> $ cat /tmp/a.cc
> templatestruct Foo {};
> int main() {
> Foo a;
> Foo b;
> Foo c;
> }
> templatestruct Foo {};
> int main() {
> Foo a;
> Foo b;
> Foo c;
> }
> ~/repos/llvm-project/build/cmake$ ./bin/lldb /tmp/a
> (lldb) target create "/tmp/a"
> Current executable set to '/tmp/a' (x86_64).
> (lldb) im loo -t 'Foo< int>'
> (lldb) im loo -t 'Foo'
> 1 match found in /tmp/a:
> id = {0x0058}, name = "Foo", byte-size = 1, decl = a.cc:1, 
> compiler_type = "template<> struct Foo {
> }"
> ```
Yeah, sorry - I understand it requires exact matching currently (because the 
index has the exact string in it) - my comment was forward-looking/idle thought 
that now that with simplified template names we can/have to lookup by base 
name, we have the option to do fuzzier matching when filtering all the base 
name matches - not suggesting that it's a regression that this currently does 
exact matching.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D135825: [LLDB] Only run lldb-server Shell tests if it gets built

2022-10-13 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord updated this revision to Diff 467587.
bulbazord added a comment.

Implement David's suggestion


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

https://reviews.llvm.org/D135825

Files:
  lldb/test/CMakeLists.txt
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/test/Shell/lldb-server/lit.local.cfg


Index: lldb/test/Shell/lldb-server/lit.local.cfg
===
--- /dev/null
+++ lldb/test/Shell/lldb-server/lit.local.cfg
@@ -0,0 +1,3 @@
+# These tests rely on lldb-server
+if not config.have_lldb_server:
+config.unsupported = True
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@
 config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", 
"lldb-shell")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -132,6 +132,9 @@
 if config.lldb_system_debugserver:
 config.available_features.add('system-debugserver')
 
+if config.have_lldb_server:
+config.available_features.add('lldb-server')
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -167,6 +167,7 @@
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_HAS_LIBCXX
+  LLDB_TOOL_LLDB_SERVER_BUILD
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 


Index: lldb/test/Shell/lldb-server/lit.local.cfg
===
--- /dev/null
+++ lldb/test/Shell/lldb-server/lit.local.cfg
@@ -0,0 +1,3 @@
+# These tests rely on lldb-server
+if not config.have_lldb_server:
+config.unsupported = True
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -22,6 +22,7 @@
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.lldb_enable_lua = @LLDB_ENABLE_LUA@
 config.lldb_build_directory = "@LLDB_TEST_BUILD_DIRECTORY@"
+config.have_lldb_server = @LLDB_TOOL_LLDB_SERVER_BUILD@
 config.lldb_system_debugserver = @LLDB_USE_SYSTEM_DEBUGSERVER@
 # The shell tests use their own module caches.
 config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-shell")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -132,6 +132,9 @@
 if config.lldb_system_debugserver:
 config.available_features.add('system-debugserver')
 
+if config.have_lldb_server:
+config.available_features.add('lldb-server')
+
 # NetBSD permits setting dbregs either if one is root
 # or if user_set_dbregs is enabled
 can_set_dbregs = True
Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -167,6 +167,7 @@
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_HAS_LIBCXX
+  LLDB_TOOL_LLDB_SERVER_BUILD
   LLDB_USE_SYSTEM_DEBUGSERVER
   LLDB_IS_64_BITS)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135917: [lldb][trace] Add a basic function call dump [2] - Implement the reconstruction algorithm

2022-10-13 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
wallace added reviewers: jj10306, persona0220.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This diff implements the reconstruction algorithm for the call tree and
add tests.

See TraceDumper.h for documentation and explanations.

One important detail is that the tree objects are in TraceDumper, even
though Trace.h is a better home. I'm leaving that as future work.

Another detail is that this code is as slow as dumping the entire
symolicated trace, which is not that bad tbh. The reason is that we use
symbols throughout the algorithm and we are not being careful about
memory and speed. This is also another area for future improvement.

Lastly, I made sure that incomplete traces work, i.e. you start tracing
very deep in the stack or failures randomly appear in the trace.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135917

Files:
  lldb/include/lldb/Target/TraceDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Target/TraceDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/inline-function/a.out
  lldb/test/API/commands/trace/inline-function/inline.cpp

Index: lldb/test/API/commands/trace/inline-function/inline.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/inline-function/inline.cpp
@@ -0,0 +1,18 @@
+__attribute__((always_inline)) inline int mult(int x, int y) {
+  int f = x * y;
+  f++;
+  f *= f;
+  return f;
+}
+
+int foo(int x) {
+  int z = mult(x, x - 1);
+  z++;
+  return z;
+}
+
+int main() {
+  int x = 12;
+  int z = foo(x);
+  return z + x;
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -68,8 +68,8 @@
   "individualCounts": {
 "software disabled tracing": 1,
 "trace synchronization point": 1,
-"HW clock tick": 8,
-"CPU core changed": 1
+"CPU core changed": 1,
+"HW clock tick": 8
   }
 },
 "continuousExecutions": 1,
Index: lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
===
--- lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
+++ lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
@@ -3,15 +3,103 @@
 from lldbsuite.test.decorators import *
 
 class TestTraceDumpInfo(TraceIntelPTTestCaseBase):
-def testDumpFunctionCalls(self):
+def testDumpSimpleFunctionCalls(self):
   self.expect("trace load -v " +
 os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
 
   self.expect("thread trace dump function-calls 2",
 error=True, substrs=['error: no thread with index: "2"'])
 
-  self.expect("thread trace dump function-calls 1 -j",
-substrs=['json = true, pretty_json = false, file = false, thread = 3842849'])
+  # We don't support yet JSON
+  self.expect("thread trace dump function-calls 1 -j", substrs=['[]'])
 
-  self.expect("thread trace dump function-calls 1 -F /tmp -J",
-substrs=['false, pretty_json = true, file = true, thread = 3842849'])
+  # We test first some code without function call
+  self.expect("thread trace dump function-calls 1",
+substrs=['''thread #1: tid = 3842849
+
+[call tree #0]
+a.out`main + 4 at main.cpp:2 to 4:0  [1, 22]'''])
+
+def testFunctionCallsWithErrors(self):
+  self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-multi-core-trace", "trace.json"))
+
+  # We expect that tracing errors appear as a different tree
+  self.expect("thread trace dump function-calls 2",
+substrs=['''thread #2: tid = 3497496
+
+[call tree #0]
+m.out`foo() + 65 at multi_thread.cpp:12:21 to 12:21  [4, 19524]
+
+[call tree #1]
+  [19532, 19532]'''])
+  self.expect("thread trace dump function-calls 3",
+substrs=['''thread #3: tid = 3497497
+
+[call tree #0]
+m.out`bar() + 30 at multi_thread.cpp:19:3 to 20:6  [5, 61831]
+
+[call tree #1]
+  [61834, 61834]'''])
+
+def testInlineFunctionCalls(self):
+  self.expect("file " + os.path.join(self.getSourceDir(), "inline-function", "a.out"))
+  self.expect("b main") # we'll trace from the beginning of main
+  self.expect("b 17")
+  self.expect("r")
+  self.expect("thread trace start")
+  self.expect("c")
+  self.expect("thread trace dump function-calls",
+substrs=['''[call tree #0]
+a.out`main + 8 at inline.cpp:15:7 to 16:14  [1, 5]
+  a.out`foo(int) at inline.cpp:8:16 to 9:15  [6, 13]
+a.out`foo(int) + 22 [inlined] mult(int, int) at inline.cpp:2:7 to 5:10  [14, 21]
+  a.out`foo(int) + 49 at inline.cp

[Lldb-commits] [PATCH] D135917: [lldb][trace] Add a basic function call dump [2] - Implement the reconstruction algorithm

2022-10-13 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 467605.
wallace added a comment.

nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135917

Files:
  lldb/include/lldb/Target/TraceDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Target/TraceDumper.cpp
  lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  lldb/test/API/commands/trace/inline-function/a.out
  lldb/test/API/commands/trace/inline-function/inline.cpp

Index: lldb/test/API/commands/trace/inline-function/inline.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/inline-function/inline.cpp
@@ -0,0 +1,18 @@
+__attribute__((always_inline)) inline int mult(int x, int y) {
+  int f = x * y;
+  f++;
+  f *= f;
+  return f;
+}
+
+int foo(int x) {
+  int z = mult(x, x - 1);
+  z++;
+  return z;
+}
+
+int main() {
+  int x = 12;
+  int z = foo(x);
+  return z + x;
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -68,8 +68,8 @@
   "individualCounts": {
 "software disabled tracing": 1,
 "trace synchronization point": 1,
-"HW clock tick": 8,
-"CPU core changed": 1
+"CPU core changed": 1,
+"HW clock tick": 8
   }
 },
 "continuousExecutions": 1,
Index: lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
===
--- lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
+++ lldb/test/API/commands/trace/TestTraceDumpFunctionCalls.py
@@ -3,15 +3,106 @@
 from lldbsuite.test.decorators import *
 
 class TestTraceDumpInfo(TraceIntelPTTestCaseBase):
-def testDumpFunctionCalls(self):
+def testDumpSimpleFunctionCalls(self):
   self.expect("trace load -v " +
 os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
 
   self.expect("thread trace dump function-calls 2",
 error=True, substrs=['error: no thread with index: "2"'])
 
-  self.expect("thread trace dump function-calls 1 -j",
-substrs=['json = true, pretty_json = false, file = false, thread = 3842849'])
+  # We don't support yet JSON
+  self.expect("thread trace dump function-calls 1 -j", substrs=['[]'])
 
-  self.expect("thread trace dump function-calls 1 -F /tmp -J",
-substrs=['false, pretty_json = true, file = true, thread = 3842849'])
+  # We test first some code without function call
+  self.expect("thread trace dump function-calls 1",
+substrs=['''thread #1: tid = 3842849
+
+[call tree #0]
+a.out`main + 4 at main.cpp:2 to 4:0  [1, 22]'''])
+
+def testFunctionCallsWithErrors(self):
+  self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-multi-core-trace", "trace.json"))
+
+  # We expect that tracing errors appear as a different tree
+  self.expect("thread trace dump function-calls 2",
+substrs=['''thread #2: tid = 3497496
+
+[call tree #0]
+m.out`foo() + 65 at multi_thread.cpp:12:21 to 12:21  [4, 19524]
+
+[call tree #1]
+  [19532, 19532]'''])
+  self.expect("thread trace dump function-calls 3",
+substrs=['''thread #3: tid = 3497497
+
+[call tree #0]
+m.out`bar() + 30 at multi_thread.cpp:19:3 to 20:6  [5, 61831]
+
+[call tree #1]
+  [61834, 61834]'''])
+
+def testInlineFunctionCalls(self):
+  self.expect("file " + os.path.join(self.getSourceDir(), "inline-function", "a.out"))
+  self.expect("b main") # we'll trace from the beginning of main
+  self.expect("b 17")
+  self.expect("r")
+  self.expect("thread trace start")
+  self.expect("c")
+  self.expect("thread trace dump function-calls",
+substrs=['''[call tree #0]
+a.out`main + 8 at inline.cpp:15:7 to 16:14  [1, 5]
+  a.out`foo(int) at inline.cpp:8:16 to 9:15  [6, 13]
+a.out`foo(int) + 22 [inlined] mult(int, int) at inline.cpp:2:7 to 5:10  [14, 21]
+  a.out`foo(int) + 49 at inline.cpp:9:15 to 12:1  [22, 26]
+a.out`main + 25 at inline.cpp:16:14 to 16:14  [27, 27]'''])
+
+def testIncompleteInlineFunctionCalls(self):
+  self.expect("file " + os.path.join(self.getSourceDir(), "inline-function", "a.out"))
+  self.expect("b 4") # we'll trace from the middle of the inline method
+  self.expect("b 17")
+  self.expect("r")
+  self.expect("thread trace start")
+  self.expect("c")
+  self.expect("thread trace dump function-calls",
+substrs=['''[call tree #0]
+a.out`main
+  a.out`foo(int)
+a.out`foo(int) + 36 [inlined] mult(int, int) + 14 at inline.cpp:4:5 to 5:10  [1, 5]
+  a.out`foo(int) + 49 at inline.cpp:9:15 to 12:1  [6, 10]
+a.out`main + 25 at inline.cpp:16:14 to 16:14  [11, 11]'''])
+
+def test

[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-13 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 created this revision.
Michael137 added reviewers: aprantl, labath, jingham.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch fixes a regression with setting breakpoints on template
functions by name. E.g.,:

  $ cat main.cpp
  template
  struct Foo {
template
void func() {}
  };
  
  int main() {
Foo f;
f.func();
  }
  
  (lldb) br se -n func

This has regressed since `3339000e0bda696c2e29173d15958c0a4978a143`
where we started using the `CPlusPlusNameParser` for getting the
basename of the function symbol and match it exactly against
the name in the breakpoint command. The parser will include template
parameters in the basename, so the exact match will always fail

**Testing**

- Added API tests
- Added unit-tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135921

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/test/API/functionalities/breakpoint/cpp/Makefile
  lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
  lldb/test/API/functionalities/breakpoint/cpp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -143,7 +143,11 @@
   CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()"));
   CPlusPlusLanguage::MethodName 
   reference_4(ConstString("bar::baz::operator bool()"));
-  
+  CPlusPlusLanguage::MethodName reference_5(
+  ConstString("bar::baz::operator bool>()"));
+  CPlusPlusLanguage::MethodName reference_6(ConstString(
+  "bar::baz::operator<<, Type>>()"));
+
   EXPECT_TRUE(reference_1.ContainsPath("func01"));
   EXPECT_TRUE(reference_1.ContainsPath("bar::func01"));
   EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01"));
@@ -160,10 +164,23 @@
   EXPECT_FALSE(reference_3.ContainsPath("func"));
   EXPECT_FALSE(reference_3.ContainsPath("bar::func01"));
 
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("baz::operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("bar::baz::operator bool"));
   EXPECT_FALSE(reference_4.ContainsPath("az::operator bool"));
+
+  EXPECT_TRUE(reference_5.ContainsPath("operator"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool>"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool>"));
+
+  EXPECT_TRUE(reference_6.ContainsPath("operator"));
+  EXPECT_TRUE(reference_6.ContainsPath("operator<<"));
+  EXPECT_TRUE(reference_6.ContainsPath(
+  "bar::baz::operator<<, Type>>()"));
+  EXPECT_FALSE(reference_6.ContainsPath("operator<<>"));
 }
 
 TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
Index: lldb/test/API/functionalities/breakpoint/cpp/main.cpp
===
--- lldb/test/API/functionalities/breakpoint/cpp/main.cpp
+++ lldb/test/API/functionalities/breakpoint/cpp/main.cpp
@@ -82,6 +82,20 @@
 };
 }
 
+namespace ns {
+template  struct Foo {
+  template  void import() {}
+
+  template  auto func() {}
+
+  operator bool() { return true; }
+
+  template  operator T() { return {}; }
+
+  template  void operator<<(T t) {}
+};
+} // namespace ns
+
 int main (int argc, char const *argv[])
 {
 a::c ac;
@@ -98,5 +112,16 @@
 bc.func3();
 cd.func2();
 cd.func3();
+
+ns::Foo f;
+f.import ();
+f.func();
+f.func>();
+f.operator bool();
+f.operator a::c();
+f.operator ns::Foo();
+f.operator<<(5);
+f.operator<< >({});
+
 return 0;
 }
Index: lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
===
--- lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
+++ lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
@@ -54,6 +54,28 @@
 {'name': 'a::c::func1()', 'loc_names': ['a::c::func1()']},
 {'name': 'b::c::func1()', 'loc_names': ['b::c::func1()']},
 {'name': 'c::d::func2()', 'loc_names': ['c::d::func2()']},
+
+# Template cases
+{'name': 'func', 'loc_names': []},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()']},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()',
+   'auto ns::Foo::func>()']},
+# {'name': 'func>', 'loc_names': ['auto ns::Foo::func>()']}, # FIXME
+
+{'name'

[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-13 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py:63
+   'auto 
ns::Foo::func>()']},
+# {'name': 'func>', 'loc_names': ['auto 
ns::Foo::func>()']}, # FIXME
+

These didn't work before this patch (or in lldb-14) either. So may xfail them 
for now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

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

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

2022-10-13 Thread J. Ryan Stinnett via Phabricator via lldb-commits 
jryans added a comment.

No, once it has been committed, it can’t really be removed from a project of 
this size. Changes can be reverted, but the existence of the commits and 
messages remains. Don’t worry about it for this one, just something to think 
about next time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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

[Lldb-commits] [PATCH] D115324: Added the ability to cache the finalized symbol tables subsequent debug sessions to start faster.

2022-10-13 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

Been experimenting with this recently and I noticed that loading in the cached 
indexes seems to do a lot of loading - specifically interning a lot of strings 
from the index and the symtab. Does this happen when reading a built-in index 
(apple_names/debug_names) (I don't have an immediately easy way to test this, 
or I Would've before asking)? I'd be surprised if that was the case, which is 
also confusing me as to why it's the case for these cached indexes? I'd have 
expected the cached index to look basically the same as the 
apple_names/debug_names builtin index and have similar performance properties, 
but maybe that's not the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115324

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

[Lldb-commits] [PATCH] D115324: Added the ability to cache the finalized symbol tables subsequent debug sessions to start faster.

2022-10-13 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

In D115324#3857081 , @dblaikie wrote:

> Been experimenting with this recently and I noticed that loading in the 
> cached indexes seems to do a lot of loading - specifically interning a lot of 
> strings from the index and the symtab. Does this happen when reading a 
> built-in index (apple_names/debug_names) (I don't have an immediately easy 
> way to test this, or I Would've before asking)? I'd be surprised if that was 
> the case, which is also confusing me as to why it's the case for these cached 
> indexes? I'd have expected the cached index to look basically the same as the 
> apple_names/debug_names builtin index and have similar performance 
> properties, but maybe that's not the case?

Yeah, the caches are currently designed to just serialize the cache to disc and 
allow it to be loaded into the same data structure as is used when the cache 
isn't used.

The DWARF indexes are more efficient where there is a base class that 
represents the cache, and the manual index will create its own data structures 
and then do lookups using the base class' API. It allows the API calls to do 
the lookups as efficiently as possible without interning any strings. An 
improvement to this caching could do the same kind of thing, but that isn't 
what we have right now. We would need to continue to serialize/deserialize the 
symbol table, but the symbol table index and debug info index could be modified 
to indirect the cache lookups through a base class so the serialized indexes 
could be made more efficient.

The index cache is purely a serialize/deserialize of the same cache structure 
that is used when manually parsing and indexing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115324

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

[Lldb-commits] [PATCH] D135921: [WIP][lldb][Breakpoint] Fix setting breakpoints on templates by basename

2022-10-13 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 updated this revision to Diff 467629.
Michael137 added a comment.

- Fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135921

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/test/API/functionalities/breakpoint/cpp/Makefile
  lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
  lldb/test/API/functionalities/breakpoint/cpp/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -143,7 +143,11 @@
   CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()"));
   CPlusPlusLanguage::MethodName 
   reference_4(ConstString("bar::baz::operator bool()"));
-  
+  CPlusPlusLanguage::MethodName reference_5(
+  ConstString("bar::baz::operator bool>()"));
+  CPlusPlusLanguage::MethodName reference_6(ConstString(
+  "bar::baz::operator<<, Type>>()"));
+
   EXPECT_TRUE(reference_1.ContainsPath("func01"));
   EXPECT_TRUE(reference_1.ContainsPath("bar::func01"));
   EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01"));
@@ -160,10 +164,23 @@
   EXPECT_FALSE(reference_3.ContainsPath("func"));
   EXPECT_FALSE(reference_3.ContainsPath("bar::func01"));
 
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("baz::operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("bar::baz::operator bool"));
   EXPECT_FALSE(reference_4.ContainsPath("az::operator bool"));
+
+  EXPECT_TRUE(reference_5.ContainsPath("operator"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool>"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool>"));
+
+  EXPECT_TRUE(reference_6.ContainsPath("operator"));
+  EXPECT_TRUE(reference_6.ContainsPath("operator<<"));
+  EXPECT_TRUE(reference_6.ContainsPath(
+  "bar::baz::operator<<, Type>>()"));
+  EXPECT_FALSE(reference_6.ContainsPath("operator<<>"));
 }
 
 TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
Index: lldb/test/API/functionalities/breakpoint/cpp/main.cpp
===
--- lldb/test/API/functionalities/breakpoint/cpp/main.cpp
+++ lldb/test/API/functionalities/breakpoint/cpp/main.cpp
@@ -82,6 +82,20 @@
 };
 }
 
+namespace ns {
+template  struct Foo {
+  template  void import() {}
+
+  template  auto func() {}
+
+  operator bool() { return true; }
+
+  template  operator T() { return {}; }
+
+  template  void operator<<(T t) {}
+};
+} // namespace ns
+
 int main (int argc, char const *argv[])
 {
 a::c ac;
@@ -98,5 +112,16 @@
 bc.func3();
 cd.func2();
 cd.func3();
+
+ns::Foo f;
+f.import ();
+f.func();
+f.func>();
+f.operator bool();
+f.operator a::c();
+f.operator ns::Foo();
+f.operator<<(5);
+f.operator<< >({});
+
 return 0;
 }
Index: lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
===
--- lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
+++ lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
@@ -54,6 +54,28 @@
 {'name': 'a::c::func1()', 'loc_names': ['a::c::func1()']},
 {'name': 'b::c::func1()', 'loc_names': ['b::c::func1()']},
 {'name': 'c::d::func2()', 'loc_names': ['c::d::func2()']},
+
+# Template cases
+{'name': 'func', 'loc_names': []},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()']},
+{'name': 'func', 'loc_names': ['auto ns::Foo::func()',
+   'auto ns::Foo::func>()']},
+# {'name': 'func>', 'loc_names': ['auto ns::Foo::func>()']}, # FIXME
+
+{'name': 'operator', 'loc_names': []},
+{'name': 'ns::Foo::operator bool', 'loc_names': ['ns::Foo::operator bool()']},
+
+{'name': 'operator a::c', 'loc_names': ['ns::Foo::operator a::c()']},
+{'name': 'operator ns::Foo', 'loc_names': ['ns::Foo::operator ns::Foo>()']},
+
+# FIXME: Currently doesn't work because 'import' is seen as a reserved keyword by the CPlusPlusNameParser
+# {'name': 'import', 'loc_names': [auto ns::Foo::import>()]},
+
+{'name': 'operator<<', 'loc_names': []},
+{'name': 'operator<<', 'loc_names': ['void ns::Foo::operator<<(int)']},
+{'name': 'ns::Foo::operator<<', 'loc_names': ['void n

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

2022-10-13 Thread Henrique Bucher via Phabricator via lldb-commits 
HenriqueBucher added a comment.

I mean my **local** changes, can I consider this done and clean up? 
Sorry for the very newbie question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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

[Lldb-commits] [PATCH] D135577: Summary: This documentation patch adds information to allow remote users to also use the plugin as it will be invisible to them using the current instructions. It solve

2022-10-13 Thread J. Ryan Stinnett via Phabricator via lldb-commits 
jryans added a comment.

Ah yes, it’s all done now, feel free to clean up your local state however you 
prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135577

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

[Lldb-commits] [lldb] 3d8d9c9 - PlatformDarwinKernel calls the ctor directly, not setting no-jit

2022-10-13 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2022-10-13T16:28:18-07:00
New Revision: 3d8d9c9884db2b2a0fbd518990019b24de6f72d8

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

LOG: PlatformDarwinKernel calls the ctor directly, not setting no-jit

Fix a small thinko in https://reviews.llvm.org/D133534 .  Normally
DynamicLoaderDarwinKernels are created via the CreateInstance plugin
method, and that plugin method sets the Process CanJIT to false.
In the above patch, I added a new code path that can call the
DynamicLoaderDarwinKernel ctor directly, without going through
CreateInstance, and CanJIT was not being correctly set for the
process.

rdar://101148552

Added: 


Modified: 

lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
index b0617ce3159c7..f1b30477b89c3 100644
--- 
a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ 
b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -186,7 +186,6 @@ DynamicLoader 
*DynamicLoaderDarwinKernel::CreateInstance(Process *process,
   // the kernel load address, we need to look around in memory to find it.
   const addr_t kernel_load_address = SearchForDarwinKernel(process);
   if (CheckForKernelImageAtAddress(kernel_load_address, process).IsValid()) {
-process->SetCanRunCode(false);
 return new DynamicLoaderDarwinKernel(process, kernel_load_address);
   }
   return nullptr;
@@ -509,6 +508,7 @@ 
DynamicLoaderDarwinKernel::DynamicLoaderDarwinKernel(Process *process,
   m_kext_summary_header(), m_known_kexts(), m_mutex(),
   m_break_id(LLDB_INVALID_BREAK_ID) {
   Status error;
+  process->SetCanRunCode(false);
   PlatformSP platform_sp =
   process->GetTarget().GetDebugger().GetPlatformList().Create(
   PlatformDarwinKernel::GetPluginNameStatic());



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

[Lldb-commits] [PATCH] D134842: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

2022-10-13 Thread Greg Clayton via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc338516463ff: Improve dynamic loader support in 
DynamicLoaderPOSIXDYLD when using core files. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134842

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -91,6 +91,9 @@
   std::map>
   m_loaded_modules;
 
+  /// Returns true if the process is for a core file.
+  bool IsCoreFile() const;
+
   /// If possible sets a breakpoint on a function called by the runtime
   /// linker each time a module is loaded or unloaded.
   bool SetRendezvousBreakpoint();
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
@@ -213,6 +213,10 @@
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return;
+
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
 LLDB_LOGF(
@@ -297,6 +301,11 @@
 
 bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
+
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return false;
+
   if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
 LLDB_LOG(log,
  "Rendezvous breakpoint breakpoint id {0} for pid {1}"
@@ -829,3 +838,7 @@
 
   return module_sp->GetFileSpec().GetPath() == "[vdso]";
 }
+
+bool DynamicLoaderPOSIXDYLD::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -267,6 +267,8 @@
 
   bool FindMetadata(const char *name, PThreadField field, uint32_t &value);
 
+  bool IsCoreFile() const;
+
   enum RendezvousAction {
 eNoAction,
 eTakeSnapshot,
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -190,6 +190,14 @@
 }
 
 DYLDRendezvous::RendezvousAction DYLDRendezvous::GetAction() const {
+  // If we have a core file, we will read the current rendezvous state
+  // from the core file's memory into m_current which can be in an inconsistent
+  // state, so we can't rely on its state to determine what we should do. We
+  // always need it to load all of the shared libraries one time when we attach
+  // to a core file.
+  if (IsCoreFile())
+return eTakeSnapshot;
+
   switch (m_current.state) {
 
   case eConsistent:
@@ -664,3 +672,7 @@
 LLDB_LOGF(log, "  Prev : %" PRIx64, I->prev);
   }
 }
+
+bool DYLDRendezvous::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
@@ -91,6 +91,9 @@
   std::map>
   m_loaded_modules;
 
+  /// Returns true if the process is for a core file.
+  bool IsCoreFile() const;
+
   /// If possible sets a breakpoint on a function called by the runtime
   /// linker each time a module is loaded or unloaded.
   bool SetRendezvousBreakpoint();
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
@@ -213,6 +213,10 @@
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return;
+
   const addr_t entry = GetEntryPoint();
   if (

[Lldb-commits] [lldb] c338516 - Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

2022-10-13 Thread Greg Clayton via lldb-commits 

Author: Greg Clayton
Date: 2022-10-13T17:40:23-07:00
New Revision: c338516463ff9bb7183c8322847f2eddf6febec9

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

LOG: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core 
files.

Prior to this fix, no shared libraries would be loaded for a core file, even if 
they exist on the current machine. The issue was the DYLDRendezvous would read 
a DYLDRendezvous::Rendezvous from memory of the process in 
DYLDRendezvous::Resolve() which would read some ld.so structures as they 
existed in the middle of a process' lifetime. In core files we see, the 
DYLDRendezvous::Rendezvous::state would be set to eAdd for running processes. 
When ProcessELFCore.cpp would load the core file, it would call 
DynamicLoaderPOSIXDYLD::DidAttach(), which would call the above Rendezvous 
functions. The issue came when during the DidAttach function it call 
DYLDRendezvous::GetAction() which would return eNoAction if the 
DYLDRendezvous::m_current.state was read from memory as eAdd. This caused no 
shared libraries to be loaded for any ELF core files. We now detect if we have 
a core file and after reading the DYLDRendezvous::m_current.state from memory 
we set it to eConsistent, which causes DYLDRendezvous::GetAction() to return 
the correct action of eTakeSnapshot and shared libraries get loaded.

We also improve the DynamicLoaderPOSIXDYLD class to not try and set any 
breakpoints to catch shared library loads/unloads when we have a core file, 
which saves a bit of time.

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index 9e7baae8a85a6..f20167b46d270 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -190,6 +190,14 @@ bool DYLDRendezvous::IsValid() {
 }
 
 DYLDRendezvous::RendezvousAction DYLDRendezvous::GetAction() const {
+  // If we have a core file, we will read the current rendezvous state
+  // from the core file's memory into m_current which can be in an inconsistent
+  // state, so we can't rely on its state to determine what we should do. We
+  // always need it to load all of the shared libraries one time when we attach
+  // to a core file.
+  if (IsCoreFile())
+return eTakeSnapshot;
+
   switch (m_current.state) {
 
   case eConsistent:
@@ -664,3 +672,7 @@ void DYLDRendezvous::DumpToLog(Log *log) const {
 LLDB_LOGF(log, "  Prev : %" PRIx64, I->prev);
   }
 }
+
+bool DYLDRendezvous::IsCoreFile() const {
+  return !m_process->IsLiveDebugSession();
+}

diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
index 04d3e665f8598..fc1dd6921455b 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
@@ -267,6 +267,8 @@ class DYLDRendezvous {
 
   bool FindMetadata(const char *name, PThreadField field, uint32_t &value);
 
+  bool IsCoreFile() const;
+
   enum RendezvousAction {
 eNoAction,
 eTakeSnapshot,

diff  --git 
a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index ee03775d230c1..27d2044d7285a 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -213,6 +213,10 @@ void DynamicLoaderPOSIXDYLD::UnloadSections(const ModuleSP 
module) {
 void DynamicLoaderPOSIXDYLD::ProbeEntry() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return;
+
   const addr_t entry = GetEntryPoint();
   if (entry == LLDB_INVALID_ADDRESS) {
 LLDB_LOGF(
@@ -297,6 +301,11 @@ bool DynamicLoaderPOSIXDYLD::EntryBreakpointHit(
 
 bool DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
+
+  // If we have a core file, we don't need any breakpoints.
+  if (IsCoreFile())
+return false;
+
   if (m_dyld_bid != LLDB_INVALID_BREAK_ID) {
 LLDB_LOG(log,
  "Rendezvous breakpoint breakpoint id {0} for pid {1}"
@@ -829,3 +838,7 @@ bool DynamicLoaderPOSIXDYLD::Always

[Lldb-commits] [PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-13 Thread Chuanqi Xu via Phabricator via lldb-commits 
ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

The codes look good and also the tests pass. So LGTM. Thanks!




Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1082-1094
+auto merge = [this, &Redecl, FD](auto &&F) {
+  auto *Existing = 
cast_or_null(Redecl.getKnownMergeTarget());
+  RedeclarableResult NewRedecl(Existing ? F(Existing) : nullptr,
+   Redecl.getFirstID(), Redecl.isKeyDecl());
+  mergeRedeclarableTemplate(F(FD), NewRedecl);
+};
+if (Kind == FunctionDecl::TK_FunctionTemplate)

nit: I feel it lack a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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