[Lldb-commits] [lldb] 1a2d25f - Revert "[lldb/DWARF] Simplify DIE extraction code slightly"

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-03-30T09:59:34+02:00
New Revision: 1a2d25fcdd732fc3326c1e9a9729b3b2b08b5d17

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

LOG: Revert "[lldb/DWARF] Simplify DIE extraction code slightly"

This reverts commit 1b96e133cf5215cb9ebfe7f14630f479c1611f22 due to
failures on windows.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
index e501bb9af7fb..86b18615da7d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -153,15 +153,17 @@ void DWARFUnit::ExtractDIEsRWLocked() {
 
   DWARFDebugInfoEntry die;
 
+  uint32_t depth = 0;
   // We are in our compile unit, parse starting at the offset we were told to
   // parse
   const DWARFDataExtractor &data = GetData();
   std::vector die_index_stack;
   die_index_stack.reserve(32);
+  die_index_stack.push_back(0);
   bool prev_die_had_children = false;
   while (offset < next_cu_offset && die.Extract(data, this, &offset)) {
 const bool null_die = die.IsNULL();
-if (die_index_stack.size() == 0) {
+if (depth == 0) {
   assert(m_die_array.empty() && "Compile unit DIE already added");
 
   // The average bytes per DIE entry has been seen to be around 14-20 so
@@ -199,12 +201,11 @@ void DWARFUnit::ExtractDIEsRWLocked() {
 m_die_array.back().SetHasChildren(false);
 }
   } else {
-die.SetParentIndex(m_die_array.size() - die_index_stack.rbegin()[1]);
+die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]);
 
 if (die_index_stack.back())
   m_die_array[die_index_stack.back()].SetSiblingIndex(
   m_die_array.size() - die_index_stack.back());
-die_index_stack.back() = m_die_array.size();
 
 // Only push the DIE if it isn't a NULL DIE
 m_die_array.push_back(die);
@@ -213,19 +214,24 @@ void DWARFUnit::ExtractDIEsRWLocked() {
 
 if (null_die) {
   // NULL DIE.
-  if (!die_index_stack.empty()) {
+  if (!die_index_stack.empty())
 die_index_stack.pop_back();
-prev_die_had_children = false;
-  }
+
+  if (depth > 0)
+--depth;
+  prev_die_had_children = false;
 } else {
+  die_index_stack.back() = m_die_array.size() - 1;
   // Normal DIE
   const bool die_has_children = die.HasChildren();
-  if (die_has_children)
+  if (die_has_children) {
 die_index_stack.push_back(0);
+++depth;
+  }
   prev_die_had_children = die_has_children;
 }
 
-if (die_index_stack.size() == 0)
+if (depth == 0)
   break; // We are done with this compile unit!
   }
 



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

[Lldb-commits] [lldb] 6919c58 - [lldb] Add a test for Obj-C properties with conflicting names

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-03-30T11:08:16+02:00
New Revision: 6919c58262b0bab682ab48903e714d70a489418b

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

LOG: [lldb] Add a test for Obj-C properties with conflicting names

This is apparently allowed in Objective-C so we should test this in LLDB.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
lldb/test/API/lang/objc/objc-property/main.m

Removed: 




diff  --git a/lldb/test/API/lang/objc/objc-property/TestObjCProperty.py 
b/lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
index 5693a77547cc..25158a58a786 100644
--- a/lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
+++ b/lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
@@ -137,3 +137,8 @@ def test_objc_properties(self):
 value = frame.EvaluateExpression("BaseClass.classInt", False)
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(1), 234)
+
+# Test that accessing two distinct class and instance properties that
+# share the same name works.
+self.expect_expr("mine.propConflict", result_value="4")
+self.expect_expr("BaseClass.propConflict", result_value="6")

diff  --git a/lldb/test/API/lang/objc/objc-property/main.m 
b/lldb/test/API/lang/objc/objc-property/main.m
index 8c84440adc43..94e5af43f415 100644
--- a/lldb/test/API/lang/objc/objc-property/main.m
+++ b/lldb/test/API/lang/objc/objc-property/main.m
@@ -22,12 +22,16 @@ - (void) mySetUnbackedInt: (int) in_int;
 
 - (int) getAccessCount;
 
++ (int) propConflict;
+
 +(BaseClass *) baseClassWithBackedInt: (int) inInt andUnbackedInt: (int) 
inOtherInt;
 
 @property(getter=myGetUnbackedInt,setter=mySetUnbackedInt:) int unbackedInt;
 @property int backedInt;
 @property (nonatomic, assign) id  idWithProtocol;
 @property(class) int classInt;
+@property(getter=propConflict,readonly) int propConflict;
+@property(readonly,class) int propConflict;
 @end
 
 @implementation BaseClass
@@ -85,6 +89,15 @@ - (int) getAccessCount
 {
   return _access_count;
 }
+
+- (int) propConflict
+{
+  return 4;
+}
++ (int) propConflict
+{
+  return 6;
+}
 @end
 
 typedef BaseClass TypedefBaseClass;
@@ -94,6 +107,7 @@ - (int) getAccessCount
 {
   BaseClass *mine = [BaseClass baseClassWithBackedInt: 10 andUnbackedInt: 20];
   TypedefBaseClass *typedefd = mine;
+  int propConflict = mine.propConflict + BaseClass.propConflict;
   
   // Set a breakpoint here.
   int nonexistant = mine.nonexistantInt;



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

[Lldb-commits] [PATCH] D99513: [lldb] Add a test for Obj-C properties with conflicting names

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6919c58262b0: [lldb] Add a test for Obj-C properties with 
conflicting names (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99513

Files:
  lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
  lldb/test/API/lang/objc/objc-property/main.m


Index: lldb/test/API/lang/objc/objc-property/main.m
===
--- lldb/test/API/lang/objc/objc-property/main.m
+++ lldb/test/API/lang/objc/objc-property/main.m
@@ -22,12 +22,16 @@
 
 - (int) getAccessCount;
 
++ (int) propConflict;
+
 +(BaseClass *) baseClassWithBackedInt: (int) inInt andUnbackedInt: (int) 
inOtherInt;
 
 @property(getter=myGetUnbackedInt,setter=mySetUnbackedInt:) int unbackedInt;
 @property int backedInt;
 @property (nonatomic, assign) id  idWithProtocol;
 @property(class) int classInt;
+@property(getter=propConflict,readonly) int propConflict;
+@property(readonly,class) int propConflict;
 @end
 
 @implementation BaseClass
@@ -85,6 +89,15 @@
 {
   return _access_count;
 }
+
+- (int) propConflict
+{
+  return 4;
+}
++ (int) propConflict
+{
+  return 6;
+}
 @end
 
 typedef BaseClass TypedefBaseClass;
@@ -94,6 +107,7 @@
 {
   BaseClass *mine = [BaseClass baseClassWithBackedInt: 10 andUnbackedInt: 20];
   TypedefBaseClass *typedefd = mine;
+  int propConflict = mine.propConflict + BaseClass.propConflict;
   
   // Set a breakpoint here.
   int nonexistant = mine.nonexistantInt;
Index: lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
===
--- lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
+++ lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
@@ -137,3 +137,8 @@
 value = frame.EvaluateExpression("BaseClass.classInt", False)
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(1), 234)
+
+# Test that accessing two distinct class and instance properties that
+# share the same name works.
+self.expect_expr("mine.propConflict", result_value="4")
+self.expect_expr("BaseClass.propConflict", result_value="6")


Index: lldb/test/API/lang/objc/objc-property/main.m
===
--- lldb/test/API/lang/objc/objc-property/main.m
+++ lldb/test/API/lang/objc/objc-property/main.m
@@ -22,12 +22,16 @@
 
 - (int) getAccessCount;
 
++ (int) propConflict;
+
 +(BaseClass *) baseClassWithBackedInt: (int) inInt andUnbackedInt: (int) inOtherInt;
 
 @property(getter=myGetUnbackedInt,setter=mySetUnbackedInt:) int unbackedInt;
 @property int backedInt;
 @property (nonatomic, assign) id  idWithProtocol;
 @property(class) int classInt;
+@property(getter=propConflict,readonly) int propConflict;
+@property(readonly,class) int propConflict;
 @end
 
 @implementation BaseClass
@@ -85,6 +89,15 @@
 {
   return _access_count;
 }
+
+- (int) propConflict
+{
+  return 4;
+}
++ (int) propConflict
+{
+  return 6;
+}
 @end
 
 typedef BaseClass TypedefBaseClass;
@@ -94,6 +107,7 @@
 {
   BaseClass *mine = [BaseClass baseClassWithBackedInt: 10 andUnbackedInt: 20];
   TypedefBaseClass *typedefd = mine;
+  int propConflict = mine.propConflict + BaseClass.propConflict;
   
   // Set a breakpoint here.
   int nonexistant = mine.nonexistantInt;
Index: lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
===
--- lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
+++ lldb/test/API/lang/objc/objc-property/TestObjCProperty.py
@@ -137,3 +137,8 @@
 value = frame.EvaluateExpression("BaseClass.classInt", False)
 self.assertTrue(value.GetError().Success())
 self.assertEquals(value.GetValueAsUnsigned(1), 234)
+
+# Test that accessing two distinct class and instance properties that
+# share the same name works.
+self.expect_expr("mine.propConflict", result_value="4")
+self.expect_expr("BaseClass.propConflict", result_value="6")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98482: [lldb] Support for multiprocess extension

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think it'd be better to commit this in two patches after all. The first one 
could include the server bits (and the multiprocess+ server thingy) -- I 
believe this is good to go already. The second patch/review could deal with the 
thread/process id business on the client (and finally enable multiprocess+ 
client-side).




Comment at: lldb/include/lldb/Utility/StringExtractorGDBRemote.h:197
+  static constexpr lldb::pid_t AllProcesses = UINT64_MAX;
+  static constexpr lldb::tid_t AllThreads = UINT64_MAX;
+

These also need a definition in the .cpp file (debug builds fail without that).



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:741
 if (response.GetChar() == 'C') {
-  m_curr_pid = response.GetHexMaxU32(false, LLDB_INVALID_PROCESS_ID);
+  m_curr_pid = response.GetHexMaxU64(false, LLDB_INVALID_PROCESS_ID);
   if (m_curr_pid != LLDB_INVALID_PROCESS_ID) {

mgorny wrote:
> I've noticed a few places where we take U32 instead of U64, so fixed that as 
> well.
Maybe just commit that separately (no review needed).



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2795
 std::vector &thread_ids, bool &sequence_mutex_unavailable) {
+  // lldb::pid_t pid = GetCurrentProcessID();
+  lldb::pid_t pid = 0;

mgorny wrote:
> @labath, any clue if we could make this work some other way? Calling 
> `GetCurrentProcessID()` causes a crash for some tests here:
> 
> ```
> Fatal Python error: Segmentation fault
> 
> Thread 0x7f918d203640 (most recent call first):
>   File 
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
>  line 470 in _handlePacket
>   File 
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
>  line 380 in _receive
>   File 
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
>  line 364 in _run
>   File "/usr/lib/python3.10/threading.py", line 910 in run
>   File "/usr/lib/python3.10/threading.py", line 972 in _bootstrap_inner
>   File "/usr/lib/python3.10/threading.py", line 930 in _bootstrap
> 
> Current thread 0x7f9196327740 (most recent call first):
>   File 
> "/home/mgorny/git/llvm-project/build.gentoo/lib/python3.10/site-packages/lldb/__init__.py",
>  line 10250 in ConnectRemote
>   File 
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
>  line 526 in connect
>   File 
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/TestRecognizeBreakpoint.py",
>  line 103 in test
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
>  line 413 in runMethod
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
>  line 383 in run
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
>  line 458 in __call__
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 117 in _wrapped_run
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 115 in _wrapped_run
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 85 in run
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/suite.py",
>  line 66 in __call__
>   File 
> "/home/mgorny/git/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/runner.py",
>  line 165 in run
>   File 
> "/home/mgorny/git/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py",
>  line 997 in run_suite
>   File "/home/mgorny/git/llvm-project/lldb/test/API/dotest.py", line 7 in 
> 
> 
> Extension modules: lldb._lldb (total: 1)
> ```
This backtrace is useless, as it only shows python threads (and the crash 
happens in a native one), but if you run the test in a debugger, the problem 
becomes obvious -- mutual recursion between `GetCurrentThreadIDs` and 
`GetCurrentProcessID`. To fix that, we need to break it.

I'd probably to in by creating a new function, which implements the core of 
q[fs]ThreadInfo functionality (for instance, it could return something like 
(`vector>` with INVALID_PID for unknown), and then call it from 
both of these functions to do their thing.


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

https://reviews.llvm.org/D98482

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

[Lldb-commits] [PATCH] D99497: [LLDB] Fix sync issue in TestVSCode_launch.test_progress_events

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D99497#2656600 , @clayborg wrote:

> In D99497#2656502 , @labath wrote:
>
>> Isn't there a better way to ensure synchronization here? Maybe, executing 
>> some command (setting a breakpoint, for instance), that will ensure that all 
>> symbols have been parsed?
>
> That is what I attempted do in the test: set a breakpoint at 'main' which 
> should trigger all of the needed events. Then we run to a breakpoint and hit 
> it. and after we hit this breakpoint, then we check the progress events.

Ok, I see what you mean. I looked (briefly) at the code, and my impression is 
that this dance will ensure that the progress events get /sent/, but they will 
not ensure that they are actually receieved/processed (as the receiving end is 
completely asynchronous).

The next question then becomes: is that intentional? If it is, then a delay is 
indeedall we can do (and I suspect we might need even more than 1s to 
sufficiently safe). Having progress events come after the operation that uses 
their results completes would be a bit weird, though it probably won't cause 
much problems in practice.

This situation actually reminds me of what we do with inferior stdout. That 
also used to be fully asynchronous (on several levels), but that ended up 
causing problems (for tests, at least), as we would try to assert that the 
inferior has written something after it hits a breakpoint, but that text would 
still be in flight. We solved that by explicitly flushing any pending output 
before sending the "stopped" event. One could imagine doing something similar 
in vscode as well -- before replying to a command (in this case, breakpoint 
set), check the progress listener, and flush any pending events.

> One thing we could do to figure things out would be to dump the vscode.txt 
> file for this test into the failing test. This would allow us to see what the 
> packets looked like when a test fails. Is there a cleanup function or 
> something we can do in the VS code tests that allows a test case to know it 
> has an error and add some stuff to the test suite output??

There are several ways to do that. The simplest (and most useful, I think) 
would be to just stream the messages (both sent and received) to stdout if 
`self.traceOn()` is true. I don't think it will help much in this case, but it 
could be pretty useful in the future.


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

https://reviews.llvm.org/D99497

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

[Lldb-commits] [lldb] d1486e6 - [lldb] Change CreateHostNativeRegisterContextLinux argument type

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-03-30T11:45:17+02:00
New Revision: d1486e65a1645ca00c3e3109e4e4bb72df1082c3

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

LOG: [lldb] Change CreateHostNativeRegisterContextLinux argument type

to NativeThreadLinux. This avoid casts down the line.

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
index fa067b78d1768..dfd0106837853 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
@@ -15,6 +15,8 @@
 namespace lldb_private {
 namespace process_linux {
 
+class NativeThreadLinux;
+
 class NativeRegisterContextLinux
 : public virtual NativeRegisterContextRegisterInfo {
 public:
@@ -24,7 +26,7 @@ class NativeRegisterContextLinux
   // variant should be compiled into the final executable.
   static std::unique_ptr
   CreateHostNativeRegisterContextLinux(const ArchSpec &target_arch,
-   NativeThreadProtocol &native_thread);
+   NativeThreadLinux &native_thread);
 
   // Invalidates cached values in register context data structures
   virtual void InvalidateAllRegisters(){}

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index e15e0f8a5651c..2bc2a923298d9 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -47,7 +47,7 @@ using namespace lldb_private::process_linux;
 
 std::unique_ptr
 NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
-const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
+const ArchSpec &target_arch, NativeThreadLinux &native_thread) {
   return std::make_unique(target_arch,
native_thread);
 }

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index fbaa7e6f5b802..5c025c0ecb43a 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -41,7 +41,7 @@ using namespace lldb_private::process_linux;
 
 std::unique_ptr
 NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
-const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
+const ArchSpec &target_arch, NativeThreadLinux &native_thread) {
   switch (target_arch.GetMachine()) {
   case llvm::Triple::arm:
 return std::make_unique(target_arch,

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
index 5abbab11d11d5..4c8ad65b66a38 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
@@ -78,7 +78,7 @@ using namespace lldb_private::process_linux;
 
 std::unique_ptr
 NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
-const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
+const ArchSpec &target_arch, NativeThreadLinux &native_thread) {
   return std::make_unique(target_arch,
   native_thread);
 }

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
index 22acbda8ebb9a..4531148c3fdc9 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
@@ -115,7 +115,7 @@ static const RegisterSet 
g_reg_sets_ppc64le[k_num_register_sets] = {
 
 std::unique_ptr
 NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
-const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
+const ArchSpec &target_

[Lldb-commits] [PATCH] D96460: [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good (and sorry for the delay). Just rebase to main to avoid the cast.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:77
+NativeProcessELF &process =
+static_cast(native_thread.GetProcess());
+

After d1486e65a164, this cast should be unnecessary.


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

https://reviews.llvm.org/D96460

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

[Lldb-commits] [PATCH] D96463: [LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.



Comment at: 
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:59-60
+ for _ in range(p_reg_size)) + '}'
+self.runCmd('register write p%i' %
+(i) + " '" + p_regs_value + "'")
+self.expect('register read p%i' % (i), substrs=[p_regs_value])

I find this combination of %-formatting and string concatenation unsettling. 
Just use % for the whole string?


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

https://reviews.llvm.org/D96463

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

[Lldb-commits] [PATCH] D99571: Update ProcessMachCore::DoLoadCore to handle binary hints with and without addresses

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
jasonmolenda requested review of this revision.

ProcessMachCore::DoLoadCore was implemented to handle the two LC_NOTEs "kern 
ver str" and "main bin spec" with only certain combinations of UUID and address 
and type present.  Other groups are starting to adopt these corefile hints and 
there are combinations that lldb wouldn't handle correctly.

This patch cleans up the handling of these hints in DoLoadCore a lot -- I've 
been wanting to clean this up for over a year now -- and adds tests for all of 
the combinations.

I did make one change to DynamicLoaderStatic to only set load addresses of 
Sections to their file addresses (slide==0) if an address hasn't already been 
set.  This gives me the ability to set the load address in ProcessMachCore and 
have it not get re-set back to the original file address later by the dynamic 
loader plugin.

The rest of this patch is is code that's my little playground of 
ProcessMachCore, so I don't know if others will have any comments, but I wanted 
to open it to the floor if anyone has suggestions or questions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99571

Files:
  lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
  lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
  lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp

Index: lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
===
--- lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
+++ lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -79,9 +80,17 @@
 
 void add_lc_note_kern_ver_str_load_command(
 std::vector> &loadcmds, std::vector &payload,
-int payload_file_offset, std::string uuid) {
+int payload_file_offset, std::string uuid, uint64_t address) {
   std::string ident = "EFI UUID=";
   ident += uuid;
+
+  if (address != 0x) {
+ident += "; stext=";
+char buf[24];
+sprintf(buf, "0x%llx", address);
+ident += buf;
+  }
+
   std::vector loadcmd_data;
 
   add_uint32(loadcmd_data, LC_NOTE); // note_command.cmd
@@ -111,7 +120,7 @@
 
 void add_lc_note_main_bin_spec_load_command(
 std::vector> &loadcmds, std::vector &payload,
-int payload_file_offset, std::string uuidstr) {
+int payload_file_offset, std::string uuidstr, uint64_t address) {
   std::vector loadcmd_data;
 
   add_uint32(loadcmd_data, LC_NOTE); // note_command.cmd
@@ -136,8 +145,8 @@
 
   // Now write the "main bin spec" payload.
   add_uint32(payload, 1);  // version
-  add_uint32(payload, 3);  // type == 3 [ firmware, standalone,e tc ]
-  add_uint64(payload, UINT64_MAX); // load address unknown/unspecified
+  add_uint32(payload, 3);  // type == 3 [ firmware, standalone, etc ]
+  add_uint64(payload, address);// load address
   uuid_t uuid;
   uuid_parse(uuidstr.c_str(), uuid);
   for (int i = 0; i < sizeof(uuid_t); i++)
@@ -259,9 +268,12 @@
 }
 
 int main(int argc, char **argv) {
-  if (argc != 4) {
-fprintf(stderr, "usage: create-empty-corefile version-string|main-bin-spec "
-" \n");
+  if (argc != 5) {
+fprintf(stderr,
+"usage: create-empty-corefile version-string|main-bin-spec "
+"  \n");
+fprintf(stderr,
+"  is base 16, 0x means unknown\n");
 fprintf(
 stderr,
 "Create a Mach-O corefile with an either LC_NOTE 'kern ver str' or \n");
@@ -284,13 +296,22 @@
   // An array of corefile contents (page data, lc_note data, etc)
   std::vector payload;
 
+  errno = 0;
+  uint64_t address = strtoull(argv[4], NULL, 16);
+  if (errno != 0) {
+fprintf(stderr, "Unable to parse address %s as base 16", argv[4]);
+exit(1);
+  }
+
   // First add all the load commands / payload so we can figure out how large
   // the load commands will actually be.
   load_commands.push_back(x86_lc_thread_load_command());
   if (strcmp(argv[1], "version-string") == 0)
-add_lc_note_kern_ver_str_load_command(load_commands, payload, 0, uuid);
+add_lc_note_kern_ver_str_load_command(load_commands, payload, 0, uuid,
+  address);
   else
-add_lc_note_main_bin_spec_load_command(load_commands, payload, 0, uuid);
+add_lc_note_main_bin_spec_load_command(load_commands, payload, 0, uuid,
+   address);
   add_lc_segment(load_commands, payload, 0);
 
   int size_of_load_commands = 0;
@@ -308,11 +329,11 @@
   load_commands.push_back(x86_lc_thread_load_command());
 
   if (strcmp(argv[1], "version-string") == 0)
-add_lc_note_kern_ver_str_loa

[Lldb-commits] [lldb] 42c3b5e - Fix cleanup error in TestVSCode_disconnect.test_launch

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2021-03-30T15:36:45+05:00
New Revision: 42c3b5e5b6ffd39041c3d34f01e7162573eefd6e

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

LOG: Fix cleanup error in TestVSCode_disconnect.test_launch

TestVSCode_disconnect.test_launch fails with clean up error because
disconnect gets called twice once from the test case and once from
the tear down hook.

This patch disables disconnect after its been called from test_launch

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 0c43f8ca5a97..7ddb3164cb3d 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -340,7 +340,8 @@ def build_and_launch(self, program, args=None, cwd=None, 
env=None,
  trace=False, initCommands=None, preRunCommands=None,
  stopCommands=None, exitCommands=None,
  terminateCommands=None, sourcePath=None,
- debuggerRoot=None, runInTerminal=False):
+ debuggerRoot=None, runInTerminal=False,
+ disconnectAutomatically=True):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and launch the process.
 '''
@@ -350,4 +351,5 @@ def build_and_launch(self, program, args=None, cwd=None, 
env=None,
 return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
 disableSTDIO, shellExpandArguments, trace,
 initCommands, preRunCommands, stopCommands, exitCommands,
-terminateCommands, sourcePath, debuggerRoot, 
runInTerminal=runInTerminal)
+terminateCommands, sourcePath, debuggerRoot, 
runInTerminal=runInTerminal,
+disconnectAutomatically=disconnectAutomatically)

diff  --git 
a/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py 
b/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
index 91b2ae783048..226aad63e234 100644
--- a/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
+++ b/lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
@@ -37,7 +37,7 @@ def test_launch(self):
 created.
 """
 program = self.getBuildArtifact("a.out")
-self.build_and_launch(program)
+self.build_and_launch(program, disconnectAutomatically=False)
 
 # We set a breakpoint right before the side effect file is created
 self.set_source_breakpoints(self.source, [line_number(self.source, '// 
breakpoint')])



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

[Lldb-commits] [PATCH] D99491: [LLDB] Fix cleanup error in TestVSCode_disconnect.test_launch

[Muhammad Omair Javaid 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 rG42c3b5e5b6ff: Fix cleanup error in 
TestVSCode_disconnect.test_launch (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99491

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py


Index: lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
===
--- lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
+++ lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
@@ -37,7 +37,7 @@
 created.
 """
 program = self.getBuildArtifact("a.out")
-self.build_and_launch(program)
+self.build_and_launch(program, disconnectAutomatically=False)
 
 # We set a breakpoint right before the side effect file is created
 self.set_source_breakpoints(self.source, [line_number(self.source, '// 
breakpoint')])
Index: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -340,7 +340,8 @@
  trace=False, initCommands=None, preRunCommands=None,
  stopCommands=None, exitCommands=None,
  terminateCommands=None, sourcePath=None,
- debuggerRoot=None, runInTerminal=False):
+ debuggerRoot=None, runInTerminal=False,
+ disconnectAutomatically=True):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and launch the process.
 '''
@@ -350,4 +351,5 @@
 return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
 disableSTDIO, shellExpandArguments, trace,
 initCommands, preRunCommands, stopCommands, exitCommands,
-terminateCommands, sourcePath, debuggerRoot, 
runInTerminal=runInTerminal)
+terminateCommands, sourcePath, debuggerRoot, 
runInTerminal=runInTerminal,
+disconnectAutomatically=disconnectAutomatically)


Index: lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
===
--- lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
+++ lldb/test/API/tools/lldb-vscode/disconnect/TestVSCode_disconnect.py
@@ -37,7 +37,7 @@
 created.
 """
 program = self.getBuildArtifact("a.out")
-self.build_and_launch(program)
+self.build_and_launch(program, disconnectAutomatically=False)
 
 # We set a breakpoint right before the side effect file is created
 self.set_source_breakpoints(self.source, [line_number(self.source, '// breakpoint')])
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -340,7 +340,8 @@
  trace=False, initCommands=None, preRunCommands=None,
  stopCommands=None, exitCommands=None,
  terminateCommands=None, sourcePath=None,
- debuggerRoot=None, runInTerminal=False):
+ debuggerRoot=None, runInTerminal=False,
+ disconnectAutomatically=True):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and launch the process.
 '''
@@ -350,4 +351,5 @@
 return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,
 disableSTDIO, shellExpandArguments, trace,
 initCommands, preRunCommands, stopCommands, exitCommands,
-terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal)
+terminateCommands, sourcePath, debuggerRoot, runInTerminal=runInTerminal,
+disconnectAutomatically=disconnectAutomatically)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98482: [lldb] Support for multiprocess extension

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Ok, I'm going to use this review to push the server part, then commit the 
uint64 client change separately, then create a new diff for client.

If you find a few more minutes, the server-side fork/vfork patch is ready for 
review (and independent of this one).


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

https://reviews.llvm.org/D98482

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

[Lldb-commits] [lldb] 64bb9cf - [lldb] [Process/gdb-remote] Fix TID reading to use U64

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-03-30T13:59:32+02:00
New Revision: 64bb9cf7bf8df85cbe75f0848840156d3c316207

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

LOG: [lldb] [Process/gdb-remote] Fix TID reading to use U64

Fix multiple instances of reading thread-id to use U64 type instead
of U32.  This is consistent with lldb::tid_t being a 64-bit type.

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d375a312ae2c..9218f8a63c01 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -729,7 +729,7 @@ lldb::pid_t 
GDBRemoteCommunicationClient::GetCurrentProcessID(bool allow_lazy) {
 PacketResult::Success) {
   if (response.GetChar() == 'Q') {
 if (response.GetChar() == 'C') {
-  m_curr_pid = response.GetHexMaxU32(false, LLDB_INVALID_PROCESS_ID);
+  m_curr_pid = response.GetHexMaxU64(false, LLDB_INVALID_PROCESS_ID);
   if (m_curr_pid != LLDB_INVALID_PROCESS_ID) {
 m_curr_pid_is_valid = eLazyBoolYes;
 return m_curr_pid;
@@ -1126,7 +1126,7 @@ bool 
GDBRemoteCommunicationClient::GetDefaultThreadId(lldb::tid_t &tid) {
 return false;
 
   if (response.GetChar() == 'Q' && response.GetChar() == 'C')
-tid = response.GetHexMaxU32(true, -1);
+tid = response.GetHexMaxU64(true, -1);
 
   return true;
 }

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 62a09a2a432c..f0fb116690f6 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1732,7 +1732,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
   // Consume the separator.
   packet.GetChar();
 
-  thread_action.tid = packet.GetHexMaxU32(false, LLDB_INVALID_THREAD_ID);
+  thread_action.tid = packet.GetHexMaxU64(false, LLDB_INVALID_THREAD_ID);
   if (thread_action.tid == LLDB_INVALID_THREAD_ID)
 return SendIllFormedResponse(
 packet, "Could not parse thread number in vCont packet");
@@ -3384,7 +3384,7 @@ GDBRemoteCommunicationServerLLGS::Handle_qThreadStopInfo(
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
 
   packet.SetFilePos(strlen("qThreadStopInfo"));
-  const lldb::tid_t tid = packet.GetHexMaxU32(false, LLDB_INVALID_THREAD_ID);
+  const lldb::tid_t tid = packet.GetHexMaxU64(false, LLDB_INVALID_THREAD_ID);
   if (tid == LLDB_INVALID_THREAD_ID) {
 LLDB_LOGF(log,
   "GDBRemoteCommunicationServerLLGS::%s failed, could not "



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

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D98822#2646052 , @mgorny wrote:

> It includes refactoring clone/fork/vfork event monitoring into a single 
> function. Right now, only forks are supported — the handler creates a local 
> `NativeProcessLinux` instance (@labath, any suggestions how to make this less 
> hacky?), duplicates parent's breakpoints into it (just like the child gets 
> parent's memory) and uses it to clear these breakpoints and detach the child 
> process.

I don't find this particularly hacky. However, I have doubts about the 
breakpoint clearing aspect. If we add that now, I think it will be tricky to 
move that process to the client in the future (how will the client know whether 
the breakpoints were auto-removed?) Given that  breakpoints in forked process 
have been borked since forever, I don't see harm in keeping that broken for a 
little while longer. Even a client with extremely limited of multiple processes 
(like the one we had in mind) should not have a problem with sending the 
appropriate `z` packets before it detaches. This patch could still come first 
(it's great that you've separated that out) -- it just needs to ensure that 
there are no breakpoints in the child path which would need clearing.

Btw, have you by any chance looked at how gdb determines which clone()s 
constitute a new thread?




Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:145
 
+  // Remove all software breakpoints and return a vector of breakpoint data
+  // that can be used to readd them.

mgorny wrote:
> mgorny wrote:
> > Long term, these functions are probably unnecessary. It just occurred to me 
> > to check how GDB handles it, and it handles clearing and restoring 
> > breakpoints on client side (i.e. by sending `z` and `Z` packets).
> Hmm, actually that depends on how much compatibility with old client versions 
> we want to preserve. If we want forks/vforks to stop being broken with 
> breakpoints when using an old client, we need to keep them as a fallback 
> logic for when fork/vfork-events aren't supported by client.
I certainly don't think we need to try to "fix" older clients by doing stuff to 
the server. (And on top of that, I am less worried about people using older 
clients, than I am about older servers -- those can be harder to update.)



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:401-404
+  struct Extension {
+static constexpr uint32_t fork = 1;
+static constexpr uint32_t vfork = 2;
+  };

mgorny wrote:
> labath wrote:
> > The llvm way to do this is via an enum class in combination with 
> > LLVM_MARK_AS_BITMASK_ENUM.
> Heh, and you've just told me to use `constexpr`s instead of `enum`s ;-).
That's cause you were using an enum to define constants. If you were using it 
to define a type, then I would be fine (though I might ask to make it a scoped 
enum).



Comment at: lldb/source/Host/linux/Host.cpp:323
+
+  if (!GetStatusInfo(tid, process_info, state, tracerpid, tgid) || tgid == 0)
+return llvm::None;

Why is this checking for zero ? (When it's being set to INVALID_PID above?)



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:924
+auto pgid_ret = getPIDForTID(child_pid);
+if (!pgid_ret || pgid_ret.getValue() != child_pid) {
+  // A new thread should have PGID matching our process' PID.

`if (pgid_ret != child_pid)` ?



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:456
+  LLDB_LOG(log, "tid {0} belongs to a different tgid {1}, assuming child",
+   pid, tgid.getValue());
+  MonitorFork(pid, false, 0);

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > For some reason, this doesn't print the correct tgid value (plain printf 
> > > works). Any clue what's wrong here?
> > what does it print?
> `tid 118029 belongs to a different tgid 4295085325, assuming child`
> 
> When tgid was 0, it also printed this huge number. If I cast it to `int`, it 
> prints fine. But then, first arg is also `lldb::pid_t` and it prints fine.
I dunno. I recommend debugging it. It smells like some kind of undefined 
behavior.



Comment at: lldb/test/Shell/Subprocess/fork-follow-parent.test:6
+process launch
+# CHECK: function run in child
+# CHECK-NOT: function run in parent

mgorny wrote:
> The tests sometimes fail if child starts after the breakpoint is hit (and 
> therefore this check happens before stop reason below). Any suggestion how to 
> make it work independently of where 'function run in child' happens?
There are several ways around that, but I don't know which one is the best, as 
I'm not sure what exactly you're trying to test. You could try to capture this 
nondeterminism with CHECK-DAGs, but CHECK-DAGs don't combine well with 
CHECK-NOTs. Another

[Lldb-commits] [PATCH] D98761: Fix "image lookup --address" Summary results for inline functions.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm not that familiar with this function, but the output seems right. Given 
that the `show_inline_callsite_line_info` argument is no longer used, can you 
remove it?




Comment at: lldb/source/Symbol/SymbolContext.cpp:133
+  // fill it in correctly with the calling file and line. Previous code
+  // was extracting the calling file and line from inlined_block_info and
+  // using it right away which is not correct. On the first call to this

I'm not sure if describing the old behavior of deleted code is that helpful. 
This may be better off in the commit message.



Comment at: lldb/test/Shell/Commands/command-image-lookup.yaml:129
+reserved3:   0x0
+  - sectname:__debug_pubnames
+segname: __DWARF

Could we delete pub***, aranges, and apple_*** sections?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98761

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

[Lldb-commits] [PATCH] D98482: [lldb] [server] Support for multiprocess extension

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 334131.
mgorny marked 2 inline comments as done.
mgorny retitled this revision from "[lldb] Support for multiprocess extension" 
to "[lldb] [server] Support for multiprocess extension".
mgorny added a comment.

Limited to server changes. Added definitions for constexpr vars.


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

https://reviews.llvm.org/D98482

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp

Index: lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp
@@ -0,0 +1,185 @@
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+#include "lldb/Utility/StringExtractorGDBRemote.h"
+#include "lldb/lldb-defines.h"
+
+TEST(StringExtractorGDBRemoteTest, GetPidTid) {
+  StringExtractorGDBRemote ex("");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  // invalid/short values
+
+  ex.Reset("narf");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset(";1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset(".1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("pnarf");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p;1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p.1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1234.");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1234.;1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("-2");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1234.-2");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p-2");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p-2.1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  // overflow
+
+  ex.Reset("p1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1.0");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p0.1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1.1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  // invalid: all processes but specific thread
+
+  ex.Reset("p-1.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p-1.1234");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p-1.123456789ABCDEF0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  // unsupported: pid/tid 0
+
+  ex.Reset("0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.-1");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.1234");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.123456789ABCDEF0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p1234.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p123456789ABCDEF0.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  // pure thread id
+
+  ex.Reset("-1");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(100, StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("1234");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(), ::testing::Pair(100, 0x1234ULL));
+
+  ex.Reset("123456789ABCDEF0");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(100, 0x123456789ABCDEF0ULL));
+
+  // pure process id
+
+  ex.Reset("p-1");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(StringExtractorGDBRemote::AllProcesses,
+  StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("p1234");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(0x1234ULL, StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("p123456789ABCDEF0");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(0x123456789ABCDEF0ULL,
+  StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("p");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::t

[Lldb-commits] [lldb] 6c1a803 - [lldb] [server] Support for multiprocess extension

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-03-30T15:09:27+02:00
New Revision: 6c1a8039de4646f6efbb3ba404d5bee5d631be67

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

LOG: [lldb] [server] Support for multiprocess extension

Add a minimal support for the multiprocess extension in lldb-server.
The server indicates support for it via qSupported, and accepts
thread-ids containing a PID.  However, it still does not support
debugging more than one inferior, so any other PID value results
in an error.

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

Added: 
lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp

Modified: 
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/unittests/Utility/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 3b6ed8030985..4ef3cea1aeb3 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -193,6 +193,15 @@ class StringExtractorGDBRemote : public StringExtractor {
 
   size_t GetEscapedBinaryData(std::string &str);
 
+  static constexpr lldb::pid_t AllProcesses = UINT64_MAX;
+  static constexpr lldb::tid_t AllThreads = UINT64_MAX;
+
+  // Read thread-id from the packet.  If the packet is valid, returns
+  // the pair (PID, TID), otherwise returns llvm::None.  If the packet
+  // does not list a PID, default_pid is used.
+  llvm::Optional>
+  GetPidTid(lldb::pid_t default_pid);
+
 protected:
   ResponseValidatorCallback m_validator;
   void *m_validator_baton;

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index 91509f609096..a28d0634061b 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -902,7 +902,8 @@ def add_qSupported_packets(self):
 "qXfer:libraries-svr4:read",
 "qXfer:features:read",
 "qEcho",
-"QPassSignals"
+"QPassSignals",
+"multiprocess",
 ]
 
 def parse_qSupported_response(self, context):

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index af3755fce774..3eab71388067 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -558,6 +558,7 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
   LazyBool m_supports_jGetSharedCacheInfo;
   LazyBool m_supports_QPassSignals;
   LazyBool m_supports_error_string_reply;
+  LazyBool m_supports_multiprocess;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
   m_supports_qUserName : 1, m_supports_qGroupName : 1,

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 1ca0290eda13..c933364cff9c 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -848,6 +848,7 @@ GDBRemoteCommunicationServerCommon::Handle_qSupported(
   response.PutCString(";qXfer:auxv:read+");
   response.PutCString(";qXfer:libraries-svr4:read+");
 #endif
+  response.PutCString(";multiprocess+");
 
   return SendPacketNoLock(response.GetString());
 }

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index f0fb116690f6..b665610a54a5 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1732,10 +1732,13 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
   // Consume the separator.
   packet.GetChar();
 
-  thread_action.tid = packet.GetH

[Lldb-commits] [PATCH] D98482: [lldb] [server] Support for multiprocess extension

[Michał Górny via 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 rG6c1a8039de46: [lldb] [server] Support for multiprocess 
extension (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98482

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp

Index: lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp
@@ -0,0 +1,185 @@
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+#include "lldb/Utility/StringExtractorGDBRemote.h"
+#include "lldb/lldb-defines.h"
+
+TEST(StringExtractorGDBRemoteTest, GetPidTid) {
+  StringExtractorGDBRemote ex("");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  // invalid/short values
+
+  ex.Reset("narf");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset(";1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset(".1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("pnarf");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p;1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p.1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1234.");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1234.;1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("-2");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1234.-2");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p-2");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p-2.1234");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  // overflow
+
+  ex.Reset("p1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1.0");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p0.1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  ex.Reset("p1.1");
+  EXPECT_EQ(ex.GetPidTid(0), llvm::None);
+
+  // invalid: all processes but specific thread
+
+  ex.Reset("p-1.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p-1.1234");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p-1.123456789ABCDEF0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  // unsupported: pid/tid 0
+
+  ex.Reset("0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.-1");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.1234");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p0.123456789ABCDEF0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p1234.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  ex.Reset("p123456789ABCDEF0.0");
+  EXPECT_EQ(ex.GetPidTid(100), llvm::None);
+
+  // pure thread id
+
+  ex.Reset("-1");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(100, StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("1234");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(), ::testing::Pair(100, 0x1234ULL));
+
+  ex.Reset("123456789ABCDEF0");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(100, 0x123456789ABCDEF0ULL));
+
+  // pure process id
+
+  ex.Reset("p-1");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(StringExtractorGDBRemote::AllProcesses,
+  StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("p1234");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(0x1234ULL, StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("p123456789ABCDEF0");
+  EXPECT_THAT(ex.GetPidTid(100).getValue(),
+  ::testing::Pair(0x123456789ABCDEF0ULL,
+  StringExtractorGDBRemote::AllThreads));
+
+  ex.Reset("p

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D98822#2658252 , @labath wrote:

> In D98822#2646052 , @mgorny wrote:
>
>> It includes refactoring clone/fork/vfork event monitoring into a single 
>> function. Right now, only forks are supported — the handler creates a local 
>> `NativeProcessLinux` instance (@labath, any suggestions how to make this 
>> less hacky?), duplicates parent's breakpoints into it (just like the child 
>> gets parent's memory) and uses it to clear these breakpoints and detach the 
>> child process.
>
> I don't find this particularly hacky. However, I have doubts about the 
> breakpoint clearing aspect. If we add that now, I think it will be tricky to 
> move that process to the client in the future (how will the client know 
> whether the breakpoints were auto-removed?) Given that  breakpoints in forked 
> process have been borked since forever, I don't see harm in keeping that 
> broken for a little while longer. Even a client with extremely limited of 
> multiple processes (like the one we had in mind) should not have a problem 
> with sending the appropriate `z` packets before it detaches. This patch could 
> still come first (it's great that you've separated that out) -- it just needs 
> to ensure that there are no breakpoints in the child path which would need 
> clearing.

Actually, that's not a problem at all. If client doesn't indicate `fork-events` 
support via `qSupported`, we handle breakpoints server-side. If it does, we 
deliver `fork` stop reason and then the client is responsible for dealing with 
breakpoints.

> Btw, have you by any chance looked at how gdb determines which clone()s 
> constitute a new thread?

If I read the code correctly, GDB always assumes `PTRACE_EVENT_CLONE` is a new 
thread, and does not support spawning processes via it.


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

https://reviews.llvm.org/D98822

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

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/test/Shell/Subprocess/fork-follow-parent.test:6
+process launch
+# CHECK: function run in child
+# CHECK-NOT: function run in parent

labath wrote:
> mgorny wrote:
> > The tests sometimes fail if child starts after the breakpoint is hit (and 
> > therefore this check happens before stop reason below). Any suggestion how 
> > to make it work independently of where 'function run in child' happens?
> There are several ways around that, but I don't know which one is the best, 
> as I'm not sure what exactly you're trying to test. You could try to capture 
> this nondeterminism with CHECK-DAGs, but CHECK-DAGs don't combine well with 
> CHECK-NOTs. Another option is to make this predictable by adding 
> synchronization to the inferior. If you just wanted to check that the child 
> process finishes correctly, you could have the parent process check its exit 
> status and match that...
The goal is to check that parent is stopped before printing but the child is 
not. I suppose your last suggestion, i.e. checking for exit status, should be 
good enough.


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

https://reviews.llvm.org/D98822

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

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D98822#2658426 , @mgorny wrote:

> In D98822#2658252 , @labath wrote:
>
>> In D98822#2646052 , @mgorny wrote:
>>
>>> It includes refactoring clone/fork/vfork event monitoring into a single 
>>> function. Right now, only forks are supported — the handler creates a local 
>>> `NativeProcessLinux` instance (@labath, any suggestions how to make this 
>>> less hacky?), duplicates parent's breakpoints into it (just like the child 
>>> gets parent's memory) and uses it to clear these breakpoints and detach the 
>>> child process.
>>
>> I don't find this particularly hacky. However, I have doubts about the 
>> breakpoint clearing aspect. If we add that now, I think it will be tricky to 
>> move that process to the client in the future (how will the client know 
>> whether the breakpoints were auto-removed?) Given that  breakpoints in 
>> forked process have been borked since forever, I don't see harm in keeping 
>> that broken for a little while longer. Even a client with extremely limited 
>> of multiple processes (like the one we had in mind) should not have a 
>> problem with sending the appropriate `z` packets before it detaches. This 
>> patch could still come first (it's great that you've separated that out) -- 
>> it just needs to ensure that there are no breakpoints in the child path 
>> which would need clearing.
>
> Actually, that's not a problem at all. If client doesn't indicate 
> `fork-events` support via `qSupported`, we handle breakpoints server-side. If 
> it does, we deliver `fork` stop reason and then the client is responsible for 
> dealing with breakpoints.

Yeah, but then we have to maintain two copies of breakpoint-unsetting code in 
perpetuity.  Given that we've managed to come this far with this being 
completely broken, I think we can wait a little longer until the client support 
is in place.

>> Btw, have you by any chance looked at how gdb determines which clone()s 
>> constitute a new thread?
>
> If I read the code correctly, GDB always assumes `PTRACE_EVENT_CLONE` is a 
> new thread, and does not support spawning processes via it.

Ah, cute. I guess someone should let them know about this (and maybe also tell 
kernel folks that differentiating cloned processes is really hard). :)


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

https://reviews.llvm.org/D98822

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

[Lldb-commits] [lldb] ce03a86 - [lldb] Remove linux/mips debugging support

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-03-30T15:24:43+02:00
New Revision: ce03a862372a6f36d2fcf80dc80052aa155fcae8

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

LOG: [lldb] Remove linux/mips debugging support

As discussed on lldb-dev
 the
mips code is unmaintained and untested. It also carries a lot of
technical debt which is not limited to mips-specific code.

Generic mips support remains (and is going to be used by the upcoming
freebsd code). Resurrecting mips support should be a matter of re-adding
the relevant register context files (while avoiding reintroducing the
debt).

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/CMakeLists.txt
lldb/source/Plugins/Process/Utility/CMakeLists.txt
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

Removed: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
lldb/source/Plugins/Process/Utility/RegisterContextLinux_mips.cpp
lldb/source/Plugins/Process/Utility/RegisterContextLinux_mips.h
lldb/source/Plugins/Process/Utility/RegisterContextLinux_mips64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextLinux_mips64.h
lldb/source/Plugins/Process/Utility/lldb-mips-linux-register-enums.h

lldb/test/API/functionalities/postmortem/elf-core/linux-mips64el-gnuabi64.core

lldb/test/API/functionalities/postmortem/elf-core/linux-mips64el-gnuabi64.out

lldb/test/API/functionalities/postmortem/elf-core/linux-mips64el-gnuabin32.core

lldb/test/API/functionalities/postmortem/elf-core/linux-mips64el-gnuabin32.out

lldb/test/API/functionalities/postmortem/elf-core/linux-mipsel-gnuabio32.core
lldb/test/API/functionalities/postmortem/elf-core/linux-mipsel-gnuabio32.out



diff  --git a/lldb/source/Plugins/Process/Linux/CMakeLists.txt 
b/lldb/source/Plugins/Process/Linux/CMakeLists.txt
index dd2a88957f48..a68ff4ffd822 100644
--- a/lldb/source/Plugins/Process/Linux/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/Linux/CMakeLists.txt
@@ -3,7 +3,6 @@ add_lldb_library(lldbPluginProcessLinux
   NativeRegisterContextLinux.cpp
   NativeRegisterContextLinux_arm.cpp
   NativeRegisterContextLinux_arm64.cpp
-  NativeRegisterContextLinux_mips64.cpp
   NativeRegisterContextLinux_ppc64le.cpp
   NativeRegisterContextLinux_s390x.cpp
   NativeRegisterContextLinux_x86_64.cpp

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
deleted file mode 100644
index 4c8ad65b66a3..
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
+++ /dev/null
@@ -1,1036 +0,0 @@
-//===-- NativeRegisterContextLinux_mips64.cpp 
-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#if defined(__mips__)
-
-#include "NativeRegisterContextLinux_mips64.h"
-
-
-#include "Plugins/Process/Linux/NativeProcessLinux.h"
-#include "Plugins/Process/Linux/Procfs.h"
-#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
-#include "Plugins/Process/Utility/RegisterContextLinux_mips.h"
-#include "Plugins/Process/Utility/RegisterContextLinux_mips64.h"
-#include "lldb/Core/EmulateInstruction.h"
-#include "lldb/Host/Host.h"
-#include "lldb/Host/HostInfo.h"
-#include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/LLDBAssert.h"
-#include "lldb/Utility/Log.h"
-#include "lldb/Utility/RegisterValue.h"
-#include "lldb/Utility/Status.h"
-#include "lldb/lldb-enumerations.h"
-#include "lldb/lldb-private-enumerations.h"
-#define NT_MIPS_MSA 0x600
-#define CONFIG5_FRE (1 << 8)
-#define SR_FR (1 << 26)
-#define NUM_REGISTERS 32
-
-#include 
-#include 
-
-#ifndef PTRACE_GET_WATCH_REGS
-enum pt_watch_style { pt_watch_style_mips32, pt_watch_style_mips64 };
-struct mips32_watch_regs {
-  uint32_t watchlo[8];
-  uint16_t watchhi[8];
-  uint16_t watch_masks[8];
-  uint32_t num_valid;
-} __attribute__((aligned(8)));
-
-struct mips64_watch_regs {
-  uint64_t watchlo[8];
-  uint16_t watchhi[8];
-  uint16_t watch_masks[8];
-  uint32_t num_valid;
-} __attribute__((aligned(8)));
-
-struct pt_watch_regs {
-  enum pt_watch_style style;
-  union {
-struct mips32_watch_regs mips32;
-struct mi

[Lldb-commits] [lldb] 04b766d - [lldb/test] Deflake TestGdbRemote_vContThreads even more

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-03-30T17:03:14+02:00
New Revision: 04b766dab0d9d786ab5695336348b0c01646cf99

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

LOG: [lldb/test] Deflake TestGdbRemote_vContThreads even more

This patch fixes an issue, where if the thread has a signal blocked when
we try to inject it into the process (via vCont), then instead of
executing straight away, the injected signal will trigger another stop
when the thread unblocks the signal.

As (linux) threads start their life with SIGUSR1 (among others)
disabled, and only enable it during initialization, injecting the signal
during this window did not behave as expected. The fix is to change the
test to ensure the signal gets injected with the signal unblocked.

The simplest way to do this was to write a dedicated inferior for this
test. I also created a new header to factor out the function retrieving
the (os-specific) thread id.

Added: 
lldb/packages/Python/lldbsuite/test/make/thread.h
lldb/test/API/tools/lldb-server/vCont-threads/Makefile
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Modified: 
lldb/test/API/tools/lldb-server/main.cpp

Removed: 
lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py



diff  --git a/lldb/packages/Python/lldbsuite/test/make/thread.h 
b/lldb/packages/Python/lldbsuite/test/make/thread.h
new file mode 100644
index ..3cfa16b84761
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/make/thread.h
@@ -0,0 +1,35 @@
+#ifndef LLDB_THREAD_H
+#define LLDB_THREAD_H
+
+#include 
+
+#if defined(__APPLE__)
+__OSX_AVAILABLE_STARTING(__MAC_10_6, __IPHONE_3_2)
+int pthread_threadid_np(pthread_t, __uint64_t *);
+#elif defined(__linux__)
+#include 
+#include 
+#elif defined(__NetBSD__)
+#include 
+#elif defined(_WIN32)
+#include 
+#endif
+
+inline uint64_t get_thread_id() {
+#if defined(__APPLE__)
+  __uint64_t tid = 0;
+  pthread_threadid_np(pthread_self(), &tid);
+  return tid;
+#elif defined(__linux__)
+  return syscall(__NR_gettid);
+#elif defined(__NetBSD__)
+  // Technically lwpid_t is 32-bit signed integer
+  return static_cast(_lwp_self());
+#elif defined(_WIN32)
+  return static_cast(::GetCurrentThreadId());
+#else
+  return -1;
+#endif
+}
+
+#endif // LLDB_THREAD_H

diff  --git a/lldb/test/API/tools/lldb-server/main.cpp 
b/lldb/test/API/tools/lldb-server/main.cpp
index 8a14c11075f1..f719e4bc52f8 100644
--- a/lldb/test/API/tools/lldb-server/main.cpp
+++ b/lldb/test/API/tools/lldb-server/main.cpp
@@ -11,6 +11,7 @@
 #include 
 #include 
 #endif
+#include "thread.h"
 #include 
 #include 
 #include 
@@ -19,17 +20,6 @@
 #include 
 #include 
 
-#if defined(__APPLE__)
-__OSX_AVAILABLE_STARTING(__MAC_10_6, __IPHONE_3_2)
-int pthread_threadid_np(pthread_t, __uint64_t *);
-#elif defined(__linux__)
-#include 
-#elif defined(__NetBSD__)
-#include 
-#elif defined(_WIN32)
-#include 
-#endif
-
 static const char *const RETVAL_PREFIX = "retval:";
 static const char *const SLEEP_PREFIX = "sleep:";
 static const char *const STDERR_PREFIX = "stderr:";
@@ -70,26 +60,6 @@ static void print_pid() {
 #endif
 }
 
-static uint64_t get_thread_id() {
-// Put in the right magic here for your platform to spit out the thread id 
(tid)
-// that debugserver/lldb-gdbserver would see as a TID.
-#if defined(__APPLE__)
-  __uint64_t tid = 0;
-  pthread_threadid_np(pthread_self(), &tid);
-  return tid;
-#elif defined(__linux__)
-  // This is a call to gettid() via syscall.
-  return syscall(__NR_gettid);
-#elif defined(__NetBSD__)
-  // Technically lwpid_t is 32-bit signed integer
-  return static_cast(_lwp_self());
-#elif defined(_WIN32)
-  return static_cast(::GetCurrentThreadId());
-#else
-  return -1;
-#endif
-}
-
 static void signal_handler(int signo) {
 #if defined(_WIN32)
   // No signal support on Windows.

diff  --git a/lldb/test/API/tools/lldb-server/vCont-threads/Makefile 
b/lldb/test/API/tools/lldb-server/vCont-threads/Makefile
new file mode 100644
index ..32bbba57db6a
--- /dev/null
+++ b/lldb/test/API/tools/lldb-server/vCont-threads/Makefile
@@ -0,0 +1,5 @@
+CFLAGS_EXTRAS := -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS
+ENABLE_THREADS := YES
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py 
b/lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
similarity index 98%
rename from lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py
rename to 
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
index 246a588db890..c7ced621c3f9 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemote_vContThreads.py
+++ 
b/lldb/test/API/tools/lldb-server/vCont

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D98822#2658443 , @labath wrote:

> Yeah, but then we have to maintain two copies of breakpoint-unsetting code in 
> perpetuity.  Given that we've managed to come this far with this being 
> completely broken, I think we can wait a little longer until the client 
> support is in place.

Ok, I guess I can do that.

What about debug registers on FreeBSD? This system is pretty unique because 
child processes inherit dbregs from parent, and therefore we need to clear 
them. GDB doesn't do that and watchpoints crash forked children. Should we keep 
the clearing part as a hack specific to FreeBSD, or have lldb generic clear all 
hardware breakpoints and watchpoints on fork? Independently of that, I'm going 
to file a bug for FreeBSD to see if this is really the intended behavior.


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

https://reviews.llvm.org/D98822

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

[Lldb-commits] [lldb] bbae066 - [lldb] Fix TestStopOnSharedlibraryEvents.py on linux

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-03-30T17:38:51+02:00
New Revision: bbae06652e076a35b94acf84e67602ffbdc5c071

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

LOG: [lldb] Fix TestStopOnSharedlibraryEvents.py on linux

and hopefully other ELF OSes. The problem was a missing "extra_images"
startup argument (which ensures LD_LIBRARY_PATH is set properly).

Added: 


Modified: 

lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
 
b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
index 18962d1443e0..77d08fec3707 100644
--- 
a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
+++ 
b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
@@ -9,13 +9,13 @@ class TestStopOnSharedlibraryEvents(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @skipIfRemote
-@skipUnlessDarwin
+@skipIfWindows
 @no_debug_info_test
 def test_stopping_breakpoints(self):
 self.do_test()
 
 @skipIfRemote
-@skipUnlessDarwin
+@skipIfWindows
 @no_debug_info_test
 def test_auto_continue(self):
 def auto_continue(bkpt):
@@ -23,15 +23,15 @@ def auto_continue(bkpt):
 self.do_test(auto_continue)
 
 @skipIfRemote
+@skipIfWindows
 @no_debug_info_test
-@skipUnlessDarwin
 def test_failing_condition(self):
 def condition(bkpt):
 bkpt.SetCondition("1 == 2")
 self.do_test(condition)
 
 @skipIfRemote
-@skipUnlessDarwin
+@skipIfWindows
 @no_debug_info_test
 def test_continue_callback(self):
 def bkpt_callback(bkpt):
@@ -43,7 +43,8 @@ def do_test(self, bkpt_modifier = None):
 main_spec = lldb.SBFileSpec("main.cpp")
 # Launch and stop before the dlopen call.
 target, process, thread, _ = lldbutil.run_to_source_breakpoint(self,
-  "// Set a 
breakpoint here", main_spec)
+"// Set a breakpoint here", main_spec, extra_images=["load_a",
+"load_b"])
 
 # Now turn on shared library events, continue and make sure we stop 
for the event.
 self.runCmd("settings set target.process.stop-on-sharedlibrary-events 
1")



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

[Lldb-commits] [lldb] 9709186 - [lldb] Add missing include in TestGdbRemote_vContThreads test

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-03-30T17:38:52+02:00
New Revision: 9709186681a73489ecb76ea56cb4e1770f8da99a

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

LOG: [lldb] Add missing include in TestGdbRemote_vContThreads test

should fix the arm builtbots.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp 
b/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
index a0ac3ecc4f18..dc1fd7ef3f28 100644
--- a/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
+++ b/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
@@ -3,6 +3,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 



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

[Lldb-commits] [lldb] 0bbe2a3 - [lldb] More missing includes in TestGdbRemote_vContThreads

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-03-30T18:05:31+02:00
New Revision: 0bbe2a3c8aae29445b73423bd54baa72e515484d

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

LOG: [lldb] More missing includes in TestGdbRemote_vContThreads

Added: 


Modified: 
lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp 
b/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
index dc1fd7ef3f28..f70be42af1a9 100644
--- a/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
+++ b/lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
@@ -3,6 +3,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -13,8 +14,9 @@ pseudo_barrier_t barrier;
 
 static void sigusr1_handler(int signo) {
   char buf[100];
-  snprintf(buf, sizeof(buf), "received SIGUSR1 on thread id: %" PRIx64 "\n",
-   get_thread_id());
+  std::snprintf(buf, sizeof(buf),
+"received SIGUSR1 on thread id: %" PRIx64 "\n",
+get_thread_id());
   write(STDOUT_FILENO, buf, strlen(buf));
 }
 
@@ -36,7 +38,7 @@ int main(int argc, char **argv) {
 
   pseudo_barrier_wait(barrier);
 
-  puts("@started");
+  std::puts("@started");
 
   for (std::thread &thread : threads)
 thread.join();



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

[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.

Add a minimal support for the multiprocess extension in gdb-remote
client.  It accepts PIDs as part of thread-ids, and rejects PIDs that
do not match the current inferior.


https://reviews.llvm.org/D99603

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -341,7 +341,7 @@
 
   size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
 
-  size_t UpdateThreadIDsFromStopReplyThreadsValue(std::string &value);
+  size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
 
   bool HandleNotifyPacket(StringExtractorGDBRemote &packet);
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1495,22 +1495,22 @@
   m_thread_pcs.clear();
 }
 
-size_t
-ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(std::string &value) {
+size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
+llvm::StringRef value) {
   m_thread_ids.clear();
-  size_t comma_pos;
-  lldb::tid_t tid;
-  while ((comma_pos = value.find(',')) != std::string::npos) {
-value[comma_pos] = '\0';
-// thread in big endian hex
-tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-if (tid != LLDB_INVALID_THREAD_ID)
-  m_thread_ids.push_back(tid);
-value.erase(0, comma_pos + 1);
-  }
-  tid = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_THREAD_ID, 16);
-  if (tid != LLDB_INVALID_THREAD_ID)
-m_thread_ids.push_back(tid);
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
+  StringExtractorGDBRemote thread_ids{value};
+
+  do {
+auto pid_tid = thread_ids.GetPidTid(pid);
+if (pid_tid && pid_tid->first == pid) {
+  lldb::tid_t tid = pid_tid->second;
+  if (tid != LLDB_INVALID_THREAD_ID &&
+  tid != StringExtractorGDBRemote::AllProcesses)
+m_thread_ids.push_back(tid);
+}
+  } while (thread_ids.GetChar() == ',');
+
   return m_thread_ids.size();
 }
 
@@ -1527,7 +1527,7 @@
 value.erase(0, comma_pos + 1);
   }
   pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-  if (pc != LLDB_INVALID_THREAD_ID)
+  if (pc != LLDB_INVALID_ADDRESS)
 m_thread_pcs.push_back(pc);
   return m_thread_pcs.size();
 }
@@ -2146,6 +2146,7 @@
 }
 
 StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
+  lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
   stop_packet.SetFilePos(0);
   const char stop_type = stop_packet.GetChar();
   switch (stop_type) {
@@ -2160,11 +2161,8 @@
 if (stop_id == 0) {
   // Our first stop, make sure we have a process ID, and also make sure we
   // know about our registers
-  if (GetID() == LLDB_INVALID_PROCESS_ID) {
-lldb::pid_t pid = m_gdb_comm.GetCurrentProcessID();
-if (pid != LLDB_INVALID_PROCESS_ID)
+  if (GetID() == LLDB_INVALID_PROCESS_ID && pid != LLDB_INVALID_PROCESS_ID)
   SetID(pid);
-  }
   BuildDynamicRegisterInfo(true);
 }
 // Stop with signal and thread info
@@ -2196,24 +2194,17 @@
 value.getAsInteger(16, x);
 exc_data.push_back(x);
   } else if (key.compare("thread") == 0) {
-// thread in big endian hex
-if (value.getAsInteger(16, tid))
+// thread-id
+StringExtractorGDBRemote thread_id{value};
+auto pid_tid = thread_id.GetPidTid(pid);
+if (pid_tid && pid_tid->first == pid)
+  tid = pid_tid->second;
+else
   tid = LLDB_INVALID_THREAD_ID;
   } else if (key.compare("threads") == 0) {
 std::lock_guard guard(
 m_thread_list_real.GetMutex());
-
-m_thread_ids.clear();
-// A comma separated list of all threads in the current
-// process that includes the thread for this stop reply packet
-lldb::tid_t tid;
-while (!value.empty()) {
-  llvm::StringRef tid_str;
-  std::tie(tid_str, value) = value.split(',');
-  if (tid_str.getAsInteger(16, tid))
-tid = LLDB_INVALID_THREAD_ID;
-  m_thread_ids.push_back(tid);
-}
+UpdateThreadIDsFromStopReplyThreadsValue(value);
   } else if (key.compare("thread-pcs") == 0) {
 m_thread_pcs.clear();
 // A comma separated list of all 

[Lldb-commits] [PATCH] D99603: [lldb] [client] Support for multiprocess extension

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

@labath, split `GetCurrentThreadIDs()` as suggested. I've also modified 
`GetCurrentProcessID()` to prefer explicit PID over TID when available.


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

https://reviews.llvm.org/D99603

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

[Lldb-commits] [PATCH] D98482: [lldb] [server] Support for multiprocess extension

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

Looks like this broke the Windows lldb buildbot:

https://lab.llvm.org/buildbot/#/builders/83/builds/5173


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98482

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

[Lldb-commits] [lldb] c62ef12 - [lldb] [test] Mark more lldb-server tests xfail on Windows

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-03-30T18:49:04+02:00
New Revision: c62ef12079bcc7ce72040dddaae13408b120d995

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

LOG: [lldb] [test] Mark more lldb-server tests xfail on Windows

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py 
b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index 6677cfff8483..0764e3d6c75a 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -426,16 +426,19 @@ def Hg_fails_on_pid(self, pass_pid):
 
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(oslist=["windows"])
 def test_Hg_fails_on_another_pid(self):
 self.build()
 self.set_inferior_startup_launch()
 self.Hg_fails_on_pid(1)
 
+@expectedFailureAll(oslist=["windows"])
 def test_Hg_fails_on_zero_pid(self):
 self.build()
 self.set_inferior_startup_launch()
 self.Hg_fails_on_pid(0)
 
+@expectedFailureAll(oslist=["windows"])
 def test_Hg_fails_on_minus_one_pid(self):
 self.build()
 self.set_inferior_startup_launch()



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

[Lldb-commits] [PATCH] D98482: [lldb] [server] Support for multiprocess extension

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D98482#2658991 , @stella.stamenova 
wrote:

> Looks like this broke the Windows lldb buildbot:
>
> https://lab.llvm.org/buildbot/#/builders/83/builds/5173

Thanks for pinging me. It seems that all Hg tests fail on Windows, so just 
added xfail to these 3 as well.




Comment at: lldb/include/lldb/Utility/StringExtractorGDBRemote.h:197
+  static constexpr lldb::pid_t AllProcesses = UINT64_MAX;
+  static constexpr lldb::tid_t AllThreads = UINT64_MAX;
+

labath wrote:
> These also need a definition in the .cpp file (debug builds fail without 
> that).
I've reproduced that and fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98482

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

[Lldb-commits] [PATCH] D99571: Update ProcessMachCore::DoLoadCore to handle binary hints with and without addresses

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: 
lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py:142
+main_addr = main_sym.GetStartAddress()
+self.assertGreater(main_addr.GetLoadAddress(self.target), 
0x700)
+self.assertNotEqual(main_addr.GetLoadAddress(self.target), 
lldb.LLDB_INVALID_ADDRESS)

`0x700` feels like it should be a variable with a name since we are 
using in multiple places.



Comment at: 
lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp:90
+char buf[24];
+sprintf(buf, "0x%llx", address);
+ident += buf;

`snprintf` we should also collect the return value and assert on the result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99571

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

[Lldb-commits] [PATCH] D99571: Update ProcessMachCore::DoLoadCore to handle binary hints with and without addresses

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:214
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+module_sp.reset(new Module(module_spec));
+  }

We can use `make_shared` here instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99571

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

[Lldb-commits] [lldb] e3d3327 - [lldb] Remove reproducer from previous test run

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-03-30T10:11:20-07:00
New Revision: e3d3327edbf133da6ed50767eed4560a541a751d

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

LOG: [lldb] Remove reproducer from previous test run

Added: 


Modified: 
lldb/test/Shell/Reproducer/Functionalities/TestImageList.test

Removed: 




diff  --git a/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test 
b/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
index ec8b36ea9576c..e630e4666d087 100644
--- a/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
+++ b/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
@@ -6,6 +6,7 @@
 # RUN: %clang_host %S/Inputs/stepping.c -g -o %t.out
 
 # RUN: rm -rf %t.txt
+# RUN: rm -rf %t.repro
 
 # RUN: echo "CAPTURE" >> %t.txt
 # RUN: %lldb -x -b  --capture --capture-path %t.repro \



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

[Lldb-commits] [PATCH] D98761: Fix "image lookup --address" Summary results for inline functions.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 334274.
clayborg added a comment.

Remove show_inline_callsite_line_info setting from SymbolContext dumping 
function as it isn't needed, show_inlined_frames controls this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98761

Files:
  lldb/include/lldb/Symbol/SymbolContext.h
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/Trace.cpp
  lldb/test/Shell/Commands/command-image-lookup.yaml

Index: lldb/test/Shell/Commands/command-image-lookup.yaml
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-image-lookup.yaml
@@ -0,0 +1,810 @@
+# RUN: yaml2obj %s -o %T/a.out
+# RUN: %lldb %T/a.out -o "image lookup --verbose --address 0x00013fa1" -o exit | FileCheck %s --check-prefix=NOINLINE
+# RUN: %lldb %T/a.out -o "image lookup --verbose --address 0x00013fa2" -o exit | FileCheck %s --check-prefix=INLINE_1
+# RUN: %lldb %T/a.out -o "image lookup --verbose --address 0x00013fa8" -o exit | FileCheck %s --check-prefix=INLINE_2
+
+#  NOINLINE: Summary: a.out`main + 33 at main.cpp:10
+# NOINLINE-NEXT: Module: file =
+
+#  INLINE_1: Summary: a.out`main + 34 [inlined] squares(int, int) at main.cpp:7:16
+# INLINE_1-NEXT:  a.out`main + 34 at main.cpp:11
+# INLINE_1-NEXT: Module: file =
+
+#  INLINE_2: Summary: a.out`main + 40 [inlined] square(int) at main.cpp:3:9
+# INLINE_2-NEXT: a.out`main + 40 [inlined] squares(int, int) + 6 at main.cpp:7
+# INLINE_2-NEXT: a.out`main + 34 at main.cpp:11
+# INLINE_2-NEXT: Module: file =
+
+
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x107
+  cpusubtype:  0x3
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1400
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:E476BFB9-CC5C-34BC-B968-BF996B298060
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   659200
+sdk: 721152
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   4
+stroff:  4160
+strsize: 54
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  16384
+fileoff: 0
+filesize:0
+maxprot: 5
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x13F50
+size:100
+offset:  0x0
+align:   4
+reloff:  0x0
+nreloc:  0
+flags:   0x8400
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE070103000A00070078051B001800E476BFB9CC5C34BCB968BF996B29806032001800010F0A010B000200181004004010
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x13FB4
+size:72
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE070103000A00070078051B001800E476BFB9CC5C34BCB968BF996B29806032001800010F0A00
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __LINKEDIT
+vmaddr:  4294983680
+vmsize:  4096
+fileoff: 4096
+filesize:118
+maxprot: 1
+initprot:1
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 952
+segname: __DWARF
+vmaddr:  4294987776
+vmsize:  4096
+fileoff: 8192
+filesize:1530
+maxprot: 7
+initprot:3
+nsects:  11
+flags:   0
+Sections:
+  - sectname:__debug_line
+segname: __DWARF
+addr:0x15000
+size:130
+offset:  0x2000
+align:   0
+reloff:  0x0
+nreloc: 

[Lldb-commits] [PATCH] D98886: Pass pointer authentication code mask from minidump and use to strip pac from pc.

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/include/lldb/Target/Process.h:79
   void SetExtraStartupCommands(const Args &args);
+  uint64_t GetPointerAuthenticationAddressMask() const;
+  void SetPointerAuthenticationAddressMask(const uint64_t mask);

This function name is too specific to AArch64 architecture. IMO, we should have 
information on significant address bits rather than PAuth mask. This is because 
we have to cover for the top byte in case of AArch64 Top Byte Ignore feature as 
well as any other memory management features.

From user process perspective we should figure out how many bits of the process 
memory address are significant for addressing while the others store extra 
information like PAC, Tags or any information inserted by software in top byte.

I propose to add a new variable (may be call it address_bits_in_use) in process 
class which is populated by default equal to process address width in our case 
64 bit. In case a we choose to update address_bits_in_use we may do it on when 
process is created or through set method during execution as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98886

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

[Lldb-commits] [PATCH] D98886: Pass pointer authentication code mask from minidump and use to strip pac from pc.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc added inline comments.



Comment at: lldb/include/lldb/Target/Process.h:79
   void SetExtraStartupCommands(const Args &args);
+  uint64_t GetPointerAuthenticationAddressMask() const;
+  void SetPointerAuthenticationAddressMask(const uint64_t mask);

omjavaid wrote:
> This function name is too specific to AArch64 architecture. IMO, we should 
> have information on significant address bits rather than PAuth mask. This is 
> because we have to cover for the top byte in case of AArch64 Top Byte Ignore 
> feature as well as any other memory management features.
> 
> From user process perspective we should figure out how many bits of the 
> process memory address are significant for addressing while the others store 
> extra information like PAC, Tags or any information inserted by software in 
> top byte.
> 
> I propose to add a new variable (may be call it address_bits_in_use) in 
> process class which is populated by default equal to process address width in 
> our case 64 bit. In case a we choose to update address_bits_in_use we may do 
> it on when process is created or through set method during execution as well.
I don't think we want to clear the top byte if TBI is enabled. This is because 
the top byte may contain a pointer tag that is necessary in order to access the 
pointer with MTE. That is exactly what a mask would let us do. The top byte of 
the mask is clear when TBI is enabled so that the pointer tag is left unchanged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98886

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

[Lldb-commits] [PATCH] D98886: Pass pointer authentication code mask from minidump and use to strip pac from pc.

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/include/lldb/Target/Process.h:79
   void SetExtraStartupCommands(const Args &args);
+  uint64_t GetPointerAuthenticationAddressMask() const;
+  void SetPointerAuthenticationAddressMask(const uint64_t mask);

pcc wrote:
> omjavaid wrote:
> > This function name is too specific to AArch64 architecture. IMO, we should 
> > have information on significant address bits rather than PAuth mask. This 
> > is because we have to cover for the top byte in case of AArch64 Top Byte 
> > Ignore feature as well as any other memory management features.
> > 
> > From user process perspective we should figure out how many bits of the 
> > process memory address are significant for addressing while the others 
> > store extra information like PAC, Tags or any information inserted by 
> > software in top byte.
> > 
> > I propose to add a new variable (may be call it address_bits_in_use) in 
> > process class which is populated by default equal to process address width 
> > in our case 64 bit. In case a we choose to update address_bits_in_use we 
> > may do it on when process is created or through set method during execution 
> > as well.
> I don't think we want to clear the top byte if TBI is enabled. This is 
> because the top byte may contain a pointer tag that is necessary in order to 
> access the pointer with MTE. That is exactly what a mask would let us do. The 
> top byte of the mask is clear when TBI is enabled so that the pointer tag is 
> left unchanged.
We dont really want to clear the top byte but rather have the information on 
"address bits used for addressing" vs "address bits used for extra features" 
and  for the purpose of unwinding all extra information from the address needs 
to cleared.

On top of my head I think this information is needed to ensure LLDB does not 
use multiple watchpoints for watching a single address which has extra info in 
some of its top bits. There could be more uses of this in memory reads and 
breakpoints management as well for code pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98886

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

[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 334296.
mgorny marked 4 inline comments as done.
mgorny added a comment.

Removed software breakpoint cleanup. Modified tests not to expect child's 
output at any specific point.


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

https://reviews.llvm.org/D98822

Files:
  lldb/include/lldb/Host/linux/Host.h
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/test/Shell/Subprocess/Inputs/clone.c
  lldb/test/Shell/Subprocess/Inputs/fork.c
  lldb/test/Shell/Subprocess/Inputs/vfork.c
  lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
  lldb/test/Shell/Subprocess/clone-follow-parent.test
  lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/fork-follow-parent.test
  lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/vfork-follow-parent.test

Index: lldb/test/Shell/Subprocess/vfork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent.test
@@ -0,0 +1,10 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/vfork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
@@ -0,0 +1,12 @@
+# REQUIRES: native && (target-x86 || target-x86_64 || target-aarch64) && dbregs-set
+# RUN: %clang_host -g %p/Inputs/vfork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent.test
@@ -0,0 +1,10 @@
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
@@ -0,0 +1,12 @@
+# REQUIRES: native && (target-x86 || target-x86_64 || target-aarch64) && dbregs-set
+# RUN: %clang_host -g %p/Inputs/fork.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/clone-follow-parent.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/clone-follow-parent.test
@@ -0,0 +1,10 @@
+# REQUIRES: native && (system-linux || system-netbsd)
+# RUN: %clang_host %p/Inputs/clone.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0
Index: lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
@@ -0,0 +1,12 @@
+# REQUIRES: native && (system-linux || system-netbsd) && (target-x86 || target-x86_64 || target-aarch64) && dbregs-set
+# RUN: %clang_host -g %p/Inputs/clone.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+process launch -s
+watchpoint set variable -w write g_val
+# CHECK: Watchpoint created:
+continue
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = watchpoint
+continue
+# CHECK: function run in pa

[Lldb-commits] [lldb] 9ab6771 - [LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2021-03-31T04:39:14+05:00
New Revision: 9ab677180091a690cd99d4ac55d5fb9e1149b1ec

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

LOG: [LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset

This patch adds a test case to test AArch64 dynamic register sets.
This tests for the availability of certain register sets and query
their registers accordingly.

Reviewed By: labath, DavidSpickett

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

Added: 
lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile

lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c

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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 958cadd3a7c81..a9928af677a6e 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1269,7 +1269,7 @@ def isPPC64le(self):
 return True
 return False
 
-def isAArch64SVE(self):
+def getCPUInfo(self):
 triple = self.dbg.GetSelectedPlatform().GetTriple()
 
 # TODO other platforms, please implement this function
@@ -1290,7 +1290,16 @@ def isAArch64SVE(self):
 except:
 return False
 
-return " sve " in cpuinfo
+return cpuinfo
+
+def isAArch64SVE(self):
+return "sve" in self.getCPUInfo()
+
+def isAArch64MTE(self):
+return "mte" in self.getCPUInfo()
+
+def isAArch64PAuth(self):
+return "paca" in self.getCPUInfo()
 
 def hasLinuxVmFlags(self):
 """ Check that the target machine has "VmFlags" lines in

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile 
b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile
new file mode 100644
index 0..5fc881c2db97c
--- /dev/null
+++ b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := main.c
+
+CFLAGS_EXTRAS := -march=armv8-a+sve
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
 
b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
new file mode 100644
index 0..ecaefb0e657e8
--- /dev/null
+++ 
b/lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
@@ -0,0 +1,109 @@
+"""
+Test AArch64 dynamic register sets
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Expected a register named %s' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Expected a register %s size == %i bytes' % (name, 
expected))
+
+def sve_regs_read_dynamic(self, sve_registers):
+vg_reg = sve_registers.GetChildMemberWithName("vg")
+vg_reg_value = sve_registers.GetChildMemberWithName(
+"vg").GetValueAsUnsigned()
+
+z_reg_size = vg_reg_value * 8
+p_reg_size = int(z_reg_size / 8)
+
+for i in range(32):
+z_regs_value = '{' + \
+' '.join('0x{:02x}'.format(i + 1)
+ for _ in range(z_reg_size)) + '}'
+self.expect('register read z%i' %
+(i), substrs=[z_regs_value])
+
+# Set P registers with random test values. The P registers are 
predicate
+# registers, which hold one bit for each byte available in a Z 
register.
+# For below mentioned values of P registers, P(0,5,10,15) will have all
+# Z register lanes set while P(4,9,14) will have no lanes set.
+p_value_bytes = ['0xff', '0x55', '0x11', '0x01', '0x00']
+for i in range(16):
+p_regs_value = '{' + \
+' '.join(p_value_bytes[i % 5] for _ in range(p_reg_size)) + '}'
+self.expect('register read p%i' % (i), substrs=[p_regs_value])
+
+self.expect("register read ffr", substrs=[p_regs_value])
+
+for i in range(32):
+z_regs_value = '{' + \
+' '.join('0x{:02x}'.format(32 - i)
+ for _ in range(z_reg_size)) + '}'
+self.runCmd("register write z%i '%s'" % (i, z_regs_value))
+

[Lldb-commits] [lldb] d6d3d21 - [LLDB] Add support for Arm64/Linux dynamic register sets

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2021-03-31T04:38:36+05:00
New Revision: d6d3d21cd1cb1567eaf7ff8c0867b07227a19d99

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

LOG: [LLDB] Add support for Arm64/Linux dynamic register sets

This is patch adds support for adding dynamic register sets for
AArch64 dynamic features in LLDB. AArch64 has optional features like
SVE, Pointer Authentication and MTE which means LLDB needs to decide
at run time which registers it needs to pull in for the current
executable based on underlying support for a certain feature.

This patch makes necessary adjustments to make way for dynamic
register infos and dynamic register sets.

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 5c025c0ecb43a..09cf72c044822 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -46,18 +46,37 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
   case llvm::Triple::arm:
 return std::make_unique(target_arch,
  native_thread);
-  case llvm::Triple::aarch64:
-return std::make_unique(target_arch,
-   native_thread);
+  case llvm::Triple::aarch64: {
+// Configure register sets supported by this AArch64 target.
+// Read SVE header to check for SVE support.
+struct user_sve_header sve_header;
+struct iovec ioVec;
+ioVec.iov_base = &sve_header;
+ioVec.iov_len = sizeof(sve_header);
+unsigned int regset = NT_ARM_SVE;
+
+Flags opt_regsets;
+if (NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET,
+  native_thread.GetID(), ®set,
+  &ioVec, sizeof(sve_header))
+.Success())
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskSVE);
+
+auto register_info_up =
+std::make_unique(target_arch, opt_regsets);
+return std::make_unique(
+target_arch, native_thread, std::move(register_info_up));
+  }
   default:
 llvm_unreachable("have no register context for architecture");
   }
 }
 
 NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
-const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
-: NativeRegisterContextRegisterInfo(
-  native_thread, new RegisterInfoPOSIX_arm64(target_arch)) {
+const ArchSpec &target_arch, NativeThreadProtocol &native_thread,
+std::unique_ptr register_info_up)
+: NativeRegisterContextRegisterInfo(native_thread,
+register_info_up.release()) {
   ::memset(&m_fpr, 0, sizeof(m_fpr));
   ::memset(&m_gpr_arm64, 0, sizeof(m_gpr_arm64));
   ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));
@@ -75,8 +94,10 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_sve_buffer_is_valid = false;
   m_sve_header_is_valid = false;
 
-  // SVE is not enabled until we query user_sve_header
-  m_sve_state = SVEState::Unknown;
+  if (GetRegisterInfo().IsSVEEnabled())
+m_sve_state = SVEState::Unknown;
+  else
+m_sve_state = SVEState::Disabled;
 }
 
 RegisterInfoPOSIX_arm64 &
@@ -451,10 +472,7 @@ bool NativeRegisterContextLinux_arm64::IsFPR(unsigned reg) 
const {
 }
 
 bool NativeRegisterContextLinux_arm64::IsSVE(unsigned reg) const {
-  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
-  RegisterInfoPOSIX_arm64::SVERegSet)
-return true;
-  return false;
+  return GetRegisterInfo().IsSVEReg(reg);
 }
 
 llvm::Error NativeRegisterContextLinux_arm64::ReadHardwareDebugInfo() {
@@ -676,23 +694,27 @@ Status NativeRegisterContextLinux_arm64::WriteAllSVE() {
 }
 
 void NativeRegisterContextLinux_arm64::ConfigureRegisterContext() {
-  // Read SVE configuration data and configure register infos.
+  // ConfigureRegisterContext gets called from InvalidateAllRegisters
+  // on eve

[Lldb-commits] [PATCH] D96463: [LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ab677180091: [LLDB] Arm64/Linux test case for MTE and 
Pointer Authentication regset (authored by omjavaid).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D96463?vs=331836&id=334304#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96463

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/commands/register/register/aarch64_dynamic_regset/Makefile
  
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
  lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c

Index: lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c
@@ -0,0 +1,72 @@
+#include 
+
+void set_sve_registers() {
+  // AArch64 SVE extension ISA adds a new set of vector and predicate registers:
+  // 32 Z registers, 16 P registers, and 1 FFR register.
+  // Code below populates SVE registers to be read back by the debugger via
+  // ptrace interface at runtime.
+  // The P registers are predicate registers and hold one bit for each byte
+  // available in a Z vector register. For example, an SVE implementation with
+  // 1024-bit Z registers has 128-bit predicate registers.
+  // ptrue/pfalse instruction is used to set a predicate lane with a pattern.
+  // pattern is decided based on size specifier, b, h, s and d. if size
+  // specified is b all lanes will be set to 1. which is needed to set all bytes
+  // in a Z registers to the specified value.
+  asm volatile("setffr\n\t");
+  asm volatile("ptrue p0.b\n\t");
+  asm volatile("ptrue p1.h\n\t");
+  asm volatile("ptrue p2.s\n\t");
+  asm volatile("ptrue p3.d\n\t");
+  asm volatile("pfalse p4.b\n\t");
+  asm volatile("ptrue p5.b\n\t");
+  asm volatile("ptrue p6.h\n\t");
+  asm volatile("ptrue p7.s\n\t");
+  asm volatile("ptrue p8.d\n\t");
+  asm volatile("pfalse p9.b\n\t");
+  asm volatile("ptrue p10.b\n\t");
+  asm volatile("ptrue p11.h\n\t");
+  asm volatile("ptrue p12.s\n\t");
+  asm volatile("ptrue p13.d\n\t");
+  asm volatile("pfalse p14.b\n\t");
+  asm volatile("ptrue p15.b\n\t");
+
+  asm volatile("cpy  z0.b, p0/z, #1\n\t");
+  asm volatile("cpy  z1.b, p5/z, #2\n\t");
+  asm volatile("cpy  z2.b, p10/z, #3\n\t");
+  asm volatile("cpy  z3.b, p15/z, #4\n\t");
+  asm volatile("cpy  z4.b, p0/z, #5\n\t");
+  asm volatile("cpy  z5.b, p5/z, #6\n\t");
+  asm volatile("cpy  z6.b, p10/z, #7\n\t");
+  asm volatile("cpy  z7.b, p15/z, #8\n\t");
+  asm volatile("cpy  z8.b, p0/z, #9\n\t");
+  asm volatile("cpy  z9.b, p5/z, #10\n\t");
+  asm volatile("cpy  z10.b, p10/z, #11\n\t");
+  asm volatile("cpy  z11.b, p15/z, #12\n\t");
+  asm volatile("cpy  z12.b, p0/z, #13\n\t");
+  asm volatile("cpy  z13.b, p5/z, #14\n\t");
+  asm volatile("cpy  z14.b, p10/z, #15\n\t");
+  asm volatile("cpy  z15.b, p15/z, #16\n\t");
+  asm volatile("cpy  z16.b, p0/z, #17\n\t");
+  asm volatile("cpy  z17.b, p5/z, #18\n\t");
+  asm volatile("cpy  z18.b, p10/z, #19\n\t");
+  asm volatile("cpy  z19.b, p15/z, #20\n\t");
+  asm volatile("cpy  z20.b, p0/z, #21\n\t");
+  asm volatile("cpy  z21.b, p5/z, #22\n\t");
+  asm volatile("cpy  z22.b, p10/z, #23\n\t");
+  asm volatile("cpy  z23.b, p15/z, #24\n\t");
+  asm volatile("cpy  z24.b, p0/z, #25\n\t");
+  asm volatile("cpy  z25.b, p5/z, #26\n\t");
+  asm volatile("cpy  z26.b, p10/z, #27\n\t");
+  asm volatile("cpy  z27.b, p15/z, #28\n\t");
+  asm volatile("cpy  z28.b, p0/z, #29\n\t");
+  asm volatile("cpy  z29.b, p5/z, #30\n\t");
+  asm volatile("cpy  z30.b, p10/z, #31\n\t");
+  asm volatile("cpy  z31.b, p15/z, #32\n\t");
+}
+
+int main() {
+  if (getauxval(AT_HWCAP) & HWCAP_SVE) // check if SVE is present
+set_sve_registers();
+
+  return 0; // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
@@ -0,0 +1,109 @@
+"""
+Test AArch64 dynamic register sets
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Expected a register named %s' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Expected a register %s size == %i bytes' % (name, expected))
+
+def sve_regs_read_dynamic(self, sve_registers):
+vg_reg = sve

[Lldb-commits] [lldb] 1164b4e - [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2021-03-31T04:39:00+05:00
New Revision: 1164b4e2957290e814c3dd781a68e504dd39148e

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

LOG: [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers

This patch adds two new dynamic register sets for AArch64 MTE and
Pointer Authentication features. These register sets are dynamic and
will only be available if underlying hardware support either of these
features. LLDB will pull in Aux vector information and create register
infos based on that information.

A follow up patch will add a test case to test these feature registers.

Reviewed By: labath, DavidSpickett

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
lldb/source/Plugins/Process/POSIX/NativeProcessELF.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 09cf72c044822..50c299b030edf 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -33,6 +33,17 @@
 #define NT_ARM_SVE 0x405 /* ARM Scalable Vector Extension */
 #endif
 
+#ifndef NT_ARM_PAC_MASK
+#define NT_ARM_PAC_MASK 0x406 /* Pointer authentication code masks */
+#endif
+
+#ifndef NT_ARM_TAGGED_ADDR_CTRL
+#define NT_ARM_TAGGED_ADDR_CTRL 0x409 /* Tagged address control register */
+#endif
+
+#define HWCAP_PACA (1 << 30)
+#define HWCAP2_MTE (1 << 18)
+
 #define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
 
 using namespace lldb;
@@ -62,6 +73,18 @@ 
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
 .Success())
   opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskSVE);
 
+NativeProcessLinux &process = native_thread.GetProcess();
+
+llvm::Optional auxv_at_hwcap =
+process.GetAuxValue(AuxVector::AUXV_AT_HWCAP);
+if (auxv_at_hwcap && (*auxv_at_hwcap & HWCAP_PACA))
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskPAuth);
+
+llvm::Optional auxv_at_hwcap2 =
+process.GetAuxValue(AuxVector::AUXV_AT_HWCAP2);
+if (auxv_at_hwcap && (*auxv_at_hwcap2 & HWCAP2_MTE))
+  opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
+
 auto register_info_up =
 std::make_unique(target_arch, opt_regsets);
 return std::make_unique(
@@ -82,6 +105,9 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));
   ::memset(&m_hbp_regs, 0, sizeof(m_hbp_regs));
   ::memset(&m_sve_header, 0, sizeof(m_sve_header));
+  ::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
+
+  m_mte_ctrl_reg = 0;
 
   // 16 is just a maximum value, query hardware for actual watchpoint count
   m_max_hwp_supported = 16;
@@ -93,6 +119,8 @@ 
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_fpu_is_valid = false;
   m_sve_buffer_is_valid = false;
   m_sve_header_is_valid = false;
+  m_pac_mask_is_valid = false;
+  m_mte_ctrl_is_valid = false;
 
   if (GetRegisterInfo().IsSVEEnabled())
 m_sve_state = SVEState::Unknown;
@@ -229,6 +257,22 @@ NativeRegisterContextLinux_arm64::ReadRegister(const 
RegisterInfo *reg_info,
 src = (uint8_t *)GetSVEBuffer() + offset;
   }
 }
+  } else if (IsPAuth(reg)) {
+error = ReadPAuthMask();
+if (error.Fail())
+  return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetPAuthOffset();
+assert(offset < GetPACMaskSize());
+src = (uint8_t *)GetPACMask() + offset;
+  } else if (IsMTE(reg)) {
+error = ReadMTEControl();
+if (error.Fail())
+  return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset();
+assert(offset < GetMTEControlSize());
+src = (uint8_t *)GetMTEControl() + offset;
   } else
 return Status("failed - register wasn't recognized to be a GPR or an FPR, "
   "write strategy unknown");
@@ -387,6 +431,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
 return WriteAllSVE();
   }
 }
+  } else if (IsMTE(reg)) {
+error = ReadMTEControl();
+if (error.Fail())
+  return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset();
+assert(offset < GetMTEControlSize());
+dst = (uint8_t *)GetM

[Lldb-commits] [PATCH] D96458: [LLDB] Add support for Arm64/Linux dynamic register sets

[Muhammad Omair Javaid 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 rGd6d3d21cd1cb: [LLDB] Add support for Arm64/Linux dynamic 
register sets (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96458

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

Index: lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -167,9 +167,8 @@
 
 switch (arch.GetMachine()) {
 case llvm::Triple::aarch64:
-  m_thread_reg_ctx_sp = std::make_shared(
-  *this, std::make_unique(arch),
-  m_gpregset_data, m_notes);
+  m_thread_reg_ctx_sp = RegisterContextCorePOSIX_arm64::Create(
+  *this, arch, m_gpregset_data, m_notes);
   break;
 case llvm::Triple::arm:
   m_thread_reg_ctx_sp = std::make_shared(
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -18,11 +18,10 @@
 
 class RegisterContextCorePOSIX_arm64 : public RegisterContextPOSIX_arm64 {
 public:
-  RegisterContextCorePOSIX_arm64(
-  lldb_private::Thread &thread,
-  std::unique_ptr register_info,
-  const lldb_private::DataExtractor &gpregset,
-  llvm::ArrayRef notes);
+  static std::unique_ptr
+  Create(lldb_private::Thread &thread, const lldb_private::ArchSpec &arch,
+ const lldb_private::DataExtractor &gpregset,
+ llvm::ArrayRef notes);
 
   ~RegisterContextCorePOSIX_arm64() override;
 
@@ -39,6 +38,13 @@
   bool HardwareSingleStep(bool enable) override;
 
 protected:
+  RegisterContextCorePOSIX_arm64(
+  lldb_private::Thread &thread,
+  std::unique_ptr register_info,
+  const lldb_private::DataExtractor &gpregset,
+  const lldb_private::DataExtractor &sveregset,
+  llvm::ArrayRef notes);
+
   bool ReadGPR() override;
 
   bool ReadFPR() override;
Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -17,10 +17,29 @@
 
 using namespace lldb_private;
 
+std::unique_ptr
+RegisterContextCorePOSIX_arm64::Create(Thread &thread, const ArchSpec &arch,
+   const DataExtractor &gpregset,
+   llvm::ArrayRef notes) {
+  DataExtractor sveregset =
+  getRegset(notes, arch.GetTriple(), AARCH64_SVE_Desc);
+
+  Flags opt_regsets = RegisterInfoPOSIX_arm64::eRegsetMaskDefault;
+  if (sveregset.GetByteSize() > sizeof(sve::user_sve_header))
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskSVE);
+  auto register_info_up =
+  std::make_unique(arch, opt_regsets);
+  return std::unique_ptr(
+  new RegisterContextCorePOSIX_arm64(thread, std::move(register_info_up),
+ gpregset, sveregset, notes));
+}
+
 RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
 Thread &thread, std::unique_ptr register_info,
-const DataExtractor &gpregset, llvm::ArrayRef notes)
-: RegisterContextPOSIX_arm64(thread, std::move(register_info)) {
+const DataExtractor &gpregset, const DataExtractor &sveregset,
+llvm::ArrayRef notes)
+: RegisterContextPOSIX_arm64(thread, std::move(register_info)),
+  m_sveregset(sveregset) {
   m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
   gpregset.GetByteSize());
   m_gpr.SetData(m_gpr_buffer);
@@ -29,10 +48,6 @@
   m_fpregset = getRegset(
   notes, m_register_info_up->GetTargetArchitecture().GetTriple(), FPR_Desc);
 
-  m_sveregset =
-  getRegset(notes, m_register_info_up->GetTargetArchitecture().GetTriple(),
-AARCH64_SVE_Desc);
-
   ConfigureRegisterContext();
 }
 
@@ -70,15 +85,16 @@
  sve::ptrace_regs_sve)
   m_sve_state = SVEState::Full;
 
-i

[Lldb-commits] [PATCH] D96460: [LLDB] Arm64/Linux Add MTE and Pointer Authentication registers

[Muhammad Omair Javaid 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 rG1164b4e29572: [LLDB] Arm64/Linux Add MTE and Pointer 
Authentication registers (authored by omjavaid).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D96460?vs=332617&id=334303#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96460

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h

Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -470,6 +470,13 @@
 LLDB_INVALID_REGNUM, lldb_kind \
   }
 
+// Generates register kinds array for registers with only lldb kind
+#define KIND_ALL_INVALID   \
+  {\
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, \
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM   \
+  }
+
 // Generates register kinds array for vector registers
 #define GPR64_KIND(reg, generic_kind) MISC_KIND(reg, gpr, generic_kind)
 #define VREG_KIND(reg) MISC_KIND(reg, fpu, LLDB_INVALID_REGNUM)
@@ -526,6 +533,13 @@
 nullptr, 0 \
   }
 
+// Defines pointer authentication mask registers
+#define DEFINE_EXTENSION_REG(reg)  \
+  {\
+#reg, nullptr, 8, 0, lldb::eEncodingUint, lldb::eFormatHex,\
+KIND_ALL_INVALID, nullptr, nullptr, nullptr, 0 \
+  }
+
 static lldb_private::RegisterInfo g_register_infos_arm64_le[] = {
 // DEFINE_GPR64(name, GENERIC KIND)
 DEFINE_GPR64(x0, LLDB_REGNUM_GENERIC_ARG1),
@@ -772,7 +786,12 @@
 {DEFINE_DBG(wcr, 13)},
 {DEFINE_DBG(wcr, 14)},
 {DEFINE_DBG(wcr, 15)}
-// clang-format on
 };
+// clang-format on
+static lldb_private::RegisterInfo g_register_infos_pauth[] = {
+DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)};
+
+static lldb_private::RegisterInfo g_register_infos_mte[] = {
+DEFINE_EXTENSION_REG(mte_ctrl)};
 
 #endif // DECLARE_REGISTER_INFOS_ARM64_STRUCT
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -26,6 +26,8 @@
   enum {
 eRegsetMaskDefault = 0,
 eRegsetMaskSVE = 1,
+eRegsetMaskPAuth = 2,
+eRegsetMaskMTE = 4,
 eRegsetMaskDynamic = ~1,
   };
 
@@ -94,6 +96,10 @@
 
   size_t GetRegisterSetFromRegisterIndex(uint32_t reg_index) const override;
 
+  void AddRegSetPAuth();
+
+  void AddRegSetMTE();
+
   uint32_t ConfigureVectorLength(uint32_t sve_vq);
 
   bool VectorSizeIsValid(uint32_t vq) {
@@ -108,12 +114,16 @@
   bool IsSVEZReg(unsigned reg) const;
   bool IsSVEPReg(unsigned reg) const;
   bool IsSVERegVG(unsigned reg) const;
+  bool IsPAuthReg(unsigned reg) const;
+  bool IsMTEReg(unsigned reg) const;
 
   uint32_t GetRegNumSVEZ0() const;
   uint32_t GetRegNumSVEFFR() const;
   uint32_t GetRegNumFPCR() const;
   uint32_t GetRegNumFPSR() const;
   uint32_t GetRegNumSVEVG() const;
+  uint32_t GetPAuthOffset() const;
+  uint32_t GetMTEOffset() const;
 
 private:
   typedef std::map>
@@ -137,6 +147,9 @@
 
   std::vector m_dynamic_reg_infos;
   std::vector m_dynamic_reg_sets;
+
+  std::vector pauth_regnum_collection;
+  std::vector m_mte_regnum_collection;
 };
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -77,6 +77,8 @@
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
   k_num_fpr_registers = fpu_fpcr - fpu_v0 + 1,
   k_num_sve_registers = sve_ffr - sve_vg + 1,
+  k_num_mte_register = 1,
+  k_num_pauth_register = 2,
   k_num_register_sets_default = 2,
   k_num_register_sets = 3
 };
@@ -175,6 +177,12 @@
 {"Scalable Vector E

[Lldb-commits] [PATCH] D91679: [trace][intel-pt] Implement trace start and trace stop

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looking really good.

See inline comments about the need for a ProcessorTraceStopReason.

Since we have the eStopReasonProcessorTrace stop reason now, we should probably 
deliver these stops to the Trace plug-in so that it can respond maybe? Fine if 
we can to do this in another patch.




Comment at: lldb/include/lldb/Target/StopInfo.h:133
+  static lldb::StopInfoSP
+  CreateStopReasonProcessorTrace(Thread &thread, const char *description);
+

Do we need an extra enumeration for eStopReasonProcessorTrace that we would add 
as a parameter? I could think of:
```
enum ProcessorTraceStopReason {
///< Max memory for process has been exceeded  (current patch and the only 
reason we stop with eStopReasonProcessorTrace , right?)
eProcessorTraceProcessBufferFull = 1
///< For using a buffer that fills and then stops the process (future patch)
eProcessorTraceThreadBufferFull, 
///< For when tracing specific threads and all threads that were traced have 
exited (future patch)
eProcessorTraceTraceEnded, 
eProcessorTrace
};
```
Then this data could be return from SBThread APIs via:
``` 
size_t SBThread::GetStopReasonDataCount();
uint64_t SBThread::GetStopReasonDataAtIndex(uint32_t idx);
```
Where SBThread::GetStopReasonDataCount() would return 1, and 
SBThread::GetStopReasonDataAtIndex(0) would return this enumeration. We would 
also use this enumeration in the Trace plug-ins when responding to stops that 
were eStopReasonProcessorTrace



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91679

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

[Lldb-commits] [PATCH] D91679: [trace][intel-pt] Implement trace start and trace stop

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

I like that idea, but i'd rather do it in a different patch once we have a 
second stop event, so that I make sure the entire thing makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91679

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

[Lldb-commits] [PATCH] D91679: [trace][intel-pt] Implement trace start and trace stop

[Greg Clayton via Phabricator via lldb-commits]

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

Sounds good. Lets gets this in then and start iterating on future patches!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91679

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

[Lldb-commits] [lldb] a4ee79c - Fix errors in 0b69756110db444282c40ea16929186b2910c3b1

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-03-30T18:03:02-07:00
New Revision: a4ee79c8ae5ca1bbfa8d78a2782918d1f23f15b2

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

LOG: Fix errors in 0b69756110db444282c40ea16929186b2910c3b1

Errors found in
https://lab.llvm.org/buildbot/#/builders/68/builds/9681/steps/6/logs/stdio

Added: 


Modified: 
lldb/source/Target/Target.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Removed: 




diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 7c62904302c9..3aa1d30cb776 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3092,7 +3092,7 @@ llvm::Expected Target::GetTraceOrCreate() {
   return llvm::createStringError(
   llvm::inconvertibleErrorCode(),
   "Couldn't start tracing the process. %s",
-  llvm::toString(trace_type.takeError()).c_str());
+  llvm::toString(trace_sp.takeError()).c_str());
   }
   return m_trace_sp;
 }

diff  --git 
a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp 
b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 0a9a42a81fde..4eeec62aee3c 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -417,7 +417,7 @@ TEST_F(GDBRemoteCommunicationClientTest, 
SendTraceSupportedPacket) {
 HandlePacket(server, "jLLDBTraceSupported", R"({"type":"intel-pt"}])");
 
 EXPECT_FALSE(result.get());
-ASSERT_STREQ(error_message.c_str(), "missing value at (root).description");
+ASSERT_STREQ(error_message.c_str(), "missing value at 
TraceSupportedResponse.description");
   }
 
   // Error response



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