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

2021-07-28 Thread Kim-Anh Tran via Phabricator via lldb-commits
kimanh marked an inline comment as not done.
kimanh added a comment.

Thanks once more for reviewing! If I should rename the variable (or anything 
else), let me know! Otherwise, could you help me to land this change (since I 
do not have committer rights)?




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:136
   continue;
 if (entry_or->getCUOffset() != cu_offset)
   continue;

jankratochvil wrote:
> It would be nice to reverse these two comparisons for `found_entry_for_cu` 
> but `getCUOffset()` is much more expensive. Just saying.
> 
Ah, yes, that's true. I could call it `found_variable_entry_for_cu` though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106355

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


[Lldb-commits] [lldb] ab5b8ee - [LLDB] Skip HW breakpoints test_step_until on Arm/Linux

2021-07-28 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2021-07-28T13:18:14+05:00
New Revision: ab5b8ee1a7a18fe097419e21224ac4f15591bcd7

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

LOG: [LLDB] Skip HW breakpoints test_step_until on Arm/Linux

test_step_until xpasses on some machines while fails on others. I am
marking it as skipped for now.

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
index f7787bbf68b12..b629d678d08e6 100644
--- 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
+++ 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
@@ -27,7 +27,7 @@ def test_breakpoint(self):
 breakpoint = target.BreakpointCreateByLocation("main.c", 1)
 self.assertTrue(breakpoint.IsHardware())
 
-@expectedFailureIfFn(supports_hw_breakpoints)
+@expectedFailureIfFn(supports_hw_breakpoints)
 def test_step_range(self):
 """Test stepping when hardware breakpoints are required."""
 self.build()
@@ -86,6 +86,7 @@ def test_step_over(self):
 'error: Could not create hardware breakpoint for thread plan.'
 ])
 
+@skipIf(oslist=["linux"], archs=["arm"])
 @expectedFailureIfFn(supports_hw_breakpoints)
 def test_step_until(self):
 """Test stepping until when hardware breakpoints are required."""



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


[Lldb-commits] [lldb] 6cd0e35 - Revert "[LLDB] Skip HW breakpoints test_step_until on Arm/Linux"

2021-07-28 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2021-07-28T13:26:06+05:00
New Revision: 6cd0e35f43ac7aab8ff05acb4c5dfaa9071958d5

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

LOG: Revert "[LLDB] Skip HW breakpoints test_step_until on Arm/Linux"

This reverts commit ab5b8ee1a7a18fe097419e21224ac4f15591bcd7.

This caused some failure on buildbots so reverting it for now.

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
index b629d678d08e6..f7787bbf68b12 100644
--- 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
+++ 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
@@ -27,7 +27,7 @@ def test_breakpoint(self):
 breakpoint = target.BreakpointCreateByLocation("main.c", 1)
 self.assertTrue(breakpoint.IsHardware())
 
-@expectedFailureIfFn(supports_hw_breakpoints)
+@expectedFailureIfFn(supports_hw_breakpoints)
 def test_step_range(self):
 """Test stepping when hardware breakpoints are required."""
 self.build()
@@ -86,7 +86,6 @@ def test_step_over(self):
 'error: Could not create hardware breakpoint for thread plan.'
 ])
 
-@skipIf(oslist=["linux"], archs=["arm"])
 @expectedFailureIfFn(supports_hw_breakpoints)
 def test_step_until(self):
 """Test stepping until when hardware breakpoints are required."""



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


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

2021-07-28 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 rG6a7a2ee8161d: [lldb] Add "memory tag write" 
command (authored by DavidSpickett).

Changed prior to commit:
  https://reviews.llvm.org/D105182?vs=362055&id=362306#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105182

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

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

[Lldb-commits] [lldb] 6a7a2ee - [lldb] Add "memory tag write" command

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

Author: David Spickett
Date: 2021-07-28T10:12:50+01:00
New Revision: 6a7a2ee8161da84d9a58a88b497b0b47c8df99f3

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

LOG: [lldb] Add "memory tag write" command

This adds a new command for writing memory tags.
It is based on the existing "memory write" command.

Syntax: memory tag write   [ [...]]
(where "value" is a tag value)

(lldb) memory tag write mte_buf 1 2
(lldb) memory tag read mte_buf mte_buf+32
Logical tag: 0x0
Allocation tags:
[0xf7ff9000, 0xf7ff9010): 0x1
[0xf7ff9010, 0xf7ff9020): 0x2

The range you are writing to will be calculated by
aligning the address down to a granule boundary then
adding as many granules as there are tags.

(a repeating mode with an end address will be in a follow
up patch)

This is why "memory tag write" uses MakeTaggedRange but has
some extra steps to get this specific behaviour.

The command does all the usual argument validation:
* Address must evaluate
* You must supply at least one tag value
  (though lldb-server would just treat that as a nop anyway)
* Those tag values must be valid for your tagging scheme
  (e.g. for MTE the value must be > 0 and < 0xf)
* The calculated range must be memory tagged

That last error will show you the final range, not just
the start address you gave the command.

(lldb) memory tag write mte_buf_2+page_size-16 6
(lldb) memory tag write mte_buf_2+page_size-16 6 7
error: Address range 0xf7ffaff0:0xf7ffb010 is not in a memory tagged 
region

(note that we do not check if the region is writeable
since lldb can write to it anyway)

The read and write tag tests have been merged into
a single set of "tag access" tests as their test programs would
have been almost identical.
(also I have renamed some of the buffers to better
show what each one is used for)

Reviewed By: omjavaid

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

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

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

Modified: 
lldb/source/Commands/CommandObjectMemoryTag.cpp
lldb/test/API/functionalities/memory/tag/TestMemoryTag.py

Removed: 
lldb/test/API/linux/aarch64/mte_tag_read/Makefile
lldb/test/API/linux/aarch64/mte_tag_read/TestAArch64LinuxMTEMemoryTagRead.py
lldb/test/API/linux/aarch64/mte_tag_read/main.c



diff  --git a/lldb/source/Commands/CommandObjectMemoryTag.cpp 
b/lldb/source/Commands/CommandObjectMemoryTag.cpp
index 1dfb32a92f3bb..7c244befe0da4 100644
--- a/lldb/source/Commands/CommandObjectMemoryTag.cpp
+++ b/lldb/source/Commands/CommandObjectMemoryTag.cpp
@@ -115,6 +115,114 @@ class CommandObjectMemoryTagRead : public 
CommandObjectParsed {
   }
 };
 
+#define LLDB_OPTIONS_memory_tag_write
+#include "CommandOptions.inc"
+
+class CommandObjectMemoryTagWrite : public CommandObjectParsed {
+public:
+  CommandObjectMemoryTagWrite(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "tag",
+"Write memory tags starting from the granule that "
+"contains the given address.",
+nullptr,
+eCommandRequiresTarget | eCommandRequiresProcess |
+eCommandProcessMustBePaused) {
+// Address
+m_arguments.push_back(
+
CommandArgumentEntry{CommandArgumentData(eArgTypeAddressOrExpression)});
+// One or more tag values
+m_arguments.push_back(CommandArgumentEntry{
+CommandArgumentData(eArgTypeValue, eArgRepeatPlus)});
+  }
+
+  ~CommandObjectMemoryTagWrite() override = default;
+
+protected:
+  bool DoExecute(Args &command, CommandReturnObject &result) override {
+if (command.GetArgumentCount() < 2) {
+  result.AppendError("wrong number of arguments; expected "
+ "  [ [...]]");
+  return false;
+}
+
+Status error;
+addr_t start_addr = OptionArgParser::ToAddress(
+&m_exe_ctx, command[0].ref(), LLDB_INVALID_ADDRESS, &error);
+if (start_addr == LLDB_INVALID_ADDRESS) {
+  result.AppendErrorWithFormatv("Invalid address expression, {0}",
+error.AsCString());
+  return false;
+}
+
+command.Shift(); // shift off start address
+
+std::vector tags;
+for (auto &entry : command) {
+  lldb::addr_t tag_value;
+  // getAsInteger returns true on failure
+  if (entry.ref().getAsInteger(0, tag_value)) {
+result.AppendErrorWithFormat(
+"'%s' is not a valid unsigned decimal string value.\n",
+entry.c_str());
+return false;
+  }
+  tags.push_back(tag_value);
+}
+
+P

[Lldb-commits] [lldb] 5db8e23 - [lldb] Temporarily bump the max length of the pexpect error message to diagnose an lldb-aarch64 test failure

2021-07-28 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-07-28T11:47:54+02:00
New Revision: 5db8e232126fc4c0f5d5b0339bdc5a49830268d1

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

LOG: [lldb] Temporarily bump the max length of the pexpect error message to 
diagnose an lldb-aarch64 test failure

This is only temporarily to gather some logs before this gets reverted. See
D106873 for a discussion about how/if we can make this change permanent.

Added: 


Modified: 
lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py

Removed: 




diff  --git a/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py 
b/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py
index 6b9ad3f63f7cd..8ef077aa7d31b 100644
--- a/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py
+++ b/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py
@@ -212,8 +212,8 @@ def __str__(self):
 s.append(repr(self))
 s.append('command: ' + str(self.command))
 s.append('args: %r' % (self.args,))
-s.append('buffer (last 100 chars): %r' % self.buffer[-100:])
-s.append('before (last 100 chars): %r' % self.before[-100:] if 
self.before else '')
+s.append('buffer (last 1 chars): %r' % self.buffer[-1:])
+s.append('before (last 1 chars): %r' % self.before[-1:] if 
self.before else '')
 s.append('after: %r' % (self.after,))
 s.append('match: %r' % (self.match,))
 s.append('match_index: ' + str(self.match_index))



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


[Lldb-commits] [lldb] 30308d1 - [LLDB] Skip HW breakpoints test_step_until on Arm/Linux

2021-07-28 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2021-07-28T15:30:47+05:00
New Revision: 30308d1eb966afa35ee2fd5c5b47b17eb0382896

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

LOG: [LLDB] Skip HW breakpoints test_step_until on Arm/Linux

test_step_until xpasses on some machines while fails on others.
Marking it as skipped for now.

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
index f7787bbf68b12..b27b53bc31d56 100644
--- 
a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
+++ 
b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/require_hw_breakpoints/TestRequireHWBreakpoints.py
@@ -13,6 +13,7 @@
 class BreakpointLocationsTestCase(HardwareBreakpointTestBase):
 mydir = TestBase.compute_mydir(__file__)
 
+@skipIf(oslist=["linux"], archs=["arm"])
 def supports_hw_breakpoints(self):
 return super().supports_hw_breakpoints()
 



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


[Lldb-commits] [lldb] 0dc9c88 - [LLDB] Skip TestGuiBasicDebug.py on Arm/AArch64 Linux

2021-07-28 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2021-07-28T15:30:48+05:00
New Revision: 0dc9c88aa38ed330ddffe92bae79d4066832c38e

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

LOG: [LLDB] Skip TestGuiBasicDebug.py on Arm/AArch64 Linux

TestGuiBasicDebug.py randomly fails due to timeouts sending out false
negatives on LLDB Arm and AArch64 Linux buildbots. I havnt found a
reliable wayy to set pexpect timeout for this test to pass regularly.

Skipping it on Arm and AArch64 Linux to silence buildbot failures.

Added: 


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

Removed: 




diff  --git a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py 
b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
index beff76e9374f..e0eca428b6bd 100644
--- a/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
+++ b/lldb/test/API/commands/gui/basicdebug/TestGuiBasicDebug.py
@@ -14,6 +14,7 @@ class TestGuiBasicDebugCommandTest(PExpectTest):
 # PExpect uses many timeouts internally and doesn't play well
 # under ASAN on a loaded machine..
 @skipIfAsan
+@skipIf(oslist=["linux"], archs=["arm","aarch64"])
 @skipIfCursesSupportMissing
 def test_gui(self):
 self.build()



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


[Lldb-commits] [PATCH] D105183: [lldb] Add "memory tag write" --end-addr option

2021-07-28 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 rG6eded00e0c6b: [lldb] Add "memory tag write" 
--end-addr option (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105183

Files:
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/Options.td
  
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
@@ -216,3 +216,59 @@
 self.expect("memory tag write mte_buf 99",
 patterns=["error: Found tag 0x63 which is > max MTE tag value of 0xf."],
 error=True)
+
+# You can provide an end address and have lldb repeat the tags as needed
+# The range is checked in the same way it is for "memory tag read"
+self.expect("memory tag write mte_buf 9 -e",
+patterns=["error: last option requires an argument"],
+error=True)
+self.expect("memory tag write mte_buf 9 -e food",
+patterns=["error: address expression \"food\" evaluation failed"],
+error=True)
+self.expect("memory tag write mte_buf_2 9 --end-addr mte_buf_2",
+patterns=["error: End address \(0x[A-Fa-f0-9]+\) must be "
+  "greater than the start address \(0x[A-Fa-f0-9]+\)"],
+error=True)
+self.expect("memory tag write mte_buf_2 9 --end-addr mte_buf_2-16",
+patterns=["error: End address \(0x[A-Fa-f0-9]+\) must be "
+  "greater than the start address \(0x[A-Fa-f0-9]+\)"],
+error=True)
+self.expect("memory tag write mte_buf_2 9 --end-addr mte_buf_2+page_size+16",
+patterns=["error: Address range 0x[0-9A-fa-f]+00:0x[0-9A-Fa-f]+10 "
+  "is not in a memory tagged region"],
+error=True)
+
+# Tags are repeated across the range
+# For these we'll read one extra to make sure we don't over write
+self.expect("memory tag write mte_buf_2 4 5 --end-addr mte_buf_2+48")
+self.expect("memory tag read mte_buf_2 mte_buf_2+64",
+patterns=["Logical tag: 0x0\n"
+  "Allocation tags:\n"
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x4\n"
+  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0x5\n"
+  "\[0x[0-9A-Fa-f]+20, 0x[0-9A-Fa-f]+30\): 0x4\n"
+  "\[0x[0-9A-Fa-f]+30, 0x[0-9A-Fa-f]+40\): 0x0$"])
+
+# Since this aligns like tag read does, the start is aligned down and the end up.
+# Meaning that start/end tells you the start/end granule that will be written.
+# This matters particularly if either are misaligned.
+
+# Here start moves down so the final range is mte_buf_2 -> mte_buf_2+32
+self.expect("memory tag write mte_buf_2+8 6 -end-addr mte_buf_2+32")
+self.expect("memory tag read mte_buf_2 mte_buf_2+48",
+patterns=["Logical tag: 0x0\n"
+  "Allocation tags:\n"
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x6\n"
+  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0x6\n"
+  "\[0x[0-9A-Fa-f]+20, 0x[0-9A-Fa-f]+30\): 0x4$"])
+
+# If we do the same with a misaligned end, it also moves but upward.
+# The intial range is 2 granules but the final range is mte_buf_2 -> mte_buf_2+48
+self.expect("memory tag write mte_buf_2+8 3 -end-addr mte_buf_2+32+8")
+self.expect("memory tag read mte_buf_2 mte_buf_2+64",
+patterns=["Logical tag: 0x0\n"
+  "Allocation tags:\n"
+  "\[0x[0-9A-Fa-f]+00, 0x[0-9A-Fa-f]+10\): 0x3\n"
+  "\[0x[0-9A-Fa-f]+10, 0x[0-9A-Fa-f]+20\): 0x3\n"
+  "\[0x[0-9A-Fa-f]+20, 0x[0-9A-Fa-f]+30\): 0x3\n"
+  "\[0x[0-9A-Fa-f]+30, 0x[0-9A-Fa-f]+40\): 0x0$"])
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -504,6 +504,14 @@
 Desc<"Start writing bytes from an offset within the input file.">;
 }
 
+let Command = "memory tag write" in {
+  def memory_write_end_addr : Option<"end-addr", "e">, Group<1>,
+  Arg<"AddressOrExpression">, Desc<
+"Set tags for start address to end-addr, repeating tags as nee

[Lldb-commits] [lldb] 6eded00 - [lldb] Add "memory tag write" --end-addr option

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

Author: David Spickett
Date: 2021-07-28T14:05:40+01:00
New Revision: 6eded00e0c6b4e06225df74292c078030556b8ce

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

LOG: [lldb] Add "memory tag write" --end-addr option

The default mode of "memory tag write" is to calculate the
range from the start address and the number of tags given.
(just like "memory write" does)

(lldb) memory tag write mte_buf 1 2
(lldb) memory tag read mte_buf mte_buf+48
Logical tag: 0x0
Allocation tags:
[0xf7ff9000, 0xf7ff9010): 0x1
[0xf7ff9010, 0xf7ff9020): 0x2
[0xf7ff9020, 0xf7ff9030): 0x0

This new option allows you to set an end address and have
the tags repeat until that point.

(lldb) memory tag write mte_buf 1 2 --end-addr mte_buf+64
(lldb) memory tag read mte_buf mte_buf+80
Logical tag: 0x0
Allocation tags:
[0xf7ff9000, 0xf7ff9010): 0x1
[0xf7ff9010, 0xf7ff9020): 0x2
[0xf7ff9020, 0xf7ff9030): 0x1
[0xf7ff9030, 0xf7ff9040): 0x2
[0xf7ff9040, 0xf7ff9050): 0x0

This is implemented using the QMemTags packet previously
added. We skip validating the number of tags in lldb and send
them on to lldb-server, which repeats them as needed.

Apart from the number of tags, all the other client side checks
remain. Tag values, memory range must be tagged, etc.

Reviewed By: omjavaid

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectMemoryTag.cpp
lldb/source/Commands/Options.td

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

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectMemoryTag.cpp 
b/lldb/source/Commands/CommandObjectMemoryTag.cpp
index 7c244befe0da..76296bf4b49a 100644
--- a/lldb/source/Commands/CommandObjectMemoryTag.cpp
+++ b/lldb/source/Commands/CommandObjectMemoryTag.cpp
@@ -7,8 +7,11 @@
 
//===--===//
 
 #include "CommandObjectMemoryTag.h"
+#include "lldb/Host/OptionParser.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionGroupFormat.h"
+#include "lldb/Interpreter/OptionValueString.h"
 #include "lldb/Target/Process.h"
 
 using namespace lldb;
@@ -120,23 +123,64 @@ class CommandObjectMemoryTagRead : public 
CommandObjectParsed {
 
 class CommandObjectMemoryTagWrite : public CommandObjectParsed {
 public:
+  class OptionGroupTagWrite : public OptionGroup {
+  public:
+OptionGroupTagWrite() : OptionGroup(), m_end_addr(LLDB_INVALID_ADDRESS) {}
+
+~OptionGroupTagWrite() override = default;
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_memory_tag_write_options);
+}
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+  ExecutionContext *execution_context) override {
+  Status status;
+  const int short_option =
+  g_memory_tag_write_options[option_idx].short_option;
+
+  switch (short_option) {
+  case 'e':
+m_end_addr = OptionArgParser::ToAddress(execution_context, 
option_value,
+LLDB_INVALID_ADDRESS, &status);
+break;
+  default:
+llvm_unreachable("Unimplemented option");
+  }
+
+  return status;
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  m_end_addr = LLDB_INVALID_ADDRESS;
+}
+
+lldb::addr_t m_end_addr;
+  };
+
   CommandObjectMemoryTagWrite(CommandInterpreter &interpreter)
   : CommandObjectParsed(interpreter, "tag",
 "Write memory tags starting from the granule that "
 "contains the given address.",
 nullptr,
 eCommandRequiresTarget | eCommandRequiresProcess |
-eCommandProcessMustBePaused) {
+eCommandProcessMustBePaused),
+m_option_group(), m_tag_write_options() {
 // Address
 m_arguments.push_back(
 
CommandArgumentEntry{CommandArgumentData(eArgTypeAddressOrExpression)});
 // One or more tag values
 m_arguments.push_back(CommandArgumentEntry{
 CommandArgumentData(eArgTypeValue, eArgRepeatPlus)});
+
+m_option_group.Append(&m_tag_write_options);
+m_option_group.Finalize();
   }
 
   ~CommandObjectMemoryTagWrite() override = default;
 
+  Options *GetOptions() override { return &m_option_group; }
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 if (command.GetArgumentCount() < 2) {
@@ -196,14 +240,24 @@ class CommandObjectMemoryTagWrite : public 
Com

[Lldb-commits] [PATCH] D104914: [lldb] Correct format of qMemTags type field

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

- Rebase onto main
- Update the way we cast down to int32_t to avoid use of static_cast.

We weren't invoking the undefined behaviour of unsigned to signed
due to the limit check. However I think this shows our intent more clearly.

It's essentially C++20's std::bitcast.

Also I was unable to reproduce the test failures,
ok to land this as is given that QMemTags has similair tests
and is already on main?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104914

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -470,19 +470,18 @@
 
 static void
 check_qmemtags(TestClient &client, MockServer &server, size_t read_len,
-   const char *packet, llvm::StringRef response,
+   int32_t type, const char *packet, llvm::StringRef response,
llvm::Optional> expected_tag_data) {
-  const auto &ReadMemoryTags = [&](size_t len, const char *packet,
-   llvm::StringRef response) {
+  const auto &ReadMemoryTags = [&]() {
 std::future result = std::async(std::launch::async, [&] {
-  return client.ReadMemoryTags(0xDEF0, read_len, 1);
+  return client.ReadMemoryTags(0xDEF0, read_len, type);
 });
 
 HandlePacket(server, packet, response);
 return result.get();
   };
 
-  auto result = ReadMemoryTags(0, packet, response);
+  auto result = ReadMemoryTags();
   if (expected_tag_data) {
 ASSERT_TRUE(result);
 llvm::ArrayRef expected(*expected_tag_data);
@@ -495,41 +494,53 @@
 
 TEST_F(GDBRemoteCommunicationClientTest, ReadMemoryTags) {
   // Zero length reads are valid
-  check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+  check_qmemtags(client, server, 0, 1, "qMemTags:def0,0:1", "m",
  std::vector{});
 
+  // Type can be negative. Put into the packet as the raw bytes
+  // (as opposed to a literal -1)
+  check_qmemtags(client, server, 0, -1, "qMemTags:def0,0:", "m",
+ std::vector{});
+  check_qmemtags(client, server, 0, std::numeric_limits::min(),
+ "qMemTags:def0,0:8000", "m", std::vector{});
+  check_qmemtags(client, server, 0, std::numeric_limits::max(),
+ "qMemTags:def0,0:7fff", "m", std::vector{});
+
   // The client layer does not check the length of the received data.
   // All we need is the "m" and for the decode to use all of the chars
-  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09",
+  check_qmemtags(client, server, 32, 2, "qMemTags:def0,20:2", "m09",
  std::vector{0x9});
 
   // Zero length response is fine as long as the "m" is present
-  check_qmemtags(client, server, 0, "qMemTags:def0,0:1", "m",
+  check_qmemtags(client, server, 0, 0x34, "qMemTags:def0,0:34", "m",
  std::vector{});
 
   // Normal responses
-  check_qmemtags(client, server, 16, "qMemTags:def0,10:1", "m66",
+  check_qmemtags(client, server, 16, 1, "qMemTags:def0,10:1", "m66",
  std::vector{0x66});
-  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m0102",
+  check_qmemtags(client, server, 32, 1, "qMemTags:def0,20:1", "m0102",
  std::vector{0x1, 0x2});
 
   // Empty response is an error
-  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "", llvm::None);
+  check_qmemtags(client, server, 17, 1, "qMemTags:def0,11:1", "", llvm::None);
   // Usual error response
-  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "E01", llvm::None);
+  check_qmemtags(client, server, 17, 1, "qMemTags:def0,11:1", "E01",
+ llvm::None);
   // Leading m missing
-  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "01", llvm::None);
+  check_qmemtags(client, server, 17, 1, "qMemTags:def0,11:1", "01", llvm::None);
   // Anything other than m is an error
-  check_qmemtags(client, server, 17, "qMemTags:def0,11:1", "z01", llvm::None);
+  check_qmemtags(client, server, 17, 1, "qMemTags:def0,11:1", "z01",
+ llvm::None);
   // Decoding tag data doesn't use all the chars in the packet
-  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "m09zz", llvm::None);
+  check_qmemtags(client, server, 32, 1, "qMemTags:def0,20:1", "m09zz",
+ llvm::None);
   // Data that is not hex bytes
-  check_qmemtags(client, server, 32, "qMemTags:def0,20:1", "mhello",
+  check_qmemtags(client, server, 32, 1, "qM

[Lldb-commits] [PATCH] D102757: [lldb] Remove non address bits when looking up memory regions

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

- Rebase onto main
- Add DoGetMemoryRegionInfo for ScriptedProcess

I'm going to build this and the follow up patch on Windows
to get some more coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102757

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/source/Plugins/Process/minidump/ProcessMinidump.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Target/Process.cpp
  lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
  
lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
  lldb/test/API/linux/aarch64/tagged_memory_region/main.c

Index: lldb/test/API/linux/aarch64/tagged_memory_region/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_region/main.c
@@ -0,0 +1,19 @@
+#include 
+#include 
+#include 
+#include 
+
+int main(int argc, char const *argv[]) {
+  void *the_page = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_EXEC,
+MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (the_page == MAP_FAILED)
+return 1;
+
+  // Put something in the top byte
+  the_page = (void *)((size_t)the_page | ((size_t)0x34 << 56));
+
+  // Then sign it
+  __asm__ __volatile__("pacdzb %0" : "=r"(the_page) : "r"(the_page));
+
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -0,0 +1,44 @@
+"""
+Test that "memory region" lookup removes the top byte and pointer
+authentication signatures of the address given.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTaggedMemoryRegionTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_mte_regions(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'])
+
+# Despite the non address bits we should find a region
+self.expect("memory region the_page", patterns=[
+"\[0x[0-9A-Fa-f]+-0x[0-9A-Fa-f]+\) r-x"])
Index: lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -march=armv8.3-a
+
+include Makefile.rules
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -5864,6 +5864,13 @@
   return retval;
 }
 
+Status Process::GetMemoryRegionInfo(lldb::addr_t load_addr,
+MemoryRegionInfo &range_info) {
+  if (auto abi = GetABI())
+load_addr = abi->FixDataAddress(load_addr);
+  return DoGetMemoryRegionInfo(load_addr, range_info);
+}
+
 Status
 Process::GetMemoryRegions(lldb_private::MemoryRegionInfos ®ion_list) {
 
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -86,9 +86,6 @@
 
   ArchSpec GetArchitecture();
 
-  Status GetMemo

[Lldb-commits] [PATCH] D103626: [lldb][AArch64] Remove non address bits from memory read arguments

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

Rebase onto main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103626

Files:
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
  
lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
  lldb/test/API/linux/aarch64/tagged_memory_read/main.c

Index: lldb/test/API/linux/aarch64/tagged_memory_read/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_read/main.c
@@ -0,0 +1,18 @@
+#include 
+
+static char *set_non_address_bits(char *ptr, size_t tag) {
+  // Set top byte tag
+  ptr = (char *)((size_t)ptr | (tag << 56));
+  // Sign it
+  __asm__ __volatile__("pacdzb %0" : "=r"(ptr) : "r"(ptr));
+  return ptr;
+}
+
+int main(int argc, char const *argv[]) {
+  char buf[32];
+
+  char *ptr1 = set_non_address_bits(buf, 0x34);
+  char *ptr2 = set_non_address_bits(buf, 0x56);
+
+  return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
@@ -0,0 +1,55 @@
+"""
+Test that "memory read" removes the top byte and pointer
+authentication signature of addresses before using them.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTaggedMemoryReadTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_mte_tagged_memory_read(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'])
+
+# If we do not remove non address bits, this can fail in two ways.
+# 1. We attempt to read much more than 16 bytes, probably more than
+#the default 1024 byte read size. (which is an error)
+# 2. We error because end address is < start address. Likely
+#because end's tag is < the beginning's tag.
+#
+# Each time we check that the printed line addresses do not include
+# either of the tags we set.
+tagged_addr_pattern = "0x(34|46)[0-9A-Fa-f]{14}:.*"
+self.expect("memory read ptr1 ptr2+16", patterns=[tagged_addr_pattern], matching=False)
+# Check that the stored previous end address is stripped
+self.expect("memory read", patterns=[tagged_addr_pattern], matching=False)
+self.expect("memory read ptr2 ptr1+16", patterns=[tagged_addr_pattern], matching=False)
+self.expect("memory read", patterns=[tagged_addr_pattern], matching=False)
Index: lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -march=armv8.3-a
+
+include Makefile.rules
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/ABI.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/MemoryHistory.h"
 #include "lldb/Target/MemoryRegionInfo.h"
@@ -590,9 +591,16 @@
   return false;
 }
 
+ABISP abi = m_exe_ctx.GetProcessPtr()->GetABI();
+if (abi)
+  addr = abi->FixDataAddress(addr);
+
 if (argc == 2) {
   lldb::addr_t end_addr = OptionArgParser::ToAddress(
   &m_exe_ctx, command[1].ref(), LLDB_INVALID_ADDRESS, nullptr);
+  if (end_addr != LLDB_INVALID_ADDRESS && abi)
+end_addr = abi->FixDataAddress(end_addr);
+
   if (end_addr == LLDB_INVALID_ADDRESS) {
 result.AppendError("invalid end address expression.");
 result.AppendError(erro

[Lldb-commits] [lldb] 83c752b - Revert "[lldb] Temporarily bump the max length of the pexpect error message to diagnose an lldb-aarch64 test failure"

2021-07-28 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2021-07-28T18:11:52+02:00
New Revision: 83c752bfa6071f34c8c4564a379d99b1b94f604a

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

LOG: Revert "[lldb] Temporarily bump the max length of the pexpect error 
message to diagnose an lldb-aarch64 test failure"

This reverts commit 5db8e232126fc4c0f5d5b0339bdc5a49830268d1. The test has
been disabled since then on the bot and we got the logs we wanted.

Added: 


Modified: 
lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py

Removed: 




diff  --git a/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py 
b/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py
index 8ef077aa7d31b..6b9ad3f63f7cd 100644
--- a/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py
+++ b/lldb/third_party/Python/module/pexpect-4.6/pexpect/pty_spawn.py
@@ -212,8 +212,8 @@ def __str__(self):
 s.append(repr(self))
 s.append('command: ' + str(self.command))
 s.append('args: %r' % (self.args,))
-s.append('buffer (last 1 chars): %r' % self.buffer[-1:])
-s.append('before (last 1 chars): %r' % self.before[-1:] if 
self.before else '')
+s.append('buffer (last 100 chars): %r' % self.buffer[-100:])
+s.append('before (last 100 chars): %r' % self.before[-100:] if 
self.before else '')
 s.append('after: %r' % (self.after,))
 s.append('match: %r' % (self.match,))
 s.append('match_index: ' + str(self.match_index))



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


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

2021-07-28 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.

IIRC the fact that the shared pointer is null is implementation defined and the 
standard just says that a moved-from object is in an undefined state.




Comment at: lldb/source/Target/ThreadPlanStack.cpp:145-147
+  // Note that `std::move` on `m_plans` elements can break its non-null
+  // invariant - if also calling functions (such as `WillPop`) that can't
+  // guarantee `m_plans` is not accessed in its inconsistent state.

Note that moving the top element of the vector leaves it in an undefined state, 
which breaks the thread plan stack guarantee that the last element is valid. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106171

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


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

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

Add setting to enable assert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

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

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -175,6 +175,9 @@
   def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
 DefaultFalse,
 Desc<"Enable debugging of LLDB-internal utility expressions.">;
+  def VerifyFileCacheMemoryReads: Property<"verify-file-cache-memory-reads", "Boolean">,
+DefaultFalse,
+Desc<"Verify that memory read from the file-cache is identical to the memory read from the process.">;
 }
 
 let Definition = "process_experimental" in {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1761,21 +1761,34 @@
   // Read from file cache if read-only section.
   if (!force_live_memory && resolved_addr.IsSectionOffset()) {
 SectionSP section_sp(resolved_addr.GetSection());
-if (section_sp) {
-  auto permissions = Flags(section_sp->GetPermissions());
-  bool is_readonly = !permissions.Test(ePermissionsWritable) &&
- permissions.Test(ePermissionsReadable);
-  if (is_readonly) {
-file_cache_bytes_read =
-ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-if (file_cache_bytes_read == dst_len)
-  return file_cache_bytes_read;
-else if (file_cache_bytes_read > 0) {
-  file_cache_read_buffer =
-  std::make_unique(file_cache_bytes_read);
-  std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+if (section_sp && section_sp->IsReadOnly()) {
+  file_cache_bytes_read =
+  ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+
+  if (GetVerifyFileCacheMemoryReads()) {
+if (ProcessIsValid() && file_cache_bytes_read == dst_len) {
+  if (load_addr == LLDB_INVALID_ADDRESS)
+load_addr = resolved_addr.GetLoadAddress(this);
+  if (load_addr != LLDB_INVALID_ADDRESS) {
+uint8_t *live_buf = (uint8_t *)malloc(dst_len);
+bytes_read =
+m_process_sp->ReadMemory(load_addr, live_buf, dst_len, error);
+if (bytes_read == dst_len) {
+  lldbassert(memcmp(live_buf, dst, dst_len) == 0 &&
+ "File cache and live memory diverge!");
+}
+free(live_buf);
+  }
 }
   }
+
+  if (file_cache_bytes_read == dst_len)
+return file_cache_bytes_read;
+  if (file_cache_bytes_read > 0) {
+file_cache_read_buffer =
+std::make_unique(file_cache_bytes_read);
+std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+  }
 }
   }
 
@@ -4368,6 +4381,17 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, debug);
 }
 
+bool TargetProperties::GetVerifyFileCacheMemoryReads() const {
+  const uint32_t idx = ePropertyVerifyFileCacheMemoryReads;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+}
+
+void TargetProperties::SetVerifyFileCacheMemoryReads(bool verify) {
+  const uint32_t idx = ePropertyVerifyFileCacheMemoryReads;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, verify);
+}
+
 // Target::TargetEventData
 
 Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp)
Index: lldb/source/Core/Section.cpp
===
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -599,3 +599,9 @@
   }
   return count;
 }
+
+bool Section::IsReadOnly() {
+  auto permissions = Flags(GetPermissions());
+  return !permissions.Test(ePermissionsWritable) &&
+ permissions.Test(ePermissionsReadable);
+}
Index: lldb/include/lldb/Target/Target.h
===
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -229,6 +229,10 @@
 
   bool GetDebugUtilityExpression() const;
 
+  void SetVerifyFileCacheMemoryReads(bool debug);
+
+  bool GetVerifyFileCacheMemoryReads() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
Index: lldb/include/lldb/Core/Section.h
===
--- lldb/include/lldb/Core/Section.h
+++ lldb/include/lldb/Core/Section.h
@@ -236,6 +236,8 @@
 
   void Se

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

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

Should this be enabled in the test suite? 
(`packages/Python/lldbsuite/test/lldbtest.py:783`, 
`test/Shell/lit-lldb-init.in`)




Comment at: lldb/source/Target/Target.cpp:1773
+  if (load_addr != LLDB_INVALID_ADDRESS) {
+uint8_t *live_buf = (uint8_t *)malloc(dst_len);
+bytes_read =

Can we use a unique_ptr here (`std::make_unique`)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

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


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

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

- Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106192

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

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

[Lldb-commits] [lldb] aad17c5 - [trace] Introduce Hierarchical Trace Representation (HTR) and add command for Intel PT trace visualization

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

Author: Walter Erquinigo
Date: 2021-07-28T11:04:13-07:00
New Revision: aad17c55a8116cd3831d4392d909139702019d65

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

LOG: [trace] Introduce Hierarchical Trace Representation (HTR) and add  command 
for Intel PT trace visualization

This diff introduces Hierarchical Trace Representation (HTR) and creates the 
`thread trace export ctf  -f  -t ` command to export an 
Intel PT trace's HTR to Chrome Trace Format (CTF) for visualization.

See `lldb/docs/htr.rst` for context/documentation on HTR.

**Overview of Changes**
- Add HTR documentation (see `lldb/docs/htr.rst`)
- Add HTR structures (layer, block, block metadata)
- Implement "Basic Super Block" HTR pass
- Add 'thread trace export ctf' command to export the HTR of an Intel PT
  trace to Chrome Trace Format (CTF)

As this diff is the first iteration of HTR and trace visualization, future 
diffs will build on this work by generalizing the internal design of HTR and 
implementing new HTR passes that provide better trace 
summarization/visualization.

See attached video for an example of Intel PT trace visualization:
{F17851042}

Original Author: jj10306

Submitted by: wallace

Reviewed By: wallace, clayborg

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

Added: 
lldb/docs/htr.rst
lldb/source/Plugins/TraceExporter/common/CMakeLists.txt
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Plugins/TraceExporter/common/TraceHTR.h
lldb/test/API/commands/trace/TestTraceExport.py
lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.cpp
lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.out

Modified: 
lldb/source/Plugins/TraceExporter/CMakeLists.txt
lldb/source/Plugins/TraceExporter/ctf/CMakeLists.txt
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
lldb/source/Plugins/TraceExporter/ctf/TraceExporterCTFOptions.td

Removed: 




diff  --git a/lldb/docs/htr.rst b/lldb/docs/htr.rst
new file mode 100644
index 0..fccf385914402
--- /dev/null
+++ b/lldb/docs/htr.rst
@@ -0,0 +1,47 @@
+Hierarchical Trace Representation (HTR)
+==
+The humongous amount of data processor traces like the ones obtained with 
Intel PT contain is not digestible to humans in its raw form. Given this, it is 
useful to summarize these massive traces by extracting useful information. 
Hierarchical Trace Representation (HTR) is the way lldb represents a summarized 
trace internally. HTR efficiently stores trace data and allows the trace data 
to be transformed in a way akin to compiler passes.
+
+Concepts
+
+**Block:** One or more contiguous units of the trace. At minimum, the unit of 
a trace is the load address of an instruction.
+
+**Block Metadata:** Metadata associated with each *block*. For processor 
traces, some metadata examples are the number of instructions in the block or 
information on what functions are called in the block.
+
+**Layer:** The representation of trace data between passes. For Intel PT there 
are two types of layers:
+
+ **Instruction Layer:** Composed of the oad addresses of the instructions in 
the trace. In an effort to save space, 
+ metadata is only stored for instructions that are of interest, not every 
instruction in the trace. HTR contains a 
+ single instruction layer.
+
+ **Block Layer:** Composed of blocks - a block in *layer n* refers to a 
sequence of blocks in *layer n - 1*. A block in 
+ *layer 1* refers to a sequence of instructions in *layer 0* (the instruction 
layer). Metadata is stored for each block in 
+ a block layer. HTR contains one or more block layers.
+
+**Pass:** A transformation applied to a *layer* that generates a new *layer* 
that is a more summarized, consolidated representation of the trace data.
+A pass merges instructions/blocks based on its specific purpose - for example, 
a pass designed to summarize a processor trace by function calls would merge 
all the blocks of a function into a single block representing the entire 
function.l
+
+The image below illusrates the transformation of a trace's representation (HTR)
+
+.. image:: media/htr-example.png
+
+Passes
+--
+A *pass* is applied to a *layer* to extract useful information (summarization) 
and compress the trace representation into a new *layer*. The idea is to have a 
series of passes where each pass specializes in extracting certain information 
about the trace. Some examples of potential passes include: identifying 
functions, identifying loops, or a more general purpose such as identifying 
long sequences of instructions that are repeated 

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

2021-07-28 Thread Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaad17c55a811: [trace] Introduce Hierarchical Trace 
Representation (HTR) and add  command for… (authored by Walter Erquinigo 
).

Changed prior to commit:
  https://reviews.llvm.org/D105741?vs=361916&id=362455#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105741

Files:
  lldb/docs/htr.rst
  lldb/source/Plugins/TraceExporter/CMakeLists.txt
  lldb/source/Plugins/TraceExporter/common/CMakeLists.txt
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.h
  lldb/source/Plugins/TraceExporter/ctf/CMakeLists.txt
  lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
  lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
  lldb/source/Plugins/TraceExporter/ctf/TraceExporterCTFOptions.td
  lldb/test/API/commands/trace/TestTraceExport.py
  lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.cpp
  lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.out

Index: lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.cpp
@@ -0,0 +1,34 @@
+void log_response(int reqest_response) {
+  // fake logging logic
+}
+
+int slow_handle_request(int id) {
+  // "slow" request handling logic
+  for (int i = 0; i < 10; i++)
+id += 2;
+  return id;
+}
+
+int fast_handle_request(int id) {
+  // "fast" request handling logic
+  return id + 2;
+}
+
+void iterative_handle_request_by_id(int id, int reps) {
+  int response;
+  for (int i = 0; i < reps; i++) {
+if (i % 2 == 0)
+  response = fast_handle_request(id);
+else
+  response = slow_handle_request(id);
+log_response(response);
+  }
+}
+
+int main() {
+  int n_requests = 10;
+  for (int id = 0; id < n_requests; id++) {
+iterative_handle_request_by_id(id, 3);
+  }
+  return 0;
+}
Index: lldb/test/API/commands/trace/TestTraceExport.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceExport.py
@@ -0,0 +1,109 @@
+from collections import defaultdict
+import lldb
+import json
+from intelpt_testcase import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+import os
+
+class TestTraceExport(TraceIntelPTTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def testErrorMessages(self):
+ctf_test_file = self.getBuildArtifact("ctf-test.json")
+# We first check the output when there are no targets
+self.expect(f"thread trace export ctf --file {ctf_test_file}",
+substrs=["error: invalid target, create a target using the 'target create' command"],
+error=True)
+
+# We now check the output when there's a non-running target
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+
+self.expect(f"thread trace export ctf --file {ctf_test_file}",
+substrs=["error: invalid process"],
+error=True)
+
+# Now we check the output when there's a running target without a trace
+self.expect("b main")
+self.expect("run")
+
+self.expect(f"thread trace export ctf --file {ctf_test_file}",
+substrs=["error: Process is not being traced"],
+error=True)
+
+def testExportCreatesFile(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+ctf_test_file = self.getBuildArtifact("ctf-test.json")
+
+if os.path.exists(ctf_test_file):
+remove_file(ctf_test_file)
+self.expect(f"thread trace export ctf --file {ctf_test_file}")
+self.assertTrue(os.path.exists(ctf_test_file))
+
+
+def testHtrBasicSuperBlockPass(self):
+'''
+Test the BasicSuperBlock pass of HTR
+
+TODO: Once the "trace save" command is implemented, gather Intel PT
+trace of this program and load it like the other tests instead of
+manually executing the commands to trace the program.
+'''
+self.expect(f"target create {os.path.join(self.getSourceDir(), 'intelpt-trace', 'export_ctf_test_program.out')}")
+self.expect("b main")
+self.expect("r")
+self.expect("b exit")
+self.expect("thread trace start")
+self.expect("c")
+
+ctf_test_file = self.getBuildArtifact("ctf-test.json")
+
+if os.path.exists(c

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

2021-07-28 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

Landed as aad17c55a8116cd3831d4392d909139702019d65 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105741

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


[Lldb-commits] [PATCH] D106984: [lldb] [gdb-remote] Add eOpenOptionReadWrite for future gdb compat

2021-07-28 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jasonmolenda, clayborg, 
JDevlieghere.
mgorny requested review of this revision.

Modify OpenOptions enum to open the future path into synchronizing
vFile:open bits with GDB.  Currently, LLDB and GDB use different flag
models effectively making it impossible to match bits.  Notably, LLDB
uses two bits to indicate read and write status, and uses union of both
for read/write.  GDB uses a value of 0 for read-only, 1 for write-only
and 2 for read/write.

In order to future-proof the code for the GDB variant:

1. Add a distinct eOpenOptionReadWrite constant to be used instead of 
(eOpenOptionRead | eOpenOptionWrite) when R/W access is required.

2. Rename eOpenOptionRead and eOpenOptionWrite to eOpenOptionReadOnly and 
eOpenOptionWriteOnly respectively, to make it clear that they do not mean to be 
combined and require update to all call sites.

3. Use the intersection of all three flags when matching against the three 
possible values.

This commit does not change the actual bits used by LLDB.


https://reviews.llvm.org/D106984

Files:
  lldb/docs/lldb-platform-packets.txt
  lldb/include/lldb/Host/File.h
  lldb/source/API/SBStream.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/StreamFile.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Host/windows/Host.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Target/ModuleCache.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/unittests/Host/FileSystemTest.cpp
  lldb/unittests/Host/FileTest.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -583,7 +583,7 @@
 
 TEST_F(PythonDataObjectsTest, TestPythonFile) {
   auto file = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
-  File::eOpenOptionRead);
+  File::eOpenOptionReadOnly);
   ASSERT_THAT_EXPECTED(file, llvm::Succeeded());
   auto py_file = PythonFile::FromFile(*file.get(), "r");
   ASSERT_THAT_EXPECTED(py_file, llvm::Succeeded());
@@ -858,4 +858,4 @@
   ASSERT_THAT_EXPECTED(r, llvm::Succeeded());
   auto g = As(globals.GetItem("g"));
   ASSERT_THAT_EXPECTED(g, llvm::HasValue("foobarbaz"));
-}
\ No newline at end of file
+}
Index: lldb/unittests/Host/FileTest.cpp
===
--- lldb/unittests/Host/FileTest.cpp
+++ lldb/unittests/Host/FileTest.cpp
@@ -46,7 +46,7 @@
   llvm::FileRemover remover(name);
   ASSERT_GE(fd, 0);
 
-  NativeFile file(fd, File::eOpenOptionWrite, true);
+  NativeFile file(fd, File::eOpenOptionWriteOnly, true);
   ASSERT_TRUE(file.IsValid());
 
   FILE *stream = file.GetStream();
Index: lldb/unittests/Host/FileSystemTest.cpp
===
--- lldb/unittests/Host/FileSystemTest.cpp
+++ lldb/unittests/Host/FileSystemTest.cpp
@@ -296,7 +296,7 @@
   FileSpec spec("/file/that/does/not/exist.txt");
 #endif
   FileSystem fs;
-  auto file = fs.Open(spec, File::eOpenOptionRead, 0, true);
+  auto file = fs.Open(spec, File::eOpenOptionReadOnly, 0, true);
   ASSERT_FALSE(file);
   std::error_code code = errorToErrorCode(file.takeError());
   EXPECT_EQ(code.category(), std::system_category());
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1022,7 +1022,7 @@
   }
 
   StreamFile out_file(path.c_str(),
-  File::eOpenOptionTruncate | File::eOpenOptionWrite |
+  File::eOpenOptionTruncate | File::eOpenOptionWriteOnly |
   File::eOp

[Lldb-commits] [PATCH] D106985: [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

2021-07-28 Thread Michał Górny via Phabricator via lldb-commits
mgorny created this revision.
mgorny added reviewers: krytarowski, labath, emaste, jasonmolenda, 
JDevlieghere, clayborg.
mgorny requested review of this revision.

Sync the mode constants used to drive vFile:open requests with these
used by GDB and defined for the gdb remote protocol.  This makes it
possible to use 'platform file open' after connecting to gdbremote
server (and to some degree to operate on the open file modulo other
incompatibilities).


https://reviews.llvm.org/D106985

Files:
  lldb/docs/lldb-platform-packets.txt
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp


Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -90,8 +90,8 @@
   .Cases("a+", "ab+", "a+b",
  eOpenOptionReadWrite | eOpenOptionAppend |
  eOpenOptionCanCreate)
-  .Default(OpenOptions());
-  if (opts)
+  .Default(eOpenOptionInvalid);
+  if (opts != eOpenOptionInvalid)
 return opts;
   return llvm::createStringError(
   llvm::inconvertibleErrorCode(),
Index: lldb/include/lldb/Host/File.h
===
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -39,27 +39,29 @@
   // NB this enum is used in the lldb platform gdb-remote packet
   // vFile:open: and existing values cannot be modified.
   //
-  // FIXME
-  // These values do not match the values used by GDB
+  // The first set of values is defined by gdb headers and can be found
+  // in the documentation at:
   // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-  // * rdar://problem/46788934
+  //
+  // The second half are LLDB extensions and use the highest uint32_t bits
+  // to avoid risk of collisions with future gdb remote protocol changes.
   enum OpenOptions : uint32_t {
-eOpenOptionReadOnly = (1u << 0),  // Open file for reading (only)
-eOpenOptionWriteOnly = (1u << 1), // Open file for writing (only)
-eOpenOptionReadWrite =
-eOpenOptionReadOnly |
-eOpenOptionWriteOnly, // Open file for both reading and writing
+eOpenOptionReadOnly = 0x0,  // Open file for reading (only)
+eOpenOptionWriteOnly = 0x1, // Open file for writing (only)
+eOpenOptionReadWrite = 0x2, // Open file for both reading and writing
 eOpenOptionAppend =
-(1u << 2), // Don't truncate file when opening, append to end of file
-eOpenOptionTruncate = (1u << 3),// Truncate file when opening
-eOpenOptionNonBlocking = (1u << 4), // File reads
-eOpenOptionCanCreate = (1u << 5),   // Create file if doesn't already exist
+0x8, // Don't truncate file when opening, append to end of file
+eOpenOptionCanCreate = 0x200, // Create file if doesn't already exist
+eOpenOptionTruncate = 0x400,  // Truncate file when opening
 eOpenOptionCanCreateNewOnly =
-(1u << 6), // Can create file only if it doesn't already exist
-eOpenOptionDontFollowSymlinks = (1u << 7),
+0x800, // Can create file only if it doesn't already exist
+
+eOpenOptionNonBlocking = (1u << 28), // File reads
+eOpenOptionDontFollowSymlinks = (1u << 29),
 eOpenOptionCloseOnExec =
-(1u << 8), // Close the file when executing a new process
-LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionCloseOnExec)
+(1u << 30), // Close the file when executing a new process
+eOpenOptionInvalid = (1u << 31), // Used as invalid value
+LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionInvalid)
   };
 
   static mode_t ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options);
Index: lldb/docs/lldb-platform-packets.txt
===
--- lldb/docs/lldb-platform-packets.txt
+++ lldb/docs/lldb-platform-packets.txt
@@ -372,12 +372,6 @@
 //  response is F followed by the opened file descriptor in base 10.
 //  "F-1,errno" with the errno if an error occurs.
 //
-//  COMPATIBILITY
-//The gdb-remote serial protocol documentatio defines a vFile:open:
-//packet which uses incompatible flag values, e.g. 1 means O_WRONLY
-//in gdb's vFile:open:, but it means eOpenOptionReadOnly to lldb's
-//implementation.
-
 //--
 // vFile:close:
 //


Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -90,8 +90,8 @@
   .Cases("a+", "ab+", "a+b",
  eOpenOptionReadWrite | eOpenOptionAppend |
  eOpenOptionCanCreate)
-  .Default(OpenOptions());
-  if (opts)
+  .Default(eOpenOptionInvalid);
+  if (opts != eOpenOptionInvalid)
 return opts;
   return llvm::createStringError(
   llvm::inconvertibleErr

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

2021-07-28 Thread Alex Langford via Phabricator via lldb-commits
bulbazord added a comment.

Thanks for the reviews and comments! It's kind of unfortunate that LLDB is like 
this right now but I believe this patch is a step in the right direction. :)




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

clayborg wrote:
> bulbazord wrote:
> > clayborg wrote:
> > > Maybe we should make a Language::FindPlugin(...) that like:
> > > ```
> > > Language *Language::FindPlugin(Mangled::ManglingScheme mangling_scheme);
> > > ```
> > > Should be easy to add since this change is kind of about refactoring and 
> > > putting the code into plug-ins. It is essentially what 
> > > "lldb::LanguageType Mangled::GuessLanguage() const" is doing. That code 
> > > could be moved to where Language::FindPlugin(...) lives.
> > `GuessLanguage` has a bunch of uses outside of `Mangled` itself, so I think 
> > that would make more sense if we performed other refactors first.
> > 
> > I don't think putting this into another `Language::FindPlugin` function is 
> > a good idea because (as I understand it) mangling schemes aren't specific 
> > to languages. 
> We _are_ using GuessLanguage to guess the language and we are using a 
> language specific plug-in to do the work. So effectively it is the same thing.
> 
> I wonder if GetName(...) should take an extra Language parameter? If a 
> lldb_private::Function wants the name as , then we _do_ know the language, 
> then it could send the language in for it to be more correct. Itanium name 
> mangling can mangle different languages right? Is there any way to 
> differentiate correctly without actually knowing the language?
Right, I understand. *Ideally* we wouldn't be doing that, but...

What you're suggesting might be better in the long run (using language 
information where available) but unfortunately I think that's far from where we 
are at currently. As for itanium name mangling, I don't think you can reliably 
determine the language the symbol came from unfortunately. Some languages have 
their own name mangling scheme luckily, so it may be easier for some languages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105215

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


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

2021-07-28 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev created this revision.
OmarEmaraDev added a reviewer: clayborg.
Herald added a reviewer: teemperor.
OmarEmaraDev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106999

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -1908,6 +1908,144 @@
   SelectionType m_selection_type;
 };
 
+class EnvironmentVariableNameFieldDelegate : public TextFieldDelegate {
+public:
+  EnvironmentVariableNameFieldDelegate(const char *label, const char *content)
+  : TextFieldDelegate(label, content, true) {}
+
+  // Environment variable names can't contain an equal sign.
+  bool IsAcceptableChar(int key) override {
+return TextFieldDelegate::IsAcceptableChar(key) && key != '=';
+  }
+
+  const std::string &GetName() { return m_content; }
+};
+
+class EnvironmentVariableFieldDelegate : public FieldDelegate {
+public:
+  EnvironmentVariableFieldDelegate()
+  : m_name_field(EnvironmentVariableNameFieldDelegate("Name", "")),
+m_value_field(TextFieldDelegate("Value", "", /*required=*/false)),
+m_selection_type(SelectionType::Name) {}
+
+  // Signify which element is selected. The variable name field or its value
+  // field.
+  enum class SelectionType { Name, Value };
+
+  // An environment variable field is drawn as two text fields with a right
+  // arrow in between. The first text field stores the name of the variable and
+  // the second stores the value if the variable.
+  //
+  // __[Name]   __[Value]___
+  // |  | > |  |
+  // |__|   |__|
+  // - Error message if it exists.
+
+  // The environment variable field has a height that is equal to the maximum
+  // height between the name and value fields.
+  int FieldDelegateGetHeight() override {
+return std::max(m_name_field.FieldDelegateGetHeight(),
+m_value_field.FieldDelegateGetHeight());
+  }
+
+  void DrawArrow(SubPad &surface) {
+surface.MoveCursor(0, 1);
+surface.PutChar(ACS_RARROW);
+  }
+
+  void FieldDelegateDraw(SubPad &surface, bool is_selected) override {
+Rect bounds = surface.GetFrame();
+Rect name_field_bounds, arrow_and_value_field_bounds;
+bounds.VerticalSplit(bounds.size.width / 2, name_field_bounds,
+ arrow_and_value_field_bounds);
+Rect arrow_bounds, value_field_bounds;
+arrow_and_value_field_bounds.VerticalSplit(1, arrow_bounds,
+   value_field_bounds);
+
+SubPad name_field_surface = SubPad(surface, name_field_bounds);
+SubPad arrow_surface = SubPad(surface, arrow_bounds);
+SubPad value_field_surface = SubPad(surface, value_field_bounds);
+
+bool name_is_selected =
+m_selection_type == SelectionType::Name && is_selected;
+m_name_field.FieldDelegateDraw(name_field_surface, name_is_selected);
+DrawArrow(arrow_surface);
+bool value_is_selected =
+m_selection_type == SelectionType::Value && is_selected;
+m_value_field.FieldDelegateDraw(value_field_surface, value_is_selected);
+  }
+
+  HandleCharResult SelectNext(int key) {
+if (FieldDelegateOnLastOrOnlyElement())
+  return eKeyNotHandled;
+
+m_selection_type = SelectionType::Value;
+m_name_field.FieldDelegateExitCallback();
+return eKeyHandled;
+  }
+
+  HandleCharResult SelectPrevious(int key) {
+if (FieldDelegateOnFirstOrOnlyElement())
+  return eKeyNotHandled;
+
+m_selection_type = SelectionType::Name;
+m_value_field.FieldDelegateExitCallback();
+return eKeyHandled;
+  }
+
+  HandleCharResult FieldDelegateHandleChar(int key) override {
+switch (key) {
+case '\t':
+  SelectNext(key);
+  return eKeyHandled;
+case KEY_SHIFT_TAB:
+  SelectPrevious(key);
+  return eKeyHandled;
+default:
+  break;
+}
+
+// If the key wasn't handled, pass the key to the selected field.
+if (m_selection_type == SelectionType::Name)
+  return m_name_field.FieldDelegateHandleChar(key);
+else
+  return m_value_field.FieldDelegateHandleChar(key);
+
+return eKeyNotHandled;
+  }
+
+  bool FieldDelegateOnFirstOrOnlyElement() override {
+return m_selection_type == SelectionType::Name;
+  }
+
+  bool FieldDelegateOnLastOrOnlyElement() override {
+return m_selection_type == SelectionType::Value;
+  }
+
+  void FieldDelegateSelectFirstElement() override {
+m_selection_type = SelectionType::Name;
+  }
+
+  void FieldDelegateSelectLastElement() override {
+m_selec

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

2021-07-28 Thread Omar Emara via Phabricator via lldb-commits
OmarEmaraDev added a comment.
Herald added a subscriber: JDevlieghere.

Example list of environment fields:

F18194751: 20210728-221522.png <https://reviews.llvm.org/F18194751>


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106999

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


[Lldb-commits] [lldb] 4b88a94 - Revert "[trace] Introduce Hierarchical Trace Representation (HTR) and add command for Intel PT trace visualization"

2021-07-28 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2021-07-29T01:34:24+05:00
New Revision: 4b88a94ebe08054ad88435cc89aaa3b84e41b938

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

LOG: Revert "[trace] Introduce Hierarchical Trace Representation (HTR) and add  
command for Intel PT trace visualization"

This reverts commit aad17c55a8116cd3831d4392d909139702019d65.

Breaks LLDB build on 32 bit Arm/Linux bot:
https://lab.llvm.org/buildbot/#/builders/17/builds/9497

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

Added: 


Modified: 
lldb/source/Plugins/TraceExporter/CMakeLists.txt
lldb/source/Plugins/TraceExporter/ctf/CMakeLists.txt
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
lldb/source/Plugins/TraceExporter/ctf/TraceExporterCTFOptions.td

Removed: 
lldb/docs/htr.rst
lldb/source/Plugins/TraceExporter/common/CMakeLists.txt
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Plugins/TraceExporter/common/TraceHTR.h
lldb/test/API/commands/trace/TestTraceExport.py
lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.cpp
lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.out



diff  --git a/lldb/docs/htr.rst b/lldb/docs/htr.rst
deleted file mode 100644
index fccf385914402..0
--- a/lldb/docs/htr.rst
+++ /dev/null
@@ -1,47 +0,0 @@
-Hierarchical Trace Representation (HTR)
-==
-The humongous amount of data processor traces like the ones obtained with 
Intel PT contain is not digestible to humans in its raw form. Given this, it is 
useful to summarize these massive traces by extracting useful information. 
Hierarchical Trace Representation (HTR) is the way lldb represents a summarized 
trace internally. HTR efficiently stores trace data and allows the trace data 
to be transformed in a way akin to compiler passes.
-
-Concepts
-
-**Block:** One or more contiguous units of the trace. At minimum, the unit of 
a trace is the load address of an instruction.
-
-**Block Metadata:** Metadata associated with each *block*. For processor 
traces, some metadata examples are the number of instructions in the block or 
information on what functions are called in the block.
-
-**Layer:** The representation of trace data between passes. For Intel PT there 
are two types of layers:
-
- **Instruction Layer:** Composed of the oad addresses of the instructions in 
the trace. In an effort to save space, 
- metadata is only stored for instructions that are of interest, not every 
instruction in the trace. HTR contains a 
- single instruction layer.
-
- **Block Layer:** Composed of blocks - a block in *layer n* refers to a 
sequence of blocks in *layer n - 1*. A block in 
- *layer 1* refers to a sequence of instructions in *layer 0* (the instruction 
layer). Metadata is stored for each block in 
- a block layer. HTR contains one or more block layers.
-
-**Pass:** A transformation applied to a *layer* that generates a new *layer* 
that is a more summarized, consolidated representation of the trace data.
-A pass merges instructions/blocks based on its specific purpose - for example, 
a pass designed to summarize a processor trace by function calls would merge 
all the blocks of a function into a single block representing the entire 
function.l
-
-The image below illusrates the transformation of a trace's representation (HTR)
-
-.. image:: media/htr-example.png
-
-Passes
---
-A *pass* is applied to a *layer* to extract useful information (summarization) 
and compress the trace representation into a new *layer*. The idea is to have a 
series of passes where each pass specializes in extracting certain information 
about the trace. Some examples of potential passes include: identifying 
functions, identifying loops, or a more general purpose such as identifying 
long sequences of instructions that are repeated (i.e. Basic Super Block). 
Below you will find a description of each pass currently implemented in lldb.
-
-**Basic Super Block Reduction**
-
-A “basic super block” is the longest sequence of blocks that always occur in 
the same order. (The concept is akin to “Basic Block'' in compiler theory, but 
refers to dynamic occurrences rather than CFG nodes).
-
-The image below shows the "basic super blocks" of the sequence. Each unique 
"basic super block" is marked with a 
diff erent color
-
-.. image:: media/basic_super_block_pass.png
-
-*Procedure to find all super blocks:*
-
-- For each block, compute the number of distinct predecessor and successor 
blocks.
-
- - **Predecessor** - the block that occurs directly before (to the left of) 
the current block
- - **Successor** 

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

2021-07-28 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

In D105741#2910908 , @wallace wrote:

> Landed as aad17c55a8116cd3831d4392d909139702019d65 
> 

Temporarily reverted this change as it breaks LLDB build on 32 bit Arm/Linux 
bot:
https://lab.llvm.org/buildbot/#/builders/17/builds/9497


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105741

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


[Lldb-commits] [PATCH] D106985: [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

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

Add a test for correct flag encoding.


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

https://reviews.llvm.org/D106985

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

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

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

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

Run file-cache verification on tests, change raw buffer to unique pointer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

Files:
  lldb/include/lldb/Core/Section.h
  lldb/include/lldb/Target/Target.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/Section.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td

Index: lldb/source/Target/TargetProperties.td
===
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -175,6 +175,9 @@
   def DebugUtilityExpression: Property<"debug-utility-expression", "Boolean">,
 DefaultFalse,
 Desc<"Enable debugging of LLDB-internal utility expressions.">;
+  def VerifyFileCacheMemoryReads: Property<"verify-file-cache-memory-reads", "Boolean">,
+DefaultFalse,
+Desc<"Verify that memory read from the file-cache is identical to the memory read from the process.">;
 }
 
 let Definition = "process_experimental" in {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1761,21 +1761,34 @@
   // Read from file cache if read-only section.
   if (!force_live_memory && resolved_addr.IsSectionOffset()) {
 SectionSP section_sp(resolved_addr.GetSection());
-if (section_sp) {
-  auto permissions = Flags(section_sp->GetPermissions());
-  bool is_readonly = !permissions.Test(ePermissionsWritable) &&
- permissions.Test(ePermissionsReadable);
-  if (is_readonly) {
-file_cache_bytes_read =
-ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-if (file_cache_bytes_read == dst_len)
-  return file_cache_bytes_read;
-else if (file_cache_bytes_read > 0) {
-  file_cache_read_buffer =
-  std::make_unique(file_cache_bytes_read);
-  std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+if (section_sp && section_sp->IsReadOnly()) {
+  file_cache_bytes_read =
+  ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+
+  if (GetVerifyFileCacheMemoryReads()) {
+if (ProcessIsValid() && file_cache_bytes_read == dst_len) {
+  if (load_addr == LLDB_INVALID_ADDRESS)
+load_addr = resolved_addr.GetLoadAddress(this);
+  if (load_addr != LLDB_INVALID_ADDRESS) {
+std::unique_ptr live_buf =
+std::make_unique(dst_len);
+bytes_read = m_process_sp->ReadMemory(load_addr, live_buf.get(),
+  dst_len, error);
+if (bytes_read == dst_len) {
+  lldbassert(memcmp(live_buf.get(), dst, dst_len) == 0 &&
+ "File cache and live memory diverge!");
+}
+  }
 }
   }
+
+  if (file_cache_bytes_read == dst_len)
+return file_cache_bytes_read;
+  if (file_cache_bytes_read > 0) {
+file_cache_read_buffer =
+std::make_unique(file_cache_bytes_read);
+std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+  }
 }
   }
 
@@ -4368,6 +4381,17 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, debug);
 }
 
+bool TargetProperties::GetVerifyFileCacheMemoryReads() const {
+  const uint32_t idx = ePropertyVerifyFileCacheMemoryReads;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+}
+
+void TargetProperties::SetVerifyFileCacheMemoryReads(bool verify) {
+  const uint32_t idx = ePropertyVerifyFileCacheMemoryReads;
+  m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, verify);
+}
+
 // Target::TargetEventData
 
 Target::TargetEventData::TargetEventData(const lldb::TargetSP &target_sp)
Index: lldb/source/Core/Section.cpp
===
--- lldb/source/Core/Section.cpp
+++ lldb/source/Core/Section.cpp
@@ -599,3 +599,9 @@
   }
   return count;
 }
+
+bool Section::IsReadOnly() {
+  auto permissions = Flags(GetPermissions());
+  return !permissions.Test(ePermissionsWritable) &&
+ permissions.Test(ePermissionsReadable);
+}
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -798,6 +798,9 @@
 'settings set symbols.clang-modules-cache-path "{}"'.format(
 configuration.lldb_module_cache_dir),
 "settings set use-color false",
+
+# Verify that file cache and live memory always match.
+   

[Lldb-commits] [PATCH] D106985: [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

2021-07-28 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Thanks for fixing this, it's long overdue.  For a long time I thought we'd need 
to operate in a compatibility mode to accept either set of constants, but I 
finally realized I was wrong and we should just align ourselves with the 
correct gdb behavior, but hadn't made the change myself yet.


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

https://reviews.llvm.org/D106985

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


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

2021-07-28 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Looks great!  Thanks for doing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

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


[Lldb-commits] [lldb] d52ba48 - [trace] Introduce Hierarchical Trace Representation (HTR) and add command for Intel PT trace visualization

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

Author: Walter Erquinigo
Date: 2021-07-28T13:56:45-07:00
New Revision: d52ba48821301c33650b97cd9f262615fcc0fdc1

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

LOG: [trace] Introduce Hierarchical Trace Representation (HTR) and add  command 
for Intel PT trace visualization

This diff introduces Hierarchical Trace Representation (HTR) and creates the 
`thread trace export ctf  -f  -t ` command to export an 
Intel PT trace's HTR to Chrome Trace Format (CTF) for visualization.

See `lldb/docs/htr.rst` for context/documentation on HTR.

**Overview of Changes**
- Add HTR documentation (see `lldb/docs/htr.rst`)
- Add HTR structures (layer, block, block metadata)
- Implement "Basic Super Block" HTR pass
- Add 'thread trace export ctf' command to export the HTR of an Intel PT
  trace to Chrome Trace Format (CTF)

As this diff is the first iteration of HTR and trace visualization, future 
diffs will build on this work by generalizing the internal design of HTR and 
implementing new HTR passes that provide better trace 
summarization/visualization.

See attached video for an example of Intel PT trace visualization:
{F17851042}

Original Author: jj10306

Submitted by: wallace

Reviewed By: wallace, clayborg

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

Added: 
lldb/docs/htr.rst
lldb/source/Plugins/TraceExporter/common/CMakeLists.txt
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Plugins/TraceExporter/common/TraceHTR.h
lldb/test/API/commands/trace/TestTraceExport.py
lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.cpp
lldb/test/API/commands/trace/intelpt-trace/export_ctf_test_program.out

Modified: 
lldb/source/Plugins/TraceExporter/CMakeLists.txt
lldb/source/Plugins/TraceExporter/ctf/CMakeLists.txt
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp
lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.h
lldb/source/Plugins/TraceExporter/ctf/TraceExporterCTFOptions.td

Removed: 




diff  --git a/lldb/docs/htr.rst b/lldb/docs/htr.rst
new file mode 100644
index 0..fccf385914402
--- /dev/null
+++ b/lldb/docs/htr.rst
@@ -0,0 +1,47 @@
+Hierarchical Trace Representation (HTR)
+==
+The humongous amount of data processor traces like the ones obtained with 
Intel PT contain is not digestible to humans in its raw form. Given this, it is 
useful to summarize these massive traces by extracting useful information. 
Hierarchical Trace Representation (HTR) is the way lldb represents a summarized 
trace internally. HTR efficiently stores trace data and allows the trace data 
to be transformed in a way akin to compiler passes.
+
+Concepts
+
+**Block:** One or more contiguous units of the trace. At minimum, the unit of 
a trace is the load address of an instruction.
+
+**Block Metadata:** Metadata associated with each *block*. For processor 
traces, some metadata examples are the number of instructions in the block or 
information on what functions are called in the block.
+
+**Layer:** The representation of trace data between passes. For Intel PT there 
are two types of layers:
+
+ **Instruction Layer:** Composed of the oad addresses of the instructions in 
the trace. In an effort to save space, 
+ metadata is only stored for instructions that are of interest, not every 
instruction in the trace. HTR contains a 
+ single instruction layer.
+
+ **Block Layer:** Composed of blocks - a block in *layer n* refers to a 
sequence of blocks in *layer n - 1*. A block in 
+ *layer 1* refers to a sequence of instructions in *layer 0* (the instruction 
layer). Metadata is stored for each block in 
+ a block layer. HTR contains one or more block layers.
+
+**Pass:** A transformation applied to a *layer* that generates a new *layer* 
that is a more summarized, consolidated representation of the trace data.
+A pass merges instructions/blocks based on its specific purpose - for example, 
a pass designed to summarize a processor trace by function calls would merge 
all the blocks of a function into a single block representing the entire 
function.l
+
+The image below illusrates the transformation of a trace's representation (HTR)
+
+.. image:: media/htr-example.png
+
+Passes
+--
+A *pass* is applied to a *layer* to extract useful information (summarization) 
and compress the trace representation into a new *layer*. The idea is to have a 
series of passes where each pass specializes in extracting certain information 
about the trace. Some examples of potential passes include: identifying 
functions, identifying loops, or a more general purpose such as identifying 
long sequences of instructions that are repeated 

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

2021-07-28 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid added a comment.

This change has caused various GUI related tests to fail randomly. 
https://lab.llvm.org/buildbot/#/builders/96/builds/10100

This also causes random failures of one of the most resource sufficient 
buildbot here:
https://lab.llvm.org/buildbot/#/builders/68/builds/16275
https://lab.llvm.org/buildbot/#/builders/68/builds/16300

@teemperor 
Referring to your comment on skipping TestGuiBasicDebug.py this may have caused 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

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


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

2021-07-28 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.

Looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106192

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


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

2021-07-28 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1924
+
+class EnvironmentVariableFieldDelegate : public FieldDelegate {
+public:

There are other things that could benefit from having two text fields, like 
source mapping:
```
(lldb) apropos target.source-map
No commands found pertaining to 'target.source-map'. Try 'help' to see a 
complete list of debugger commands.

The following settings variables may relate to 'target.source-map':
  target.source-map -- Source path remappings apply substitutions to the paths 
of source files, typically needed to debug from a different host than the one 
that built the
   target.  The source-map property consists of an array of 
pairs, the first element is a path prefix, and the second is its replacement.  
The syntax is
   `prefix1 replacement1 prefix2 replacement2...`.  The 
pairs are checked in order, the first prefix that matches is used, and that 
prefix is substituted
   with the replacement.  A common pattern is to use 
source-map in conjunction with the clang -fdebug-prefix-map flag.  In the 
build, use
   `-fdebug-prefix-map=/path/to/build_dir=.` to rewrite the 
host specific build directory to `.`.  Then for debugging, use `settings set 
target.source-map .
   /path/to/local_dir` to convert `.` to a valid local path.
```

This would be very close to the EnvironmentVariableFieldDelegate class, but 
instead of "Name" and "Value" the fields would be "Prefix" and "Replacement". 
In the source map case the two fields would be a DirectoryFieldDelegate 
objects. So we could make a base class like PairFieldDelegate that much of the 
functionality in this class could be moved to. The constructor for the 
PairFieldDelegate could take two "FieldDelegate *" arguments. So then 
EnvironmentVariableFieldDelegate could inherit from PairFieldDelegate and this 
constructor would look like:

```
EnvironmentVariableFieldDelegate(): 
  PairFieldDelegate(new EnvironmentVariableNameFieldDelegate("Name", ""),
   new TextFieldDelegate("Value", "", 
/*required=*/false))
```
Not required, but it might make it a more useful set of reusable classes for 
this curses field stuff. Let me know if you are up for doing an approach like 
this or not


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106999

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


[Lldb-commits] [PATCH] D107015: Add "current" token for the -t option to "break set/modify"

2021-07-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
jingham added reviewers: kastiglione, JDevlieghere.
Herald added a subscriber: dang.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It's fairly common to want to set a breakpoint that is limited to the current 
thread.  For instance, if you hit function A on thread 1, then you want to set 
a breakpoint on function B but only when this thread hits it.  You can do this 
in Python, but it should be simple to do this just from the command line.  With 
this patch you can do:

break set -t current -n foo

or something like that.

This actually uses the thread from the ExecutionContext passed to the 
CommandObjectBreakpointSet::DoExecute, not the currently selected thread, so it 
will work in breakpoint commands as well as from the command line.  That's the 
right thing to do.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107015

Files:
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/Options.td
  
lldb/test/API/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py


Index: 
lldb/test/API/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
===
--- 
lldb/test/API/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
+++ 
lldb/test/API/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
@@ -9,11 +9,15 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-def set_thread_id(thread, breakpoint):
+def using_current(test, thread, breakpoint):
+bp_id = breakpoint.GetID()
+test.runCmd("break modify -t current {0}".format(bp_id))
+
+def set_thread_id(test, thread, breakpoint):
 id = thread.id
 breakpoint.SetThreadID(id)
 
-def set_thread_name(thread, breakpoint):
+def set_thread_name(test, thread, breakpoint):
 breakpoint.SetThreadName("main-thread")
 
 class ThreadSpecificBreakTestCase(TestBase):
@@ -32,6 +36,10 @@
 def test_thread_name(self):
 self.do_test(set_thread_name)
 
+@expectedFailureAll(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], 
archs=['armv7', 'armv7k'], bugnumber='rdar://problem/34563920') # armv7 ios 
problem - breakpoint with tid qualifier isn't working
+def test_current_token(self):
+self.do_test(using_current)
+
 def do_test(self, setter_method):
 """Test that we obey thread conditioned breakpoints."""
 self.build()
@@ -51,7 +59,7 @@
 # thread joins the secondary thread, and then the main thread will
 # execute the code at the breakpoint.  If the thread-specific
 # breakpoint works, the next stop will be on the main thread.
-setter_method(main_thread, thread_breakpoint)
+setter_method(self, main_thread, thread_breakpoint)
 
 process.Continue()
 next_stop_state = process.GetState()
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -73,7 +73,7 @@
 "index matches this argument.">;
   def breakpoint_modify_thread_id : Option<"thread-id", "t">, Group<1>,
 Arg<"ThreadID">, Desc<"The breakpoint stops only for the thread whose TID "
-"matches this argument.">;
+"matches this argument.  The token 'current' resolves to the current 
thread's ID.">;
   def breakpoint_modify_thread_name : Option<"thread-name", "T">, Group<1>,
 Arg<"ThreadName">, Desc<"The breakpoint stops only for the thread whose "
 "thread name matches this argument.">;
Index: lldb/source/Commands/CommandObjectBreakpoint.cpp
===
--- lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -110,7 +110,19 @@
 case 't': {
   lldb::tid_t thread_id = LLDB_INVALID_THREAD_ID;
   if (option_arg[0] != '\0') {
-if (option_arg.getAsInteger(0, thread_id))
+if (option_arg == "current") {
+  if (!execution_context) {
+error.SetErrorStringWithFormat("No context to determine current "
+   "thread");
+  } else {
+ThreadSP ctx_thread_sp = execution_context->GetThreadSP();
+if (!ctx_thread_sp || !ctx_thread_sp->IsValid()) {
+  error.SetErrorStringWithFormat("No currently selected thread");
+} else {
+  thread_id = ctx_thread_sp->GetID();
+}
+  }
+} else if (option_arg.getAsInteger(0, thread_id))
   error.SetErrorStringWithFormat("invalid thread id string '%s'",
  option_arg.str().c_str());
   }


Index: lldb/test/API/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
=

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

2021-07-28 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment.

In D106171#2910693 , @JDevlieghere 
wrote:

> IIRC the fact that the shared pointer is null is implementation defined and 
> the standard just says that a moved-from object is in an undefined state.

According to cppreference, the move constructor says:

> After the construction, `*this` contains a copy of the previous state of `r`, 
> **`r` is empty and its stored pointer is null**.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106171

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


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

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

Seems like this is still missing form `lit-lldb-init.in` which means the shell 
tests (and by extension the Swift REPL tests downstream) will run with the 
setting off. Other than that this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584

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