[Lldb-commits] [PATCH] D92063: [LLDB] RegisterInfoPOSIX_arm64 remove unused bytes from g/G packet

2020-11-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

In D92063#2416162 , @labath wrote:

> +@mgorny, as he's been navigating these waters lately...
>
> So... I presume we can't just slap `__attribute__((packed))` on the 
> structure, because the kernel actually expects that the data structure will 
> have the extra space for the padding. Is that so?
>
> Even if we can't, I'm wondering if it wouldn't be cleaner to use two 
> structures for this. Something like:
>
>   LLVM_PACKED_START
>   struct GPR {
> // as before...
>   };
>   /// Big comment explaining the purpose of padding
>   struct GPRBuffer: GPR {
> uint32_t pad;
>   };
>   LLVM_PACKED_END
>
> and then using GPR or GPRBuffer accordingly. What do you think?

That would imply adding additional offset field to the register lists, wouldn't 
it? Not that I'm opposed — it might be reasonable to have the option to 
override the offset for system structs, coredumps...


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

https://reviews.llvm.org/D92063

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


[Lldb-commits] [lldb] 53a14a4 - [lldb] Fix TestThreadStepOut.py after "Flush local value map on every instruction"

2020-11-26 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-26T09:43:47+01:00
New Revision: 53a14a47ee89dadb8798ca8ed19848f33f4551d5

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

LOG: [lldb] Fix TestThreadStepOut.py after "Flush local value map on every 
instruction"

After cf1c774d6ace59c5adc9ab71b31e762c1be695b1, Clang seems to generate code
that is more similar to icc/Clang, so we can use the same line numbers for
all compilers in this test.

Added: 


Modified: 
lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
lldb/test/API/functionalities/thread/step_out/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py 
b/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
index eb2d264ec2e3..ae46530f4ab5 100644
--- a/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
+++ b/lldb/test/API/functionalities/thread/step_out/TestThreadStepOut.py
@@ -70,12 +70,8 @@ def setUp(self):
 self.bkpt_string = '// Set breakpoint here'
 self.breakpoint = line_number('main.cpp', self.bkpt_string)   
 
-if "gcc" in self.getCompiler() or self.isIntelCompiler():
-self.step_out_destination = line_number(
-'main.cpp', '// Expect to stop here after step-out (icc and 
gcc)')
-else:
-self.step_out_destination = line_number(
-'main.cpp', '// Expect to stop here after step-out (clang)')
+self.step_out_destination = line_number(
+ 'main.cpp', '// Expect to stop here after step-out.')
 
 def step_out_single_thread_with_cmd(self):
 self.step_out_with_cmd("this-thread")

diff  --git a/lldb/test/API/functionalities/thread/step_out/main.cpp 
b/lldb/test/API/functionalities/thread/step_out/main.cpp
index 14d84010de8a..e7dd230d239c 100644
--- a/lldb/test/API/functionalities/thread/step_out/main.cpp
+++ b/lldb/test/API/functionalities/thread/step_out/main.cpp
@@ -19,10 +19,10 @@ thread_func ()
 pseudo_barrier_wait(g_barrier);
 
 // Do something
-step_out_of_here(); // Expect to stop here after step-out (clang)
+step_out_of_here();
 
 // Return
-return NULL;  // Expect to stop here after step-out (icc and gcc)
+return NULL;  // Expect to stop here after step-out.
 }
 
 int main ()



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


[Lldb-commits] [PATCH] D92063: [LLDB] RegisterInfoPOSIX_arm64 remove unused bytes from g/G packet

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

In D92063#2417815 , @mgorny wrote:

> In D92063#2416162 , @labath wrote:
>
>> +@mgorny, as he's been navigating these waters lately...
>>
>> So... I presume we can't just slap `__attribute__((packed))` on the 
>> structure, because the kernel actually expects that the data structure will 
>> have the extra space for the padding. Is that so?
>>
>> Even if we can't, I'm wondering if it wouldn't be cleaner to use two 
>> structures for this. Something like:
>>
>>   LLVM_PACKED_START
>>   struct GPR {
>> // as before...
>>   };
>>   /// Big comment explaining the purpose of padding
>>   struct GPRBuffer: GPR {
>> uint32_t pad;
>>   };
>>   LLVM_PACKED_END
>>
>> and then using GPR or GPRBuffer accordingly. What do you think?
>
> That would imply adding additional offset field to the register lists, 
> wouldn't it? Not that I'm opposed — it might be reasonable to have the option 
> to override the offset for system structs, coredumps...

Register infos should contain g/G packet offset and Ideally offset calculation 
should look something like this: reg[index].byte_offset = reg[index - 
1].byte_offset + reg[index - 1].byte_size.

In case of AArch64 we are using GPR struct to calculate offsets which I think 
is inspired from the thinking that offset == ptrace offset rather than the g/G 
packet offset. Coincidentally ptrace offsets do no interfere with g/G packet 
offset the way we are calculating them right now except for this pad bytes 
added at the end.  And I have fixed that anyway in my latest update.


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

https://reviews.llvm.org/D92063

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


[Lldb-commits] [PATCH] D92063: [LLDB] RegisterInfoPOSIX_arm64 remove unused bytes from g/G packet

2020-11-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

I disagree. Since we're repeating gdb protocol, it would be nice to use offsets 
consistent with the gdb protocol, even if it means some extra padding. I do 
realize that this is broken right now and not trivially fixable but I don't 
think we should make things worse.


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

https://reviews.llvm.org/D92063

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


[Lldb-commits] [PATCH] D92063: [LLDB] RegisterInfoPOSIX_arm64 remove unused bytes from g/G packet

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

In D92063#2417948 , @mgorny wrote:

> I disagree. Since we're repeating gdb protocol, it would be nice to use 
> offsets consistent with the gdb protocol, even if it means some extra 
> padding. I do realize that this is broken right now and not trivially fixable 
> but I don't think we should make things worse.

I dont understand your disagreement. If you look at the current update of this 
patch, this is exactly what we are doing i-e Making offsets consistent with GDB 
protocol.


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

https://reviews.llvm.org/D92063

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


[Lldb-commits] [PATCH] D92063: [LLDB] RegisterInfoPOSIX_arm64 remove unused bytes from g/G packet

2020-11-26 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

(I haven't looked at the new changes yet.)

In D92063#2417948 , @mgorny wrote:

> I disagree. Since we're repeating gdb protocol, it would be nice to use 
> offsets consistent with the gdb protocol, even if it means some extra padding.

I'm not sure what you mean by that. Are you implying that the gdb protocol (as 
implemented by gdb, let's say) does indeed have this padding in its `g` packet?

The point of this patch series is to make the our `g` packet more consistent 
with the "official" gdb-remote definition. The motivation for that are the SVE 
registers on arm which have a length that can change at runtime. The point of 
this makes the "offset" fields in the qRegisterInfo packets (and target.xml) 
meaningless. So we made a choice to just stop using them (in the packets, we 
still obviously need to know where the registers go) and have the client 
recompute the offsets according to the official algorithm. This means that 
there can be no random gaps in the packet data. The goal is to make the SVE 
implementation possible/saner and also bring us closer to the gdb's definition 
of these packets (which does not include an "offset" field in its target.xml).


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

https://reviews.llvm.org/D92063

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


[Lldb-commits] [PATCH] D92063: [LLDB] RegisterInfoPOSIX_arm64 remove unused bytes from g/G packet

2020-11-26 Thread Michał Górny via Phabricator via lldb-commits
mgorny added a comment.

I was referring to:

> Ideally offset calculation should look something like this: 
> reg[index].byte_offset = reg[index - 1].byte_offset + reg[index - 
> 1].byte_size.

I'm not saying this is wrong for the time being but IMO we should assume that 
we might need to have a non-obvious offset in the future.


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

https://reviews.llvm.org/D92063

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


[Lldb-commits] [PATCH] D92164: Make SBDebugger internal variable getter and setter not use CommandInterpreter's context

2020-11-26 Thread Tatyana Krasnukha via Phabricator via lldb-commits
tatyana-krasnukha created this revision.
tatyana-krasnukha added reviewers: jingham, JDevlieghere, labath.
tatyana-krasnukha added a project: LLDB.
Herald added a subscriber: lldb-commits.
tatyana-krasnukha requested review of this revision.

SBDebugger asks CommandInterpreter for execution context, however, the 
interpreter's context will not be updated until a command will be executed (may 
never happen when using SB API). It means that the behavior of these functions 
depends on previous user actions. The context can stay uninitialized, point to 
a currently selected target, or point to one of the previously selected targets.

This patch makes SBDebugger use execution context built upon the selected 
target. Notice, that even SBCommandInterpreter::GetProcess doesn't use 
CommandInterpreter's execution context.
Also, add error logging to GetInternalVariableValue.

Added test reproduces the issue. Without this fix, the last assertion fails 
because the interpreter's execution context is empty until running "target 
list", so, the value of the global property was updated instead of the 
process's local instance.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92164

Files:
  lldb/packages/Python/lldbsuite/test/python_api/debugger/Makefile
  lldb/packages/Python/lldbsuite/test/python_api/debugger/main.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/test/API/python_api/debugger/TestDebuggerAPI.py

Index: lldb/test/API/python_api/debugger/TestDebuggerAPI.py
===
--- lldb/test/API/python_api/debugger/TestDebuggerAPI.py
+++ lldb/test/API/python_api/debugger/TestDebuggerAPI.py
@@ -43,3 +43,54 @@
 target = lldb.SBTarget()
 self.assertFalse(target.IsValid())
 self.dbg.DeleteTarget(target)
+
+def test_debugger_internal_variable(self):
+"""Ensure that SBDebugger reachs the same instance of properties
+   regardless CommandInterpreter's context initialization"""
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+property_name = "target.process.memory-cache-line-size"
+
+def get_cache_line_size():
+value_list = lldb.SBStringList()
+value_list = self.dbg.GetInternalVariableValue(property_name,
+   self.dbg.GetInstanceName())
+
+self.assertEqual(value_list.GetSize(), 1)
+try:
+return int(value_list.GetStringAtIndex(0))
+except ValueError as error:
+self.fail("Value is not a number: " + error)
+
+# Get global property value while there are no processes.
+global_cache_line_size = get_cache_line_size()
+
+# Run a process via SB interface. CommandInterpreter's execution context
+# remains empty.
+error = lldb.SBError()
+launch_info = lldb.SBLaunchInfo(None)
+launch_info.SetLaunchFlags(lldb.eLaunchFlagStopAtEntry)
+process = target.Launch(launch_info, error)
+self.assertTrue(process, PROCESS_IS_VALID)
+
+# This should change the value of a process's local property.
+new_cache_line_size = global_cache_line_size + 512
+error = self.dbg.SetInternalVariable(property_name,
+ str(new_cache_line_size),
+ self.dbg.GetInstanceName())
+self.assertTrue(error.Success(),
+property_name + " value was changed successfully")
+
+# Check that it was set actually.
+self.assertEqual(get_cache_line_size(), new_cache_line_size)
+
+# Run any command to initialize CommandInterpreter's execution context.
+self.runCmd("target list")
+
+# Test the local property again, is it set to new_cache_line_size?
+self.assertEqual(get_cache_line_size(), new_cache_line_size)
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1268,16 +1268,18 @@
   SBError sb_error;
   DebuggerSP debugger_sp(Debugger::FindDebuggerWithInstanceName(
   ConstString(debugger_instance_name)));
-  Status error;
-  if (debugger_sp) {
-ExecutionContext exe_ctx(
-debugger_sp->GetCommandInterpreter().GetExecutionContext());
-error = debugger_sp->SetPropertyValue(&exe_ctx, eVarSetOperationAssign,
-  var_name, value);
-  } else {
-error.SetErrorStringWithFormat("invalid debugger instance name '%s'",
-   debugger_instance_name);
+
+  if (!debugger_sp) {
+sb_error.SetErrorStringWithFormat("invalid debugger instance name '%s'",
+  debugger_i

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

2020-11-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 307822.
DavidSpickett added a comment.

- Use auto for map find
- Document PortMap functions
- Remove unnecessary casts
- Move tests to Process/gdb-remote
- Cleanup Expected asserts

The behaviour of AssociatePortWithProcess is a bit
non obvious (that it can move a port to a different pid)
but I'm not inclined to change it now.

So I've just added a FIXME to note that this usage
will probably fail if we do later share the PortMap.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91634

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/unittests/Process/gdb-remote/CMakeLists.txt
  lldb/unittests/Process/gdb-remote/PortMapTest.cpp

Index: lldb/unittests/Process/gdb-remote/PortMapTest.cpp
===
--- /dev/null
+++ lldb/unittests/Process/gdb-remote/PortMapTest.cpp
@@ -0,0 +1,115 @@
+//===-- PortMapTest.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
+//
+//===--===//
+
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h"
+
+using namespace lldb_private::process_gdb_remote;
+
+TEST(PortMapTest, Constructors) {
+  // Default construct to empty map
+  GDBRemoteCommunicationServerPlatform::PortMap p1;
+  ASSERT_TRUE(p1.empty());
+
+  // Empty means no restrictions, return 0 and and bind to get a port
+  llvm::Expected available_port = p1.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(0));
+
+  // Adding any port makes it not empty
+  p1.AllowPort(1);
+  ASSERT_FALSE(p1.empty());
+
+  // So we will return the added port this time
+  available_port = p1.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(1));
+
+  // Construct from a range of ports
+  GDBRemoteCommunicationServerPlatform::PortMap p2(1, 4);
+  ASSERT_FALSE(p2.empty());
+
+  // Use up all the ports
+  for (uint16_t expected = 1; expected < 4; ++expected) {
+available_port = p2.GetNextAvailablePort();
+ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(expected));
+p2.AssociatePortWithProcess(*available_port, 1);
+  }
+
+  // Now we fail since we're not an empty port map but all ports are used
+  available_port = p2.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::Failed());
+}
+
+TEST(PortMapTest, FreePort) {
+  GDBRemoteCommunicationServerPlatform::PortMap p(1, 4);
+  // Use up all the ports
+  for (uint16_t port = 1; port < 4; ++port) {
+p.AssociatePortWithProcess(port, 1);
+  }
+
+  llvm::Expected available_port = p.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::Failed());
+
+  // Can't free a port that isn't in the map
+  ASSERT_FALSE(p.FreePort(0));
+  ASSERT_FALSE(p.FreePort(4));
+
+  // After freeing a port it becomes available
+  ASSERT_TRUE(p.FreePort(2));
+  available_port = p.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(2));
+}
+
+TEST(PortMapTest, FreePortForProcess) {
+  GDBRemoteCommunicationServerPlatform::PortMap p;
+  p.AllowPort(1);
+  p.AllowPort(2);
+  ASSERT_TRUE(p.AssociatePortWithProcess(1, 11));
+  ASSERT_TRUE(p.AssociatePortWithProcess(2, 22));
+
+  // All ports have been used
+  llvm::Expected available_port = p.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::Failed());
+
+  // Can't free a port for a process that doesn't have any
+  ASSERT_FALSE(p.FreePortForProcess(33));
+
+  // You can move a used port to a new pid
+  ASSERT_TRUE(p.AssociatePortWithProcess(1, 99));
+
+  ASSERT_TRUE(p.FreePortForProcess(22));
+  available_port = p.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::Succeeded());
+  ASSERT_EQ(2, *available_port);
+
+  // proces 22 no longer has a port
+  ASSERT_FALSE(p.FreePortForProcess(22));
+}
+
+TEST(PortMapTest, AllowPort) {
+  GDBRemoteCommunicationServerPlatform::PortMap p;
+
+  // Allow port 1 and tie it to process 11
+  p.AllowPort(1);
+  ASSERT_TRUE(p.AssociatePortWithProcess(1, 11));
+
+  // Allowing it a second time shouldn't change existing mapping
+  p.AllowPort(1);
+  llvm::Expected available_port = p.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::Failed());
+
+  // A new port is marked as free when allowed
+  p.AllowPort(2);
+  available_port = p.GetNextAva

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

2020-11-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett marked 6 inline comments as done.
DavidSpickett added inline comments.



Comment at: lldb/tools/lldb-server/lldb-platform.cpp:234
   if (ch == 'P')
-gdbserver_portmap[portnum] = LLDB_INVALID_PROCESS_ID;
+gdbserver_portmap.AllowPort(static_cast(portnum));
   else if (ch == 'm')

labath wrote:
> Why the cast?
Me misreading a warning I think, removed here and below.


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] D92035: [lldb] Use llvm::Optional for port in LaunchGDBServer

2020-11-26 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett updated this revision to Diff 307824.
DavidSpickett added a comment.

- Reformat GDBRemoteCommunicationServerPlatform.h to fix include order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92035

Files:
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
  lldb/tools/lldb-server/lldb-platform.cpp

Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -349,13 +349,13 @@
 if (platform.IsConnected()) {
   if (inferior_arguments.GetArgumentCount() > 0) {
 lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
-uint16_t port = 0;
+llvm::Optional port = 0;
 std::string socket_name;
 Status error = platform.LaunchGDBServer(inferior_arguments,
 "", // hostname
 pid, port, socket_name);
 if (error.Success())
-  platform.SetPendingGdbServer(pid, port, socket_name);
+  platform.SetPendingGdbServer(pid, *port, socket_name);
 else
   fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
   }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -16,6 +16,7 @@
 #include "GDBRemoteCommunicationServerCommon.h"
 #include "lldb/Host/Socket.h"
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Error.h"
 
 namespace lldb_private {
@@ -84,8 +85,10 @@
 
   void SetInferiorArguments(const lldb_private::Args &args);
 
+  // Set port if you want to use a specific port number.
+  // Otherwise port will be set to the port that was chosen for you.
   Status LaunchGDBServer(const lldb_private::Args &args, std::string hostname,
- lldb::pid_t &pid, uint16_t &port,
+ lldb::pid_t &pid, llvm::Optional &port,
  std::string &socket_name);
 
   void SetPendingGdbServer(lldb::pid_t pid, uint16_t port,
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
@@ -157,8 +157,8 @@
 
 Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
 const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-uint16_t &port, std::string &socket_name) {
-  if (port == UINT16_MAX) {
+llvm::Optional &port, std::string &socket_name) {
+  if (!port) {
 llvm::Expected available_port = m_port_map.GetNextAvailablePort();
 if (available_port)
   port = *available_port;
@@ -179,7 +179,7 @@
 
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM));
   LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-port);
+*port);
 
   // Do not run in a new session so that it can not linger after the platform
   // closes.
@@ -194,7 +194,7 @@
 #if !defined(__APPLE__)
   url << m_socket_scheme << "://";
 #endif
-  uint16_t *port_ptr = &port;
+  uint16_t *port_ptr = port.getPointer();
   if (m_socket_protocol == Socket::ProtocolTcp) {
 llvm::StringRef platform_scheme;
 llvm::StringRef platform_ip;
@@ -205,7 +205,7 @@
platform_port, platform_path);
 UNUSED_IF_ASSERT_DISABLED(ok);
 assert(ok);
-url << platform_ip.str() << ":" << port;
+url << platform_ip.str() << ":" << *port;
   } else {
 socket_name = GetDomainSocketPath("gdbserver").GetPath();
 url << socket_name;
@@ -219,11 +219,11 @@
   if (pid != LLDB_INVALID_PROCESS_ID) {
 std::lock_guard guard(m_spawned_pids_mutex);
 m_spawned_pids.insert(pid);
-if (port > 0)
-  m_port_map.AssociatePortWithProcess(port, pid);
+if (*port > 0)
+  m_port_map.AssociatePortWithProcess(*port, pid);
   } else {
-if (port > 0)
-  m_port_map.FreePort(port);
+if (*port > 0)
+  m_port_map.FreePort(*port);
   }
   return error;
 }
@@ -243,12 +243,15 @@
   packet.SetFilePos(::strlen("qLaunchGDBServer;"));
   llvm::StringRef name;
   llvm::StringRef value;
-  uint16_t port = UINT16_MAX;
+  llvm::Optional port;
   while (packet.GetNameColonValue(name, value)) {
 if (name.equals("host"))
   hostname = std::string(value);
-else if (name.equals("port"))
-  value.getAsInteger(0, port);
+else if (name.e

[Lldb-commits] [PATCH] D92187: [lldb] [FreeBSD] Fix establishing DT_NEEDED libraries from dyld

2020-11-26 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.

Always set up entry point breakpoint on FreeBSD in order to establish
the initial list of loaded libraries.  Normally this list is created
through dyld library load events but apparently these are not emitted
for DT_NEEDED libraries on FreeBSD, only for libraries loaded
afterwards.

This makes it possible to reenable memory maps and therefore let
the related tests pass.


https://reviews.llvm.org/D92187

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  
lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
  lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test

Index: lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
===
--- lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
+++ lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test
@@ -3,7 +3,6 @@
 
 # REQUIRES: target-x86_64
 # UNSUPPORTED: system-windows
-# XFAIL: system-freebsd
 
 # RUN: %clang_host %p/Inputs/call-asm.c -x assembler-with-cpp %p/Inputs/thread-step-out-ret-addr-check.s -o %t
 # RUN: not %lldb %t -s %s -b 2>&1 | FileCheck %s
Index: lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
===
--- lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
+++ lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
@@ -120,7 +120,7 @@
 
 @llgs_test
 @skipUnlessPlatform(["linux", "android", "freebsd", "netbsd"])
-@expectedFailureAll(oslist=["freebsd", "netbsd"])
+@expectedFailureAll(oslist=["netbsd"])
 def test_libraries_svr4_load_addr(self):
 self.setup_test()
 self.libraries_svr4_has_correct_load_addr()
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -918,7 +918,6 @@
 self.qMemoryRegionInfo_is_supported()
 
 @llgs_test
-@expectedFailureAll(oslist=["freebsd"])
 def test_qMemoryRegionInfo_is_supported_llgs(self):
 self.init_llgs_test()
 self.build()
@@ -983,7 +982,6 @@
 self.qMemoryRegionInfo_reports_code_address_as_executable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_code_address_as_executable_llgs(self):
 self.init_llgs_test()
@@ -1050,7 +1048,6 @@
 self.qMemoryRegionInfo_reports_stack_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_stack_address_as_readable_writeable_llgs(
 self):
@@ -1117,7 +1114,6 @@
 self.qMemoryRegionInfo_reports_heap_address_as_readable_writeable()
 
 @skipIfWindows # No pty support to test any inferior output
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test_qMemoryRegionInfo_reports_heap_address_as_readable_writeable_llgs(
 self):
Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,7 +23,6 @@
 'main.cpp',
 '// Run here before printing memory regions')
 
-@expectedFailureAll(oslist=["freebsd"])
 def test(self):
 self.build()
 
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -485,9 +485,6 @@
 Status NativeProcessFreeBSD::GetMemoryRegionInfo(lldb::addr_t load_addr,
  MemoryRegionInfo &range_info) {
 
-  // TODO: figure out why it breaks stuff
-  return Status("currently breaks determining module list");
-
   if (m_supports_mem_region == LazyBool::eLazyBoolNo) {
 // We're done.
 return Status("unsupported");
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/D

[Lldb-commits] [lldb] 3f6c856 - [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-26 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2020-11-26T18:01:30+01:00
New Revision: 3f6c856bb5ae4426a586426bca9f1ef2848a2b12

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

LOG: [ASTImporter] Import the default argument of TemplateTypeParmDecl

The test case isn't using the AST matchers for all checks as there doesn't seem 
to be support for
matching TemplateTypeParmDecl default arguments. Otherwise this is simply 
importing the
default arguments.

Also updates several LLDB tests that now as intended omit the default template
arguments of several std templates.

Reviewed By: martong

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py

lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py

lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/list/TestListFromStdModule.py

lldb/test/API/commands/expression/import-std-module/queue/TestQueueFromStdModule.py

lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py

lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py

lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector-bool/TestVectorBoolFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py

lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 835551528e0d..5159682da85f 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5158,8 +5158,6 @@ 
ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
   // context. This context will be fixed when the actual template declaration
   // is created.
 
-  // FIXME: Import default argument  and constraint expression.
-
   ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
   if (!BeginLocOrErr)
 return BeginLocOrErr.takeError();
@@ -5206,6 +5204,14 @@ 
ASTNodeImporter::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
 ToIDC);
   }
 
+  if (D->hasDefaultArgument()) {
+Expected ToDefaultArgOrErr =
+import(D->getDefaultArgumentInfo());
+if (!ToDefaultArgOrErr)
+  return ToDefaultArgOrErr.takeError();
+ToD->setDefaultArgument(*ToDefaultArgOrErr);
+  }
+
   return ToD;
 }
 

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp 
b/clang/unittests/AST/ASTImporterTest.cpp
index 33e4b7226fba..5a93a7348e7a 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -880,6 +880,25 @@ TEST_P(ImportExpr, DependentSizedArrayType) {
  has(fieldDecl(hasType(dependentSizedArrayType(;
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, TemplateTypeParmDeclNoDefaultArg) {
+  Decl *FromTU = getTuDecl("template struct X {};", Lang_CXX03);
+  auto From = FirstDeclMatcher().match(
+  FromTU, templateTypeParmDecl(hasName("T")));
+  TemplateTypeParmDecl *To = Import(From, Lang_CXX03);
+  ASSERT_FALSE(To->hasDefaultArgument());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, TemplateTypeParmDeclDefaultArg) {
+  Decl *FromTU =
+  getTuDecl("template struct X {};", Lang_CXX03);
+  auto From = FirstDeclMatcher().match(
+  FromTU, templateTypeParmDecl(hasName("T")));
+  TemplateTypeParmDecl *To = Import(From, Lang_CXX03);
+  ASSERT_TRUE(To->hasDefaultArgument());
+  QualType ToArg = To->getDefaultArgument();
+  ASSERT_EQ(ToArg, QualType(To->getASTContext().IntTy));
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, ImportBeginLocOfDeclRefExpr) {
   Decl *FromTU =
   getTuDecl("class A { public: static int X; }; void f() { (void)A::X; }",

diff  --git 
a/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
index 18bd8ae37ff9..0eaa50a12727 100644
--- 
a/lldb/test/API

[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-26 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f6c856bb5ae: [ASTImporter] Import the default argument of 
TemplateTypeParmDecl (authored by teemperor).
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D92103?vs=307614&id=307891#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/list/TestListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/queue/TestQueueFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
  
lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-bool/TestVectorBoolFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py

Index: lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
@@ -22,7 +22,7 @@
 
 self.runCmd("settings set target.import-std-module true")
 
-vector_type = "std::vector >"
+vector_type = "std::vector"
 size_type = vector_type + "::size_type"
 value_type = "std::__vector_base >::value_type"
 iterator = vector_type + "::iterator"
Index: lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
@@ -20,13 +20,9 @@
   "// Set break point at this line.",
   lldb.SBFileSpec("main.cpp"))
 
-vector_type = "std::vector >"
-vector_of_vector_type = "std::vector<" + vector_type + \
-", std::allocator > > >"
-size_type = (
-"std::vector >, " +
-"std::allocator > >" +
-" >::size_type")
+vector_type = "std::vector"
+vector_of_vector_type = "std::vector<" + vector_type + " >"
+size_type = vector_of_vector_type + "::size_type"
 value_type = "std::__vector_base >::value_type"
 
 self.runCmd("settings set target.import-std-module true")
@@ -35,13 +31,13 @@
 "a",
 result_type=vector_of_vector_type,
 result_children=[
-ValueCheck(type="std::vector >",
+ValueCheck(type="std::vector",
children=[
ValueCheck(value='1'),
ValueCheck(value='2'),
ValueCheck(value='3'),
]),
-ValueCheck(type="std::vector >",
+ValueCheck(type="std::vector",
children=[
ValueCheck(value='3'),
ValueCheck(value='2'),
Index: lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -23,7 +23,7 @@
 
 self.runCmd("s

[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-26 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: clang/lib/AST/ASTImporter.cpp:5161
 
-  // FIXME: Import default argument  and constraint expression.
+  // FIXME: Import constraint expression.
 

martong wrote:
> I wonder whether this FIXME is already fixed, at line 5182?
I think so. I removed the whole comment instead. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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


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

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

This update improves on last patch by adding support for register ordering in 
increasing order of register number even if register numbers are used randomly 
in target xml. Also we set offset to LLDB_INVALID_INDEX32 while initializing 
and if an offset is provided by remote it will be updated. However if no offset 
was provided an appropriate offset will be calculated incrementally by 
traversing a map which sorts registers in increasing order of remote register 
numbers.

A AArch64 test is included which places cpsr at the top of target xml 
description with regnum field populated. Also w registers derive their offset 
from corresponding x registers. All register values are checked with a provided 
g packet result.


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/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegOffsets.py
  
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/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegOffsets.py
===
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestAArch64XMLRegOffsets.py
@@ -0,0 +1,151 @@
+from __future__ import print_function
+from textwrap import dedent
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class MyResponder(MockGDBServerResponder):
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+return dedent("""\
+
+  
+aarch64
+
+  
+  
+  
+  
+