[Lldb-commits] [PATCH] D105630: [lldb][AArch64] Refactor memory tag range handling

omjavaid added a comment. I think this approach is a lot better given that the process of tagged ranged calculation deserved a separation and may be more explanation too. I guess somewhere in the comments of same function you can explain interaction between memory regions and tagged ranges. It

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

clayborg added a comment. Check my current comments before I proceed to look at the rest. Now that we are using the API for something, it is showing some issues with the usability of the API in TraceCursor. Let me know what you think Comment at:

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

wallace updated this revision to Diff 357371. wallace added a comment. Address comments. - I ended up creating new method Next(int count) and Prev(int count) in the cursor for the skip operation. It might be useful for other things. I didn't add the count parameter to the existing Prev and

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

wallace added inline comments. Comment at: lldb/source/Target/TraceInstructionDumper.cpp:291 +s.Printf("\n"); +TryMoveOneInstruction(); + } clayborg wrote: > You should be watching the return value of this right? What if this returns > false? that's

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

wallace marked 9 inline comments as done. wallace added inline comments. Comment at: lldb/source/Commands/CommandObjectThread.cpp:2111 bool m_create_repeat_command_just_invoked; - size_t m_consecutive_repetitions = 0; + std::map> m_dumpers; }; clayborg

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

> On Jul 8, 2021, at 12:25 PM, Alex Langford via Phabricator > wrote: > > bulbazord added a comment. > > In D105215#2850988 , @jingham wrote: > >> In D105215#2850821 , @bulbazord >> wrote: >> >>> I kind of

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

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/Commands/CommandObjectThread.cpp:2111 bool m_create_repeat_command_just_invoked; - size_t m_consecutive_repetitions = 0; +

[Lldb-commits] [PATCH] D105655: [LLDB][GUI] Add Process Attach form

you think? F17834972: 20210708-223744.mp4 <https://reviews.llvm.org/F17834972> Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105655/new/ https://reviews.llvm.org/D105655 ___ lldb-commits mailing l

[Lldb-commits] [lldb] 1def257 - PR51018: Remove explicit conversions from SmallString to StringRef to future-proof against C++23

Author: David Blaikie Date: 2021-07-08T13:37:57-07:00 New Revision: 1def2579e10dd84405465f403e8c31acebff0c97 URL: https://github.com/llvm/llvm-project/commit/1def2579e10dd84405465f403e8c31acebff0c97 DIFF: https://github.com/llvm/llvm-project/commit/1def2579e10dd84405465f403e8c31acebff0c97.diff

[Lldb-commits] [PATCH] D105655: [LLDB][GUI] Add Process Attach form

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 a form window to attach a process, either by PID

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

wallace updated this revision to Diff 357331. wallace added a comment. Addressed all issues - Created a TraceInstructionDumper class that keeps the index to print, which leaves the TraceCursor unaffected. It also keeps some other useful state. Repository: rG LLVM Github Monorepo CHANGES

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

wallace added inline comments. 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:

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

bulbazord added a comment. In D105215#2850988 , @jingham wrote: > In D105215#2850821 , @bulbazord > wrote: > >> I kind of feel that `Language::GetDemangledFunctionNameWithoutArguments` may >> be a bit too

[Lldb-commits] [PATCH] D105649: [LLDB] Move Trace-specific classes into separate library

wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. this looks good to me. Let's wait for Greg's opinion as well Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D105649: [LLDB] Move Trace-specific classes into separate library

bulbazord created this revision. bulbazord added reviewers: wallace, clayborg. Herald added a subscriber: mgorny. bulbazord requested review of this revision. Herald added a project: LLDB. These two classes, TraceSessionFileParser and ThreadPostMortemTrace, seem to be useful primarily for

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

housel added a comment. In D105389#2864941 , @clayborg wrote: > LGTM. Jim, chime in soon if you have any other objections! I don't have commit access, could some one take care of committing this for me? Thanks! CHANGES SINCE LAST ACTION

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

jingham accepted this revision. jingham added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105389/new/ https://reviews.llvm.org/D105389 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. LGTM. Jim, chime in soon if you have any other objections! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105389/new/ https://reviews.llvm.org/D105389

[Lldb-commits] [PATCH] D105470: [lldb] Clear children of ValueObject on value update

jingham added a comment. This change is wrong for ValueObjectConstResult's. The original point of ValueObjectConstResult's was to store the results of expressions so that, even if the process is not at the original stop point, you could still check the value. It should have the value it had

[Lldb-commits] [PATCH] D105630: [lldb][AArch64] Refactor memory tag range handling

DavidSpickett added a comment. Similarly, I have a patch locally that has a "best effort" read where it returns a sparse set of results (for annotating memory reads). For that there would be a `MakeTaggedRange<...>` that returns a list of ranges that you can then read in a loop. Repository:

[Lldb-commits] [PATCH] D105630: [lldb][AArch64] Refactor memory tag range handling

DavidSpickett added a comment. Herald added a subscriber: JDevlieghere. This is a response to feedback on https://reviews.llvm.org/D105181 but would be intended to come before any of the changes in that series if it looks good. I'm yet to refactor the tag writing changes on to this, wanted to

[Lldb-commits] [PATCH] D105630: [lldb][AArch64] Refactor memory tag range handling

DavidSpickett created this revision. Herald added subscribers: danielkiss, kristof.beyls. DavidSpickett requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Previously GetMemoryTagManager checked many things in one: - architecture supports

[Lldb-commits] [PATCH] D105470: [lldb] Clear children of ValueObject on value update

werat added a reviewer: jingham. werat added a comment. Jim, can you take a look, please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105470/new/ https://reviews.llvm.org/D105470 ___ lldb-commits

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

DavidSpickett added inline comments. Comment at: lldb/source/Target/Process.cpp:6108 +range_callback) { Architecture *arch = GetTarget().GetArchitecturePlugin(); const MemoryTagManager *tag_manager = DavidSpickett wrote: > omjavaid wrote: > > The

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

DavidSpickett added a comment. Any other comments? Comment at: lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py:22 """ Test that qMemTags packets are parsed correctly and/or rejected. """ self.build() omjavaid

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

DavidSpickett 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"] + omjavaid wrote: > DavidSpickett

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

DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. LGTM with the comments added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101361/new/ https://reviews.llvm.org/D101361

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

DavidSpickett added inline comments. Comment at: lldb/test/API/commands/watchpoints/watch_tagged_addr/main.c:11 + + __asm__ __volatile__("pacdza %0" : "=r"(tagged_ptr) : "r"(tagged_ptr)); + Add comments to these inline asm lines to explain the instructions.