[Lldb-commits] [lldb] b6e04ac - [lldb/test] Avoid the socket "pump" thread

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-11-30T09:07:45+01:00
New Revision: b6e04ac5aa881c1fbb66da884b04e48dfb102474

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

LOG: [lldb/test] Avoid the socket "pump" thread

A separate thread is not necessary, as we can do its work on the main
thread, while waiting for the packet to arrive. This makes the code
easier to understand and debug (other simplifications are possible too,
but I'll leave that for separate patches). The new implementation also
avoids busy waiting.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
index 0c01bdfb1c69..b5c635a77b5c 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
@@ -236,7 +236,7 @@ def expect_lldb_gdbserver_replay(
 if sequence_entry.is_output_matcher():
 try:
 # Grab next entry from the output queue.
-content = pump_queues.output_queue().get(True, 
timeout_seconds)
+content = pump.get_output(timeout_seconds)
 except queue.Empty:
 if logger:
 logger.warning(
@@ -247,7 +247,7 @@ def expect_lldb_gdbserver_replay(
 pump.get_accumulated_output()))
 else:
 try:
-content = pump_queues.packet_queue().get(True, 
timeout_seconds)
+content = pump.get_packet(timeout_seconds)
 except queue.Empty:
 if logger:
 logger.warning(

diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
index 3de76345896d..6c41ed473b45 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/socket_packet_pump.py
@@ -5,6 +5,7 @@
 import re
 import select
 import threading
+import time
 import traceback
 
 from six.moves import queue
@@ -74,8 +75,6 @@ def __init__(self, pump_socket, pump_queues, logger=None):
 if not pump_socket:
 raise Exception("pump_socket cannot be None")
 
-self._thread = None
-self._stop_thread = False
 self._socket = pump_socket
 self._logger = logger
 self._receive_buffer = ""
@@ -83,29 +82,42 @@ def __init__(self, pump_socket, pump_queues, logger=None):
 self._pump_queues = pump_queues
 
 def __enter__(self):
-"""Support the python 'with' statement.
-
-Start the pump thread."""
-self.start_pump_thread()
+self._receive_buffer = ""
+self._accumulated_output = ""
 return self
 
 def __exit__(self, exit_type, value, the_traceback):
-"""Support the python 'with' statement.
-
-Shut down the pump thread."""
-self.stop_pump_thread()
+pass
+
+def _read(self, timeout_seconds, q):
+now = time.monotonic()
+deadline = now + timeout_seconds
+while q.empty() and now <= deadline:
+can_read, _, _ = select.select([self._socket], [], [], 
deadline-now)
+now = time.monotonic()
+if can_read and self._socket in can_read:
+try:
+new_bytes = 
seven.bitcast_to_string(self._socket.recv(4096))
+if self._logger and new_bytes and len(new_bytes) > 0:
+self._logger.debug(
+"pump received bytes: {}".format(new_bytes))
+except:
+# Likely a closed socket.  Done with the pump thread.
+if self._logger:
+self._logger.debug(
+"socket read failed, stopping pump read thread\n" +
+traceback.format_exc(3))
+break
+self._process_new_bytes(new_bytes)
+if q.empty():
+raise queue.Empty()
+return q.get(True)
 
-def start_pump_thread(self):
-if self._thread:
-raise Exception("pump thread is already running")
-self._stop_thread = False
-self._thread = threading.Thread(target=self._run_method)
-self._thr

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

What's the state of TestBreakpointInGlobalConstructor.py  after this change? 
Does it continue to fail?

If so, then this is probably a bug in FreeBSD that is worth reporting, as it 
means that it is not possible to debug the initialization code (global 
constructors, `__attribute__((init))`, etc.) of libraries loaded at startup 
(DT_NEEDED). It might be also worth checking what gdb does in this case. If gdb 
is able to break in this code, then it means that there is a different way to 
fetch the loaded libraries early enough, and we could possibly emulate that... 
(I would very much love that, as the current approach is pretty hacky already)


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

https://reviews.llvm.org/D92187

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

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

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

mgorny added a comment.

In D92187#2422085 , @labath wrote:

> What's the state of TestBreakpointInGlobalConstructor.py  after this change? 
> Does it continue to fail?

Yes, it does.

> If so, then this is probably a bug in FreeBSD that is worth reporting, as it 
> means that it is not possible to debug the initialization code (global 
> constructors, `__attribute__((init))`, etc.) of libraries loaded at startup 
> (DT_NEEDED). It might be also worth checking what gdb does in this case. If 
> gdb is able to break in this code, then it means that there is a different 
> way to fetch the loaded libraries early enough, and we could possibly emulate 
> that... (I would very much love that, as the current approach is pretty hacky 
> already)

I will check that.


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

https://reviews.llvm.org/D92187

___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks great. One question about the member variable...




Comment at: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h:90
   dynamic_reg_size_map m_dynamic_reg_size_map;
+  reg_to_reg_map m_remote_to_local_regnum_map;
   size_t m_reg_data_byte_size = 0u; // The number of bytes required to store

If, I understand correctly, this is only used inside the Finalize function? 
Could this be a local variable instead of a member?


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] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/bindings/lua/lua-wrapper.swig:21-22
+{
+   lldb::SBFrame sb_frame(stop_frame_sp);
+   lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
+

tammela wrote:
> labath wrote:
> > What's up with the copying? Is it to ensure the callback does not modify 
> > the caller's values? If so, you could just drop the `&` from the signature, 
> > and have the compiler copy these for you...
> `SBFrame` and `SBBreakpointLocation` only provide copy constructors. I can't 
> see the difference between the two strategies, could you elaborate? 
Ok, let's try this.

My main point was that a function which receives an argument as a (non-const) 
reference, and then does not modify the referenced object is weird. One just 
doesn't do that. It should either take a const reference or a plain value. 
Using a plain value is simpler, particularly in cases where you need to locally 
modify the object you received, but you don't want the modifications to affect 
the callers value. That's because in this case the compiler will do the copying 
for you:
```
void foo(SBFrame foo, SBFrame &bar, const SBFrame &baz) {
  // I don't have to worry about modifying foo, because it's my local copy
  foo = ...;
  
  // Modifications to bar will be visible in the caller
  bar = ...;

  // I can't modify baz. If I really need to do that, I need to make a local 
copy
  SBFrame baz_copy = baz;
  baz_copy = ...;
}
```



Comment at: 
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test:10
+run
+# CHECK: frame #0
+# CHECK: Process {{[0-9]+}} exited with status = 0

"frame #0" will also get printed if the process stops for any reason. I'd 
suggest printing a completely random string instead (`print("bacon")`, for 
example)



Comment at: 
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test:5
+b main
+breakpoint command add -s lua -o 'print(frame)'
+run

tammela wrote:
> labath wrote:
> > Could we also have a test where calling the callback raises an error? And 
> > for the different values of the "stop" argument that can be returned from 
> > the callback? And when compiling the callback expression results in an 
> > error?
> Apparently when the user sets the breakpoint and it fails to compile the code 
> lldb will not report any errors and it will fail silently.  I will address 
> this particular test case in another patch since it affects Python as well.
Sounds good.


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] D92223: [lldb] Add support for looking up static const members

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I am not sure if this is the right way to implement this feature. Changing 
ManualDWARFIndex to provide this additional information is easy enough, but it 
means that the other index classes will never be able to support this 
functionality (without, essentially, reimplementing ManualDWARFIndex and 
scanning all debug info). I am also worried about the increase to the manual 
index size, as this would mean that every occurrence of the DW_TAG_member would 
be placed in the index, whereas now we only place the one which has a location 
(which is normally just in a single compile unit).

I think it might be better to do something at the 
`SymbolFileDWARF::FindGlobalVariables` level, so that it is common to all 
indexes. For example, this function could (in case the normal search does not 
provide any information) try to strip the last component of the name 
(`foo::bar::baz` => `foo::bar`) and then try to look up the result as a class 
(type). If it finds the type, it could then check it for the specific (static) 
variable name.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3183
+// Allows only members with DW_AT_const_value attribute, i.e. static const
+// or static constexr.
+return nullptr;

typo

(also "static constexpr" implies "static const" so maybe there's no need to 
explicitly mention the latter...)



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3248
+  (parent_tag == DW_TAG_compile_unit || parent_tag == DW_TAG_partial_unit 
||
+   tag == DW_TAG_member) &&
+  (parent_context_die.Tag() == DW_TAG_class_type ||

Why is this needed. I would expect this to evaluate to `true` even without this 
addition...



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3270
 if ((parent_tag == DW_TAG_compile_unit ||
- parent_tag == DW_TAG_partial_unit) &&
+ parent_tag == DW_TAG_partial_unit || tag == DW_TAG_member) &&
 Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(

Same here.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3518
   if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) ||
+  (tag == DW_TAG_member) ||
   (tag == DW_TAG_formal_parameter && sc.function)) {

Why is this needed? I wouldn't expect a member inside a function...



Comment at: lldb/test/API/python_api/target/globals/TestTargetGlobals.py:15-17
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)

delete


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92223

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

[Lldb-commits] [PATCH] D92264: [lldb] [POSIX-DYLD] Update the cached exe path after attach

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Seems reasonable(ish), but needs a test case. Do you need to launch a separate 
server instance, or would just a plain attach work?


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

https://reviews.llvm.org/D92264

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

[Lldb-commits] [PATCH] D92249: [LLDB/Python] Fix segfault on Python scripted breakpoints

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D92249#2421639 , @JDevlieghere 
wrote:

> It appears there are many similar calls in this file, can you update those as 
> well?

If there are many of these calls, adding a static helper function for this 
purpose might be a good idea as well...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92249

___
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

[Pavel Labath via Phabricator via lldb-commits]

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

In D92063#2417989 , @mgorny wrote:

> 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.

Ok. I think I understand what you meant now. Overall, I think that having the 
registers placed in the `g` packet in the right order and without any gaps (as 
the spec prescribes) is a good idea. Doing that cleanly might not be so easy 
though.

The way I see it, our main problem is that the RegisterInfo struct just serves 
too many masters. The `byte_offset` field in particular is used both as a 
gdb-remote offset, and as the offset into various os data structures.

Sometimes one can use the same offset for both numbers, and then everything is 
fine. But there are cases where this is not possible and then things start to 
get ugly.

I don't think that adding another field to this struct is a good solution, as 
it does not scale. In reality, noone needs to know both numbers. 
NativeXXXProcess only deals with the ptrace and it doesn't (shouldn't) care 
about gdb-remote offsets. gdb-remote code only cares about laying out the `g` 
packet and does not care how the register values are obtained.

One solution for this would be to invert our representation of register 
information (vector of structs => struct of vectors). That way it would be easy 
for anyone to add a parallel vector to represent any additional register 
information it wants, and the rest of the code could just ignore that. llvm's 
register information is organized is a somewhat similar way.

But that's a pretty long way away. For now we have to figure out a way to share 
the offset fields, and I think this patch makes a good effort at that.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h:99-100
 
+  size_t GetGPRBufferSize() { return sizeof(m_gpr_arm64); }
+
 private:

Please put this next to the `GetGPRBuffer` function. And add comments to 
explain the difference between the two.


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] D91634: [lldb] Error when there are no ports to launch a gdbserver on

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 308289.
DavidSpickett added a comment.

- Document default constructor behaviour.


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.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(2));
+
+  // 11 should still be tied to port 1
+  ASSERT_TRUE(p.FreePortForProcess(11));
+}
Index: lldb/unittests/Process/gdb-remote/CMakeLists.txt
===
--- lldb/unittests/Process/gdb-remote/CMakeLists.txt
+++ lldb/unitte

[Lldb-commits] [PATCH] D92264: [lldb] [POSIX-DYLD] Update the cached exe path after attach

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

mgorny added a comment.

In D92264#2422199 , @labath wrote:

> Seems reasonable(ish), but needs a test case. Do you need to launch a 
> separate server instance, or would just a plain attach work?

It failed only when connecting to remote lldb-server.


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

https://reviews.llvm.org/D92264

___
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

[David Spickett via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG98e87f76d0f4: [lldb] Error when there are no ports to launch 
a gdbserver on (authored by DavidSpickett).

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.GetNextAvailablePort();
+  ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(2));
+
+  // 11 should still be tied to port 1
+  ASSERT_TRUE(p.FreePortForProcess(11));
+}
Index: lldb/unittests/Process/gdb-remote/CMakeLists.txt

[Lldb-commits] [lldb] 98e87f7 - [lldb] Error when there are no ports to launch a gdbserver on

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2020-11-30T10:19:14Z
New Revision: 98e87f76d0f486122d76b334232102e0d7a9254d

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

LOG: [lldb] Error when there are no ports to launch a gdbserver on

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.

To fix this make PortMap it's own class within
GDBRemoteCommunicationServerPlatform.

This almost the same as the old typedef but for
GetNextAvailablePort() returning an llvm::Expected.
This means we have to handle not finding a port,
by returning an error packet.

Also add unit tests for this new PortMap class.

Reviewed By: labath

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

Added: 
lldb/unittests/Process/gdb-remote/PortMapTest.cpp

Modified: 
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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
index 63567bb9b5de..a1cf70f9cd1a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
@@ -27,7 +27,6 @@ class ProcessGDBRemote;
 
 class GDBRemoteCommunicationServer : public GDBRemoteCommunication {
 public:
-  using PortMap = std::map;
   using PacketHandler =
   std::function;

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index 7e94afb9ec68..ae1260221e56 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -42,6 +42,69 @@ using namespace lldb;
 using namespace lldb_private::process_gdb_remote;
 using namespace lldb_private;
 
+GDBRemoteCommunicationServerPlatform::PortMap::PortMap(uint16_t min_port,
+   uint16_t max_port) {
+  for (; min_port < max_port; ++min_port)
+m_port_map[min_port] = LLDB_INVALID_PROCESS_ID;
+}
+
+void GDBRemoteCommunicationServerPlatform::PortMap::AllowPort(uint16_t port) {
+  // Do not modify existing mappings
+  m_port_map.insert({port, LLDB_INVALID_PROCESS_ID});
+}
+
+llvm::Expected
+GDBRemoteCommunicationServerPlatform::PortMap::GetNextAvailablePort() {
+  if (m_port_map.empty())
+return 0; // Bind to port zero and get a port, we didn't have any
+  // limitations
+
+  for (auto &pair : m_port_map) {
+if (pair.second == LLDB_INVALID_PROCESS_ID) {
+  pair.second = ~(lldb::pid_t)LLDB_INVALID_PROCESS_ID;
+  return pair.first;
+}
+  }
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "No free port found in port map");
+}
+
+bool GDBRemoteCommunicationServerPlatform::PortMap::AssociatePortWithProcess(
+uint16_t port, lldb::pid_t pid) {
+  auto pos = m_port_map.find(port);
+  if (pos != m_port_map.end()) {
+pos->second = pid;
+return true;
+  }
+  return false;
+}
+
+bool GDBRemoteCommunicationServerPlatform::PortMap::FreePort(uint16_t port) {
+  std::map::iterator pos = m_port_map.find(port);
+  if (pos != m_port_map.end()) {
+pos->second = LLDB_INVALID_PROCESS_ID;
+return true;
+  }
+  return false;
+}
+
+bool GDBRemoteCommunicationServerPlatform::PortMap::FreePortForProcess(
+lldb::pid_t pid) {
+  if (!m_port_map.empty()) {
+for (auto &pair : m_port_map) {
+  if (pair.second == pid) {
+pair.second = LLDB_INVALID_PROCESS_ID;
+return true;
+  }
+}
+  }
+  return false;
+}
+
+bool GDBRemoteCommunicationServerPlatform::PortMap::empty() const {
+  return m_port_map.empty();
+}
+
 // GDBRemoteCommunicationServerPlatform constructor
 GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
 co

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

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

mgorny added a comment.

GDB indeed handles it somehow. I'm going to investigate.


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

https://reviews.llvm.org/D92187

___
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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D91241#2422100 , @labath wrote:

> Looks great. One question about the member variable...

m_remote_to_local_regnum_map is used by AddRegister where we add a (key,value) 
pair for each of registers. We are doing this to maintain a list of register 
sorted in increasing order of remote register numbers. This list is then used 
in Finalize where we are calculating offsets. Keeping this map as member saves 
us from generating a sorted list of remote register numbers.


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

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2020-11-30T11:20:39Z
New Revision: a7f8d96b16a394a87230d592c727906f67a4ba07

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

LOG: [lldb] Use llvm::Optional for port in LaunchGDBServer

Previously we used UINT16_MAX to mean no port/no specifc
port. This leads to confusion because 65535 is a valid
port number.

Instead use an optional. If you want a specific port call
LaunchGDBServer as normal, otherwise pass an empty optional
and it will be set to the port that gets chosen.
(or left empty in the case where we fail to find a port)

Reviewed By: labath

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

Added: 


Modified: 

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

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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index ae1260221e56..4aa153460941 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -157,8 +157,8 @@ 
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() {}
 
 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 @@ Status 
GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
 
   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 @@ Status 
GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
 #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 @@ Status 
GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
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 @@ Status 
GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
   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 @@ 
GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
   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.equals("port")) {
+  // Make the Optional valid so we can use its value
+  port = 0;
+  value.getAsInteger(0, port.getValue());
+}
   }
 
   lldb::pid_t debugserver_pid = LLDB_INVALID_PROCESS_ID;
@@ -269,8 +272,9 @@ 
GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
 __FUNCTION__, debugserver_pid);
 
   StreamGDBRemote response;
+  assert(port);
   response.Printf("pid:%" PRIu64 ";port:%u;", debugserver_pid,
-  port + m_port_offset);
+  *port + m_port_offset);
   if (!socket_name.empty()) {
 response.PutCString("socket_name:");
 response.PutStringAsRawHex8(socket_name);

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
index 69261dd2ad64..6b964da4a279 100644
--- 
a/lldb/source/Plugins/Proces

[Lldb-commits] [PATCH] D92035: [lldb] Use llvm::Optional for port in LaunchGDBServer

[David Spickett via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7f8d96b16a3: [lldb] Use llvm::Optional for port in 
LaunchGDBServer (authored by DavidSpickett).

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 {
@@ -86,8 +87,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);
-

[Lldb-commits] [PATCH] D92223: [lldb] Add support for looking up static const members

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

Pavel, thanks for review!

In D92223#2422184 , @labath wrote:

> I am not sure if this is the right way to implement this feature. Changing 
> ManualDWARFIndex to provide this additional information is easy enough, but 
> it means that the other index classes will never be able to support this 
> functionality (without, essentially, reimplementing ManualDWARFIndex and 
> scanning all debug info). 
> ...
> I think it might be better to do something at the 
> `SymbolFileDWARF::FindGlobalVariables` level, so that it is common to all 
> indexes. For example, this function could (in case the normal search does not 
> provide any information) try to strip the last component of the name 
> (`foo::bar::baz` => `foo::bar`) and then try to look up the result as a class 
> (type). If it finds the type, it could then check it for the specific 
> (static) variable name.

This makes sense, let me try to implement this approach. A few questions before 
I jump into coding. Do you suggest to call `m_index->GetTypes` from 
`SymbolFileDWARF::FindGlobalVariables` and inspect the DIE for static members? 
Or use something more "high-level", e.g. `SymbolFileDWARF::FindTypes` and 
inspect the returned `lldb_private::Type` object? I had a brief look at `Type` 
and didn't find an easy way to get to static members there. Can you give me a 
pointer?

> I am also worried about the increase to the manual index size, as this would 
> mean that every occurrence of the DW_TAG_member would be placed in the index, 
> whereas now we only place the one which has a location (which is normally 
> just in a single compile unit).

Regarding the index size, we don't put ALL occurrences of `DW_TAG_member` in 
the index, only the ones with constant values. So it should be the same as if 
all these members had a location.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3248
+  (parent_tag == DW_TAG_compile_unit || parent_tag == DW_TAG_partial_unit 
||
+   tag == DW_TAG_member) &&
+  (parent_context_die.Tag() == DW_TAG_class_type ||

labath wrote:
> Why is this needed. I would expect this to evaluate to `true` even without 
> this addition...
In case of `DW_TAG_member` parent_tag is 
`DW_TAG_class_type/DW_TAG_structure_type`, so this check doesn't pass.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3270
 if ((parent_tag == DW_TAG_compile_unit ||
- parent_tag == DW_TAG_partial_unit) &&
+ parent_tag == DW_TAG_partial_unit || tag == DW_TAG_member) &&
 Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(

labath wrote:
> Same here.
As as above, `parent_tag` for `DW_TAG_member` is 
`DW_TAG_class_type/DW_TAG_structure_type`



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3518
   if ((tag == DW_TAG_variable) || (tag == DW_TAG_constant) ||
+  (tag == DW_TAG_member) ||
   (tag == DW_TAG_formal_parameter && sc.function)) {

labath wrote:
> Why is this needed? I wouldn't expect a member inside a function...
I'm not sure I understand, wdym by "member inside a function"?  
`ParseVariables` is called directly from `FindGlobalVariables`, so as far as I 
understand it this check applies for processing global variables as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92223

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

[Lldb-commits] [PATCH] D92249: [LLDB/Python] Fix segfault on Python scripted breakpoints

[Pedro Tammela via Phabricator via lldb-commits]

tammela added a comment.

In D92249#2422205 , @labath wrote:

> In D92249#2421639 , @JDevlieghere 
> wrote:
>
>> It appears there are many similar calls in this file, can you update those 
>> as well?
>
> If there are many of these calls, adding a static helper function for this 
> purpose might be a good idea as well...

Yes, it looks like the same 3-line routine is being replicated across multiple 
functions. I will clean it up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92249

___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D91241#2422403 , @omjavaid wrote:

> In D91241#2422100 , @labath wrote:
>
>> Looks great. One question about the member variable...
>
> m_remote_to_local_regnum_map is used by AddRegister where we add a 
> (key,value) pair for each of registers. We are doing this to maintain a list 
> of register sorted in increasing order of remote register numbers. This list 
> is then used in Finalize where we are calculating offsets. Keeping this map 
> as member saves us from generating a sorted list of remote register numbers.

Yes, but there's no reason that this _must_ be done in AddRegister, is there?

You could just build a local map directly inside the finalize function, right?


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] D91508: [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 308319.
tammela added a comment.

Addressing 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

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -13,6 +13,30 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
+  lldb::StackFrameSP stop_frame_sp,
+  lldb::BreakpointLocationSP bp_loc_sp) {
+  return false;
+}
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,18 @@
+# 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 'return false'
+run
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o 'print(bacon)'
+run
+# CHECK: bacon
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o "return true"
+run
+# CHECK: Process {{[0-9]+}} stopped
+breakpoint command add -s lua -o 'error("my error message")'
+run
+# CHECK: my error message
+# CHECK: Process {{[0-9]+}} stopped
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/ScriptInterpreterLua.cpp
@@ -8,14 +8,17 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/Timer.h"
 #include "llvm/Support/

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

[Pavel Labath via Phabricator via lldb-commits]

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

I think this is ok now. Thanks for your patience.


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] D92223: [lldb] Add support for looking up static const members

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D92223#2422184 , @labath wrote:

> I am not sure if this is the right way to implement this feature. Changing 
> ManualDWARFIndex to provide this additional information is easy enough, but 
> it means that the other index classes will never be able to support this 
> functionality

I wanted to post here rather my agreement with the patch. This looks to me as 
giving up on the high performance index benefits. (But maybe what you say is 
the right sweet spot compromise.)

I rather find missing a feature to cross-check ManualDWARFIndex vs. on-disk 
indexes (lld/LTO produced .debug_names, `gdb-add-index -dwarf-5` produced 
.debug_names and how Apple produces their index). Fedora is probably going to 
have two indexes (`.gdb_index` and `.debug_names`) for each LLVM-built binary 
as the format of GDB `.debug_names` is currently incompatible with LLVM/LLDB 
`.debug_names`. I also expect current `ManualDWARFIndex` is producing different 
index than lld.

> I am also worried about the increase to the manual index size, as this would 
> mean that every occurrence of the DW_TAG_member would be placed in the index, 
> whereas now we only place the one which has a location (which is normally 
> just in a single compile unit).

With `-flto` (even with `-O0`) there is only a single definition of each class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92223

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

[Lldb-commits] [lldb] b69c09b - Support custom expedited register set in gdb-remote

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-11-30T17:34:19+05:00
New Revision: b69c09bf43527e79a770efd9886b79e611f8fd59

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

LOG: Support custom expedited register set in gdb-remote

This patch adds capability to introduce a custom expedited register set
in gdb remote. Currently we send register set 0 as expedited register set
but for the case of AArch64 SVE we intend to send additional information
about SVE registers size/offset configuration which can be calculated
from vg register. Therefore we will expedited Vg register in case of
AArch64 is in SVE mode to speedup register configuration calculations.

Reviewed By: labath

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

Added: 


Modified: 
lldb/include/lldb/Host/common/NativeRegisterContext.h
lldb/source/Host/common/NativeRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/common/NativeRegisterContext.h 
b/lldb/include/lldb/Host/common/NativeRegisterContext.h
index 3480d5d59217..a8c74358c718 100644
--- a/lldb/include/lldb/Host/common/NativeRegisterContext.h
+++ b/lldb/include/lldb/Host/common/NativeRegisterContext.h
@@ -16,6 +16,8 @@ namespace lldb_private {
 
 class NativeThreadProtocol;
 
+enum class ExpeditedRegs { Minimal, Full };
+
 class NativeRegisterContext
 : public std::enable_shared_from_this {
 public:
@@ -116,6 +118,9 @@ class NativeRegisterContext
 
   virtual NativeThreadProtocol &GetThread() { return m_thread; }
 
+  virtual std::vector
+  GetExpeditedRegisters(ExpeditedRegs expType) const;
+
   const RegisterInfo *GetRegisterInfoByName(llvm::StringRef reg_name,
 uint32_t start_idx = 0);
 

diff  --git a/lldb/source/Host/common/NativeRegisterContext.cpp 
b/lldb/source/Host/common/NativeRegisterContext.cpp
index f6d16dcf2551..9bb877fff878 100644
--- a/lldb/source/Host/common/NativeRegisterContext.cpp
+++ b/lldb/source/Host/common/NativeRegisterContext.cpp
@@ -424,3 +424,32 @@ 
NativeRegisterContext::ConvertRegisterKindToRegisterNumber(uint32_t kind,
 
   return LLDB_INVALID_REGNUM;
 }
+
+std::vector
+NativeRegisterContext::GetExpeditedRegisters(ExpeditedRegs expType) const {
+  if (expType == ExpeditedRegs::Minimal) {
+// Expedite only a minimum set of important generic registers.
+static const uint32_t k_expedited_registers[] = {
+LLDB_REGNUM_GENERIC_PC, LLDB_REGNUM_GENERIC_SP, LLDB_REGNUM_GENERIC_FP,
+LLDB_REGNUM_GENERIC_RA};
+
+std::vector expedited_reg_nums;
+for (uint32_t gen_reg : k_expedited_registers) {
+  uint32_t reg_num =
+  ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric, gen_reg);
+  if (reg_num == LLDB_INVALID_REGNUM)
+continue; // Target does not support the given register.
+  else
+expedited_reg_nums.push_back(reg_num);
+}
+
+return expedited_reg_nums;
+  }
+
+  if (GetRegisterSetCount() > 0 && expType == ExpeditedRegs::Full)
+return std::vector(GetRegisterSet(0)->registers,
+ GetRegisterSet(0)->registers +
+ GetRegisterSet(0)->num_registers);
+
+  return std::vector();
+}

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 2cf88c0d9f70..694c88c49eba 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -503,7 +503,7 @@ static void WriteRegisterValueInHexFixedWidth(
   }
 }
 
-static llvm::Expected
+static llvm::Optional
 GetRegistersAsJSON(NativeThreadProtocol &thread) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
 
@@ -512,30 +512,16 @@ GetRegistersAsJSON(NativeThreadProtocol &thread) {
   json::Object register_object;
 
 #ifdef LLDB_JTHREADSINFO_FULL_REGISTER_SET
-  // Expedite all registers in the first register set (i.e. should be GPRs)
-  // that are not contained in other registers.
-  const RegisterSet *reg_set_p = reg_ctx_sp->GetRegisterSet(0);
-  if (!reg_set_p)
-return llvm::make_error("failed to get registers",
-   llvm::inconvertibleErrorCode());
-  for (const uint32_t *reg_num_p = reg_set_p->registers;
-   *reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
-uint32_t reg_num = *reg_num_p;
+  const auto expedited_regs =
+  reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Full);
 #else
-  // Expedite only a couple of registers until we figure out why sending
-  // registers is expensive.
-  static const uint32_t k_exped

[Lldb-commits] [lldb] 4e8aeb9 - Send SVE vg register in custom expedited registerset

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2020-11-30T17:34:19+05:00
New Revision: 4e8aeb97ca41eb202c9c90a9c640a630903c769b

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

LOG: Send SVE vg register in custom expedited registerset

This patch ovverides GetExpeditedRegisterSet for
NativeRegisterContextLinux_arm64 to send vector granule register in
expedited register set if SVE mode is selected.

Reviewed By: labath

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index ce700d9403f4..a1c420d1fa03 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -992,6 +992,13 @@ def find_generic_register_with_name(self, reg_infos, 
generic_name):
 return reg_info
 return None
 
+def find_register_with_name_and_dwarf_regnum(self, reg_infos, name, 
dwarf_num):
+self.assertIsNotNone(reg_infos)
+for reg_info in reg_infos:
+if (reg_info["name"] == name) and (reg_info["dwarf"] == dwarf_num):
+return reg_info
+return None
+
 def decode_gdbremote_binary(self, encoded_bytes):
 decoded_bytes = ""
 i = 0

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 1fa87e13a0aa..49badd8ef940 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -1125,4 +1125,14 @@ void *NativeRegisterContextLinux_arm64::GetSVEBuffer() {
   return m_sve_ptrace_payload.data();
 }
 
+std::vector NativeRegisterContextLinux_arm64::GetExpeditedRegisters(
+ExpeditedRegs expType) const {
+  std::vector expedited_reg_nums =
+  NativeRegisterContext::GetExpeditedRegisters(expType);
+  if (m_sve_state == SVEState::FPSIMD || m_sve_state == SVEState::Full)
+expedited_reg_nums.push_back(GetRegisterInfo().GetRegNumSVEVG());
+
+  return expedited_reg_nums;
+}
+
 #endif // defined (__arm64__) || defined (__aarch64__)

diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
index da45f1c2d2c3..3d0656dceb62 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -44,6 +44,9 @@ class NativeRegisterContextLinux_arm64 : public 
NativeRegisterContextLinux {
 
   void InvalidateAllRegisters() override;
 
+  std::vector
+  GetExpeditedRegisters(ExpeditedRegs expType) const override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
index 701c88c31258..515c9f44e1e2 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -342,3 +342,5 @@ uint32_t RegisterInfoPOSIX_arm64::GetRegNumSVEFFR() const { 
return sve_ffr; }
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPCR() const { return fpu_fpcr; }
 
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPSR() const { return fpu_fpsr; }
+
+uint32_t RegisterInfoPOSIX_arm64::GetRegNumSVEVG() const { return sve_vg; }

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
index d5eaf4cfbe9e..37f7c23b62c5 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -101,6 +101,7 @@ class RegisterInfoPOSIX_arm64
   uint32_t GetRegNumSVEFFR() const;
   uint32_t GetRegNumFPCR() const;
   uint32_t GetRegNumFPSR() const;
+  uint32_t GetRegNumSVEVG() const;
 
 private:
   typedef std::map>

diff  --git 
a/lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py 
b/lldb/test/AP

[Lldb-commits] [PATCH] D82853: [LLDB] Support custom expedited register set in gdb-remote

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb69c09bf4352: Support custom expedited register set in 
gdb-remote (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82853

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -503,7 +503,7 @@
   }
 }
 
-static llvm::Expected
+static llvm::Optional
 GetRegistersAsJSON(NativeThreadProtocol &thread) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
 
@@ -512,30 +512,16 @@
   json::Object register_object;
 
 #ifdef LLDB_JTHREADSINFO_FULL_REGISTER_SET
-  // Expedite all registers in the first register set (i.e. should be GPRs)
-  // that are not contained in other registers.
-  const RegisterSet *reg_set_p = reg_ctx_sp->GetRegisterSet(0);
-  if (!reg_set_p)
-return llvm::make_error("failed to get registers",
-   llvm::inconvertibleErrorCode());
-  for (const uint32_t *reg_num_p = reg_set_p->registers;
-   *reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
-uint32_t reg_num = *reg_num_p;
+  const auto expedited_regs =
+  reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Full);
 #else
-  // Expedite only a couple of registers until we figure out why sending
-  // registers is expensive.
-  static const uint32_t k_expedited_registers[] = {
-  LLDB_REGNUM_GENERIC_PC, LLDB_REGNUM_GENERIC_SP, LLDB_REGNUM_GENERIC_FP,
-  LLDB_REGNUM_GENERIC_RA, LLDB_INVALID_REGNUM};
-
-  for (const uint32_t *generic_reg_p = k_expedited_registers;
-   *generic_reg_p != LLDB_INVALID_REGNUM; ++generic_reg_p) {
-uint32_t reg_num = reg_ctx.ConvertRegisterKindToRegisterNumber(
-eRegisterKindGeneric, *generic_reg_p);
-if (reg_num == LLDB_INVALID_REGNUM)
-  continue; // Target does not support the given register.
+  const auto expedited_regs =
+  reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Minimal);
 #endif
+  if (expedited_regs.empty())
+return llvm::None;
 
+  for (auto ®_num : expedited_regs) {
 const RegisterInfo *const reg_info_p =
 reg_ctx.GetRegisterInfoAtIndex(reg_num);
 if (reg_info_p == nullptr) {
@@ -628,12 +614,8 @@
 json::Object thread_obj;
 
 if (!abridged) {
-  if (llvm::Expected registers =
-  GetRegistersAsJSON(*thread)) {
+  if (llvm::Optional registers = GetRegistersAsJSON(*thread))
 thread_obj.try_emplace("registers", std::move(*registers));
-  } else {
-return registers.takeError();
-  }
 }
 
 thread_obj.try_emplace("tid", static_cast(tid));
@@ -814,46 +796,27 @@
 
   // Grab the register context.
   NativeRegisterContext& reg_ctx = thread->GetRegisterContext();
-  // Expedite all registers in the first register set (i.e. should be GPRs)
-  // that are not contained in other registers.
-  const RegisterSet *reg_set_p;
-  if (reg_ctx.GetRegisterSetCount() > 0 &&
-  ((reg_set_p = reg_ctx.GetRegisterSet(0)) != nullptr)) {
-LLDB_LOGF(log,
-  "GDBRemoteCommunicationServerLLGS::%s expediting registers "
-  "from set '%s' (registers set count: %zu)",
-  __FUNCTION__, reg_set_p->name ? reg_set_p->name : "",
-  reg_set_p->num_registers);
+  const auto expedited_regs =
+  reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Full);
 
-for (const uint32_t *reg_num_p = reg_set_p->registers;
- *reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
-  const RegisterInfo *const reg_info_p =
-  reg_ctx.GetRegisterInfoAtIndex(*reg_num_p);
-  if (reg_info_p == nullptr) {
-LLDB_LOGF(log,
-  "GDBRemoteCommunicationServerLLGS::%s failed to get "
-  "register info for register set '%s', register index "
-  "%" PRIu32,
+  for (auto ®_num : expedited_regs) {
+const RegisterInfo *const reg_info_p =
+reg_ctx.GetRegisterInfoAtIndex(reg_num);
+// Only expediate registers that are not contained in other registers.
+if (reg_info_p != nullptr && reg_info_p->value_regs == nullptr) {
+  RegisterValue reg_value;
+  Status error = reg_ctx.ReadRegister(reg_info_p, reg_value);
+  if (error.Success()) {
+response.Printf("%.02x:", reg_num);
+WriteRegisterValueInHexFixedWidth(response, reg_ctx, *reg_info_p,
+  ®_value, lldb::eByteOrderBig);
+response.PutChar(';');
+  } else {
+

[Lldb-commits] [PATCH] D82855: [LLDB] Send SVE vg register in custom expedited registerset

[Muhammad Omair Javaid via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e8aeb97ca41: Send SVE vg register in custom expedited 
registerset (authored by omjavaid).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82855

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteExpeditedRegisters.py
@@ -8,8 +8,8 @@
 gdbremote_testcase.GdbRemoteTestCaseBase):
 
 mydir = TestBase.compute_mydir(__file__)
-@skipIfDarwinEmbedded #  lldb-server tests not updated to work on ios etc yet
-
+#  lldb-server tests not updated to work on ios etc yet
+@skipIfDarwinEmbedded
 def gather_expedited_registers(self):
 # Setup the stub and set the gdb remote command stream.
 procs = self.prep_debug_monitor_and_inferior(inferior_args=["sleep:2"])
@@ -58,6 +58,25 @@
 self.assertTrue(reg_info["lldb_register_index"] in expedited_registers)
 self.trace("{} reg_info:{}".format(generic_register_name, reg_info))
 
+def stop_notification_contains_aarch64_vg_register(self):
+# Generate a stop reply, parse out expedited registers from stop
+# notification.
+expedited_registers = self.gather_expedited_registers()
+self.assertIsNotNone(expedited_registers)
+self.assertTrue(len(expedited_registers) > 0)
+
+# Gather target register infos.
+reg_infos = self.gather_register_infos()
+
+# Find the vg register.
+reg_info = self.find_register_with_name_and_dwarf_regnum(
+reg_infos, 'vg', '46')
+self.assertIsNotNone(reg_info)
+
+# Ensure the expedited registers contained it.
+self.assertTrue(reg_info["lldb_register_index"] in expedited_registers)
+self.trace("{} reg_info:{}".format('vg', reg_info))
+
 def stop_notification_contains_any_registers(self):
 # Generate a stop reply, parse out expedited registers from stop
 # notification.
@@ -157,3 +176,14 @@
 self.build()
 self.set_inferior_startup_launch()
 self.stop_notification_contains_sp_register()
+
+@llgs_test
+@skipIf(archs=no_match(["aarch64"]))
+@skipIf(oslist=no_match(['linux']))
+def test_stop_notification_contains_vg_register_llgs(self):
+if not self.isAArch64SVE():
+self.skipTest('SVE registers must be supported.')
+self.init_llgs_test()
+self.build()
+self.set_inferior_startup_launch()
+self.stop_notification_contains_aarch64_vg_register()
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -101,6 +101,7 @@
   uint32_t GetRegNumSVEFFR() const;
   uint32_t GetRegNumFPCR() const;
   uint32_t GetRegNumFPSR() const;
+  uint32_t GetRegNumSVEVG() const;
 
 private:
   typedef std::map>
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
@@ -342,3 +342,5 @@
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPCR() const { return fpu_fpcr; }
 
 uint32_t RegisterInfoPOSIX_arm64::GetRegNumFPSR() const { return fpu_fpsr; }
+
+uint32_t RegisterInfoPOSIX_arm64::GetRegNumSVEVG() const { return sve_vg; }
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -44,6 +44,9 @@
 
   void InvalidateAllRegisters() override;
 
+  std::vector
+  GetExpeditedRegisters(ExpeditedRegs expType) const override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegi

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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D91241#2422517 , @labath wrote:

> In D91241#2422403 , @omjavaid wrote:
>
>> In D91241#2422100 , @labath wrote:
>>
>>> Looks great. One question about the member variable...
>>
>> m_remote_to_local_regnum_map is used by AddRegister where we add a 
>> (key,value) pair for each of registers. We are doing this to maintain a list 
>> of register sorted in increasing order of remote register numbers. This list 
>> is then used in Finalize where we are calculating offsets. Keeping this map 
>> as member saves us from generating a sorted list of remote register numbers.
>
> Yes, but there's no reason that this _must_ be done in AddRegister, is there?
>
> You could just build a local map directly inside the finalize function, right?

Yes this just avoid an extra iteration over m_regs list.


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] D92164: Make SBDebugger internal variable getter and setter not use CommandInterpreter's context

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 308325.
tatyana-krasnukha added a comment.

Removed refactoring to make the changes clearer.


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

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
@@ -1270,13 +1270,12 @@
   ConstString(debugger_instance_name)));
   Status error;
   if (debugger_sp) {
-ExecutionContext exe_ctx(
-debugger_sp->GetCommandInterpreter().GetExecutionContext());
+ExecutionContext exe_ctx(debugger_sp->GetSelectedTarget(), true);
 error = debugger_sp->SetPropertyValue(&exe_ctx, eVarSetOperationAssign,
   var_name, value);
   } else {
-error.SetErrorStringWithFormat("invalid debugger instance name '%s'",
-   debugger_instance_name);
+error.SetErrorStringWithFormatv("invalid debugger instance name {0}",
+debugger_instance_name);
   }
   if (error.Fail())
 sb_error.SetError(error);
@@ -1295,8 +1294,7 @@
   ConstString(debugger_instance_name)));
   Status error;
   if (debugger_sp) {
-ExecutionContext exe_ctx(
-debugger_sp->GetCommandInterpreter().GetExecutionContext());
+ExecutionContext exe_ctx(debugger_sp->GetSelectedTarget(), true);
 lldb::OptionValueSP value_sp(
 debugger_sp->GetPropertyValue(&exe_ctx, var_name, false, error));
 if (value_sp) {
@@ -1308,7 +1306,15 @@
 string_list.SplitIntoLines(value_str);
 return LLDB_RECORD_RESULT(SBStringList(&string_list));
   }
+} else {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+  LLDB_LOG_ERROR(log, error.ToError(), "cannot get property value: {0}");
 }
+  } else {
+Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+error.SetErrorStringWithFormatv("invalid debugger instance name {0}",
+debugger_instance_name);
+LLDB_LOG_E

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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 308338.
omjavaid added a comment.

Updated incorporating suggested changes.


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

https://reviews.llvm.org/D92063

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h


Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -29,6 +29,7 @@
   };
 
   // based on RegisterContextDarwin_arm64.h
+  LLVM_PACKED_START
   struct GPR {
 uint64_t x[29]; // x0-x28
 uint64_t fp;// x29
@@ -37,6 +38,7 @@
 uint64_t pc;// pc
 uint32_t cpsr;  // cpsr
   };
+  LLVM_PACKED_END
 
   // based on RegisterContextDarwin_arm64.h
   struct VReg {
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -95,6 +95,10 @@
 
   void *GetGPRBuffer() override { return &m_gpr_arm64; }
 
+  // GetGPRBufferSize returns sizeof arm64 GPR ptrace buffer, it is different
+  // from GetGPRSize which returns sizeof RegisterInfoPOSIX_arm64::GPR.
+  size_t GetGPRBufferSize() { return sizeof(m_gpr_arm64); }
+
   void *GetFPRBuffer() override { return &m_fpr; }
 
   size_t GetFPRSize() override { return sizeof(m_fpr); }
@@ -106,7 +110,7 @@
 
   bool m_sve_header_is_valid;
 
-  RegisterInfoPOSIX_arm64::GPR m_gpr_arm64; // 64-bit general purpose 
registers.
+  struct user_pt_regs m_gpr_arm64; // 64-bit general purpose registers.
 
   RegisterInfoPOSIX_arm64::FPU
   m_fpr; // floating-point registers including extended register sets.
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
@@ -936,9 +936,9 @@
 
   struct iovec ioVec;
   ioVec.iov_base = GetGPRBuffer();
-  ioVec.iov_len = GetGPRSize();
+  ioVec.iov_len = GetGPRBufferSize();
 
-  error = ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
+  error = ReadRegisterSet(&ioVec, GetGPRBufferSize(), NT_PRSTATUS);
 
   if (error.Success())
 m_gpr_is_valid = true;
@@ -953,11 +953,11 @@
 
   struct iovec ioVec;
   ioVec.iov_base = GetGPRBuffer();
-  ioVec.iov_len = GetGPRSize();
+  ioVec.iov_len = GetGPRBufferSize();
 
   m_gpr_is_valid = false;
 
-  return WriteRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
+  return WriteRegisterSet(&ioVec, GetGPRBufferSize(), NT_PRSTATUS);
 }
 
 Status NativeRegisterContextLinux_arm64::ReadFPR() {


Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -29,6 +29,7 @@
   };
 
   // based on RegisterContextDarwin_arm64.h
+  LLVM_PACKED_START
   struct GPR {
 uint64_t x[29]; // x0-x28
 uint64_t fp;// x29
@@ -37,6 +38,7 @@
 uint64_t pc;// pc
 uint32_t cpsr;  // cpsr
   };
+  LLVM_PACKED_END
 
   // based on RegisterContextDarwin_arm64.h
   struct VReg {
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -95,6 +95,10 @@
 
   void *GetGPRBuffer() override { return &m_gpr_arm64; }
 
+  // GetGPRBufferSize returns sizeof arm64 GPR ptrace buffer, it is different
+  // from GetGPRSize which returns sizeof RegisterInfoPOSIX_arm64::GPR.
+  size_t GetGPRBufferSize() { return sizeof(m_gpr_arm64); }
+
   void *GetFPRBuffer() override { return &m_fpr; }
 
   size_t GetFPRSize() override { return sizeof(m_fpr); }
@@ -106,7 +110,7 @@
 
   bool m_sve_header_is_valid;
 
-  RegisterInfoPOSIX_arm64::GPR m_gpr_arm64; // 64-bit general purpose registers.
+  struct user_pt_regs m_gpr_arm64; // 64-bit general purpose registers.
 
   RegisterInfoPOSIX_arm64::FPU
   m_fpr; // floating-point registers including extended register sets.
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
@@ -936,9 +936,9 @@

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

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 308339.
omjavaid added a comment.

Updated after incorporating suggested changes.


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/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
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+ 

[Lldb-commits] [PATCH] D82857: [LLDB] Add per-thread register infos shared pointer in gdb-remote

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 308340.
omjavaid added a comment.

Updated after re-base over D91241 .


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

https://reviews.llvm.org/D82857

Files:
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h

Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -14,6 +14,8 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/StructuredData.h"
 
+#include "GDBRemoteRegisterContext.h"
+
 class StringExtractor;
 
 namespace lldb_private {
@@ -101,6 +103,10 @@
   m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
 
+  GDBRemoteDynamicRegisterInfoSP m_reg_info_sp;
+
+  void SetThreadRegisterInfo();
+
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef data);
 
   bool PrivateSetRegisterValue(uint32_t reg, uint64_t regval);
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -42,6 +42,9 @@
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
GetID());
+  // At this point we can clone reg_info for architectures supporting
+  // run-time update to register sizes and offsets..
+  SetThreadRegisterInfo();
 }
 
 ThreadGDBRemote::~ThreadGDBRemote() {
@@ -307,8 +310,8 @@
   !pSupported || gdb_process->m_use_g_packet_for_reading;
   bool write_all_registers_at_once = !pSupported;
   reg_ctx_sp = std::make_shared(
-  *this, concrete_frame_idx, gdb_process->m_register_info,
-  read_all_registers_at_once, write_all_registers_at_once);
+  *this, concrete_frame_idx, m_reg_info_sp, read_all_registers_at_once,
+  write_all_registers_at_once);
 }
   } else {
 reg_ctx_sp = GetUnwinder().CreateRegisterContextForFrame(frame);
@@ -316,6 +319,23 @@
   return reg_ctx_sp;
 }
 
+void ThreadGDBRemote::SetThreadRegisterInfo() {
+  ProcessSP process_sp(GetProcess());
+  if (process_sp) {
+ProcessGDBRemote *gdb_process =
+static_cast(process_sp.get());
+
+if (!m_reg_info_sp) {
+  if (!gdb_process->m_register_info_sp->IsReconfigurable())
+m_reg_info_sp = gdb_process->m_register_info_sp;
+  else {
+m_reg_info_sp = std::make_shared();
+m_reg_info_sp->CloneFrom(gdb_process->m_register_info_sp);
+  }
+}
+  }
+}
+
 bool ThreadGDBRemote::PrivateSetRegisterValue(uint32_t reg,
   llvm::ArrayRef data) {
   GDBRemoteRegisterContext *gdb_reg_ctx =
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -254,7 +254,7 @@
  // the last stop
  // packet variable
   std::recursive_mutex m_last_stop_packet_mutex;
-  GDBRemoteDynamicRegisterInfo m_register_info;
+  GDBRemoteDynamicRegisterInfoSP m_register_info_sp;
   Broadcaster m_async_broadcaster;
   lldb::ListenerSP m_async_listener_sp;
   HostThread m_async_thread;
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
@@ -249,7 +249,7 @@
ListenerSP listener_sp)
 : Process(target_sp, listener_sp),
   m_debugserver_pid(LLDB_INVALID_PROCESS_ID), m_last_stop_packet_mutex(),
-  m_register_info(),
+  m_register_info_sp(nullptr),
   m_async_broadcaster(nullptr, "lldb.process.gdb-remote.async-broadcaster"),
   m_async_listener_sp(
   Listener::MakeListener("lldb.process.gdb-remote.async-listener")),
@@ -368,8 +368,8 @@
   m_breakpoint_pc_offset = breakpoint_pc_int_value->GetValue();
   }
 
-  if (m_register_info.SetReg

[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 308341.
omjavaid added a comment.

Updated after re-base over D91241 


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

https://reviews.llvm.org/D82863

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

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
@@ -1766,6 +1766,19 @@
 gdb_thread->PrivateSetRegisterValue(pair.first, buffer_sp->GetData());
   }
 
+  // AArch64 SVE specific code below calls AArch64SVEReconfigure to update
+  // SVE register sizes and offsets if value of VG register has changed
+  // since last stop.
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (arch.IsValid() && arch.GetTriple().isAArch64()) {
+GDBRemoteRegisterContext *reg_ctx_sp =
+static_cast(
+gdb_thread->GetRegisterContext().get());
+
+if (reg_ctx_sp)
+  reg_ctx_sp->AArch64SVEReconfigure();
+  }
+
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());
 
   gdb_thread->SetThreadDispatchQAddr(thread_dispatch_qaddr);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -40,6 +40,9 @@
 
   void HardcodeARMRegisters(bool from_scratch);
 
+  bool UpdateARM64SVERegistersInfos(uint64_t vg, uint32_t &end_reg_offset,
+std::vector &invalidate_regs);
+
   void CloneFrom(GDBRemoteDynamicRegisterInfoSP process_reginfo);
 };
 
@@ -79,6 +82,8 @@
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
uint32_t num) override;
 
+  bool AArch64SVEReconfigure();
+
 protected:
   friend class ThreadGDBRemote;
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -213,8 +213,8 @@
   for (int i = 0; i < regcount; i++) {
 struct RegisterInfo *reginfo =
 m_reg_info_sp->GetRegisterInfoAtIndex(i);
-if (reginfo->byte_offset + reginfo->byte_size 
-   <= buffer_sp->GetByteSize()) {
+if (reginfo->byte_offset + reginfo->byte_size <=
+buffer_sp->GetByteSize()) {
   m_reg_valid[i] = true;
 } else {
   m_reg_valid[i] = false;
@@ -343,6 +343,15 @@
   if (dst == nullptr)
 return false;
 
+  // Code below is specific to AArch64 target in SVE state
+  // If vector granule (vg) register is being written then thread's
+  // register context reconfiguration is triggered on success.
+  bool do_reconfigure_arm64_sve = false;
+  const ArchSpec &arch = process->GetTarget().GetArchitecture();
+  if (arch.IsValid() && arch.GetTriple().isAArch64())
+if (strcmp(reg_info->name, "vg") == 0)
+  do_reconfigure_arm64_sve = true;
+
   if (data.CopyByteOrderedData(data_offset,// src offset
reg_info->byte_size,// src length
dst,// dst
@@ -362,6 +371,11 @@
 
 {
   SetAllRegisterValid(false);
+
+  if (do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AArch64SVEReconfigure();
+
   return true;
 }
   } else {
@@ -390,6 +404,10 @@
 } else {
   // This is an actual register, write it
   success = SetPrimordialRegister(reg_info, gdb_comm);
+
+  if (success && do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AArch64SVEReconfigure();
 }
 
 // Check if writing this register will invalidate any other register
@@ -655,9 +673,8 @@
   if (m_thread.GetProcess().get()) {
 const ArchSpec &arch =
 m_thread.GetProcess()->GetTarget().GetArchitecture();
-if (arch.IsValid() && 
-(arch.GetMachine() == llvm::Triple::aarch64 ||
- arch.GetMachine() == llvm::Triple::aarch64_32) &&
+   

[Lldb-commits] [PATCH] D92314: [lldb] [Process/FreeBSDRemote] Implement GetLoadedModuleFileSpec() and GetFileLoadAddress()

[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.

Copy the Linux implementation of GetLoadedModuleFileSpec()
and GetFileLoadAddress() into NativeProcessFreeBSD.  This does not seem
to change anything at the moment but reducing the differences between
the plugins should help us in the long term.


https://reviews.llvm.org/D92314

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -630,14 +630,40 @@
 
 Status NativeProcessFreeBSD::GetLoadedModuleFileSpec(const char *module_path,
  FileSpec &file_spec) {
-  return Status("Unimplemented");
+  Status error = PopulateMemoryRegionCache();
+  if (error.Fail())
+return error;
+
+  FileSpec module_file_spec(module_path);
+  FileSystem::Instance().Resolve(module_file_spec);
+
+  file_spec.Clear();
+  for (const auto &it : m_mem_region_cache) {
+if (it.second.GetFilename() == module_file_spec.GetFilename()) {
+  file_spec = it.second;
+  return Status();
+}
+  }
+  return Status("Module file (%s) not found in process' memory map!",
+module_file_spec.GetFilename().AsCString());
 }
 
 Status
 NativeProcessFreeBSD::GetFileLoadAddress(const llvm::StringRef &file_name,
  lldb::addr_t &load_addr) {
   load_addr = LLDB_INVALID_ADDRESS;
-  return Status();
+  Status error = PopulateMemoryRegionCache();
+  if (error.Fail())
+return error;
+
+  FileSpec file(file_name);
+  for (const auto &it : m_mem_region_cache) {
+if (it.second == file) {
+  load_addr = it.first.GetRange().GetRangeBase();
+  return Status();
+}
+  }
+  return Status("No load address found for file %s.", file_name.str().c_str());
 }
 
 void NativeProcessFreeBSD::SigchldHandler() {


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -630,14 +630,40 @@
 
 Status NativeProcessFreeBSD::GetLoadedModuleFileSpec(const char *module_path,
  FileSpec &file_spec) {
-  return Status("Unimplemented");
+  Status error = PopulateMemoryRegionCache();
+  if (error.Fail())
+return error;
+
+  FileSpec module_file_spec(module_path);
+  FileSystem::Instance().Resolve(module_file_spec);
+
+  file_spec.Clear();
+  for (const auto &it : m_mem_region_cache) {
+if (it.second.GetFilename() == module_file_spec.GetFilename()) {
+  file_spec = it.second;
+  return Status();
+}
+  }
+  return Status("Module file (%s) not found in process' memory map!",
+module_file_spec.GetFilename().AsCString());
 }
 
 Status
 NativeProcessFreeBSD::GetFileLoadAddress(const llvm::StringRef &file_name,
  lldb::addr_t &load_addr) {
   load_addr = LLDB_INVALID_ADDRESS;
-  return Status();
+  Status error = PopulateMemoryRegionCache();
+  if (error.Fail())
+return error;
+
+  FileSpec file(file_name);
+  for (const auto &it : m_mem_region_cache) {
+if (it.second == file) {
+  load_addr = it.first.GetRange().GetRangeBase();
+  return Status();
+}
+  }
+  return Status("No load address found for file %s.", file_name.str().c_str());
 }
 
 void NativeProcessFreeBSD::SigchldHandler() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82866: [LLDB] Test SVE dynamic resize with multiple threads

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 308343.
omjavaid added a comment.

Updated after rebase over D91241 


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

https://reviews.llvm.org/D82866

Files:
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
@@ -0,0 +1,102 @@
+#include 
+#include 
+
+static inline void write_sve_registers() {
+  asm volatile("setffr\n\t");
+  asm volatile("ptrue p0.b\n\t");
+  asm volatile("ptrue p1.h\n\t");
+  asm volatile("ptrue p2.s\n\t");
+  asm volatile("ptrue p3.d\n\t");
+  asm volatile("pfalse p4.b\n\t");
+  asm volatile("ptrue p5.b\n\t");
+  asm volatile("ptrue p6.h\n\t");
+  asm volatile("ptrue p7.s\n\t");
+  asm volatile("ptrue p8.d\n\t");
+  asm volatile("pfalse p9.b\n\t");
+  asm volatile("ptrue p10.b\n\t");
+  asm volatile("ptrue p11.h\n\t");
+  asm volatile("ptrue p12.s\n\t");
+  asm volatile("ptrue p13.d\n\t");
+  asm volatile("pfalse p14.b\n\t");
+  asm volatile("ptrue p15.b\n\t");
+
+  asm volatile("cpy  z0.b, p0/z, #1\n\t");
+  asm volatile("cpy  z1.b, p5/z, #2\n\t");
+  asm volatile("cpy  z2.b, p10/z, #3\n\t");
+  asm volatile("cpy  z3.b, p15/z, #4\n\t");
+  asm volatile("cpy  z4.b, p0/z, #5\n\t");
+  asm volatile("cpy  z5.b, p5/z, #6\n\t");
+  asm volatile("cpy  z6.b, p10/z, #7\n\t");
+  asm volatile("cpy  z7.b, p15/z, #8\n\t");
+  asm volatile("cpy  z8.b, p0/z, #9\n\t");
+  asm volatile("cpy  z9.b, p5/z, #10\n\t");
+  asm volatile("cpy  z10.b, p10/z, #11\n\t");
+  asm volatile("cpy  z11.b, p15/z, #12\n\t");
+  asm volatile("cpy  z12.b, p0/z, #13\n\t");
+  asm volatile("cpy  z13.b, p5/z, #14\n\t");
+  asm volatile("cpy  z14.b, p10/z, #15\n\t");
+  asm volatile("cpy  z15.b, p15/z, #16\n\t");
+  asm volatile("cpy  z16.b, p0/z, #17\n\t");
+  asm volatile("cpy  z17.b, p5/z, #18\n\t");
+  asm volatile("cpy  z18.b, p10/z, #19\n\t");
+  asm volatile("cpy  z19.b, p15/z, #20\n\t");
+  asm volatile("cpy  z20.b, p0/z, #21\n\t");
+  asm volatile("cpy  z21.b, p5/z, #22\n\t");
+  asm volatile("cpy  z22.b, p10/z, #23\n\t");
+  asm volatile("cpy  z23.b, p15/z, #24\n\t");
+  asm volatile("cpy  z24.b, p0/z, #25\n\t");
+  asm volatile("cpy  z25.b, p5/z, #26\n\t");
+  asm volatile("cpy  z26.b, p10/z, #27\n\t");
+  asm volatile("cpy  z27.b, p15/z, #28\n\t");
+  asm volatile("cpy  z28.b, p0/z, #29\n\t");
+  asm volatile("cpy  z29.b, p5/z, #30\n\t");
+  asm volatile("cpy  z30.b, p10/z, #31\n\t");
+  asm volatile("cpy  z31.b, p15/z, #32\n\t");
+}
+
+void *thread_func(void *x_void_ptr) {
+  if (*((int *)x_void_ptr) == 0) {
+prctl(PR_SVE_SET_VL, 8 * 4);
+write_sve_registers();
+write_sve_registers(); // Thread X breakpoint 1
+return NULL;   // Thread X breakpoint 2
+  }
+
+  if (*((int *)x_void_ptr) == 1) {
+prctl(PR_SVE_SET_VL, 8 * 2);
+write_sve_registers();
+write_sve_registers(); // Thread Y breakpoint 1
+return NULL;   // Thread Y breakpoint 2
+  }
+
+  return NULL;
+}
+
+int main() {
+  /* this variable is our reference to the second thread */
+  pthread_t x_thread, y_thread;
+
+  int x = 0, y = 1;
+
+  /* Set vector length to 8 and write SVE registers values */
+  prctl(PR_SVE_SET_VL, 8 * 8);
+  write_sve_registers();
+
+  /* create a second thread which executes with argument x */
+  if (pthread_create(&x_thread, NULL, thread_func, &x)) // Break in main thread
+return 1;
+
+  /* create a second thread which executes with argument y */
+  if (pthread_create(&y_thread, NULL, thread_func, &y))
+return 1;
+
+  /* wait for the first thread to finish */
+  if (pthread_join(x_thread, NULL))
+return 2;
+
+  /* wait for the second thread to finish */
+  if (pthread_join(y_thread, NULL))
+return 2;
+
+  return 0;
+}
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
@@ -0,0 +1,138 @@
+"""
+Test the AArch64 SVE registers dynamic resize with multiple threads.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_registers(self, vg_test_value):
+z_reg_size = vg_test_value * 8
+p_reg_size = int(z_reg_size / 8)
+
+  

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

[Pavel Labath via Phabricator via lldb-commits]

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

In D91241#2422571 , @omjavaid wrote:

> In D91241#2422517 , @labath wrote:
>
>> Yes, but there's no reason that this _must_ be done in AddRegister, is there?
>>
>> You could just build a local map directly inside the finalize function, 
>> right?
>
> Yes this just avoid an extra iteration over m_regs list.

This is much better. The iteration doesn't cost much (if anything), but the 
member variable increases size of the object permentently.


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] [lldb] a0d7406 - [LLDB/Lua] add support for one-liner breakpoint callback

[Pedro Tammela via lldb-commits]

Author: Pedro Tammela
Date: 2020-11-30T14:12:26Z
New Revision: a0d7406ae800c45dd9cb438c7ae1f55329d198e2

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

LOG: [LLDB/Lua] add support for one-liner breakpoint callback

These callbacks are set using the following:
   breakpoint command add -s lua -o "print('hello world!')"

The user supplied script is executed as:
   function (frame, bp_loc, ...)
  
   end

So the local variables 'frame', 'bp_loc' and vararg are all accessible.
Any global variables declared will persist in the Lua interpreter.
A user should never hold 'frame' and 'bp_loc' in a global variable as
these userdatas are context dependent.

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

Added: 
lldb/bindings/lua/lua-swigsafecast.swig
lldb/bindings/lua/lua-wrapper.swig
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test

Modified: 
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/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Removed: 




diff  --git a/lldb/bindings/lua/lua-swigsafecast.swig 
b/lldb/bindings/lua/lua-swigsafecast.swig
new file mode 100644
index ..d19308976899
--- /dev/null
+++ b/lldb/bindings/lua/lua-swigsafecast.swig
@@ -0,0 +1,15 @@
+template 
+void
+PushSBClass (lua_State* L, SBClass* obj);
+
+void
+PushSBClass (lua_State* L, lldb::SBFrame* frame_sb)
+{
+   SWIG_NewPointerObj(L, frame_sb, SWIGTYPE_p_lldb__SBFrame, 0);
+}
+
+void
+PushSBClass (lua_State* L, lldb::SBBreakpointLocation* breakpoint_location_sb)
+{
+   SWIG_NewPointerObj(L, breakpoint_location_sb, 
SWIGTYPE_p_lldb__SBBreakpointLocation, 0);
+}

diff  --git a/lldb/bindings/lua/lua-wrapper.swig 
b/lldb/bindings/lua/lua-wrapper.swig
new file mode 100644
index ..4e3df74999dc
--- /dev/null
+++ b/lldb/bindings/lua/lua-wrapper.swig
@@ -0,0 +1,46 @@
+%header %{
+
+template 
+void
+PushSBClass(lua_State* L, T* obj);
+
+%}
+
+%wrapper %{
+
+// This function is called from Lua::CallBreakpointCallback
+SWIGEXPORT llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction
+(
+   lua_State *L,
+   lldb::StackFrameSP stop_frame_sp,
+   lldb::BreakpointLocationSP bp_loc_sp
+)
+{
+   lldb::SBFrame sb_frame(stop_frame_sp);
+   lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
+
+   // Push the Lua wrappers
+   PushSBClass(L, &sb_frame);
+   PushSBClass(L, &sb_bp_loc);
+
+   // Call into the Lua callback passing 'sb_frame' and 'sb_bp_loc'.
+   // Expects a boolean return.
+   if (lua_pcall(L, 2, 1, 0) != LUA_OK) {
+  llvm::Error E = llvm::make_error(
+llvm::formatv("{0}\n", lua_tostring(L, -1)),
+llvm::inconvertibleErrorCode());
+  // Pop error message from the stack.
+  lua_pop(L, 1);
+  return std::move(E);
+   }
+
+   // Boolean return from the callback
+   bool stop = lua_toboolean(L, -1);
+   lua_pop(L, 1);
+
+   return stop;
+}
+
+
+%}

diff  --git a/lldb/bindings/lua/lua.swig b/lldb/bindings/lua/lua.swig
index 524d1b5a8036..c702e4964081 100644
--- a/lldb/bindings/lua/lua.swig
+++ b/lldb/bindings/lua/lua.swig
@@ -14,8 +14,12 @@
 %include "headers.swig"
 
 %{
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "../bindings/lua/lua-swigsafecast.swig"
 using namespace lldb_private;
 using namespace lldb;
 %}
 
 %include "interfaces.swig"
+%include "lua-wrapper.swig"

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index dc3fd84a3bfb..ca1d181a6940 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -9,11 +9,34 @@
 #include "Lua.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/FileSpec.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 
 using namespace lldb_private;
 using namespace lldb;
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
+  lldb::StackFrameSP stop_frame_sp,
+  lldb::BreakpointLocationSP bp_loc_sp);
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 static int lldb_print(lua_State *L) {
   in

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

[Pedro Tammela via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0d7406ae800: [LLDB/Lua] add support for one-liner 
breakpoint callback (authored by tammela).

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

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -13,6 +13,30 @@
 
 extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wreturn-type-c-linkage"
+
+// Disable warning C4190: 'LLDBSwigPythonBreakpointCallbackFunction' has
+// C-linkage specified, but returns UDT 'llvm::Expected' which is
+// incompatible with C
+#if _MSC_VER
+#pragma warning (push)
+#pragma warning (disable : 4190)
+#endif
+
+extern "C" llvm::Expected
+LLDBSwigLuaBreakpointCallbackFunction(lua_State *L,
+  lldb::StackFrameSP stop_frame_sp,
+  lldb::BreakpointLocationSP bp_loc_sp) {
+  return false;
+}
+
+#if _MSC_VER
+#pragma warning (pop)
+#endif
+
+#pragma clang diagnostic pop
+
 TEST(LuaTest, RunValid) {
   Lua lua;
   llvm::Error error = lua.Run("foo = 1");
Index: lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test
@@ -0,0 +1,18 @@
+# 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 'return false'
+run
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o 'print(bacon)'
+run
+# CHECK: bacon
+# CHECK: Process {{[0-9]+}} exited with status = 0
+breakpoint command add -s lua -o "return true"
+run
+# CHECK: Process {{[0-9]+}} stopped
+breakpoint command add -s lua -o 'error("my error message")'
+run
+# CHECK: my error message
+# CHECK: Process {{[0-9]+}} stopped
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/ScriptInterpreterLua.cpp
@@ -8,14 +8,17 @@
 
 #include "ScriptInterpreterLua.h"
 #include "Lua.h"
+#include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Targe

[Lldb-commits] [PATCH] D92223: [lldb] Add support for looking up static const members

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

FYI it would not work with GNU C++ 4.4 (2008, RHEL-5) as it used 
`DW_TAG_variable` instead of `DW_TAG_member`. But that is probably not a 
problem, Red Hat's last supported RHEL for new toolchains is since RHEL-6 and 
for LLVM even just since RHEL-7:

  <0>: Abbrev Number: 1 (DW_TAG_compile_unit)
  DW_AT_producer: GNU C++ 4.1.2 20080704 (Red Hat 4.1.2-55)
  DW_AT_language: 4  (C++)
  ...
  <1><70>: Abbrev Number: 2 (DW_TAG_class_type)
  DW_AT_name: C
  ...
  <2><7a>: Abbrev Number: 3 (DW_TAG_variable)
  DW_AT_name: i
  DW_AT_decl_file   : 1
  DW_AT_decl_line   : 2
  DW_AT_MIPS_linkage_name: _ZN1C1iE
  DW_AT_type: <90>
  DW_AT_external: 1
  DW_AT_accessibility: 3 (private)
  DW_AT_declaration : 1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92223

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

[Lldb-commits] [PATCH] D92314: [lldb] [Process/FreeBSDRemote] Implement GetLoadedModuleFileSpec() and GetFileLoadAddress()

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

No objection, but maybe add a comment explaining the status of this 
implementation? Does/will NetBSD do the same?


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

https://reviews.llvm.org/D92314

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

[Lldb-commits] [lldb] e0e7bbe - [lldb] Always include template arguments that have their default value in the internal type name

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-30T16:40:50+01:00
New Revision: e0e7bbeb545516c50a0354efc34d329453558c9c

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

LOG: [lldb] Always include template arguments that have their default value in 
the internal type name

Our type formatters/summaries match on the internal type name we generate in 
LLDB for Clang types.

These names were generated using Clang's default printing policy. However 
Clang's
default printing policy got tweaked over the last month to make the generated 
type
names more readable (by for example excluding inline/anonymous namespaces and
removing template arguments that have their default value). This broke the 
formatter
system where LLDB's matching logic now no longer can format certain types as
the new type names generated by Clang's default printing policy no longer match
the type names that LLDB/the user specified.

I already introduced LLDB's own type printing policy and fixed the 
inline/anonymous
namespaces in da121fff1184267a405f81a87f7314df2d474e1c (just to get the
test suite passing again).

This patch is restoring the old type printing behaviour where always include 
the template
arguments in the internal type name (even if they match the default args). This 
should get
template type formatters/summaries working again in the rare situation where we 
do
know template default arguments within LLDB. This can only happen when either 
having
a template that was parsed in the expression parser or when we get type 
information from a C++ module.

The Clang change that removed defaulted template arguments from Clang's 
printing policy was
e7f3e2103cdb567dda4fd52f81bf4bc07179f5a8

Reviewed By: labath

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

Added: 
lldb/test/API/lang/cpp/default-template-args/Makefile
lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
lldb/test/API/lang/cpp/default-template-args/main.cpp

Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 77470486dd45..bab50a3b068c 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1998,6 +1998,18 @@ PrintingPolicy TypeSystemClang::GetTypePrintingPolicy() {
   // and libstdc++ are 
diff erentiated by their inline namespaces).
   printing_policy.SuppressInlineNamespace = false;
   printing_policy.SuppressUnwrittenScope = false;
+  // Default arguments are also always important for type formatters. Otherwise
+  // we would need to always specify two type names for the setups where we do
+  // know the default arguments and where we don't know default arguments.
+  //
+  // For example, without this we would need to have formatters for both:
+  //   std::basic_string
+  // and
+  //   std::basic_string, std::allocator >
+  // to support setups where LLDB was able to reconstruct default arguments
+  // (and we then would have suppressed them from the type name) and also 
setups
+  // where LLDB wasn't able to reconstruct the default arguments.
+  printing_policy.SuppressDefaultTemplateArgs = false;
   return printing_policy;
 }
 

diff  --git a/lldb/test/API/lang/cpp/default-template-args/Makefile 
b/lldb/test/API/lang/cpp/default-template-args/Makefile
new file mode 100644
index ..8b20bcb0
--- /dev/null
+++ b/lldb/test/API/lang/cpp/default-template-args/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py 
b/lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
new file mode 100644
index ..970c3522a175
--- /dev/null
+++ b/lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
@@ -0,0 +1,41 @@
+"""
+Test default template arguments.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+# Declare a template with a template argument that has a default 
argument.
+self.expect("expr --top-level -- template struct $X 
{ int v; };")
+
+# The type we display to the user should omit the argument with the 
default
+# value.
+result = self.expect_expr("$X<> x; x",  result_type="$X<>")
+# The internal name should also always show all argume

[Lldb-commits] [PATCH] D92311: [lldb] Always include template arguments that have their default value in the internal type name

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe0e7bbeb5455: [lldb] Always include template arguments that 
have their default value in the… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92311

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/default-template-args/Makefile
  lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
  lldb/test/API/lang/cpp/default-template-args/main.cpp


Index: lldb/test/API/lang/cpp/default-template-args/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/default-template-args/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
@@ -0,0 +1,41 @@
+"""
+Test default template arguments.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@no_debug_info_test
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.cpp"))
+
+# Declare a template with a template argument that has a default 
argument.
+self.expect("expr --top-level -- template struct $X 
{ int v; };")
+
+# The type we display to the user should omit the argument with the 
default
+# value.
+result = self.expect_expr("$X<> x; x",  result_type="$X<>")
+# The internal name should also always show all arguments (even if they
+# have their default value).
+self.assertEqual(result.GetTypeName(), "$X")
+
+# Test the template but this time specify a non-default value for the
+# template argument.
+# Both internal type name and the one we display to the user should
+# show the non-default value in the type name.
+result = self.expect_expr("$X x; x", result_type="$X")
+self.assertEqual(result.GetTypeName(), "$X")
+
+# Test that the formatters are using the internal type names that
+# always include all template arguments.
+self.expect("type summary add '$X' --summary-string 'summary1'")
+self.expect_expr("$X<> x; x", result_summary="summary1")
+self.expect("type summary add '$X' --summary-string 'summary2'")
+self.expect_expr("$X x; x", result_summary="summary2")
Index: lldb/test/API/lang/cpp/default-template-args/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/default-template-args/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1998,6 +1998,18 @@
   // and libstdc++ are differentiated by their inline namespaces).
   printing_policy.SuppressInlineNamespace = false;
   printing_policy.SuppressUnwrittenScope = false;
+  // Default arguments are also always important for type formatters. Otherwise
+  // we would need to always specify two type names for the setups where we do
+  // know the default arguments and where we don't know default arguments.
+  //
+  // For example, without this we would need to have formatters for both:
+  //   std::basic_string
+  // and
+  //   std::basic_string, std::allocator >
+  // to support setups where LLDB was able to reconstruct default arguments
+  // (and we then would have suppressed them from the type name) and also 
setups
+  // where LLDB wasn't able to reconstruct the default arguments.
+  printing_policy.SuppressDefaultTemplateArgs = false;
   return printing_policy;
 }
 


Index: lldb/test/API/lang/cpp/default-template-args/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/default-template-args/main.cpp
@@ -0,0 +1,3 @@
+int main() {
+  return 0; // break here
+}
Index: lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
@@ -0,0 +1,41 @@
+"""
+Test default template arguments.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test impor

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

[Ed Maste via Phabricator via lldb-commits]

emaste added a subscriber: bsdjhb.
emaste added a comment.

I'm curious how gdb handles this, and asked @bsdjhb if he knows off hand.

Is there a brief description of how this works on Linux and/or NetBSD?


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

https://reviews.llvm.org/D92187

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

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

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

In D92187#2423018 , @emaste wrote:

> I'm curious how gdb handles this, and asked @bsdjhb if he knows off hand.
>
> Is there a brief description of how this works on Linux and/or NetBSD?

We are working on researching this, NetBSD, Linux and FreeBSD should behave 
almost in the same way. At least GNU gdbserver reused the Linux code on NetBSD 
practically as-is.

One thing that FreeBSD should do, is to upgrade to the protocol version 1 
(stored in r_version), like Linux, NetBSD and OpenBSD.


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

https://reviews.llvm.org/D92187

___
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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/python_api/debugger/main.cpp:1
+//===-- main.cpp *- C++ 
-*-===//
+//

Test source files shouldn't have the header.



Comment at: lldb/source/API/SBDebugger.cpp:1315
+Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+error.SetErrorStringWithFormatv("invalid debugger instance name {0}",
+debugger_instance_name);

Does this append or override the error message? If it overrides you might as 
well us `LLDB_LOG` directly which supports formatv. 


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

https://reviews.llvm.org/D92164

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

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

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

Link to Linux info:
https://sourceware.org/gdb/wiki/LinkerInterface


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

https://reviews.llvm.org/D92187

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

[Lldb-commits] [PATCH] D92223: [lldb] Add support for looking up static const members

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3181
 
+  if (tag == DW_TAG_member && !const_value_form.IsValid()) {
+// Allows only members with DW_AT_const_value attribute, i.e. static const

This is a nitpick but if we move the comment above the `if` we can remove the 
braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92223

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

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

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

> One thing that FreeBSD should do, is to upgrade to the protocol version 1 
> (stored in r_version), like Linux, NetBSD and OpenBSD.

It looks like Linux has always used r_version=1 (since 1995).

AFAICT we can just add r_ldbase and set version to 1.


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

https://reviews.llvm.org/D92187

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

[Lldb-commits] [lldb] 1b9f214 - [lldb] Give TestDefaultTemplateArgs a unique class name

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-11-30T14:41:35-08:00
New Revision: 1b9f214efca7d5855f4e3dd1969c4cbe77078f97

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

LOG: [lldb] Give TestDefaultTemplateArgs a unique class name

Multiple tests cannot share the same test class name.

Added: 


Modified: 
lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py

Removed: 




diff  --git 
a/lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py 
b/lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
index 970c3522a175..e009b23d5b63 100644
--- a/lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
+++ b/lldb/test/API/lang/cpp/default-template-args/TestDefaultTemplateArgs.py
@@ -7,7 +7,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestCase(TestBase):
+class TestDefaultTemplateArgs(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 



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

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

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

> One thing that FreeBSD should do, is to upgrade to the protocol version 1 
> (stored in r_version), like Linux, NetBSD and OpenBSD.

It looks like Linux has always used r_version=1 (since 1995).

AFAICT we can just add r_ldbase and set version to 1.
See https://reviews.freebsd.org/D27429


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

https://reviews.llvm.org/D92187

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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py:25
 
-deque_type = "std::deque >"
 size_type = deque_type + "::size_type"

Why do the default arguments not show up in the results anymore?


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] D92164: Make SBDebugger internal variable getter and setter not use CommandInterpreter's context

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

I don't see any reason for, and lots of reasons against having more than one 
source of truth for a Debugger's "Currently Selected ExecutionContext".  In 
particular, I can't see any good uses of the Debugger and the 
CommandInterpreter being able to have different "currently selected 
targets/threads/frames".  For instance, I think people would generally be 
surprised if calling SBDebugger.SetSelectedTarget didn't also change the 
default target that subsequent command interpreter commands use.

This patch solves the problem that the CommandInterpreter's version of this 
truth is out of sync with the Debugger's notion by ignoring the 
CommandInterpreter's version.  That seems to me the wrong way to solve the 
problem.

Rather, we should make sure that everyone is looking to the same source.  I'm 
not sure it makes sense for the CommandInterpreter to be the one holding the 
"Currently Selected Execution Context".  Maybe that should be kept in the 
Debugger, and the Command Interpreter could then call 
Debugger::GetSelectedExecutionContext?


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

https://reviews.llvm.org/D92164

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

[Lldb-commits] [lldb] 173bb3c - [lldb] Refactor GetDeviceSupportDirectoryNames and GetPlatformName (NFC)

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-11-30T19:37:12-08:00
New Revision: 173bb3c2eb094920708ab8f61dae2fe22d331773

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

LOG: [lldb] Refactor GetDeviceSupportDirectoryNames and GetPlatformName (NFC)

Both functions are effectively returning a single string literal. Change
the interface to return a llvm::StringRef instead of populating a vector
of std::strings or returning a std::string respectively.

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteiOS.h

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
index 1cb8b9c37031..f3ee92a9d27b 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.cpp
@@ -172,15 +172,10 @@ bool 
PlatformRemoteAppleBridge::GetSupportedArchitectureAtIndex(uint32_t idx,
   return false;
 }
 
-
-void PlatformRemoteAppleBridge::GetDeviceSupportDirectoryNames 
(std::vector &dirnames) 
-{
-dirnames.clear();
-dirnames.push_back("BridgeOS DeviceSupport");
+llvm::StringRef PlatformRemoteAppleBridge::GetDeviceSupportDirectoryName() {
+  return "BridgeOS DeviceSupport";
 }
 
-std::string PlatformRemoteAppleBridge::GetPlatformName ()
-{
-return "BridgeOS.platform";
+llvm::StringRef PlatformRemoteAppleBridge::GetPlatformName() {
+  return "BridgeOS.platform";
 }
-

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h
index 5e7420e2508b..2d574894a283 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleBridge.h
@@ -48,12 +48,8 @@ class PlatformRemoteAppleBridge : public 
PlatformRemoteDarwinDevice {
lldb_private::ArchSpec &arch) override;
 
 protected:
-
-  // lldb_private::PlatformRemoteDarwinDevice functions
-
-  void GetDeviceSupportDirectoryNames (std::vector &dirnames) 
override;
-
-  std::string GetPlatformName () override;
+  llvm::StringRef GetDeviceSupportDirectoryName() override;
+  llvm::StringRef GetPlatformName() override;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLEBRIDGE_H

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
index 082ddcc0f568..15e91b239a35 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
@@ -223,15 +223,10 @@ bool 
PlatformRemoteAppleTV::GetSupportedArchitectureAtIndex(uint32_t idx,
   return false;
 }
 
-
-void PlatformRemoteAppleTV::GetDeviceSupportDirectoryNames 
(std::vector &dirnames) 
-{
-dirnames.clear();
-dirnames.push_back("tvOS DeviceSupport");
+llvm::StringRef PlatformRemoteAppleTV::GetDeviceSupportDirectoryName() {
+  return "tvOS DeviceSupport";
 }
 
-std::string PlatformRemoteAppleTV::GetPlatformName ()
-{
-return "AppleTVOS.platform";
+llvm::StringRef PlatformRemoteAppleTV::GetPlatformName() {
+  return "AppleTVOS.platform";
 }
-

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h
index c556476340fc..15be923cca46 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.h
@@ -48,12 +48,8 @@ class PlatformRemoteAppleTV : public 
PlatformRemoteDarwinDevice {
lldb_private::ArchSpec &arch) override;
 
 protected:
-
-  // lldb_private::PlatformRemoteDarwinDevice functions
-
-  void GetDeviceSupportDirectoryNames (std::vector &dirnames) 
override;
-
-  std::string GetPlatformName () override;
+  llvm::StringRef GetDeviceSupportDirectoryName() override;
+  llvm::StringRef GetPlatformName() override;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMREMOTEAPPLETV_H

diff  --git a/lldb/source/Plugins/Platform/MacOS