[Lldb-commits] [PATCH] D118794: [lldb] Remove non-address bits from read/write addresses in lldb

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 430276.
DavidSpickett added a comment.

Correct "addr" -> "fixed_addr" in a couple of comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

Files:
  lldb/source/Target/Process.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -0,0 +1,25 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  // Note that we allocate memory here because if we used
+  // stack or globals lldb might read it in the course of
+  // running to the breakpoint. Before the test can look
+  // for those reads.
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE,
+   MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+  if (buf == MAP_FAILED)
+return 1;
+
+#define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
+
+  // Set top byte to something.
+  char *buf_with_non_address = (char *)((size_t)buf | (size_t)0xff << 56);
+  sign_ptr(buf_with_non_address);
+  // Address is now:
+  // <8 bit top byte tag>
+
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -0,0 +1,182 @@
+"""
+Test that lldb removes non-address bits in situations where they would cause
+failures if not removed. Like when reading memory. Tests are done at command
+and API level because commands may remove non-address bits for display
+reasons which can make it seem like the operation as a whole works but at the
+API level it won't if we don't remove them there also.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxNonAddressBitMemoryAccessTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup_test(self):
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+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', '// Set break point at this line.'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+def check_cmd_read_write(self, write_to, read_from, data):
+self.runCmd("memory write {} {}".format(write_to, data))
+self.expect("memory read {}".format(read_from),
+substrs=[data])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_non_address_bit_memory_read_write_cmds(self):
+self.setup_test()
+
+# Writes should be visible through either pointer
+self.check_cmd_read_write("buf", "buf", "01 02 03 04")
+self.check_cmd_read_write("buf_with_non_address", "buf_with_non_address", "02 03 04 05")
+self.check_cmd_read_write("buf", "buf_with_non_address", "03 04 05 06")
+self.check_cmd_read_write("buf_with_non_address", "buf", "04 05 06 07")
+
+# Printing either should get the same result
+self.expect("expression -f hex -- *(uint32_t*)buf", substrs=["0x07060504"])
+self.expect("expression -f hex -- *(uint32_t*)buf_with_non_address",
+substrs=["0x07060504"])
+
+def get_ptr_values(self):
+frame  = self.process().GetThreadAtIndex(0).GetFrameAtIndex(0)
+buf = frame.FindVariable("buf").GetValueAsUnsigned()
+buf_with_non_address = frame.FindVariable("buf_with_non_address").GetValueAsUnsigned()
+return buf, buf_with_non_address
+
+def check_api_read_write(self, write_to, read_from, data):
+error = lldb.SBError()
+written = self.process().WriteMemory(write_to, data, error)
+self.assertTrue(error.Success())
+self.assertEqual(len(data), written)
+buf

[Lldb-commits] [lldb] 7d8ec4d - [lldb] const a couple of getters on MemoryRegionInfo

2022-05-18 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2022-05-18T09:00:00Z
New Revision: 7d8ec4dc4461102bafed8063977a66e40562bbb3

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

LOG: [lldb] const a couple of getters on MemoryRegionInfo

GetDirtyPageList was being assigned to const & in most places anyway.
If you wanted to change the list you'd make a new one and call
SetDirtyPageList.

GetPageSize is just an int so no issues being const.

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

Added: 


Modified: 
lldb/include/lldb/Target/MemoryRegionInfo.h
lldb/source/API/SBMemoryRegionInfo.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/MemoryRegionInfo.h 
b/lldb/include/lldb/Target/MemoryRegionInfo.h
index fc5fcff5159ee..acca66e838337 100644
--- a/lldb/include/lldb/Target/MemoryRegionInfo.h
+++ b/lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -107,13 +107,13 @@ class MemoryRegionInfo {
   /// Get the target system's VM page size in bytes.
   /// \return
   /// 0 is returned if this information is unavailable.
-  int GetPageSize() { return m_pagesize; }
+  int GetPageSize() const { return m_pagesize; }
 
   /// Get a vector of target VM pages that are dirty -- that have been
   /// modified -- within this memory region.  This is an Optional return
   /// value; it will only be available if the remote stub was able to
   /// detail this.
-  llvm::Optional> &GetDirtyPageList() {
+  const llvm::Optional> &GetDirtyPageList() const {
 return m_dirty_pages;
   }
 

diff  --git a/lldb/source/API/SBMemoryRegionInfo.cpp 
b/lldb/source/API/SBMemoryRegionInfo.cpp
index 7d9db478dde17..1a16622b55742 100644
--- a/lldb/source/API/SBMemoryRegionInfo.cpp
+++ b/lldb/source/API/SBMemoryRegionInfo.cpp
@@ -133,7 +133,7 @@ uint32_t SBMemoryRegionInfo::GetNumDirtyPages() {
   LLDB_INSTRUMENT_VA(this);
 
   uint32_t num_dirty_pages = 0;
-  llvm::Optional> dirty_page_list =
+  const llvm::Optional> &dirty_page_list =
   m_opaque_up->GetDirtyPageList();
   if (dirty_page_list.hasValue())
 num_dirty_pages = dirty_page_list.getValue().size();



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


[Lldb-commits] [PATCH] D125786: [lldb] const a couple of getters on MemoryRegionInfo

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d8ec4dc4461: [lldb] const a couple of getters on 
MemoryRegionInfo (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125786

Files:
  lldb/include/lldb/Target/MemoryRegionInfo.h
  lldb/source/API/SBMemoryRegionInfo.cpp


Index: lldb/source/API/SBMemoryRegionInfo.cpp
===
--- lldb/source/API/SBMemoryRegionInfo.cpp
+++ lldb/source/API/SBMemoryRegionInfo.cpp
@@ -133,7 +133,7 @@
   LLDB_INSTRUMENT_VA(this);
 
   uint32_t num_dirty_pages = 0;
-  llvm::Optional> dirty_page_list =
+  const llvm::Optional> &dirty_page_list =
   m_opaque_up->GetDirtyPageList();
   if (dirty_page_list.hasValue())
 num_dirty_pages = dirty_page_list.getValue().size();
Index: lldb/include/lldb/Target/MemoryRegionInfo.h
===
--- lldb/include/lldb/Target/MemoryRegionInfo.h
+++ lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -107,13 +107,13 @@
   /// Get the target system's VM page size in bytes.
   /// \return
   /// 0 is returned if this information is unavailable.
-  int GetPageSize() { return m_pagesize; }
+  int GetPageSize() const { return m_pagesize; }
 
   /// Get a vector of target VM pages that are dirty -- that have been
   /// modified -- within this memory region.  This is an Optional return
   /// value; it will only be available if the remote stub was able to
   /// detail this.
-  llvm::Optional> &GetDirtyPageList() {
+  const llvm::Optional> &GetDirtyPageList() const {
 return m_dirty_pages;
   }
 


Index: lldb/source/API/SBMemoryRegionInfo.cpp
===
--- lldb/source/API/SBMemoryRegionInfo.cpp
+++ lldb/source/API/SBMemoryRegionInfo.cpp
@@ -133,7 +133,7 @@
   LLDB_INSTRUMENT_VA(this);
 
   uint32_t num_dirty_pages = 0;
-  llvm::Optional> dirty_page_list =
+  const llvm::Optional> &dirty_page_list =
   m_opaque_up->GetDirtyPageList();
   if (dirty_page_list.hasValue())
 num_dirty_pages = dirty_page_list.getValue().size();
Index: lldb/include/lldb/Target/MemoryRegionInfo.h
===
--- lldb/include/lldb/Target/MemoryRegionInfo.h
+++ lldb/include/lldb/Target/MemoryRegionInfo.h
@@ -107,13 +107,13 @@
   /// Get the target system's VM page size in bytes.
   /// \return
   /// 0 is returned if this information is unavailable.
-  int GetPageSize() { return m_pagesize; }
+  int GetPageSize() const { return m_pagesize; }
 
   /// Get a vector of target VM pages that are dirty -- that have been
   /// modified -- within this memory region.  This is an Optional return
   /// value; it will only be available if the remote stub was able to
   /// detail this.
-  llvm::Optional> &GetDirtyPageList() {
+  const llvm::Optional> &GetDirtyPageList() const {
 return m_dirty_pages;
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D125786: [lldb] const a couple of getters on MemoryRegionInfo

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett added a comment.

Landed as obvious as I'll shortly land the follow up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125786

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


[Lldb-commits] [PATCH] D111791: [lldb] Add --all option to "memory region"

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 430293.
DavidSpickett added a comment.

Fixup the tagged memory region test (it doesn't care about the
content of the usage just that the command eventually fails) and rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111791

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  
lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
 Changes to LLDB
 -
 
+* The "memory region" command now has a "--all" option to list all
+  memory regions (including unmapped ranges). This is the equivalent
+  of using address 0 then repeating the command until all regions
+  have been listed.
+
 Changes to Sanitizers
 -
 
Index: lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
===
--- lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
+++ lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -55,7 +55,7 @@
 if result.Succeeded():
 repeats += 1
 else:
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertRegexpMatches(result.GetError(), "Usage: memory region")
 break
 
 # This time repeat until we get the last region. At that
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,6 +23,12 @@
 'main.cpp',
 '// Run here before printing memory regions')
 
+def test_help(self):
+""" Test that help shows you must have one of address or --all, not both."""
+self.expect("help memory region",
+substrs=["memory region ",
+ "memory region -a"])
+
 def test(self):
 self.build()
 
@@ -39,7 +45,15 @@
 # Test that the first 'memory region' command prints the usage.
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertEqual(result.GetError(),
+"error: 'memory region' takes one argument or \"--all\" option:\n"
+"Usage: memory region  (or --all)\n")
+
+# We allow --all or an address argument, not both
+interp.HandleCommand("memory region --all 0", result)
+self.assertFalse(result.Succeeded())
+self.assertRegexpMatches(result.GetError(),
+"The \"--all\" option cannot be used when an address argument is given")
 
 # Test that when the address fails to parse, we show an error and do not continue
 interp.HandleCommand("memory region not_an_address", result)
@@ -47,18 +61,28 @@
 self.assertEqual(result.GetError(),
 "error: invalid address argument \"not_an_address\": address expression \"not_an_address\" evaluation failed\n")
 
+# Accumulate the results to compare with the --all output
+all_regions = ""
+
 # Now let's print the memory region starting at 0 which should always work.
 interp.HandleCommand("memory region 0x0", result)
 self.assertTrue(result.Succeeded())
 self.assertRegexpMatches(result.GetOutput(), "\\[0x0+-")
+all_regions += result.GetOutput()
 
 # Keep printing memory regions until we printed all of them.
 while True:
 interp.HandleCommand("memory region", result)
 if not result.Succeeded():
 break
+all_regions += result.GetOutput()
 
 # Now that we reached the end, 'memory region' should again print the usage.
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertRegexpMatches(result.GetError(), "Usage: memory region  \(or \-\-all\)")
+
+# --all should match what repeating the command gives you
+interp.HandleCommand("memory region --all", result)
+self.assertTrue(result.Succeeded())
+self.assertEqual(result.GetOutput(), all_regions)
Index: lldb/source/Commands/Options.td
==

[Lldb-commits] [PATCH] D111791: [lldb] Add --all option to "memory region"

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 430312.
DavidSpickett added a comment.

Fixup the loop on systems that have more non-address bits than just top byte 
ignore.

With only top byte ignore:

- We ask the remote for the last region
- It says ok it's unmapped and it ends at 0xFF
- Then we mask that address using just the top byte ignore but because the PAC 
sign bit is set we actually or in the mask which keeps all the bits set.
- We ask for a region at LLDB_INVALID_ADDRESS which fails and we break.

With top byte ignore plus pointer auth:

- We ask for the last region
- It returns a range with end address 0x0001
- Then we ask for the next region
- Which masks that address with 0x0...0 and gets 0
- Which gives you the first region and we loop forever

This is the same thing I did to fix the manual command repetition but
because we got away without it on a TBI only system I didn't notice it
until now.

I figured it needed the same fix but took a bit to figure out why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111791

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  
lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
 Changes to LLDB
 -
 
+* The "memory region" command now has a "--all" option to list all
+  memory regions (including unmapped ranges). This is the equivalent
+  of using address 0 then repeating the command until all regions
+  have been listed.
+
 Changes to Sanitizers
 -
 
Index: lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
===
--- lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
+++ lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -55,7 +55,7 @@
 if result.Succeeded():
 repeats += 1
 else:
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertRegexpMatches(result.GetError(), "Usage: memory region")
 break
 
 # This time repeat until we get the last region. At that
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,6 +23,12 @@
 'main.cpp',
 '// Run here before printing memory regions')
 
+def test_help(self):
+""" Test that help shows you must have one of address or --all, not both."""
+self.expect("help memory region",
+substrs=["memory region ",
+ "memory region -a"])
+
 def test(self):
 self.build()
 
@@ -39,7 +45,15 @@
 # Test that the first 'memory region' command prints the usage.
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertEqual(result.GetError(),
+"error: 'memory region' takes one argument or \"--all\" option:\n"
+"Usage: memory region  (or --all)\n")
+
+# We allow --all or an address argument, not both
+interp.HandleCommand("memory region --all 0", result)
+self.assertFalse(result.Succeeded())
+self.assertRegexpMatches(result.GetError(),
+"The \"--all\" option cannot be used when an address argument is given")
 
 # Test that when the address fails to parse, we show an error and do not continue
 interp.HandleCommand("memory region not_an_address", result)
@@ -47,18 +61,28 @@
 self.assertEqual(result.GetError(),
 "error: invalid address argument \"not_an_address\": address expression \"not_an_address\" evaluation failed\n")
 
+# Accumulate the results to compare with the --all output
+all_regions = ""
+
 # Now let's print the memory region starting at 0 which should always work.
 interp.HandleCommand("memory region 0x0", result)
 self.assertTrue(result.Succeeded())
 self.assertRegexpMatches(result.GetOutput(), "\\[0x0+-")
+all_regions += result.GetOutput()
 
 # Keep printing memory regions until we printed all of them.
 while True:
 interp.HandleCommand("memory region", resul

[Lldb-commits] [PATCH] D111791: [lldb] Add --all option to "memory region"

2022-05-18 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 rG8e648f195c3d: [lldb] Add --all option to "memory 
region" (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111791

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/Options.td
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  
lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
 Changes to LLDB
 -
 
+* The "memory region" command now has a "--all" option to list all
+  memory regions (including unmapped ranges). This is the equivalent
+  of using address 0 then repeating the command until all regions
+  have been listed.
+
 Changes to Sanitizers
 -
 
Index: lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
===
--- lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
+++ lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -55,7 +55,7 @@
 if result.Succeeded():
 repeats += 1
 else:
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertRegexpMatches(result.GetError(), "Usage: memory region")
 break
 
 # This time repeat until we get the last region. At that
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,6 +23,12 @@
 'main.cpp',
 '// Run here before printing memory regions')
 
+def test_help(self):
+""" Test that help shows you must have one of address or --all, not both."""
+self.expect("help memory region",
+substrs=["memory region ",
+ "memory region -a"])
+
 def test(self):
 self.build()
 
@@ -39,7 +45,15 @@
 # Test that the first 'memory region' command prints the usage.
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertEqual(result.GetError(),
+"error: 'memory region' takes one argument or \"--all\" option:\n"
+"Usage: memory region  (or --all)\n")
+
+# We allow --all or an address argument, not both
+interp.HandleCommand("memory region --all 0", result)
+self.assertFalse(result.Succeeded())
+self.assertRegexpMatches(result.GetError(),
+"The \"--all\" option cannot be used when an address argument is given")
 
 # Test that when the address fails to parse, we show an error and do not continue
 interp.HandleCommand("memory region not_an_address", result)
@@ -47,18 +61,28 @@
 self.assertEqual(result.GetError(),
 "error: invalid address argument \"not_an_address\": address expression \"not_an_address\" evaluation failed\n")
 
+# Accumulate the results to compare with the --all output
+all_regions = ""
+
 # Now let's print the memory region starting at 0 which should always work.
 interp.HandleCommand("memory region 0x0", result)
 self.assertTrue(result.Succeeded())
 self.assertRegexpMatches(result.GetOutput(), "\\[0x0+-")
+all_regions += result.GetOutput()
 
 # Keep printing memory regions until we printed all of them.
 while True:
 interp.HandleCommand("memory region", result)
 if not result.Succeeded():
 break
+all_regions += result.GetOutput()
 
 # Now that we reached the end, 'memory region' should again print the usage.
 interp.HandleCommand("memory region", result)
 self.assertFalse(result.Succeeded())
-self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+self.assertRegexpMatches(result.GetError(), "Usage: memory region  \(or \-\-all\)")
+
+# --all should match what repeating the command gives you
+interp.HandleCommand("memory region --all", result)
+self.assertTrue(result.Succeeded())
+self.assertEqual(result.GetOutput(), all_regions)
Index: lldb/source/Commands/Options.td
===

[Lldb-commits] [lldb] 8e648f1 - [lldb] Add --all option to "memory region"

2022-05-18 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2022-05-18T10:33:39Z
New Revision: 8e648f195c3d57e573fdd8023edcfd80e0516c61

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

LOG: [lldb] Add --all option to "memory region"

This adds an option to the memory region command
to print all regions at once. Like you can do by
starting at address 0 and repeating the command
manually.

memory region [-a] []

(lldb) memory region --all
[0x-0x0040) ---
[0x0040-0x00401000) r-x <...>/a.out PT_LOAD[0]
<...>
[0xfffdf000-0x0001) rw- [stack]
[0x0001-0x) ---

The output matches exactly what you'd get from
repeating the command. Including that it shows
unmapped areas between the mapped regions.

(this is why Process GetMemoryRegions is not
used, that skips unmapped areas)

Help text has been updated to show that you can have
an address or --all but not both.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/Options.td
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
llvm/docs/ReleaseNotes.rst

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectMemory.cpp 
b/lldb/source/Commands/CommandObjectMemory.cpp
index e94306ce33bfe..88fbf05fc057d 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1641,19 +1641,117 @@ class CommandObjectMemoryHistory : public 
CommandObjectParsed {
 // CommandObjectMemoryRegion
 #pragma mark CommandObjectMemoryRegion
 
+#define LLDB_OPTIONS_memory_region
+#include "CommandOptions.inc"
+
 class CommandObjectMemoryRegion : public CommandObjectParsed {
 public:
+  class OptionGroupMemoryRegion : public OptionGroup {
+  public:
+OptionGroupMemoryRegion() : OptionGroup(), m_all(false, false) {}
+
+~OptionGroupMemoryRegion() override = default;
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_memory_region_options);
+}
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+  ExecutionContext *execution_context) override {
+  Status status;
+  const int short_option = 
g_memory_region_options[option_idx].short_option;
+
+  switch (short_option) {
+  case 'a':
+m_all.SetCurrentValue(true);
+m_all.SetOptionWasSet();
+break;
+  default:
+llvm_unreachable("Unimplemented option");
+  }
+
+  return status;
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  m_all.Clear();
+}
+
+OptionValueBoolean m_all;
+  };
+
   CommandObjectMemoryRegion(CommandInterpreter &interpreter)
   : CommandObjectParsed(interpreter, "memory region",
 "Get information on the memory region containing "
 "an address in the current target process.",
-"memory region ADDR",
+"memory region  (or --all)",
 eCommandRequiresProcess | eCommandTryTargetAPILock 
|
-eCommandProcessMustBeLaunched) {}
+eCommandProcessMustBeLaunched) {
+// Address in option set 1.
+m_arguments.push_back(CommandArgumentEntry{CommandArgumentData(
+eArgTypeAddressOrExpression, eArgRepeatPlain, LLDB_OPT_SET_1)});
+// "--all" will go in option set 2.
+m_option_group.Append(&m_memory_region_options);
+m_option_group.Finalize();
+  }
 
   ~CommandObjectMemoryRegion() override = default;
 
+  Options *GetOptions() override { return &m_option_group; }
+
 protected:
+  void DumpRegion(CommandReturnObject &result, Target &target,
+  const MemoryRegionInfo &range_info, lldb::addr_t load_addr) {
+lldb::addr_t resolve_addr = load_addr;
+if (m_memory_region_options.m_all)
+  resolve_addr = range_info.GetRange().GetRangeBase();
+
+lldb_private::Address addr;
+ConstString section_name;
+if (target.ResolveLoadAddress(resolve_addr, addr)) {
+  SectionSP section_sp(addr.GetSection());
+  if (section_sp) {
+// Got the top most section, not the deepest section
+while (section_sp->GetParent())
+  section_sp = section_sp->GetParent();
+section_name = section_sp->GetName();
+  }
+}
+
+ConstString name = range_info.GetName();
+result.AppendMessageWithFormatv(
+"[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}",
+range_info.GetRange().GetRange

[Lldb-commits] [lldb] 3e928c4 - Revert "[lldb] Add --all option to "memory region""

2022-05-18 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2022-05-18T11:57:20Z
New Revision: 3e928c4b9dfb01efd2cb968795e605760828e873

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

LOG: Revert "[lldb] Add --all option to "memory region""

This reverts commit 8e648f195c3d57e573fdd8023edcfd80e0516c61
due to test failures on Windows:
https://lab.llvm.org/buildbot/#/builders/83/builds/19094

Added: 


Modified: 
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/Options.td
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
llvm/docs/ReleaseNotes.rst

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectMemory.cpp 
b/lldb/source/Commands/CommandObjectMemory.cpp
index 88fbf05fc057..e94306ce33bf 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1641,117 +1641,19 @@ class CommandObjectMemoryHistory : public 
CommandObjectParsed {
 // CommandObjectMemoryRegion
 #pragma mark CommandObjectMemoryRegion
 
-#define LLDB_OPTIONS_memory_region
-#include "CommandOptions.inc"
-
 class CommandObjectMemoryRegion : public CommandObjectParsed {
 public:
-  class OptionGroupMemoryRegion : public OptionGroup {
-  public:
-OptionGroupMemoryRegion() : OptionGroup(), m_all(false, false) {}
-
-~OptionGroupMemoryRegion() override = default;
-
-llvm::ArrayRef GetDefinitions() override {
-  return llvm::makeArrayRef(g_memory_region_options);
-}
-
-Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
-  ExecutionContext *execution_context) override {
-  Status status;
-  const int short_option = 
g_memory_region_options[option_idx].short_option;
-
-  switch (short_option) {
-  case 'a':
-m_all.SetCurrentValue(true);
-m_all.SetOptionWasSet();
-break;
-  default:
-llvm_unreachable("Unimplemented option");
-  }
-
-  return status;
-}
-
-void OptionParsingStarting(ExecutionContext *execution_context) override {
-  m_all.Clear();
-}
-
-OptionValueBoolean m_all;
-  };
-
   CommandObjectMemoryRegion(CommandInterpreter &interpreter)
   : CommandObjectParsed(interpreter, "memory region",
 "Get information on the memory region containing "
 "an address in the current target process.",
-"memory region  (or --all)",
+"memory region ADDR",
 eCommandRequiresProcess | eCommandTryTargetAPILock 
|
-eCommandProcessMustBeLaunched) {
-// Address in option set 1.
-m_arguments.push_back(CommandArgumentEntry{CommandArgumentData(
-eArgTypeAddressOrExpression, eArgRepeatPlain, LLDB_OPT_SET_1)});
-// "--all" will go in option set 2.
-m_option_group.Append(&m_memory_region_options);
-m_option_group.Finalize();
-  }
+eCommandProcessMustBeLaunched) {}
 
   ~CommandObjectMemoryRegion() override = default;
 
-  Options *GetOptions() override { return &m_option_group; }
-
 protected:
-  void DumpRegion(CommandReturnObject &result, Target &target,
-  const MemoryRegionInfo &range_info, lldb::addr_t load_addr) {
-lldb::addr_t resolve_addr = load_addr;
-if (m_memory_region_options.m_all)
-  resolve_addr = range_info.GetRange().GetRangeBase();
-
-lldb_private::Address addr;
-ConstString section_name;
-if (target.ResolveLoadAddress(resolve_addr, addr)) {
-  SectionSP section_sp(addr.GetSection());
-  if (section_sp) {
-// Got the top most section, not the deepest section
-while (section_sp->GetParent())
-  section_sp = section_sp->GetParent();
-section_name = section_sp->GetName();
-  }
-}
-
-ConstString name = range_info.GetName();
-result.AppendMessageWithFormatv(
-"[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}",
-range_info.GetRange().GetRangeBase(),
-range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
-range_info.GetWritable(), range_info.GetExecutable(), name ? " " : "",
-name, section_name ? " " : "", section_name);
-MemoryRegionInfo::OptionalBool memory_tagged = 
range_info.GetMemoryTagged();
-if (memory_tagged == MemoryRegionInfo::OptionalBool::eYes)
-  result.AppendMessage("memory tagging: enabled");
-
-const llvm::Optional> &dirty_page_list =
-range_info.GetDirtyPageList();
-if (dirty_page_list.hasValue()) {
-  const size_t page_count = dirty_page_list.getValue().size();
-  result.AppendMessageWithFormat(

[Lldb-commits] [lldb] d9398a9 - [lldb] Remove non-address bits from read/write addresses in lldb

2022-05-18 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2022-05-18T12:59:34+01:00
New Revision: d9398a91e2a6b8837a47a5fda2164c9160e86199

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

LOG: [lldb] Remove non-address bits from read/write addresses in lldb

Non-address bits are not part of the virtual address in a pointer.
So they must be removed before passing to interfaces like ptrace.

Some of them we get way with not removing, like AArch64's top byte.
However this is only because of a hardware feature that ignores them.

This change updates all the Process/Target Read/Write memory methods
to remove non-address bits before using addresses.

Doing it in this way keeps lldb-server simple and also fixes the
memory caching when differently tagged pointers for the same location
are read.

Removing the bits is done at the ReadMemory level not DoReadMemory
because particualrly for process, many subclasses override DoReadMemory.

Tests have been added for read/write at the command and API level,
for process and target. This includes variants like
ReadFromMemory. Commands are tested to make sure we remove
at the command and API level.

"memory find" is not included because:
* There is no API for it.
* It already has its own address handling tests.

Software breakpoints do use these methods but they are not tested
here because there are bigger issues to fix with those. This will
happen in another change.

Reviewed By: omjavaid

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

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

lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Modified: 
lldb/source/Target/Process.cpp
lldb/source/Target/ProcessTrace.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 79adb2c9d155..1f62e95377f4 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1919,6 +1919,9 @@ Status Process::DisableSoftwareBreakpoint(BreakpointSite 
*bp_site) {
 //#define VERIFY_MEMORY_READS
 
 size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) 
{
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixAnyAddress(addr);
+
   error.Clear();
   if (!GetDisableMemoryCache()) {
 #if defined(VERIFY_MEMORY_READS)
@@ -2031,6 +2034,9 @@ size_t Process::ReadMemoryFromInferior(addr_t addr, void 
*buf, size_t size,
Status &error) {
   LLDB_SCOPED_TIMER();
 
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixAnyAddress(addr);
+
   if (buf == nullptr || size == 0)
 return 0;
 
@@ -2113,6 +2119,9 @@ size_t Process::WriteMemoryPrivate(addr_t addr, const 
void *buf, size_t size,
 
 size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
 Status &error) {
+  if (ABISP abi_sp = GetABI())
+addr = abi_sp->FixAnyAddress(addr);
+
 #if defined(ENABLE_MEMORY_CACHING)
   m_memory_cache.Flush(addr, size);
 #endif

diff  --git a/lldb/source/Target/ProcessTrace.cpp 
b/lldb/source/Target/ProcessTrace.cpp
index 41d5b01b61d8..6b147cb285a7 100644
--- a/lldb/source/Target/ProcessTrace.cpp
+++ b/lldb/source/Target/ProcessTrace.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Target.h"
 
@@ -80,6 +81,9 @@ Status ProcessTrace::DoDestroy() { return Status(); }
 
 size_t ProcessTrace::ReadMemory(addr_t addr, void *buf, size_t size,
 Status &error) {
+  if (const ABISP &abi = GetABI())
+addr = abi->FixAnyAddress(addr);
+
   // Don't allow the caching that lldb_private::Process::ReadMemory does since
   // we have it all cached in the trace files.
   return DoReadMemory(addr, buf, size, error);

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ed733f2645c3..993e7efd02ba 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1732,6 +1732,12 @@ size_t Target::ReadMemory(const Address &addr, void 
*dst, size_t dst_len,
   lldb::addr_t *load_addr_ptr) {
   error.Clear();
 
+  Address fixed_addr = addr;
+  if (ProcessIsValid())
+if (const ABISP &abi = m_process_sp->GetABI())
+  fixed_addr.SetLoadAddress(abi->FixAnyAddress(addr.GetLoadAddress(this)),
+this);
+
   // if we end up reading this from process memory, we will fill this with the
   // actual load address
   if (load_addr_ptr)
@@ -1742,26 +1748,28 @@ size_t Target::ReadMemory(

[Lldb-commits] [PATCH] D118794: [lldb] Remove non-address bits from read/write addresses in lldb

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd9398a91e2a6: [lldb] Remove non-address bits from read/write 
addresses in lldb (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

Files:
  lldb/source/Target/Process.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/Makefile
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -0,0 +1,25 @@
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  size_t page_size = sysconf(_SC_PAGESIZE);
+  // Note that we allocate memory here because if we used
+  // stack or globals lldb might read it in the course of
+  // running to the breakpoint. Before the test can look
+  // for those reads.
+  char *buf = mmap(0, page_size, PROT_READ | PROT_WRITE,
+   MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+  if (buf == MAP_FAILED)
+return 1;
+
+#define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
+
+  // Set top byte to something.
+  char *buf_with_non_address = (char *)((size_t)buf | (size_t)0xff << 56);
+  sign_ptr(buf_with_non_address);
+  // Address is now:
+  // <8 bit top byte tag>
+
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -0,0 +1,182 @@
+"""
+Test that lldb removes non-address bits in situations where they would cause
+failures if not removed. Like when reading memory. Tests are done at command
+and API level because commands may remove non-address bits for display
+reasons which can make it seem like the operation as a whole works but at the
+API level it won't if we don't remove them there also.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxNonAddressBitMemoryAccessTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def setup_test(self):
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+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', '// Set break point at this line.'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+def check_cmd_read_write(self, write_to, read_from, data):
+self.runCmd("memory write {} {}".format(write_to, data))
+self.expect("memory read {}".format(read_from),
+substrs=[data])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_non_address_bit_memory_read_write_cmds(self):
+self.setup_test()
+
+# Writes should be visible through either pointer
+self.check_cmd_read_write("buf", "buf", "01 02 03 04")
+self.check_cmd_read_write("buf_with_non_address", "buf_with_non_address", "02 03 04 05")
+self.check_cmd_read_write("buf", "buf_with_non_address", "03 04 05 06")
+self.check_cmd_read_write("buf_with_non_address", "buf", "04 05 06 07")
+
+# Printing either should get the same result
+self.expect("expression -f hex -- *(uint32_t*)buf", substrs=["0x07060504"])
+self.expect("expression -f hex -- *(uint32_t*)buf_with_non_address",
+substrs=["0x07060504"])
+
+def get_ptr_values(self):
+frame  = self.process().GetThreadAtIndex(0).GetFrameAtIndex(0)
+buf = frame.FindVariable("buf").GetValueAsUnsigned()
+buf_with_non_address = frame.FindVariable("buf_with_non_address").GetValueAsUnsigned()
+return buf, buf_with_non_address
+
+def check_api_read_write(self, write_to, read_from, data):
+error = lldb.SBError()
+written = self.process().WriteMemory(write_to, data, error)
+self.assertTrue(error.Success(

[Lldb-commits] [lldb] 00a1258 - [lldb][AArch64] Fix corefile memory reads when there are non-address bits

2022-05-18 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2022-05-18T14:13:42+01:00
New Revision: 00a12585933ef63ff1204bf5cd265f0071d04642

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

LOG: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

Previously if you read a code/data mask before there was a valid thread
you would get the top byte mask. This meant the value was "valid" as in,
don't read it again.

When using a corefile we ask for the data mask very early on and this
meant that later once you did have a thread it wouldn't read the
register to get the rest of the mask.

This fixes that and adds a corefile test generated from the same program
as in my previous change on this theme.

Depends on D118794

Reviewed By: omjavaid

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

Added: 
lldb/test/API/linux/aarch64/non_address_bit_memory_access/corefile

Modified: 
lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Removed: 




diff  --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 99623ce74eff4..2896f5920db90 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -794,14 +794,20 @@ lldb::addr_t ABISysV_arm64::FixAddress(addr_t pc, addr_t 
mask) {
 // Reads code or data address mask for the current Linux process.
 static lldb::addr_t ReadLinuxProcessAddressMask(lldb::ProcessSP process_sp,
 llvm::StringRef reg_name) {
-  // Linux configures user-space virtual addresses with top byte ignored.
-  // We set default value of mask such that top byte is masked out.
-  uint64_t address_mask = ~((1ULL << 56) - 1);
-  // If Pointer Authentication feature is enabled then Linux exposes
-  // PAC data and code mask register. Try reading relevant register
-  // below and merge it with default address mask calculated above.
+  // 0 means there isn't a mask or it has not been read yet.
+  // We do not return the top byte mask unless thread_sp is valid.
+  // This prevents calls to this function before the thread is setup locking
+  // in the value to just the top byte mask, in cases where pointer
+  // authentication might also be active.
+  uint64_t address_mask = 0;
   lldb::ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
   if (thread_sp) {
+// Linux configures user-space virtual addresses with top byte ignored.
+// We set default value of mask such that top byte is masked out.
+address_mask = ~((1ULL << 56) - 1);
+// If Pointer Authentication feature is enabled then Linux exposes
+// PAC data and code mask register. Try reading relevant register
+// below and merge it with default address mask calculated above.
 lldb::RegisterContextSP reg_ctx_sp = thread_sp->GetRegisterContext();
 if (reg_ctx_sp) {
   const RegisterInfo *reg_info =

diff  --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp 
b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
index fe22bcbd75a14..58b4fe3add1b3 100644
--- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Target.h"
@@ -281,6 +282,9 @@ bool ProcessElfCore::IsAlive() { return true; }
 // Process Memory
 size_t ProcessElfCore::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
   Status &error) {
+  if (lldb::ABISP abi_sp = GetABI())
+addr = abi_sp->FixAnyAddress(addr);
+
   // Don't allow the caching that lldb_private::Process::ReadMemory does since
   // in core files we have it all cached our our core file anyway.
   return DoReadMemory(addr, buf, size, error);

diff  --git 
a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
 
b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
index b9b5b54d4a308..d3de12d1b88a0 100644
--- 
a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
+++ 
b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -163,6 +163,10 @@ def test_non_address_bit_memory_caching(self):

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00a12585933e: [lldb][AArch64] Fix corefile memory reads when 
there are non-address bits (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122411

Files:
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  
lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/corefile
  lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c

Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
===
--- lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/main.c
@@ -13,6 +13,13 @@
   if (buf == MAP_FAILED)
 return 1;
 
+  // Some known values to go in the corefile, since we cannot
+  // write to corefile memory.
+  buf[0] = 'L';
+  buf[1] = 'L';
+  buf[2] = 'D';
+  buf[3] = 'B';
+
 #define sign_ptr(ptr) __asm__ __volatile__("pacdza %0" : "=r"(ptr) : "r"(ptr))
 
   // Set top byte to something.
@@ -21,5 +28,11 @@
   // Address is now:
   // <8 bit top byte tag>
 
+  // Uncomment this line to crash and generate a corefile.
+  // Prints so we know what fixed address to look for in testing.
+  // printf("buf: %p\n", buf);
+  // printf("buf_with_non_address: %p\n", buf_with_non_address);
+  // *(char*)0 = 0;
+
   return 0; // Set break point at this line.
 }
Index: lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
===
--- lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
+++ lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -163,6 +163,10 @@
 # Open log ignoring utf-8 decode errors
 with open(log_file, 'r', errors='ignore') as f:
 read_packet = "send packet: $x{:x}"
+
+# Since we allocated a 4k page that page will be aligned to 4k, which
+# also fits the 512 byte line size of the memory cache. So we can expect
+# to find a packet with its exact address.
 read_buf_packet = read_packet.format(buf)
 read_buf_with_non_address_packet = read_packet.format(buf_with_non_address)
 
@@ -180,3 +184,51 @@
 
 if not found_read_buf:
 self.fail("Did not find any reads of buf.")
+
+@skipIfLLVMTargetMissing("AArch64")
+def test_non_address_bit_memory_corefile(self):
+self.runCmd("target create --core corefile")
+
+self.expect("thread list", substrs=['stopped',
+'stop reason = signal SIGSEGV'])
+
+# No caching (the program/corefile are the cache) and no writing
+# to memory. So just check that tagged/untagged addresses read
+# the same location.
+
+# These are known addresses in the corefile, since we won't have symbols.
+buf = 0xa75a5000
+buf_with_non_address = 0xff0ba75a5000
+
+expected = ["4c 4c 44 42", "LLDB"]
+self.expect("memory read 0x{:x}".format(buf), substrs=expected)
+self.expect("memory read 0x{:x}".format(buf_with_non_address), substrs=expected)
+
+# This takes a more direct route to ReadMemory. As opposed to "memory read"
+# above that might fix the addresses up front for display reasons.
+self.expect("expression (char*)0x{:x}".format(buf), substrs=["LLDB"])
+self.expect("expression (char*)0x{:x}".format(buf_with_non_address), substrs=["LLDB"])
+
+def check_reads(addrs, method, num_bytes, expected):
+error = lldb.SBError()
+for addr in addrs:
+if num_bytes is None:
+got = method(addr, error)
+else:
+got = method(addr, num_bytes, error)
+
+self.assertTrue(error.Success())
+self.assertEqual(expected, got)
+
+addr_buf = lldb.SBAddress()
+addr_buf.SetLoadAddress(buf, self.target())
+addr_buf_with_non_address = lldb.SBAddress()
+addr_buf_with_non_address.SetLoadAddress(buf_with_non_address, self.target())
+check_reads([addr_buf, addr_buf_with_non_address], self.target().ReadMemory,
+4, b'LLDB')
+
+addrs = [buf, buf_with_non_address]
+check_reads(addrs, self.process().ReadMemory, 4, b'LLDB')
+check_reads(addrs, self.process().ReadCStringFromMemory, 5, "LLDB")
+check_reads(addrs, self.process().ReadUnsignedFromMemory, 4, 0x42444c4c)
+check_reads(addrs, self.process(

[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

2022-05-18 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits
PatriosTheGreat updated this revision to Diff 430357.
PatriosTheGreat marked 5 inline comments as done.

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

https://reviews.llvm.org/D125325

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/include/lldb/API/SBProcess.h
  lldb/source/API/SBProcess.cpp
  
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Index: lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
===
--- lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -21,6 +21,7 @@
 self.build()
 exe = self.getBuildArtifact("a.out")
 core = self.getBuildArtifact("core.dmp")
+core_sb = self.getBuildArtifact("core_sb.dmp")
 try:
 target = self.dbg.CreateTarget(exe)
 process = target.LaunchSimple(
@@ -43,6 +44,17 @@
 # save core and, kill process and verify corefile existence
 self.runCmd("process save-core --plugin-name=minidump --style=stack " + core)
 self.assertTrue(os.path.isfile(core))
+
+# validate savinig via SBProcess
+error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreStackOnly)
+self.assertTrue(error.Success())
+self.assertTrue(os.path.isfile(core_sb))
+
+error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreFull)
+self.assertTrue(error.Fail())
+error = process.SaveCore(core_sb, "minidump", lldb.eSaveCoreDirtyOnly)
+self.assertTrue(error.Fail())
+
 self.assertSuccess(process.Kill())
 
 # To verify, we'll launch with the mini dump
@@ -77,3 +89,5 @@
 self.assertTrue(self.dbg.DeleteTarget(target))
 if (os.path.isfile(core)):
 os.unlink(core)
+if (os.path.isfile(core_sb)):
+os.unlink(core_sb)
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1138,6 +1138,13 @@
 
 lldb::SBError SBProcess::SaveCore(const char *file_name) {
   LLDB_INSTRUMENT_VA(this, file_name);
+  return SaveCore(file_name, "", SaveCoreStyle::eSaveCoreFull);
+}
+
+lldb::SBError SBProcess::SaveCore(const char *file_name,
+  const char *flavor,
+  SaveCoreStyle core_style) {
+  LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style);
 
   lldb::SBError error;
   ProcessSP process_sp(GetSP());
@@ -1155,8 +1162,9 @@
   }
 
   FileSpec core_file(file_name);
-  SaveCoreStyle core_style = SaveCoreStyle::eSaveCoreFull;
-  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, "");
+  error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style,
+flavor);
+
   return error;
 }
 
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -337,7 +337,21 @@
 
   bool IsInstrumentationRuntimePresent(InstrumentationRuntimeType type);
 
-  /// Save the state of the process in a core file (or mini dump on Windows).
+  /// Save the state of the process in a core file.
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
+  ///
+  /// \param[in] flavor - Specify the flavor of a core file plug-in to save.
+  /// Currently supported flavors include "mach-o" and "minidump"
+  ///
+  /// \param[in] core_style - Specify the style of a core file to save.
+  lldb::SBError SaveCore(const char *file_name, const char *flavor,
+ SaveCoreStyle core_style);
+
+  /// Save the state of the process with the a flavor that matches the
+  /// current process' main executable (if supported).
+  ///
+  /// \param[in] file_name - The name of the file to save the core file to.
   lldb::SBError SaveCore(const char *file_name);
 
   /// Query the address load_addr and store the details of the memory
Index: lldb/bindings/interface/SBProcess.i
===
--- lldb/bindings/interface/SBProcess.i
+++ lldb/bindings/interface/SBProcess.i
@@ -397,6 +397,9 @@
 bool
 IsInstrumentationRuntimePresent(lldb::InstrumentationRuntimeType type);
 
+lldb::SBError
+SaveCore(const char *file_name, const char *flavor, lldb::SaveCoreStyle core_style);
+
 lldb::SBError
 SaveCore(const char *file_name);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

2022-05-18 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits
PatriosTheGreat added a comment.

Hi Greg,
Sorry for a delay had an issue with tests local run.
I fixed the previous feedback.


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

https://reviews.llvm.org/D125325

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


[Lldb-commits] [PATCH] D125089: [lldb] Change implementation of memory read --show-tags option

2022-05-18 Thread David Spickett via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG29e556fc2ba9: [lldb] Change implementation of memory read 
--show-tags option (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125089

Files:
  lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/Options.td
  lldb/source/Interpreter/CMakeLists.txt
  lldb/source/Interpreter/OptionGroupMemoryTag.cpp
  lldb/test/API/commands/help/TestHelp.py
  
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
@@ -411,3 +411,33 @@
 self.expect("memory read mte_buf mte_buf+32 -f \"uint8_t[]\" -s 16 -l 1 --show-tags",
 patterns=["0x[0-9A-Fa-f]+00: \{(0x00 ){15}0x00\} \(tag: 0x0\)\n"
   "0x[0-9A-Fa-f]+10: \{(0x00 ){15}0x00\} \(tag: 0x1\)"])
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+@skipUnlessAArch64MTELinuxCompiler
+def test_mte_memory_read_tag_display_repeated(self):
+"""Test that the --show-tags option is kept when repeating the memory read command."""
+self.setup_mte_test()
+
+self.expect("memory read mte_buf mte_buf+16 -f \"x\" --show-tags",
+patterns=["0x[0-9A-fa-f]+00: 0x0+ 0x0+ 0x0+ 0x0+ \(tag: 0x0\)"])
+# Equivalent to just pressing enter on the command line.
+self.expect("memory read",
+patterns=["0x[0-9A-fa-f]+10: 0x0+ 0x0+ 0x0+ 0x0+ \(tag: 0x1\)"])
+
+# You can add the argument to an existing repetition without resetting
+# the whole command. Though all other optional arguments will reset to
+# their default values when you do this.
+self.expect("memory read mte_buf mte_buf+16 -f \"x\"",
+patterns=["0x[0-9A-fa-f]+00: 0x0+ 0x0+ 0x0+ 0x0+"])
+self.expect("memory read",
+patterns=["0x[0-9A-fa-f]+10: 0x0+ 0x0+ 0x0+ 0x0+"])
+# Note that the formatting returns to default here.
+self.expect("memory read --show-tags",
+patterns=["0x[0-9A-fa-f]+20: (00 )+ \.+ \(tag: 0x2\)"])
+self.expect("memory read",
+patterns=["0x[0-9A-fa-f]+30: (00 )+ \.+ \(tag: 0x3\)"])
+
+# A fresh command reverts to the default of tags being off.
+self.expect("memory read mte_buf mte_buf+16 -f \"x\"",
+patterns=["0x[0-9A-fa-f]+00: 0x0+ 0x0+ 0x0+ 0x0+"])
Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -262,9 +262,9 @@
 """Test that we put a break between the usage and the options help lines,
and between the options themselves."""
 self.expect("help memory read", substrs=[
-"[]\n\n   --show-tags",
-# Starts with the end of the show-tags line
-"output).\n\n   -A"])
+"[]\n\n   -A ( --show-all-children )",
+# Starts with the end of the show-all-children line
+"to show.\n\n   -D"])
 
 @no_debug_info_test
 def test_help_detailed_information_ordering(self):
Index: lldb/source/Interpreter/OptionGroupMemoryTag.cpp
===
--- /dev/null
+++ lldb/source/Interpreter/OptionGroupMemoryTag.cpp
@@ -0,0 +1,59 @@
+//===-- OptionGroupMemoryTag.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Interpreter/OptionGroupMemoryTag.h"
+
+#include "lldb/Host/OptionParser.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+OptionGroupMemoryTag::OptionGroupMemoryTag() : m_show_tags(false, false) {}
+
+static const uint32_t SHORT_OPTION_SHOW_TAGS = 0x54414753; // 'tags'
+
+static constexpr OptionDefinition g_option_table[] = {
+{LLDB_OPT_SET_1,
+ false,
+ "show-tags",
+ SHORT_OPTION_SHOW_TAGS,
+ OptionParser::eNoArgument,
+ nullptr,
+ {},
+ 0,
+ eArgTypeNone,
+ "Include memory tags in output (does not apply to binary output)."},
+};
+
+llvm::ArrayRef OptionGroupMemoryTag::GetDefiniti

[Lldb-commits] [lldb] 29e556f - [lldb] Change implementation of memory read --show-tags option

2022-05-18 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2022-05-18T16:49:09+01:00
New Revision: 29e556fc2ba93028f0dc45c4c2636da6283e9c28

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

LOG: [lldb] Change implementation of memory read --show-tags option

This does 2 things:
* Moves it after the short options. Which makes sense given it's
  a niche, default off option.
  (if 2 files for one option seems a bit much, I am going to reuse
  them for "memory find" later)
* Fixes the use of repeated commands. For example:
memory read buf --show-tags

memory read


Added tests for the repetition and updated existing help tests.

Reviewed By: omjavaid

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

Added: 
lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
lldb/source/Interpreter/OptionGroupMemoryTag.cpp

Modified: 
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/Options.td
lldb/source/Interpreter/CMakeLists.txt
lldb/test/API/commands/help/TestHelp.py

lldb/test/API/linux/aarch64/mte_tag_access/TestAArch64LinuxMTEMemoryTagAccess.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h 
b/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
new file mode 100644
index ..956ec3f07a9b
--- /dev/null
+++ b/lldb/include/lldb/Interpreter/OptionGroupMemoryTag.h
@@ -0,0 +1,40 @@
+//===-- OptionGroupMemoryTag.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_INTERPRETER_OPTIONGROUPMEMORYTAG_H
+#define LLDB_INTERPRETER_OPTIONGROUPMEMORYTAG_H
+
+#include "lldb/Interpreter/OptionValueBoolean.h"
+#include "lldb/Interpreter/Options.h"
+
+namespace lldb_private {
+
+class OptionGroupMemoryTag : public OptionGroup {
+public:
+  OptionGroupMemoryTag();
+
+  ~OptionGroupMemoryTag() override = default;
+
+  llvm::ArrayRef GetDefinitions() override;
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+ExecutionContext *execution_context) override;
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override;
+
+  bool AnyOptionWasSet() const { return m_show_tags.OptionWasSet(); }
+
+  OptionValueBoolean GetShowTags() { return m_show_tags; };
+
+protected:
+  OptionValueBoolean m_show_tags;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_INTERPRETER_OPTIONGROUPMEMORYTAG_H

diff  --git a/lldb/source/Commands/CommandObjectMemory.cpp 
b/lldb/source/Commands/CommandObjectMemory.cpp
index e94306ce33bf..f13f749da2d2 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -16,6 +16,7 @@
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Interpreter/OptionGroupFormat.h"
+#include "lldb/Interpreter/OptionGroupMemoryTag.h"
 #include "lldb/Interpreter/OptionGroupOutputFile.h"
 #include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
 #include "lldb/Interpreter/OptionValueLanguage.h"
@@ -90,10 +91,6 @@ class OptionGroupReadMemory : public OptionGroup {
   error = m_offset.SetValueFromString(option_value);
   break;
 
-case '\x01':
-  m_show_tags = true;
-  break;
-
 default:
   llvm_unreachable("Unimplemented option");
 }
@@ -107,7 +104,6 @@ class OptionGroupReadMemory : public OptionGroup {
 m_force = false;
 m_offset.Clear();
 m_language_for_type.Clear();
-m_show_tags = false;
   }
 
   Status FinalizeSettings(Target *target, OptionGroupFormat &format_options) {
@@ -281,7 +277,6 @@ class OptionGroupReadMemory : public OptionGroup {
   bool m_force;
   OptionValueUInt64 m_offset;
   OptionValueLanguage m_language_for_type;
-  bool m_show_tags = false;
 };
 
 // Read memory from the inferior process
@@ -336,6 +331,8 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
 m_option_group.Append(&m_outfile_options, LLDB_OPT_SET_ALL,
   LLDB_OPT_SET_1 | LLDB_OPT_SET_2 | LLDB_OPT_SET_3);
 m_option_group.Append(&m_varobj_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_3);
+m_option_group.Append(&m_memory_tag_options, LLDB_OPT_SET_ALL,
+  LLDB_OPT_SET_ALL);
 m_option_group.Finalize();
   }
 
@@ -555,11 +552,13 @@ class CommandObjectMemoryRead : public 
CommandObjectParsed {
   if (!m_format_options.AnyOptionWasSet() &&
   !m_memory_options.AnyOptionWasSet() &&
   !m_outfile_options.AnyOptionWasSet() &&
-  !m_varobj

[Lldb-commits] [PATCH] D125897: [trace][intelpt] Support system-wide tracing [9] - Collect and return context switch traces

2022-05-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Add collection of context switches per cpu grouped with the per-cpu intel pt 
traces.
- Move the state handling from the interl pt trace class to the PerfEvent one.
- Add support for stopping and enabling perf event groups.
- Return context switch entries as part of the jLLDBTraceGetState response.
- Move the triggers of whenever the process stopped or resumed. Now the 
will-resume notification is in a better location, which will ensure that we'll 
capture the instructions that will be executed.
- Add unit tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125897

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -1,4 +1,5 @@
 import lldb
+import json
 from intelpt_testcase import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -195,8 +196,40 @@
 self.traceStopThread(error="True",
 substrs=["Can't stop tracing an individual thread when per-core process tracing is enabled"])
 
-# The GetState packet should return trace buffers per core and at least one traced thread
-self.expect("""process plugin packet send 'jLLDBTraceGetState:{"type":"intel-pt"}]'""",
-substrs=['''[{"kind":"traceBuffer","size":4096}],"coreId":''', '"tid":'])
+# We move forward a little bit to collect some data
+self.expect("b 19")
+self.expect("c")
+
+# We will assert that the trace state will contain valid context switch and trace buffer entries
+
+# We first parse the json response from the custom packet
+self.runCmd("""process plugin packet send 'jLLDBTraceGetState:{"type":"intel-pt"}]'""")
+response_header = 'response: '
+output = None
+for line in self.res.GetOutput().splitlines():
+if line.find(response_header) != -1:
+response = line[line.find(response_header) + len(response_header):].strip()
+output = json.loads(response)
+
+self.assertTrue(output is not None)
+self.assertIn("cores", output)
+found_non_empty_context_switch = False
+
+for core in output["cores"]:
+context_switch_size = None
+trace_buffer_size = None
+for binary_data in core["binaryData"]:
+if binary_data["kind"] == "traceBuffer":
+trace_buffer_size = binary_data["size"]
+elif binary_data["kind"] == "perfContextSwitchTrace":
+context_switch_size = binary_data["size"]
+self.assertTrue(context_switch_size is not None)
+self.assertTrue(trace_buffer_size is not None)
+if context_switch_size > 0:
+found_non_empty_context_switch = True
+
+# We must have captured the context switch of when the target resumed
+self.assertTrue(found_non_empty_context_switch)
+
 
 self.traceStopProcess()
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -15,6 +15,8 @@
 
 const char *IntelPTDataKinds::kProcFsCpuInfo = "procfsCpuInfo";
 const char *IntelPTDataKinds::kTraceBuffer = "traceBuffer";
+const char *IntelPTDataKinds::kPerfContextSwitchTrace =
+"perfContextSwitchTrace";
 
 bool TraceIntelPTStartRequest::IsPerCoreTracing() const {
   return per_core_tracing.getValueOr(false);
Index: lldb/s

[Lldb-commits] [PATCH] D125434: Make a more convenient way to allow Darwin users to ignore certain Mach Exceptions

2022-05-18 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

Addressed review comments.




Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:80-111
+// Static Variables
+static uint32_t g_initialize_count = 0;
+
+void PlatformDarwin::Initialize() {
+  Platform::Initialize();
+
+  if (g_initialize_count++ == 0) {

JDevlieghere wrote:
> What's the point of having this? Isn't `PlatformDarwin` an abstract class 
> anyway? 
As it currently stands, you can't create a plugin setting if the plugin doesn't 
have a CreateInstance.  I wanted `settings set 
platform.plugin.darwin.ignored-exceptions`, rather than repeating the setting 
on all the concrete instances of PlatformDarwin, since this is a Darwin 
feature, so I had to add that.



Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:176-177
+  const char *ignored_exceptions = 
GetGlobalProperties().GetIgnoredExceptions();
+  if (!ignored_exceptions || ignored_exceptions[0] == '\0' )
+return {};
+  Args ret_args;

JDevlieghere wrote:
> Unless you expect this string to contain a null byte you could turn this into 
> a StringRef and check `::empty()` instead. 
This won't contain nulls, so that part's okay.  Using a StringRef simplifies 
the check here, but in order to make up the packet I have to call 
ignored_exceptions_as_StringRef.str() in order to have something to pass to 
packet.append.  This isn't performance critical, so I don't mind the copy, but 
since we're going to end up making a std string anyway, I used std::string 
rather than StringRef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125434

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


[Lldb-commits] [PATCH] D125434: Make a more convenient way to allow Darwin users to ignore certain Mach Exceptions

2022-05-18 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 430435.
jingham added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125434

Files:
  lldb/include/lldb/Interpreter/OptionValueString.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/macosx/ignore_exceptions/Makefile
  lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
  lldb/test/API/macosx/ignore_exceptions/main.c
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachException.cpp
  lldb/tools/debugserver/source/MacOSX/MachException.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachTask.h
  lldb/tools/debugserver/source/MacOSX/MachTask.mm
  lldb/tools/debugserver/source/RNBContext.cpp
  lldb/tools/debugserver/source/RNBContext.h
  lldb/tools/debugserver/source/RNBRemote.cpp
  lldb/tools/debugserver/source/RNBRemote.h
  lldb/tools/debugserver/source/debugserver.cpp

Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -368,7 +368,7 @@
   DNBLogThreadedIf(LOG_RNB_MINIMAL, "%s Attaching to pid %i...", __FUNCTION__,
attach_pid);
   char err_str[1024];
-  pid = DNBProcessAttach(attach_pid, NULL, ctx.GetUnmaskSignals(), err_str,
+  pid = DNBProcessAttach(attach_pid, NULL, ctx.GetIgnoredExceptions(), err_str,
  sizeof(err_str));
   g_pid = pid;
 
@@ -1275,7 +1275,7 @@
   break;
 
 case 'U':
-  ctx.SetUnmaskSignals(true);
+  ctx.AddDefaultIgnoredExceptions();
   break;
 
 case '2':
@@ -1574,7 +1574,7 @@
 
 RNBLogSTDOUT("Attaching to process %s...\n", attach_pid_name.c_str());
 nub_process_t pid = DNBProcessAttachByName(
-attach_pid_name.c_str(), timeout_ptr, ctx.GetUnmaskSignals(),
+attach_pid_name.c_str(), timeout_ptr, ctx.GetIgnoredExceptions(),
 err_str, sizeof(err_str));
 g_pid = pid;
 if (pid == INVALID_NUB_PROCESS) {
Index: lldb/tools/debugserver/source/RNBRemote.h
===
--- lldb/tools/debugserver/source/RNBRemote.h
+++ lldb/tools/debugserver/source/RNBRemote.h
@@ -110,6 +110,7 @@
 start_noack_mode,  // 'QStartNoAckMode'
 prefix_reg_packets_with_tid,// 'QPrefixRegisterPacketsWithThreadID
 set_logging_mode,   // 'QSetLogging:'
+set_ignored_exceptions, // 'QSetIgnoredExceptions'   
 set_max_packet_size,// 'QSetMaxPacketSize:'
 set_max_payload_size,   // 'QSetMaxPayloadSize:'
 set_environment_variable,   // 'QEnvironment:'
@@ -197,6 +198,7 @@
   rnb_err_t HandlePacket_QStartNoAckMode(const char *p);
   rnb_err_t HandlePacket_QThreadSuffixSupported(const char *p);
   rnb_err_t HandlePacket_QSetLogging(const char *p);
+  rnb_err_t HandlePacket_QSetIgnoredExceptions(const char *p);
   rnb_err_t HandlePacket_QSetDisableASLR(const char *p);
   rnb_err_t HandlePacket_QSetSTDIO(const char *p);
   rnb_err_t HandlePacket_QSetWorkingDir(const char *p);
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -394,9 +394,12 @@
  "'G', 'p', and 'P') support having the thread ID appended "
  "to the end of the command"));
   t.push_back(Packet(set_logging_mode, &RNBRemote::HandlePacket_QSetLogging,
- NULL, "QSetLogging:", "Check if register packets ('g', "
-   "'G', 'p', and 'P' support having "
-   "the thread ID prefix"));
+ NULL, "QSetLogging:", "Turn on log channels in debugserver"));
+  t.push_back(Packet(set_ignored_exceptions, &RNBRemote::HandlePacket_QSetIgnoredExceptions,
+ NULL, "QSetIgnoredExceptions:", "Set the exception types "
+   "debugserver won't wait for, allowing "
+   "them to be turned into the equivalent "
+   "BSD signals by the normal 

[Lldb-commits] [lldb] bff4673 - Add a darwin platform setting to specify which exceptions debugserver

2022-05-18 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2022-05-18T10:16:11-07:00
New Revision: bff4673b41781ec5bff6b96b52cf321d2271726c

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

LOG: Add a darwin platform setting to specify which exceptions debugserver
should not receive as exceptions (some will get converted to BSD
signals instead).  This is really the only stable way to ensure that
a Mach exception gets converted to it's equivalent BSD signal.  For
programs that rely on BSD signal handlers, this has to happen or you
can't even get the program to invoke the signal handler when under
the debugger.

This builds on a previous solution to this problem which required you
start debugserver with the -U flag.  This was not very discoverable
and required lldb be the one to launch debugserver, which is not always
the case.

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

Added: 
lldb/test/API/macosx/ignore_exceptions/Makefile
lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
lldb/test/API/macosx/ignore_exceptions/main.c

Modified: 
lldb/include/lldb/Interpreter/OptionValueString.h
lldb/include/lldb/Target/Platform.h
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/Platform.cpp
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/MacOSX/MachException.cpp
lldb/tools/debugserver/source/MacOSX/MachException.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/MacOSX/MachTask.h
lldb/tools/debugserver/source/MacOSX/MachTask.mm
lldb/tools/debugserver/source/RNBContext.cpp
lldb/tools/debugserver/source/RNBContext.h
lldb/tools/debugserver/source/RNBRemote.cpp
lldb/tools/debugserver/source/RNBRemote.h
lldb/tools/debugserver/source/debugserver.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/OptionValueString.h 
b/lldb/include/lldb/Interpreter/OptionValueString.h
index be42deb80da77..820656d19e2cb 100644
--- a/lldb/include/lldb/Interpreter/OptionValueString.h
+++ b/lldb/include/lldb/Interpreter/OptionValueString.h
@@ -109,6 +109,11 @@ class OptionValueString : public 
Cloneable {
   bool IsCurrentValueEmpty() const { return m_current_value.empty(); }
 
   bool IsDefaultValueEmpty() const { return m_default_value.empty(); }
+  
+  void SetValidator(ValidatorCallback validator, void *baton = nullptr) {
+m_validator = validator;
+m_validator_baton = baton;
+  }
 
 protected:
   std::string m_current_value;

diff  --git a/lldb/include/lldb/Target/Platform.h 
b/lldb/include/lldb/Target/Platform.h
index 8b8ca6268071f..603200820478c 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -847,6 +847,8 @@ class Platform : public PluginInterface {
   }
 
   virtual CompilerType GetSiginfoType(const llvm::Triple &triple);
+  
+  virtual Args GetExtraStartupCommands();
 
 protected:
   /// Create a list of ArchSpecs with the given OS and a architectures. The

diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 459adae044526..f10bfb14e2b55 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -174,6 +174,7 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_QMemTags, // write memory tags
 
 eServerPacketType_qLLDBSaveCore,
+eServerPacketType_QSetIgnoredExceptions
   };
 
   ServerPacketType GetServerPacketType() const;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index cac9c672afb4c..455b760cb27a9 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -19,11 +19,15 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/XML.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/OptionValueProperties.h"
+#include "lldb/Interpreter/OptionValueString.h"
+#include "lldb/Interpreter/Options.h"
 #include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h

[Lldb-commits] [PATCH] D125434: Make a more convenient way to allow Darwin users to ignore certain Mach Exceptions

2022-05-18 Thread Jim Ingham 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 rGbff4673b4178: Add a darwin platform setting to specify which 
exceptions debugserver (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125434

Files:
  lldb/include/lldb/Interpreter/OptionValueString.h
  lldb/include/lldb/Target/Platform.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/macosx/ignore_exceptions/Makefile
  lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
  lldb/test/API/macosx/ignore_exceptions/main.c
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachException.cpp
  lldb/tools/debugserver/source/MacOSX/MachException.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachTask.h
  lldb/tools/debugserver/source/MacOSX/MachTask.mm
  lldb/tools/debugserver/source/RNBContext.cpp
  lldb/tools/debugserver/source/RNBContext.h
  lldb/tools/debugserver/source/RNBRemote.cpp
  lldb/tools/debugserver/source/RNBRemote.h
  lldb/tools/debugserver/source/debugserver.cpp

Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -368,7 +368,7 @@
   DNBLogThreadedIf(LOG_RNB_MINIMAL, "%s Attaching to pid %i...", __FUNCTION__,
attach_pid);
   char err_str[1024];
-  pid = DNBProcessAttach(attach_pid, NULL, ctx.GetUnmaskSignals(), err_str,
+  pid = DNBProcessAttach(attach_pid, NULL, ctx.GetIgnoredExceptions(), err_str,
  sizeof(err_str));
   g_pid = pid;
 
@@ -1275,7 +1275,7 @@
   break;
 
 case 'U':
-  ctx.SetUnmaskSignals(true);
+  ctx.AddDefaultIgnoredExceptions();
   break;
 
 case '2':
@@ -1574,7 +1574,7 @@
 
 RNBLogSTDOUT("Attaching to process %s...\n", attach_pid_name.c_str());
 nub_process_t pid = DNBProcessAttachByName(
-attach_pid_name.c_str(), timeout_ptr, ctx.GetUnmaskSignals(),
+attach_pid_name.c_str(), timeout_ptr, ctx.GetIgnoredExceptions(),
 err_str, sizeof(err_str));
 g_pid = pid;
 if (pid == INVALID_NUB_PROCESS) {
Index: lldb/tools/debugserver/source/RNBRemote.h
===
--- lldb/tools/debugserver/source/RNBRemote.h
+++ lldb/tools/debugserver/source/RNBRemote.h
@@ -110,6 +110,7 @@
 start_noack_mode,  // 'QStartNoAckMode'
 prefix_reg_packets_with_tid,// 'QPrefixRegisterPacketsWithThreadID
 set_logging_mode,   // 'QSetLogging:'
+set_ignored_exceptions, // 'QSetIgnoredExceptions'   
 set_max_packet_size,// 'QSetMaxPacketSize:'
 set_max_payload_size,   // 'QSetMaxPayloadSize:'
 set_environment_variable,   // 'QEnvironment:'
@@ -197,6 +198,7 @@
   rnb_err_t HandlePacket_QStartNoAckMode(const char *p);
   rnb_err_t HandlePacket_QThreadSuffixSupported(const char *p);
   rnb_err_t HandlePacket_QSetLogging(const char *p);
+  rnb_err_t HandlePacket_QSetIgnoredExceptions(const char *p);
   rnb_err_t HandlePacket_QSetDisableASLR(const char *p);
   rnb_err_t HandlePacket_QSetSTDIO(const char *p);
   rnb_err_t HandlePacket_QSetWorkingDir(const char *p);
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -394,9 +394,12 @@
  "'G', 'p', and 'P') support having the thread ID appended "
  "to the end of the command"));
   t.push_back(Packet(set_logging_mode, &RNBRemote::HandlePacket_QSetLogging,
- NULL, "QSetLogging:", "Check if register packets ('g', "
-   "'G', 'p', and 'P' support having "
-   "the thread ID prefix"));
+ NULL, "QSetLogging:", "Turn on log channels in debugserver"));
+  t.push_back(Packet(set_ignored_exceptions, &RNBRemote::HandlePacket_QSetIgnoredExceptions,
+ NULL, "QSetIgnoredExceptions:", "Set the exception types "
+   "debugserver won't wait for, allowin

[Lldb-commits] [PATCH] D125915: [lldb/Test] Add `use_colors` argument to PExpect.launch wrapper

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch adds a new `use_colors` argument to the PExpect.launch
method.

As the name suggests, it allows the user to conditionally enable color
support in the debugger, which can be helpful to test functionalities that
rely on that, like progress reporting. It defaults to False.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125915

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


Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -24,16 +24,20 @@
 self.child.expect_exact(self.PROMPT)
 
 def launch(self, executable=None, extra_args=None, timeout=60,
-   dimensions=None, run_under=None, post_spawn=None):
+   dimensions=None, run_under=None, post_spawn=None,
+   use_colors=False):
 logfile = getattr(sys.stdout, 'buffer',
 sys.stdout) if self.TraceOn() else None
 
 args = []
 if run_under is not None:
 args += run_under
-args += [lldbtest_config.lldbExec, '--no-lldbinit', '--no-use-colors']
+args += [lldbtest_config.lldbExec, '--no-lldbinit']
+if not use_colors:
+args += '--no-use-colors'
 for cmd in self.setUpCommands():
-args += ['-O', cmd]
+if use_colors and "use-color false" not in cmd:
+args += ['-O', cmd]
 if executable is not None:
 args += ['--file', executable]
 if extra_args is not None:
@@ -54,8 +58,9 @@
 post_spawn()
 self.expect_prompt()
 for cmd in self.setUpCommands():
-self.child.expect_exact(cmd)
-self.expect_prompt()
+if use_colors and "use-color false" not in cmd:
+self.child.expect_exact(cmd)
+self.expect_prompt()
 if executable is not None:
 self.child.expect_exact("target create")
 self.child.expect_exact("Current executable set to")


Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -24,16 +24,20 @@
 self.child.expect_exact(self.PROMPT)
 
 def launch(self, executable=None, extra_args=None, timeout=60,
-   dimensions=None, run_under=None, post_spawn=None):
+   dimensions=None, run_under=None, post_spawn=None,
+   use_colors=False):
 logfile = getattr(sys.stdout, 'buffer',
 sys.stdout) if self.TraceOn() else None
 
 args = []
 if run_under is not None:
 args += run_under
-args += [lldbtest_config.lldbExec, '--no-lldbinit', '--no-use-colors']
+args += [lldbtest_config.lldbExec, '--no-lldbinit']
+if not use_colors:
+args += '--no-use-colors'
 for cmd in self.setUpCommands():
-args += ['-O', cmd]
+if use_colors and "use-color false" not in cmd:
+args += ['-O', cmd]
 if executable is not None:
 args += ['--file', executable]
 if extra_args is not None:
@@ -54,8 +58,9 @@
 post_spawn()
 self.expect_prompt()
 for cmd in self.setUpCommands():
-self.child.expect_exact(cmd)
-self.expect_prompt()
+if use_colors and "use-color false" not in cmd:
+self.child.expect_exact(cmd)
+self.expect_prompt()
 if executable is not None:
 self.child.expect_exact("target create")
 self.child.expect_exact("Current executable set to")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 430465.
mib added a comment.

Add test


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

https://reviews.llvm.org/D124785

Files:
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Index: lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
@@ -0,0 +1,60 @@
+"""
+Test trimming long progress report in tiny terminal windows
+"""
+
+import os
+import pexpect
+import tempfile
+import random
+import re
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestTrimmedProgressReporting(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfEditlineSupportMissing
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
+def test_long_progress_message(self):
+self.build()
+# Start with a small window
+self.launch(use_colors=True)
+# Set the terminal to a random width
+term_width = random.randint(10, 42)
+self.expect("set set show-progress true")
+self.expect("set show show-progress", substrs=["show-progress (boolean) = true"])
+self.expect("set set term-width " + str(term_width))
+self.expect("set show term-width", substrs=["term-width (int) = " + str(term_width)])
+
+lines = []
+with tempfile.NamedTemporaryFile() as tmpfile:
+if self.TraceOn():
+print("logfile: " + tmpfile.name)
+with open(tmpfile.name, 'wb') as logfile:
+self.child.logfile = logfile
+self.expect("file " + self.getBuildArtifact("a.out"),
+substrs=["Current executable set to"])
+
+with open(tmpfile.name) as logfile:
+lines = list(filter(lambda line : line != '\n',
+logfile.readlines()))
+
+self.assertGreater(len(lines), 0)
+
+ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])')
+
+# Skip the first `file a.out` lines
+for idx, line in enumerate(lines[2:-2]):
+# Remove ASNI sequences and trailing new line character
+message = ansi_escape.sub('', line)[:-1]
+if len(message) == 0:
+continue
+self.assertLessEqual(len(message), term_width, str(idx) + " " + message)
+
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1858,11 +1858,26 @@
 output->Printf(
 "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
+  // Trim the progress message if it exceeds the window's width and print it.
   std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
+  uint64_t progress_total = data->GetTotal();
+  uint32_t term_width = GetTerminalWidth();
+
+  size_t prefix_width = 0;
+  if (data->IsFinite()) {
+prefix_width += 4; // '[%PRIu64/%PRIu64] %s'
+prefix_width += std::to_string(progress_total).size() * 2;
+  }
+
+  const size_t suffix_width = 3; // %s...
+
+  if (message.size() + prefix_width + suffix_width >= term_width)
+message.erase(message.begin() + term_width - prefix_width - suffix_width,
+  message.end());
+
+  if (data->IsFinite()) {
 output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-   data->GetTotal(), message.c_str());
+   progress_total, message.c_str());
   } else {
 output->Printf("%s...", message.c_str());
   }
Index: lldb/include/lldb/Core/DebuggerEvents.h
===
--- lldb/include/lldb/Core/DebuggerEvents.h
+++ lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@
 
   static const ProgressEventData *GetEventDataFromEvent(const Event *event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string &GetMessage() const { return m_message; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D125915: [lldb/Test] Add `use_colors` argument to PExpect.launch wrapper

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125915

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


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: 
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py:29-30
+self.launch(use_colors=True)
+# Set the terminal to a random width
+term_width = random.randint(10, 42)
+self.expect("set set show-progress true")

I understand the motivation, but I think it's a bad idea to intentionally 
introduce non-determinism in a test. Let's just make this a constant or repeat 
the test for two (fixed) widths.



Comment at: 
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py:36-59
+lines = []
+with tempfile.NamedTemporaryFile() as tmpfile:
+if self.TraceOn():
+print("logfile: " + tmpfile.name)
+with open(tmpfile.name, 'wb') as logfile:
+self.child.logfile = logfile
+self.expect("file " + self.getBuildArtifact("a.out"),

Why not use `self.child.expect_exact` with the exact sequence (including escape 
characters) that you're trying to match? This seems to reimplementing part of 
the pexpect functionality and I'm not sure we actually need it.


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

https://reviews.llvm.org/D124785

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


[Lldb-commits] [PATCH] D124731: [lldb] Consider binary as module of last resort

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Still a few places where we have a dedicated assert




Comment at: 
lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:25
+debugger.HandleCommand(f"break set -a {main_address:#x}")
+self.assertTrue(target.GetNumBreakpoints() == 1)
+





Comment at: 
lldb/test/API/commands/breakpoint/set/address-nomodule/TestBreakpointAddressNoModule.py:28
+bp = target.GetBreakpointAtIndex(0)
+self.assertTrue(bp != None)
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124731

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


[Lldb-commits] [PATCH] D125897: [trace][intelpt] Support system-wide tracing [9] - Collect and return context switch traces

2022-05-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 430490.
wallace edited the summary of this revision.
wallace added a comment.

nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125897

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.cpp
  lldb/source/Plugins/Process/Linux/IntelPTThreadTraceCollection.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/Linux/Perf.h
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -1,4 +1,5 @@
 import lldb
+import json
 from intelpt_testcase import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
@@ -195,8 +196,40 @@
 self.traceStopThread(error="True",
 substrs=["Can't stop tracing an individual thread when per-core process tracing is enabled"])
 
-# The GetState packet should return trace buffers per core and at least one traced thread
-self.expect("""process plugin packet send 'jLLDBTraceGetState:{"type":"intel-pt"}]'""",
-substrs=['''[{"kind":"traceBuffer","size":4096}],"coreId":''', '"tid":'])
+# We move forward a little bit to collect some data
+self.expect("b 19")
+self.expect("c")
+
+# We will assert that the trace state will contain valid context switch and trace buffer entries
+
+# We first parse the json response from the custom packet
+self.runCmd("""process plugin packet send 'jLLDBTraceGetState:{"type":"intel-pt"}]'""")
+response_header = 'response: '
+output = None
+for line in self.res.GetOutput().splitlines():
+if line.find(response_header) != -1:
+response = line[line.find(response_header) + len(response_header):].strip()
+output = json.loads(response)
+
+self.assertTrue(output is not None)
+self.assertIn("cores", output)
+found_non_empty_context_switch = False
+
+for core in output["cores"]:
+context_switch_size = None
+trace_buffer_size = None
+for binary_data in core["binaryData"]:
+if binary_data["kind"] == "traceBuffer":
+trace_buffer_size = binary_data["size"]
+elif binary_data["kind"] == "perfContextSwitchTrace":
+context_switch_size = binary_data["size"]
+self.assertTrue(context_switch_size is not None)
+self.assertTrue(trace_buffer_size is not None)
+if context_switch_size > 0:
+found_non_empty_context_switch = True
+
+# We must have captured the context switch of when the target resumed
+self.assertTrue(found_non_empty_context_switch)
+
 
 self.traceStopProcess()
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -15,6 +15,8 @@
 
 const char *IntelPTDataKinds::kProcFsCpuInfo = "procfsCpuInfo";
 const char *IntelPTDataKinds::kTraceBuffer = "traceBuffer";
+const char *IntelPTDataKinds::kPerfContextSwitchTrace =
+"perfContextSwitchTrace";
 
 bool TraceIntelPTStartRequest::IsPerCoreTracing() const {
   return per_core_tracing.getValueOr(false);
Index: lldb/source/Plugins/Process/Linux/Perf.h
===
--- lldb/source/Plugins/Process/Linux/Perf.h
+++ lldb/source/Plugins/Process/Linux/Perf.h
@@ -98,6 +98,11 @@
 /// Handles the management of the event's file descriptor and mmap'ed
 /// regions.
 class PerfEvent {
+  enum class CollectionState {
+Enabled,
+Disabled,
+  };
+
 public:
   /// Create a new performance monitoring event 

[Lldb-commits] [PATCH] D125928: [lldb/crashlog] Fix line entries resolution in interactive mode

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch substracts 1 to the pc of any frame about frame 0 to get the
previous line entry and display the right line in the debugger.

This also rephrase some old comment from `48d157dd4`.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125928

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -16,6 +16,7 @@
 return
 
 self.pid = crash_log.process_id
+self.addr_mask = crash_log.addr_mask
 self.crashed_thread_idx = crash_log.crashed_thread_idx
 self.loaded_images = []
 
@@ -122,11 +123,13 @@
 return None
 
 for frame in self.backing_thread.frames:
+frame_pc = frame.pc & self.scripted_process.addr_mask
+pc = frame_pc if frame.index == 0  or frame_pc == 0 else frame_pc 
- 1
 sym_addr = lldb.SBAddress()
-sym_addr.SetLoadAddress(frame.pc, self.target)
+sym_addr.SetLoadAddress(pc, self.target)
 if not sym_addr.IsValid():
 continue
-self.frames.append({"idx": frame.index, "pc": frame.pc})
+self.frames.append({"idx": frame.index, "pc": pc})
 
 return self.frames
 
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -115,8 +115,8 @@
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) 
and frame_idx < options.disassemble_depth
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# We should substract 1 to every frame's pc above frame zero
+# to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask
 pc = pc if frame_idx == 0 or pc == 0 else pc - 1
 symbolicated_frame_addresses = crash_log.symbolicate(pc, 
options.verbose)


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -16,6 +16,7 @@
 return
 
 self.pid = crash_log.process_id
+self.addr_mask = crash_log.addr_mask
 self.crashed_thread_idx = crash_log.crashed_thread_idx
 self.loaded_images = []
 
@@ -122,11 +123,13 @@
 return None
 
 for frame in self.backing_thread.frames:
+frame_pc = frame.pc & self.scripted_process.addr_mask
+pc = frame_pc if frame.index == 0  or frame_pc == 0 else frame_pc - 1
 sym_addr = lldb.SBAddress()
-sym_addr.SetLoadAddress(frame.pc, self.target)
+sym_addr.SetLoadAddress(pc, self.target)
 if not sym_addr.IsValid():
 continue
-self.frames.append({"idx": frame.index, "pc": frame.pc})
+self.frames.append({"idx": frame.index, "pc": pc})
 
 return self.frames
 
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -115,8 +115,8 @@
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) and frame_idx < options.disassemble_depth
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# We should substract 1 to every frame's pc above frame zero
+# to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask
 pc = pc if frame_idx == 0 or pc == 0 else pc - 1
 symbolicated_frame_addresses = crash_log.symbolicate(pc, options.verbose)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

2022-05-18 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 430502.
zequanwu added a comment.

Add missing sort.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

Files:
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Expression/DWARFExpressionList.h
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/include/lldb/Target/StackFrame.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Core/ValueObjectVariable.cpp
  lldb/source/Expression/CMakeLists.txt
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Expression/DWARFExpressionList.cpp
  lldb/source/Expression/Materializer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/NativePDB/DWARFLocationExpression.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/source/Target/StackFrame.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
  llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/source/Expression/BUILD.gn
@@ -23,6 +23,7 @@
   include_dirs = [ ".." ]
   sources = [
 "DWARFExpression.cpp",
+"DWARFExpressionList.cpp",
 "DiagnosticManager.cpp",
 "Expression.cpp",
 "ExpressionVariable.cpp",
Index: llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
===
--- llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
+++ llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
@@ -177,8 +177,8 @@
   return visitLocationList(&Offset, [&](const DWARFLocationEntry &E) {
 Expected> Loc = Interp.Interpret(E);
 if (!Loc)
-  return Callback(Loc.takeError());
-if (*Loc)
+  consumeError(Loc.takeError());
+else if (*Loc)
   return Callback(**Loc);
 return true;
   });
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -19,29 +19,29 @@
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x0
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F0
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x-0x0001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0001, 0x0001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x, 0x0001): DW_OP_reg0 RAX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0001}, language = "", file = '1.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x2
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F1
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0001-0x0002)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0003, 0x0001): DW_OP_reg1 RDX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0001, 0x0002): DW_OP_reg1 RDX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0002}, language = "", file = '2.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x4
 # SYMBOLS-NEXT:   Function{{.*}}, demangled = F2
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x0002-0x0003)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0005, 0x0001): DW_OP_reg2 RCX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x0002, 0x0003): DW_OP_reg2 RCX
 # SYMBOLS-EMPTY:
 # SYMBOLS-NEXT: CompileUnit{0x0003}, language = "", file = '3.c'
 # SYMBOLS-NEXT:   Variable{{.*}}, name = "A", {{.*}}, location = DW_OP_GNU_addr_index 0x6
 # SYMBOLS-NEXT:   Function{{.*}}, demangled

[Lldb-commits] [PATCH] D125928: [lldb/crashlog] Fix line entries resolution in interactive mode

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: lldb/examples/python/crashlog.py:118-119
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# We should substract 1 to every frame's pc above frame zero
+# to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask

Maybe 

>  Except for the zeroth frame, we should subtract 1 from every frame pc in 
> order to get to the previous line entry



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125928

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


[Lldb-commits] [PATCH] D125325: Pass plugin_name in SBProcess::SaveCore

2022-05-18 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.

Thanks for the changes. LGTM


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

https://reviews.llvm.org/D125325

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


[Lldb-commits] [PATCH] D125928: [lldb/crashlog] Fix line entries resolution in interactive mode

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 430505.
mib marked an inline comment as done.
mib edited the summary of this revision.
mib added a comment.

Updated comment


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

https://reviews.llvm.org/D125928

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -16,6 +16,7 @@
 return
 
 self.pid = crash_log.process_id
+self.addr_mask = crash_log.addr_mask
 self.crashed_thread_idx = crash_log.crashed_thread_idx
 self.loaded_images = []
 
@@ -122,11 +123,13 @@
 return None
 
 for frame in self.backing_thread.frames:
+frame_pc = frame.pc & self.scripted_process.addr_mask
+pc = frame_pc if frame.index == 0  or frame_pc == 0 else frame_pc 
- 1
 sym_addr = lldb.SBAddress()
-sym_addr.SetLoadAddress(frame.pc, self.target)
+sym_addr.SetLoadAddress(pc, self.target)
 if not sym_addr.IsValid():
 continue
-self.frames.append({"idx": frame.index, "pc": frame.pc})
+self.frames.append({"idx": frame.index, "pc": pc})
 
 return self.frames
 
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -115,8 +115,8 @@
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) 
and frame_idx < options.disassemble_depth
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# Except for the zeroth frame, we should subtract 1 from every
+# frame pc to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask
 pc = pc if frame_idx == 0 or pc == 0 else pc - 1
 symbolicated_frame_addresses = crash_log.symbolicate(pc, 
options.verbose)


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -16,6 +16,7 @@
 return
 
 self.pid = crash_log.process_id
+self.addr_mask = crash_log.addr_mask
 self.crashed_thread_idx = crash_log.crashed_thread_idx
 self.loaded_images = []
 
@@ -122,11 +123,13 @@
 return None
 
 for frame in self.backing_thread.frames:
+frame_pc = frame.pc & self.scripted_process.addr_mask
+pc = frame_pc if frame.index == 0  or frame_pc == 0 else frame_pc - 1
 sym_addr = lldb.SBAddress()
-sym_addr.SetLoadAddress(frame.pc, self.target)
+sym_addr.SetLoadAddress(pc, self.target)
 if not sym_addr.IsValid():
 continue
-self.frames.append({"idx": frame.index, "pc": frame.pc})
+self.frames.append({"idx": frame.index, "pc": pc})
 
 return self.frames
 
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -115,8 +115,8 @@
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) and frame_idx < options.disassemble_depth
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# Except for the zeroth frame, we should subtract 1 from every
+# frame pc to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask
 pc = pc if frame_idx == 0 or pc == 0 else pc - 1
 symbolicated_frame_addresses = crash_log.symbolicate(pc, options.verbose)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D125928: [lldb/crashlog] Fix line entries resolution in interactive mode

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D125928

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


[Lldb-commits] [PATCH] D125932: [trace][intelpt] Support system-wide tracing [10] - Return warnings and tsc information from lldb-server.

2022-05-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Add a warnings field in the jLLDBGetState response, for warnings to be 
delivered to the client for troubleshooting. This removes the need to silently 
log lldb-server's llvm::Errors and not expose them easily to the user
- Simplify the tscPerfZeroConversion struct and schema. It used to extend a 
base abstract class, but I'm doubting that we'll ever add other conversion 
mechanisms because all modern kernels support perf zero. It is also the one who 
is supposed to work with the timestamps produced by the context switch trace, 
so expecting it is imperative.
- Force tsc collection for cpu tracing.
- Add a test checking that tscPerfZeroConversion is returned by the GetState 
request
- Add a pre-check for cpu tracing that makes sure that perf zero values are 
available.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125932

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTCollector.h
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.cpp
  lldb/source/Plugins/Process/Linux/IntelPTPerThreadProcessTrace.h
  lldb/source/Plugins/Process/Linux/IntelPTProcessTrace.h
  lldb/source/Utility/TraceGDBRemotePackets.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
  lldb/unittests/Process/Linux/PerfTests.cpp

Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -77,7 +77,7 @@
 GTEST_SKIP() << toString(tsc_after_sleep.takeError());
 
   std::chrono::nanoseconds converted_tsc_diff =
-  params->Convert(*tsc_after_sleep) - params->Convert(*tsc_before_sleep);
+  params->ToNanos(*tsc_after_sleep) - params->ToNanos(*tsc_before_sleep);
 
   std::chrono::microseconds acceptable_overhead(500);
 
Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -200,7 +200,8 @@
 self.expect("b 19")
 self.expect("c")
 
-# We will assert that the trace state will contain valid context switch and trace buffer entries
+# We will assert that the trace state will contain valid context switch and trace buffer entries.
+# Besides that, we need to get tsc-to-nanos conversion information.
 
 # We first parse the json response from the custom packet
 self.runCmd("""process plugin packet send 'jLLDBTraceGetState:{"type":"intel-pt"}]'""")
@@ -213,6 +214,7 @@
 
 self.assertTrue(output is not None)
 self.assertIn("cores", output)
+self.assertIn("tscPerfZeroConversion", output)
 found_non_empty_context_switch = False
 
 for core in output["cores"]:
Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -50,67 +50,47 @@
   return base;
 }
 
-std::chrono::nanoseconds
-LinuxPerfZeroTscConversion::Convert(uint64_t raw_counter_value) {
-  uint64_t quot = raw_counter_value >> m_time_shift;
-  uint64_t rem_flag = (((uint64_t)1 << m_time_shift) - 1);
-  uint64_t rem = raw_counter_value & rem_flag;
-  return std::chrono::nanoseconds{m_time_zero + quot * m_time_mult +
-  ((rem * m_time_mult) >> m_time_shift)};
+std::chrono::nanoseconds LinuxPerfZeroTscConversion::ToNanos(uint64_t tsc) {
+  uint64_t quot = tsc >> time_shift;
+  uint64_t rem_flag = (((uint64_t)1 << time_shift) - 1);
+  uint64_t rem = tsc & rem_flag;
+  return std::chrono::nanoseconds{time_zero + quot * time_mult +
+  ((rem * time_mult) >> time_shift)};
 }
 
-json::Value LinuxPerfZeroTscConversion::toJSON() {
+json::Value toJSON(const LinuxPerfZeroTscConversion &packet) {
   return json::Value(json::Object{
-  {"kind", "tsc-perf-zero-conversion"},
-  {"time_mult", m_time_mult},
-  {"time_shift", m_time_shift},
-  {"time_zero", m_time_zero},
+  {"kind", "tscPerfZeroConversion"},
+  {"timeMult", packet.time_mult},
+  {"timeShift", packet

[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Separating the notion of an expression and an expression list seems worthwhile. 
I did a very high level pass but I'll take a more detailed look tomorrow.




Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:1-2
+//===-- DWARFExpressionList.h ---*- C++
+//-*-===//
+//

Formatting



Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:22
+
+// It's the interface to get or evaluate a DWARFExpression.
+class DWARFExpressionList {

This should be a Doxygen comment. But more importantly, this doesn't really 
tell me what the class does. DWARFExpressionList implies it holds a list of 
expressions so that seems useful to include there.



Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:24
+class DWARFExpressionList {
+private:
+  // RangeDataVector requires a comparator for DWARFExpression, but it doesn't

I think we generally put public first, followed by protected and private last. 



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s:22-23
 # SYMBOLS-NEXT:   Block{{.*}}, ranges = [0x-0x0001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location = 
-# SYMBOLS-NEXT:   DW_LLE_startx_length   (0x0001, 
0x0001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT:   [0x, 0x0001): DW_OP_reg0 RAX
 # SYMBOLS-EMPTY:

I guess the patch is not NFC is the output changes? Would it be possible to 
split the functional and non-functional part of this patch into separate 
patches?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125509

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


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 430548.
mib added a comment.

Address @JDevlieghere comments


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

https://reviews.llvm.org/D124785

Files:
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Index: lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
@@ -0,0 +1,50 @@
+"""
+Test trimming long progress report in tiny terminal windows
+"""
+
+import os
+import pexpect
+import tempfile
+import re
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestTrimmedProgressReporting(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def do_test(self, term_width, pattern_list):
+self.build()
+# Start with a small window
+self.launch(use_colors=True)
+self.expect("set set show-progress true")
+self.expect("set show show-progress", substrs=["show-progress (boolean) = true"])
+self.expect("set set term-width " + str(term_width))
+self.expect("set show term-width", substrs=["term-width (int) = " + str(term_width)])
+
+self.child.send("file " + self.getBuildArtifact("a.out") + "\n")
+self.child.expect(pattern_list)
+
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfEditlineSupportMissing
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
+def test_trimmed_progress_message(self):
+self.do_test(19, ['Locating externa...',
+  'Loading Apple DW...',
+  'Parsing symbol t...'])
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfEditlineSupportMissing
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
+def test_long_progress_message(self):
+self.do_test(80, ['Locating external symbol file for a.out...',
+  'Loading Apple DWARF index for a.out...',
+  'Parsing symbol table for dyld...'])
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1852,20 +1852,26 @@
 return;
   }
 
+  // Trim the progress message if it exceeds the window's width and print it.
+  std::string message = data->GetMessage();
+  if (data->IsFinite())
+message = llvm::formatv("[{0}/{1}] {2}", data->GetCompleted(),
+data->GetTotal(), message)
+  .str();
+
+  // Trim the progress message if it exceeds the window's width and print it.
+  const uint32_t term_width = GetTerminalWidth();
+  const uint32_t ellipsis = 3;
+  if (message.size() + ellipsis >= term_width)
+message = message.substr(0, term_width - ellipsis);
+
   const bool use_color = GetUseColor();
   llvm::StringRef ansi_prefix = GetShowProgressAnsiPrefix();
   if (!ansi_prefix.empty())
 output->Printf(
 "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
-  std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
-output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-   data->GetTotal(), message.c_str());
-  } else {
-output->Printf("%s...", message.c_str());
-  }
+  output->Printf("%s...", message.c_str());
 
   llvm::StringRef ansi_suffix = GetShowProgressAnsiSuffix();
   if (!ansi_suffix.empty())
Index: lldb/include/lldb/Core/DebuggerEvents.h
===
--- lldb/include/lldb/Core/DebuggerEvents.h
+++ lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@
 
   static const ProgressEventData *GetEventDataFromEvent(const Event *event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string &GetMessage() const { return m_message; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: 
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py:49-50
+self.do_test(80, ['Locating external symbol file for a.out...',
+  'Loading Apple DWARF index for a.out...',
+  'Parsing symbol table for dyld...'])

These two things are pretty Apple specific so maybe use the `skipUnlessDarwin` 
decorator? I think the locating symbol files will be printed on all platforms, 
but if we need to skip the test on Linux anyway that doesn't really buy us much 
to reduce the test coverage.


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

https://reviews.llvm.org/D124785

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


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM with the decorator


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

https://reviews.llvm.org/D124785

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


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib marked an inline comment as done.
mib added inline comments.



Comment at: 
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py:49-50
+self.do_test(80, ['Locating external symbol file for a.out...',
+  'Loading Apple DWARF index for a.out...',
+  'Parsing symbol table for dyld...'])

JDevlieghere wrote:
> These two things are pretty Apple specific so maybe use the 
> `skipUnlessDarwin` decorator? I think the locating symbol files will be 
> printed on all platforms, but if we need to skip the test on Linux anyway 
> that doesn't really buy us much to reduce the test coverage.
Indeed! Thanks!


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

https://reviews.llvm.org/D124785

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


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib updated this revision to Diff 430552.
mib marked an inline comment as done.
mib added a comment.

Only run test on Darwin platforms


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

https://reviews.llvm.org/D124785

Files:
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Index: lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
@@ -0,0 +1,50 @@
+"""
+Test trimming long progress report in tiny terminal windows
+"""
+
+import os
+import pexpect
+import tempfile
+import re
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestTrimmedProgressReporting(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def do_test(self, term_width, pattern_list):
+self.build()
+# Start with a small window
+self.launch(use_colors=True)
+self.expect("set set show-progress true")
+self.expect("set show show-progress", substrs=["show-progress (boolean) = true"])
+self.expect("set set term-width " + str(term_width))
+self.expect("set show term-width", substrs=["term-width (int) = " + str(term_width)])
+
+self.child.send("file " + self.getBuildArtifact("a.out") + "\n")
+self.child.expect(pattern_list)
+
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipUnlessDarwin
+@skipIfEditlineSupportMissing
+def test_trimmed_progress_message(self):
+self.do_test(19, ['Locating externa...',
+  'Loading Apple DW...',
+  'Parsing symbol t...'])
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipUnlessDarwin
+@skipIfEditlineSupportMissing
+def test_long_progress_message(self):
+self.do_test(80, ['Locating external symbol file for a.out...',
+  'Loading Apple DWARF index for a.out...',
+  'Parsing symbol table for dyld...'])
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1852,20 +1852,26 @@
 return;
   }
 
+  // Trim the progress message if it exceeds the window's width and print it.
+  std::string message = data->GetMessage();
+  if (data->IsFinite())
+message = llvm::formatv("[{0}/{1}] {2}", data->GetCompleted(),
+data->GetTotal(), message)
+  .str();
+
+  // Trim the progress message if it exceeds the window's width and print it.
+  const uint32_t term_width = GetTerminalWidth();
+  const uint32_t ellipsis = 3;
+  if (message.size() + ellipsis >= term_width)
+message = message.substr(0, term_width - ellipsis);
+
   const bool use_color = GetUseColor();
   llvm::StringRef ansi_prefix = GetShowProgressAnsiPrefix();
   if (!ansi_prefix.empty())
 output->Printf(
 "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
-  std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
-output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-   data->GetTotal(), message.c_str());
-  } else {
-output->Printf("%s...", message.c_str());
-  }
+  output->Printf("%s...", message.c_str());
 
   llvm::StringRef ansi_suffix = GetShowProgressAnsiSuffix();
   if (!ansi_suffix.empty())
Index: lldb/include/lldb/Core/DebuggerEvents.h
===
--- lldb/include/lldb/Core/DebuggerEvents.h
+++ lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@
 
   static const ProgressEventData *GetEventDataFromEvent(const Event *event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string &GetMessage() const { return m_message; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] d71d1a9 - [lldb/Test] Add `use_colors` argument to the PExpect.launch wrapper

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T18:22:46-07:00
New Revision: d71d1a947bee1247e952f22c13ad3ed3d041e36a

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

LOG: [lldb/Test] Add `use_colors` argument to the PExpect.launch wrapper

This patch adds a new `use_colors` argument to the PExpect.launch
method.

As the name suggests, it allows the user to conditionally enable color
support in the debugger, which can be helpful to test functionalities that
rely on that, like progress reporting. It defaults to False.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbpexpect.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index de8f819e2552a..fe44c86d14f2f 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -24,16 +24,20 @@ def expect_prompt(self):
 self.child.expect_exact(self.PROMPT)
 
 def launch(self, executable=None, extra_args=None, timeout=60,
-   dimensions=None, run_under=None, post_spawn=None):
+   dimensions=None, run_under=None, post_spawn=None,
+   use_colors=False):
 logfile = getattr(sys.stdout, 'buffer',
 sys.stdout) if self.TraceOn() else None
 
 args = []
 if run_under is not None:
 args += run_under
-args += [lldbtest_config.lldbExec, '--no-lldbinit', '--no-use-colors']
+args += [lldbtest_config.lldbExec, '--no-lldbinit']
+if not use_colors:
+args += '--no-use-colors'
 for cmd in self.setUpCommands():
-args += ['-O', cmd]
+if use_colors and "use-color false" not in cmd:
+args += ['-O', cmd]
 if executable is not None:
 args += ['--file', executable]
 if extra_args is not None:
@@ -54,8 +58,9 @@ def launch(self, executable=None, extra_args=None, timeout=60,
 post_spawn()
 self.expect_prompt()
 for cmd in self.setUpCommands():
-self.child.expect_exact(cmd)
-self.expect_prompt()
+if use_colors and "use-color false" not in cmd:
+self.child.expect_exact(cmd)
+self.expect_prompt()
 if executable is not None:
 self.child.expect_exact("target create")
 self.child.expect_exact("Current executable set to")



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


[Lldb-commits] [lldb] 3e54ea0 - [lldb/crashlog] Fix line entries resolution in interactive mode

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T18:22:47-07:00
New Revision: 3e54ea0cfa3074e36ebee11848e072785437a8b9

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

LOG: [lldb/crashlog] Fix line entries resolution in interactive mode

This patch subtracts 1 to the pc of any frame above frame 0 to get the
previous line entry and display the right line in the debugger.

This also rephrase some old comment from `48d157dd4`.

rdar://9268

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/examples/python/scripted_process/crashlog_scripted_process.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index a20798ab11940..b2145762d7a8f 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -115,8 +115,8 @@ def dump_symbolicated(self, crash_log, options):
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) 
and frame_idx < options.disassemble_depth
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# Except for the zeroth frame, we should subtract 1 from every
+# frame pc to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask
 pc = pc if frame_idx == 0 or pc == 0 else pc - 1
 symbolicated_frame_addresses = crash_log.symbolicate(pc, 
options.verbose)

diff  --git 
a/lldb/examples/python/scripted_process/crashlog_scripted_process.py 
b/lldb/examples/python/scripted_process/crashlog_scripted_process.py
index f3b11e3699d98..70198da563aa3 100644
--- a/lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ b/lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -16,6 +16,7 @@ def parse_crashlog(self):
 return
 
 self.pid = crash_log.process_id
+self.addr_mask = crash_log.addr_mask
 self.crashed_thread_idx = crash_log.crashed_thread_idx
 self.loaded_images = []
 
@@ -122,11 +123,13 @@ def create_stackframes(self):
 return None
 
 for frame in self.backing_thread.frames:
+frame_pc = frame.pc & self.scripted_process.addr_mask
+pc = frame_pc if frame.index == 0  or frame_pc == 0 else frame_pc 
- 1
 sym_addr = lldb.SBAddress()
-sym_addr.SetLoadAddress(frame.pc, self.target)
+sym_addr.SetLoadAddress(pc, self.target)
 if not sym_addr.IsValid():
 continue
-self.frames.append({"idx": frame.index, "pc": frame.pc})
+self.frames.append({"idx": frame.index, "pc": pc})
 
 return self.frames
 



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


[Lldb-commits] [lldb] 051a5ae - [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T18:22:46-07:00
New Revision: 051a5ae99824fab879d9aade9d794e60ebc5c2e5

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

LOG: [lldb/Core] Fix "sticky" long progress messages

When the terminal window is too small, lldb would wrap progress messages
accross multiple lines which would break the progress event handling
code that is supposed to clear the message once the progress is completed.

This causes the progress message to remain on the screen, sometimes partially,
which can be confusing for the user.

To fix this issue, this patch trims the progress message to the terminal
width taking into account the progress counter leading the message for
finite progress events and also the trailing `...`.

rdar://91993836

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

Signed-off-by: Med Ismail Bennani 

Added: 

lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Modified: 
lldb/include/lldb/Core/DebuggerEvents.h
lldb/source/Core/Debugger.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/DebuggerEvents.h 
b/lldb/include/lldb/Core/DebuggerEvents.h
index 8388d5ce66f1c..b1ddb1fc47e2d 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@ class ProgressEventData : public EventData {
 
   static const ProgressEventData *GetEventDataFromEvent(const Event 
*event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string &GetMessage() const { return m_message; }

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index ff158dd1cfdfe..5f0b1cc66c0d3 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1852,20 +1852,26 @@ void Debugger::HandleProgressEvent(const lldb::EventSP 
&event_sp) {
 return;
   }
 
+  // Trim the progress message if it exceeds the window's width and print it.
+  std::string message = data->GetMessage();
+  if (data->IsFinite())
+message = llvm::formatv("[{0}/{1}] {2}", data->GetCompleted(),
+data->GetTotal(), message)
+  .str();
+
+  // Trim the progress message if it exceeds the window's width and print it.
+  const uint32_t term_width = GetTerminalWidth();
+  const uint32_t ellipsis = 3;
+  if (message.size() + ellipsis >= term_width)
+message = message.substr(0, term_width - ellipsis);
+
   const bool use_color = GetUseColor();
   llvm::StringRef ansi_prefix = GetShowProgressAnsiPrefix();
   if (!ansi_prefix.empty())
 output->Printf(
 "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
-  std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
-output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-   data->GetTotal(), message.c_str());
-  } else {
-output->Printf("%s...", message.c_str());
-  }
+  output->Printf("%s...", message.c_str());
 
   llvm::StringRef ansi_suffix = GetShowProgressAnsiSuffix();
   if (!ansi_suffix.empty())

diff  --git 
a/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
 
b/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
new file mode 100644
index 0..53765b0b37936
--- /dev/null
+++ 
b/lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
@@ -0,0 +1,50 @@
+"""
+Test trimming long progress report in tiny terminal windows
+"""
+
+import os
+import pexpect
+import tempfile
+import re
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestTrimmedProgressReporting(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def do_test(self, term_width, pattern_list):
+self.build()
+# Start with a small window
+self.launch(use_colors=True)
+self.expect("set set show-progress true")
+self.expect("set show show-progress", substrs=["show-progress 
(boolean) = true"])
+self.expect("set set term-width " + str(term_width))
+self.expect("set show term-width", substrs=["term-width (int) = " + 
str(term_width)])
+
+self.child.send("file " + self.getBuildArtifact("a.out") + "\n")
+self.child.expect(pattern_list)
+
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipUnlessDarwin
+@skipIfEditlineSupportMissing
+def test_tr

[Lldb-commits] [PATCH] D125915: [lldb/Test] Add `use_colors` argument to PExpect.launch wrapper

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd71d1a947bee: [lldb/Test] Add `use_colors` argument to the 
PExpect.launch wrapper (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125915

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


Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -24,16 +24,20 @@
 self.child.expect_exact(self.PROMPT)
 
 def launch(self, executable=None, extra_args=None, timeout=60,
-   dimensions=None, run_under=None, post_spawn=None):
+   dimensions=None, run_under=None, post_spawn=None,
+   use_colors=False):
 logfile = getattr(sys.stdout, 'buffer',
 sys.stdout) if self.TraceOn() else None
 
 args = []
 if run_under is not None:
 args += run_under
-args += [lldbtest_config.lldbExec, '--no-lldbinit', '--no-use-colors']
+args += [lldbtest_config.lldbExec, '--no-lldbinit']
+if not use_colors:
+args += '--no-use-colors'
 for cmd in self.setUpCommands():
-args += ['-O', cmd]
+if use_colors and "use-color false" not in cmd:
+args += ['-O', cmd]
 if executable is not None:
 args += ['--file', executable]
 if extra_args is not None:
@@ -54,8 +58,9 @@
 post_spawn()
 self.expect_prompt()
 for cmd in self.setUpCommands():
-self.child.expect_exact(cmd)
-self.expect_prompt()
+if use_colors and "use-color false" not in cmd:
+self.child.expect_exact(cmd)
+self.expect_prompt()
 if executable is not None:
 self.child.expect_exact("target create")
 self.child.expect_exact("Current executable set to")


Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -24,16 +24,20 @@
 self.child.expect_exact(self.PROMPT)
 
 def launch(self, executable=None, extra_args=None, timeout=60,
-   dimensions=None, run_under=None, post_spawn=None):
+   dimensions=None, run_under=None, post_spawn=None,
+   use_colors=False):
 logfile = getattr(sys.stdout, 'buffer',
 sys.stdout) if self.TraceOn() else None
 
 args = []
 if run_under is not None:
 args += run_under
-args += [lldbtest_config.lldbExec, '--no-lldbinit', '--no-use-colors']
+args += [lldbtest_config.lldbExec, '--no-lldbinit']
+if not use_colors:
+args += '--no-use-colors'
 for cmd in self.setUpCommands():
-args += ['-O', cmd]
+if use_colors and "use-color false" not in cmd:
+args += ['-O', cmd]
 if executable is not None:
 args += ['--file', executable]
 if extra_args is not None:
@@ -54,8 +58,9 @@
 post_spawn()
 self.expect_prompt()
 for cmd in self.setUpCommands():
-self.child.expect_exact(cmd)
-self.expect_prompt()
+if use_colors and "use-color false" not in cmd:
+self.child.expect_exact(cmd)
+self.expect_prompt()
 if executable is not None:
 self.child.expect_exact("target create")
 self.child.expect_exact("Current executable set to")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D124785: [lldb/Core] Fix "sticky" long progress messages

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG051a5ae99824: [lldb/Core] Fix "sticky" long 
progress messages (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124785

Files:
  lldb/include/lldb/Core/DebuggerEvents.h
  lldb/source/Core/Debugger.cpp
  
lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py

Index: lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
===
--- /dev/null
+++ lldb/test/API/functionalities/progress_reporting/TestTrimmedProgressReporting.py
@@ -0,0 +1,50 @@
+"""
+Test trimming long progress report in tiny terminal windows
+"""
+
+import os
+import pexpect
+import tempfile
+import re
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+class TestTrimmedProgressReporting(PExpectTest):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def do_test(self, term_width, pattern_list):
+self.build()
+# Start with a small window
+self.launch(use_colors=True)
+self.expect("set set show-progress true")
+self.expect("set show show-progress", substrs=["show-progress (boolean) = true"])
+self.expect("set set term-width " + str(term_width))
+self.expect("set show term-width", substrs=["term-width (int) = " + str(term_width)])
+
+self.child.send("file " + self.getBuildArtifact("a.out") + "\n")
+self.child.expect(pattern_list)
+
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipUnlessDarwin
+@skipIfEditlineSupportMissing
+def test_trimmed_progress_message(self):
+self.do_test(19, ['Locating externa...',
+  'Loading Apple DW...',
+  'Parsing symbol t...'])
+
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipUnlessDarwin
+@skipIfEditlineSupportMissing
+def test_long_progress_message(self):
+self.do_test(80, ['Locating external symbol file for a.out...',
+  'Loading Apple DWARF index for a.out...',
+  'Parsing symbol table for dyld...'])
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1852,20 +1852,26 @@
 return;
   }
 
+  // Trim the progress message if it exceeds the window's width and print it.
+  std::string message = data->GetMessage();
+  if (data->IsFinite())
+message = llvm::formatv("[{0}/{1}] {2}", data->GetCompleted(),
+data->GetTotal(), message)
+  .str();
+
+  // Trim the progress message if it exceeds the window's width and print it.
+  const uint32_t term_width = GetTerminalWidth();
+  const uint32_t ellipsis = 3;
+  if (message.size() + ellipsis >= term_width)
+message = message.substr(0, term_width - ellipsis);
+
   const bool use_color = GetUseColor();
   llvm::StringRef ansi_prefix = GetShowProgressAnsiPrefix();
   if (!ansi_prefix.empty())
 output->Printf(
 "%s", ansi::FormatAnsiTerminalCodes(ansi_prefix, use_color).c_str());
 
-  // Print the progress message.
-  std::string message = data->GetMessage();
-  if (data->GetTotal() != UINT64_MAX) {
-output->Printf("[%" PRIu64 "/%" PRIu64 "] %s...", data->GetCompleted(),
-   data->GetTotal(), message.c_str());
-  } else {
-output->Printf("%s...", message.c_str());
-  }
+  output->Printf("%s...", message.c_str());
 
   llvm::StringRef ansi_suffix = GetShowProgressAnsiSuffix();
   if (!ansi_suffix.empty())
Index: lldb/include/lldb/Core/DebuggerEvents.h
===
--- lldb/include/lldb/Core/DebuggerEvents.h
+++ lldb/include/lldb/Core/DebuggerEvents.h
@@ -32,6 +32,7 @@
 
   static const ProgressEventData *GetEventDataFromEvent(const Event *event_ptr);
   uint64_t GetID() const { return m_id; }
+  bool IsFinite() const { return m_total != UINT64_MAX; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   const std::string &GetMessage() const { return m_message; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D125928: [lldb/crashlog] Fix line entries resolution in interactive mode

2022-05-18 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e54ea0cfa30: [lldb/crashlog] Fix line entries resolution in 
interactive mode (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125928

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -16,6 +16,7 @@
 return
 
 self.pid = crash_log.process_id
+self.addr_mask = crash_log.addr_mask
 self.crashed_thread_idx = crash_log.crashed_thread_idx
 self.loaded_images = []
 
@@ -122,11 +123,13 @@
 return None
 
 for frame in self.backing_thread.frames:
+frame_pc = frame.pc & self.scripted_process.addr_mask
+pc = frame_pc if frame.index == 0  or frame_pc == 0 else frame_pc 
- 1
 sym_addr = lldb.SBAddress()
-sym_addr.SetLoadAddress(frame.pc, self.target)
+sym_addr.SetLoadAddress(pc, self.target)
 if not sym_addr.IsValid():
 continue
-self.frames.append({"idx": frame.index, "pc": frame.pc})
+self.frames.append({"idx": frame.index, "pc": pc})
 
 return self.frames
 
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -115,8 +115,8 @@
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) 
and frame_idx < options.disassemble_depth
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# Except for the zeroth frame, we should subtract 1 from every
+# frame pc to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask
 pc = pc if frame_idx == 0 or pc == 0 else pc - 1
 symbolicated_frame_addresses = crash_log.symbolicate(pc, 
options.verbose)


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -16,6 +16,7 @@
 return
 
 self.pid = crash_log.process_id
+self.addr_mask = crash_log.addr_mask
 self.crashed_thread_idx = crash_log.crashed_thread_idx
 self.loaded_images = []
 
@@ -122,11 +123,13 @@
 return None
 
 for frame in self.backing_thread.frames:
+frame_pc = frame.pc & self.scripted_process.addr_mask
+pc = frame_pc if frame.index == 0  or frame_pc == 0 else frame_pc - 1
 sym_addr = lldb.SBAddress()
-sym_addr.SetLoadAddress(frame.pc, self.target)
+sym_addr.SetLoadAddress(pc, self.target)
 if not sym_addr.IsValid():
 continue
-self.frames.append({"idx": frame.index, "pc": frame.pc})
+self.frames.append({"idx": frame.index, "pc": pc})
 
 return self.frames
 
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -115,8 +115,8 @@
 disassemble = (
 this_thread_crashed or options.disassemble_all_threads) and frame_idx < options.disassemble_depth
 
-# Any frame above frame zero and we have to subtract one to
-# get the previous line entry.
+# Except for the zeroth frame, we should subtract 1 from every
+# frame pc to get the previous line entry.
 pc = frame.pc & crash_log.addr_mask
 pc = pc if frame_idx == 0 or pc == 0 else pc - 1
 symbolicated_frame_addresses = crash_log.symbolicate(pc, options.verbose)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 80589f2 - [lldb/test] Make some tests as XFAIL while I investigate the issue

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T18:51:10-07:00
New Revision: 80589f272c200798b57a5151680a993bc2cc00a7

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

LOG: [lldb/test] Make some tests as XFAIL while I investigate the issue

This is very likely to be caused by d71d1a947bee1247e952f22c13ad3ed3d041e36a.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 

lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
lldb/test/API/iohandler/unicode/TestUnicode.py
lldb/test/API/repl/clang/TestClangREPL.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
 
b/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
index d580d936dc4a2..3bf3c05106219 100644
--- 
a/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
+++ 
b/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
@@ -28,6 +28,7 @@ def exit_expression_editor(self):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
+@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr49408')

diff  --git a/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py 
b/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
index 633aa1f570088..009d9df18a9e3 100644
--- a/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
+++ b/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
@@ -13,6 +13,7 @@ class GuiViewLargeCommandTest(PExpectTest):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
+@expectedFailureAll()
 @skipIfAsan
 @skipIfCursesSupportMissing
 @skipIfRemote # "run" command will not work correctly for remote debug

diff  --git a/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py 
b/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
index 80e45222a8157..362f446228200 100644
--- a/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
+++ b/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
@@ -23,6 +23,7 @@ class TestCase(PExpectTest):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
+@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion_add_spaces(self):
@@ -37,6 +38,7 @@ def test_autosuggestion_add_spaces(self):
 
 self.quit()
 
+@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion(self):
@@ -104,6 +106,7 @@ def test_autosuggestion(self):
 
 self.quit()
 
+@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion_custom_ansi_prefix_suffix(self):

diff  --git a/lldb/test/API/iohandler/unicode/TestUnicode.py 
b/lldb/test/API/iohandler/unicode/TestUnicode.py
index 7d239341d59c9..1c0696432d939 100644
--- a/lldb/test/API/iohandler/unicode/TestUnicode.py
+++ b/lldb/test/API/iohandler/unicode/TestUnicode.py
@@ -16,6 +16,7 @@ class TestCase(PExpectTest):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
+@expectedFailureAll()
 @skipIfAsan
 @skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 def test_unicode_input(self):

diff  --git a/lldb/test/API/repl/clang/TestClangREPL.py 
b/lldb/test/API/repl/clang/TestClangREPL.py
index 9ad67af74f9d8..8340c49850df5 100644
--- a/lldb/test/API/repl/clang/TestClangREPL.py
+++ b/lldb/test/API/repl/clang/TestClangREPL.py
@@ -21,6 +21,7 @@ def expect_repl(self, expr, substrs=[]):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
+@expectedFailureAll()
 @skipIfAsan
 @skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 @skipIfEditlineSupportMissing



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


[Lldb-commits] [lldb] 821ee17 - dyld patch

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T18:46:00-07:00
New Revision: 821ee172cdcd7196b6130321b53b6cc66bf1222b

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

LOG: dyld patch

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 033394bbfe1ed..043504be510aa 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -611,6 +611,8 @@ ModuleSP DynamicLoaderDarwin::GetDYLDModule() {
   return dyld_sp;
 }
 
+void DynamicLoaderDarwin::ClearDYLDModule() { m_dyld_module_wp.reset(); }
+
 bool DynamicLoaderDarwin::AddModulesUsingImageInfos(
 ImageInfo::collection &image_infos) {
   std::lock_guard guard(m_mutex);

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
index 1b705e9444680..5a72de25e5491 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
@@ -71,6 +71,8 @@ class DynamicLoaderDarwin : public 
lldb_private::DynamicLoader {
 
   lldb::ModuleSP GetDYLDModule();
 
+  void ClearDYLDModule();
+
   class Segment {
   public:
 Segment() : name() {}

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index e6bcc4c00feab..51ce96da82a9d 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -14,6 +14,7 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/ABI.h"
+#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
@@ -76,13 +77,16 @@ DynamicLoader *DynamicLoaderMacOS::CreateInstance(Process 
*process,
 // Constructor
 DynamicLoaderMacOS::DynamicLoaderMacOS(Process *process)
 : DynamicLoaderDarwin(process), m_image_infos_stop_id(UINT32_MAX),
-  m_break_id(LLDB_INVALID_BREAK_ID), m_mutex(),
+  m_break_id(LLDB_INVALID_BREAK_ID), 
+  m_dyld_handover_break_id(LLDB_INVALID_BREAK_ID), m_mutex(),
   m_maybe_image_infos_address(LLDB_INVALID_ADDRESS) {}
 
 // Destructor
 DynamicLoaderMacOS::~DynamicLoaderMacOS() {
   if (LLDB_BREAK_ID_IS_VALID(m_break_id))
 m_process->GetTarget().RemoveBreakpointByID(m_break_id);
+  if (LLDB_BREAK_ID_IS_VALID(m_dyld_handover_break_id))
+m_process->GetTarget().RemoveBreakpointByID(m_dyld_handover_break_id);
 }
 
 bool DynamicLoaderMacOS::ProcessDidExec() {
@@ -135,8 +139,11 @@ void DynamicLoaderMacOS::DoClear() {
 
   if (LLDB_BREAK_ID_IS_VALID(m_break_id))
 m_process->GetTarget().RemoveBreakpointByID(m_break_id);
+  if (LLDB_BREAK_ID_IS_VALID(m_dyld_handover_break_id))
+m_process->GetTarget().RemoveBreakpointByID(m_dyld_handover_break_id);
 
   m_break_id = LLDB_INVALID_BREAK_ID;
+  m_dyld_handover_break_id = LLDB_INVALID_BREAK_ID;
 }
 
 // Check if we have found DYLD yet
@@ -286,13 +293,51 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
 }
 if (dyld_mode == 0) {
   // dyld_notify_adding
-  dyld_instance->AddBinaries(image_load_addresses);
+  if (process->GetTarget().GetImages().GetSize() == 0) {
+// When all images have been removed, we're doing the
+// dyld handover from a launch-dyld to a shared-cache-dyld,
+// and we've just hit our one-shot address breakpoint in
+// the sc-dyld.  Note that the image addresses passed to
+// this function are inferior sizeof(void*) not uint64_t's
+// like our normal notification, so don't even look at
+// image_load_addresses.
+
+dyld_instance->ClearDYLDHandoverBreakpoint();
+
+dyld_instance->DoInitialImageFetch();
+dyld_instance->SetNotificationBreakpoint();
+  } else {
+dyld_instance->AddBinaries(image_load_addresses);
+  }
 } else if (dyld_mode == 1) {
   // dyld_notify_removing
 

[Lldb-commits] [lldb] fd25ad5 - Revert 821ee172cdcd7196b6130321b53b6cc66bf1222b

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T19:07:47-07:00
New Revision: fd25ad51224e0558328cc9dfc6bf4533fd54403b

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

LOG: Revert 821ee172cdcd7196b6130321b53b6cc66bf1222b

This reverts commit 821ee172cdcd7196b6130321b53b6cc66bf1222b, that
landed by mistake.

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 043504be510aa..033394bbfe1ed 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -611,8 +611,6 @@ ModuleSP DynamicLoaderDarwin::GetDYLDModule() {
   return dyld_sp;
 }
 
-void DynamicLoaderDarwin::ClearDYLDModule() { m_dyld_module_wp.reset(); }
-
 bool DynamicLoaderDarwin::AddModulesUsingImageInfos(
 ImageInfo::collection &image_infos) {
   std::lock_guard guard(m_mutex);

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
index 5a72de25e5491..1b705e9444680 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
@@ -71,8 +71,6 @@ class DynamicLoaderDarwin : public 
lldb_private::DynamicLoader {
 
   lldb::ModuleSP GetDYLDModule();
 
-  void ClearDYLDModule();
-
   class Segment {
   public:
 Segment() : name() {}

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index 51ce96da82a9d..e6bcc4c00feab 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -14,7 +14,6 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Target/ABI.h"
-#include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
@@ -77,16 +76,13 @@ DynamicLoader *DynamicLoaderMacOS::CreateInstance(Process 
*process,
 // Constructor
 DynamicLoaderMacOS::DynamicLoaderMacOS(Process *process)
 : DynamicLoaderDarwin(process), m_image_infos_stop_id(UINT32_MAX),
-  m_break_id(LLDB_INVALID_BREAK_ID), 
-  m_dyld_handover_break_id(LLDB_INVALID_BREAK_ID), m_mutex(),
+  m_break_id(LLDB_INVALID_BREAK_ID), m_mutex(),
   m_maybe_image_infos_address(LLDB_INVALID_ADDRESS) {}
 
 // Destructor
 DynamicLoaderMacOS::~DynamicLoaderMacOS() {
   if (LLDB_BREAK_ID_IS_VALID(m_break_id))
 m_process->GetTarget().RemoveBreakpointByID(m_break_id);
-  if (LLDB_BREAK_ID_IS_VALID(m_dyld_handover_break_id))
-m_process->GetTarget().RemoveBreakpointByID(m_dyld_handover_break_id);
 }
 
 bool DynamicLoaderMacOS::ProcessDidExec() {
@@ -139,11 +135,8 @@ void DynamicLoaderMacOS::DoClear() {
 
   if (LLDB_BREAK_ID_IS_VALID(m_break_id))
 m_process->GetTarget().RemoveBreakpointByID(m_break_id);
-  if (LLDB_BREAK_ID_IS_VALID(m_dyld_handover_break_id))
-m_process->GetTarget().RemoveBreakpointByID(m_dyld_handover_break_id);
 
   m_break_id = LLDB_INVALID_BREAK_ID;
-  m_dyld_handover_break_id = LLDB_INVALID_BREAK_ID;
 }
 
 // Check if we have found DYLD yet
@@ -293,51 +286,13 @@ bool DynamicLoaderMacOS::NotifyBreakpointHit(void *baton,
 }
 if (dyld_mode == 0) {
   // dyld_notify_adding
-  if (process->GetTarget().GetImages().GetSize() == 0) {
-// When all images have been removed, we're doing the
-// dyld handover from a launch-dyld to a shared-cache-dyld,
-// and we've just hit our one-shot address breakpoint in
-// the sc-dyld.  Note that the image addresses passed to
-// this function are inferior sizeof(void*) not uint64_t's
-// like our normal notification, so don't even look at
-// image_load_addresses.
-
-dyld_instance->ClearDYLDHandoverBreakpoint();
-
-dyld_instance->DoInitialImageFetch();
-dyld_instance->SetNotificationBreakpoint();
-  } else {
-dyld_instance->AddBinaries(image_load_addresses);
-  }
+  dyld_instance->AddBinaries(image_load_addres

[Lldb-commits] [PATCH] D125943: [trace][intelpt] Support system-wide tracing [11] - Read warnings and perf conversion in the client

2022-05-18 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a subscriber: pengfei.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Add logging for when the live state of the process is refreshed
- Move error handling of the live state refreshing to Trace from TraceIntelPT. 
This allows refreshing to fail either at the plug-in level or at the base class 
level. The error is cached and it can be gotten every time 
RefreshLiveProcessState is invoked.
- Allow DoRefreshLiveProcessState to handle plugin-specific parameters.
- Add some encapsulation to prevent TraceIntelPT from accessing variables 
belonging to Trace.

Test done via logging:

  (lldb) b main
  Breakpoint 1: where = a.out`main + 20 at main.cpp:27:20, address = 
0x004023d9
  (lldb) r
  Process 2359706 launched: '/home/wallace/a.out' (x86_64)
  Process 2359706 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x004023d9 a.out`main at main.cpp:27:20
 24   };
 25  
 26   int main() {
  -> 27 std::vector vvv;
 28 for (int i = 0; i < 10; i++)
 29   vvv.push_back(i);
 30  
  (lldb) process trace start
(lldb) log enable lldb target -F(lldb) n
  Process 2359706 stopped
  * thread #1, name = 'a.out', stop reason = step over
  frame #0: 0x004023e8 a.out`main at main.cpp:28:12
 25  
 26   int main() {
 27 std::vector vvv;
  -> 28 for (int i = 0; i < 10; i++)
 29   vvv.push_back(i);
 30  
 31 std::deque dq1 = {1, 2, 3};
  (lldb) thread trace dump instructions -c 2 -t 
Trace.cpp:RefreshLiveProcessState   
 Trace::RefreshLiveProcessState invoked
  TraceIntelPT.cpp:DoRefreshLiveProcessState   TraceIntelPT 
found tsc conversion information
  thread #1: tid = 2359706
a.out`std::vector>::vector() + 26 at 
stl_vector.h:395:19
  54: [tsc=unavailable] 0x00403a7cretq   

See the logging lines at the end of the dump. They indicate that refreshing 
happened and that perf conversion information was found.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125943

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Thread.h"
+#include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Stream.h"
 
 using namespace lldb;
@@ -175,28 +176,37 @@
   return m_live_process->TraceGetBinaryData(request);
 }
 
-void Trace::RefreshLiveProcessState() {
+const char *Trace::RefreshLiveProcessState() {
   if (!m_live_process)
-return;
+return nullptr;
 
   uint32_t new_stop_id = m_live_process->GetStopID();
   if (new_stop_id == m_stop_id)
-return;
+return nullptr;
+
+  Log *log = GetLog(LLDBLog::Target);
+  LLDB_LOG(log, "Trace::RefreshLiveProcessState invoked");
 
   m_stop_id = new_stop_id;
   m_live_thread_data.clear();
+  m_live_refresh_error.reset();
+
+  auto HandleError = [&](Error &&err) -> const char * {
+m_live_refresh_error = toString(std::move(err));
+return m_live_refresh_error->c_str();
+  };
 
   Expected json_string = GetLiveProcessState();
-  if (!json_string) {
-DoRefreshLiveProcessState(json_string.takeError());
-return;
-  }
+  if (!json_string)
+return HandleError(json_string.takeError());
+
   Expected live_process_state =
   json::parse(*json_string, "TraceGetStateResponse");
-  if (!live_process_state) {
-DoRefreshLiveProcessState(live_process_state.takeError());
-return;
-  }
+  if (!live_process_state)
+return HandleError(live_process_state.takeError());
+
+  for (std::string &warning : live_process_state->warnings)
+LLDB_LOG(log, "Warning when fetching the trace state: {0}", warning);
 
   for (const TraceThreadState &thread_state :
live_process_state->traced_threads) {
@@ -207,9 +217,15 @@
   for (const TraceBinaryData &item : live_process_state->process_binary_data)
 m_live_process_data[item.kind] = item.size;
 
-  DoRefreshLiveProcessState(std::move(live_process_state));
+  if (Error err = DoRefreshLiveProcessState(std::move(*live_process_state),
+*json_string))
+return HandleError(std::move(err));
+
+  return nullptr;
 }
 
+Process *Trace::GetLiveProcess() { return m_live_process; }
+
 uint32_t Trace::GetStopID() {
   RefreshLiveProcessState();
   return

[Lldb-commits] [lldb] 1351a9b - [lldb/test] Fix failures caused by a previous PExpect.launch change

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T21:12:44-07:00
New Revision: 1351a9b19ecf48ebb2caad8ae6d8776a6dc1786d

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

LOG: [lldb/test] Fix failures caused by a previous PExpect.launch change

This should fix the issues introduced by d71d1a9, which skipped all the
test setup commands.

This also fixes the test failures happening in TestAutosuggestion.py.

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbpexpect.py
lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index fe44c86d14f2f..1083705a20119 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -36,8 +36,9 @@ def launch(self, executable=None, extra_args=None, timeout=60,
 if not use_colors:
 args += '--no-use-colors'
 for cmd in self.setUpCommands():
-if use_colors and "use-color false" not in cmd:
-args += ['-O', cmd]
+if "use-color false" in cmd and use_colors:
+continue
+args += ['-O', cmd]
 if executable is not None:
 args += ['--file', executable]
 if extra_args is not None:
@@ -58,9 +59,10 @@ def launch(self, executable=None, extra_args=None, 
timeout=60,
 post_spawn()
 self.expect_prompt()
 for cmd in self.setUpCommands():
-if use_colors and "use-color false" not in cmd:
-self.child.expect_exact(cmd)
-self.expect_prompt()
+if "use-color false" in cmd and use_colors:
+continue
+self.child.expect_exact(cmd)
+self.expect_prompt()
 if executable is not None:
 self.child.expect_exact("target create")
 self.child.expect_exact("Current executable set to")

diff  --git a/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py 
b/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
index 80e45222a8157..7d29d95291abd 100644
--- a/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
+++ b/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
@@ -26,7 +26,8 @@ class TestCase(PExpectTest):
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion_add_spaces(self):
-self.launch(extra_args=["-o", "settings set show-autosuggestion true", 
"-o", "settings set use-color true"])
+self.launch(use_colors=True,
+extra_args=["-o", "settings set show-autosuggestion true", 
"-o", "settings set use-color true"])
 
 
 # Check if spaces are added to hide the previous gray characters.
@@ -40,7 +41,8 @@ def test_autosuggestion_add_spaces(self):
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion(self):
-self.launch(extra_args=["-o", "settings set show-autosuggestion true", 
"-o", "settings set use-color true"])
+self.launch(use_colors=True,
+extra_args=["-o", "settings set show-autosuggestion true", 
"-o", "settings set use-color true"])
 
 # Common input codes.
 ctrl_f = "\x06"
@@ -107,7 +109,8 @@ def test_autosuggestion(self):
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion_custom_ansi_prefix_suffix(self):
-self.launch(extra_args=["-o", "settings set show-autosuggestion true",
+self.launch(use_colors=True,
+extra_args=["-o", "settings set show-autosuggestion true",
 "-o", "settings set use-color true",
 "-o", "settings set 
show-autosuggestion-ansi-prefix ${ansi.fg.red}",
 "-o", "setting set 
show-autosuggestion-ansi-suffix ${ansi.fg.cyan}"])



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


[Lldb-commits] [lldb] fbf0c42 - Revert "[lldb/test] Make some tests as XFAIL while I investigate the issue"

2022-05-18 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-05-18T21:12:43-07:00
New Revision: fbf0c4229410fb17842cc0c0db882eeba6252ae8

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

LOG: Revert "[lldb/test] Make some tests as XFAIL while I investigate the issue"

This reverts commit 80589f272c200798b57a5151680a993bc2cc00a7.

Added: 


Modified: 

lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
lldb/test/API/iohandler/unicode/TestUnicode.py
lldb/test/API/repl/clang/TestClangREPL.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
 
b/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
index 3bf3c05106219..d580d936dc4a2 100644
--- 
a/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
+++ 
b/lldb/test/API/commands/expression/multiline-completion/TestMultilineCompletion.py
@@ -28,7 +28,6 @@ def exit_expression_editor(self):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
-@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 @expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr49408')

diff  --git a/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py 
b/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
index 009d9df18a9e3..633aa1f570088 100644
--- a/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
+++ b/lldb/test/API/commands/gui/viewlarge/TestGuiViewLarge.py
@@ -13,7 +13,6 @@ class GuiViewLargeCommandTest(PExpectTest):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
-@expectedFailureAll()
 @skipIfAsan
 @skipIfCursesSupportMissing
 @skipIfRemote # "run" command will not work correctly for remote debug

diff  --git a/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py 
b/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
index 362f446228200..80e45222a8157 100644
--- a/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
+++ b/lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py
@@ -23,7 +23,6 @@ class TestCase(PExpectTest):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
-@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion_add_spaces(self):
@@ -38,7 +37,6 @@ def test_autosuggestion_add_spaces(self):
 
 self.quit()
 
-@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion(self):
@@ -106,7 +104,6 @@ def test_autosuggestion(self):
 
 self.quit()
 
-@expectedFailureAll()
 @skipIfAsan
 @skipIfEditlineSupportMissing
 def test_autosuggestion_custom_ansi_prefix_suffix(self):

diff  --git a/lldb/test/API/iohandler/unicode/TestUnicode.py 
b/lldb/test/API/iohandler/unicode/TestUnicode.py
index 1c0696432d939..7d239341d59c9 100644
--- a/lldb/test/API/iohandler/unicode/TestUnicode.py
+++ b/lldb/test/API/iohandler/unicode/TestUnicode.py
@@ -16,7 +16,6 @@ class TestCase(PExpectTest):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
-@expectedFailureAll()
 @skipIfAsan
 @skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 def test_unicode_input(self):

diff  --git a/lldb/test/API/repl/clang/TestClangREPL.py 
b/lldb/test/API/repl/clang/TestClangREPL.py
index 8340c49850df5..9ad67af74f9d8 100644
--- a/lldb/test/API/repl/clang/TestClangREPL.py
+++ b/lldb/test/API/repl/clang/TestClangREPL.py
@@ -21,7 +21,6 @@ def expect_repl(self, expr, substrs=[]):
 
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
-@expectedFailureAll()
 @skipIfAsan
 @skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Randomly fails on 
buildbot
 @skipIfEditlineSupportMissing



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