[Lldb-commits] [PATCH] D105741: [trace] Add `thread trace export` command for Intel PT trace visualization

[walter erquinigo via Phabricator via lldb-commits]

wallace accepted this revision.
wallace added a comment.

Nice! I'll land this for you


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

https://reviews.llvm.org/D105741

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

[Lldb-commits] [PATCH] D105180: [lldb][AArch64] Add memory tag writing to lldb-server

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

@DavidSpickett Lets merge this patch series (if it is working on your desk) but 
we should add information in LLDB wiki or any other appropriate place about how 
to reliably test mte feature in absence of hardware on desk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105180

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

[Lldb-commits] [PATCH] D105180: [lldb][AArch64] Add memory tag writing to lldb-server

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Great.

The QEMU testing docs https://lldb.llvm.org/use/qemu-testing.html include MTE 
options. Apart from having a compatible toolchain there's no difference to SVE.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105180

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

[Lldb-commits] [PATCH] D105180: [lldb][AArch64] Add memory tag writing to lldb-server

[David Spickett via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d27230de333: [lldb][AArch64] Add memory tag writing to 
lldb-server (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105180

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

Index: lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
===
--- lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
+++ lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
@@ -3,27 +3,40 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+"""
+Check that lldb-server correctly processes qMemTags and QMemTags packets.
+
+In the tests below E03 means the packet wasn't formed correctly
+and E01 means it was but we had some other error acting upon it.
+
+We do not test reading or writing over a page boundary
+within the same mapping. That logic is handled in the kernel
+so it's just a single ptrace call for lldb-server.
+"""
+
 class TestGdbRemoteMemoryTagging(gdbremote_testcase.GdbRemoteTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-def check_qmemtags_response(self, body, expected):
-self.test_sequence.add_log_lines(["read packet: $qMemTags:{}#00".format(body),
+def check_memtags_response(self, packet_name, body, expected):
+self.test_sequence.add_log_lines(["read packet: ${}:{}#00".format(packet_name, body),
   "send packet: ${}#00".format(expected),
   ],
  True)
 self.expect_gdbremote_sequence()
 
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-@skipUnlessAArch64MTELinuxCompiler
-def test_qmemtags_packets(self):
-""" Test that qMemTags packets are parsed correctly and/or rejected. """
+def check_tag_read(self, body, expected):
+self.check_memtags_response("qMemTags", body, expected)
 
+def prep_memtags_test(self):
 self.build()
 self.set_inferior_startup_launch()
 procs = self.prep_debug_monitor_and_inferior()
 
+# We don't use isAArch64MTE here because we cannot do runCmd in an
+# lldb-server test. Instead we run the example and skip if it fails
+# to allocate an MTE buffer.
+
 # Run the process
 self.test_sequence.add_log_lines(
 [
@@ -56,61 +69,153 @@
 buf_address = int(buf_address, 16)
 page_size = int(page_size, 16)
 
-# In the tests below E03 means the packet wasn't formed correctly
-# and E01 means it was but we had some other error acting upon it.
+return buf_address, page_size
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_qMemTags_packets(self):
+""" Test that qMemTags packets are parsed correctly and/or rejected. """
+buf_address, page_size = self.prep_memtags_test()
 
 # Sanity check that address is correct
-self.check_qmemtags_response("{:x},20:1".format(buf_address), "m0001")
+self.check_tag_read("{:x},20:1".format(buf_address), "m0001")
 
 # Invalid packets
 
 # No content
-self.check_qmemtags_response("", "E03")
+self.check_tag_read("", "E03")
 # Only address
-self.check_qmemtags_response("{:x}".format(buf_address), "E03")
+self.check_tag_read("{:x}".format(buf_address), "E03")
 # Only address and length
-self.check_qmemtags_response("{:x},20".format(buf_address), "E03")
+self.check_tag_read("{:x},20".format(buf_address), "E03")
 # Empty address
-self.check_qmemtags_response(",20:1", "E03")
+self.check_tag_read(",20:1", "E03")
 # Invalid addresses
-self.check_qmemtags_response("aardvark,20:1", "E03")
-self.check_qmemtags_response("-100,20:1", "E03")
-self.check_qmemtags_response("cafe?,20:1", "E03")
+self.check_tag_read("aardvark,20:1", "E03")
+self.check_tag_read("-100,20:1", "E03")
+self.check_tag_read("cafe?,20:1", "E03")
 # Empty length
-self.check_qmemtags_

[Lldb-commits] [lldb] 7d27230 - [lldb][AArch64] Add memory tag writing to lldb-server

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-07-27T12:02:17+01:00
New Revision: 7d27230de3336b8c79bfdc90f59858f6dad28fa5

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

LOG: [lldb][AArch64] Add memory tag writing to lldb-server

This is implemented using the QMemTags packet, as specified
by GDB in:
https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#General-Query-Packets

(recall that qMemTags was previously added to read tags)

On receipt of a valid packet lldb-server will:
* align the given address and length to granules
  (most of the time lldb will have already done this
  but the specification doesn't guarantee it)
* Repeat the supplied tags as many times as needed to cover
  the range. (if tags > range we just use as many as needed)
* Call ptrace POKEMTETAGS to write the tags.

The ptrace step will loop just like the tag read does,
until all tags are written or we get an error.
Meaning that if ptrace succeeds it could be a partial write.
So we call it again and if we then get an error, return an error to
lldb.

We are not going to attempt to restore tags after a partial
write followed by an error. This matches the behaviour of the
existing memory writes.

The lldb-server tests have been extended to include read and
write in the same test file. With some updated function names
since "qMemTags" vs "QMemTags" isn't very clear when they're
next to each other.

Reviewed By: omjavaid

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

Added: 


Modified: 
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/source/Host/common/NativeProcessProtocol.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py

Removed: 




diff  --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h 
b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 0d835a5201f8f..770149e3fb283 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -90,6 +90,9 @@ class NativeProcessProtocol {
   virtual Status ReadMemoryTags(int32_t type, lldb::addr_t addr, size_t len,
 std::vector &tags);
 
+  virtual Status WriteMemoryTags(int32_t type, lldb::addr_t addr, size_t len,
+ const std::vector &tags);
+
   /// Reads a null terminated string from memory.
   ///
   /// Reads up to \p max_size bytes of memory until it finds a '\0'.

diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 6ec9e93e8238e..c67c05bdf1823 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -168,7 +168,8 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_jLLDBTraceGetState,
 eServerPacketType_jLLDBTraceGetBinaryData,
 
-eServerPacketType_qMemTags,
+eServerPacketType_qMemTags, // read memory tags
+eServerPacketType_QMemTags, // write memory tags
   };
 
   ServerPacketType GetServerPacketType() const;

diff  --git a/lldb/source/Host/common/NativeProcessProtocol.cpp 
b/lldb/source/Host/common/NativeProcessProtocol.cpp
index 2beedd392c10a..ea80a05430f70 100644
--- a/lldb/source/Host/common/NativeProcessProtocol.cpp
+++ b/lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -58,6 +58,13 @@ NativeProcessProtocol::ReadMemoryTags(int32_t type, 
lldb::addr_t addr,
   return Status("not implemented");
 }
 
+lldb_private::Status
+NativeProcessProtocol::WriteMemoryTags(int32_t type, lldb::addr_t addr,
+   size_t len,
+   const std::vector &tags) {
+  return Status("not implemented");
+}
+
 llvm::Optional NativeProcessProtocol::GetExitStatus() {
   if (m_state == lldb::eStateExited)
 return m_exit_status;

diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index b816d945181c2..8c45796ae0b3c 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1486,6 +1486,77 @@ Status NativeProcessLinux::ReadMemoryTags(int32_t type, 
lldb::addr_t addr,
   return Status();
 }
 
+Status NativeProcessLinux::WriteMemoryTags(int32_t type, lldb::addr_t addr,

[Lldb-commits] [lldb] 43e45f0 - [lldb] Wait in TestGuiBasicDebug for the interface to open before quitting the welcome screen

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-07-27T13:58:49+02:00
New Revision: 43e45f0ec920b45d6073c0aff47597c44948f52c

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

LOG: [lldb] Wait in TestGuiBasicDebug for the interface to open before quitting 
the welcome screen

Speculative fix for the failing lldb-aarch64-ubuntu bot.

Added: 


Modified: 
lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py

Removed: 




diff  --git a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py 
b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
index 9deb700da39c6..beff76e9374f2 100644
--- a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
+++ b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -26,6 +26,7 @@ def test_gui(self):
 
 # Start the GUI and close the welcome window.
 self.child.sendline("gui")
+self.child.expect("Welcome to the LLDB curses GUI.")
 self.child.send(escape_key)
 
 # Simulate a simple debugging session.



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

[Lldb-commits] [PATCH] D106584: [lldb] Improve checking of file cache read eligibility for mach-O

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 added a comment.

In D106584#2901529 , @vsk wrote:

> Hey Augusto, thanks for tackling this, I'm just now slowly paging things in.
>
> Is this a correct statement of the problem: LLDB is failing to disable its 
> file cache optimization when reading writable segments (say, __DATA) from a 
> MachO sourced from the shared cache?
>
> If that's right, then I wonder whether you considered "simply" doing a bounds 
> check on the address? The shared region should be mapped at a fixed virtual 
> range in the debuggee process, and we can determine that range using dyld 
> APIs.

Hey Vedant! Not exactly. Jason's description is spot on.

@jasonmolenda thanks for putting so much thought into this!

> (post-shared-cache on-disk-binaries may have the same offsets as it was in 
> the original shared cache -- but it's actually going to be a MORE subtle bug 
> in that case because people plug in different iPhones and Xcode only extracts 
> the shared cache for one of them if they're all running the same build. But 
> different model iphones will have different shared caches, so using the 
> offsets from the post-shared-cache on-disk-binary-image from one device for 
> another device will be wrong.)

I finally got a watch to test with, and found that the bug still happens on 
dylibs extracted from the shared cache, and this might be the problem.

I'm updating the patch and keeping only the assert, and think of another way of 
solving the problem.




Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:7040
+  // from, since offsets may be changed by the shared cache builder.
+  bool contains_split_info = ContainsLoadCommand(LC_SEGMENT_SPLIT_INFO);
+

vsk wrote:
> Is LC_SEGMENT_SPLIT_INFO present if and only if the MachO is from the shared 
> cache?
No, it means that a dylib is //eligible// for being incorporated in the shared 
cache later on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

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

[Lldb-commits] [PATCH] D106584: [lldb] Improve checking of file cache read eligibility for mach-O

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 updated this revision to Diff 361991.
augusto2112 added a comment.

Keep only filecache and live memory equality assertion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

Files:
  lldb/include/lldb/Core/Section.h
  lldb/source/Core/Section.cpp
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1761,21 +1761,34 @@
   // Read from file cache if read-only section.
   if (!force_live_memory && resolved_addr.IsSectionOffset()) {
 SectionSP section_sp(resolved_addr.GetSection());
-if (section_sp) {
-  auto permissions = Flags(section_sp->GetPermissions());
-  bool is_readonly = !permissions.Test(ePermissionsWritable) &&
- permissions.Test(ePermissionsReadable);
-  if (is_readonly) {
-file_cache_bytes_read =
-ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-if (file_cache_bytes_read == dst_len)
-  return file_cache_bytes_read;
-else if (file_cache_bytes_read > 0) {
-  file_cache_read_buffer =
-  std::make_unique(file_cache_bytes_read);
-  std::memcpy(file_cache_read_buffer.get(), dst, 
file_cache_bytes_read);
+if (section_sp && section_sp->IsReadOnly()) {
+  file_cache_bytes_read =
+  ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+
+#ifndef NDEBUG // Assert that file cache matches the process memory
+  if (ProcessIsValid() && file_cache_bytes_read == dst_len) {
+if (load_addr == LLDB_INVALID_ADDRESS)
+  load_addr = resolved_addr.GetLoadAddress(this);
+if (load_addr != LLDB_INVALID_ADDRESS) {
+  uint8_t *live_buf = (uint8_t *)malloc(dst_len);
+  bytes_read =
+  m_process_sp->ReadMemory(load_addr, live_buf, dst_len, error);
+  if (bytes_read == dst_len) {
+assert(memcmp(live_buf, dst, dst_len) == 0 &&
+   "File cache and live memory diverge!");
+  }
+  free(live_buf);
 }
   }
+#endif
+
+  if (file_cache_bytes_read == dst_len)
+return file_cache_bytes_read;
+  if (file_cache_bytes_read > 0) {
+file_cache_read_buffer =
+std::make_unique(file_cache_bytes_read);
+std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+  }
 }
   }
 
Index: lldb/source/Core/Section.cpp
===
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -599,3 +599,9 @@
   }
   return count;
 }
+
+bool Section::IsReadOnly() {
+  auto permissions = Flags(GetPermissions());
+  return !permissions.Test(ePermissionsWritable) &&
+ permissions.Test(ePermissionsReadable);
+}
Index: lldb/include/lldb/Core/Section.h
===
--- lldb/include/lldb/Core/Section.h
+++ lldb/include/lldb/Core/Section.h
@@ -236,6 +236,8 @@
 
   void SetIsRelocated(bool b) { m_relocated = b; }
 
+  bool IsReadOnly();
+
 protected:
   ObjectFile *m_obj_file;   // The object file that data for this section 
should
 // be read from


Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1761,21 +1761,34 @@
   // Read from file cache if read-only section.
   if (!force_live_memory && resolved_addr.IsSectionOffset()) {
 SectionSP section_sp(resolved_addr.GetSection());
-if (section_sp) {
-  auto permissions = Flags(section_sp->GetPermissions());
-  bool is_readonly = !permissions.Test(ePermissionsWritable) &&
- permissions.Test(ePermissionsReadable);
-  if (is_readonly) {
-file_cache_bytes_read =
-ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-if (file_cache_bytes_read == dst_len)
-  return file_cache_bytes_read;
-else if (file_cache_bytes_read > 0) {
-  file_cache_read_buffer =
-  std::make_unique(file_cache_bytes_read);
-  std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+if (section_sp && section_sp->IsReadOnly()) {
+  file_cache_bytes_read =
+  ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+
+#ifndef NDEBUG // Assert that file cache matches the process memory
+  if (ProcessIsValid() && file_cache_bytes_read == dst_len) {
+if (load_addr == LLDB_INVALID_ADDRESS)
+  load_addr = resolved_addr.GetLoadAddress(this);
+if (load_addr != LLDB_INVALID_ADDRESS) {
+  uint8_t *live_buf = (uint8_t *)malloc(dst_len);
+  bytes_read =
+  m_process_sp->ReadMemory(load_add

[Lldb-commits] [lldb] 5ea091a - [lldb][AArch64] Add memory tag writing to lldb

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-07-27T15:18:42+01:00
New Revision: 5ea091a8174bcce78839156bd044831cb5211d06

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

LOG: [lldb][AArch64] Add memory tag writing to lldb

This adds memory tag writing to Process and the
GDB remote code. Supporting work for the
"memory tag write" command. (to follow)

Process WriteMemoryTags is similair to ReadMemoryTags.
It will pack the tags then call DoWriteMemoryTags.
That function will send the QMemTags packet to the gdb-remote.

The QMemTags packet follows the GDB specification in:
https://sourceware.org/gdb/current/onlinedocs/gdb/General-Query-Packets.html#General-Query-Packets

Note that lldb-server will be treating partial writes as
complete failures. So lldb doesn't need to handle the partial
write case in any special way.

Reviewed By: omjavaid

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

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Target/Process.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 2a993c498d302..aaa2470d29319 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1737,6 +1737,29 @@ class Process : public 
std::enable_shared_from_this,
   llvm::Expected> ReadMemoryTags(lldb::addr_t addr,
size_t len);
 
+  /// Write memory tags for a range of memory.
+  /// (calls DoWriteMemoryTags to do the target specific work)
+  ///
+  /// \param[in] addr
+  /// The address to start writing tags from. It is assumed that this
+  /// address is granule aligned.
+  ///
+  /// \param[in] len
+  /// The size of the range to write tags for. It is assumed that this
+  /// is some multiple of the granule size. This len can be 
diff erent
+  /// from (number of tags * granule size) in the case where you want
+  /// lldb-server to repeat tags across the range.
+  ///
+  /// \param[in] tags
+  /// Allocation tags to be written. Since lldb-server can repeat tags for 
a
+  /// range, the number of tags doesn't have to match the number of 
granules
+  /// in the range. (though most of the time it will)
+  ///
+  /// \return
+  /// A Status telling you if the write succeeded or not.
+  Status WriteMemoryTags(lldb::addr_t addr, size_t len,
+ const std::vector &tags);
+
   /// Resolve dynamically loaded indirect functions.
   ///
   /// \param[in] address
@@ -2777,6 +2800,30 @@ void PruneThreadPlans();
GetPluginName().GetCString());
   }
 
+  /// Does the final operation to write memory tags. E.g. sending a GDB packet.
+  /// It assumes that WriteMemoryTags has checked that memory tagging is 
enabled
+  /// and has packed the tag data.
+  ///
+  /// \param[in] addr
+  ///Start of address range to write memory tags for.
+  ///
+  /// \param[in] len
+  ///Length of the memory range to write tags for (in bytes).
+  ///
+  /// \param[in] type
+  ///Type of tags to read (get this from a MemoryTagManager)
+  ///
+  /// \param[in] tags
+  ///Packed tags to be written.
+  ///
+  /// \return
+  /// Status telling you whether the write succeeded.
+  virtual Status DoWriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
+   const std::vector &tags) {
+return Status("%s does not support writing memory tags",
+  GetPluginName().GetCString());
+  }
+
   // Type definitions
   typedef std::map
   LanguageRuntimeCollection;

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 5e80317d5327c..b16aed4f5c90f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -628,6 +628,24 @@ DataBufferSP 
GDBRemoteCommunicationClient::ReadMemoryTags(lldb::addr_t addr,
   return buffer_sp;
 }
 
+Status GDBRemoteCommunicationClient::WriteMemoryTags(
+lldb::addr_t addr, size_t len, int32_t type,
+const std::vector &tags) {
+  // Format QMemTags:address,length:type:tags
+  StreamString packet;
+  packet.Printf("QMemTags:%" PRIx64 ",%zx:%" PRIx32 ":", addr, len, type);
+  packet.PutBytesAsRawHex8

[Lldb-commits] [PATCH] D105181: [lldb][AArch64] Add memory tag writing to lldb

[David Spickett via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ea091a8174b: [lldb][AArch64] Add memory tag writing to lldb 
(authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105181

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include 
+#include 
 
 using namespace lldb_private::process_gdb_remote;
 using namespace lldb_private;
@@ -531,3 +532,51 @@
   check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m01020",
  llvm::None);
 }
+
+static void check_Qmemtags(TestClient &client, MockServer &server,
+   lldb::addr_t addr, size_t len, int32_t type,
+   const std::vector &tags, const char *packet,
+   llvm::StringRef response, bool should_succeed) {
+  const auto &WriteMemoryTags = [&]() {
+std::future result = std::async(std::launch::async, [&] {
+  return client.WriteMemoryTags(addr, len, type, tags);
+});
+
+HandlePacket(server, packet, response);
+return result.get();
+  };
+
+  auto result = WriteMemoryTags();
+  if (should_succeed)
+ASSERT_TRUE(result.Success());
+  else
+ASSERT_TRUE(result.Fail());
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, WriteMemoryTags) {
+  check_Qmemtags(client, server, 0xABCD, 0x20, 1,
+ std::vector{0x12, 0x34}, "QMemTags:abcd,20:1:1234",
+ "OK", true);
+
+  // The GDB layer doesn't care that the number of tags !=
+  // the length of the write.
+  check_Qmemtags(client, server, 0x4321, 0x20, 9, std::vector{},
+ "QMemTags:4321,20:9:", "OK", true);
+
+  check_Qmemtags(client, server, 0x8877, 0x123, 0x34,
+ std::vector{0x55, 0x66, 0x77},
+ "QMemTags:8877,123:34:556677", "E01", false);
+
+  // Type is a signed integer but is packed as its raw bytes,
+  // instead of having a +/-.
+  check_Qmemtags(client, server, 0x456789, 0, -1, std::vector{0x99},
+ "QMemTags:456789,0::99", "E03", false);
+  check_Qmemtags(client, server, 0x456789, 0,
+ std::numeric_limits::max(),
+ std::vector{0x99}, "QMemTags:456789,0:7fff:99",
+ "E03", false);
+  check_Qmemtags(client, server, 0x456789, 0,
+ std::numeric_limits::min(),
+ std::vector{0x99}, "QMemTags:456789,0:8000:99",
+ "E03", false);
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -6102,3 +6102,21 @@
   return tag_manager->UnpackTagsData(*tag_data,
  len / tag_manager->GetGranuleSize());
 }
+
+Status Process::WriteMemoryTags(lldb::addr_t addr, size_t len,
+const std::vector &tags) {
+  llvm::Expected tag_manager_or_err =
+  GetMemoryTagManager();
+  if (!tag_manager_or_err)
+return Status(tag_manager_or_err.takeError());
+
+  const MemoryTagManager *tag_manager = *tag_manager_or_err;
+  llvm::Expected> packed_tags =
+  tag_manager->PackTags(tags);
+  if (!packed_tags) {
+return Status(packed_tags.takeError());
+  }
+
+  return DoWriteMemoryTags(addr, len, tag_manager->GetAllocationTagType(),
+   *packed_tags);
+}
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -411,6 +411,9 @@
   llvm::Expected>
   DoReadMemoryTags(lldb::addr_t addr, size_t len, int32_t type) override;
 
+  Status DoWriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
+   const std::vector &tags) override;
+
 private:
   // For ProcessGDBRemote only
   std::string m_partial_profile_data;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/

[Lldb-commits] [PATCH] D106880: [lldb][AArch64] Mark mismatched tags in tag read output

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: danielkiss, kristof.beyls.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The "memory tag read" command will now tell you
when the allocation tag read does not match the logical
tag.

(lldb) memory tag read mte_buf+(8*16) mte_buf+(8*16)+48
Logical tag: 0x9
Allocation tags:
[0xf7ff7080, 0xf7ff7090): 0x8 (mismatch)
[0xf7ff7090, 0xf7ff70a0): 0x9
[0xf7ff70a0, 0xf7ff70b0): 0xa (mismatch)

The logical tag will be taken from the start address
so the end could have a different tag. You could for example
read from ptr_to_array_1 to ptr_to_array_2. Where the latter
is tagged differently to prevent buffer overflow.

The existing command will read 1 granule if you leave
off the end address. So you can also use it as a quick way
to check a single location.

(lldb) memory tag read mte_buf
Logical tag: 0x9
Allocation tags:
[0xf7ff7000, 0xf7ff7010): 0x0 (mismatch)

This avoids the need for a seperate "memory tag check" command.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106880

Files:
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  
lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py

Index: lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
===
--- lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
+++ lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
@@ -81,20 +81,20 @@
 self.expect("memory tag read mte_buf",
 patterns=["Logical tag: 0x9\n"
   "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0$"])
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0 \(mismatch\)$"])
 
 # Range of <1 granule is rounded up to 1 granule
 self.expect("memory tag read mte_buf mte_buf+8",
 patterns=["Logical tag: 0x9\n"
   "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0$"])
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0 \(mismatch\)$"])
 
 # Start address is aligned down, end aligned up
 self.expect("memory tag read mte_buf+8 mte_buf+24",
 patterns=["Logical tag: 0x9\n"
   "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0\n"
-  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0x1$"])
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0 \(mismatch\)\n"
+  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0x1 \(mismatch\)$"])
 
 # You may read up to the end of the tagged region
 # Layout is mte_buf, mte_buf_2, non_mte_buf.
@@ -119,15 +119,23 @@
 self.expect("memory tag read mte_buf+page_size-16 mte_buf+page_size+16",
 patterns=["Logical tag: 0x9\n"
   "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+f0, 0x[0-9A-Fa-f]+00\): 0xf\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0$"])
+  "\[0x[0-9A-Fa-f]+f0, 0x[0-9A-Fa-f]+00\): 0xf \(mismatch\)\n"
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0 \(mismatch\)$"])
 
 # Tags in start/end are ignored when creating the range.
 # So this is not an error despite start/end having different tags
-self.expect("memory tag read mte_buf mte_buf_alt_tag+16 ",
+self.expect("memory tag read mte_buf mte_buf_alt_tag+16",
 patterns=["Logical tag: 0x9\n"
   "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0$"])
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0 \(mismatch\)$"])
+
+# Mismatched tags are marked. The logical tag is taken from the start address.
+self.expect("memory tag read mte_buf+(8*16) mte_buf_alt_tag+(8*16)+48",
+patterns=["Logical tag: 0x9\n"
+  "Allocation tags:\n"
+  "\[0x[0-9A-Fa-f]+80, 0x[0-9A-Fa-f]+90\): 0x8 \(mismatch\)\n"
+  "\[0x[0-9A-Fa-f]+90, 0x[0-9A-Fa-f]+a0\): 0x9\n"
+  "\[0x[0-9A-Fa-f]+a0, 0x[0-9A-Fa-f]+b0\): 0xa \(mismatch\)$"])
 
 @skipUnlessArch("aarch64")
 @skipUnlessPlatform(["linux"])
@@ -162,16 +170,16 @@
 patterns=["Logical tag: 0x9\n"
   "Allocation tags:\n"
   "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x9\n"
-  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0x1$"])
+  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0

[Lldb-commits] [PATCH] D106880: [lldb][AArch64] Mark mismatched tags in tag read output

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: omjavaid.
DavidSpickett added a comment.

This gives us the equivalent of GDB's "mtag check" but with some more 
flexibility and no extra command.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106880

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

[Lldb-commits] [PATCH] D105182: [lldb] Add "memory tag write" command

[David Spickett via Phabricator via lldb-commits]

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

Add a link to intrinsics documentation in the test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105182

Files:
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/test/API/functionalities/memory/tag/TestMemoryTag.py
  lldb/test/API/linux/aarch64/mte_tag_access/Makefile
  
lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py
  lldb/test/API/linux/aarch64/mte_tag_access/main.c
  lldb/test/API/linux/aarch64/mte_tag_read/Makefile
  lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
  lldb/test/API/linux/aarch64/mte_tag_read/main.c

Index: lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
===
--- lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
+++ /dev/null
@@ -1,126 +0,0 @@
-"""
-Test "memory tag read" command on AArch64 Linux with MTE.
-"""
-
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class AArch64LinuxMTEMemoryTagReadTestCase(TestBase):
-
-mydir = TestBase.compute_mydir(__file__)
-
-NO_DEBUG_INFO_TESTCASE = True
-
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-@skipUnlessAArch64MTELinuxCompiler
-def test_mte_tag_read(self):
-if not self.isAArch64MTE():
-self.skipTest('Target must support MTE.')
-
-self.build()
-self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
-
-lldbutil.run_break_set_by_file_and_line(self, "main.c",
-line_number('main.c', '// Breakpoint here'),
-num_expected_locations=1)
-
-self.runCmd("run", RUN_SUCCEEDED)
-
-if self.process().GetState() == lldb.eStateExited:
-self.fail("Test program failed to run.")
-
-self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-substrs=['stopped',
- 'stop reason = breakpoint'])
-
-# Argument validation
-self.expect("memory tag read",
-substrs=["error: wrong number of arguments; expected at least , "
- "at most  "],
-error=True)
-self.expect("memory tag read buf buf+16 32",
-substrs=["error: wrong number of arguments; expected at least , "
- "at most  "],
-error=True)
-self.expect("memory tag read not_a_symbol",
-substrs=["error: Invalid address expression, address expression \"not_a_symbol\" "
- "evaluation failed"],
-error=True)
-self.expect("memory tag read buf not_a_symbol",
-substrs=["error: Invalid end address expression, address expression \"not_a_symbol\" "
- "evaluation failed"],
-error=True)
-# Inverted range
-self.expect("memory tag read buf buf-16",
-patterns=["error: End address \(0x[A-Fa-f0-9]+\) must be "
-  "greater than the start address \(0x[A-Fa-f0-9]+\)"],
-error=True)
-# Range of length 0
-self.expect("memory tag read buf buf",
-patterns=["error: End address \(0x[A-Fa-f0-9]+\) must be "
-  "greater than the start address \(0x[A-Fa-f0-9]+\)"],
-error=True)
-
-
-# Can't read from a region without tagging
-self.expect("memory tag read non_mte_buf",
-patterns=["error: Address range 0x[0-9A-Fa-f]+00:0x[0-9A-Fa-f]+10 is not "
- "in a memory tagged region"],
-error=True)
-
-# If there's no end address we assume 1 granule
-self.expect("memory tag read buf",
-patterns=["Logical tag: 0x9\n"
-  "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0$"])
-
-# Range of <1 granule is rounded up to 1 granule
-self.expect("memory tag read buf buf+8",
-patterns=["Logical tag: 0x9\n"
-  "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0$"])
-
-# Start address is aligned down, end aligned up
-self.expect("memory tag read buf+8 buf+24",
-patterns=["Logical tag: 0x9\n"
-  "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x0\n"
-  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0x1$"])
-
-# You may read up to the end of the tagged region
-# Layout is buf (MTE), buf2 (MTE), 
-# so we read from the end of buf2 here.
-self

[Lldb-commits] [PATCH] D106584: [lldb] Assert file cache and live memory are equal on debug builds

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Looks good!  Maybe we should add a setting like 
target.file-cache-memory-reads-verify (a boolean) to enable/disable this check 
instead of keying off NDEBUG.  We can turn it on in the testsuite, and we can 
optionally have people with release installs of lldb enable it if we ever 
suspect that it might be an issue.  The settings are added with an entry in 
source/Target/TargetProperties.td and a getter/setter in TargetProperties.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

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

[Lldb-commits] [PATCH] D106584: [lldb] Assert file cache and live memory are equal on debug builds

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D106584#2907898 , @jasonmolenda 
wrote:

> Looks good!  Maybe we should add a setting like 
> target.file-cache-memory-reads-verify (a boolean) to enable/disable this 
> check instead of keying off NDEBUG.  We can turn it on in the testsuite, and 
> we can optionally have people with release installs of lldb enable it if we 
> ever suspect that it might be an issue.  The settings are added with an entry 
> in source/Target/TargetProperties.td and a getter/setter in TargetProperties.

If we do that we should make it an `lldbassert` so it prints something in 
release builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

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

[Lldb-commits] [lldb] 0018c71 - Fix "break delete --disabled" with no arguments.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-07-27T13:38:09-07:00
New Revision: 0018c7123be3e090ba546fb730ed316fa2567655

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

LOG: Fix "break delete --disabled" with no arguments.

The code that figured out which breakpoints to delete was supposed
to set the result status if it found breakpoints, and then the code
that actually deleted them checked that the result's status was set.

The code for "break delete --disabled" failed to set the status if
no "protected" breakpoints were provided.  This was a confusing way
to implement this, so I reworked it with early returns so it was less
error prone, and added a test case for the no arguments case.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectBreakpoint.cpp

lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index ba66a12267191..722d5c4d8f472 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -1458,6 +1458,7 @@ class CommandObjectBreakpointDelete : public 
CommandObjectParsed {
   return false;
 }
 
+// Handle the delete all breakpoints case:
 if (command.empty() && !m_options.m_delete_disabled) {
   if (!m_options.m_force &&
   !m_interpreter.Confirm(
@@ -1471,67 +1472,73 @@ class CommandObjectBreakpointDelete : public 
CommandObjectParsed {
 (uint64_t)num_breakpoints, num_breakpoints > 1 ? "s" : "");
   }
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
-} else {
-  // Particular breakpoint selected; disable that breakpoint.
-  BreakpointIDList valid_bp_ids;
-  
-  if (m_options.m_delete_disabled) {
-BreakpointIDList excluded_bp_ids;
+  return result.Succeeded();
+}
+ 
+// Either we have some kind of breakpoint specification(s),
+// or we are handling "break disable --deleted".  Gather the list
+// of breakpoints to delete here, the we'll delete them below.
+BreakpointIDList valid_bp_ids;
+
+if (m_options.m_delete_disabled) {
+  BreakpointIDList excluded_bp_ids;
 
-if (!command.empty()) {
-  CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-  command, &target, result, &excluded_bp_ids,
-  BreakpointName::Permissions::PermissionKinds::deletePerm);
-}
-for (auto breakpoint_sp : breakpoints.Breakpoints()) {
-  if (!breakpoint_sp->IsEnabled() && breakpoint_sp->AllowDelete()) {
-BreakpointID bp_id(breakpoint_sp->GetID());
-size_t pos = 0;
-if (!excluded_bp_ids.FindBreakpointID(bp_id, &pos))
-  valid_bp_ids.AddBreakpointID(breakpoint_sp->GetID());
-  }
-}
-if (valid_bp_ids.GetSize() == 0) {
-  result.AppendError("No disabled breakpoints.");
-  return false;
-}
-  } else {
+  if (!command.empty()) {
 CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-command, &target, result, &valid_bp_ids,
+command, &target, result, &excluded_bp_ids,
 BreakpointName::Permissions::PermissionKinds::deletePerm);
+if (!result.Succeeded())
+  return false;
   }
-  
-  if (result.Succeeded()) {
-int delete_count = 0;
-int disable_count = 0;
-const size_t count = valid_bp_ids.GetSize();
-for (size_t i = 0; i < count; ++i) {
-  BreakpointID cur_bp_id = valid_bp_ids.GetBreakpointIDAtIndex(i);
 
-  if (cur_bp_id.GetBreakpointID() != LLDB_INVALID_BREAK_ID) {
-if (cur_bp_id.GetLocationID() != LLDB_INVALID_BREAK_ID) {
-  Breakpoint *breakpoint =
-  target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
-  BreakpointLocation *location =
-  
breakpoint->FindLocationByID(cur_bp_id.GetLocationID()).get();
-  // It makes no sense to try to delete individual locations, so we
-  // disable them instead.
-  if (location) {
-location->SetEnabled(false);
-++disable_count;
-  }
-} else {
-  target.RemoveBreakpointByID(cur_bp_id.GetBreakpointID());
-  ++delete_count;
-}
+  for (auto breakpoint_sp : breakpoints.Breakpoints()) {
+if (!breakpoint_sp->IsEnabled() && breakpoint_sp->AllowDelete()) {
+  BreakpointID bp_id(breakpoint_sp->GetID());
+  size_t pos = 0;
+  if (!excluded_bp_ids.Fi

[Lldb-commits] [lldb] 910353c - When calculating the "currently selected thread" in

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-07-27T13:38:09-07:00
New Revision: 910353c1048e5efac64b639a65b4d968aba3aa81

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

LOG: When calculating the "currently selected thread" in
Process::HandleStateChangedEvent, we check whether a thread stopped
for eStopReasonSignal is stopped for a signal that's currently set to
"no-stop". If it is, then we don't set that thread as the currently
selected thread.

But that only happens in the part of the algorithm that's handling the
case where the previously selected thread has no stop reason. Since we
want to keep on a thread as long as it is doing something interesting,
we always prefer the current thread. That's almost right, but we
forgot to check whether the previously selected thread stopped with an
eStopReasonSignal for a "no-stop" signal. If it did, then we shouldn't
select it.

This patch adds that check. I can't figure out a good way to test
this. This is the sort of thing that Ismail's scripted process plugin
will make easy once it is a real boy. But figuring out how to do this
in a real process is not trivial.

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

Added: 


Modified: 
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 5cd739efe53a..8ecc66b592ea 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -777,13 +777,30 @@ bool Process::HandleProcessStateChangedEvent(const 
EventSP &event_sp,
 ThreadSP curr_thread(thread_list.GetSelectedThread());
 ThreadSP thread;
 StopReason curr_thread_stop_reason = eStopReasonInvalid;
-if (curr_thread) {
+bool prefer_curr_thread = false;
+if (curr_thread && curr_thread->IsValid()) {
   curr_thread_stop_reason = curr_thread->GetStopReason();
+  switch (curr_thread_stop_reason) {
+  case eStopReasonNone:
+  case eStopReasonInvalid:
+// Don't prefer the current thread if it didn't stop for a reason.
+break;
+  case eStopReasonSignal: {
+// We need to do the same computation we do for other threads
+// below in case the current thread happens to be the one that
+// stopped for the no-stop signal.
+uint64_t signo = curr_thread->GetStopInfo()->GetValue();
+if (process_sp->GetUnixSignals()->GetShouldStop(signo))
+  prefer_curr_thread = true;
+  } break;
+  default:
+prefer_curr_thread = true;
+break;
+  }
   curr_thread_stop_info_sp = curr_thread->GetStopInfo();
 }
-if (!curr_thread || !curr_thread->IsValid() ||
-curr_thread_stop_reason == eStopReasonInvalid ||
-curr_thread_stop_reason == eStopReasonNone) {
+
+if (!prefer_curr_thread) {
   // Prefer a thread that has just completed its plan over another
   // thread as current thread.
   ThreadSP plan_thread;



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

[Lldb-commits] [PATCH] D106623: Fix a logic error in "break delete --disabled" when no "protected" breakpoints are specified.

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0018c7123be3: Fix "break delete --disabled" with 
no arguments. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106623

Files:
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
===
--- lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -313,3 +313,22 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertTrue(bp_3.IsValid(), "DeleteMeNot didn't protect disabled breakpoint 3")
+
+# Reset the first breakpoint, disable it, and do this again with no protected name:
+bp_1 = target.BreakpointCreateByName("main")
+
+bp_1.SetEnabled(False)
+
+bp_id_1 = bp_1.GetID()
+
+self.runCmd("breakpoint delete --disabled")
+
+bp_1 = target.FindBreakpointByID(bp_id_1)
+self.assertFalse(bp_1.IsValid(), "Didn't delete disabled breakpoint 1")
+
+bp_2 = target.FindBreakpointByID(bp_id_2)
+self.assertTrue(bp_2.IsValid(), "Deleted enabled breakpoint 2")
+
+bp_3 = target.FindBreakpointByID(bp_id_3)
+self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
+
Index: lldb/source/Commands/CommandObjectBreakpoint.cpp
===
--- lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -1458,6 +1458,7 @@
   return false;
 }
 
+// Handle the delete all breakpoints case:
 if (command.empty() && !m_options.m_delete_disabled) {
   if (!m_options.m_force &&
   !m_interpreter.Confirm(
@@ -1471,67 +1472,73 @@
 (uint64_t)num_breakpoints, num_breakpoints > 1 ? "s" : "");
   }
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
-} else {
-  // Particular breakpoint selected; disable that breakpoint.
-  BreakpointIDList valid_bp_ids;
-  
-  if (m_options.m_delete_disabled) {
-BreakpointIDList excluded_bp_ids;
+  return result.Succeeded();
+}
+ 
+// Either we have some kind of breakpoint specification(s),
+// or we are handling "break disable --deleted".  Gather the list
+// of breakpoints to delete here, the we'll delete them below.
+BreakpointIDList valid_bp_ids;
+
+if (m_options.m_delete_disabled) {
+  BreakpointIDList excluded_bp_ids;
 
-if (!command.empty()) {
-  CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-  command, &target, result, &excluded_bp_ids,
-  BreakpointName::Permissions::PermissionKinds::deletePerm);
-}
-for (auto breakpoint_sp : breakpoints.Breakpoints()) {
-  if (!breakpoint_sp->IsEnabled() && breakpoint_sp->AllowDelete()) {
-BreakpointID bp_id(breakpoint_sp->GetID());
-size_t pos = 0;
-if (!excluded_bp_ids.FindBreakpointID(bp_id, &pos))
-  valid_bp_ids.AddBreakpointID(breakpoint_sp->GetID());
-  }
-}
-if (valid_bp_ids.GetSize() == 0) {
-  result.AppendError("No disabled breakpoints.");
-  return false;
-}
-  } else {
+  if (!command.empty()) {
 CommandObjectMultiwordBreakpoint::VerifyBreakpointOrLocationIDs(
-command, &target, result, &valid_bp_ids,
+command, &target, result, &excluded_bp_ids,
 BreakpointName::Permissions::PermissionKinds::deletePerm);
+if (!result.Succeeded())
+  return false;
   }
-  
-  if (result.Succeeded()) {
-int delete_count = 0;
-int disable_count = 0;
-const size_t count = valid_bp_ids.GetSize();
-for (size_t i = 0; i < count; ++i) {
-  BreakpointID cur_bp_id = valid_bp_ids.GetBreakpointIDAtIndex(i);
 
-  if (cur_bp_id.GetBreakpointID() != LLDB_INVALID_BREAK_ID) {
-if (cur_bp_id.GetLocationID() != LLDB_INVALID_BREAK_ID) {
-  Breakpoint *breakpoint =
-  target.GetBreakpointByID(cur_bp_id.GetBreakpointID()).get();
-  BreakpointLocation *location =
-  breakpoint->FindLocationByID(cur_bp_id.GetLocationID()).get();
-  // It makes no sense to try to delete individual locations, so we
-  // disable them instead.
-  if (location) {
-lo

[Lldb-commits] [lldb] 6952928 - Add a test for top-level expressions using "expr --top-level".

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-07-27T13:38:09-07:00
New Revision: 69529286ce2dcd90563bca97537ce570c6d115b4

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

LOG: Add a test for top-level expressions using "expr --top-level".

This was broken for a while even though the Python version
continued to work.  This adds a test so it doesn't regress.

Added: 


Modified: 
lldb/test/API/commands/expression/top-level/TestTopLevelExprs.py

Removed: 




diff  --git a/lldb/test/API/commands/expression/top-level/TestTopLevelExprs.py 
b/lldb/test/API/commands/expression/top-level/TestTopLevelExprs.py
index 92707eabec0ce..220332759ec4d 100644
--- a/lldb/test/API/commands/expression/top-level/TestTopLevelExprs.py
+++ b/lldb/test/API/commands/expression/top-level/TestTopLevelExprs.py
@@ -92,6 +92,12 @@ def test_top_level_expressions(self):
 resultFromCode,
 resultFromTopLevel.GetValueAsUnsigned())
 
+# Make sure the command line version works as well:
+self.runCmd("expr --top-level -- int TopLevelFunction() { return 101; 
}")
+resultFromTopLevel = 
self.frame().EvaluateExpression("TopLevelFunction()")
+self.assertTrue(resultFromTopLevel.IsValid())
+self.assertEqual(101, resultFromTopLevel.GetValueAsUnsigned(), 
"Command line version works.")
+
 def test_top_level_expression_without_target(self):
 self.expect("expr --top-level -- void func() {}", error=True,
 substrs=["Top-level code needs to be inserted into a 
runnable target"])



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

[Lldb-commits] [PATCH] D106712: Remember to check whether the current thread is stopped for a no-stop signal

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG910353c1048e: When calculating the "currently selected 
thread" in (authored by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D106712?vs=361342&id=362168#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106712

Files:
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -777,13 +777,30 @@
 ThreadSP curr_thread(thread_list.GetSelectedThread());
 ThreadSP thread;
 StopReason curr_thread_stop_reason = eStopReasonInvalid;
-if (curr_thread) {
+bool prefer_curr_thread = false;
+if (curr_thread && curr_thread->IsValid()) {
   curr_thread_stop_reason = curr_thread->GetStopReason();
+  switch (curr_thread_stop_reason) {
+  case eStopReasonNone:
+  case eStopReasonInvalid:
+// Don't prefer the current thread if it didn't stop for a reason.
+break;
+  case eStopReasonSignal: {
+// We need to do the same computation we do for other threads
+// below in case the current thread happens to be the one that
+// stopped for the no-stop signal.
+uint64_t signo = curr_thread->GetStopInfo()->GetValue();
+if (process_sp->GetUnixSignals()->GetShouldStop(signo))
+  prefer_curr_thread = true;
+  } break;
+  default:
+prefer_curr_thread = true;
+break;
+  }
   curr_thread_stop_info_sp = curr_thread->GetStopInfo();
 }
-if (!curr_thread || !curr_thread->IsValid() ||
-curr_thread_stop_reason == eStopReasonInvalid ||
-curr_thread_stop_reason == eStopReasonNone) {
+
+if (!prefer_curr_thread) {
   // Prefer a thread that has just completed its plan over another
   // thread as current thread.
   ThreadSP plan_thread;


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -777,13 +777,30 @@
 ThreadSP curr_thread(thread_list.GetSelectedThread());
 ThreadSP thread;
 StopReason curr_thread_stop_reason = eStopReasonInvalid;
-if (curr_thread) {
+bool prefer_curr_thread = false;
+if (curr_thread && curr_thread->IsValid()) {
   curr_thread_stop_reason = curr_thread->GetStopReason();
+  switch (curr_thread_stop_reason) {
+  case eStopReasonNone:
+  case eStopReasonInvalid:
+// Don't prefer the current thread if it didn't stop for a reason.
+break;
+  case eStopReasonSignal: {
+// We need to do the same computation we do for other threads
+// below in case the current thread happens to be the one that
+// stopped for the no-stop signal.
+uint64_t signo = curr_thread->GetStopInfo()->GetValue();
+if (process_sp->GetUnixSignals()->GetShouldStop(signo))
+  prefer_curr_thread = true;
+  } break;
+  default:
+prefer_curr_thread = true;
+break;
+  }
   curr_thread_stop_info_sp = curr_thread->GetStopInfo();
 }
-if (!curr_thread || !curr_thread->IsValid() ||
-curr_thread_stop_reason == eStopReasonInvalid ||
-curr_thread_stop_reason == eStopReasonNone) {
+
+if (!prefer_curr_thread) {
   // Prefer a thread that has just completed its plan over another
   // thread as current thread.
   ThreadSP plan_thread;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106192: [LLDB][GUI] Add Create Target form

[Omar Emara via Phabricator via lldb-commits]

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

- Rebase on main.
- Add basic remote debugging support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106192

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -36,6 +36,7 @@
 
 #include "lldb/Interpreter/CommandCompletions.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/OptionGroupPlatform.h"
 
 #if LLDB_ENABLE_CURSES
 #include "lldb/Breakpoint/BreakpointLocation.h"
@@ -2651,6 +2652,195 @@
   ProcessPluginFieldDelegate *m_plugin_field;
 };
 
+class TargetCreateFormDelegate : public FormDelegate {
+public:
+  TargetCreateFormDelegate(Debugger &debugger) : m_debugger(debugger) {
+m_executable_field = AddFileField("Executable", "", true, true);
+m_core_file_field = AddFileField("Core File", "", true, false);
+m_symbol_file_field = AddFileField("Symbol File", "", true, false);
+m_show_advanced_field = AddBooleanField("Show advanced settings.", false);
+m_remote_file_field = AddFileField("Remote File", "", false, false);
+m_arch_field = AddArchField("Architecture", "", false);
+m_platform_field = AddPlatformPluginField(debugger);
+m_load_dependent_files_field =
+AddChoicesField("Load Dependents", 3, GetLoadDependentFilesChoices());
+
+AddAction("Create", [this](Window &window) { CreateTarget(window); });
+  }
+
+  std::string GetName() override { return "Create Target"; }
+
+  void UpdateFieldsVisibility() override {
+if (m_show_advanced_field->GetBoolean()) {
+  m_remote_file_field->FieldDelegateShow();
+  m_arch_field->FieldDelegateShow();
+  m_platform_field->FieldDelegateShow();
+  m_load_dependent_files_field->FieldDelegateShow();
+} else {
+  m_remote_file_field->FieldDelegateHide();
+  m_arch_field->FieldDelegateHide();
+  m_platform_field->FieldDelegateHide();
+  m_load_dependent_files_field->FieldDelegateHide();
+}
+  }
+
+  std::vector GetLoadDependentFilesChoices() {
+std::vector load_depentents_options;
+load_depentents_options.push_back(
+std::string("Only if the \"Executable\" is an executable file."));
+load_depentents_options.push_back(std::string("Yes."));
+load_depentents_options.push_back(std::string("No."));
+return load_depentents_options;
+  }
+
+  LoadDependentFiles GetLoadDependentFiles() {
+std::string choice = m_load_dependent_files_field->GetChoiceContent();
+if (choice == "No.")
+  return eLoadDependentsNo;
+if (choice == "Yes.")
+  return eLoadDependentsYes;
+return eLoadDependentsDefault;
+  }
+
+  OptionGroupPlatform GetPlatformOptions() {
+OptionGroupPlatform platform_options(false);
+platform_options.SetPlatformName(m_platform_field->GetPluginName().c_str());
+return platform_options;
+  }
+
+  TargetSP GetTarget() {
+OptionGroupPlatform platform_options = GetPlatformOptions();
+TargetSP target_sp;
+Status status = m_debugger.GetTargetList().CreateTarget(
+m_debugger, m_executable_field->GetPath(),
+m_arch_field->GetArchString(), GetLoadDependentFiles(),
+&platform_options, target_sp);
+
+if (status.Fail()) {
+  SetError(status.AsCString());
+  return nullptr;
+}
+
+m_debugger.GetTargetList().SetSelectedTarget(target_sp);
+
+return target_sp;
+  }
+
+  void SetSymbolFile(TargetSP target_sp) {
+if (!m_symbol_file_field->IsSpecified())
+  return;
+
+ModuleSP module_sp(target_sp->GetExecutableModule());
+if (!module_sp)
+  return;
+
+module_sp->SetSymbolFileFileSpec(
+m_symbol_file_field->GetResolvedFileSpec());
+  }
+
+  void SetCoreFile(TargetSP target_sp) {
+if (!m_core_file_field->IsSpecified())
+  return;
+
+FileSpec core_file_spec = m_core_file_field->GetResolvedFileSpec();
+
+FileSpec core_file_directory_spec;
+core_file_directory_spec.GetDirectory() = core_file_spec.GetDirectory();
+target_sp->AppendExecutableSearchPaths(core_file_directory_spec);
+
+ProcessSP process_sp(target_sp->CreateProcess(
+m_debugger.GetListener(), llvm::StringRef(), &core_file_spec, false));
+
+if (!process_sp) {
+  SetError("Unable to find process plug-in for core file!");
+  return;
+}
+
+Status status = process_sp->LoadCore();
+if (status.Fail()) {
+  SetError("Can't find plug-in for core file!");
+  return;
+}
+  }
+
+  void SetRemoteFile(TargetSP target_sp) {
+if (!m_remote_file_field->IsSpecified())
+  return;
+
+PlatformSP platform_sp = target_sp->GetPlatform();
+if (!platform_sp) {
+  SetError("No platform found for target!");
+  return;
+}
+
+Fil

[Lldb-commits] [PATCH] D106192: [LLDB][GUI] Add Create Target form

[Omar Emara via Phabricator via lldb-commits]

OmarEmaraDev added a comment.

I still can't get remote debugging to work unfortunately, or maybe I don't 
understand it really. The way I understand it is as follows:

- If the remote file is specified, then that means we are creating a target for 
remote debugging.
- If the remote file doesn't exist, we upload the executable to the location 
specified by the remote file.
- If the remote file exists but the executable doesn't exist, then the remote 
file is downloaded to the local file. (Why? And does that mean the executable 
needn't exist if the remote file is specified? And also, does that mean the 
executable isn't a required field?)
- Everything happens using the platform of the created target, which is 
specified in the platform field. (When/Where does the connection with the 
server get established? `platform connect` connects the currently selected 
platform, but the target have its own platform which may not necessary be the 
currently selected one.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106192

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

[Lldb-commits] [lldb] 0a74fbb - [lldb][NFC] Fix incorrect log and comment

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2021-07-27T14:43:42-07:00
New Revision: 0a74fbb7b1d3e04ac03389f1fc455ac593c2e5ee

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

LOG: [lldb][NFC] Fix incorrect log and comment

Likely copy & paste issue that was overlooked years ago

Added: 


Modified: 
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
index c7ae128d874f5..348a081cfe179 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -66,7 +66,7 @@ bool ABIMacOSX_arm64::PrepareTrivialCall(
 
   if (log) {
 StreamString s;
-s.Printf("ABISysV_x86_64::PrepareTrivialCall (tid = 0x%" PRIx64
+s.Printf("ABIMacOSX_arm64::PrepareTrivialCall (tid = 0x%" PRIx64
  ", sp = 0x%" PRIx64 ", func_addr = 0x%" PRIx64
  ", return_addr = 0x%" PRIx64,
  thread.GetID(), (uint64_t)sp, (uint64_t)func_addr,
@@ -86,7 +86,7 @@ bool ABIMacOSX_arm64::PrepareTrivialCall(
   eRegisterKindGeneric, LLDB_REGNUM_GENERIC_RA);
 
   // x0 - x7 contain first 8 simple args
-  if (args.size() > 8) // TODO handle more than 6 arguments
+  if (args.size() > 8) // TODO handle more than 8 arguments
 return false;
 
   for (size_t i = 0; i < args.size(); ++i) {



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

[Lldb-commits] [PATCH] D105215: [lldb] Remove CPlusPlusLanguage from Mangled

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:322
   if (preference == ePreferDemangledWithoutArguments) {
-return GetDemangledNameWithoutArguments(m_mangled, demangled);
+if (Language *lang = Language::FindPlugin(GuessLanguage())) {
+  return lang->GetDemangledFunctionNameWithoutArguments(*this);

clayborg wrote:
> Maybe we should make a Language::FindPlugin(...) that like:
> ```
> Language *Language::FindPlugin(Mangled::ManglingScheme mangling_scheme);
> ```
> Should be easy to add since this change is kind of about refactoring and 
> putting the code into plug-ins. It is essentially what "lldb::LanguageType 
> Mangled::GuessLanguage() const" is doing. That code could be moved to where 
> Language::FindPlugin(...) lives.
`GuessLanguage` has a bunch of uses outside of `Mangled` itself, so I think 
that would make more sense if we performed other refactors first.

I don't think putting this into another `Language::FindPlugin` function is a 
good idea because (as I understand it) mangling schemes aren't specific to 
languages. 



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:67
 
+ConstString CPlusPlusLanguage::GetDemangledFunctionNameWithoutArguments(
+Mangled mangled) const {

clayborg wrote:
> We are doing the work over and over here. Who calls this? Is this something 
> we should think about caching? It would be easy to make a map of ConstString 
> (demangled name) to ConstString (result of this function call) and we can 
> possibly improve the efficiency of this. 
The only use of `GetDemangledFunctionNameWithoutArguments` is 
`Mangled::GetName`, and the only time it's ever actually executed is if the 
argument to `GetName` is `ePreferDemangledWithoutArguments`.  This happens 
twice from what I can see: `Symbol::GetNameNoArguments` and 
`Function::GetNameNoArguments`. I believe these methods are used for dumping 
and formatting.

Not sure how often they are actually used though. May be useful to profile and 
potentially cache if it's used enough?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105215

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

[Lldb-commits] [PATCH] D105215: [lldb] Remove CPlusPlusLanguage from Mangled

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

My comments were mostly about how we should refactor in a perfect world. Not 
sure we need to solve that right away. I am ok with this patch as it is if Jim 
is.

Jim? What do you think about this patch? OK? I am good if you are.




Comment at: lldb/source/Core/Mangled.cpp:322
   if (preference == ePreferDemangledWithoutArguments) {
-return GetDemangledNameWithoutArguments(m_mangled, demangled);
+if (Language *lang = Language::FindPlugin(GuessLanguage())) {
+  return lang->GetDemangledFunctionNameWithoutArguments(*this);

bulbazord wrote:
> clayborg wrote:
> > Maybe we should make a Language::FindPlugin(...) that like:
> > ```
> > Language *Language::FindPlugin(Mangled::ManglingScheme mangling_scheme);
> > ```
> > Should be easy to add since this change is kind of about refactoring and 
> > putting the code into plug-ins. It is essentially what "lldb::LanguageType 
> > Mangled::GuessLanguage() const" is doing. That code could be moved to where 
> > Language::FindPlugin(...) lives.
> `GuessLanguage` has a bunch of uses outside of `Mangled` itself, so I think 
> that would make more sense if we performed other refactors first.
> 
> I don't think putting this into another `Language::FindPlugin` function is a 
> good idea because (as I understand it) mangling schemes aren't specific to 
> languages. 
We _are_ using GuessLanguage to guess the language and we are using a language 
specific plug-in to do the work. So effectively it is the same thing.

I wonder if GetName(...) should take an extra Language parameter? If a 
lldb_private::Function wants the name as , then we _do_ know the language, then 
it could send the language in for it to be more correct. Itanium name mangling 
can mangle different languages right? Is there any way to differentiate 
correctly without actually knowing the language?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105215

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

[Lldb-commits] [PATCH] D105215: [lldb] Remove CPlusPlusLanguage from Mangled

[Jim Ingham via Phabricator via lldb-commits]

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

I still think the "Demangled" part of the 
GetDemangledFunctionNameWithoutArguments is redundant.  I'm not sure what sense 
it would make to get a mangled name without arguments?  And you could have a 
language that doesn't mangle symbols but still has namespaces, classes & 
overloading, for which the notion of getting the full name w/o arguments makes 
sense even though no mangling/demangling is involved.  But the current name 
isn't horrible.

We also have a general awkwardness with demangling names using Languages 
because C++ by itself is underdetermined - you really need {C++, mangling 
scheme}.  We just end up guessing about the mangling based on symbols - or in 
this case so far as I can see we only ever handle the Itanium mangling scheme.

But this patch doesn't make that worse.  So this is fine by me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105215

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

[Lldb-commits] [PATCH] D106192: [LLDB][GUI] Add Create Target form

[Greg Clayton via Phabricator via lldb-commits]

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

Many inline comments with simple fixes! Very close.




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2658
+  TargetCreateFormDelegate(Debugger &debugger) : m_debugger(debugger) {
+m_executable_field = AddFileField("Executable", "", true, true);
+m_core_file_field = AddFileField("Core File", "", true, false);

Lots of "true, true" and "true, false" in the file field parameters. Add 
comments with parameter names:
```
m_executable_field = AddFileField("Executable", "", *need_to_exist=*/true, 
/*required=*/true);
```




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2659-2662
+m_core_file_field = AddFileField("Core File", "", true, false);
+m_symbol_file_field = AddFileField("Symbol File", "", true, false);
+m_show_advanced_field = AddBooleanField("Show advanced settings.", false);
+m_remote_file_field = AddFileField("Remote File", "", false, false);

Ditto for all these: add comment variable names as suggested above.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2663
+m_remote_file_field = AddFileField("Remote File", "", false, false);
+m_arch_field = AddArchField("Architecture", "", false);
+m_platform_field = AddPlatformPluginField(debugger);

inlined comment for "false" above



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2690
+load_depentents_options.push_back(
+std::string("Only if the \"Executable\" is an executable file."));
+load_depentents_options.push_back(std::string("Yes."));

I would vote to shorten this string with something like "Executable only" with 
no trailing '.' character.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2691
+std::string("Only if the \"Executable\" is an executable file."));
+load_depentents_options.push_back(std::string("Yes."));
+load_depentents_options.push_back(std::string("No."));

remove the '.'



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2692
+load_depentents_options.push_back(std::string("Yes."));
+load_depentents_options.push_back(std::string("No."));
+return load_depentents_options;

remote the '.'



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2698
+std::string choice = m_load_dependent_files_field->GetChoiceContent();
+if (choice == "No.")
+  return eLoadDependentsNo;

remove the '.'. You might want to create a constexpr string value as a static 
outside of the functions and use them in GetLoadDependentFilesChoices() and 
GetLoadDependentFiles(). The static would be:
```
static constexpr const char *kLoadDependentFilesNo = "No";
static constexpr const char *kLoadDependentFilesYes = "Yes";
static constexpr const char *kLoadDependentFilesExecOnly = "Only for executable 
files";
```
Then use these in the GetLoadDependentFilesChoices() and 
GetLoadDependentFiles() functions. That way if you change the strings, you only 
have to change one location.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2700
+  return eLoadDependentsNo;
+if (choice == "Yes.")
+  return eLoadDependentsYes;

remove the '.'



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2770-2785
+PlatformSP platform_sp = target_sp->GetPlatform();
+if (!platform_sp) {
+  SetError("No platform found for target!");
+  return;
+}
+
+FileSpec remote_file_spec = m_remote_file_field->GetFileSpec();

Remove all of this (getting the platform, putting the file), no need to do 
anything other than module_sp->SetPlatformFileSpec(...) that you are doing 
below. This information will be used in Process::Launch(...) to do the right 
thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106192

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

[Lldb-commits] [PATCH] D106171: [lldb] Avoid moving ThreadPlanSP from plans vector

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 362229.
kastiglione added a comment.

Re-simplify following discussion with Adrian and Jim.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106171

Files:
  lldb/source/Target/ThreadPlanStack.cpp


Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -142,7 +142,10 @@
 lldb::ThreadPlanSP ThreadPlanStack::PopPlan() {
   assert(m_plans.size() > 1 && "Can't pop the base thread plan");
 
-  lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+  // Note that `std::move` on `m_plans` elements can break its non-null
+  // invariant - if also calling functions (such as `WillPop`) that can't
+  // guarantee `m_plans` is not accessed in its inconsistent state.
+  lldb::ThreadPlanSP plan_sp = m_plans.back();
   m_completed_plans.push_back(plan_sp);
   plan_sp->WillPop();
   m_plans.pop_back();
@@ -152,7 +155,10 @@
 lldb::ThreadPlanSP ThreadPlanStack::DiscardPlan() {
   assert(m_plans.size() > 1 && "Can't discard the base thread plan");
 
-  lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+  // Note that `std::move` on `m_plans` elements can break its non-null
+  // invariant - if also calling functions (such as `WillPop`) that can't
+  // guarantee `m_plans` is not accessed in its inconsistent state.
+  lldb::ThreadPlanSP plan_sp = m_plans.back();
   m_discarded_plans.push_back(plan_sp);
   plan_sp->WillPop();
   m_plans.pop_back();


Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -142,7 +142,10 @@
 lldb::ThreadPlanSP ThreadPlanStack::PopPlan() {
   assert(m_plans.size() > 1 && "Can't pop the base thread plan");
 
-  lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+  // Note that `std::move` on `m_plans` elements can break its non-null
+  // invariant - if also calling functions (such as `WillPop`) that can't
+  // guarantee `m_plans` is not accessed in its inconsistent state.
+  lldb::ThreadPlanSP plan_sp = m_plans.back();
   m_completed_plans.push_back(plan_sp);
   plan_sp->WillPop();
   m_plans.pop_back();
@@ -152,7 +155,10 @@
 lldb::ThreadPlanSP ThreadPlanStack::DiscardPlan() {
   assert(m_plans.size() > 1 && "Can't discard the base thread plan");
 
-  lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+  // Note that `std::move` on `m_plans` elements can break its non-null
+  // invariant - if also calling functions (such as `WillPop`) that can't
+  // guarantee `m_plans` is not accessed in its inconsistent state.
+  lldb::ThreadPlanSP plan_sp = m_plans.back();
   m_discarded_plans.push_back(plan_sp);
   plan_sp->WillPop();
   m_plans.pop_back();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ec1a491 - Create synthetic symbol names on demand to improve memory consumption and startup times.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2021-07-27T16:51:12-07:00
New Revision: ec1a49170129ddb62f268ff0b3f12b3d09987a7e

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

LOG: Create synthetic symbol names on demand to improve memory consumption and 
startup times.

This is a resubmission of https://reviews.llvm.org/D105160 after fixing testing 
issues.

This fix was created after profiling the target creation of a large C/C++/ObjC 
application that contained almost 4,000,000 redacted symbol names. The symbol 
table parsing code was creating names for each of these synthetic symbols and 
adding them to the name indexes. The code was also adding the object file 
basename to the end of the symbol name which doesn't allow symbols from 
different shared libraries to share the names in the constant string pool.

Prior to this fix this was creating 180MB of "___lldb_unnamed_symbol" symbol 
names and was taking a long time to generate each name, add them to the string 
pool and then add each of these names to the name index.

This patch fixes the issue by:

not adding a name to synthetic symbols at creation time, and allows name to be 
dynamically generated when accessed
doesn't add synthetic symbol names to the name indexes, but catches this 
special case as name lookup time. Users won't typically set breakpoints or 
lookup these synthetic names, but support was added to do the lookup in case it 
does happen
removes the object file baseanme from the generated names to allow the names to 
be shared in the constant string pool
Prior to this fix the startup times for a large application was:
12.5 seconds (cold file caches)
8.5 seconds (warm file caches)

After this fix:
9.7 seconds (cold file caches)
5.7 seconds (warm file caches)

The names of the symbols are auto generated by appending the symbol's UserID to 
the end of the "___lldb_unnamed_symbol" string and is only done when the name 
is requested from a synthetic symbol if it has no name.

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

Added: 


Modified: 
lldb/include/lldb/Symbol/ObjectFile.h
lldb/include/lldb/Symbol/Symbol.h
lldb/include/lldb/Symbol/Symtab.h
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Symbol/ObjectFile.cpp
lldb/source/Symbol/Symbol.cpp
lldb/source/Symbol/Symtab.cpp
lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
lldb/test/Shell/SymbolFile/Breakpad/symtab.test

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index 24bb3b106eb34..4ccd7f92064d5 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -722,8 +722,6 @@ class ObjectFile : public 
std::enable_shared_from_this,
   /// false otherwise.
   bool SetModulesArchitecture(const ArchSpec &new_arch);
 
-  ConstString GetNextSyntheticSymbolName();
-
   static lldb::DataBufferSP MapFileData(const FileSpec &file, uint64_t Size,
 uint64_t Offset);
 

diff  --git a/lldb/include/lldb/Symbol/Symbol.h 
b/lldb/include/lldb/Symbol/Symbol.h
index 3abe3114863de..cb23854687029 100644
--- a/lldb/include/lldb/Symbol/Symbol.h
+++ b/lldb/include/lldb/Symbol/Symbol.h
@@ -113,14 +113,20 @@ class Symbol : public SymbolContextScope {
   lldb::LanguageType GetLanguage() const {
 // TODO: See if there is a way to determine the language for a symbol
 // somehow, for now just return our best guess
-return m_mangled.GuessLanguage();
+return GetMangled().GuessLanguage();
   }
 
   void SetID(uint32_t uid) { m_uid = uid; }
 
-  Mangled &GetMangled() { return m_mangled; }
+  Mangled &GetMangled() {
+SynthesizeNameIfNeeded();
+return m_mangled;
+  }
 
-  const Mangled &GetMangled() const { return m_mangled; }
+  const Mangled &GetMangled() const {
+SynthesizeNameIfNeeded();
+return m_mangled;
+  }
 
   ConstString GetReExportedSymbolName() const;
 
@@ -149,6 +155,8 @@ class Symbol : public SymbolContextScope {
 
   bool IsSynthetic() const { return m_is_synthetic; }
 
+  bool IsSyntheticWithAutoGeneratedName() const;
+
   void SetIsSynthetic(bool b) { m_is_synthetic = b; }
 
   bool GetSizeIsSynthesized() const { return m_size_is_synthesized; }
@@ -166,9 +174,9 @@ class Symbol : public SymbolContextScope {
   bool IsTrampoline() const;
 
   bool IsIndirect() const;
-  
+
   bool IsWeak() const { return m_is_weak; }
-  
+
   void SetIsWeak (bool b) { m_is_weak = b; }
 
   bool GetByteSizeIsValid() const { return m_size_is_valid; }
@@ -223,6 +231,10 @@ class Symbol : public SymbolContextScope {
 
   bool ContainsFileAddress(lldb::addr_t file_addr) const;
 
+  static llvm::StringRef GetSyntheticSymbolPrefix(

[Lldb-commits] [PATCH] D106837: Create synthetic symbol names on demand to improve memory consumption and startup times.

[Greg Clayton 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 rGec1a49170129: Create synthetic symbol names on demand to 
improve memory consumption and… (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106837

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Symbol/Symbol.h
  lldb/include/lldb/Symbol/Symtab.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Symbol/Symtab.cpp
  lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
  lldb/test/Shell/SymbolFile/Breakpad/symtab.test

Index: lldb/test/Shell/SymbolFile/Breakpad/symtab.test
===
--- lldb/test/Shell/SymbolFile/Breakpad/symtab.test
+++ lldb/test/Shell/SymbolFile/Breakpad/symtab.test
@@ -5,7 +5,7 @@
 # CHECK-LABEL: (lldb) image dump symtab symtab.out
 # CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 5:
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
-# CHECK: [0]  0  SX Code0x00400x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}$$symtab.out
+# CHECK: [0]  0  SX Code0x00400x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}
 # CHECK: [1]  0   X Code0x004000b00x000c 0x f1_func
 # CHECK: [2]  0   X Code0x004000a00x000d 0x func_only
 # CHECK: [3]  0   X Code0x004000c00x0010 0x f2
Index: lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
@@ -3,8 +3,8 @@
 
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
 # CHECK: [0]  1 SourceFile  0x0x 0x0004 -
-# CHECK: [1]  2  SX Code0x002011800x0010 0x ___lldb_unnamed_symbol1$${{.*}}
-# CHECK: [2]  3  SX Code0x002011900x0006 0x ___lldb_unnamed_symbol2$${{.*}}
+# CHECK: [1]  2  SX Code0x002011800x0010 0x ___lldb_unnamed_symbol{{[0-9]*}}
+# CHECK: [2]  3  SX Code0x002011900x0006 0x ___lldb_unnamed_symbol{{[0-9]*}}
 
 --- !ELF
 FileHeader:
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -300,8 +300,10 @@
   // Don't let trampolines get into the lookup by name map If we ever need
   // the trampoline symbols to be searchable by name we can remove this and
   // then possibly add a new bool to any of the Symtab functions that
-  // lookup symbols by name to indicate if they want trampolines.
-  if (symbol->IsTrampoline())
+  // lookup symbols by name to indicate if they want trampolines. We also
+  // don't want any synthetic symbols with auto generated names in the
+  // name lookups.
+  if (symbol->IsTrampoline() || symbol->IsSyntheticWithAutoGeneratedName())
 continue;
 
   // If the symbol's name string matched a Mangled::ManglingScheme, it is
@@ -628,6 +630,36 @@
   }
 }
 
+uint32_t Symtab::GetNameIndexes(ConstString symbol_name,
+std::vector &indexes) {
+  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
+  const uint32_t count = name_to_index.GetValues(symbol_name, indexes);
+  if (count)
+return count;
+  // Synthetic symbol names are not added to the name indexes, but they start
+  // with a prefix and end with a the symbol UserID. This allows users to find
+  // these symbols without having to add them to the name indexes. These
+  // queries will not happen very often since the names don't mean anything, so
+  // performance is not paramount in this case.
+  llvm::StringRef name = symbol_name.GetStringRef();
+  // String the synthetic prefix if the name starts with it.
+  if (!name.consume_front(Symbol::GetSyntheticSymbolPrefix()))
+return 0; // Not a synthetic symbol name
+
+  // Extract the user ID from the symbol name
+  unsigned long long uid = 0;
+  if (getAsUnsignedIn

[Lldb-commits] [lldb] 3c45476 - Fix a thinko in the parsing of substitutions in CommandObjectRegexCommand.

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2021-07-27T18:58:56-07:00
New Revision: 3c4547692368239fca21ec294a5a406ea5a44889

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

LOG: Fix a thinko in the parsing of substitutions in CommandObjectRegexCommand.

The old code incorrectly calculated the start position for the search
for the third (and subsequent) instance of a particular substitution
pattern (e.g. %1).

I also added a few test cases for this parsing covering this failure.

Added: 
lldb/test/API/commands/command/regex/TestRegexCommand.py
lldb/test/API/commands/command/regex/echo_command.py

Modified: 
lldb/source/Commands/CommandObjectRegexCommand.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectRegexCommand.cpp 
b/lldb/source/Commands/CommandObjectRegexCommand.cpp
index be0f84d8142c5..46295421834a8 100644
--- a/lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ b/lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -43,7 +43,7 @@ bool CommandObjectRegexCommand::DoExecute(llvm::StringRef 
command,
  percent_var, idx)) != std::string::npos;) {
 new_command.erase(percent_var_idx, percent_var_len);
 new_command.insert(percent_var_idx, match_str);
-idx += percent_var_idx + match_str.size();
+idx = percent_var_idx + match_str.size();
   }
 }
   }

diff  --git a/lldb/test/API/commands/command/regex/TestRegexCommand.py 
b/lldb/test/API/commands/command/regex/TestRegexCommand.py
new file mode 100644
index 0..5a70dc919f7f8
--- /dev/null
+++ b/lldb/test/API/commands/command/regex/TestRegexCommand.py
@@ -0,0 +1,31 @@
+"""
+This tests some simple examples of parsing regex commands
+"""
+
+import os
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestCommandRegexParsing(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_sample_rename_this(self):
+"""Try out some simple regex commands, make sure they parse 
correctly."""
+self.runCmd("command regex one-substitution 's/(.+)/echo-cmd %1-first 
%1-second %1-third/'")
+self.expect("one-substitution ASTRING",
+substrs = ["ASTRING-first", "ASTRING-second", 
"ASTRING-third"])
+
+self.runCmd("command regex two-substitution 's/([^ ]+) ([^ 
]+)/echo-cmd %1-first %2-second %1-third %2-fourth/'")
+self.expect("two-substitution ASTRING BSTRING",
+substrs = ["ASTRING-first", "BSTRING-second", 
"ASTRING-third", "BSTRING-fourth"])
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+self.runCmd("command script import " + 
os.path.join(self.getSourceDir(), "echo_command.py"))
+self.runCmd("command script add echo-cmd -f echo_command.echo_command")

diff  --git a/lldb/test/API/commands/command/regex/echo_command.py 
b/lldb/test/API/commands/command/regex/echo_command.py
new file mode 100644
index 0..cc2d7ebce2d57
--- /dev/null
+++ b/lldb/test/API/commands/command/regex/echo_command.py
@@ -0,0 +1,6 @@
+import lldb
+
+def echo_command(debugger, args, result, dict):
+result.Print(args+'\n')
+result.SetStatus(lldb.eReturnStatusSuccessFinishResult)
+return True



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

[Lldb-commits] [PATCH] D106355: [DWARF5] Only fallback to manual index if no entry was found

[Kim-Anh Tran via Phabricator via lldb-commits]

kimanh updated this revision to Diff 362287.
kimanh marked an inline comment as done.
kimanh added a comment.

Remove braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106355

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -125,6 +125,7 @@
 void DebugNamesDWARFIndex::GetGlobalVariables(
 const DWARFUnit &cu, llvm::function_ref callback) {
   uint64_t cu_offset = cu.GetOffset();
+  bool found_entry_for_cu = false;
   for (const DebugNames::NameIndex &ni: *m_debug_names_up) {
 for (DebugNames::NameTableEntry nte: ni) {
   uint64_t entry_offset = nte.getEntryOffset();
@@ -135,6 +136,7 @@
 if (entry_or->getCUOffset() != cu_offset)
   continue;
 
+found_entry_for_cu = true;
 if (!ProcessEntry(*entry_or, callback,
   llvm::StringRef(nte.getString(
   return;
@@ -142,8 +144,10 @@
   MaybeLogLookupError(entry_or.takeError(), ni, nte.getString());
 }
   }
-
-  m_fallback.GetGlobalVariables(cu, callback);
+  // If no name index for that particular CU was found, fallback to
+  // creating the manual index.
+  if (!found_entry_for_cu)
+m_fallback.GetGlobalVariables(cu, callback);
 }
 
 void DebugNamesDWARFIndex::GetCompleteObjCClass(


Index: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -125,6 +125,7 @@
 void DebugNamesDWARFIndex::GetGlobalVariables(
 const DWARFUnit &cu, llvm::function_ref callback) {
   uint64_t cu_offset = cu.GetOffset();
+  bool found_entry_for_cu = false;
   for (const DebugNames::NameIndex &ni: *m_debug_names_up) {
 for (DebugNames::NameTableEntry nte: ni) {
   uint64_t entry_offset = nte.getEntryOffset();
@@ -135,6 +136,7 @@
 if (entry_or->getCUOffset() != cu_offset)
   continue;
 
+found_entry_for_cu = true;
 if (!ProcessEntry(*entry_or, callback,
   llvm::StringRef(nte.getString(
   return;
@@ -142,8 +144,10 @@
   MaybeLogLookupError(entry_or.takeError(), ni, nte.getString());
 }
   }
-
-  m_fallback.GetGlobalVariables(cu, callback);
+  // If no name index for that particular CU was found, fallback to
+  // creating the manual index.
+  if (!found_entry_for_cu)
+m_fallback.GetGlobalVariables(cu, callback);
 }
 
 void DebugNamesDWARFIndex::GetCompleteObjCClass(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits