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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/include/lldb/Target/MemoryTagManager.h:35
+  // alignment/expansion over again.
+  struct TagManagerWithRange {
+const MemoryTagManager *manager;

omjavaid wrote:
> I was wondering if you can explain reason for hosting this struct. Is there a 
> association between  MemoryTagManager and Tag Range. 
> 
> I think same tag manager was associated with the whole of process address 
> space? so why host tag manager pointer along with the range when we already 
> have a pointer to process. This implies there could be different tag managers 
> for different ranges? Our initial implementation introduced per architecture 
> tag manager and for Process AArch64 we can use AArch64 Tag Manager for all 
> our tag ranges. This appears to have over complicated range expansion.
> 
> 
The comment here is a bit misleading re. the "validity" of the manager. The 
manager itself is valid if you've got tagging, wherever it might be, you're 
right there.

The commit message has a better justification:
```
To save the commands then redoing that alignment
both memory tag manager methods now return a struct
of the range we checked and the manager itself.
```

So this is not "this manager is valid for this range", it's "here's a manager 
and an aligned range". Since we have to align the original range, then check 
the memory flags, then finally return the manager. Might as well give them the 
range too.

I agree that the comment here doesn't explain that well, I'll make it more 
precise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105181

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

[Lldb-commits] [PATCH] D105179: [lldb][AArch64] Add tag repeat and unpack to memory tag manager

[David Spickett via Phabricator via lldb-commits]

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

- Make granules default to 0 for UnpackTagsData
- Add more comments to tests

(I'll add a method to repeat packed tags in the later patch
that'll use it)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105179

Files:
  lldb/include/lldb/Target/MemoryTagManager.h
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp
  lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.h
  lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp

Index: lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
===
--- lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
+++ lldb/unittests/Process/Utility/MemoryTagManagerAArch64MTETest.cpp
@@ -21,7 +21,7 @@
   manager.UnpackTagsData(input, 2),
   llvm::FailedWithMessage(
   "Packed tag data size does not match expected number of tags. "
-  "Expected 2 tag(s) for 2 granules, got 0 tag(s)."));
+  "Expected 2 tag(s) for 2 granule(s), got 0 tag(s)."));
 
   // This is out of the valid tag range
   input.push_back(0x1f);
@@ -41,6 +41,43 @@
   manager.UnpackTagsData(input, 2);
   ASSERT_THAT_EXPECTED(got, llvm::Succeeded());
   ASSERT_THAT(expected, testing::ContainerEq(*got));
+
+  // Error for too much tag data
+  ASSERT_THAT_EXPECTED(
+  manager.UnpackTagsData(input, 1),
+  llvm::FailedWithMessage(
+  "Packed tag data size does not match expected number of tags. "
+  "Expected 1 tag(s) for 1 granule(s), got 2 tag(s)."));
+
+  // By default, we don't check number of tags
+  llvm::Expected> got_zero =
+  manager.UnpackTagsData(input);
+  ASSERT_THAT_EXPECTED(got_zero, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*got));
+
+  // Which is the same as granules=0
+  got_zero = manager.UnpackTagsData(input, 0);
+  ASSERT_THAT_EXPECTED(got_zero, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*got));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, PackTags) {
+  MemoryTagManagerAArch64MTE manager;
+
+  // Error for tag out of range
+  llvm::Expected> invalid_tag_err =
+  manager.PackTags({0x10});
+  ASSERT_THAT_EXPECTED(
+  invalid_tag_err,
+  llvm::FailedWithMessage(
+  "Found tag 0x10 which is > max MTE tag value of 0xf."));
+
+  // 0xf here is the max tag value that we can pack
+  std::vector tags{0, 1, 0xf};
+  std::vector expected{0, 1, 0xf};
+  llvm::Expected> packed = manager.PackTags(tags);
+  ASSERT_THAT_EXPECTED(packed, llvm::Succeeded());
+  ASSERT_THAT(expected, testing::ContainerEq(*packed));
 }
 
 TEST(MemoryTagManagerAArch64MTETest, GetLogicalTag) {
@@ -118,3 +155,53 @@
   ASSERT_EQ(-32, manager.AddressDiff(0x55114400, 0x44114420));
   ASSERT_EQ(65, manager.AddressDiff(0x99114441, 0x66114400));
 }
+
+// Helper to check that repeating "tags" over "range" gives you
+// "expected_tags".
+static void
+test_repeating_tags(const std::vector &tags,
+MemoryTagManagerAArch64MTE::TagRange range,
+const std::vector &expected_tags) {
+  MemoryTagManagerAArch64MTE manager;
+  llvm::Expected> tags_or_err =
+  manager.RepeatTagsForRange(tags, range);
+  ASSERT_THAT_EXPECTED(tags_or_err, llvm::Succeeded());
+  ASSERT_THAT(expected_tags, testing::ContainerEq(*tags_or_err));
+}
+
+TEST(MemoryTagManagerAArch64MTETest, RepeatTagsForRange) {
+  MemoryTagManagerAArch64MTE manager;
+
+  // Must have some tags if your range is not empty
+  llvm::Expected> no_tags_err =
+  manager.RepeatTagsForRange({},
+ MemoryTagManagerAArch64MTE::TagRange{0, 16});
+  ASSERT_THAT_EXPECTED(
+  no_tags_err, llvm::FailedWithMessage(
+   "Expected some tags to cover given range, got zero."));
+
+  // If the range is empty, you get no tags back
+  test_repeating_tags({1, 2, 3}, MemoryTagManagerAArch64MTE::TagRange{0, 0},
+  {});
+  // And you don't need tags for an empty range
+  test_repeating_tags({}, MemoryTagManagerAArch64MTE::TagRange{0, 0}, {});
+
+  // A single tag will just be multiplied as many times as needed
+  test_repeating_tags({5}, MemoryTagManagerAArch64MTE::TagRange{0, 16}, {5});
+  test_repeating_tags({6}, MemoryTagManagerAArch64MTE::TagRange{0, 32}, {6, 6});
+
+  // If you've got as many tags as granules, it's a roundtrip
+  test_repeating_tags({7, 8}, MemoryTagManagerAArch64MTE::TagRange{0, 32},
+  {7, 8});
+
+  // If you've got fewer tags than granules, they repeat. Exactly or partially
+  // as needed.
+  test_repeating_tags({7, 8}, MemoryTagManagerAArch64MTE::TagRange{0, 64},
+  {7, 8, 7, 8});
+  test_repeating_tags({7, 8}, MemoryTagManagerAArch64MTE::TagRange{0, 48},
+  {7, 8, 7}

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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/include/lldb/Target/MemoryTagManager.h:35
+  // alignment/expansion over again.
+  struct TagManagerWithRange {
+const MemoryTagManager *manager;

DavidSpickett wrote:
> omjavaid wrote:
> > I was wondering if you can explain reason for hosting this struct. Is there 
> > a association between  MemoryTagManager and Tag Range. 
> > 
> > I think same tag manager was associated with the whole of process address 
> > space? so why host tag manager pointer along with the range when we already 
> > have a pointer to process. This implies there could be different tag 
> > managers for different ranges? Our initial implementation introduced per 
> > architecture tag manager and for Process AArch64 we can use AArch64 Tag 
> > Manager for all our tag ranges. This appears to have over complicated range 
> > expansion.
> > 
> > 
> The comment here is a bit misleading re. the "validity" of the manager. The 
> manager itself is valid if you've got tagging, wherever it might be, you're 
> right there.
> 
> The commit message has a better justification:
> ```
> To save the commands then redoing that alignment
> both memory tag manager methods now return a struct
> of the range we checked and the manager itself.
> ```
> 
> So this is not "this manager is valid for this range", it's "here's a manager 
> and an aligned range". Since we have to align the original range, then check 
> the memory flags, then finally return the manager. Might as well give them 
> the range too.
> 
> I agree that the comment here doesn't explain that well, I'll make it more 
> precise.
The question I have is why we have to return the manager pointer with our 
expanded ranges. Tag ranges are per-process rather than per-tag-manager and we 
can get tag manager by keeping a reference to it in the process class then why 
return a manager alongside the expanded range.





Comment at: lldb/source/Target/Process.cpp:6089
+llvm::Expected
+Process::GetMemoryTagManagerForGranules(lldb::addr_t addr, size_t granules) {
+  return GetMemoryTagManagerImpl(

Main operation of this function is doing the expansion of ranges according to 
granule while the function name suggests that we are getting memory unique tag 
manager for addr, granules. Name doesnt even suggest that we are actually here 
to do range alignment/expansion.



Comment at: lldb/source/Target/Process.cpp:6108
+range_callback) {
   Architecture *arch = GetTarget().GetArchitecturePlugin();
   const MemoryTagManager *tag_manager =

The point I was trying to establish above is that GetMemoryTagManagerImpl 
function here may remain as it was i-e GetMemoryTagManager. It could only run 
once on Process object creation. We know our process architecture so we can 
host a pointer to relevent tag manager in Process class. All the tag ranges 
corresponding to the current process address space should be able to get a 
pointer to MemoryTagManager as was being done previously. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105181

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

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py:56
 # Run the packet stream
 context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)

omjavaid wrote:
> This test is still not stable given we wait for stop notification while 
> process may have exited. This causes test to hang till timeout expiry.
Sure, but I don't think there's a better way to do it. Short of reading from a 
process that's already exited. (which seems to sort of work for memory but tags 
have weird results so I don't think it's meant to work)

Note that the test file does pause after the print: 
https://github.com/llvm/llvm-project/blob/main/lldb/test/API/tools/lldb-server/memory-tagging/main.c#L16

We've got a few scenarios:
* All goes well, we get the print. The test program is in it's delay and we 
stop it.
* The test crashes at some point before sending the stop. The test program 
waits 60s then exits.
* The test program crashes before printing. The test waits on the print until 
it times out.
* The test picks up the print just as the test program is exiting, or has 
already exited. The test times out waiting for the stop response.

It's not free of timing issues but it means that you won't have either test or 
test program hanging around if either fails.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105180

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

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

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/source/Target/Process.cpp:6108
+range_callback) {
   Architecture *arch = GetTarget().GetArchitecturePlugin();
   const MemoryTagManager *tag_manager =

omjavaid wrote:
> The point I was trying to establish above is that GetMemoryTagManagerImpl 
> function here may remain as it was i-e GetMemoryTagManager. It could only run 
> once on Process object creation. We know our process architecture so we can 
> host a pointer to relevent tag manager in Process class. All the tag ranges 
> corresponding to the current process address space should be able to get a 
> pointer to MemoryTagManager as was being done previously. 
I see what you mean. I'm going to try this on top of main as a new change, then 
I'll refactor this based on that if it works out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105181

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

[Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, thanks!




Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:512
   virtual bool
-  LoadScriptingModule(const char *filename, bool init_session,
+  LoadScriptingModule(const char *filename, bool init_session, bool silent,
   lldb_private::Status &error,

I think this case where we have two bools next to each other is a good place to 
actually use the bugprone-arg-comment's `/*silent=*/` comments to have a way to 
actually check that those don't get mixed up. (Or we could make it a dedicated 
`enum` which I know you're a big fan of)



Comment at: lldb/source/Commands/Options.td:749
+  def silent : Option<"silent", "s">, Group<1>,
+Desc<"If true don't print output while importing.">;
 }

`don't print any script output` because we would still print LLDB errors and 
what not.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1212
+return Status(io_redirect_or_error.takeError());
+  }
+

no curly braces around if


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

https://reviews.llvm.org/D105327

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

[Lldb-commits] [PATCH] D105483: [LLDB] Testsuite: Add helper to check for AArch64 target

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1296
+"""Returns true if the architecture is AArch64."""
+return self.getArchitecture().lower() in ["aarch64"]
+

DavidSpickett wrote:
> This can be:
> ```
> return self.getArchitecture().lower() == "aarch64"
> ```
> 
> Unless you're expecting "aarch64_be" or "aarch64_32" as well.
> ```
> return "aarch64" in self.getArchitecture().lower()
> ```
> 
> Not sure if lldb just has the single name.
This was intentional as I wanted to keep this helper checking for platform 
architecture regardless of ABI or endianess. For an ILP32 inferior or be 
inferior our Native* are same as normal aarch64.


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

https://reviews.llvm.org/D105483

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

[Lldb-commits] [lldb] 29cc50e - [LLDB][GUI] Add initial forms support

[Raphael Isemann via lldb-commits]

Author: Omar Emara
Date: 2021-07-07T17:17:46+02:00
New Revision: 29cc50e17a6800ca75cd23ed85ae1ddf3e3dcc14

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

LOG: [LLDB][GUI] Add initial forms support

This patch adds initial support for forms for the LLDB GUI. The currently
supported form elements are Text fields, Integer fields, Boolean fields, Choices
fields, File fields, Directory fields, and List fields.

A form can be created by subclassing FormDelegate. In the constructor, field
factory methods can be used to add new fields, storing the returned pointer in a
member variable. One or more actions can be added using the AddAction method.
The method takes a function with an interface void(Window &). This function will
be executed whenever the user executes the action.

Example form definition:

```lang=cpp
class TestFormDelegate : public FormDelegate {
public:
  TestFormDelegate() {
m_text_field = AddTextField("Text", "The big brown fox.");
m_file_field = AddFileField("File", "/tmp/a");
m_directory_field = AddDirectoryField("Directory", "/tmp/");
m_integer_field = AddIntegerField("Number", 5);
std::vector choices;
choices.push_back(std::string("Choice 1"));
choices.push_back(std::string("Choice 2"));
choices.push_back(std::string("Choice 3"));
choices.push_back(std::string("Choice 4"));
choices.push_back(std::string("Choice 5"));
m_choices_field = AddChoicesField("Choices", 3, choices);
m_bool_field = AddBooleanField("Boolean", true);
TextFieldDelegate default_field =
TextFieldDelegate("Text", "The big brown fox.");
m_text_list_field = AddListField("Text List", default_field);

AddAction("Submit", [this](Window &window) { Submit(window); });
  }

  void Submit(Window &window) { SetError("An example error."); }

protected:
  TextFieldDelegate *m_text_field;
  FileFieldDelegate *m_file_field;
  DirectoryFieldDelegate *m_directory_field;
  IntegerFieldDelegate *m_integer_field;
  BooleanFieldDelegate *m_bool_field;
  ChoicesFieldDelegate *m_choices_field;
  ListFieldDelegate *m_text_list_field;
};
```

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index bd29d280c53f7..d37271bd1e53a 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 using namespace lldb;
@@ -85,6 +86,8 @@ using llvm::StringRef;
 #define KEY_RETURN 10
 #define KEY_ESCAPE 27
 
+#define KEY_SHIFT_TAB (KEY_MAX + 1)
+
 namespace curses {
 class Menu;
 class MenuDelegate;
@@ -336,93 +339,52 @@ class HelpDialogDelegate : public WindowDelegate {
   int m_first_visible_line;
 };
 
-class Window {
+// A surface is an abstraction for something than can be drawn on. The surface
+// have a width, a height, a cursor position, and a multitude of drawing
+// operations. This type should be sub-classed to get an actually useful 
ncurses
+// object, such as a Window, SubWindow, Pad, or a SubPad.
+class Surface {
 public:
-  Window(const char *name)
-  : m_name(name), m_window(nullptr), m_panel(nullptr), m_parent(nullptr),
-m_subwindows(), m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(false),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {}
+  Surface() : m_window(nullptr) {}
 
-  Window(const char *name, WINDOW *w, bool del = true)
-  : m_name(name), m_window(nullptr), m_panel(nullptr), m_parent(nullptr),
-m_subwindows(), m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(del),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
-if (w)
-  Reset(w);
-  }
+  WINDOW *get() { return m_window; }
 
-  Window(const char *name, const Rect &bounds)
-  : m_name(name), m_window(nullptr), m_parent(nullptr), m_subwindows(),
-m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(true),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
-Reset(::newwin(bounds.size.height, bounds.size.width, bounds.origin.y,
-   bounds.origin.y));
-  }
+  operator WINDOW *() { return m_window; }
 
-  virtual ~Window() {
-RemoveSubWindows();
-Reset();
+  // Copy a region of the surface to another surface.
+  void CopyToSurface(Surface &target, Point source_origin, Point target_origin,
+ Size size) {
+::copywin(m_window, target.get

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

[Raphael Isemann 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 rG29cc50e17a68: [LLDB][GUI] Add initial forms support 
(authored by OmarEmaraDev, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104395

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 using namespace lldb;
@@ -85,6 +86,8 @@
 #define KEY_RETURN 10
 #define KEY_ESCAPE 27
 
+#define KEY_SHIFT_TAB (KEY_MAX + 1)
+
 namespace curses {
 class Menu;
 class MenuDelegate;
@@ -336,93 +339,52 @@
   int m_first_visible_line;
 };
 
-class Window {
+// A surface is an abstraction for something than can be drawn on. The surface
+// have a width, a height, a cursor position, and a multitude of drawing
+// operations. This type should be sub-classed to get an actually useful ncurses
+// object, such as a Window, SubWindow, Pad, or a SubPad.
+class Surface {
 public:
-  Window(const char *name)
-  : m_name(name), m_window(nullptr), m_panel(nullptr), m_parent(nullptr),
-m_subwindows(), m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(false),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {}
+  Surface() : m_window(nullptr) {}
 
-  Window(const char *name, WINDOW *w, bool del = true)
-  : m_name(name), m_window(nullptr), m_panel(nullptr), m_parent(nullptr),
-m_subwindows(), m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(del),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
-if (w)
-  Reset(w);
-  }
+  WINDOW *get() { return m_window; }
 
-  Window(const char *name, const Rect &bounds)
-  : m_name(name), m_window(nullptr), m_parent(nullptr), m_subwindows(),
-m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(true),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
-Reset(::newwin(bounds.size.height, bounds.size.width, bounds.origin.y,
-   bounds.origin.y));
-  }
+  operator WINDOW *() { return m_window; }
 
-  virtual ~Window() {
-RemoveSubWindows();
-Reset();
+  // Copy a region of the surface to another surface.
+  void CopyToSurface(Surface &target, Point source_origin, Point target_origin,
+ Size size) {
+::copywin(m_window, target.get(), source_origin.y, source_origin.x,
+  target_origin.y, target_origin.x,
+  target_origin.y + size.height - 1,
+  target_origin.x + size.width - 1, false);
   }
 
-  void Reset(WINDOW *w = nullptr, bool del = true) {
-if (m_window == w)
-  return;
-
-if (m_panel) {
-  ::del_panel(m_panel);
-  m_panel = nullptr;
-}
-if (m_window && m_delete) {
-  ::delwin(m_window);
-  m_window = nullptr;
-  m_delete = false;
-}
-if (w) {
-  m_window = w;
-  m_panel = ::new_panel(m_window);
-  m_delete = del;
-}
-  }
+  int GetCursorX() const { return getcurx(m_window); }
+  int GetCursorY() const { return getcury(m_window); }
+  void MoveCursor(int x, int y) { ::wmove(m_window, y, x); }
 
   void AttributeOn(attr_t attr) { ::wattron(m_window, attr); }
   void AttributeOff(attr_t attr) { ::wattroff(m_window, attr); }
-  void Box(chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
-::box(m_window, v_char, h_char);
-  }
-  void Clear() { ::wclear(m_window); }
-  void Erase() { ::werase(m_window); }
-  Rect GetBounds() const {
-return Rect(GetParentOrigin(), GetSize());
-  } // Get the rectangle in our parent window
-  int GetChar() { return ::wgetch(m_window); }
-  int GetCursorX() const { return getcurx(m_window); }
-  int GetCursorY() const { return getcury(m_window); }
-  Rect GetFrame() const {
-return Rect(Point(), GetSize());
-  } // Get our rectangle in our own coordinate system
-  Point GetParentOrigin() const { return Point(GetParentX(), GetParentY()); }
-  Size GetSize() const { return Size(GetWidth(), GetHeight()); }
-  int GetParentX() const { return getparx(m_window); }
-  int GetParentY() const { return getpary(m_window); }
+
   int GetMaxX() const { return getmaxx(m_window); }
   int GetMaxY() const { return getmaxy(m_window); }
   int GetWidth() const { return GetMaxX(); }
   int GetHeight() const { return GetMaxY(); }
-  void MoveCursor(int x, int y) { ::wmove(m_window, y, x); }
-  void MoveWindow(int x, int y) { MoveWindow(Point(x, y)); }
-  void Resize(int w, int h) { ::wresize(m_window, h, w); }
- 

[Lldb-commits] [lldb] d4cb286 - [NFC][lldb-vscode] Fix launch test

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-07-07T10:01:19-07:00
New Revision: d4cb286b05f5192f6cbae310bd30cad3f05f13ac

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

LOG: [NFC][lldb-vscode] Fix launch test

Using br for creating breakpoints fails if there are other commands that
start with br.

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py 
b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index 593701e7ca64c..ff798364c9573 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -388,8 +388,8 @@ def test_extra_launch_commands(self):
 # breakpoints get hit
 launchCommands = [
 'target create "%s"' % (program),
-'br s -f main.c -l %d' % first_line,
-'br s -f main.c -l %d' % second_line,
+'breakpoint s -f main.c -l %d' % first_line,
+'breakpoint s -f main.c -l %d' % second_line,
 'process launch --stop-at-entry'
 ]
 



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

[Lldb-commits] [PATCH] D103172: [lldb][NFC] Allow range-based for loops over DWARFDIE's children

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

LGTM


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

https://reviews.llvm.org/D103172

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

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, teemperor, labath.
Herald added a subscriber: arphaman.
shafik requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Currently when we have a member function that has an auto return type and the 
definition is out of line we generate two DWARF DIE. 
One for the declaration and a second one for the definition, the definition 
holds the deduced type but does not contain a DW_AT_name nor
a DW_AT_linkage_name so there was not way to look up the definition DIE.

This fix modifies `CGDebugInfo::CreateCXXMemberFunction` so that it now only 
emits the linkage name for the definition of a method with a
deduced return type. It also modifies the `DWARFASTParserClang` to detect we 
have a function with deduced return type and lookup the 
defintion to obtain the correct return type and adjust the `FunctionDecl` to 
reflect this.

This required modifying the various indexes to support a lookup method for this 
case similar to `FindCompleteObjCDefinitionTypeForDIE`


https://reviews.llvm.org/D105564

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_appleDwarfIndex.s
  lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_manualDwarfIndex.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_manualDwarfIndex.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_manualDwarfIndex.s
@@ -0,0 +1,255 @@
+# This tests that lldb when using ManualDWARFIndex is able to find the definition
+# for an auto return function.
+
+# RUN: llvm-mc -triple x86_64-gnu-linux %s -filetype=obj > %t.o
+# RUN: lldb-test symbols --dump-clang-ast %t.o | FileCheck %s
+
+# CHECK: CXXMethodDecl {{.*}} <>  f 'int ()'
+
+# This was compiled from the following code:
+#
+# struct A {
+# auto f();
+# };
+#
+# auto A::f() {
+# return 0;
+# }
+#
+# Compiled using:
+#
+#   -target x86_64-gnu-linux
+
+
+	.text
+	.globl	_ZN1A1fEv   # -- Begin function _ZN1A1fEv
+	.p2align	4, 0x90
+	.type	_ZN1A1fEv,@function
+_ZN1A1fEv:  # @_ZN1A1fEv
+.Lfunc_begin0:
+	.cfi_startproc
+# %bb.0:# %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	movq	%rdi, -8(%rbp)
+.Ltmp0:
+	xorl	%eax, %eax
+	popq	%rbp
+	.cfi_def_cfa %rsp, 8
+	retq
+.Ltmp1:
+.Lfunc_end0:
+	.size	_ZN1A1fEv, .Lfunc_end0-_ZN1A1fEv
+	.cfi_endproc
+# -- End function
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	14  # DW_FORM_strp
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	14  # DW_FORM_strp
+	.byte	17  # DW_AT_low_pc
+	.byte	1   # DW_FORM_addr
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	19  # DW_TAG_structure_type
+	.byte	1   # DW_CHILDREN_yes
+	.byte	54  # DW_AT_calling_convention
+	.byte	11  # DW_FORM_data1
+	.byte	3   # DW_AT_name
+	.byte	14  # DW_FORM_strp
+	.byte	11  # DW_AT_byte_size
+	.byte	11  # DW_FORM_data1
+	.byte	58  # DW_AT_decl_file
+	.byte	11 

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I think it would be best to split out the Clang change into a separately tested 
patch.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1694
+  // If the declared return type is "auto" we want the linkage name to go
+  // with the defintion. In case the definiton is out of line, it needs to
+  // have a name so we can find it via DWARF index.

typo: definition (2x)




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

https://reviews.llvm.org/D105564

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

[Lldb-commits] [lldb] 3ebfeb2 - [lldb][docs] Force documentation emission of special Python class members

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-07-07T19:49:50+02:00
New Revision: 3ebfeb258698db82b7525cfaa1efd04db93d72ba

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

LOG: [lldb][docs] Force documentation emission of special Python class members

The current LLDB Python docs are missing documentation for all the special
members such as conversion functions (`__int__`) and other special functions
(`__len__`).

The reason for that is that the `automodapi` plugin we're using to generate the
*.rst files simply doesn't emit them. There doesn't seem to be any config option
to enable those in `automodapi` and it's not even clear why they are filtered. I
assume the leading underscore in their names makes them look like private
methods.

This patch just forcibly adds a few selected special members functions to the
list of functions that sphinx should always document.  This will cause sphinx to
warn if a class doesn't have one of those functions but it's better than not
having them documented.

The main motivation here is that since `SBAddress.__int__` is one of the few
functions that is only available in the embedded Python REPL which would be good
to have in the public documentation.

Fixes rdar://64647665

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/docs/conf.py

Removed: 




diff  --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index b614f43f55c00..0c3cd59fc11ef 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -44,6 +44,10 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax', 
'sphinx.ext.intersphinx']
 
+autodoc_default_options = {
+'special-members': '__int__, __len__, __hex__, __oct__, __iter__',
+}
+
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:



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

[Lldb-commits] [PATCH] D105480: [lldb][docs] Force documentation emission of special Python class members

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ebfeb258698: [lldb][docs] Force documentation emission of 
special Python class members (authored by teemperor).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105480

Files:
  lldb/docs/conf.py


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -44,6 +44,10 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax', 
'sphinx.ext.intersphinx']
 
+autodoc_default_options = {
+'special-members': '__int__, __len__, __hex__, __oct__, __iter__',
+}
+
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -44,6 +44,10 @@
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax', 'sphinx.ext.intersphinx']
 
+autodoc_default_options = {
+'special-members': '__int__, __len__, __hex__, __oct__, __iter__',
+}
+
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e81ba28 - [lldb/lua] Add scripted watchpoints for Lua

[Pedro Tammela via lldb-commits]

Author: Siger Yang
Date: 2021-07-07T14:51:02-03:00
New Revision: e81ba283131cf76ae62fa9b601a24d080578efaa

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

LOG: [lldb/lua] Add scripted watchpoints for Lua

Add support for Lua scripted watchpoints, with basic tests.

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

Added: 


Modified: 
lldb/bindings/lua/lua-swigsafecast.swig
lldb/bindings/lua/lua-wrapper.swig
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Removed: 




diff  --git a/lldb/bindings/lua/lua-swigsafecast.swig 
b/lldb/bindings/lua/lua-swigsafecast.swig
index a3ed372795466..0b67c41434e9e 100644
--- a/lldb/bindings/lua/lua-swigsafecast.swig
+++ b/lldb/bindings/lua/lua-swigsafecast.swig
@@ -14,6 +14,12 @@ PushSBClass (lua_State* L, lldb::SBBreakpointLocation* 
breakpoint_location_sb)
SWIG_NewPointerObj(L, breakpoint_location_sb, 
SWIGTYPE_p_lldb__SBBreakpointLocation, 0);
 }
 
+void
+PushSBClass (lua_State* L, lldb::SBWatchpoint* watchpoint_sb)
+{
+   SWIG_NewPointerObj(L, watchpoint_sb, SWIGTYPE_p_lldb__SBWatchpoint, 0);
+}
+
 void
 PushSBClass (lua_State* L, lldb::SBStructuredData* structured_data_sb)
 {

diff  --git a/lldb/bindings/lua/lua-wrapper.swig 
b/lldb/bindings/lua/lua-wrapper.swig
index 90c22920ddc11..e070bae23683e 100644
--- a/lldb/bindings/lua/lua-wrapper.swig
+++ b/lldb/bindings/lua/lua-wrapper.swig
@@ -53,5 +53,40 @@ LLDBSwigLuaBreakpointCallbackFunction
return stop;
 }
 
+// This function is called from Lua::CallWatchpointCallback
+SWIGEXPORT llvm::Expected
+LLDBSwigLuaWatchpointCallbackFunction
+(
+   lua_State *L,
+   lldb::StackFrameSP stop_frame_sp,
+   lldb::WatchpointSP wp_sp
+)
+{
+   lldb::SBFrame sb_frame(stop_frame_sp);
+   lldb::SBWatchpoint sb_wp(wp_sp);
+   int nargs = 2;
+
+   // Push the Lua wrappers
+   PushSBClass(L, &sb_frame);
+   PushSBClass(L, &sb_wp);
+
+   // Call into the Lua callback passing 'sb_frame' and 'sb_wp'.
+   // Expects a boolean return.
+   if (lua_pcall(L, nargs, 1, 0) != LUA_OK) {
+  llvm::Error E = llvm::make_error(
+llvm::formatv("{0}\n", lua_tostring(L, -1)),
+llvm::inconvertibleErrorCode());
+  // Pop error message from the stack.
+  lua_pop(L, 1);
+  return std::move(E);
+   }
+
+   // Boolean return from the callback
+   bool stop = lua_toboolean(L, -1);
+   lua_pop(L, 1);
+
+   return stop;
+}
+
 
 %}

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index f14e2732f6ebe..e99b7b88379a7 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -30,6 +30,9 @@ extern "C" llvm::Expected 
LLDBSwigLuaBreakpointCallbackFunction(
 lua_State *L, lldb::StackFrameSP stop_frame_sp,
 lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl);
 
+extern "C" llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);
+
 #if _MSC_VER
 #pragma warning (pop)
 #endif
@@ -113,6 +116,32 @@ Lua::CallBreakpointCallback(void *baton, 
lldb::StackFrameSP stop_frame_sp,
bp_loc_sp, extra_args_impl);
 }
 
+llvm::Error Lua::RegisterWatchpointCallback(void *baton, const char *body) {
+  lua_pushlightuserdata(m_lua_state, baton);
+  const char *fmt_str = "return function(frame, wp, ...) {0} end";
+  std::string func_str = llvm::formatv(fmt_str, body).str();
+  if (luaL_dostring(m_lua_state, func_str.c_str()) != LUA_OK) {
+llvm::Error e = llvm::make_error(
+llvm::formatv("{0}", lua_tostring(m_lua_state, -1)),
+llvm::inconvertibleErrorCode());
+// Pop error message from the stack.
+lua_pop(m_lua_state, 2);
+return e;
+  }
+  lua_settable(m_lua_state, LUA_REGISTRYINDEX);
+  return llvm::Error::success();
+}
+
+llvm::Expected
+Lua::CallWatchpointCallback(void *baton, lldb::StackFrameSP stop_frame_sp,
+lldb::WatchpointSP wp_sp) {
+
+  lua_pushlightuserdata(m_lua_state, baton);
+  lua_gettable(m_lua_state, LUA_REGISTRYINDEX);
+  return LLDBSwigLuaWatchpointCallbackFunction(m_lua_state, stop_frame_sp,
+   wp_sp);
+}
+
 llvm::Error Lua::CheckSyntax(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer");

diff  --git a/lldb/source/Plugins/Sc

[Lldb-commits] [PATCH] D105034: [lldb/lua] Add scripted watchpoints for Lua

[Pedro Tammela 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 rGe81ba283131c: [lldb/lua] Add scripted watchpoints for Lua 
(authored by Siger Yang , committed by tammela).

Changed prior to commit:
  https://reviews.llvm.org/D105034?vs=355429&id=357000#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105034

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -30,6 +30,11 @@
   return false;
 }
 
+extern "C" llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp) {
+  return false;
+}
+
 #if _MSC_VER
 #pragma warning (pop)
 #endif
Index: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -1,9 +1,33 @@
 # REQUIRES: lua
 # XFAIL: system-netbsd
-# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t
+# RUN: echo "int main() { int val = 1; val++; return 0; }" | %clang_host -x c - -g -o %t
 # RUN: %lldb -s %s --script-language lua %t 2>&1 | FileCheck %s
 b main
 r
-watchpoint set expr 0x0
+watchpoint set variable val
 watchpoint command add -s lua
-# CHECK: error: This script interpreter does not support watchpoint callbacks
+print("val=" .. tostring(frame:FindVariable("val"):GetValue()))
+quit
+c
+# CHECK: val=1
+# CHECK: val=2
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set variable val
+watchpoint modify 1 -c "(val == 1)"
+watchpoint command add -s lua
+print("conditional watchpoint")
+wp:SetEnabled(false)
+quit
+c
+# CHECK-COUNT-1: conditional watchpoint
+# CHECK-NOT: conditional watchpoint
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set expr 0x00
+watchpoint command add -s lua
+print("never triggers")
+quit
+c
+# CHECK-NOT: never triggers
+# CHECK: Process {{[0-9]+}} exited
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -11,6 +11,7 @@
 
 #include 
 
+#include "lldb/Breakpoint/WatchpointOptions.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Utility/Status.h"
@@ -63,6 +64,10 @@
  lldb::user_id_t break_id,
  lldb::user_id_t break_loc_id);
 
+  static bool WatchpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t watch_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -77,9 +82,16 @@
   std::vector> &bp_options_vec,
   CommandReturnObject &result) override;
 
+  void
+  CollectDataForWatchpointCommandCallback(WatchpointOptions *wp_options,
+  CommandReturnObject &result) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
   const char *command_body_text) override;
 
+  void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
+const char *command_body_text) override;
+
   Status SetBreakpointCommandCallbackFunction(
   BreakpointOptions &bp_options, const char *function_name,
   StructuredData::ObjectSP extra_args_sp) override;
@@ -91,6 +103,10 @@
   Status RegisterBreakpointCallback(BreakpointOptions &bp_options,
 const char *command_body_text,
 StructuredData::ObjectSP extra_args_sp);
+
+  Status RegisterWatchpointCallback(WatchpointOptions *wp_options,
+const char *command_body_text,
+StructuredData::ObjectSP extra_args_sp);
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
=

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I think it would be best to split out the Clang change into a separately tested 
patch.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693
+if (!try_resolving_type)
+  return true;
+

This block looks like it's more complicated than it needs to be. Could you just 
say

```
if (other_die != die)
  if (other_die.Tag()) != DW_TAG_subprogram)
   return false;

```
or am I missing something?


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

https://reviews.llvm.org/D105564

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

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693
+if (!try_resolving_type)
+  return true;
+

aprantl wrote:
> This block looks like it's more complicated than it needs to be. Could you 
> just say
> 
> ```
> if (other_die != die)
>   if (other_die.Tag()) != DW_TAG_subprogram)
>return false;
> 
> ```
> or am I missing something?
Definitely missed something :-)

```
if (other_die == die || (other_die.Tag()) != DW_TAG_subprogram))
return false;
```


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

https://reviews.llvm.org/D105564

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

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Could LLDB find the linkage name on the declaration, look that name up in the 
symbol table, and find the DW_TAG_subprogram DIE for the symbol's start address?


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

https://reviews.llvm.org/D105564

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

[Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 357037.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.

- Address code review feedback
- Use `LoadScriptOptions`


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

https://reviews.llvm.org/D105327

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Module.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test

Index: lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test
@@ -0,0 +1,8 @@
+# REQUIRES: python
+
+# RUN: %lldb --script-language python -o 'command script import lldb.macosx.crashlog' 2>&1 | FileCheck %s
+# RUN: %lldb --script-language python -o 'command script import -s lldb.macosx.crashlog' 2>&1 | FileCheck %s --check-prefix SILENT
+# RUN: %lldb --script-language python -o 'command script import --silent lldb.macosx.crashlog' 2>&1 | FileCheck %s --check-prefix SILENT
+
+CHECK: commands have been installed
+SILENT-NOT: commands have been installed
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -234,7 +234,8 @@
   bool RunScriptFormatKeyword(const char *impl_function, ValueObject *value,
   std::string &output, Status &error) override;
 
-  bool LoadScriptingModule(const char *filename, bool init_session,
+  bool LoadScriptingModule(const char *filename,
+   const LoadScriptOptions &options,
lldb_private::Status &error,
StructuredData::ObjectSP *module_sp = nullptr,
FileSpec extra_search_dir = {}) override;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1077,11 +1077,24 @@
 llvm::StringRef in_string, ScriptInterpreter::ScriptReturnType return_type,
 void *ret_value, const ExecuteScriptOptions &options) {
 
+  llvm::Expected>
+  io_redirect_or_error = ScriptInterpreterIORedirect::Create(
+  options.GetEnableIO(), m_debugger, /*result=*/nullptr);
+
+  if (!io_redirect_or_error) {
+llvm::consumeError(io_redirect_or_error.takeError());
+return false;
+  }
+
+  ScriptInterpreterIORedirect &io_redirect = **io_redirect_or_error;
+
   Locker locker(this,
 Locker::AcquireLock | Locker::InitSession |
 (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) |
 Locker::NoSTDIN,
-Locker::FreeAcquiredLock | Locker::TearDownSession);
+Locker::FreeAcquiredLock | Locker::TearDownSession,
+io_redirect.GetInputFile(), io_redirect.GetOutputFile(),
+io_redirect.GetErrorFile());
 
   PythonModule &main_module = GetMainModule();
   PythonDictionary globals = main_module.GetDictionary();
@@ -1190,11 +1203,22 @@
   if (in_string == nullptr)
 return Status();
 
+  llvm::Expected>
+  io_redirect_or_error = ScriptInterpreterIORedirect::Create(
+  options.GetEnableIO(), m_debugger, /*result=*/nullptr);
+
+  if (!io_redirect_or_error)
+return Status(io_redirect_or_error.takeError());
+
+  ScriptInterpreterIORedirect &io_redirect = **io_redirect_or_error;
+
   Locker locker(this,
 Locker::AcquireLock | Locker::InitSession |
 (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) |
 Locker::NoSTDIN,
-Locker::FreeAcquiredLock | Locker::TearDownSession);
+Locker::FreeAcquiredLock | Locker::TearDownSession,
+io_redirect.GetInputFile(), io_redirect.GetOutputFile(),
+io_redirect.GetErrorFile());
 
   PythonModule &main_module = GetMainModule();
   PythonDictionary globals = main_module.GetDictionary();
@@ -2057,7 +2081,10 @@
 
   StructuredData::ObjectSP module_sp;
 
-  if (LoadScriptingModule(file_spec.G

[Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2795
  Locker::FreeAcquiredLock |
- (init_session ? Locker::TearDownSession : 0));
+ (init_session ? Locker::TearDownSession : 0),
+ io_redirect.GetInputFile(), io_redirect.GetOutputFile(),

shafik wrote:
> Not sure how I feel about using `0` as kind of a noop for an enum rather then 
> explicitly name it as such. 
FWIW this is all copied from other uses of the locker. 



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2799
 
-  auto ExtendSysPath = [this](std::string directory) -> llvm::Error {
+  auto ExtendSysPath = [&](std::string directory) -> llvm::Error {
 if (directory.empty()) {

shafik wrote:
> `&directory` so we are explicit about what we are capturing.
I can't find it in the LLVM coding standards, but I believe it's common in LLVM 
to capture everything, or at least I've gotten that feedback in past code 
reviews. 


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

https://reviews.llvm.org/D105327

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

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

[walter erquinigo via Phabricator via lldb-commits]

wallace requested changes to this revision.
wallace added inline comments.



Comment at: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:292
+Tests the evaluated expression expands successfully after "scopes" 
packets
+and permanent
+'''

i think you are missing a verb here



Comment at: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:305-378
+# 1. Verify locals
+locals = self.vscode.get_local_variables()
+buffer_children = make_buffer_verify_dict(0, 32)
+verify_locals = {
+'argc': {
+'equals': {'type': 'int', 'value': '1'}
+},

I suggest removing these numbers (1 to 5), because if later we want to add a 
check step in the middle, we'll have to modify many more lines polluting the 
git blame



Comment at: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py:359-364
+self.verify_values(expandable_expression['response'], 
response['body'], permanent_expr_varref_dict, expandable_expression['name'])
+
+# 2.2. Evaluate from temporary UI.
+temporary_expr_varref_dict = {}
+response = self.vscode.request_evaluate(expandable_expression['name'])
+self.verify_values(expandable_expression['response'], 
response['body'], temporary_expr_varref_dict, expandable_expression['name'])

try to stick to 80 cols



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:33
 VSCode::VSCode()
-: variables(), broadcaster("lldb-vscode"), num_regs(0), num_locals(0),
-  num_globals(0), log(),
+: broadcaster("lldb-vscode"),
   exception_breakpoints(

probably you shouldn't have removed log()



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:536
+  expandable_variables.clear();
+  next_temporary_var_ref = VARREF_FIRST_VAR_IDX;
+}

Better not reset it, if every variable has a distinct ref throughout the debug 
session, it'll be easier for debugging the DAP messages



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:540-541
+int64_t Variables::GetNewVariableRefence(bool is_permanent) {
+  return is_permanent ? (PermanentVariableBitMask | next_permanent_var_ref++)
+  : (next_temporary_var_ref++);
+}

clayborg wrote:
> use if statement. Easier to read
+1



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:563
+   bool is_permanent) {
+  auto var_ref = GetNewVariableRefence(is_permanent);
+  if (is_permanent)

clayborg wrote:
> don't use auto, nice to see what the type is: int64_t
yes, in lldb auto should only be used if it really helps readability, and in 
this case it only helps writing, not reading



Comment at: lldb/tools/lldb-vscode/VSCode.h:57-60
 #define VARREF_LOCALS (int64_t)1
 #define VARREF_GLOBALS (int64_t)2
 #define VARREF_REGS (int64_t)3
 #define VARREF_FIRST_VAR_IDX (int64_t)4

tbh it might be better if you just make them consts variables



Comment at: lldb/tools/lldb-vscode/VSCode.h:83
 
+struct Variables {
+  // Bit mask to tell if a variableReference is inside

Move it to a new file and add a global comment explaining the different kinds 
of variables that exist



Comment at: lldb/tools/lldb-vscode/VSCode.h:92-93
+
+  int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
+  int64_t next_permanent_var_ref{VARREF_FIRST_VAR_IDX};
+  llvm::DenseMap expandable_variables;

if we are already distinguishing by the bitmask field, then I don't think we 
need to have these two counters. Just one should be enough.



Comment at: lldb/tools/lldb-vscode/VSCode.h:94
+  int64_t next_permanent_var_ref{VARREF_FIRST_VAR_IDX};
+  llvm::DenseMap expandable_variables;
+  llvm::DenseMap expandable_permanent_variables;

maybe call this expandable_temporary_variables to distinguish it from the next 
variable, otherwise it seems that the permanent ones are a subset of the other 
one



Comment at: lldb/tools/lldb-vscode/VSCode.h:97
+
+  /// Check if \p var_ref points to variable in a
+  /// expandable_permanent_variables.

better explain the broad goal of this method instead of the implementation. 
Something like this might be better

  Check if \p var_ref points to a variable that should persist for the entire 
duration of the debug session, e.g. repl expandable variables.



Comment at: lldb/tools/lldb-vscode/VSCode.h:101-102
+
+  /// \return a new variableReference. If is_permanent is true the returned
+  /// value will have the PermanentVariableBitMask bit set.
+  int64_t GetNewVariableRefence(bool is_permanent);

This formatting is wrong, \return should be alone in one singl

[Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We also have the notion of a IOHandler being interactive or not:

  bool IOHandler::GetIsInteractive();

Maybe we can use this to set the "silent" option in the LoadScriptOptions 
object correctly in more cases than just when the user specifies the option?




Comment at: lldb/source/Core/Module.cpp:1531
 scripting_fspec.Dump(scripting_stream.AsRawOstream());
-const bool init_lldb_globals = false;
+LoadScriptOptions options;
 bool did_load = script_interpreter->LoadScriptingModule(

Do we want to default to silent here?



Comment at: 
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp:94
 if (!os_plugin_class_name.empty()) {
-  const bool init_session = false;
+  LoadScriptOptions options;
   char python_module_path_cstr[PATH_MAX];

Do we not want to default to silent here?


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

https://reviews.llvm.org/D105327

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

[Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I think the only time we should not be silent is when the user types "command 
script import" from an interactive terminal. If this command comes in from the 
equivalent of "command source ...", sourcing init files then that IOHandler 
isn't interactive and we can easily suppress this.


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

https://reviews.llvm.org/D105327

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

Re: [Lldb-commits] [PATCH] D105327: [lldb] Add the ability to silently import scripted commands

[Jim Ingham via lldb-commits]

Scripts can actually do work when they get imported (not just register 
commands) and you might for instance `command script import` a module that's 
going to do some work & present output in a breakpoint command or stop hook.  
But breakpoint commands and stop hooks are not interactive.  So I'm a little 
hesitant about ganging "suppress output" and "interactive".

Jim


> On Jul 7, 2021, at 1:47 PM, Greg Clayton via Phabricator 
>  wrote:
> 
> clayborg added a comment.
> 
> I think the only time we should not be silent is when the user types "command 
> script import" from an interactive terminal. If this command comes in from 
> the equivalent of "command source ...", sourcing init files then that 
> IOHandler isn't interactive and we can easily suppress this.
> 
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D105327/new/
> 
> https://reviews.llvm.org/D105327
> 

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

[Lldb-commits] [PATCH] D105531: [trace][intel pt] Implement the Intel PT cursor

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/include/lldb/Target/TraceCursor.h:64-65
 
+  TraceCursor(lldb::ThreadSP thread_sp) : m_thread_sp(thread_sp) {}
+
   /// Move the cursor to the next instruction more recent chronologically in 
the

move this above destructor above.



Comment at: lldb/include/lldb/Target/TraceCursor.h:105-116
+  /// \param[in] s
+  /// The stream object where the instructions are printed.
+  ///
+  /// \param[in] count
+  /// The number of instructions to print.
+  ///
+  /// \param[in] skip

Some of these are out of order (skip and count), or missing (direction). 



Comment at: lldb/include/lldb/Target/TraceCursor.h:117
+  /// information.
+  void DumpInstructions(Stream &s, lldb::TraceDirection direction, size_t skip,
+size_t count, bool raw);

Should we just use a bool for direction? There can't be any direction other 
than forward or backward.



Comment at: lldb/include/lldb/Target/TraceCursor.h:117-118
+  /// information.
+  void DumpInstructions(Stream &s, lldb::TraceDirection direction, size_t skip,
+size_t count, bool raw);
 

clayborg wrote:
> Should we just use a bool for direction? There can't be any direction other 
> than forward or backward.
Remove the "size_t skip" from this API. It isn't needed because the user can 
move the cursor around before they call this API to skip or position the cursor 
as needed, then call this function.



Comment at: lldb/include/lldb/Target/TraceCursor.h:144-151
+  /// Get a number indicating the position of the current instruction in the
+  /// trace. This number can be negative if the trace is being traversed
+  /// backwards and the total number of instructions is not known.
+  ///
+  /// The objective of this number is to help distinguish consecutive
+  /// instructions for presentation purposes, and not for using it as an
+  /// accessor.

This description is pretty vague. It should probably be removed or fully 
documented to state exactly what is expected of trace clients. This also seems 
like something that might be returned when moving the cursor around instead of 
a separate method? From the description, I am unclear as to what I should 
expect from traversing backwards. What happens when I call SeekToEnd(), or 
SeekToBegin()? What happens if I call Next(...) or Prev(...) when the 
granularity is set in a specific way?





Comment at: lldb/include/lldb/Target/TraceCursor.h:160
+  /// The thread that owns this cursor.
+  lldb::ThreadSP m_thread_sp;
 };

Probably best to store this as a: ExecutionContextRef m_exe_ref. See 
description of ExecutionContextRef in ExecutionContext.h for reasons why. The 
main issue is we don't want a TraceCursor to keep a strong reference to a 
thread and keep the thread and possibly the process object alive. Keeping a 
weak reference is safer and allows you to try to fetch the thread when needed 
by calling ExecutionContextRef::GetThreadSP()



Comment at: lldb/include/lldb/lldb-enumerations.h:962-966
+/// Enum for the direction to use when traversing instructions.
+enum TraceDirection {
+  eTraceDirectionForwards = 0,
+  eTraceDirectionBackwards,
+};

Unless there is another direction, I would just switch this to a bool and 
remove this from the public API. Fine to use it internally if needed.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2101-2105
+cursor.DumpInstructions(result.GetOutputStream(),
+m_options.m_forwards ? eTraceDirectionForwards
+ : eTraceDirectionBackwards,
+IsRepeatCommand() ? 0 : m_options.m_skip,
+m_options.m_count, m_options.m_raw);

take care of skip on the cursor _before_ calling this API, and remove skip 
argument.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2102-2103
+cursor.DumpInstructions(result.GetOutputStream(),
+m_options.m_forwards ? eTraceDirectionForwards
+ : eTraceDirectionBackwards,
+IsRepeatCommand() ? 0 : m_options.m_skip,

just pass a bool? Enum seems a bit much for a simple bool argument. Unless 
there is going to be another direction in the future, but I can't think of one.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:121
 /// stopped at. See \a Trace::GetCursorPosition for more information.
-class DecodedThread {
+class DecodedThread : public std::enable_shared_from_this {
 public:

You 

[Lldb-commits] [PATCH] D105389: [lldb] Add AllocateMemory/DeallocateMemory to the SBProcess API

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/bindings/interface/SBProcess.i:420
 
+lldb::addr_t
+AllocateMemory(size_t size, uint32_t permissions, lldb::SBError &error);

Add doc string that describes this function for python users. Something about 
"permissions" is an integer formed by combining logical OR'ing 
"lldb.ePermissionsWritable" "lldb.ePermissionsReadable" and 
"lldb.ePermissionsExecutable".



Comment at: lldb/bindings/interface/SBProcess.i:423
+
+lldb::SBError
+DeallocateMemory(lldb::addr_t ptr);

doc string would be nice here too specifying that the address should have come 
from a call to "AllocateMemory(...)"


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

https://reviews.llvm.org/D105389

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

[Lldb-commits] [PATCH] D101361: [LLDB] Support AArch64/Linux watchpoint on tagged addresses

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 357101.
omjavaid edited the summary of this revision.
omjavaid added a comment.

This adds pointer signing before we set watchpoint on the tagged_ptr. LLDB 
should be able to successfully set a watchpoint on a signed pointer. We can hit 
the watchpoint on tagged + signed pointer using variable but need to 
authenticate before increment using tagged pointer.


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

https://reviews.llvm.org/D101361

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
  lldb/source/Target/Target.cpp
  lldb/test/API/commands/watchpoints/watch_tagged_addr/Makefile
  lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
  lldb/test/API/commands/watchpoints/watch_tagged_addr/main.c

Index: lldb/test/API/commands/watchpoints/watch_tagged_addr/main.c
===
--- /dev/null
+++ lldb/test/API/commands/watchpoints/watch_tagged_addr/main.c
@@ -0,0 +1,26 @@
+#include 
+
+uint32_t global_var = 0; // Watchpoint variable declaration.
+
+int main(int argc, char **argv) {
+  int dummy = 0;
+  // Move address of global variable into tagged_ptr after tagging
+  // Simple tagging scheme where 62nd bit of tagged address is set
+  uint32_t *tagged_ptr = (uint32_t *)((uint64_t)&global_var | (1ULL << 62));
+
+  __asm__ __volatile__("pacdza %0" : "=r"(tagged_ptr) : "r"(tagged_ptr));
+
+  ++dummy; // Set break point at this line.
+
+  // Increment global_var
+  ++global_var;
+
+  ++dummy;
+
+  __asm__ __volatile__("autdza %0" : "=r"(tagged_ptr) : "r"(tagged_ptr));
+
+  // Increment global_var using tagged_ptr
+  ++*tagged_ptr;
+
+  return 0;
+}
Index: lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
===
--- /dev/null
+++ lldb/test/API/commands/watchpoints/watch_tagged_addr/TestWatchTaggedAddress.py
@@ -0,0 +1,135 @@
+"""
+Test LLDB can set and hit watchpoints on tagged addresses
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestWatchTaggedAddresses(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+# Set source filename.
+self.source = 'main.c'
+
+# Invoke the default build rule.
+self.build()
+
+# Get the path of the executable
+exe = self.getBuildArtifact("a.out")
+
+# Create a target by the debugger.
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test_watch_hit_tagged_ptr_access(self):
+"""
+Test that LLDB hits watchpoint installed on an untagged address with
+memory access by a tagged pointer.
+"""
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+# Add a breakpoint to set a watchpoint when stopped on the breakpoint.
+lldbutil.run_break_set_by_symbol(self, 'main')
+
+# Run the program.
+self.runCmd("run", RUN_SUCCEEDED)
+
+# We should be stopped due to the breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# Set the watchpoint variable declaration line number.
+self.decl = line_number(self.source,
+'// Watchpoint variable declaration.')
+
+# Now let's set a watchpoint on 'global_var'.
+self.expect(
+"watchpoint set variable global_var",
+WATCHPOINT_CREATED,
+substrs=[
+'Watchpoint created',
+'size = 4',
+'type = w',
+'%s:%d' %
+(self.source,
+ self.decl)])
+
+self.verify_watch_hits()
+
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test_watch_set_on_tagged_ptr(self):
+"""Test that LLDB can install and hit watchpoint on a tagged address"""
+
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+# Find the line number to break inside main().
+self.line = line_number(self.source, '// Set break point at this line.')
+
+# Add a breakpoint to set a watchpoint when stopped on the breakpoint.
+lldbutil.run_break_set_by_file_and_line(
+   

[Lldb-commits] [PATCH] D105389: [lldb] Add AllocateMemory/DeallocateMemory to the SBProcess API

[Peter S. Housel via Phabricator via lldb-commits]

housel updated this revision to Diff 357131.
housel added a comment.

Updated based on reviewer suggestions, thanks.


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

https://reviews.llvm.org/D105389

Files:
  lldb/bindings/interface/SBProcess.i
  lldb/include/lldb/API/SBProcess.h
  lldb/source/API/SBProcess.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py
  lldb/test/API/python_api/process/main.cpp

Index: lldb/test/API/python_api/process/main.cpp
===
--- lldb/test/API/python_api/process/main.cpp
+++ lldb/test/API/python_api/process/main.cpp
@@ -21,3 +21,13 @@
 return 0; // Set break point at this line and check variable 'my_char'.
   // Use lldb Python API to set memory content for my_int and check the result.
 }
+
+char test_read (char *ptr)
+{
+return *ptr;
+}
+
+void test_write (char *ptr, char c)
+{
+*ptr = c;
+}
Index: lldb/test/API/python_api/process/TestProcessAPI.py
===
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -398,3 +398,58 @@
 "Process effective group ID is invalid")
 
 process_info.GetParentProcessID()
+
+def test_allocate_deallocate_memory(self):
+"""Test Python SBProcess.AllocateMemory() and SBProcess.DeallocateMemory() APIs."""
+self.build()
+(target, process, main_thread, main_breakpoint) = lldbutil.run_to_source_breakpoint(
+self, "// Set break point at this line", lldb.SBFileSpec("main.cpp"))
+
+# Allocate a block of memory in the target process
+error = lldb.SBError()
+addr = process.AllocateMemory(16384, lldb.ePermissionsReadable, error)
+if not error.Success() or addr == lldb.LLDB_INVALID_ADDRESS:
+self.fail("SBProcess.AllocateMemory() failed")
+
+# Now use WriteMemory() API to write 'a' into the allocated
+# memory. Note that the debugger can do this even though the
+# block is not set writable.
+result = process.WriteMemory(addr, 'a', error)
+if not error.Success() or result != 1:
+self.fail("SBProcess.WriteMemory() failed")
+
+# Read from the memory location.  This time it should be 'a'.
+# Due to the typemap magic (see lldb.swig), we pass in 1 to ReadMemory and
+# expect to get a Python string as the result object!
+content = process.ReadMemory(addr, 1, error)
+if not error.Success():
+self.fail("SBProcess.ReadMemory() failed")
+if self.TraceOn():
+print("memory content:", content)
+
+self.expect(
+content,
+"Result from SBProcess.ReadMemory() matches our expected output: 'a'",
+exe=False,
+startstr=b'a')
+
+# Verify that the process itself can read the allocated memory
+frame = main_thread.GetFrameAtIndex(0)
+val = frame.EvaluateExpression(
+"test_read(reinterpret_cast({:#x}))".format(addr))
+self.expect(val.GetValue(),
+"Result of test_read() matches expected output 'a'",
+exe=False,
+startstr="'a'")
+
+# Verify that the process cannot write into the block
+val = frame.EvaluateExpression(
+"test_write(reinterpret_cast({:#x}), 'b')".format(addr))
+if val.GetError().Success():
+self.fail(
+"test_write() to allocated memory without write permission unexpectedly succeeded")
+
+# Deallocate the memory
+error = process.DeallocateMemory(addr)
+if not error.Success():
+self.fail("SBProcess.DeallocateMemory() failed")
Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1288,6 +1288,51 @@
   return LLDB_RECORD_RESULT(sb_proc_info);
 }
 
+lldb::addr_t SBProcess::AllocateMemory(size_t size, uint32_t permissions,
+   lldb::SBError &sb_error) {
+  LLDB_RECORD_METHOD(lldb::addr_t, SBProcess, AllocateMemory,
+ (size_t, uint32_t, lldb::SBError &), size, permissions,
+ sb_error);
+
+  lldb::addr_t addr = LLDB_INVALID_ADDRESS;
+  ProcessSP process_sp(GetSP());
+  if (process_sp) {
+Process::StopLocker stop_locker;
+if (stop_locker.TryLock(&process_sp->GetRunLock())) {
+  std::lock_guard guard(
+  process_sp->GetTarget().GetAPIMutex());
+  addr = process_sp->AllocateMemory(size, permissions, sb_error.ref());
+} else {
+  sb_error.SetErrorString("process is running");
+}
+  } else {
+sb_error.SetErrorString("SBProcess is invalid");
+  }
+  return addr;
+}
+
+lldb::SBError SBProcess::DeallocateMemory(lldb::addr_t ptr) {
+  LLDB_RECORD_