[Lldb-commits] [PATCH] D110033: [lldb] [gdb-remote] Always send PID when detaching w/ multiprocess

2021-09-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
mgorny requested review of this revision.

Always send PID in the detach packet when multiprocess extensions are
enabled.  This is required by qemu's GDB server, as plain 'D' packet
results in an error and the emulated system is not resumed.


https://reviews.llvm.org/D110033

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


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1547,14 +1547,16 @@
 }
   }
 
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-if (!m_supports_multiprocess) {
-  error.SetErrorString(
-  "Multiprocess extension not supported by the server.");
-  return error;
-}
+  if (m_supports_multiprocess) {
+// Some servers (e.g. qemu) require specifying the PID even if only a 
single
+// process is running.
+if (pid == LLDB_INVALID_PROCESS_ID)
+  pid = GetCurrentProcessID();
 packet.PutChar(';');
 packet.PutHex64(pid);
+  } else if (pid != LLDB_INVALID_PROCESS_ID) {
+error.SetErrorString("Multiprocess extension not supported by the 
server.");
+return error;
   }
 
   StringExtractorGDBRemote response;


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1547,14 +1547,16 @@
 }
   }
 
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-if (!m_supports_multiprocess) {
-  error.SetErrorString(
-  "Multiprocess extension not supported by the server.");
-  return error;
-}
+  if (m_supports_multiprocess) {
+// Some servers (e.g. qemu) require specifying the PID even if only a single
+// process is running.
+if (pid == LLDB_INVALID_PROCESS_ID)
+  pid = GetCurrentProcessID();
 packet.PutChar(';');
 packet.PutHex64(pid);
+  } else if (pid != LLDB_INVALID_PROCESS_ID) {
+error.SetErrorString("Multiprocess extension not supported by the server.");
+return error;
   }
 
   StringExtractorGDBRemote response;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

2021-09-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 373451.
mgorny added a comment.

Update for using local (LLDB) regnos.


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

https://reviews.llvm.org/D110023

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
@@ -134,3 +134,104 @@
   EXPECT_EQ(added_reg->invalidate_regs[1], i1);
   EXPECT_EQ(added_reg->invalidate_regs[2], LLDB_INVALID_REGNUM);
 }
+
+TEST(DynamicRegisterInfoTest, add_supplementary_register) {
+  DynamicRegisterInfo info;
+
+  // Add a base register
+  uint32_t rax = AddRegister(info, "rax", 8);
+
+  // Register numbers
+  uint32_t eax = 1;
+  uint32_t ax = 2;
+  uint32_t ah = 3;
+  uint32_t al = 4;
+
+  ConstString group{"supplementary registers"};
+  uint32_t value_regs[2] = {rax, LLDB_INVALID_REGNUM};
+  struct RegisterInfo eax_reg {
+"eax", nullptr, 4, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, eax,
+ eax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(eax_reg, group);
+
+  struct RegisterInfo ax_reg {
+"ax", nullptr, 2, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ax, ax},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ax_reg, group);
+
+  struct RegisterInfo ah_reg {
+"ah", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, ah, ah},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(ah_reg, group);
+
+  struct RegisterInfo al_reg {
+"al", nullptr, 1, LLDB_INVALID_INDEX32, lldb::eEncodingUint,
+lldb::eFormatUnsigned,
+{LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, al, al},
+value_regs, nullptr, nullptr, 0
+  };
+  info.AddSupplementaryRegister(al_reg, group);
+
+  const RegisterInfo *added_reg = info.GetRegisterInfoAtIndex(0);
+  ASSERT_NE(added_reg, nullptr);
+  ASSERT_NE(added_reg->invalidate_regs, nullptr);
+  EXPECT_EQ(added_reg->invalidate_regs[0], eax);
+  EXPECT_EQ(added_reg->invalidate_regs[1], ax);
+  EXPECT_EQ(added_reg->invalidate_regs[2], ah);
+  EXPECT_EQ(added_reg->invalidate_regs[3], al);
+  EXPECT_EQ(added_reg->invalidate_regs[4], LLDB_INVALID_REGNUM);
+
+  added_reg = info.GetRegisterInfoAtIndex(1);
+  ASSERT_NE(added_reg, nullptr);
+  EXPECT_EQ(added_reg->value_regs[0], rax);
+  EXPECT_EQ(added_reg->value_regs[1], LLDB_INVALID_REGNUM);
+  ASSERT_NE(added_reg->invalidate_regs, nullptr);
+  EXPECT_EQ(added_reg->invalidate_regs[0], rax);
+  EXPECT_EQ(added_reg->invalidate_regs[1], ax);
+  EXPECT_EQ(added_reg->invalidate_regs[2], ah);
+  EXPECT_EQ(added_reg->invalidate_regs[3], al);
+  EXPECT_EQ(added_reg->invalidate_regs[4], LLDB_INVALID_REGNUM);
+
+  added_reg = info.GetRegisterInfoAtIndex(2);
+  ASSERT_NE(added_reg, nullptr);
+  EXPECT_EQ(added_reg->value_regs[0], rax);
+  EXPECT_EQ(added_reg->value_regs[1], LLDB_INVALID_REGNUM);
+  ASSERT_NE(added_reg->invalidate_regs, nullptr);
+  EXPECT_EQ(added_reg->invalidate_regs[0], rax);
+  EXPECT_EQ(added_reg->invalidate_regs[1], eax);
+  EXPECT_EQ(added_reg->invalidate_regs[2], ah);
+  EXPECT_EQ(added_reg->invalidate_regs[3], al);
+  EXPECT_EQ(added_reg->invalidate_regs[4], LLDB_INVALID_REGNUM);
+
+  added_reg = info.GetRegisterInfoAtIndex(3);
+  ASSERT_NE(added_reg, nullptr);
+  EXPECT_EQ(added_reg->value_regs[0], rax);
+  EXPECT_EQ(added_reg->value_regs[1], LLDB_INVALID_REGNUM);
+  ASSERT_NE(added_reg->invalidate_regs, nullptr);
+  EXPECT_EQ(added_reg->invalidate_regs[0], rax);
+  EXPECT_EQ(added_reg->invalidate_regs[1], eax);
+  EXPECT_EQ(added_reg->invalidate_regs[2], ax);
+  EXPECT_EQ(added_reg->invalidate_regs[3], al);
+  EXPECT_EQ(added_reg->invalidate_regs[4], LLDB_INVALID_REGNUM);
+
+  added_reg = info.GetRegisterInfoAtIndex(4);
+  ASSERT_NE(added_reg, nullptr);
+  EXPECT_EQ(added_reg->value_regs[0], rax);
+  EXPECT_EQ(added_reg->value_regs[1], LLDB_INVALID_REGNUM);
+  ASSERT_NE(added_reg->invalidate_regs, nullptr);
+  EXPECT_EQ(added_reg->invalidate_regs[0], rax);
+  EXPECT_EQ(added_reg->invalidate_regs[1], eax);
+  EXPECT_EQ(added_reg->invalidate_regs[2], ax);
+  EXPECT_EQ(added_reg->invalidate_regs[3], ah);
+  EXPECT_EQ(added_reg->invalidate_regs[4], LLDB_INVALID_REGNUM);
+}
Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h

[Lldb-commits] [PATCH] D110023: [lldb] [DynamicRegisterInfo] Add a convenience method to add suppl. registers

2021-09-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added inline comments.



Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp:482
+// copy invalidate_regs from the parent register
+llvm::append_range(to_add[reg_num], m_invalidate_regs_map[value_reg]);
+

@labath, any suggestion how to do this nicely while not copying the terminating 
`LLDB_INVALID_REGNUM`?


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

https://reviews.llvm.org/D110023

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

[Lldb-commits] [PATCH] D109876: [lldb] [ABI/AArch64] Add pseudo-regs if missing [WIP]

2021-09-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 373452.
mgorny added a comment.

Rebase. Clang-format. Eliminate remote regnums entirely.


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

https://reviews.llvm.org/D109876

Files:
  lldb/include/lldb/Target/ABI.h
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIAArch64.h
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -380,6 +380,7 @@
 return "".join(reg_data)
 
 def writeRegisters(self, reg_hex):
+reg_data[:] = [reg_hex]
 return "OK"
 
 def haltReason(self):
@@ -429,3 +430,43 @@
["v0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
 self.match("register read v31",
["v31 = {0xa1 0xa2 0xa3 0xa4 0xa5 0xa6 0xa7 0xa8 0xa9 0xaa 0xab 0xac 0xad 0xae 0xaf 0xb0}"])
+
+# test partial registers
+self.match("register read w0",
+   ["w0 = 0x04030201"])
+self.runCmd("register write w0 0xfffefdfc")
+self.match("register read x0",
+   ["x0 = 0x08070605fffefdfc"])
+
+self.match("register read w1",
+   ["w1 = 0x14131211"])
+self.runCmd("register write w1 0xefeeedec")
+self.match("register read x1",
+   ["x1 = 0x18171615efeeedec"])
+
+self.match("register read w30",
+   ["w30 = 0x44434241"])
+self.runCmd("register write w30 0xdfdedddc")
+self.match("register read x30",
+   ["x30 = 0x48474645dfdedddc"])
+
+self.match("register read w31",
+   ["w31 = 0x54535251"])
+self.runCmd("register write w31 0xcfcecdcc")
+self.match("register read x31",
+   ["sp = 0x58575655cfcecdcc"])
+
+# test FPU registers (overlapping with vector registers)
+self.runCmd("register write d0 16")
+self.match("register read v0",
+   ["v0 = {0x00 0x00 0x00 0x00 0x00 0x00 0x30 0x40 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.runCmd("register write v31 '{0x00 0x00 0x00 0x00 0x00 0x00 0x50 0x40 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff}'")
+self.match("register read d31",
+   ["d31 = 64"])
+
+self.runCmd("register write s0 32")
+self.match("register read v0",
+   ["v0 = {0x00 0x00 0x00 0x42 0x00 0x00 0x30 0x40 0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
+self.runCmd("register write v31 '{0x00 0x00 0x00 0x43 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff}'")
+self.match("register read s31",
+   ["s31 = 128"])
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
@@ -453,6 +453,10 @@
   if (GetGDBServerRegisterInfo(arch_to_use))
 return;
 
+  // We have to make a temporary ABI here, and not use the GetABI because
+  // this code gets called in DidAttach, when the target architecture
+  // (and consequently the ABI we'll get from the process) may be wrong.
+  ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
   char packet[128];
   uint32_t reg_offset = LLDB_INVALID_INDEX32;
   uint32_t reg_num = 0;
@@ -579,10 +583,7 @@
 
 reg_info.name = reg_name.AsCString();
 reg_info.alt_name = alt_name.AsCString();
-// We have to make a temporary ABI here, and not use the GetABI because
-// this code gets called in DidAttach, when the target architecture
-// (and consequently the ABI we'll get from the process) may be wrong.
-if (ABISP abi_sp = ABI::FindPlugin(shared_from_this(), arch_to_use))
+if (abi_sp)
   abi_sp->AugmentRegisterInfo(reg_info);
 
 m_register_info_sp->AddRegister(reg_info, set_name);
@@ -595,6 +596,8 @@
   }
 
   if (m_register_info_sp->GetNumRegisters() > 0) {
+if (abi_sp)
+  abi_sp->PreFinalizeDynamicRegisterInfo(*m_register_info_sp);
 m_register_info_sp->Finalize(GetTarget().GetArchitecture());
 return;
   }
@@ -618,6 +621,8 @@
   }
 
   // At this point, we can finalize our register info.
+  if (abi_sp)
+abi_sp->PreFinalizeDynamicRegisterInfo(*m_register_info_sp);
   m_register_info_sp->Finalize(GetTarget().GetArchitecture());
 }
 
@@ -4730,6 +4735,8 @@
   m

[Lldb-commits] [PATCH] D108768: [lldb] DynamicRegisterInfo: fix wrong regnos in Dump()

2021-09-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny abandoned this revision.
mgorny added a comment.

Replaced by D110027 .


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

https://reviews.llvm.org/D108768

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

[Lldb-commits] [PATCH] D109937: [lldb] Handle malformed qfThreadInfo reply

2021-09-19 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D109937#3007071 , @ted wrote:

> I've got a test written. It doesn't crash like the debugger in the wild does, 
> but it does give a tid of 0 for each thread I ask about. So I can assert if 
> the threads don't have the correct tid.

I guess you forgot to upload it (?)

> Which leads me to 1 more question - what should I do when we have a malformed 
> response, and there are no threads listed? I'm leaning towards just falling 
> through to the if at line 2931, and returning a pid and tid of 1. That's what 
> we do if there's and empty response.

Sounds reasonable to me. We'd be basically treating it as if the stub did not 
support the packet at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109937

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

[Lldb-commits] [PATCH] D110010: [lldb] Extract adding symbols for UUID/File/Frame (NFC)

2021-09-19 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

seems straight-forward enough




Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4203
+
+  bool AddSymbolsForFrame(Target *target, CommandReturnObject &result,
+  bool &flush) {

Do we need to pass the target around? I would assume that can be retrieved from 
`m_exe_ctx` as well..


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

https://reviews.llvm.org/D110010

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

[Lldb-commits] [PATCH] D110020: [lldb] [gdb-remote] Remove unused arg from GDBRemoteRegisterContext::ReadRegisterBytes()

2021-09-19 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

yay


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

https://reviews.llvm.org/D110020

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