[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-17 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid updated this revision to Diff 305680.
omjavaid added a comment.

This update tries to fix concerns raised by removing AArch64 specific code and 
makes offset assignment more generic.

We now have a new flag which is set to true if offset field was present and on 
basis of that we assign offsets to Primary and Pseudo registers.

If offset is not specified for a primary register then its offset is equal to 
last_register_offset + last_register_size. This is what LLDB was doing already 
we just restrict this scheme to primary registers only.

If offset is not specified for a pseudo register then fall-back offset is same 
as the offset of its first value_reg in value_regs list.

For assigning offsets to pseudo registers all primary registers should have 
valid offsets. Primary registers  are assigned offsets in ParseRegisters and 
ProcessGDBRemote::BuildDynamicRegisterInfo while pseudo register offsets are 
update once their value_regs are known in DynamicRegisterInfo::Finalize


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

https://reviews.llvm.org/D91241

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  
lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
  lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
  lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -219,6 +219,7 @@
 }
 
 Error TestClient::qRegisterInfos() {
+  uint32_t reg_offset = 0;
   for (unsigned int Reg = 0;; ++Reg) {
 std::string Message = formatv("qRegisterInfo{0:x-}", Reg).str();
 Expected InfoOr = SendMessage(Message);
@@ -227,6 +228,12 @@
   break;
 }
 m_register_infos.emplace_back(std::move(*InfoOr));
+
+if (m_register_infos[Reg].byte_offset == LLDB_INVALID_INDEX32)
+  m_register_infos[Reg].byte_offset = reg_offset;
+
+reg_offset =
+m_register_infos[Reg].byte_offset + m_register_infos[Reg].byte_size;
 if (m_register_infos[Reg].kinds[eRegisterKindGeneric] ==
 LLDB_REGNUM_GENERIC_PC)
   m_pc_register = Reg;
Index: lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
===
--- lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -171,7 +171,7 @@
   Info.byte_size /= CHAR_BIT;
 
   if (!to_integer(Elements["offset"], Info.byte_offset, 10))
-return make_parsing_error("qRegisterInfo: offset");
+Info.byte_offset = LLDB_INVALID_INDEX32;
 
   Info.encoding = Args::StringToEncoding(Elements["encoding"]);
   if (Info.encoding == eEncodingInvalid)
Index: lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
===
--- lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
+++ lldb/test/API/tools/lldb-server/registers-target-xml-reading/TestGdbRemoteTargetXmlPacket.py
@@ -65,5 +65,8 @@
 self.assertEqual(q_info_reg["set"], xml_info_reg.get("group"))
 self.assertEqual(q_info_reg["format"], xml_info_reg.get("format"))
 self.assertEqual(q_info_reg["bitsize"], xml_info_reg.get("bitsize"))
-self.assertEqual(q_info_reg["offset"], xml_info_reg.get("offset"))
+
+if not self.getArchitecture() == 'aarch64':
+self.assertEqual(q_info_reg["offset"], xml_info_reg.get("offset"))
+
 self.assertEqual(q_info_reg["encoding"], xml_info_reg.get("encoding"))
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
@@ -472,6 +472,7 @@
 std::vector value_regs;
 std::vector invalidate_regs;
 std::vector dwarf_opcode_bytes;
+bool offset_field_set = false;
 RegisterInfo reg_info = {
 nullptr,   // Name
 nullptr,   // Alt name
@@ -501,6 +502,7 @@
 value.getAsInteger(0, reg_info.byte_size);
 reg_info.byte_size /= CHAR_BIT;
   } else if (name.equals("offset")) {
+offset_field_set = true;
 if (value.getAsInteger(0, reg_offset))
   reg_offs

[Lldb-commits] [PATCH] D91241: [LLDB] Make offset field optional in RegisterInfo packet for Arm64

2020-11-17 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

In D91241#2397083 , @labath wrote:

> I disagree, and my main reason for that is that this approach requires 
> hardcoding the "supported architectures" for which we do this fixup in the 
> DynamicRegisterInfo class. Besides being wrong as a matter of priniciple, I 
> suspect this will also be a problem in practice -- for example, I have no 
> idea what will happen when this code starts interacting with the aarch64 
> debugserver (which will still be sending the offsets). Doing this decision 
> based on whether the server sends the offsets avoids this issue, and the 
> process can be seen as an extension of what we're already doing when 
> communicating with offset-less stubs.
>
> In D91241#2396445 , @omjavaid wrote:
>
>> In LLDB we are trying to support legacy offset field
>
> I wouldn't go so far as to call this a "legacy" field. Let's just say that 
> for this particular problem on this architecture, we chose to stop using the 
> offset field.
>
>> which means offset calculation has to be done in location where we parse an 
>> XML register entry (ParseRegisters) or qRegisterInfo packet 
>> (ProcessGDBRemote::BuildDynamicRegisterInfo).
>
> Why is that? I don't see the connection here.
>
> I don't see why the offsets could not be computed in the DynamicRegisterInfo 
> class. The `ParseRegisters`/`BuildDynamicRegisterInfo` functions could just 
> refrain from filling in the offset fields (leave them as invalid, or 
> something) if the server did not provide them. Then DynamicRegisterInfo could 
> use that invalid value to know which offsets need filling in.
>
> In fact, if we wanted to be truly faithful to the gdb algorithm you described 
> ("gdb creates a sorted list based of register numbers and then assigns 
> offsets accordingly"), then it seems like we would _have to_ do this as a 
> separate step, once all register numbers are known.

I have made some updates which are sufficient for current AArch64 scenario 
where offset field is not present. For ordering registers based on register 
numbers I think we may skip doing that for now though i intend to come back to 
it after getting SVE through.


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

https://reviews.llvm.org/D91241

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

[Lldb-commits] [PATCH] D91422: [lldb] [Process/FreeBSDRemote] Check for regset support early [WIP]

2020-11-17 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

In D91422#2396697 , @labath wrote:

> My guess is that this is because of the extra debug registers that you're 
> exposing. You'll notice that Get(User)RegisterSetCount returns just a number, 
> and this means that it is not really possible to selectively disable register 
> sets -- all you can do is disable the ones that come last.
>
> We ran into a similar problem on arm with @omjavaid.
>
> For the time being, we may be able to work around this by making the debug 
> registers come earlier in the list (after ensuring they work on linux). Or we 
> could hide them on BSDs as well...

Register sets are stored in statically which means we have to honor the order 
as @labath  pointed out. Ideally register sets array should be a dynamic list 
with all currently available registerset inserted in the list.


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

https://reviews.llvm.org/D91422

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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-17 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela added inline comments.



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:60-78
+static int runCallback(lua_State *L) {
+  LuaCallback *callback = static_cast(lua_touserdata(L, -1));
+  return (*callback)(L);
+}
+
+llvm::Error Lua::Run(LuaCallback &callback) {
+  lua_pushcfunction(m_lua_state, runCallback);

labath wrote:
> I'm confused. Why use lua to call a c callback, when you could just do 
> `calllback()`?
Some Lua API functions throw errors, if there's any it will `abort()` the 
program if no panic handler or protected call is provided.
To guarantee that the callback always runs in a protected environment and it 
has error handling, we do the above.
Delegating this to the callback itself makes it cumbersome to write.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

2020-11-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
DavidSpickett requested review of this revision.
Herald added a subscriber: JDevlieghere.

init_llgs_test no longer takes an argument
but these two were not updated.

Also fix some mistakes in TestAutoInstallMainExecutable
to get it passing again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91612

Files:
  
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
  
lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py


Index: 
lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
===
--- 
lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
+++ 
lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
@@ -16,7 +16,7 @@
 @expectedFailureAll(hostoslist=["windows"], triple='.*-android')
 def test_platform_process_connect(self):
 self.build()
-self.init_llgs_test(False)
+self.init_llgs_test()
 
 working_dir = lldb.remote_platform.GetWorkingDirectory()
 src = lldb.SBFileSpec(self.getBuildArtifact("a.out"))
Index: 
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
===
--- 
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++ 
lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
@@ -19,7 +19,7 @@
 @expectedFailureAll(hostoslist=["windows"], triple='.*-android')
 def test_target_auto_install_main_executable(self):
 self.build()
-self.init_llgs_test(False)
+self.init_llgs_test()
 
 # Manually install the modified binary.
 working_dir = lldb.remote_platform.GetWorkingDirectory()
@@ -77,10 +77,10 @@
 
(os.path.join(working_dir,dest.GetFilename()),
 self.getBuildArtifact("a.out")))
 
-target = new_debugger.GetSelectedTarget()
+target = self.dbg.GetSelectedTarget()
 breakpoint = target.BreakpointCreateByName("main")
 
-launch_info = taget.GetLaunchInfo()
+launch_info = target.GetLaunchInfo()
 error = lldb.SBError()
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)


Index: lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
===
--- lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
+++ lldb/test/API/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
@@ -16,7 +16,7 @@
 @expectedFailureAll(hostoslist=["windows"], triple='.*-android')
 def test_platform_process_connect(self):
 self.build()
-self.init_llgs_test(False)
+self.init_llgs_test()
 
 working_dir = lldb.remote_platform.GetWorkingDirectory()
 src = lldb.SBFileSpec(self.getBuildArtifact("a.out"))
Index: lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
===
--- lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
+++ lldb/test/API/commands/target/auto-install-main-executable/TestAutoInstallMainExecutable.py
@@ -19,7 +19,7 @@
 @expectedFailureAll(hostoslist=["windows"], triple='.*-android')
 def test_target_auto_install_main_executable(self):
 self.build()
-self.init_llgs_test(False)
+self.init_llgs_test()
 
 # Manually install the modified binary.
 working_dir = lldb.remote_platform.GetWorkingDirectory()
@@ -77,10 +77,10 @@
 (os.path.join(working_dir,dest.GetFilename()),
 self.getBuildArtifact("a.out")))
 
-target = new_debugger.GetSelectedTarget()
+target = self.dbg.GetSelectedTarget()
 breakpoint = target.BreakpointCreateByName("main")
 
-launch_info = taget.GetLaunchInfo()
+launch_info = target.GetLaunchInfo()
 error = lldb.SBError()
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

2020-11-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a reviewer: JDevlieghere.
DavidSpickett added a comment.

I presume these tests aren't commonly run or have been masked by the expected 
failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91612

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

[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

2020-11-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a reviewer: labath.
DavidSpickett added a comment.

For reference the argument was removed in: https://reviews.llvm.org/D90313


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91612

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

[Lldb-commits] [PATCH] D91057: [LLDB] Update SVE Z reg info to remove invalidate regs

2020-11-17 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfcca6fe93f04: [LLDB] Update SVE Z reg info to remove 
invalidate regs (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91057

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h


Index: lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
@@ -266,71 +266,6 @@
 static uint32_t g_sve_v31_invalidates[] = {sve_z31, fpu_d31, fpu_s31,
LLDB_INVALID_REGNUM};
 
-static uint32_t g_sve_z0_invalidates[] = {fpu_v0, fpu_d0, fpu_s0,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z1_invalidates[] = {fpu_v1, fpu_d1, fpu_s1,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z2_invalidates[] = {fpu_v2, fpu_d2, fpu_s2,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z3_invalidates[] = {fpu_v3, fpu_d3, fpu_s3,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z4_invalidates[] = {fpu_v4, fpu_d4, fpu_s4,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z5_invalidates[] = {fpu_v5, fpu_d5, fpu_s5,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z6_invalidates[] = {fpu_v6, fpu_d6, fpu_s6,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z7_invalidates[] = {fpu_v7, fpu_d7, fpu_s7,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z8_invalidates[] = {fpu_v8, fpu_d8, fpu_s8,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z9_invalidates[] = {fpu_v9, fpu_d9, fpu_s9,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z10_invalidates[] = {fpu_v10, fpu_d10, fpu_s10,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z11_invalidates[] = {fpu_v11, fpu_d11, fpu_s11,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z12_invalidates[] = {fpu_v12, fpu_d12, fpu_s12,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z13_invalidates[] = {fpu_v13, fpu_d13, fpu_s13,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z14_invalidates[] = {fpu_v14, fpu_d14, fpu_s14,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z15_invalidates[] = {fpu_v15, fpu_d15, fpu_s15,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z16_invalidates[] = {fpu_v16, fpu_d16, fpu_s16,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z17_invalidates[] = {fpu_v17, fpu_d17, fpu_s17,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z18_invalidates[] = {fpu_v18, fpu_d18, fpu_s18,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z19_invalidates[] = {fpu_v19, fpu_d19, fpu_s19,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z20_invalidates[] = {fpu_v20, fpu_d20, fpu_s20,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z21_invalidates[] = {fpu_v21, fpu_d21, fpu_s21,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z22_invalidates[] = {fpu_v22, fpu_d22, fpu_s22,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z23_invalidates[] = {fpu_v23, fpu_d23, fpu_s23,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z24_invalidates[] = {fpu_v24, fpu_d24, fpu_s24,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z25_invalidates[] = {fpu_v25, fpu_d25, fpu_s25,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z26_invalidates[] = {fpu_v26, fpu_d26, fpu_s26,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z27_invalidates[] = {fpu_v27, fpu_d27, fpu_s27,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z28_invalidates[] = {fpu_v28, fpu_d28, fpu_s28,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z29_invalidates[] = {fpu_v29, fpu_d29, fpu_s29,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z30_invalidates[] = {fpu_v30, fpu_d30, fpu_s30,
-

[Lldb-commits] [lldb] fcca6fe - [LLDB] Update SVE Z reg info to remove invalidate regs

2020-11-17 Thread Muhammad Omair Javaid via lldb-commits 

Author: Muhammad Omair Javaid
Date: 2020-11-17T17:14:44+05:00
New Revision: fcca6fe93f04f50c302f9ce2d1bad8f5f3fa369a

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

LOG: [LLDB] Update SVE Z reg info to remove invalidate regs

In our recent discussion we are aiming to make LLDB registers exchange minimum
possible information in qRegisterInfo or XMl register descriptions.
For SVE registers, Z registers are catagorized as primary registers and should
not have any infomration about any pseudo registers. All pseudo registers
should have the information on which primary register they belong to.
This patch removes invalidate_regs list from Z registers and will mitigate its
impact on SVE resize patch in a follow up update.

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
index 752f738fd470..ea43ef8fe457 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
@@ -266,71 +266,6 @@ static uint32_t g_sve_v30_invalidates[] = {sve_z30, 
fpu_d30, fpu_s30,
 static uint32_t g_sve_v31_invalidates[] = {sve_z31, fpu_d31, fpu_s31,
LLDB_INVALID_REGNUM};
 
-static uint32_t g_sve_z0_invalidates[] = {fpu_v0, fpu_d0, fpu_s0,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z1_invalidates[] = {fpu_v1, fpu_d1, fpu_s1,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z2_invalidates[] = {fpu_v2, fpu_d2, fpu_s2,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z3_invalidates[] = {fpu_v3, fpu_d3, fpu_s3,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z4_invalidates[] = {fpu_v4, fpu_d4, fpu_s4,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z5_invalidates[] = {fpu_v5, fpu_d5, fpu_s5,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z6_invalidates[] = {fpu_v6, fpu_d6, fpu_s6,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z7_invalidates[] = {fpu_v7, fpu_d7, fpu_s7,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z8_invalidates[] = {fpu_v8, fpu_d8, fpu_s8,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z9_invalidates[] = {fpu_v9, fpu_d9, fpu_s9,
-  LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z10_invalidates[] = {fpu_v10, fpu_d10, fpu_s10,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z11_invalidates[] = {fpu_v11, fpu_d11, fpu_s11,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z12_invalidates[] = {fpu_v12, fpu_d12, fpu_s12,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z13_invalidates[] = {fpu_v13, fpu_d13, fpu_s13,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z14_invalidates[] = {fpu_v14, fpu_d14, fpu_s14,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z15_invalidates[] = {fpu_v15, fpu_d15, fpu_s15,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z16_invalidates[] = {fpu_v16, fpu_d16, fpu_s16,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z17_invalidates[] = {fpu_v17, fpu_d17, fpu_s17,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z18_invalidates[] = {fpu_v18, fpu_d18, fpu_s18,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z19_invalidates[] = {fpu_v19, fpu_d19, fpu_s19,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z20_invalidates[] = {fpu_v20, fpu_d20, fpu_s20,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z21_invalidates[] = {fpu_v21, fpu_d21, fpu_s21,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z22_invalidates[] = {fpu_v22, fpu_d22, fpu_s22,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z23_invalidates[] = {fpu_v23, fpu_d23, fpu_s23,
-   LLDB_INVALID_REGNUM};
-static uint32_t g_sve_z24_invalidates[] = {fpu_v24, fpu_d24, fpu_s24,
-

[Lldb-commits] [lldb] 661e404 - [LLDB] Fix SVE reginfo for sequential offset in g packet

2020-11-17 Thread Muhammad Omair Javaid via lldb-commits 

Author: Muhammad Omair Javaid
Date: 2020-11-17T17:18:34+05:00
New Revision: 661e4040ac60b3df06b2495ea916302b35d9bac2

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

LOG: [LLDB] Fix SVE reginfo for sequential offset in g packet

This moves in the direction of our effort to synchronize register descriptions
between LLDB and GDB xml description. We want to able to send registers in a
way that their offset fields can be re-constructed based on register sizes
in the increasing order of register number.

In context to Arm64 SVE, FPCR and FPSR are same registers in FPU regset and
SVE regset. Previously FPSR/FPCR offset was set at the end of SVE data
because Linux ptrace data placed FPCR and FPSR at the end of SVE register set.

Considering interoperability with other stubs like QEMU and that g packets
should generate register data in increasing order of register numbers. We
have to move FPCR/FPSR offset up to its original location according to
register numbering scheme of ARM64 registers with SVE registers included.

Reviewed By: labath

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

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 810bc8479742f..1fa87e13a0aac 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -,7 +,7 @@ uint32_t 
NativeRegisterContextLinux_arm64::CalculateSVEOffset(
 sve_reg_offset =
 SVE_PT_FPSIMD_OFFSET + (reg - GetRegisterInfo().GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 SVE_SIG_REGS_OFFSET + reg_info->byte_offset - sve_z0_offset;
   }

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
index 2e8335f23b65b..701c88c312586 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -288,8 +288,10 @@ 
RegisterInfoPOSIX_arm64::ConfigureVectorRegisterInfos(uint32_t sve_vq) {
 
 uint32_t offset = SVE_REGS_DEFAULT_OFFSET_LINUX;
 
-reg_info_ref[sve_vg].byte_offset = offset;
-offset += reg_info_ref[sve_vg].byte_size;
+reg_info_ref[fpu_fpsr].byte_offset = offset;
+reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+reg_info_ref[sve_vg].byte_offset = offset + 8;
+offset += 16;
 
 // Update Z registers size and offset
 uint32_t s_reg_base = fpu_s0;
@@ -314,8 +316,7 @@ 
RegisterInfoPOSIX_arm64::ConfigureVectorRegisterInfos(uint32_t sve_vq) {
   offset += reg_info_ref[it].byte_size;
 }
 
-reg_info_ref[fpu_fpsr].byte_offset = offset;
-reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+m_per_vq_reg_infos[sve_vq] = reg_info_ref;
   }
 
   m_register_info_p = reg_info_ref.data();

diff  --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
index c19fe88aa6639..129a887a550cf 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -89,7 +89,7 @@ uint32_t RegisterContextCorePOSIX_arm64::CalculateSVEOffset(
 const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 sve_reg_offset = sve::ptrace_fpsimd_offset + (reg - GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 sve::SigRegsOffset() + reg_info->byte_offset - sve_z0_offset;
   }



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

[Lldb-commits] [PATCH] D90741: [LLDB] Fix SVE reginfo for sequential offset in g packet

2020-11-17 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG661e4040ac60: [LLDB] Fix SVE reginfo for sequential offset 
in g packet (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90741

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


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
@@ -89,7 +89,7 @@
 const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 sve_reg_offset = sve::ptrace_fpsimd_offset + (reg - GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 sve::SigRegsOffset() + reg_info->byte_offset - sve_z0_offset;
   }
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
@@ -288,8 +288,10 @@
 
 uint32_t offset = SVE_REGS_DEFAULT_OFFSET_LINUX;
 
-reg_info_ref[sve_vg].byte_offset = offset;
-offset += reg_info_ref[sve_vg].byte_size;
+reg_info_ref[fpu_fpsr].byte_offset = offset;
+reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+reg_info_ref[sve_vg].byte_offset = offset + 8;
+offset += 16;
 
 // Update Z registers size and offset
 uint32_t s_reg_base = fpu_s0;
@@ -314,8 +316,7 @@
   offset += reg_info_ref[it].byte_size;
 }
 
-reg_info_ref[fpu_fpsr].byte_offset = offset;
-reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+m_per_vq_reg_infos[sve_vq] = reg_info_ref;
   }
 
   m_register_info_p = reg_info_ref.data();
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -,7 +,7 @@
 sve_reg_offset =
 SVE_PT_FPSIMD_OFFSET + (reg - GetRegisterInfo().GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 SVE_SIG_REGS_OFFSET + reg_info->byte_offset - sve_z0_offset;
   }


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
@@ -89,7 +89,7 @@
 const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 sve_reg_offset = sve::ptrace_fpsimd_offset + (reg - GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 sve::SigRegsOffset() + reg_info->byte_offset - sve_z0_offset;
   }
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
@@ -288,8 +288,10 @@
 
 uint32_t offset = SVE_REGS_DEFAULT_OFFSET_LINUX;
 
-reg_info_ref[sve_vg].byte_offset = offset;
-offset += reg_info_ref[sve_vg].byte_size;
+reg_info_ref[fpu_fpsr].byte_offset = offset;
+reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+reg_info_ref[sve_vg].byte_offset = offset + 8;
+offset += 16;
 
 // Update Z registers size and offset
 uint32_t s_reg_base = fpu_s0;
@@ -314,8 +316,7 @@
   offset += reg_info_ref[it].byte_size;
 }
 
-reg_info_ref[fpu_fpsr].byte_offset = offset;
-reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+m_per_vq_reg_infos[sve_vq] = reg_info_ref;
   }
 
   m_register_info_p = reg_info_ref.data();
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -,7 +,7 @@
 sve_reg_offset =
 SVE_PT_FPSIMD_OFFSET + (reg

[Lldb-commits] [PATCH] D89315: [debugserver] Add option to propagate SIGSEGV to target process

2020-11-17 Thread Alessandro Arzilli via Phabricator via lldb-commits 
aarzilli added a comment.

I don't want to be overbearing but should I be doing something else? Is this 
going to be merged and, if it is, what timeline should I expect?


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

https://reviews.llvm.org/D89315

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

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

2020-11-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
DavidSpickett requested review of this revision.
Herald added a subscriber: JDevlieghere.

Previously if you did:
$ lldb-server platform --server <...> --min-gdbserver-port 12346
--max-gdbserver-port 12347
(meaning only use port 12346 for gdbservers)

Then tried to launch two gdbservers on the same connection,
the second one would return port 65535. Which is a real port
number but it actually means lldb-server didn't find one it was
allowed to use.

send packet: $qLaunchGDBServer;<...>
read packet: $pid:1919;port:12346;#c0
<...>
send packet: $qLaunchGDBServer;<...>
read packet: $pid:1927;port:65535;#c7

This situation should be an error even if port 65535 does happen
to be available on the current machine.

Found while investigating: https://bugs.llvm.org/show_bug.cgi?id=48205


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91634

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


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -97,6 +97,8 @@
 uint16_t &port, std::string &socket_name) {
   if (port == UINT16_MAX)
 port = GetNextAvailablePort();
+  if (port == UINT16_MAX)
+return Status("Could not find an available port to launch a gdbserver.");
 
   // Spawn a new thread to accept the port that gets bound after binding to
   // port 0 (zero).


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -97,6 +97,8 @@
 uint16_t &port, std::string &socket_name) {
   if (port == UINT16_MAX)
 port = GetNextAvailablePort();
+  if (port == UINT16_MAX)
+return Status("Could not find an available port to launch a gdbserver.");
 
   // Spawn a new thread to accept the port that gets bound after binding to
   // port 0 (zero).
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91634: [lldb] Error when there are no ports to launch a gdbserver on

2020-11-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

I have found existing tests that launch an lldb-server with specific arguments, 
using an existing lldb-server platform to do it.

The ideal test would be:

- run lldb-server with min port some known to be available port, max is that +1
- connect the client
- send qLaunchGDBServer twice, expect that the second one has an E response

What I can't work out how to do is decide which ports you'd need to use for the 
`--min/max-gdbserver-port` options. I couldn't find a way to predict what ports 
would be
available before you launch the lldb-server. Or guarantee that any ports that 
were available wouldn't get used up in the time between you finding them and 
launching the new lldb-server.

You have to give lldb-server at least one port number to use so giving it none 
and expecting error doesn't work.
Giving it a port you know is already used doesn't help either, since this code 
would still say it's available but then fail connect to it.

So presented without a test (and the most minimal code change). Ideas welcome 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91634

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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-17 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D91508#2396392 , @tammela wrote:

> @JDevlieghere
>
> When writing this patch I noticed that there is no mechanism in-place to 
> remove the Python/Lua function when the breakpoint is removed or when the 
> callback function is replaced.
> The class that sets the callback provides `ClearCallback()` but it never 
> calls into the `ScriptInterpreter` to clean up.
> Although the real world impact is not that big, it's crucial for a small 
> memory footprint. Any thoughts on this?

I see, it looks like we just clean up the LLDB side but never tell the runtimes 
about it. What exactly are we leaking in that case? Do you think it's worth it 
to wire that up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-17 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

This looks good with Pavel's comments addressed and an integration test added 
for the breakpoint callback. (Watchpoints are not supported everywhere and it's 
hard to determine whether they are from a Shell test).




Comment at: lldb/bindings/lua/lua-wrapper.swig:29
+
+   lua_call(L, 2, 1);
+

Might be nice to add a comment to explain the "magic values" here. 



Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h:27
 
+using LuaCallback = std::function;
+

Let's move this into the `Lua` class (and rename it to `Callback`) so we don't 
taint every CU that includes this file. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D89315: [debugserver] Add option to propagate SIGSEGV to target process

2020-11-17 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D89315#2399805 , @aarzilli wrote:

> I don't want to be overbearing but should I be doing something else? Is this 
> going to be merged and, if it is, what timeline should I expect?

I think nobody realized you didn't have commit access, feel free to ask someone 
to merge something for you in the future.  I'll take care of this patch for you.


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

https://reviews.llvm.org/D89315

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

[Lldb-commits] [lldb] 27012c0 - [debugserver] Add option to propagate SIGSEGV to target process

2020-11-17 Thread Jonas Devlieghere via lldb-commits 

Author: Alessandro Arzilli
Date: 2020-11-17T09:27:52-08:00
New Revision: 27012c0f75c2e4891277d0d14f9f97a9f564d596

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

LOG: [debugserver] Add option to propagate SIGSEGV to target process

Adds a command line option that makes debugserver propagate the SIGSEGV
signal to the target process.

Motivation: I'm one of the maintainers of Delve [1] a debugger for Go.
We use debugserver as our backend on macOS and one of the most often
reported bugs is that, on macOS, we don't propagate SIGSEGV back to the
target process [2]. Sometimes some programs will actually cause a
SIGSEGV, by design, and then handle it. Those programs can not be
debugged at all.

Since catching signals isn't very important for a Go debugger I'd much
rather have a command line option in debugserver that causes it to let
SIGSEGV go directly to the target process.

[1] https://github.com/go-delve/delve/
[2] https://github.com/go-delve/delve/issues/852

Differential revision: https://reviews.llvm.org/D89315

Added: 


Modified: 
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/MacOSX/MachTask.h
lldb/tools/debugserver/source/MacOSX/MachTask.mm
lldb/tools/debugserver/source/RNBContext.h
lldb/tools/debugserver/source/RNBRemote.cpp
lldb/tools/debugserver/source/debugserver.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index afafe0d0474a..2d6516e8c654 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -319,20 +319,21 @@ static bool spawn_waitpid_thread(pid_t pid) {
 }
 
 nub_process_t DNBProcessLaunch(
-const char *path, char const *argv[], const char *envp[],
+RNBContext *ctx, const char *path, char const *argv[], const char *envp[],
 const char *working_directory, // NULL => don't change, non-NULL => set
// working directory for inferior to this
 const char *stdin_path, const char *stdout_path, const char *stderr_path,
-bool no_stdio, nub_launch_flavor_t launch_flavor, int disable_aslr,
-const char *event_data, char *err_str, size_t err_len) {
-  DNBLogThreadedIf(LOG_PROCESS, "%s ( path='%s', argv = %p, envp = %p, "
-"working_dir=%s, stdin=%s, stdout=%s, "
-"stderr=%s, no-stdio=%i, launch_flavor = %u, "
-"disable_aslr = %d, err = %p, err_len = "
-"%llu) called...",
+bool no_stdio, int disable_aslr, const char *event_data, char *err_str,
+size_t err_len) {
+  DNBLogThreadedIf(LOG_PROCESS,
+   "%s ( path='%s', argv = %p, envp = %p, "
+   "working_dir=%s, stdin=%s, stdout=%s, "
+   "stderr=%s, no-stdio=%i, launch_flavor = %u, "
+   "disable_aslr = %d, err = %p, err_len = "
+   "%llu) called...",
__FUNCTION__, path, static_cast(argv),
static_cast(envp), working_directory, stdin_path,
-   stdout_path, stderr_path, no_stdio, launch_flavor,
+   stdout_path, stderr_path, no_stdio, ctx->LaunchFlavor(),
disable_aslr, static_cast(err_str),
static_cast(err_len));
 
@@ -349,10 +350,10 @@ nub_process_t DNBProcessLaunch(
   MachProcessSP processSP(new MachProcess);
   if (processSP.get()) {
 DNBError launch_err;
-pid_t pid = processSP->LaunchForDebug(path, argv, envp, working_directory,
-  stdin_path, stdout_path, stderr_path,
-  no_stdio, launch_flavor, 
disable_aslr,
-  event_data, launch_err);
+pid_t pid = processSP->LaunchForDebug(
+path, argv, envp, working_directory, stdin_path, stdout_path,
+stderr_path, no_stdio, ctx->LaunchFlavor(), disable_aslr, event_data,
+ctx->GetUnmaskSignals(), launch_err);
 if (err_str) {
   *err_str = '\0';
   if (launch_err.Fail()) {
@@ -412,7 +413,8 @@ nub_process_t DNBProcessGetPIDByName(const char *name) {
 }
 
 nub_process_t DNBProcessAttachByName(const char *name, struct timespec 
*timeout,
- char *err_str, size_t err_len) {
+ bool unmask_signals, char *err_str,
+ size_t err_len) {
   if (err_str && err_len > 0)
 err_str[0] = '\0';
   std::vector matching_proc_infos;
@@ -433,12 +435,12 @@ nub_proce

[Lldb-commits] [PATCH] D89315: [debugserver] Add option to propagate SIGSEGV to target process

2020-11-17 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG27012c0f75c2: [debugserver] Add option to propagate SIGSEGV 
to target process (authored by aarzilli, committed by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89315

Files:
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachTask.h
  lldb/tools/debugserver/source/MacOSX/MachTask.mm
  lldb/tools/debugserver/source/RNBContext.h
  lldb/tools/debugserver/source/RNBRemote.cpp
  lldb/tools/debugserver/source/debugserver.cpp

Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -245,8 +245,8 @@
: ctx.GetWorkingDirectory());
   const char *process_event = ctx.GetProcessEvent();
   nub_process_t pid = DNBProcessLaunch(
-  resolved_path, &inferior_argv[0], &inferior_envp[0], cwd, stdin_path,
-  stdout_path, stderr_path, no_stdio, launch_flavor, g_disable_aslr,
+  &ctx, resolved_path, &inferior_argv[0], &inferior_envp[0], cwd,
+  stdin_path, stdout_path, stderr_path, no_stdio, g_disable_aslr,
   process_event, launch_err_str, sizeof(launch_err_str));
 
   g_pid = pid;
@@ -368,7 +368,8 @@
   DNBLogThreadedIf(LOG_RNB_MINIMAL, "%s Attaching to pid %i...", __FUNCTION__,
attach_pid);
   char err_str[1024];
-  pid = DNBProcessAttach(attach_pid, NULL, err_str, sizeof(err_str));
+  pid = DNBProcessAttach(attach_pid, NULL, ctx.GetUnmaskSignals(), err_str,
+ sizeof(err_str));
   g_pid = pid;
 
   if (pid == INVALID_NUB_PROCESS) {
@@ -889,6 +890,10 @@
  'F'}, // When debugserver launches the process, forward debugserver's
// current environment variables to the child process ("./debugserver
// -F localhost:1234 -- /bin/ls"
+{"unmask-signals", no_argument, NULL,
+ 'U'}, // debugserver will ignore EXC_MASK_BAD_ACCESS,
+   // EXC_MASK_BAD_INSTRUCTION and EXC_MASK_ARITHMETIC, which results in
+   // SIGSEGV, SIGILL and SIGFPE being propagated to the target process.
 {NULL, 0, NULL, 0}};
 
 int communication_fd = -1;
@@ -1260,6 +1265,10 @@
   forward_env = true;
   break;
 
+case 'U':
+  ctx.SetUnmaskSignals(true);
+  break;
+
 case '2':
   // File descriptor passed to this process during fork/exec and is already
   // open and ready for communication.
@@ -1514,8 +1523,8 @@
 RNBLogSTDOUT("Waiting to attach to process %s...\n",
  waitfor_pid_name.c_str());
 nub_process_t pid = DNBProcessAttachWait(
-waitfor_pid_name.c_str(), launch_flavor, ignore_existing,
-timeout_ptr, waitfor_interval, err_str, sizeof(err_str));
+&ctx, waitfor_pid_name.c_str(), ignore_existing, timeout_ptr,
+waitfor_interval, err_str, sizeof(err_str));
 g_pid = pid;
 
 if (pid == INVALID_NUB_PROCESS) {
@@ -1550,7 +1559,8 @@
 
 RNBLogSTDOUT("Attaching to process %s...\n", attach_pid_name.c_str());
 nub_process_t pid = DNBProcessAttachByName(
-attach_pid_name.c_str(), timeout_ptr, err_str, sizeof(err_str));
+attach_pid_name.c_str(), timeout_ptr, ctx.GetUnmaskSignals(),
+err_str, sizeof(err_str));
 g_pid = pid;
 if (pid == INVALID_NUB_PROCESS) {
   ctx.LaunchStatus().SetError(-1, DNBError::Generic);
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3927,8 +3927,8 @@
   }
   const bool ignore_existing = true;
   attach_pid = DNBProcessAttachWait(
-  attach_name.c_str(), m_ctx.LaunchFlavor(), ignore_existing, NULL,
-  1000, err_str, sizeof(err_str), RNBRemoteShouldCancelCallback);
+  &m_ctx, attach_name.c_str(), ignore_existing, NULL, 1000, err_str,
+  sizeof(err_str), RNBRemoteShouldCancelCallback);
 
 } else if (strstr(p, "vAttachOrWait;") == p) {
   p += strlen("vAttachOrWait;");
@@ -3939,8 +3939,8 @@
   }
   const bool ignore_existing = false;
   attach_pid = DNBProcessAttachWait(
-  attach_name.c_str(), m_ctx.LaunchFlavor(), ignore_existing, NULL,
-  1000, err_str, sizeof(err_str), RNBRemoteShouldCancelCallback);
+  &m_ctx, attach_name.c_str(), ignore_existing, NULL, 1000, err_str,
+  sizeof(err_str), RNBRemoteShouldCancelCallback);
 } else if (strstr(p, "vAttachName;") == p) {
   p += strlen("vAttachNam

[Lldb-commits] [PATCH] D91612: [lldb] Fix a couple of remote llgs tests

2020-11-17 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.

In D91612#2399408 , @DavidSpickett 
wrote:

> I presume these tests aren't commonly run or have been masked by the expected 
> failures.

Yes, that seems to be the case, though I don't actually see a reason for that.

This is definitely the right fix, though it seems that we have a bigger problem 
in that one of the decorators is misbehaving and causing the tests not to run.

My money is on `@skipIf(remote=False)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91612

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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-17 Thread Pavel Labath via Phabricator via lldb-commits 
labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.

I'll have more questions about this, but probably won't get around to asking 
them today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D91645: [lldb] [test] Un-XFAIL tests on freebsd/i386

2020-11-17 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
Herald added a subscriber: arichardson.
mgorny requested review of this revision.

Restrict i386-specific XFAIL on a few tests to non-FreeBSD systems,
as they pass on FreeBSD.


https://reviews.llvm.org/D91645

Files:
  lldb/test/API/functionalities/exec/TestExec.py
  lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py


Index: lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
===
--- lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
+++ lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
@@ -31,6 +31,7 @@
 ">=",
 "3.9"],
 archs=["i386"],
+oslist=no_match(["freebsd"]),
 bugnumber="llvm.org/pr28549")
 def test_step_over_with_python(self):
 """Test stepping over using avoid-no-debug with dwarf."""
@@ -47,6 +48,7 @@
 ">=",
 "3.9"],
 archs=["i386"],
+oslist=no_match(["freebsd"]),
 bugnumber="llvm.org/pr28549")
 @expectedFailureAll(archs=["arm64"], 
bugnumber="")  # lldb doesn't step past last source 
line in function on arm64
 @expectedFailureAll(archs=["aarch64"], oslist=["linux"],
Index: lldb/test/API/functionalities/exec/TestExec.py
===
--- lldb/test/API/functionalities/exec/TestExec.py
+++ lldb/test/API/functionalities/exec/TestExec.py
@@ -16,7 +16,9 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
+@expectedFailureAll(archs=['i386'],
+oslist=no_match(["freebsd"]),
+bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
 @skipIfAsan # rdar://problem/43756823
@@ -24,7 +26,9 @@
 def test_hitting_exec (self):
 self.do_test(False)
 
-@expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
+@expectedFailureAll(archs=['i386'],
+oslist=no_match(["freebsd"]),
+bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], 
bugnumber="rdar://problem/34559552") # this exec test has problems on ios 
systems
 @expectedFailureNetBSD
 @skipIfAsan # rdar://problem/43756823


Index: lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
===
--- lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
+++ lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
@@ -31,6 +31,7 @@
 ">=",
 "3.9"],
 archs=["i386"],
+oslist=no_match(["freebsd"]),
 bugnumber="llvm.org/pr28549")
 def test_step_over_with_python(self):
 """Test stepping over using avoid-no-debug with dwarf."""
@@ -47,6 +48,7 @@
 ">=",
 "3.9"],
 archs=["i386"],
+oslist=no_match(["freebsd"]),
 bugnumber="llvm.org/pr28549")
 @expectedFailureAll(archs=["arm64"], bugnumber="")  # lldb doesn't step past last source line in function on arm64
 @expectedFailureAll(archs=["aarch64"], oslist=["linux"],
Index: lldb/test/API/functionalities/exec/TestExec.py
===
--- lldb/test/API/functionalities/exec/TestExec.py
+++ lldb/test/API/functionalities/exec/TestExec.py
@@ -16,7 +16,9 @@
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
+@expectedFailureAll(archs=['i386'],
+oslist=no_match(["freebsd"]),
+bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], bugnumber="rdar://problem/34559552") # this exec test has problems on ios systems
 @expectedFailureNetBSD
 @skipIfAsan # rdar://problem/43756823
@@ -24,7 +26,9 @@
 def test_hitting_exec (self):
 self.do_test(False)
 
-@expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
+@expectedFailureAll(archs=['i386'],
+oslist=no_match(["freebsd"]),
+bugnumber="rdar://28656532")
 @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], bugnumber="rdar://problem/34559552") # this exec test has problems on ios systems
 @expectedFailureNetBSD
 @skipIfAsan # rdar://problem/43756823
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-17 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela updated this revision to Diff 305874.
tammela marked 4 inline comments as done and an inline comment as not done.
tammela added a comment.

Adressing comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

Index: lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/ScriptInterpreterTests.cpp
@@ -43,6 +43,13 @@
 };
 } // namespace
 
+struct lua_State;
+extern "C" bool LLDBSwigLuaBreakpointCallbackFunction(
+lua_State *L, void *baton, const lldb::StackFrameSP &sb_frame,
+const lldb::BreakpointLocationSP &sb_bp_loc) {
+  return false;
+}
+
 TEST_F(ScriptInterpreterTest, Plugin) {
   EXPECT_EQ(ScriptInterpreterLua::GetPluginNameStatic(), "script-lua");
   EXPECT_EQ(ScriptInterpreterLua::GetPluginDescriptionStatic(),
Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -26,3 +26,27 @@
   EXPECT_EQ(llvm::toString(std::move(error)),
 "[string \"buffer\"]:1: unexpected symbol near 'nil'\n");
 }
+
+TEST(LuaTest, RunCallbackValid) {
+  Lua lua;
+  Lua::Callback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "foo = 1") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_FALSE(static_cast(error));
+}
+
+TEST(LuaTest, RunCallbackInvalid) {
+  Lua lua;
+  Lua::Callback callback = [](lua_State *L) -> int {
+if (luaL_dostring(L, "nil = foo") != LUA_OK)
+  return lua_error(L);
+return 0;
+  };
+  llvm::Error error = lua.Run(callback);
+  EXPECT_TRUE(static_cast(error));
+  EXPECT_EQ(llvm::toString(std::move(error)),
+"[string \"nil = foo\"]:1: unexpected symbol near 'nil'\n");
+}
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,7 @@
+# REQUIRES: lua
+# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t
+# RUN: %lldb -s %s --script-language lua %t 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o 'print(frame)'
+run
+# CHECK: frame #0
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -10,11 +10,20 @@
 #define liblldb_ScriptInterpreterLua_h_
 
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-enumerations.h"
 
 namespace lldb_private {
 class Lua;
 class ScriptInterpreterLua : public ScriptInterpreter {
 public:
+  class CommandDataLua : public BreakpointOptions::CommandData {
+  public:
+CommandDataLua() : BreakpointOptions::CommandData() {
+  interpreter = lldb::eScriptLanguageLua;
+}
+  };
+
   ScriptInterpreterLua(Debugger &debugger);
 
   ~ScriptInterpreterLua() override;
@@ -41,6 +50,11 @@
 
   static const char *GetPluginDescriptionStatic();
 
+  static bool BreakpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t break_id,
+ lldb::user_id_t break_loc_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -51,6 +65,9 @@
   llvm::Error EnterSession(lldb::user_id_t debugger_id);
   llvm::Error LeaveSession();
 
+  Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
+  const char *command_body_text) override;
+
 private:
   std::unique_ptr m_lua;
   bool m_session_is_active = false;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpr

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-17 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp:200
+  Debugger &debugger = target->GetDebugger();
+  ScriptInterpreterLua *lua_interpreter =
+  static_cast(debugger.GetScriptInterpreter());

labath wrote:
> How is `lua_interpreter` different from `this`?
Are you asking why I didn't go for `m_script_interpreter`?

This function is static (on the class declaration), perhaps it's clearer a 
`static` keyword here as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

2020-11-17 Thread Pedro Tammela via Phabricator via lldb-commits 
tammela added a comment.

In D91508#2400222 , @JDevlieghere 
wrote:

> In D91508#2396392 , @tammela wrote:
>
>> @JDevlieghere
>>
>> When writing this patch I noticed that there is no mechanism in-place to 
>> remove the Python/Lua function when the breakpoint is removed or when the 
>> callback function is replaced.
>> The class that sets the callback provides `ClearCallback()` but it never 
>> calls into the `ScriptInterpreter` to clean up.
>> Although the real world impact is not that big, it's crucial for a small 
>> memory footprint. Any thoughts on this?
>
> I see, it looks like we just clean up the LLDB side but never tell the 
> runtimes about it. What exactly are we leaking in that case? Do you think 
> it's worth it to wire that up?

For both interpreters, the function objects itself. I think it's worth it, 
specially for long running debugging sessions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91508

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

[Lldb-commits] [PATCH] D91645: [lldb] [test] Un-XFAIL tests on freebsd/i386

2020-11-17 Thread Ed Maste via Phabricator via lldb-commits 
emaste added inline comments.



Comment at: lldb/test/API/functionalities/exec/TestExec.py:19
 
-@expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532")
+@expectedFailureAll(archs=['i386'],
+oslist=no_match(["freebsd"]),

Interesting, so FreeBSD is now the only OS where this test passes on i386?

Of course rdar://28656532 is opaque, would be nice if Apple folks can provide 
some insight on this.


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

https://reviews.llvm.org/D91645

___
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

2020-11-17 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
wallace added a reviewer: clayborg.
Herald added subscribers: lldb-commits, dang.
Herald added a project: LLDB.
wallace requested review of this revision.
Herald added a subscriber: JDevlieghere.

This implements the interactive trace start and stop methods.

There's a lot of boilerplate code due to the gdb-remote protocol, but the main 
changes are:

- Implementation of the jLLDBTraceStop packet, which is quite simple.
- Implementation of the jLLDBTraceStart packet, which accepts plug-in specific 
params.
- Implementation of the jLLDBTraceQueryData, used for querying in json format 
various data, in a generic way so that it can be resused by trace plug-ins. I'm 
using it for querying the trace buffers and cpu config needed to decode.
- Created a set of well-defined structures that represent all packets and 
params send across the gdb-remote protocol, so that serialization and 
deserialization with JSON is as strongly-typed as possible.
- Created an IntelPTDecoder for live threads, that use the debugger's stop id 
as checkpoint for its internal cache.
- Added tests

Depends on D90729 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91679

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/include/lldb/Utility/TraceOptions.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectThreadUtil.cpp
  lldb/source/Commands/CommandObjectThreadUtil.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
  lldb/source/Plugins/Process/Linux/ProcessorTrace.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/source/Utility/TraceOptions.cpp
  lldb/test/API/commands/trace/TestTraceStartStop.py

Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- lldb/test/API/commands/trace/TestTraceStartStop.py
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -3,6 +3,8 @@
 from lldbsuite.test import lldbutil
 from lldbsuite.test.decorators import *
 
+ADDRESS_REGEX = '0x[0-9a-fA-F]*'
+
 class TestTraceLoad(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
@@ -49,20 +51,57 @@
 
 self.expect("r")
 
+# This fails because "trace start" hasn't been called yet
+self.expect("thread trace stop", error=True,
+substrs=["error: Process is not being traced"])
+
+
 # the help command should be the intel-pt one now
 self.expect("help thread trace start",
 substrs=["Start tracing one or more threads with intel-pt.",
  "Syntax: thread trace start [  ...] []"])
 
-self.expect("thread trace start",
-patterns=["would trace tid .* with size_in_kb 4 and custom_config 0"])
-
-self.expect("thread trace start --size 20 --custom-config 1",
-patterns=["would trace tid .* with size_in_kb 20 and custom_config 1"])
-
-# This fails because "trace stop" is not yet implemented.
+# We start tracing with a small buffer size
+self.expect("thread trace start --size 1")
+
+# We fail if we try to trace again
+self.expect("thread trace start", error=True, 
+substrs=["error: tracing already active on this thread"])
+
+# We can reconstruct the single instruction executed in the first line
+self.expect("n")
+self.expect("thread trace dump instructions", 
+

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

2020-11-17 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

LGTM. A few inline nits, but nothing major. I only question if 
"RecordedProcess" is the right name for the base class for a core file/trace 
file, but don't have much of a better name. Maybe SerializedProcess? 
SavedProcess? Don't we usually have Pavel in on these reviews?




Comment at: lldb/include/lldb/Target/RecordedProcess.h:18
+/// Base class for all processes that don't represent a live process, such as
+/// coredumps or processes traced in the past.
+class RecordedProcess : public Process {

Might want to comment something like "common lldb_private::Process virtual 
functions overrides that are common between these kinds of processes can have 
default implementations in this class".



Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:209
   GetTarget().SetArchitecture(target_arch);
- 
+
   SetUnixSignals(UnixSignals::Create(GetArchitecture()));

undo space only change



Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:246-247
   if (exe_module_spec.GetFileSpec()) {
-exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec, 
-  true /* notify */);
+exe_module_sp =
+GetTarget().GetOrCreateModule(exe_module_spec, true /* notify */);
 if (exe_module_sp)

undo space only change



Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:766
 /// - NT_FILE - Files mapped into memory
-/// 
+///
 /// Additionally, for each thread in the process the core file will contain at

undo space only change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

___
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

2020-11-17 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So I think I get the gist of what you were trying to go for with this patch. I 
stopped inline comments after I got the idea.

A few things:

We need to be able to enable tracing on all threads for a process and have 
lldb-server just "do the right thing" and enable tracing for all threads as 
they get created. If we have to stop the process each time a thread is created 
just to give LLDB the chance to enable tracing for that thread, this will be 
too slow and cumbersome. The ptrace implementation for unix already has to 
attach to each thread as it comes along by running some code in the 
NativeProcessProtocol subclass, so we should be able to start tracing any and 
all threads as needed.

NativeProcessProtocol should have generic plug-in like functions for starting 
and stopping tracing that each subclass will override. See inlined comments.

See inlined comments about the jLLDB packets and let me know what you think.




Comment at: lldb/docs/lldb-gdb-remote.txt:276
+//  {
+//"type": 
+//"tid": 

Do we need "type" here? Only one kind of trace is currently returned by 
jLLDBTraceSupportedType right now. I am ok with passing it down again in case 
we change to returning a list from jLLDBTraceSupportedType.



Comment at: lldb/docs/lldb-gdb-remote.txt:277
+//"type": 
+//"tid": 
+//"params": {

Do we want this as optional? What if we want to trace all threads? I would be a 
real pain if we always have to detect new threads showing up and then sending a 
jLLDBTraceStart as this will really slow down the process for no good reason. 
So it would be nice if we can specify this as optional and if the "tid" is not 
specified, have the lldb-server start tracing any new threads as they come 
along. 



Comment at: lldb/docs/lldb-gdb-remote.txt:307
+//  {
+//"type": 
+//"tid": 

Do we need "type" here just like in jLLDBTraceStart?



Comment at: lldb/docs/lldb-gdb-remote.txt:308
+//"type": 
+//"tid": 
+//  }

Is this optional? Want if we want to stop tracing all threads?



Comment at: lldb/docs/lldb-gdb-remote.txt:323-332
+// SCHEMA
+//  The schema for the input is
+//
+//  {
+//"type": 
+//"query": 
+//"params": {

Would it be better for this command to just return all the data we need? Maybe 
a single packet like:

jLLDBTraceGetInfo

And it would return JSON that includes all available data like:
```
{
  "vendor": "intel" | "unknown",
  "family": ,
  "model": ,
  "stepping": ,
  "tids": [
{
  "tid": ,
  "dataSize": , // Size of all trace data for this thread in case 
it is huge, so we can download parts of it?
}
  ]
 }
```

Then we have another packet to fetch the data for each thread with a packet 
like "jLLDBTraceGetData" whose schema for input would be "tid" and "offset" and 
"size". This would allow us to fetch partial data in case we need to interrupt 
the fetching since this data can be bigger. The return value for this function 
would be just like the "binary memory read" where the bytes are much more 
efficiently encoded as binary and not as HEX8 format.







Comment at: lldb/include/lldb/Core/PluginManager.h:336-337
+  ConstString name, const char *description,
+  TraceCreateInstanceForSessionFile create_callback_for_session_file,
+  TraceCreateInstanceForLiveProcess create_callback_for_live_process,
+  llvm::StringRef schema, TraceGetStartCommand get_start_command);

Why do we need these callbacks? Can we just keep the "TraceCreateInstance 
create_callback" and then add virtual functions to the API in Trace.h?



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:320
+  /// \a llvm::Error otherwise.
+  virtual llvm::Error StopIntelPTTrace(lldb::tid_t tid) {
+return llvm::make_error();

Shouldn't this be generic so it can work with any NativeProcessProtocol 
implementation? Think about doing this same stuff on ARM. We don't want to have 
functions in NativeProcessProtocol like "StopIntelPTrace" and "StopARMTrace" 
and "StopARM64Trace" that are all stubbed out. It would be better to have 
something generic that passes along structured data to each function where the 
type of the trace is specified in the structured data.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:341
+  /// \a llvm::Error otherwise.
+  virtual llvm::Error StartIntelPTTrace(lldb::tid_t tid,
+size_t buffer_size_in_kb,

Ditto above comment about making this generic.



Comment at: lldb/include/lldb/Target/Process.h:2566
+  /// \a llvm::Error otherwise.
+  virtual llvm::Error StartT