[Lldb-commits] [lldb] d510b5f - [lldb][AArch64] Annotate synchronous tag faults

2021-07-29 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2021-07-29T10:26:37+01:00
New Revision: d510b5f199d6e7a3062b5a6ea43181c4cc00a605

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

LOG: [lldb][AArch64] Annotate synchronous tag faults

In the latest Linux kernels synchronous tag faults
include the tag bits in their address.
This change adds logical and allocation tags to the
description of synchronous tag faults.
(asynchronous faults have no address)

Process 1626 stopped
* thread #1, name = 'a.out', stop reason = signal SIGSEGV: sync tag check fault 
(fault address: 0x900f7ff9010 logical tag: 0x9 allocation tag: 0x0)

This extends the existing description and will
show as much as it can on the rare occasion something
fails.

This change supports AArch64 MTE only but other
architectures could be added by extending the
switch at the start of AnnotateSyncTagCheckFault.
The rest of the function is generic code.

Tests have been added for synchronous and asynchronous
MTE faults.

Reviewed By: omjavaid

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

Added: 
lldb/test/API/linux/aarch64/mte_tag_faults/Makefile

lldb/test/API/linux/aarch64/mte_tag_faults/TestAArch64LinuxMTEMemoryTagFaults.py
lldb/test/API/linux/aarch64/mte_tag_faults/main.c

Modified: 
lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
lldb/source/Plugins/Process/Linux/NativeThreadLinux.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index d8ba5415a983e..a7e4e9b13ff0a 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -26,6 +26,7 @@
 #include "llvm/ADT/SmallString.h"
 
 #include "Plugins/Process/POSIX/CrashReason.h"
+#include "Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h"
 
 #include 
 // Try to define a macro to encapsulate the tgkill syscall
@@ -299,11 +300,69 @@ void NativeThreadLinux::SetStoppedBySignal(uint32_t signo,
   ? CrashReason::eInvalidAddress
   : GetCrashReason(*info);
   m_stop_description = GetCrashReasonString(reason, *info);
+
+  if (reason == CrashReason::eSyncTagCheckFault) {
+AnnotateSyncTagCheckFault(info);
+  }
+
   break;
 }
   }
 }
 
+void NativeThreadLinux::AnnotateSyncTagCheckFault(const siginfo_t *info) {
+  int32_t allocation_tag_type = 0;
+  switch (GetProcess().GetArchitecture().GetMachine()) {
+  // aarch64_32 deliberately not here because there's no 32 bit MTE
+  case llvm::Triple::aarch64:
+  case llvm::Triple::aarch64_be:
+allocation_tag_type = MemoryTagManagerAArch64MTE::eMTE_allocation;
+break;
+  default:
+return;
+  }
+
+  auto details =
+  GetRegisterContext().GetMemoryTaggingDetails(allocation_tag_type);
+  if (!details) {
+llvm::consumeError(details.takeError());
+return;
+  }
+
+  // We assume that the stop description is currently:
+  // signal SIGSEGV: sync tag check fault (fault address: )
+  // Remove the closing )
+  m_stop_description.pop_back();
+
+  std::stringstream ss;
+  lldb::addr_t fault_addr = reinterpret_cast(info->si_addr);
+  std::unique_ptr manager(std::move(details->manager));
+
+  ss << " logical tag: 0x" << std::hex << manager->GetLogicalTag(fault_addr);
+
+  std::vector allocation_tag_data;
+  // The fault address may not be granule aligned. ReadMemoryTags will granule
+  // align any range you give it, potentially making it larger.
+  // To prevent this set len to 1. This always results in a range that is at
+  // most 1 granule in size and includes fault_addr.
+  Status status = GetProcess().ReadMemoryTags(allocation_tag_type, fault_addr,
+  1, allocation_tag_data);
+
+  if (status.Success()) {
+llvm::Expected> allocation_tag =
+manager->UnpackTagsData(allocation_tag_data, 1);
+if (allocation_tag) {
+  ss << " allocation tag: 0x" << std::hex << allocation_tag->front() << 
")";
+} else {
+  llvm::consumeError(allocation_tag.takeError());
+  ss << ")";
+}
+  } else
+ss << ")";
+
+  m_stop_description += ss.str();
+}
+
 bool NativeThreadLinux::IsStopped(int *signo) {
   if (!StateIsStoppedState(m_state, false))
 return false;

diff  --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h 
b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
index f03de755c7bf3..c18665b0107ef 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
@@ -102,6 +102,11 @@ class NativeThreadLinux : public NativeThreadProtocol {
 
   void SetStopped();
 
+  /// Extend m_stop_description with logical and allocation tag values.
+  /// If there is 

[Lldb-commits] [PATCH] D105178: [lldb][AArch64] Annotate synchronous tag faults

2021-07-29 Thread 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 rGd510b5f199d6: [lldb][AArch64] Annotate synchronous tag 
faults (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105178

Files:
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/test/API/linux/aarch64/mte_tag_faults/Makefile
  
lldb/test/API/linux/aarch64/mte_tag_faults/TestAArch64LinuxMTEMemoryTagFaults.py
  lldb/test/API/linux/aarch64/mte_tag_faults/main.c

Index: lldb/test/API/linux/aarch64/mte_tag_faults/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_faults/main.c
@@ -0,0 +1,59 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Set bits 59-56 to tag, removing any existing tag
+static char *set_tag(char *ptr, size_t tag) {
+  return (char *)(((size_t)ptr & ~((size_t)0xf << 56)) | (tag << 56));
+}
+
+int main(int argc, char const *argv[]) {
+  // We assume that the test runner has checked we're on an MTE system
+
+  // Only expect to get the fault type
+  if (argc != 2)
+return 1;
+
+  unsigned long prctl_arg2 = 0;
+  if (!strcmp(argv[1], "sync"))
+prctl_arg2 = PR_MTE_TCF_SYNC;
+  else if (!strcmp(argv[1], "async"))
+prctl_arg2 = PR_MTE_TCF_ASYNC;
+  else
+return 1;
+
+  // Set fault type
+  if (prctl(PR_SET_TAGGED_ADDR_CTRL, prctl_arg2, 0, 0, 0))
+return 1;
+
+  // Allocate some memory with tagging enabled that we
+  // can read/write if we use correct tags.
+  char *buf = mmap(0, sysconf(_SC_PAGESIZE), PROT_MTE | PROT_READ | PROT_WRITE,
+   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (buf == MAP_FAILED)
+return 1;
+
+  // Our pointer will have tag 9
+  char *tagged_buf = set_tag(buf, 9);
+  // Set allocation tags for the first 2 granules
+  __arm_mte_set_tag(set_tag(tagged_buf, 9));
+  __arm_mte_set_tag(set_tag(tagged_buf + 16, 10));
+
+  // Confirm that we can write when tags match
+  *tagged_buf = ' ';
+
+  // Breakpoint here
+  // Faults because tag 9 in the ptr != allocation tag of 10.
+  // + 16 puts us in the second granule and +1 makes the fault address
+  // misaligned relative to the granule size. This misalignment must
+  // be accounted for by lldb-server.
+  *(tagged_buf + 16 + 1) = '?';
+
+  return 0;
+}
Index: lldb/test/API/linux/aarch64/mte_tag_faults/TestAArch64LinuxMTEMemoryTagFaults.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/mte_tag_faults/TestAArch64LinuxMTEMemoryTagFaults.py
@@ -0,0 +1,62 @@
+"""
+Test reporting of MTE tag access faults.
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxMTEMemoryTagFaultsTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup_mte_test(self, fault_type):
+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 {}".format(fault_type), 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'])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_tag_fault_sync(self):
+self.setup_mte_test("sync")
+# The logical tag should be included in the fault address
+# and we know what the bottom byte should be.
+# It will be 0x10 (to be in the 2nd granule), +1 to be 0x11.
+# Which tests that lldb-server handles fault addresses that
+# are not granule aligned.
+self.expect("continue",
+patterns=[
+"\* thread #1, name = 'a.out', stop reason = signal SIGSEGV: "
+"sync tag check fault \(fault address: 0x9[0-9A-Fa-f]+11\ "
+"logical tag: 0x9 allocation tag: 0xa\)"])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_tag_fault_async(self):
+self.setup_mte_test("async")
+self.expect("continue",
+substrs=[
+"* thread #1, name = 'a.out', stop reason = "
+ 

[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization

2021-07-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added inline comments.



Comment at: 
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp:78
+  : LLDB_INVALID_THREAD_ID;
+result.AppendErrorWithFormat(
+"Thread index %lu is out of range (valid values are 0 - %u).\n", tid,

This generates a compiler warning:
```
[3344/4085] Building CXX object 
tools/lldb/source/Plugins/TraceExporter/ctf/CMakeFiles/lldbPluginTraceExporterCTF.dir/CommandObjectThreadTraceExportCTF.cpp.o
/Users/teemperor/3llvm/llvm-project/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp:79:82:
 warning: format specifies type 'unsigned long long' but the argument has type 
'size_t' (aka 'unsigned long') [-Wformat]
"Thread index %" PRIu64 " is out of range (valid values are 0 - 
%u).\n", tid,
  ~ 
 ^~~
```

The variadic format methods are obsolete and you can just use 
`llvm::formatv(...).str()` until we have a type-safe replacement.


Repository:
  rG LLVM Github Monorepo

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] D106985: [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

2021-07-29 Thread Michał Górny via Phabricator via lldb-commits
mgorny updated this revision to Diff 362729.
mgorny added a comment.

Removed obsolete FIXME from server as well.


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

https://reviews.llvm.org/D106985

Files:
  lldb/docs/lldb-platform-packets.txt
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -181,6 +181,8 @@
 return self.qfProcessInfo(packet)
 if packet.startswith("qPathComplete:"):
 return self.qPathComplete()
+if packet.startswith("vFile:"):
+return self.vFile(packet)
 
 return self.other(packet)
 
@@ -288,6 +290,9 @@
 def qPathComplete(self):
 return ""
 
+def vFile(self, packet):
+return ""
+
 """
 Raised when we receive a packet for which there is no default action.
 Override the responder class to implement behavior suitable for the test at
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -0,0 +1,25 @@
+from gdbclientutils import *
+
+class TestGDBRemotePlatformFile(GDBRemoteTestBase):
+
+def test_file_open(self):
+"""Test mock-opening a remote file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F10"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.runCmd("platform file open /some/file.txt -v 0755")
+self.assertPacketLogContains([
+"vFile:open:2f736f6d652f66696c652e747874,020a,01ed"
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -501,10 +501,6 @@
   packet.GetHexByteStringTerminatedBy(path, ',');
   if (!path.empty()) {
 if (packet.GetChar() == ',') {
-  // FIXME
-  // The flag values for OpenOptions do not match the values used by GDB
-  // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-  // * rdar://problem/46788934
   auto flags = File::OpenOptions(packet.GetHexMaxU32(false, 0));
   if (packet.GetChar() == ',') {
 mode_t mode = packet.GetHexMaxU32(false, 0600);
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -90,8 +90,8 @@
   .Cases("a+", "ab+", "a+b",
  eOpenOptionReadWrite | eOpenOptionAppend |
  eOpenOptionCanCreate)
-  .Default(OpenOptions());
-  if (opts)
+  .Default(eOpenOptionInvalid);
+  if (opts != eOpenOptionInvalid)
 return opts;
   return llvm::createStringError(
   llvm::inconvertibleErrorCode(),
Index: lldb/include/lldb/Host/File.h
===
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -39,27 +39,29 @@
   // NB this enum is used in the lldb platform gdb-remote packet
   // vFile:open: and existing values cannot be modified.
   //
-  // FIXME
-  // These values do not match the values used by GDB
+  // The first set of values is defined by gdb headers and can be found
+  // in the documentation at:
   // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-  // * rdar://problem/46788934
+  //
+  // The second half are LLDB extensions and use the highest uint32_t bits
+  // to avoid risk of collisions with future gdb remote protocol changes.
   enum OpenOptions : uint32_t {
-eOpenOptionReadOnly = (1u << 0),  // Open file for reading (only)
-eOpenOptionWriteOnly = (1u << 1), // Open file for writing (only)
-eOpenOptionReadWrite =
-eOpenOptionReadOnly |
-eOpenOptionWriteOnly, // Open file for both readin

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

2021-07-29 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 updated this revision to Diff 362735.
augusto2112 added a comment.

Add test to test/Shell/lit-lldb-init.in


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/include/lldb/Target/Target.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/Section.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/test/Shell/lit-lldb-init.in

Index: lldb/test/Shell/lit-lldb-init.in
===
--- lldb/test/Shell/lit-lldb-init.in
+++ lldb/test/Shell/lit-lldb-init.in
@@ -6,3 +6,4 @@
 settings set target.auto-apply-fixits false
 settings set target.inherit-tcc true
 settings set target.detach-on-error false
+settings set target.verify-file-cache-memory-reads true
Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -175,6 +175,9 @@
   def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
 DefaultFalse,
 Desc<"Enable debugging of LLDB-internal utility expressions.">;
+  def VerifyFileCacheMemoryReads: Property<"verify-file-cache-memory-reads", "Boolean">,
+DefaultFalse,
+Desc<"Verify that memory read from the file-cache is identical to the memory read from the process.">;
 }
 
 let Definition = "process_experimental" in {
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);
+
+  if (GetVerifyFileCacheMemoryReads()) {
+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) {
+std::unique_ptr live_buf =
+std::make_unique(dst_len);
+bytes_read = m_process_sp->ReadMemory(load_addr, live_buf.get(),
+  dst_len, error);
+if (bytes_read == dst_len) {
+  lldbassert(memcmp(live_buf.get(), dst, dst_len) == 0 &&
+ "File cache and live memory diverge!");
+}
+  }
 }
   }
+
+  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);
+  }
 }
   }
 
@@ -4368,6 +4381,17 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, debug);
 }
 
+bool TargetProperties::GetVerifyFileCacheMemoryReads() const {
+  const uint32_t idx = ePropertyVerifyFileCacheMemoryReads;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+}
+
+void TargetProperties::SetVerifyFileCacheMemoryReads(bool verify) {
+  const uint32_t idx = ePropertyVerifyFileCacheMemoryReads;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, verify);
+}
+
 // Target::TargetEventData
 
 Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp)
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/packages/Python/lldbsuite/test/lldbtest.py
===
--- ll

[Lldb-commits] [lldb] 77e9d10 - [lldb] Assert filecache and live memory match on debug under a setting

2021-07-29 Thread Augusto Noronha via lldb-commits

Author: Augusto Noronha
Date: 2021-07-29T10:29:34-03:00
New Revision: 77e9d10f0fbfe04a14e6ce61753376dd78e0c2f0

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

LOG: [lldb] Assert filecache and live memory match on debug under a setting

Added: 


Modified: 
lldb/include/lldb/Core/Section.h
lldb/include/lldb/Target/Target.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Core/Section.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/Shell/lit-lldb-init.in

Removed: 




diff  --git a/lldb/include/lldb/Core/Section.h 
b/lldb/include/lldb/Core/Section.h
index 3d4ab154e743f..3914469aafc1a 100644
--- a/lldb/include/lldb/Core/Section.h
+++ b/lldb/include/lldb/Core/Section.h
@@ -236,6 +236,8 @@ class Section : public 
std::enable_shared_from_this,
 
   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

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index ac8d002b09a12..d15370259f467 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -229,6 +229,10 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  void SetVerifyFileCacheMemoryReads(bool debug);
+
+  bool GetVerifyFileCacheMemoryReads() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 7e1fdce36ede6..366c40db81507 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -798,6 +798,9 @@ def setUpCommands(cls):
 'settings set symbols.clang-modules-cache-path "{}"'.format(
 configuration.lldb_module_cache_dir),
 "settings set use-color false",
+
+# Verify that file cache and live memory always match.
+"settings set target.verify-file-cache-memory-reads true",
 ]
 
 # Set any user-overridden settings.

diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index a5a10141aa649..af5cee2f9b9b9 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -599,3 +599,9 @@ size_t SectionList::Slide(addr_t slide_amount, bool 
slide_children) {
   }
   return count;
 }
+
+bool Section::IsReadOnly() {
+  auto permissions = Flags(GetPermissions());
+  return !permissions.Test(ePermissionsWritable) &&
+ permissions.Test(ePermissionsReadable);
+}

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 1f8e8c54fa9e1..33691e8e97856 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1763,21 +1763,34 @@ size_t Target::ReadMemory(const Address &addr, void 
*dst, size_t dst_len,
   // 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);
+
+  if (GetVerifyFileCacheMemoryReads()) {
+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) {
+std::unique_ptr live_buf =
+std::make_unique(dst_len);
+bytes_read = m_process_sp->ReadMemory(load_addr, live_buf.get(),
+  dst_len, error);
+if (bytes_read == dst_len) {
+  lldbassert(memcmp(live_buf.get(), dst, dst_len) == 0 &&
+ "File cache and live memory diverge!");
+}
+  }
 }
   }
+
+  if (file_cache_by

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

2021-07-29 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 closed this revision.
augusto2112 added a comment.

Commit 77e9d10f0fbfe04a14e6ce61753376dd78e0c2f0 



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] D106999: [LLDB][GUI] Add Environment Variable Field

2021-07-29 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev updated this revision to Diff 362755.
OmarEmaraDev added a comment.

- Generalize the implementation to any Mapping field


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106999

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1908,6 +1908,176 @@
   SelectionType m_selection_type;
 };
 
+template 
+class MappingFieldDelegate : public FieldDelegate {
+public:
+  MappingFieldDelegate(KeyFieldDelegateType key_field,
+   ValueFieldDelegateType value_field)
+  : m_key_field(key_field), m_value_field(value_field),
+m_selection_type(SelectionType::Key) {}
+
+  // Signify which element is selected. The key field or its value field.
+  enum class SelectionType { Key, Value };
+
+  // A mapping field is drawn as two text fields with a right arrow in between.
+  // The first field stores the key of the mapping and the second stores the
+  // value if the mapping.
+  //
+  // __[Key]_   __[Value]___
+  // |  | > |  |
+  // |__|   |__|
+  // - Error message if it exists.
+
+  // The mapping field has a height that is equal to the maximum height between
+  // the key and value fields.
+  int FieldDelegateGetHeight() override {
+return std::max(m_key_field.FieldDelegateGetHeight(),
+m_value_field.FieldDelegateGetHeight());
+  }
+
+  void DrawArrow(SubPad &surface) {
+surface.MoveCursor(0, 1);
+surface.PutChar(ACS_RARROW);
+  }
+
+  void FieldDelegateDraw(SubPad &surface, bool is_selected) override {
+Rect bounds = surface.GetFrame();
+Rect key_field_bounds, arrow_and_value_field_bounds;
+bounds.VerticalSplit(bounds.size.width / 2, key_field_bounds,
+ arrow_and_value_field_bounds);
+Rect arrow_bounds, value_field_bounds;
+arrow_and_value_field_bounds.VerticalSplit(1, arrow_bounds,
+   value_field_bounds);
+
+SubPad key_field_surface = SubPad(surface, key_field_bounds);
+SubPad arrow_surface = SubPad(surface, arrow_bounds);
+SubPad value_field_surface = SubPad(surface, value_field_bounds);
+
+bool key_is_selected =
+m_selection_type == SelectionType::Key && is_selected;
+m_key_field.FieldDelegateDraw(key_field_surface, key_is_selected);
+DrawArrow(arrow_surface);
+bool value_is_selected =
+m_selection_type == SelectionType::Value && is_selected;
+m_value_field.FieldDelegateDraw(value_field_surface, value_is_selected);
+  }
+
+  HandleCharResult SelectNext(int key) {
+if (FieldDelegateOnLastOrOnlyElement())
+  return eKeyNotHandled;
+
+if (!m_key_field.FieldDelegateOnLastOrOnlyElement()) {
+  return m_key_field.FieldDelegateHandleChar(key);
+}
+
+m_key_field.FieldDelegateExitCallback();
+m_selection_type = SelectionType::Value;
+m_value_field.FieldDelegateSelectFirstElement();
+return eKeyHandled;
+  }
+
+  HandleCharResult SelectPrevious(int key) {
+if (FieldDelegateOnFirstOrOnlyElement())
+  return eKeyNotHandled;
+
+if (!m_value_field.FieldDelegateOnFirstOrOnlyElement()) {
+  return m_value_field.FieldDelegateHandleChar(key);
+}
+
+m_value_field.FieldDelegateExitCallback();
+m_selection_type = SelectionType::Key;
+m_key_field.FieldDelegateSelectLastElement();
+return eKeyHandled;
+  }
+
+  HandleCharResult FieldDelegateHandleChar(int key) override {
+switch (key) {
+case '\t':
+  SelectNext(key);
+  return eKeyHandled;
+case KEY_SHIFT_TAB:
+  SelectPrevious(key);
+  return eKeyHandled;
+default:
+  break;
+}
+
+// If the key wasn't handled, pass the key to the selected field.
+if (m_selection_type == SelectionType::Key)
+  return m_key_field.FieldDelegateHandleChar(key);
+else
+  return m_value_field.FieldDelegateHandleChar(key);
+
+return eKeyNotHandled;
+  }
+
+  bool FieldDelegateOnFirstOrOnlyElement() override {
+return m_selection_type == SelectionType::Key;
+  }
+
+  bool FieldDelegateOnLastOrOnlyElement() override {
+return m_selection_type == SelectionType::Value;
+  }
+
+  void FieldDelegateSelectFirstElement() override {
+m_selection_type = SelectionType::Key;
+  }
+
+  void FieldDelegateSelectLastElement() override {
+m_selection_type = SelectionType::Value;
+  }
+
+  bool FieldDelegateHasError() override {
+return m_key_field.FieldDelegateHasError() ||
+   m_value_field.FieldDelegateHasError();
+  }
+
+  KeyFieldDelegateType &GetKeyField() { return m_key_field; }
+
+  ValueFieldDelegateType &GetValueField() { return m_value_field; }
+
+protected:
+  KeyFieldDelegateTyp

[Lldb-commits] [PATCH] D106999: [LLDB][GUI] Add Environment Variable Field

2021-07-29 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev marked an inline comment as done.
OmarEmaraDev added a comment.

I generalized the implementation as suggested and added a ready to use list 
version of the field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106999

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


[Lldb-commits] [lldb] 2e9853e - [DWARF5] Only fallback to manual index if no entry was found

2021-07-29 Thread Jan Kratochvil via lldb-commits

Author: Kim-Anh Tran
Date: 2021-07-29T16:16:42+02:00
New Revision: 2e9853e0e9ff263a948ebef70221fd0cec9c4830

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

LOG: [DWARF5] Only fallback to manual index if no entry was found

If we succeed at gathering global variables for a compile
unit, there is no need to fallback to generating a manual index.

Reviewed By: jankratochvil

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index cb3e662a6cdfe..5dfa5a176d384 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -125,6 +125,7 @@ void DebugNamesDWARFIndex::GetGlobalVariables(
 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 @@ void DebugNamesDWARFIndex::GetGlobalVariables(
 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 @@ void DebugNamesDWARFIndex::GetGlobalVariables(
   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


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

2021-07-29 Thread Jan Kratochvil 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 rG2e9853e0e9ff: [DWARF5] Only fallback to manual index if no 
entry was found (authored by kimanh, committed by jankratochvil).

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


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

2021-07-29 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

You can also consider coding `lldb-add-index` for better performance as that is 
too much for my available time.

In D106355#2909462 , @kimanh wrote:

> Otherwise, could you help me to land this change (since I do not have 
> committer rights)?

I guess you could already ask for a commit access, you have multiple patches 
checked in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106355

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


[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor created this revision.
teemperor added a reviewer: LLDB.
teemperor added a project: LLDB.
Herald added a subscriber: JDevlieghere.
teemperor requested review of this revision.

LLDB has a quite extensive logging framework that is mostly used to help with 
bug reporting where users
are usually requested to attach the generated logs after reproducing a bug 
locally.

Right now all logging is done in plain text where usually one line in the text 
file equals one log message (but
that is more of a guidelines, there are multiline log messages too).

While the plain text logs are easy to read without any tools, they are often 
also really limiting the usefulness
of LLDB's logging framework:

- The plain text logs become hard to read when they are filled out with too 
much information. Because of that

we often only request users to enable a small subset of the logging 
functionality in LLDB. When later on the
bug is outside one of the initially activated logging channels, we have to go 
back to the user and ask them to
recreate the logs with more channels. We also essentially never tell users to 
enable other useful parts of the
logging system (such as the automatic stacktraces) for that reason.

- We can't easily write tools that make viewing logs easier. Writing a parser 
that extracts the different parts of a

log message from the plain text format is rather difficult.

This patch changes the output format of the log files to JSON. Every log 
message is now a serialized JSON object
(that is now also guaranteed to occupy exactly one line) that is just appended 
to the log file. For convenience I
also added a trailing comma behind every JSON object so that a log file can 
always be turned into a single JSON
object again that by just surrounding it with `[]` and removing the last comma 
(for strict parsers).


https://reviews.llvm.org/D107079

Files:
  lldb/include/lldb/Utility/Log.h
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/source/Utility/Log.cpp
  lldb/test/API/api/log/TestAPILog.py
  
lldb/test/API/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
  lldb/unittests/Utility/LogTest.cpp

Index: lldb/unittests/Utility/LogTest.cpp
===
--- lldb/unittests/Utility/LogTest.cpp
+++ lldb/unittests/Utility/LogTest.cpp
@@ -11,6 +11,7 @@
 
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Threading.h"
 #include 
@@ -82,6 +83,16 @@
   llvm::StringRef takeOutput();
   llvm::StringRef logAndTakeOutput(llvm::StringRef Message);
 
+  llvm::json::Object logAndTakeLogObject(llvm::StringRef Message) {
+llvm::StringRef Out = logAndTakeOutput(Message).trim();
+EXPECT_TRUE(Out.consume_back(","));
+llvm::Expected parsed = llvm::json::parse(Out);
+EXPECT_TRUE(static_cast(parsed));
+llvm::json::Object *parsed_obj = parsed->getAsObject();
+EXPECT_NE(parsed_obj, nullptr);
+return *parsed_obj;
+  }
+
 public:
   void SetUp() override;
 };
@@ -211,37 +222,49 @@
 
 TEST_F(LogChannelEnabledTest, log_options) {
   std::string Err;
-  EXPECT_EQ("Hello World\n", logAndTakeOutput("Hello World"));
+  EXPECT_EQ("{\"message\":\"Hello World\"},\n",
+logAndTakeOutput("Hello World"));
   EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_THREADSAFE, "chan", {},
 Err));
-  EXPECT_EQ("Hello World\n", logAndTakeOutput("Hello World"));
+  EXPECT_EQ("{\"message\":\"Hello World\"},\n",
+logAndTakeOutput("Hello World"));
 
   {
 EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_PREPEND_SEQUENCE,
   "chan", {}, Err));
-llvm::StringRef Msg = logAndTakeOutput("Hello World");
-int seq_no;
-EXPECT_EQ(1, sscanf(Msg.str().c_str(), "%d Hello World", &seq_no));
+llvm::json::Object parsed_obj = logAndTakeLogObject("Hello World");
+// Check that any sequence-id was added.
+EXPECT_TRUE(parsed_obj.getInteger("sequence-id"));
   }
 
   {
 EXPECT_TRUE(EnableChannel(getStream(), LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION,
   "chan", {}, Err));
-llvm::StringRef Msg = logAndTakeOutput("Hello World");
-char File[12];
-char Function[17];
-  
-sscanf(Msg.str().c_str(), "%[^:]:%s Hello World", File, Function);
-EXPECT_STRCASEEQ("LogTest.cpp", File);
-EXPECT_STREQ("logAndTakeOutput", Function);
+llvm::json::Object parsed_obj = logAndTakeLogObject("Hello World");
+// Check that function/file parameters were added.
+llvm::Optional function = parsed_obj.getString("function");
+llvm::Optional file = parsed_obj.getString("file");
+EXPECT_TRUE(function.hasValue());
+EXPECT_TRUE(file.hasValue());
+
+EXPECT_EQ(*function, "logAndTakeOutput");
+EXPECT_EQ(*file, "LogTest.cpp");
   }
 
-  EXPECT_TRUE

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

2021-07-29 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil added a comment.

It broke some Linux buildbots and it also broke testsuite on my Fedora machine:
`functionalities/gdb_remote_client/TestTargetXMLArch.py`

- lldb-x86_64-fedora: https://lab.llvm.org/staging/#/builders/16/builds/8988
- lldb-aarch64-ubuntu: https://lab.llvm.org/buildbot/#/builders/96/builds/10245
- lldb-arm-ubuntu: https://lab.llvm.org/buildbot/#/builders/17/builds/9542

Some buildbots do not use `-DLLVM_ENABLE_ASSERTIONS=ON` which may be the reason 
why they PASS.


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] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

I hacked together a quick example log viewer in HTML here: 
https://teemperor.de/pub/logviewer.html (I already loaded in a log from my 
machine, so you can just hit "parse log"). Note that you can search the log 
messages and you can click each one of them to reveal their associated stack 
trace. You can also export the log in a different format (Excel, CSV, etc.).

Some general notes:

- Why JSON? In LLVM we have pretty much just JSON and YAML as formats available 
(well, and the bitcode format I guess). YAML is a much more complicated format 
and the main idea here is to let people write tools, so JSON seemed like an 
obvious pick.

- Will there be an upstream log viewer? I don't know, but we could easily put a 
simple one on the website that just formats a log as a simple HTML table. That 
is easily done in vanilla JavaScript. But that should be a follow up patch.

- Do we want to keep the old plain text format too? To be honest, I don't 
really see much

- What about logs that are printed to stdin and we want to parse? I think you 
can easily make a tool that filters out the non-JSON output. Logs are 
guaranteed to have every object on its own line, so you can just go 
line-by-line and filter them based on that.

Some planned follow ups:

- Enable more logging meta-information by default. Things such as pid/tid seems 
like obvious cheap choices.

- For people that really like the old format, it should be a simple patch to 
add a converter that just takes a JSON file and outputs the old plaintext.

- I want to remove the ability to turn off thread-safe logging. It seems like 
it could potentially crash LLDB but it would certainly break the JSON parser. 
It's usually (and luckily) enabled by default anyway.

- Some more meta information would be actually helpful sometimes and there 
isn't any reason to not add them. E.g., classifying messages as 
error/warning/info.


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Actually forgot this one: Having the ability to log the current channel in each 
message is also planned. This originally came up when I kept telling people to 
do 'log enable lldb all' and people were tired of filtering through the very 
verbose logs (and Jim suggested that something more structured than plaintext 
would be nice to have). With the added channels in each message we could just 
do the filtering on our side and tell people to just enable every channel in 
every category.


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

> With the added channels in each message we could just do the filtering on our 
> side and tell people to just enable every channel in every category.

+1 to this, was the first thing I tried to do on the viewer you linked.

> Will there be an upstream log viewer? I don't know, but we could easily put a 
> simple one on the website that just formats a log as a simple HTML table. 
> That is easily done in vanilla JavaScript. But that should be a follow up 
> patch.

As in on llvm.org? That would be neat.

Python does great with JSON so I'm sure it would be easy to hack on more 
specific scripts later too.

Will you be adding documentation about what the format is? (said web page would 
be a good place for it actually)

I know it won't be that complicated but it's always nice to see the total set 
of fields you might expect.




Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:115
+if not log_contents.endswith("]"):
+  log_contents = log_contents + "]"
+

What is the situation where you *would* have [, ] already? Will this be parsing 
raw text as well as output that is already JSON.


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:1579
+# Remove the suffix that isn't part of the JSON payload
+reply = reply[:-3]
+dylib_info = json.loads(reply)

Could you show what the suffix is here?
```
# Remove the suffix ("ABC") that isn't part of the JSON payload
```



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

https://reviews.llvm.org/D107079

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


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

2021-07-29 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 362794.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

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\): 0x1 \(mismatch\)$"])
 
 # You can write multiple tags, range calculated for you
 self.expect("memory tag write mte_buf 10 11 12")
 self.expect("memory tag read mte_buf mte_buf+48",
 patterns=["Logical tag: 0x9\n"
   "Allocation tags:\n"
-  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0xa\n"
-  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0xb\n"
-  "\[0x[0-9A-Fa-f]+20, 0x[0-9A-Fa-f]+30\): 0xc$"])
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0xa \(mismatch\)\n"
+  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0xb \(mismatch\)\n"
+  "\[0x[0-9A-Fa-f]+20, 0x[0-9A-Fa-f]+30\): 0xc \(mismatch\)$"])
 
 # You may write up to the end of a tagged region
 # (mte_buf_2's intial tags will all be 0)
@@ -179,7 +187,7 @@
 self.e

[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

This looks like a great effort to improve logging and debugging! Have you 
considered also adding logging levels (info, debug, warning, error ...) ? I 
think the `CommandReturnObject` and `Status` classes already makes the 
distinction between warnings and errors messages. We could enrich logging 
further more by having logging levels to filter / color-code the messages.




Comment at: lldb/include/lldb/Utility/Log.h:188-189
+  ///
+  /// Meta-information includes things such as the source file/function that
+  /// is trying to write to the log, the current pid/tid and the backtrace.
+  ///

+1 to this! We should standardise the log message format to always have the 
source info, as a follow-up (May be clang-tidy check ?).


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

Quick side-note: `Status` only has an "Error String". I guess this is for 
historical reason but I think we could change it (on another patch) to convey 
more information inside the debugger.


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

I'm supportive of this effort, but if we're removing the plaintext format, I 
think it's worth sending this out as an RFC.

In D107079#2913760 , @teemperor wrote:

> Some general notes:
>
> - Why JSON? In LLVM we have pretty much just JSON and YAML as formats 
> available (well, and the bitcode format I guess). YAML is a much more 
> complicated format and the main idea here is to let people write tools, so 
> JSON seemed like an obvious pick.

The YAML I/O library is also relatively slow, which would hurt the overhead of 
enabling logging (even more).

> - Will there be an upstream log viewer? I don't know, but we could easily put 
> a simple one on the website that just formats a log as a simple HTML table. 
> That is easily done in vanilla JavaScript. But that should be a follow up 
> patch.

Personally I'd also want a script to view the logs in the terminal. If we write 
it in Python we can ship it with lldb.

> - Do we want to keep the old plain text format too? To be honest, I don't 
> really see much

Me neither on the condition that users can convert the logs to some kind of 
plain text format (part of the motivation for my previous comment.)

> - What about logs that are printed to stdin and we want to parse? I think you 
> can easily make a tool that filters out the non-JSON output. Logs are 
> guaranteed to have every object on its own line, so you can just go 
> line-by-line and filter them based on that.
>
> Some planned follow ups:
>
> - Enable more logging meta-information by default. Things such as pid/tid 
> seems like obvious cheap choices.

+1, if the data is structured we should always emit it (unless there's some 
large associated cost)

> - For people that really like the old format, it should be a simple patch to 
> add a converter that just takes a JSON file and outputs the old plaintext.
>
> - I want to remove the ability to turn off thread-safe logging. It seems like 
> it could potentially crash LLDB but it would certainly break the JSON parser. 
> It's usually (and luckily) enabled by default anyway.
>
> - Some more meta information would be actually helpful sometimes and there 
> isn't any reason to not add them. E.g., classifying messages as 
> error/warning/info.

+1 on everything here.


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [lldb] 172a55e - [lldb] Fix FunctionDecl::Create after D102343

2021-07-29 Thread Fangrui Song via lldb-commits

Author: Fangrui Song
Date: 2021-07-29T09:57:10-07:00
New Revision: 172a55e7a40d27c7882be2e86d515696d8e12427

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

LOG: [lldb] Fix FunctionDecl::Create after D102343

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
index 829afa5ffcecc..d95b79a9b1f82 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
@@ -77,7 +77,7 @@ clang::NamedDecl *NameSearchContext::AddFunDecl(const 
CompilerType &type,
 
   clang::FunctionDecl *func_decl = FunctionDecl::Create(
   ast, context, SourceLocation(), SourceLocation(), decl_name, qual_type,
-  nullptr, SC_Extern, isInlineSpecified, hasWrittenPrototype,
+  nullptr, SC_Extern, /*UsesFPIntrin=*/false, isInlineSpecified, 
hasWrittenPrototype,
   isConstexprSpecified ? ConstexprSpecKind::Constexpr
: ConstexprSpecKind::Unspecified);
 



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


[Lldb-commits] [lldb] 72a8367 - Replace LLVM_ATTRIBUTE_NORETURN with C++11 [[noreturn]]. NFC

2021-07-29 Thread Fangrui Song via lldb-commits

Author: Fangrui Song
Date: 2021-07-29T09:59:45-07:00
New Revision: 72a83674dd3a13b59442cd7cb07b53902f7d6a33

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

LOG: Replace LLVM_ATTRIBUTE_NORETURN with C++11 [[noreturn]]. NFC

[[noreturn]] can be used since Oct 2016 when the minimum compiler requirement 
was bumped to GCC 4.8/MSVC 2015.

Added: 


Modified: 
clang-tools-extra/pp-trace/PPTrace.cpp
clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
flang/include/flang/Optimizer/Support/FatalError.h
lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
lldb/source/Plugins/Process/Linux/SingleStepCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/pp-trace/PPTrace.cpp 
b/clang-tools-extra/pp-trace/PPTrace.cpp
index 3fa16498fbefc..0b078c49a55b7 100644
--- a/clang-tools-extra/pp-trace/PPTrace.cpp
+++ b/clang-tools-extra/pp-trace/PPTrace.cpp
@@ -69,7 +69,7 @@ static cl::opt OutputFileName(
 cl::desc("Output trace to the given file name or '-' for stdout."),
 cl::cat(Cat));
 
-LLVM_ATTRIBUTE_NORETURN static void error(Twine Message) {
+[[noreturn]] static void error(Twine Message) {
   WithColor::error() << Message << '\n';
   exit(1);
 }

diff  --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp 
b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
index 014c1adcd8092..ad4bb8d78ced3 100644
--- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -614,7 +614,7 @@ struct DiagnosticTextBuilder {
 return It->second.Root;
   }
 
-  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const &Msg) const {
+  [[noreturn]] void PrintFatalError(llvm::Twine const &Msg) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }

diff  --git a/flang/include/flang/Optimizer/Support/FatalError.h 
b/flang/include/flang/Optimizer/Support/FatalError.h
index 602045346587c..8450b16a5baf4 100644
--- a/flang/include/flang/Optimizer/Support/FatalError.h
+++ b/flang/include/flang/Optimizer/Support/FatalError.h
@@ -20,8 +20,8 @@ namespace fir {
 
 /// Fatal error reporting helper. Report a fatal error with a source location
 /// and immediately abort flang.
-LLVM_ATTRIBUTE_NORETURN inline void emitFatalError(mlir::Location loc,
-   const llvm::Twine &message) 
{
+[[noreturn]] inline void emitFatalError(mlir::Location loc,
+const llvm::Twine &message) {
   mlir::emitError(loc, message);
   llvm::report_fatal_error("aborting");
 }

diff  --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp 
b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
index 25dcf1e592c57..b49b541927afe 100644
--- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -46,8 +46,8 @@ static void FixupEnvironment(Environment &env) {
 #endif
 }
 
-static void LLVM_ATTRIBUTE_NORETURN ExitWithError(int error_fd,
-  const char *operation) {
+[[noreturn]] static void ExitWithError(int error_fd,
+   const char *operation) {
   int err = errno;
   llvm::raw_fd_ostream os(error_fd, true);
   os << operation << " failed: " << llvm::sys::StrError(err);
@@ -88,8 +88,8 @@ static void DupDescriptor(int error_fd, const FileSpec 
&file_spec, int fd,
   return;
 }
 
-static void LLVM_ATTRIBUTE_NORETURN ChildFunc(int error_fd,
-  const ProcessLaunchInfo &info) {
+[[noreturn]] static void ChildFunc(int error_fd,
+   const ProcessLaunchInfo &info) {
   if (info.GetFlags().Test(eLaunchFlagLaunchInSeparateProcessGroup)) {
 if (setpgid(0, 0) != 0)
   ExitWithError(error_fd, "setpgid");

diff  --git a/lldb/source/Plugins/Process/Linux/SingleStepCheck.cpp 
b/lldb/source/Plugins/Process/Linux/SingleStepCheck.cpp
index 5b337f1974ea5..5f8057d2fef63 100644
--- a/lldb/source/Plugins/Process/Linux/SingleStepCheck.cpp
+++ b/lldb/source/Plugins/Process/Linux/SingleStepCheck.cpp
@@ -29,7 +29,7 @@ using namespace lldb_private::process_linux;
 #if defined(__arm64__) || defined(__aarch64__)
 namespace {
 
-void LLVM_ATTRIBUTE_NORETURN Child() {
+[[noreturn]] void Child() {
   if (ptrace(PTRACE_TRACEME, 0, nullptr, nullptr) == -1)
 _exit(1);
 



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


Re: [Lldb-commits] [lldb] 172a55e - [lldb] Fix FunctionDecl::Create after D102343

2021-07-29 Thread Shafik Yaghmour via lldb-commits
Thanks for catching and fixing this!

> On Jul 29, 2021, at 9:57 AM, Fangrui Song via lldb-commits 
>  wrote:
> 
> 
> Author: Fangrui Song
> Date: 2021-07-29T09:57:10-07:00
> New Revision: 172a55e7a40d27c7882be2e86d515696d8e12427
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/172a55e7a40d27c7882be2e86d515696d8e12427
> DIFF: 
> https://github.com/llvm/llvm-project/commit/172a55e7a40d27c7882be2e86d515696d8e12427.diff
> 
> LOG: [lldb] Fix FunctionDecl::Create after D102343
> 
> Added: 
> 
> 
> Modified: 
>lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp 
> b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
> index 829afa5ffcecc..d95b79a9b1f82 100644
> --- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
> +++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp
> @@ -77,7 +77,7 @@ clang::NamedDecl *NameSearchContext::AddFunDecl(const 
> CompilerType &type,
> 
>   clang::FunctionDecl *func_decl = FunctionDecl::Create(
>   ast, context, SourceLocation(), SourceLocation(), decl_name, qual_type,
> -  nullptr, SC_Extern, isInlineSpecified, hasWrittenPrototype,
> +  nullptr, SC_Extern, /*UsesFPIntrin=*/false, isInlineSpecified, 
> hasWrittenPrototype,
>   isConstexprSpecified ? ConstexprSpecKind::Constexpr
>: ConstexprSpecKind::Unspecified);
> 
> 
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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


[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added a comment.

More a comment than anything else: One thing I always wanted to explore was to 
implement LLDB's logging on Darwin on top of os_log 
(https://developer.apple.com/documentation/os/logging) which is also structured 
and also faster since it moves the the formatting out of process. This patch is 
great because it also works for non-Darwin platforms, and I guess there isn't 
anything preventing us from moving to os_log even if we take patch.


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [lldb] 66ba4e3 - Revert "[lldb] Assert filecache and live memory match on debug under a setting"

2021-07-29 Thread Stella Stamenova via lldb-commits

Author: Stella Stamenova
Date: 2021-07-29T10:48:57-07:00
New Revision: 66ba4e3dc608156797df8f863d61fa106608e45c

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

LOG: Revert "[lldb] Assert filecache and live memory match on debug under a 
setting"

This reverts commit 77e9d10f0fbfe04a14e6ce61753376dd78e0c2f0.

This change broke the Windows LLDB bot:
https://lab.llvm.org/buildbot/#/builders/83/builds/8784/steps/7/logs/stdio

Added: 


Modified: 
lldb/include/lldb/Core/Section.h
lldb/include/lldb/Target/Target.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Core/Section.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/Shell/lit-lldb-init.in

Removed: 




diff  --git a/lldb/include/lldb/Core/Section.h 
b/lldb/include/lldb/Core/Section.h
index 3914469aafc1a..3d4ab154e743f 100644
--- a/lldb/include/lldb/Core/Section.h
+++ b/lldb/include/lldb/Core/Section.h
@@ -236,8 +236,6 @@ class Section : public 
std::enable_shared_from_this,
 
   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

diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index d15370259f467..ac8d002b09a12 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -229,10 +229,6 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
-  void SetVerifyFileCacheMemoryReads(bool debug);
-
-  bool GetVerifyFileCacheMemoryReads() const;
-
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 366c40db81507..7e1fdce36ede6 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -798,9 +798,6 @@ def setUpCommands(cls):
 'settings set symbols.clang-modules-cache-path "{}"'.format(
 configuration.lldb_module_cache_dir),
 "settings set use-color false",
-
-# Verify that file cache and live memory always match.
-"settings set target.verify-file-cache-memory-reads true",
 ]
 
 # Set any user-overridden settings.

diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index af5cee2f9b9b9..a5a10141aa649 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -599,9 +599,3 @@ size_t SectionList::Slide(addr_t slide_amount, bool 
slide_children) {
   }
   return count;
 }
-
-bool Section::IsReadOnly() {
-  auto permissions = Flags(GetPermissions());
-  return !permissions.Test(ePermissionsWritable) &&
- permissions.Test(ePermissionsReadable);
-}

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 33691e8e97856..1f8e8c54fa9e1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1763,34 +1763,21 @@ size_t Target::ReadMemory(const Address &addr, void 
*dst, size_t dst_len,
   // Read from file cache if read-only section.
   if (!force_live_memory && resolved_addr.IsSectionOffset()) {
 SectionSP section_sp(resolved_addr.GetSection());
-if (section_sp && section_sp->IsReadOnly()) {
-  file_cache_bytes_read =
-  ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-
-  if (GetVerifyFileCacheMemoryReads()) {
-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) {
-std::unique_ptr live_buf =
-std::make_unique(dst_len);
-bytes_read = m_process_sp->ReadMemory(load_addr, live_buf.get(),
-  dst_len, error);
-if (bytes_read == dst_len) {
-  lldbassert(memcmp(live_buf.get(), dst, dst_len) == 0 &&
- "File cache and live memory diverge!");
-}
-  }
+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_buff

[Lldb-commits] [PATCH] D107079: [lldb] Change the log format to JSON instead of plain text

2021-07-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

When you are using the logging for a reliably reproducible problem (some 
parsing issue, for instance) then there's no harm to the general "turn on all 
logging and we'll sort it out on our end".  But for any issues that are timing 
sensitive the problem is that too much logging changes the timing and often 
causes bugs to disappear.  This patch will add volume to log emission and slow 
its emission down, so it goes a bit in the wrong direction.  I don't think 
that's a reason NOT to do this, hopefully the overhead is small.  But it means 
we can't assume "turning on all logs" is a reasonable default behavior for 
logging.  If anything, we need to go in the other direction and add a more 
fine-grained verbosity so we can ask for "a bit of log channel A and a bit of B 
and more of C" or whatever.

I'm not entirely sure what you intend when you say:

> Every log message is now a serialized JSON object (that is now also 
> guaranteed to occupy exactly one line) that is just appended to the log file.

Log messages are going to have new-lines in them because it's quite convenient 
to use the various object Description or Dump methods to log objects and those 
are generally laid out to be human readable (and are used for that purpose in 
other contexts.)  By "single line" it seems you mean "all log messages with new 
lines will have the newline converted to "\n" as a character string before 
being inserted into the JSON.  This seems more an implementation detail, I'm 
not sure why you call it out.  In any case, preserving the multi-line structure 
of log content is important for eventually being able to read them, so as long 
as the encoders & printers do the right thing, it doesn't really matter how 
they are encoded.

Conversely, however, it is NOT the case that each LLDB_LOGF call is a single 
Log message.  For instance, at the beginning and end of each round of 
"ShouldStop" negotiation, I run through the ThreadPlanStack of each thread that 
had a non-trivial stop reason and print each thread plan's description in 
order.  I LOG a header for the output, then (fortunately) I have a routine that 
dumps all the info to a stream, so I can log that part in one go.  Then I print 
an end message with another LLDB_LOGF.  Putting these into separate JSON 
entries makes keeping them together harder - thus making automated tools harder 
to write - harder to scan with anything but a plain text dumper.

We could require that all unitary log messages build up the whole message 
off-line into a stream, then log in one go.  But it might be more convenient to 
have a "start message/end message" pair, so you can call LLDB_LOGF in a loop 
printing individual objects, then close off the message.  Some kind of RAII 
deal would be convenient for this.

It would be even more convenient if this allows nesting.  Then you can do 
"logging the shared library state change - start the list of dylibs - start the 
log of dylib A, end dylib A, etc..."  That way the individual entity dumps 
would be structured in the log message that dumped them, and you could 
progressively reveal them in a neat way.

We definitely need a plain text dumper that we provide.  A very common task in 
looking through logs is to find an address of some entity in one log message, 
and then search through the other logs looking at each one to see if it is 
interesting.  And very often to see if it is embedded in another interesting 
log transaction, so the context is important.  That's best done with a plain 
text dump of the log.




Comment at: lldb/source/Utility/Log.cpp:362
+
+  llvm::json::OStream J(message);
+  J.object([this, &J, file, function, &payload] {

Don't use single capitol letter variable names.  They are sort of okay in tiny 
functions, but tiny functions tend not to stay tiny and then you're trying to 
find a single letter amongst all the other letters when searching for 
definitions and uses.


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

https://reviews.llvm.org/D107079

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


[Lldb-commits] [PATCH] D106999: [LLDB][GUI] Add Environment Variable Field

2021-07-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Very nice! Looks great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106999

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


[Lldb-commits] [lldb] 18c25cd - [LLDB][GUI] Add Create Target form

2021-07-29 Thread Greg Clayton via lldb-commits

Author: Omar Emara
Date: 2021-07-29T13:27:53-07:00
New Revision: 18c25cd376b6ea38971abb407751cdf08e1e5622

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

LOG: [LLDB][GUI] Add Create Target form

This patch adds a Create Target form for the LLDB GUI. Additionally, an
Arch Field was introduced to input an arch and the file and directory
fields now have a required property.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 4bed788d48630..aab5dc9cd8017 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/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,185 @@ class ProcessAttachFormDelegate : public FormDelegate {
   ProcessPluginFieldDelegate *m_plugin_field;
 };
 
+class TargetCreateFormDelegate : public FormDelegate {
+public:
+  TargetCreateFormDelegate(Debugger &debugger) : m_debugger(debugger) {
+m_executable_field = AddFileField("Executable", "", /*need_to_exist=*/true,
+  /*required=*/true);
+m_core_file_field = AddFileField("Core File", "", /*need_to_exist=*/true,
+ /*required=*/false);
+m_symbol_file_field = AddFileField(
+"Symbol File", "", /*need_to_exist=*/true, /*required=*/false);
+m_show_advanced_field = AddBooleanField("Show advanced settings.", false);
+m_remote_file_field = AddFileField(
+"Remote File", "", /*need_to_exist=*/false, /*required=*/false);
+m_arch_field = AddArchField("Architecture", "", /*required=*/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();
+}
+  }
+
+  static constexpr const char *kLoadDependentFilesNo = "No";
+  static constexpr const char *kLoadDependentFilesYes = "Yes";
+  static constexpr const char *kLoadDependentFilesExecOnly = "Executable only";
+
+  std::vector GetLoadDependentFilesChoices() {
+std::vector load_depentents_options;
+load_depentents_options.push_back(kLoadDependentFilesExecOnly);
+load_depentents_options.push_back(kLoadDependentFilesYes);
+load_depentents_options.push_back(kLoadDependentFilesNo);
+return load_depentents_options;
+  }
+
+  LoadDependentFiles GetLoadDependentFiles() {
+std::string choice = m_load_dependent_files_field->GetChoiceContent();
+if (choice == kLoadDependentFilesNo)
+  return eLoadDependentsNo;
+if (choice == kLoadDependentFilesYes)
+  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())

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

2021-07-29 Thread 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 rG18c25cd376b6: [LLDB][GUI] Add Create Target form (authored 
by OmarEmaraDev, committed by clayborg).

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,185 @@
   ProcessPluginFieldDelegate *m_plugin_field;
 };
 
+class TargetCreateFormDelegate : public FormDelegate {
+public:
+  TargetCreateFormDelegate(Debugger &debugger) : m_debugger(debugger) {
+m_executable_field = AddFileField("Executable", "", /*need_to_exist=*/true,
+  /*required=*/true);
+m_core_file_field = AddFileField("Core File", "", /*need_to_exist=*/true,
+ /*required=*/false);
+m_symbol_file_field = AddFileField(
+"Symbol File", "", /*need_to_exist=*/true, /*required=*/false);
+m_show_advanced_field = AddBooleanField("Show advanced settings.", false);
+m_remote_file_field = AddFileField(
+"Remote File", "", /*need_to_exist=*/false, /*required=*/false);
+m_arch_field = AddArchField("Architecture", "", /*required=*/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();
+}
+  }
+
+  static constexpr const char *kLoadDependentFilesNo = "No";
+  static constexpr const char *kLoadDependentFilesYes = "Yes";
+  static constexpr const char *kLoadDependentFilesExecOnly = "Executable only";
+
+  std::vector GetLoadDependentFilesChoices() {
+std::vector load_depentents_options;
+load_depentents_options.push_back(kLoadDependentFilesExecOnly);
+load_depentents_options.push_back(kLoadDependentFilesYes);
+load_depentents_options.push_back(kLoadDependentFilesNo);
+return load_depentents_options;
+  }
+
+  LoadDependentFiles GetLoadDependentFiles() {
+std::string choice = m_load_dependent_files_field->GetChoiceContent();
+if (choice == kLoadDependentFilesNo)
+  return eLoadDependentsNo;
+if (choice == kLoadDependentFilesYes)
+  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, fals

[Lldb-commits] [lldb] 62bd331 - [LLDB][GUI] Add Environment Variable Field

2021-07-29 Thread Greg Clayton via lldb-commits

Author: Omar Emara
Date: 2021-07-29T13:28:49-07:00
New Revision: 62bd33158d924f907dfe2ccd250f20ddb8ed09a9

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

LOG: [LLDB][GUI] Add Environment Variable Field

This patch adds an environment variable field. This is usually used as
the basic type of a List field. This is needed to create the process
launch form.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index aab5dc9cd8017..010f9300aa2e5 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1909,6 +1909,176 @@ template  class ListFieldDelegate : public 
FieldDelegate {
   SelectionType m_selection_type;
 };
 
+template 
+class MappingFieldDelegate : public FieldDelegate {
+public:
+  MappingFieldDelegate(KeyFieldDelegateType key_field,
+   ValueFieldDelegateType value_field)
+  : m_key_field(key_field), m_value_field(value_field),
+m_selection_type(SelectionType::Key) {}
+
+  // Signify which element is selected. The key field or its value field.
+  enum class SelectionType { Key, Value };
+
+  // A mapping field is drawn as two text fields with a right arrow in between.
+  // The first field stores the key of the mapping and the second stores the
+  // value if the mapping.
+  //
+  // __[Key]_   __[Value]___
+  // |  | > |  |
+  // |__|   |__|
+  // - Error message if it exists.
+
+  // The mapping field has a height that is equal to the maximum height between
+  // the key and value fields.
+  int FieldDelegateGetHeight() override {
+return std::max(m_key_field.FieldDelegateGetHeight(),
+m_value_field.FieldDelegateGetHeight());
+  }
+
+  void DrawArrow(SubPad &surface) {
+surface.MoveCursor(0, 1);
+surface.PutChar(ACS_RARROW);
+  }
+
+  void FieldDelegateDraw(SubPad &surface, bool is_selected) override {
+Rect bounds = surface.GetFrame();
+Rect key_field_bounds, arrow_and_value_field_bounds;
+bounds.VerticalSplit(bounds.size.width / 2, key_field_bounds,
+ arrow_and_value_field_bounds);
+Rect arrow_bounds, value_field_bounds;
+arrow_and_value_field_bounds.VerticalSplit(1, arrow_bounds,
+   value_field_bounds);
+
+SubPad key_field_surface = SubPad(surface, key_field_bounds);
+SubPad arrow_surface = SubPad(surface, arrow_bounds);
+SubPad value_field_surface = SubPad(surface, value_field_bounds);
+
+bool key_is_selected =
+m_selection_type == SelectionType::Key && is_selected;
+m_key_field.FieldDelegateDraw(key_field_surface, key_is_selected);
+DrawArrow(arrow_surface);
+bool value_is_selected =
+m_selection_type == SelectionType::Value && is_selected;
+m_value_field.FieldDelegateDraw(value_field_surface, value_is_selected);
+  }
+
+  HandleCharResult SelectNext(int key) {
+if (FieldDelegateOnLastOrOnlyElement())
+  return eKeyNotHandled;
+
+if (!m_key_field.FieldDelegateOnLastOrOnlyElement()) {
+  return m_key_field.FieldDelegateHandleChar(key);
+}
+
+m_key_field.FieldDelegateExitCallback();
+m_selection_type = SelectionType::Value;
+m_value_field.FieldDelegateSelectFirstElement();
+return eKeyHandled;
+  }
+
+  HandleCharResult SelectPrevious(int key) {
+if (FieldDelegateOnFirstOrOnlyElement())
+  return eKeyNotHandled;
+
+if (!m_value_field.FieldDelegateOnFirstOrOnlyElement()) {
+  return m_value_field.FieldDelegateHandleChar(key);
+}
+
+m_value_field.FieldDelegateExitCallback();
+m_selection_type = SelectionType::Key;
+m_key_field.FieldDelegateSelectLastElement();
+return eKeyHandled;
+  }
+
+  HandleCharResult FieldDelegateHandleChar(int key) override {
+switch (key) {
+case '\t':
+  SelectNext(key);
+  return eKeyHandled;
+case KEY_SHIFT_TAB:
+  SelectPrevious(key);
+  return eKeyHandled;
+default:
+  break;
+}
+
+// If the key wasn't handled, pass the key to the selected field.
+if (m_selection_type == SelectionType::Key)
+  return m_key_field.FieldDelegateHandleChar(key);
+else
+  return m_value_field.FieldDelegateHandleChar(key);
+
+return eKeyNotHandled;
+  }
+
+  bool FieldDelegateOnFirstOrOnlyElement() override {
+return m_selection_type == SelectionType::Key;
+  }
+
+  bool FieldDelegateOnLastOrOnlyElement() override {
+return m_selection_type == SelectionType::Value;
+  }
+
+  void FieldDelegateS

[Lldb-commits] [PATCH] D106999: [LLDB][GUI] Add Environment Variable Field

2021-07-29 Thread 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 rG62bd33158d92: [LLDB][GUI] Add Environment Variable Field 
(authored by OmarEmaraDev, committed by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106999

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1909,6 +1909,176 @@
   SelectionType m_selection_type;
 };
 
+template 
+class MappingFieldDelegate : public FieldDelegate {
+public:
+  MappingFieldDelegate(KeyFieldDelegateType key_field,
+   ValueFieldDelegateType value_field)
+  : m_key_field(key_field), m_value_field(value_field),
+m_selection_type(SelectionType::Key) {}
+
+  // Signify which element is selected. The key field or its value field.
+  enum class SelectionType { Key, Value };
+
+  // A mapping field is drawn as two text fields with a right arrow in between.
+  // The first field stores the key of the mapping and the second stores the
+  // value if the mapping.
+  //
+  // __[Key]_   __[Value]___
+  // |  | > |  |
+  // |__|   |__|
+  // - Error message if it exists.
+
+  // The mapping field has a height that is equal to the maximum height between
+  // the key and value fields.
+  int FieldDelegateGetHeight() override {
+return std::max(m_key_field.FieldDelegateGetHeight(),
+m_value_field.FieldDelegateGetHeight());
+  }
+
+  void DrawArrow(SubPad &surface) {
+surface.MoveCursor(0, 1);
+surface.PutChar(ACS_RARROW);
+  }
+
+  void FieldDelegateDraw(SubPad &surface, bool is_selected) override {
+Rect bounds = surface.GetFrame();
+Rect key_field_bounds, arrow_and_value_field_bounds;
+bounds.VerticalSplit(bounds.size.width / 2, key_field_bounds,
+ arrow_and_value_field_bounds);
+Rect arrow_bounds, value_field_bounds;
+arrow_and_value_field_bounds.VerticalSplit(1, arrow_bounds,
+   value_field_bounds);
+
+SubPad key_field_surface = SubPad(surface, key_field_bounds);
+SubPad arrow_surface = SubPad(surface, arrow_bounds);
+SubPad value_field_surface = SubPad(surface, value_field_bounds);
+
+bool key_is_selected =
+m_selection_type == SelectionType::Key && is_selected;
+m_key_field.FieldDelegateDraw(key_field_surface, key_is_selected);
+DrawArrow(arrow_surface);
+bool value_is_selected =
+m_selection_type == SelectionType::Value && is_selected;
+m_value_field.FieldDelegateDraw(value_field_surface, value_is_selected);
+  }
+
+  HandleCharResult SelectNext(int key) {
+if (FieldDelegateOnLastOrOnlyElement())
+  return eKeyNotHandled;
+
+if (!m_key_field.FieldDelegateOnLastOrOnlyElement()) {
+  return m_key_field.FieldDelegateHandleChar(key);
+}
+
+m_key_field.FieldDelegateExitCallback();
+m_selection_type = SelectionType::Value;
+m_value_field.FieldDelegateSelectFirstElement();
+return eKeyHandled;
+  }
+
+  HandleCharResult SelectPrevious(int key) {
+if (FieldDelegateOnFirstOrOnlyElement())
+  return eKeyNotHandled;
+
+if (!m_value_field.FieldDelegateOnFirstOrOnlyElement()) {
+  return m_value_field.FieldDelegateHandleChar(key);
+}
+
+m_value_field.FieldDelegateExitCallback();
+m_selection_type = SelectionType::Key;
+m_key_field.FieldDelegateSelectLastElement();
+return eKeyHandled;
+  }
+
+  HandleCharResult FieldDelegateHandleChar(int key) override {
+switch (key) {
+case '\t':
+  SelectNext(key);
+  return eKeyHandled;
+case KEY_SHIFT_TAB:
+  SelectPrevious(key);
+  return eKeyHandled;
+default:
+  break;
+}
+
+// If the key wasn't handled, pass the key to the selected field.
+if (m_selection_type == SelectionType::Key)
+  return m_key_field.FieldDelegateHandleChar(key);
+else
+  return m_value_field.FieldDelegateHandleChar(key);
+
+return eKeyNotHandled;
+  }
+
+  bool FieldDelegateOnFirstOrOnlyElement() override {
+return m_selection_type == SelectionType::Key;
+  }
+
+  bool FieldDelegateOnLastOrOnlyElement() override {
+return m_selection_type == SelectionType::Value;
+  }
+
+  void FieldDelegateSelectFirstElement() override {
+m_selection_type = SelectionType::Key;
+  }
+
+  void FieldDelegateSelectLastElement() override {
+m_selection_type = SelectionType::Value;
+  }
+
+  bool FieldDelegateHasError() override {
+return m_key_field.FieldDelegateHasError() ||
+   m_value_field.FieldDelegateHasError();
+  }
+
+  KeyFieldDelegateType &GetKeyField() { return 

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

2021-07-29 Thread Dave Lee via Phabricator via lldb-commits
kastiglione updated this revision to Diff 362880.
kastiglione added a comment.

- Reword comment using Jonas's suggestion
- Pop the last plan right away, and thus change from WillPop to DidPop


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106171

Files:
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanCallFunction.h
  lldb/include/lldb/Target/ThreadPlanCallUserExpression.h
  lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanCallUserExpression.cpp
  lldb/source/Target/ThreadPlanStack.cpp
  lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp

Index: lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
===
--- lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
+++ lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
@@ -123,9 +123,7 @@
   ReenableBreakpointSite();
 }
 
-void ThreadPlanStepOverBreakpoint::WillPop() {
-  ReenableBreakpointSite();
-}
+void ThreadPlanStepOverBreakpoint::DidPop() { ReenableBreakpointSite(); }
 
 bool ThreadPlanStepOverBreakpoint::MischiefManaged() {
   lldb::addr_t pc_addr = GetThread().GetRegisterContext()->GetPC();
Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -142,20 +142,26 @@
 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());
-  m_completed_plans.push_back(plan_sp);
-  plan_sp->WillPop();
+  // Note that moving the top element of the vector would leave it in an
+  // undefined state, and break the guarantee that the stack's thread plans are
+  // all valid.
+  lldb::ThreadPlanSP plan_sp = m_plans.back();
   m_plans.pop_back();
+  m_completed_plans.push_back(plan_sp);
+  plan_sp->DidPop();
   return plan_sp;
 }
 
 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());
-  m_discarded_plans.push_back(plan_sp);
-  plan_sp->WillPop();
+  // Note that moving the top element of the vector would leave it in an
+  // undefined state, and break the guarantee that the stack's thread plans are
+  // all valid.
+  lldb::ThreadPlanSP plan_sp = m_plans.back();
   m_plans.pop_back();
+  m_discarded_plans.push_back(plan_sp);
+  plan_sp->DidPop();
   return plan_sp;
 }
 
Index: lldb/source/Target/ThreadPlanCallUserExpression.cpp
===
--- lldb/source/Target/ThreadPlanCallUserExpression.cpp
+++ lldb/source/Target/ThreadPlanCallUserExpression.cpp
@@ -58,8 +58,8 @@
 m_user_expression_sp->WillStartExecuting();
 }
 
-void ThreadPlanCallUserExpression::WillPop() {
-  ThreadPlanCallFunction::WillPop();
+void ThreadPlanCallUserExpression::DidPop() {
+  ThreadPlanCallFunction::DidPop();
   if (m_user_expression_sp)
 m_user_expression_sp.reset();
 }
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -208,7 +208,7 @@
   }
 }
 
-void ThreadPlanCallFunction::WillPop() { DoTakedown(PlanSucceeded()); }
+void ThreadPlanCallFunction::DidPop() { DoTakedown(PlanSucceeded()); }
 
 void ThreadPlanCallFunction::GetDescription(Stream *s, DescriptionLevel level) {
   if (level == eDescriptionLevelBrief) {
Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -150,7 +150,7 @@
 
 void ThreadPlan::DidPush() {}
 
-void ThreadPlan::WillPop() {}
+void ThreadPlan::DidPop() {}
 
 bool ThreadPlan::OkayToDiscard() { return m_okay_to_discard; }
 
Index: lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
===
--- lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
+++ lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
@@ -26,7 +26,7 @@
   bool StopOthers() override;
   lldb::StateType GetPlanRunState() override;
   void WillStop() override;
-  void WillPop() override;
+  void DidPop() override;
   bool MischiefManaged() override;
   void ThreadDestroyed() override;
   void SetAutoContinue(bool do_it);
Index: lldb/include/lldb/Target/ThreadPlanCallUserExpression.h
===
--- lldb/include/lldb/Target/ThreadPlanCallUserExpression.h
+++ lldb/include/lldb/Target/ThreadPlanCallUserExpression.h
@@ -32,7 +32,7 @@
 
   void DidPush() 

[Lldb-commits] [lldb] 993220a - [lldb] Remove CPlusPlusLanguage from Mangled

2021-07-29 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2021-07-29T13:58:35-07:00
New Revision: 993220a99ccef6657d6d5d04dc51c06255f285f2

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

LOG: [lldb] Remove CPlusPlusLanguage from Mangled

The only remaining plugin dependency in Mangled is CPlusPlusLanguage which it
uses to extract information from C++ mangled names. The static function
GetDemangledNameWithoutArguments is written specifically for C++, so it
would make sense for this specific functionality to live in a
C++-related plugin. In order to keep this functionality in Mangled
without maintaining this dependency, I added
`Language::GetDemangledFunctionNameWithoutArguments`.

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

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 11b9daa389454..2ad9677ea3f8c 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -243,6 +243,14 @@ class Language : public PluginInterface {
   FunctionNameRepresentation 
representation,
   Stream &s);
 
+  virtual ConstString
+  GetDemangledFunctionNameWithoutArguments(Mangled mangled) const {
+if (ConstString demangled = mangled.GetDemangledName())
+  return demangled;
+
+return mangled.GetMangledName();
+  }
+
   virtual void GetExceptionResolverDescription(bool catch_on, bool throw_on,
Stream &s);
 

diff  --git a/lldb/source/Core/Mangled.cpp b/lldb/source/Core/Mangled.cpp
index fbaf9ff7151a3..b2ceb6312be06 100644
--- a/lldb/source/Core/Mangled.cpp
+++ b/lldb/source/Core/Mangled.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Core/Mangled.h"
 
 #include "lldb/Core/RichManglingContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
@@ -16,8 +17,6 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-enumerations.h"
 
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/Support/Compiler.h"
@@ -34,35 +33,6 @@ static inline bool cstring_is_mangled(llvm::StringRef s) {
   return Mangled::GetManglingScheme(s) != Mangled::eManglingSchemeNone;
 }
 
-static ConstString GetDemangledNameWithoutArguments(ConstString mangled,
-ConstString demangled) {
-  const char *mangled_name_cstr = mangled.GetCString();
-
-  if (demangled && mangled_name_cstr && mangled_name_cstr[0]) {
-if (mangled_name_cstr[0] == '_' && mangled_name_cstr[1] == 'Z' &&
-(mangled_name_cstr[2] != 'T' && // avoid virtual table, VTT structure,
-// typeinfo structure, and typeinfo
-// mangled_name
- mangled_name_cstr[2] != 'G' && // avoid guard variables
- mangled_name_cstr[2] != 'Z')) // named local entities (if we 
eventually
-   // handle eSymbolTypeData, we will want
-   // this back)
-{
-  CPlusPlusLanguage::MethodName cxx_method(demangled);
-  if (!cxx_method.GetBasename().empty()) {
-std::string shortname;
-if (!cxx_method.GetContext().empty())
-  shortname = cxx_method.GetContext().str() + "::";
-shortname += cxx_method.GetBasename().str();
-return ConstString(shortname);
-  }
-}
-  }
-  if (demangled)
-return demangled;
-  return mangled;
-}
-
 #pragma mark Mangled
 
 Mangled::ManglingScheme Mangled::GetManglingScheme(llvm::StringRef const name) 
{
@@ -344,14 +314,16 @@ ConstString Mangled::GetName(Mangled::NamePreference 
preference) const {
   if (preference == ePreferMangled && m_mangled)
 return m_mangled;
 
+  // Call the accessor to make sure we get a demangled name in case it hasn't
+  // been demangled yet...
   ConstString demangled = GetDemangledName();
 
   if (preference == ePreferDemangledWithoutArguments) {
-return GetDemangledNameWithoutArguments(m_mangled, demangled);
+if (Language *lang = Language::FindPlugin(GuessLanguage())) {
+  return lang->GetDemangledFunctionNameWithoutArguments(*this);
+}
   }
   if (preference == ePreferDemangled) {
-// Call the accessor to make sure we get a demangled name in case it hasn't
-// been demangled yet...
 if (demangled)
   return demangled;
 return m_mang

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

2021-07-29 Thread Alex Langford 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 rG993220a99cce: [lldb] Remove CPlusPlusLanguage from Mangled 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105215

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

Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -106,6 +106,9 @@
 
   bool SymbolNameFitsToLanguage(Mangled mangled) const override;
 
+  ConstString
+  GetDemangledFunctionNameWithoutArguments(Mangled mangled) const override;
+
   static bool IsCPPMangledName(llvm::StringRef name);
 
   // Extract C++ context and identifier from a string using heuristic matching
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -64,6 +64,35 @@
   return mangled_name && CPlusPlusLanguage::IsCPPMangledName(mangled_name);
 }
 
+ConstString CPlusPlusLanguage::GetDemangledFunctionNameWithoutArguments(
+Mangled mangled) const {
+  const char *mangled_name_cstr = mangled.GetMangledName().GetCString();
+  ConstString demangled_name = mangled.GetDemangledName();
+  if (demangled_name && mangled_name_cstr && mangled_name_cstr[0]) {
+if (mangled_name_cstr[0] == '_' && mangled_name_cstr[1] == 'Z' &&
+(mangled_name_cstr[2] != 'T' && // avoid virtual table, VTT structure,
+// typeinfo structure, and typeinfo
+// mangled_name
+ mangled_name_cstr[2] != 'G' && // avoid guard variables
+ mangled_name_cstr[2] != 'Z'))  // named local entities (if we
+// eventually handle eSymbolTypeData,
+// we will want this back)
+{
+  CPlusPlusLanguage::MethodName cxx_method(demangled_name);
+  if (!cxx_method.GetBasename().empty()) {
+std::string shortname;
+if (!cxx_method.GetContext().empty())
+  shortname = cxx_method.GetContext().str() + "::";
+shortname += cxx_method.GetBasename().str();
+return ConstString(shortname);
+  }
+}
+  }
+  if (demangled_name)
+return demangled_name;
+  return mangled.GetMangledName();
+}
+
 // PluginInterface protocol
 
 lldb_private::ConstString CPlusPlusLanguage::GetPluginName() {
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Core/Mangled.h"
 
 #include "lldb/Core/RichManglingContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
@@ -16,8 +17,6 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-enumerations.h"
 
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
-
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/Support/Compiler.h"
@@ -34,35 +33,6 @@
   return Mangled::GetManglingScheme(s) != Mangled::eManglingSchemeNone;
 }
 
-static ConstString GetDemangledNameWithoutArguments(ConstString mangled,
-ConstString demangled) {
-  const char *mangled_name_cstr = mangled.GetCString();
-
-  if (demangled && mangled_name_cstr && mangled_name_cstr[0]) {
-if (mangled_name_cstr[0] == '_' && mangled_name_cstr[1] == 'Z' &&
-(mangled_name_cstr[2] != 'T' && // avoid virtual table, VTT structure,
-// typeinfo structure, and typeinfo
-// mangled_name
- mangled_name_cstr[2] != 'G' && // avoid guard variables
- mangled_name_cstr[2] != 'Z')) // named local entities (if we eventually
-   // handle eSymbolTypeData, we will want
-   // this back)
-{
-  CPlusPlusLanguage::MethodName cxx_method(demangled);
-  if (!cxx_method.GetBasename().empty()) {
-std::string shortname;
-if (!cxx_method.GetContext().empty())
-  shortname = cxx_method.GetContext().str() + "::";
-shortname += cxx_method.GetBasename().str();
-return ConstString(shortname);
-  }
-}
-  }
-  if (demangled)
-return demangled;
-  return mangled;
-}

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

2021-07-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision.
jingham added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106171

___
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

2021-07-29 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106171

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


[Lldb-commits] [lldb] 26ba774 - Simplify testcase to use v instead of p (NFC)

2021-07-29 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2021-07-29T15:15:00-07:00
New Revision: 26ba774f6865f0c6bb15dbbe80dc23e1db33d54b

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

LOG: Simplify testcase to use v instead of p (NFC)

Added: 


Modified: 
lldb/test/API/commands/process/attach/TestProcessAttach.py

Removed: 




diff  --git a/lldb/test/API/commands/process/attach/TestProcessAttach.py 
b/lldb/test/API/commands/process/attach/TestProcessAttach.py
index c224b9f109b32..b2173f40d4dab 100644
--- a/lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ b/lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -106,7 +106,7 @@ def 
test_attach_to_process_by_id_correct_executable_offset(self):
 lldbutil.run_break_set_by_file_and_line(
 self, "main.cpp", self.line, num_expected_locations=1, 
loc_exact=False)
 self.runCmd("process continue")
-self.expect("p g_val", substrs=["$0 = 12345"])
+self.expect("v g_val", substrs=["12345"])
 
 def tearDown(self):
 # Destroy process before TestBase.tearDown()



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


[Lldb-commits] [lldb] 0fd813c - Fix typo

2021-07-29 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2021-07-29T16:23:13-07:00
New Revision: 0fd813cf19c754a6cb3f2483332038a49eac33bd

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

LOG: Fix typo

Added: 


Modified: 
lldb/test/API/commands/process/attach/TestProcessAttach.py

Removed: 




diff  --git a/lldb/test/API/commands/process/attach/TestProcessAttach.py 
b/lldb/test/API/commands/process/attach/TestProcessAttach.py
index b2173f40d4da..b33aeebccdd1 100644
--- a/lldb/test/API/commands/process/attach/TestProcessAttach.py
+++ b/lldb/test/API/commands/process/attach/TestProcessAttach.py
@@ -102,7 +102,7 @@ def 
test_attach_to_process_by_id_correct_executable_offset(self):
 
 self.runCmd("process attach -p " + str(popen.pid))
 
-# Make suer we did not attach to early
+# Make sure we did not attach too early.
 lldbutil.run_break_set_by_file_and_line(
 self, "main.cpp", self.line, num_expected_locations=1, 
loc_exact=False)
 self.runCmd("process continue")



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


[Lldb-commits] [lldb] 648844f - Make testcase more robust against codegen changes

2021-07-29 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2021-07-29T16:23:13-07:00
New Revision: 648844fd69fa304a04aab2e39dcb114c89d078bf

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

LOG: Make testcase more robust against codegen changes

Added: 


Modified: 
lldb/test/API/commands/process/attach/main.cpp

Removed: 




diff  --git a/lldb/test/API/commands/process/attach/main.cpp 
b/lldb/test/API/commands/process/attach/main.cpp
index a43ed8eac5b2..b4ed48fade30 100644
--- a/lldb/test/API/commands/process/attach/main.cpp
+++ b/lldb/test/API/commands/process/attach/main.cpp
@@ -12,9 +12,8 @@ int main(int argc, char const *argv[]) {
 // Waiting to be attached by the debugger.
 temp = 0;
 
-while (temp < 30) // Waiting to be attached...
-{
-std::this_thread::sleep_for(std::chrono::seconds(2));
+while (temp < 30) {
+std::this_thread::sleep_for(std::chrono::seconds(2)); // Waiting to be 
attached...
 temp++;
 }
 



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


[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

2021-07-29 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments.
Herald added a subscriber: dang.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py:21
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
-def test_breakpoint_command_sequence(self):
+def not_test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""

I've been trying to track a regression in source maps between Android Studio 
versions and it seems that this test shouldn't have been disabled. :(


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68671

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


[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

2021-07-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py:21
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
-def test_breakpoint_command_sequence(self):
+def not_test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""

wallace wrote:
> I've been trying to track a regression in source maps between Android Studio 
> versions and it seems that this test shouldn't have been disabled. :(
Yup, my bad.  I sometimes disable tests this way when I'm only trying to run 
one test in the set a bunch of times.  I must have forgotten to set it back.  
Thanks for catching this!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68671

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


[Lldb-commits] [PATCH] D107126: [source map] fix relative path breakpoints

2021-07-29 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added a reviewer: clayborg.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

https://reviews.llvm.org/D45592 added a nice feature to be able to specify a 
breakpoint by a relative path. E.g. passing foo.cpp or bar/foo.cpp or 
zaz/bar/foo.cpp is fine. However, https://reviews.llvm.org/D68671 by mistake 
disabled the test that ensured this functionality works. With time, someone 
made a small mistake and fully broke the functionality.

So, I'm making a very simple fix and the test passes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107126

Files:
  lldb/source/Breakpoint/BreakpointResolverFileLine.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
@@ -18,7 +18,7 @@
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 @skipIfReproducer # side_effect bypasses reproducer
-def not_test_breakpoint_command_sequence(self):
+def test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""
 self.build()
 self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@
 bp_id_1 = bp_1.GetID()
 bp_id_2 = bp_2.GetID()
 bp_id_3 = bp_3.GetID()
-
+
 self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -320,7 +320,7 @@
 bp_1.SetEnabled(False)
 
 bp_id_1 = bp_1.GetID()
-
+
 self.runCmd("breakpoint delete --disabled")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -331,4 +331,3 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
-
Index: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -188,7 +188,7 @@
 // is 0, then we can't do this calculation.  That can happen if
 // GetStartLineSourceInfo gets an error, or if the first line number in
 // the function really is 0 - which happens for some languages.
-
+
 // But only do this calculation if the line number we found in the SC
 // was different from the one requested in the source file.  If we actually
 // found an exact match it must be valid.
@@ -233,14 +233,18 @@
   const bool is_relative = search_file_spec.IsRelative();
   if (is_relative)
 search_file_spec.GetDirectory().Clear();
+  SourceLocationSpec search_location_spec(
+  search_file_spec, m_location_spec.GetLine().getValueOr(0),
+  m_location_spec.GetColumn(), m_location_spec.GetCheckInlines(),
+  m_location_spec.GetExactMatch());
 
   const size_t num_comp_units = context.module_sp->GetNumCompileUnits();
   for (size_t i = 0; i < num_comp_units; i++) {
 CompUnitSP cu_sp(context.module_sp->GetCompileUnitAtIndex(i));
 if (cu_sp) {
   if (filter.CompUnitPasses(*cu_sp))
-cu_sp->ResolveSymbolContext(m_location_spec, eSymbolContextEverything,
-sc_list);
+cu_sp->ResolveSymbolContext(search_location_spec,
+eSymbolContextEverything, sc_list);
 }
   }
 


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
@@ -18,7 +18,7 @@
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 @skipIfReproducer # side_effect bypasses reproducer
-def not_test_breakpoint_command_sequence(self):
+def test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""
 self.build()
 self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@
 bp_id_1 = bp_1.GetID()
 bp_id_2 = bp_2.GetID()
 bp_id_3 = bp_3.GetID()
-
+
 self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -320,7 +320,7 @@
 bp_1.SetEnabled(False)
 
 bp_id_1 = bp_1.GetID()
-
+
 self.runCmd("breakpoint delete --disabled")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -331,4 +331,3 @@
 
 bp_3 = target.FindBr

[Lldb-commits] [PATCH] D68671: Add the ability to pass extra args to a Python breakpoint command function

2021-07-29 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py:21
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
-def test_breakpoint_command_sequence(self):
+def not_test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""

jingham wrote:
> wallace wrote:
> > I've been trying to track a regression in source maps between Android 
> > Studio versions and it seems that this test shouldn't have been disabled. :(
> Yup, my bad.  I sometimes disable tests this way when I'm only trying to run 
> one test in the set a bunch of times.  I must have forgotten to set it back.  
> Thanks for catching this!
No worries!! It was fun, though


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68671

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


[Lldb-commits] [PATCH] D107126: [source map] fix relative path breakpoints

2021-07-29 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 362943.
wallace added a comment.
Herald added a subscriber: JDevlieghere.

add a comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107126

Files:
  lldb/source/Breakpoint/BreakpointResolverFileLine.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
@@ -18,7 +18,7 @@
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 @skipIfReproducer # side_effect bypasses reproducer
-def not_test_breakpoint_command_sequence(self):
+def test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""
 self.build()
 self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@
 bp_id_1 = bp_1.GetID()
 bp_id_2 = bp_2.GetID()
 bp_id_3 = bp_3.GetID()
-
+
 self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -320,7 +320,7 @@
 bp_1.SetEnabled(False)
 
 bp_id_1 = bp_1.GetID()
-
+
 self.runCmd("breakpoint delete --disabled")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -331,4 +331,3 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
-
Index: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -188,7 +188,7 @@
 // is 0, then we can't do this calculation.  That can happen if
 // GetStartLineSourceInfo gets an error, or if the first line number in
 // the function really is 0 - which happens for some languages.
-
+
 // But only do this calculation if the line number we found in the SC
 // was different from the one requested in the source file.  If we actually
 // found an exact match it must be valid.
@@ -229,18 +229,25 @@
   const uint32_t line = m_location_spec.GetLine().getValueOr(0);
   const llvm::Optional column = m_location_spec.GetColumn();
 
+  // We'll create a new SourceLocationSpec that can take into account the
+  // relative path case, and we'll use it to resolve the symbol context
+  // of the CUs.
   FileSpec search_file_spec = m_location_spec.GetFileSpec();
   const bool is_relative = search_file_spec.IsRelative();
   if (is_relative)
 search_file_spec.GetDirectory().Clear();
+  SourceLocationSpec search_location_spec(
+  search_file_spec, m_location_spec.GetLine().getValueOr(0),
+  m_location_spec.GetColumn(), m_location_spec.GetCheckInlines(),
+  m_location_spec.GetExactMatch());
 
   const size_t num_comp_units = context.module_sp->GetNumCompileUnits();
   for (size_t i = 0; i < num_comp_units; i++) {
 CompUnitSP cu_sp(context.module_sp->GetCompileUnitAtIndex(i));
 if (cu_sp) {
   if (filter.CompUnitPasses(*cu_sp))
-cu_sp->ResolveSymbolContext(m_location_spec, eSymbolContextEverything,
-sc_list);
+cu_sp->ResolveSymbolContext(search_location_spec,
+eSymbolContextEverything, sc_list);
 }
   }
 


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
@@ -18,7 +18,7 @@
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 @skipIfReproducer # side_effect bypasses reproducer
-def not_test_breakpoint_command_sequence(self):
+def test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""
 self.build()
 self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@
 bp_id_1 = bp_1.GetID()
 bp_id_2 = bp_2.GetID()
 bp_id_3 = bp_3.GetID()
-
+
 self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -320,7 +320,7 @@
 bp_1.SetEnabled(False)
 
 bp_id_1 = bp_1.GetID()
-
+
 self.runCmd("breakpoint delete --disabled")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -331,4 +331,3 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFal

[Lldb-commits] [PATCH] D107126: [source map] fix relative path breakpoints

2021-07-29 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107126

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


[Lldb-commits] [PATCH] D107126: [source map] fix relative path breakpoints

2021-07-29 Thread 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 rG0a68443bd07c: [source map] fix relative path breakpoints 
(authored by Walter Erquinigo ).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107126

Files:
  lldb/source/Breakpoint/BreakpointResolverFileLine.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
@@ -18,7 +18,7 @@
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 @skipIfReproducer # side_effect bypasses reproducer
-def not_test_breakpoint_command_sequence(self):
+def test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""
 self.build()
 self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@
 bp_id_1 = bp_1.GetID()
 bp_id_2 = bp_2.GetID()
 bp_id_3 = bp_3.GetID()
-
+
 self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -320,7 +320,7 @@
 bp_1.SetEnabled(False)
 
 bp_id_1 = bp_1.GetID()
-
+
 self.runCmd("breakpoint delete --disabled")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -331,4 +331,3 @@
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
-
Index: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -188,7 +188,7 @@
 // is 0, then we can't do this calculation.  That can happen if
 // GetStartLineSourceInfo gets an error, or if the first line number in
 // the function really is 0 - which happens for some languages.
-
+
 // But only do this calculation if the line number we found in the SC
 // was different from the one requested in the source file.  If we actually
 // found an exact match it must be valid.
@@ -229,18 +229,25 @@
   const uint32_t line = m_location_spec.GetLine().getValueOr(0);
   const llvm::Optional column = m_location_spec.GetColumn();
 
+  // We'll create a new SourceLocationSpec that can take into account the
+  // relative path case, and we'll use it to resolve the symbol context
+  // of the CUs.
   FileSpec search_file_spec = m_location_spec.GetFileSpec();
   const bool is_relative = search_file_spec.IsRelative();
   if (is_relative)
 search_file_spec.GetDirectory().Clear();
+  SourceLocationSpec search_location_spec(
+  search_file_spec, m_location_spec.GetLine().getValueOr(0),
+  m_location_spec.GetColumn(), m_location_spec.GetCheckInlines(),
+  m_location_spec.GetExactMatch());
 
   const size_t num_comp_units = context.module_sp->GetNumCompileUnits();
   for (size_t i = 0; i < num_comp_units; i++) {
 CompUnitSP cu_sp(context.module_sp->GetCompileUnitAtIndex(i));
 if (cu_sp) {
   if (filter.CompUnitPasses(*cu_sp))
-cu_sp->ResolveSymbolContext(m_location_spec, eSymbolContextEverything,
-sc_list);
+cu_sp->ResolveSymbolContext(search_location_spec,
+eSymbolContextEverything, sc_list);
 }
   }
 


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
@@ -18,7 +18,7 @@
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 @skipIfReproducer # side_effect bypasses reproducer
-def not_test_breakpoint_command_sequence(self):
+def test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""
 self.build()
 self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@
 bp_id_1 = bp_1.GetID()
 bp_id_2 = bp_2.GetID()
 bp_id_3 = bp_3.GetID()
-
+
 self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -320,7 +320,7 @@
 bp_1.SetEnabled(False)
 
 bp_id_1 = bp_1.GetID()
-
+
 self.runCmd("breakpoint delete --disabled")
 
 bp_1 = tar

[Lldb-commits] [lldb] 0a68443 - [source map] fix relative path breakpoints

2021-07-29 Thread Walter Erquinigo via lldb-commits

Author: Walter Erquinigo
Date: 2021-07-29T18:36:06-07:00
New Revision: 0a68443bd07c48dbaf554f6e3b26b3ab5ed1d383

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

LOG: [source map] fix relative path breakpoints

https://reviews.llvm.org/D45592 added a nice feature to be able to specify a 
breakpoint by a relative path. E.g. passing foo.cpp or bar/foo.cpp or 
zaz/bar/foo.cpp is fine. However, https://reviews.llvm.org/D68671 by mistake 
disabled the test that ensured this functionality works. With time, someone 
made a small mistake and fully broke the functionality.

So, I'm making a very simple fix and the test passes.

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

Added: 


Modified: 
lldb/source/Breakpoint/BreakpointResolverFileLine.cpp

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

Removed: 




diff  --git a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp 
b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
index 1d1ac2e90bdc..be4616064f9e 100644
--- a/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
@@ -188,7 +188,7 @@ void 
BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list,
 // is 0, then we can't do this calculation.  That can happen if
 // GetStartLineSourceInfo gets an error, or if the first line number in
 // the function really is 0 - which happens for some languages.
-
+
 // But only do this calculation if the line number we found in the SC
 // was 
diff erent from the one requested in the source file.  If we actually
 // found an exact match it must be valid.
@@ -229,18 +229,25 @@ Searcher::CallbackReturn 
BreakpointResolverFileLine::SearchCallback(
   const uint32_t line = m_location_spec.GetLine().getValueOr(0);
   const llvm::Optional column = m_location_spec.GetColumn();
 
+  // We'll create a new SourceLocationSpec that can take into account the
+  // relative path case, and we'll use it to resolve the symbol context
+  // of the CUs.
   FileSpec search_file_spec = m_location_spec.GetFileSpec();
   const bool is_relative = search_file_spec.IsRelative();
   if (is_relative)
 search_file_spec.GetDirectory().Clear();
+  SourceLocationSpec search_location_spec(
+  search_file_spec, m_location_spec.GetLine().getValueOr(0),
+  m_location_spec.GetColumn(), m_location_spec.GetCheckInlines(),
+  m_location_spec.GetExactMatch());
 
   const size_t num_comp_units = context.module_sp->GetNumCompileUnits();
   for (size_t i = 0; i < num_comp_units; i++) {
 CompUnitSP cu_sp(context.module_sp->GetCompileUnitAtIndex(i));
 if (cu_sp) {
   if (filter.CompUnitPasses(*cu_sp))
-cu_sp->ResolveSymbolContext(m_location_spec, eSymbolContextEverything,
-sc_list);
+cu_sp->ResolveSymbolContext(search_location_spec,
+eSymbolContextEverything, sc_list);
 }
   }
 

diff  --git 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
 
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
index 5f10cd751451..3fc21ad7eed9 100644
--- 
a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
+++ 
b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
@@ -18,7 +18,7 @@ class BreakpointCommandTestCase(TestBase):
 
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24528")
 @skipIfReproducer # side_effect bypasses reproducer
-def not_test_breakpoint_command_sequence(self):
+def test_breakpoint_command_sequence(self):
 """Test a sequence of breakpoint command add, list, and delete."""
 self.build()
 self.breakpoint_command_sequence()
@@ -302,7 +302,7 @@ def test_breakpoint_delete_disabled(self):
 bp_id_1 = bp_1.GetID()
 bp_id_2 = bp_2.GetID()
 bp_id_3 = bp_3.GetID()
-
+
 self.runCmd("breakpoint delete --disabled DeleteMeNot")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -320,7 +320,7 @@ def test_breakpoint_delete_disabled(self):
 bp_1.SetEnabled(False)
 
 bp_id_1 = bp_1.GetID()
-
+
 self.runCmd("breakpoint delete --disabled")
 
 bp_1 = target.FindBreakpointByID(bp_id_1)
@@ -331,4 +331,3 @@ def test_breakpoint_delete_disabled(self):
 
 bp_3 = target.FindBreakpointByID(bp_id_3)
 self.assertFalse(bp_3.IsValid(), "Didn't delete disabled breakpoint 3")
-



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

[Lldb-commits] [PATCH] D105741: [trace] Introduce Hierarchical Trace Representation (HTR) and add `thread trace export ctf` command for Intel PT trace visualization

2021-07-29 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments.



Comment at: 
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp:78
+  : LLDB_INVALID_THREAD_ID;
+result.AppendErrorWithFormat(
+"Thread index %lu is out of range (valid values are 0 - %u).\n", tid,

teemperor wrote:
> This generates a compiler warning:
> ```
> [3344/4085] Building CXX object 
> tools/lldb/source/Plugins/TraceExporter/ctf/CMakeFiles/lldbPluginTraceExporterCTF.dir/CommandObjectThreadTraceExportCTF.cpp.o
> /Users/teemperor/3llvm/llvm-project/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp:79:82:
>  warning: format specifies type 'unsigned long long' but the argument has 
> type 'size_t' (aka 'unsigned long') [-Wformat]
> "Thread index %" PRIu64 " is out of range (valid values are 0 - 
> %u).\n", tid,
>   ~   
>^~~
> ```
> 
> The variadic format methods are obsolete and you can just use 
> `llvm::formatv(...).str()` until we have a type-safe replacement.
thanks for the suggestion. TIL :) 
I'll make a quick fix for this


Repository:
  rG LLVM Github Monorepo

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] [lldb] 5839976 - [nfc][trace] use formatv instead of the old Printf

2021-07-29 Thread Walter Erquinigo via lldb-commits

Author: Walter Erquinigo
Date: 2021-07-29T19:04:59-07:00
New Revision: 5839976976bc018b674f45f56f8a13707b870443

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

LOG: [nfc][trace] use formatv instead of the old Printf

It was suggested in https://reviews.llvm.org/D105741 and it makes sense.

Added: 


Modified: 
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp 
b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
index 873eda687dd8..9e994ea829e9 100644
--- 
a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
+++ 
b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
@@ -75,9 +75,11 @@ bool CommandObjectThreadTraceExportCTF::DoExecute(Args 
&command,
 const uint32_t num_threads = process->GetThreadList().GetSize();
 size_t tid = m_options.m_thread_index ? *m_options.m_thread_index
   : LLDB_INVALID_THREAD_ID;
-result.AppendErrorWithFormat(
-"Thread index %" PRIu64 " is out of range (valid values are 0 - 
%u).\n", tid,
-num_threads);
+result.AppendError(
+llvm::formatv(
+"Thread index {0} is out of range (valid values are 1 - {1}).\n",
+tid, num_threads)
+.str());
 return false;
   } else {
 TraceHTR htr(*thread, *trace_sp->GetCursor(*thread));



___
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

2021-07-29 Thread Kim-Anh Tran via Phabricator via lldb-commits
kimanh added a comment.

In D106355#2913605 , @jankratochvil 
wrote:

> You can also consider coding `lldb-add-index` for better performance as that 
> is too much for my available time.

I don't think that I'll get to it either. But we'll also evaluate the 
performance for our users and may need to do more, and in that case we might 
have a look!

> I guess you could already ask for a commit access, you have multiple patches 
> checked in.

Thanks for the suggestion, I just requested it now!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106355

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