date:20211001

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

2021-10-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Some general thoughts:

- instead of throwing patches around in might be good to have a discussion 
thread to determine all the requirements and figure out the general direction
- it would be better to have a series of smaller patches (like this one) 
instead one big patch implementing everything




Comment at: lldb/include/lldb/Utility/Analytics.h:43
+private:
+  std::vector m_counters;
+  bool m_collecting_analytics = false;

This assumes all statistics can be represented as integers. Here's an idea how 
that could be done with a flexible type representation 
.



Comment at: lldb/source/Utility/Analytics.cpp:40-41
+#include "lldb/Utility/Metrics.def"
+  case Metric::MaxID:
+llvm_unreachable("invalid metric");
+  }

You could get rid of these if you made MaxID a synonym for the highest valued 
entry.


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

https://reviews.llvm.org/D110893

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

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty [WIP]

2021-10-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D110878#3034748 , @mgorny wrote:

> @labath, this is the part where I ask for help ;-).
>
>   An exception happened when receiving the response from the gdb server. 
> Closing the client...
>   Traceback (most recent call last):
> File 
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
>  line 405, in _run
>   data = seven.bitcast_to_string(self._client.recv(4096))
> File 
> "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py",
>  line 538, in recv
>   return self.read(size)
>   OSError: [Errno 5] Input/output error
>   PASS: LLDB (/home/mgorny/git/llvm-project/build/bin/clang-x86_64) :: 
> test_process_connect_sync_dwarf (TestPty.TestPty)
>
> I think it's getting in the 'controlling terminal' magic and getting EIO when 
> lldb finishes or sth like that. Any suggestion how to avoid this exception?

Catch it and turn it into EOF? (btw, I've found this link 

 to be pretty good at explaining the behavior across different OSs.)




Comment at: 
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py:530-535
+def accept(self):
+return self, ''
+
+def close(self):
+# the socket is reused, so don't close it yet
+pass

I think it would be cleaner if we had like a single new interface, which 
describes what to we expect of the connection, and then a socket- and pty-based 
implementation of that interface.



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py:560-564
+def get_connect_address(self):
+libc = ctypes.CDLL(None)
+libc.ptsname.argtypes = (ctypes.c_int,)
+libc.ptsname.restype = ctypes.c_char_p
+return libc.ptsname(self._master).decode()

I'm wondering if we should have an overload of SBTarget::ConnectRemote that 
accepts a file descriptor (or SBFile, SBCommunication, ...).

This would make the socket case easier as well, and it seems like it could be 
generally useful.


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

https://reviews.llvm.org/D110878

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

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

2021-10-01 Thread Jaroslav Sevcik via Phabricator via lldb-commits

jarin added a comment.

In D110571#3033481 , @labath wrote:

> In D110571#3033282 , @jarin wrote:
>
>> In D110571#3033078 , @labath wrote:
>>
>>> Here's one more question. AIUI, lldb relies on the order of formal 
>>> parameter declarations in dwarf to establish the the function signature 
>>> (dwarf doesn't leave us much choice. This then affects how the function is 
>>> printed in the backtrace, for instance. What will be the resulting order of 
>>> arguments for these functions? I'm wondering if we don't need a two-pass 
>>> algorithm, which first parses the arguments in the function declaration (to 
>>> establish their order), and then do another pass over the concrete instance 
>>> to fill in the missing information. (I'm sorry if you're doing this 
>>> already, but I'm still too scared of the code to figure it out myself :P ).
>>
>> The code already does the merging. For DW_TAG_inlined_subroutine, it first 
>> collects the formal_parameter list from from its abstract_origin and then it 
>> will parse/insert all the missing one while parsing the concrete instance. 
>> This will preserve the order of formal parameters. Now that I think about 
>> this, it might add some formal parameters after local variables, but I hope 
>> this is not a real problem for LLDB. If this is a problem, we could perhaps 
>> flush the abstract formal parameters whenever we encounter DW_TAG_variable.
>
> Cool. I see you're way ahead of me. If you're not careful you may end up as 
> the dwarf maintainer. :P

Pavel, it is unfortunately really the case that with the current patch, the 
parameters might get interleaved with locals:

  #include 
  
  void f(int used, int unused) {
int local = 1 + used;
printf("Hello %i", local); // break here
  }
  
  int main() {
f(4, 3);
return 0;
  }

Here is the LLDB session:

  $ bin/lldb a.out
  ...
  (lldb) b f
  Breakpoint 1: 2 locations.
  (lldb) r
  ...
  * thread #1, name = 'a.out', stop reason = breakpoint 1.2
  frame #0: 0x00401151 a.out`main [inlined] f(used=4, 
unused=) at a.cc:5:3
  (lldb) frame var
  (int) used = 4
  (int) local = 5  <--- HERE, a local variables got between the parameters 
because we append unused parameters at the end.
  (int) unused = 

Let me try to rewrite the code so that the trailing unused parameters are 
inserted after the last concrete parameter (or at the beginning of variable 
list if there are no concrete parameters). Let me know if you think it is 
unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571

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

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty [WIP]

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny added inline comments.



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py:560-564
+def get_connect_address(self):
+libc = ctypes.CDLL(None)
+libc.ptsname.argtypes = (ctypes.c_int,)
+libc.ptsname.restype = ctypes.c_char_p
+return libc.ptsname(self._master).decode()

labath wrote:
> I'm wondering if we should have an overload of SBTarget::ConnectRemote that 
> accepts a file descriptor (or SBFile, SBCommunication, ...).
> 
> This would make the socket case easier as well, and it seems like it could be 
> generally useful.
I'm not saying 'no' but I think we'd still want to explicitly test `process 
connect` command somewhere.


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

https://reviews.llvm.org/D110878

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

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

2021-10-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Just a couple of questions inline.

In D110571#3035814 , @jarin wrote:

> - thread #1, name = 'a.out', stop reason = breakpoint 1.2 frame #0: 
> 0x00401151 a.out`main [inlined] f(used=4, unused=) at 
> a.cc:5:3
>
> (lldb) frame var
> (int) used = 4
> (int) local = 5  <--- HERE, a local variables got between the parameters 
> because we append unused parameters at the end.
> (int) unused = 
>
>   Let me try to rewrite the code so that the trailing unused parameters are 
> inserted after the last concrete parameter (or at the beginning of variable 
> list if there are no concrete parameters). Let me know if you think it is 
> unnecessary.

TBH, I have no idea. The function description comes out right, so it seems at 
least some parts of lldb are prepared to handle this. I suppose it would be 
nicer if they were grouped together, but if it makes the code significantly 
more complex, then I probably wouldn't bother.

Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3561-3562
+const lldb::addr_t func_low_pc, VariableList &variable_list,
+llvm::SmallVectorImpl &abstract_formal_parameters,
+size_t &abstract_parameter_index) {
   size_t vars_added = 0;

I'm wondering if one could pass a single `ArrayRef &` argument 
instead of the array+index pair. FWICS, you're processing the list in a 
left-to-right fashion, which seems ideal for `ArrayRef::drop_front`. WDYT?

Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3575-3576
+  bool found = false;
+  while (parameter) {
+parameter = parameter.GetReferencedDIE(DW_AT_abstract_origin);
+if (parameter == abstract_parameter) {

All of these abstract origin loops make me uneasy. They make it very easy to 
hang (whether deliberately or not) the debugger with a bit of incorrect dwarf 
(self-referencing DIEs). Do we actually know of any abstract_origin chains?
It's not really clear to me what would be the right interpretation of that, so 
I can't even say whether this algorithm would be correct there.

Maybe just stick to a single "dereference" ?

Comment at: 
lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py:17
+
+# For the unused parameters, only check the types.
+self.assertEqual("void *", 
self.frame().FindVariable("unused1").GetType().GetName())

Maybe we could check something else as well... Do `GetDescription` or 
`str(value)` return something reasonable here?

Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test:1
+# UNSUPPORTED: system-darwin, system-windows
+

You should be able to drop this now.

Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test:33
+# Note: image lookup does not show variables outside of their
+#   location, so |partial| is missing here.
+# CHECK: name = "unused3", type = "int", location = 

You could add `CHECK-NOT: partial` here

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110571

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

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

2021-10-01 Thread Pavel Labath via Phabricator via lldb-commits

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

Is anyone passing save_process_group = true now? If not, I'd really like to 
delete it, as the implementation is quite scary.

Comment at: lldb/include/lldb/Host/Terminal.h:51
 /// descriptor and later restore that state as it originally was.
 class TerminalState {
 public:

mgorny wrote:
> labath wrote:
> > I guess it would be good to delete the copy operations as well.
> This is now implicit via PImpl; or do you prefer explicit?
implicit is fine

Comment at: lldb/include/lldb/Host/Terminal.h:130
+  int m_tflags = -1;///< Cached tflags information.
+  std::unique_ptr m_data; ///< Platform-specific implementation.
   lldb::pid_t m_process_group = -1; ///< Cached process group information.

Maybe call the class `Data` as well?

Comment at: lldb/source/Host/common/Terminal.cpp:92
+: m_tty(term) {
+  Save(term, save_process_group);
 }

Could we make `Save` private now? Or even inline it into the constructor ?

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

https://reviews.llvm.org/D110721

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

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty [WIP]

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 376456.
mgorny added a comment.

Replaced the class hacks with an abstraction over socket & connection sockets.


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

https://reviews.llvm.org/D110878

Files:
  lldb/test/API/functionalities/gdb_remote_client/TestPty.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -1,8 +1,12 @@
+import ctypes
 import errno
+import io
 import os
 import os.path
+import pty
 import threading
 import socket
+import tty
 import lldb
 import binascii
 import traceback
@@ -332,6 +336,101 @@
 pass
 
 
+class ServerSocket:
+"""
+A wrapper class for TCP or pty-based server.
+"""
+
+def get_connect_address(self):
+"""Get address for the client to connect to."""
+
+def close_server(self):
+"""Close all resources used by the server."""
+
+def accept(self):
+"""Accept a single client connection to the server."""
+
+def close_connection(self):
+"""Close all resources used by the accepted connection."""
+
+def recv(self):
+"""Receive a data packet from the connected client."""
+
+def sendall(self, data):
+"""Send the data to the connected client."""
+
+
+class TCPServerSocket(ServerSocket):
+def __init__(self):
+family, type, proto, _, addr = socket.getaddrinfo(
+"localhost", 0, proto=socket.IPPROTO_TCP)[0]
+self._server_socket = socket.socket(family, type, proto)
+self._connection = None
+
+self._server_socket.bind(addr)
+self._server_socket.listen(1)
+
+def get_connect_address(self):
+return "[{}]:{}".format(*self._server_socket.getsockname())
+
+def close_server(self):
+self._server_socket.close()
+
+def accept(self):
+assert self._connection is None
+# accept() is stubborn and won't fail even when the socket is
+# shutdown, so we'll use a timeout
+self._server_socket.settimeout(30.0)
+client, client_addr = self._server_socket.accept()
+# The connected client inherits its timeout from self._socket,
+# but we'll use a blocking socket for the client
+client.settimeout(None)
+self._connection = client
+
+def close_connection(self):
+assert self._connection is not None
+self._connection.close()
+self._connection = None
+
+def recv(self):
+assert self._connection is not None
+return self._connection.recv(4096)
+
+def sendall(self, data):
+assert self._connection is not None
+return self._connection.sendall(data)
+
+
+class PtyServerSocket(ServerSocket):
+def __init__(self):
+master, slave = pty.openpty()
+tty.setraw(master)
+self._master = io.FileIO(master, 'r+b')
+self._slave = io.FileIO(slave, 'r+b')
+
+def get_connect_address(self):
+libc = ctypes.CDLL(None)
+libc.ptsname.argtypes = (ctypes.c_int,)
+libc.ptsname.restype = ctypes.c_char_p
+return libc.ptsname(self._master.fileno()).decode()
+
+def close_server(self):
+self._slave.close()
+self._master.close()
+
+def recv(self):
+try:
+return self._master.read(4096)
+except OSError as e:
+# closing the pty results in EIO on Linux, convert it to EOF
+if e.errno == errno.EIO:
+return b''
+raise
+
+def sendall(self, data):
+return self._master.write(data)
+
+
 class MockGDBServer:
 """
 A simple TCP-based GDB server that can test client behavior by receiving
@@ -342,8 +441,8 @@
 """
 
 responder = None
+_socket_class = TCPServerSocket
 _socket = None
-_client = None
 _thread = None
 _receivedData = None
 _receivedDataOffset = None
@@ -353,38 +452,24 @@
 self.responder = MockGDBServerResponder()
 
 def start(self):
-family, type, proto, _, addr = socket.getaddrinfo("localhost", 0,
-proto=socket.IPPROTO_TCP)[0]
-self._socket = socket.socket(family, type, proto)
-
-
-self._socket.bind(addr)
-self._socket.listen(1)
-
+self._socket = self._socket_class()
 # Start a thread that waits for a client connection.
 self._thread = threading.Thread(target=self._run)
 self._thread.start()
 
 def stop(self):
-self._socket.close()
+self._socket.close_server()
 self._thread.join()
 self._thread = None
 
 def get_connect_address(self):
-return "[{}]:{}".format(*self._socket.getsockname())
+return self._socket.get_connect_addres

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty [WIP]

2021-10-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/test/API/functionalities/gdb_remote_client/TestPty.py:21
+
+@skipIfWindows
+def test_process_connect_sync(self):

I think this can be applied at class level



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py:409
+self._master = io.FileIO(master, 'r+b')
+self._slave = io.FileIO(slave, 'r+b')
+

do you actually need to keep the slave open here?


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

https://reviews.llvm.org/D110878

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

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny marked 2 inline comments as done.
mgorny added a comment.

In D110721#3035879 , @labath wrote:

> Is anyone passing save_process_group = true now? If not, I'd really like to 
> delete it, as the implementation is quite scary.

Yes, `Debugger::SaveInputTerminalState()` does.




Comment at: lldb/source/Host/common/Terminal.cpp:92
+: m_tty(term) {
+  Save(term, save_process_group);
 }

labath wrote:
> Could we make `Save` private now? Or even inline it into the constructor ?
Unfortunately, no. `Debugger` still manually saves/restores.


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

https://reviews.llvm.org/D110721

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

[Lldb-commits] [lldb] 58b4501 - [lldb] [Host] Refactor TerminalState

2021-10-01 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-10-01T12:53:21+02:00
New Revision: 58b4501eeabb2728a5c48e05295f3636db0ecee1

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

LOG: [lldb] [Host] Refactor TerminalState

Refactor TerminalState to make the code simpler.  Move 'struct termios'
to a PImpl-style subclass.  Add an RAII interface to automatically store
and restore the state.

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

Added: 


Modified: 
lldb/include/lldb/Host/Terminal.h
lldb/source/Host/common/Terminal.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Terminal.h 
b/lldb/include/lldb/Host/Terminal.h
index b22b96776c335..081a427fdb50d 100644
--- a/lldb/include/lldb/Host/Terminal.h
+++ b/lldb/include/lldb/Host/Terminal.h
@@ -13,8 +13,6 @@
 #include "lldb/Host/Config.h"
 #include "lldb/lldb-private.h"
 
-struct termios;
-
 namespace lldb_private {
 
 class Terminal {
@@ -41,17 +39,29 @@ class Terminal {
   int m_fd; // This may or may not be a terminal file descriptor
 };
 
-/// \class State Terminal.h "lldb/Host/Terminal.h"
-/// A terminal state saving/restoring class.
+/// \class TerminalState Terminal.h "lldb/Host/Terminal.h"
+/// A RAII-friendly terminal state saving/restoring class.
 ///
 /// This class can be used to remember the terminal state for a file
 /// descriptor and later restore that state as it originally was.
 class TerminalState {
+  class Data;
+
 public:
-  /// Default constructor
-  TerminalState();
+  /// Construct a new instance and optionally save terminal state.
+  ///
+  /// \param[in] term
+  /// The Terminal instance holding the file descriptor to save the state
+  /// of.  If the instance is not associated with a fd, no state will
+  /// be saved.
+  ///
+  /// \param[in] save_process_group
+  /// If \b true, save the process group settings, else do not
+  /// save the process group settings for a TTY.
+  TerminalState(Terminal term = -1, bool save_process_group = false);
 
-  /// Destructor
+  /// Destroy the instance, restoring terminal state if saved.  If restoring
+  /// state is undesirable, the instance needs to be reset before destruction.
   ~TerminalState();
 
   /// Save the TTY state for \a fd.
@@ -60,8 +70,8 @@ class TerminalState {
   /// "save_process_group" is true, attempt to save the process group info for
   /// the TTY.
   ///
-  /// \param[in] fd
-  /// The file descriptor to save the state of.
+  /// \param[in] term
+  /// The Terminal instance holding fd to save.
   ///
   /// \param[in] save_process_group
   /// If \b true, save the process group settings, else do not
@@ -70,7 +80,7 @@ class TerminalState {
   /// \return
   /// Returns \b true if \a fd describes a TTY and if the state
   /// was able to be saved, \b false otherwise.
-  bool Save(int fd, bool save_process_group);
+  bool Save(Terminal term, bool save_process_group);
 
   /// Restore the TTY state to the cached state.
   ///
@@ -115,12 +125,9 @@ class TerminalState {
   bool ProcessGroupIsValid() const;
 
   // Member variables
-  Terminal m_tty; ///< A terminal
-  int m_tflags = -1; ///< Cached tflags information.
-#if LLDB_ENABLE_TERMIOS
-  std::unique_ptr
-  m_termios_up; ///< Cached terminal state information.
-#endif
+  Terminal m_tty;   ///< A terminal
+  int m_tflags = -1;///< Cached tflags information.
+  std::unique_ptr m_data; ///< Platform-specific implementation.
   lldb::pid_t m_process_group = -1; ///< Cached process group information.
 };
 

diff  --git a/lldb/source/Host/common/Terminal.cpp 
b/lldb/source/Host/common/Terminal.cpp
index 147bbdcde137d..6adcfa6a92c41 100644
--- a/lldb/source/Host/common/Terminal.cpp
+++ b/lldb/source/Host/common/Terminal.cpp
@@ -81,63 +81,45 @@ bool Terminal::SetCanonical(bool enabled) {
   return false;
 }
 
-// Default constructor
-TerminalState::TerminalState()
-: m_tty()
+struct TerminalState::Data {
 #if LLDB_ENABLE_TERMIOS
-  ,
-  m_termios_up()
+  struct termios m_termios; ///< Cached terminal state information.
 #endif
-{
+};
+
+TerminalState::TerminalState(Terminal term, bool save_process_group)
+: m_tty(term) {
+  Save(term, save_process_group);
 }
 
-// Destructor
-TerminalState::~TerminalState() = default;
+TerminalState::~TerminalState() { Restore(); }
 
 void TerminalState::Clear() {
   m_tty.Clear();
   m_tflags = -1;
-#if LLDB_ENABLE_TERMIOS
-  m_termios_up.reset();
-#endif
+  m_data.reset();
   m_process_group = -1;
 }
 
-// Save the current state of the TTY for the file descriptor "fd" and if
-// "save_process_gro

[Lldb-commits] [PATCH] D110721: [lldb] [Host] Refactor TerminalState

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG58b4501eeabb: [lldb] [Host] Refactor TerminalState (authored 
by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D110721?vs=376271&id=376468#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110721

Files:
  lldb/include/lldb/Host/Terminal.h
  lldb/source/Host/common/Terminal.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4351,9 +4351,8 @@
 
 SetIsDone(false);
 const int read_fd = m_read_file.GetDescriptor();
-TerminalState terminal_state;
-terminal_state.Save(read_fd, false);
 Terminal terminal(read_fd);
+TerminalState terminal_state(terminal, false);
 terminal.SetCanonical(false);
 terminal.SetEcho(false);
 // FD_ZERO, FD_SET are not supported on windows
@@ -4399,7 +4398,6 @@
 }
 m_is_running = false;
 #endif
-terminal_state.Restore();
   }
 
   void Cancel() override {
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -430,11 +430,9 @@
   int stdin_fd = GetInputFD();
   if (stdin_fd >= 0) {
 Terminal terminal(stdin_fd);
-TerminalState terminal_state;
-const bool is_a_tty = terminal.IsATerminal();
+TerminalState terminal_state(terminal);
 
-if (is_a_tty) {
-  terminal_state.Save(stdin_fd, false);
+if (terminal.IsATerminal()) {
   terminal.SetCanonical(false);
   terminal.SetEcho(true);
 }
@@ -464,9 +462,6 @@
 run_string.Printf("run_python_interpreter (%s)",
   m_python->GetDictionaryName());
 PyRun_SimpleString(run_string.GetData());
-
-if (is_a_tty)
-  terminal_state.Restore();
   }
 }
 SetIsDone(true);
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -355,7 +355,6 @@
 PyEval_InitThreads();
   }
 
-  TerminalState m_stdin_tty_state;
   PyGILState_STATE m_gil_state = PyGILState_UNLOCKED;
   bool m_was_already_initialized = false;
 };
Index: lldb/source/Host/common/Terminal.cpp
===
--- lldb/source/Host/common/Terminal.cpp
+++ lldb/source/Host/common/Terminal.cpp
@@ -81,63 +81,45 @@
   return false;
 }
 
-// Default constructor
-TerminalState::TerminalState()
-: m_tty()
+struct TerminalState::Data {
 #if LLDB_ENABLE_TERMIOS
-  ,
-  m_termios_up()
+  struct termios m_termios; ///< Cached terminal state information.
 #endif
-{
+};
+
+TerminalState::TerminalState(Terminal term, bool save_process_group)
+: m_tty(term) {
+  Save(term, save_process_group);
 }
 
-// Destructor
-TerminalState::~TerminalState() = default;
+TerminalState::~TerminalState() { Restore(); }
 
 void TerminalState::Clear() {
   m_tty.Clear();
   m_tflags = -1;
-#if LLDB_ENABLE_TERMIOS
-  m_termios_up.reset();
-#endif
+  m_data.reset();
   m_process_group = -1;
 }
 
-// Save the current state of the TTY for the file descriptor "fd" and if
-// "save_process_group" is true, attempt to save the process group info for the
-// TTY.
-bool TerminalState::Save(int fd, bool save_process_group) {
-  m_tty.SetFileDescriptor(fd);
+bool TerminalState::Save(Terminal term, bool save_process_group) {
+  Clear();
+  m_tty = term;
   if (m_tty.IsATerminal()) {
+int fd = m_tty.GetFileDescriptor();
 #if LLDB_ENABLE_POSIX
 m_tflags = ::fcntl(fd, F_GETFL, 0);
-#endif
 #if LLDB_ENABLE_TERMIOS
-if (m_termios_up == nullptr)
-  m_termios_up.reset(new struct termios);
-int err = ::tcgetattr(fd, m_termios_up.get());
-if (err != 0)
-  m_termios_up.reset();
-#endif // #if LLDB_ENABLE_TERMIOS
-#if LLDB_ENABLE_POSIX
+std::unique_ptr new_data{new Data()};
+if (::tcgetattr(fd, &new_data->m_termios) != 0)
+  m_data = std::move(new_data);
+#endif // LLDB_ENABLE_TERMIOS
 if (save_process_group)
-  m_process_group = ::tcgetpgrp(0);
-else
-  m_process_group = -1;
-#endif
-  } else {
-m_tty.Clear();
-m_tflags = -1;
-#if LLDB_ENABLE_TERMIOS
-m_termios_up.reset();
-#endif
-m_process_group

[Lldb-commits] [PATCH] D110914: [lldb] Remove "dwarf dynamic register size expressions" from RegisterInfo

2021-10-01 Thread Jessica Clarke via Phabricator via lldb-commits

jrtc27 added a comment.

Is this removing this going to be a problem for RISC-V, where the 
floating-point registers could be 32-bit or 64-bit (or 128-bit in future), 
depending on which extensions you have?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110914

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

[Lldb-commits] [PATCH] D110914: [lldb] Remove "dwarf dynamic register size expressions" from RegisterInfo

2021-10-01 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D110914#3036079 , @jrtc27 wrote:

> Is this removing this going to be a problem for RISC-V, where the 
> floating-point registers could be 32-bit or 64-bit (or 128-bit in future), 
> depending on which extensions you have?

Is the size of the register fixed for the lifetime of a process ? If yes (it 
sounds like this is the case), then this is not a problem, as you'd just need 
to provide the correct definitions when the process is created.

And if their size can change, then it's still should not be a problem, as you 
can copy the ARM SVE model. (One of the reasons for removing this is to avoid 
having two ways of doing the same thing.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110914

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

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty [WIP]

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny marked 2 inline comments as done.
mgorny added inline comments.



Comment at: 
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py:409
+self._master = io.FileIO(master, 'r+b')
+self._slave = io.FileIO(slave, 'r+b')
+

labath wrote:
> do you actually need to keep the slave open here?
Yes, it seems this way. If I close it early, the test hangs.


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

https://reviews.llvm.org/D110878

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

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 376480.
mgorny marked an inline comment as done.
mgorny retitled this revision from "[lldb] Add a gdb_remote_client test for 
connecting to pty [WIP]" to "[lldb] Add a gdb_remote_client test for connecting 
to pty".
mgorny added a comment.

Make it easier to override socket class at `GDBRemoteTestBase` level. Add async 
test, mark the whole class `skipIfWindows`.


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

https://reviews.llvm.org/D110878

Files:
  lldb/test/API/functionalities/gdb_remote_client/TestPty.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -1,8 +1,12 @@
+import ctypes
 import errno
+import io
 import os
 import os.path
+import pty
 import threading
 import socket
+import tty
 import lldb
 import binascii
 import traceback
@@ -332,6 +336,101 @@
 pass
 
 
+class ServerSocket:
+"""
+A wrapper class for TCP or pty-based server.
+"""
+
+def get_connect_address(self):
+"""Get address for the client to connect to."""
+
+def close_server(self):
+"""Close all resources used by the server."""
+
+def accept(self):
+"""Accept a single client connection to the server."""
+
+def close_connection(self):
+"""Close all resources used by the accepted connection."""
+
+def recv(self):
+"""Receive a data packet from the connected client."""
+
+def sendall(self, data):
+"""Send the data to the connected client."""
+
+
+class TCPServerSocket(ServerSocket):
+def __init__(self):
+family, type, proto, _, addr = socket.getaddrinfo(
+"localhost", 0, proto=socket.IPPROTO_TCP)[0]
+self._server_socket = socket.socket(family, type, proto)
+self._connection = None
+
+self._server_socket.bind(addr)
+self._server_socket.listen(1)
+
+def get_connect_address(self):
+return "[{}]:{}".format(*self._server_socket.getsockname())
+
+def close_server(self):
+self._server_socket.close()
+
+def accept(self):
+assert self._connection is None
+# accept() is stubborn and won't fail even when the socket is
+# shutdown, so we'll use a timeout
+self._server_socket.settimeout(30.0)
+client, client_addr = self._server_socket.accept()
+# The connected client inherits its timeout from self._socket,
+# but we'll use a blocking socket for the client
+client.settimeout(None)
+self._connection = client
+
+def close_connection(self):
+assert self._connection is not None
+self._connection.close()
+self._connection = None
+
+def recv(self):
+assert self._connection is not None
+return self._connection.recv(4096)
+
+def sendall(self, data):
+assert self._connection is not None
+return self._connection.sendall(data)
+
+
+class PtyServerSocket(ServerSocket):
+def __init__(self):
+master, slave = pty.openpty()
+tty.setraw(master)
+self._master = io.FileIO(master, 'r+b')
+self._slave = io.FileIO(slave, 'r+b')
+
+def get_connect_address(self):
+libc = ctypes.CDLL(None)
+libc.ptsname.argtypes = (ctypes.c_int,)
+libc.ptsname.restype = ctypes.c_char_p
+return libc.ptsname(self._master.fileno()).decode()
+
+def close_server(self):
+self._slave.close()
+self._master.close()
+
+def recv(self):
+try:
+return self._master.read(4096)
+except OSError as e:
+# closing the pty results in EIO on Linux, convert it to EOF
+if e.errno == errno.EIO:
+return b''
+raise
+
+def sendall(self, data):
+return self._master.write(data)
+
+
 class MockGDBServer:
 """
 A simple TCP-based GDB server that can test client behavior by receiving
@@ -343,48 +442,34 @@
 
 responder = None
 _socket = None
-_client = None
 _thread = None
 _receivedData = None
 _receivedDataOffset = None
 _shouldSendAck = True
 
-def __init__(self):
+def __init__(self, socket_class):
+self._socket_class = socket_class
 self.responder = MockGDBServerResponder()
 
 def start(self):
-family, type, proto, _, addr = socket.getaddrinfo("localhost", 0,
-proto=socket.IPPROTO_TCP)[0]
-self._socket = socket.socket(family, type, proto)
-
-
-self._socket.bind(addr)
-self._socket.listen(1)
-
+self._socket = self._socket_class()
 # Start a thread that waits for a client connection.
 self._thread = threading.Thread(target=self._run)

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 376487.
mgorny added a comment.

Add `get_connect_url()` to simplify combining address with proper 
protocol/prefix.


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

https://reviews.llvm.org/D110878

Files:
  lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
  lldb/test/API/functionalities/gdb_remote_client/TestPty.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -1,8 +1,12 @@
+import ctypes
 import errno
+import io
 import os
 import os.path
+import pty
 import threading
 import socket
+import tty
 import lldb
 import binascii
 import traceback
@@ -332,6 +336,110 @@
 pass
 
 
+class ServerSocket:
+"""
+A wrapper class for TCP or pty-based server.
+"""
+
+def get_connect_address(self):
+"""Get address for the client to connect to."""
+
+def get_connect_url(self):
+"""Get URL suitable for process connect command."""
+
+def close_server(self):
+"""Close all resources used by the server."""
+
+def accept(self):
+"""Accept a single client connection to the server."""
+
+def close_connection(self):
+"""Close all resources used by the accepted connection."""
+
+def recv(self):
+"""Receive a data packet from the connected client."""
+
+def sendall(self, data):
+"""Send the data to the connected client."""
+
+
+class TCPServerSocket(ServerSocket):
+def __init__(self):
+family, type, proto, _, addr = socket.getaddrinfo(
+"localhost", 0, proto=socket.IPPROTO_TCP)[0]
+self._server_socket = socket.socket(family, type, proto)
+self._connection = None
+
+self._server_socket.bind(addr)
+self._server_socket.listen(1)
+
+def get_connect_address(self):
+return "[{}]:{}".format(*self._server_socket.getsockname())
+
+def get_connect_url(self):
+return "connect://" + self.get_connect_address()
+
+def close_server(self):
+self._server_socket.close()
+
+def accept(self):
+assert self._connection is None
+# accept() is stubborn and won't fail even when the socket is
+# shutdown, so we'll use a timeout
+self._server_socket.settimeout(30.0)
+client, client_addr = self._server_socket.accept()
+# The connected client inherits its timeout from self._socket,
+# but we'll use a blocking socket for the client
+client.settimeout(None)
+self._connection = client
+
+def close_connection(self):
+assert self._connection is not None
+self._connection.close()
+self._connection = None
+
+def recv(self):
+assert self._connection is not None
+return self._connection.recv(4096)
+
+def sendall(self, data):
+assert self._connection is not None
+return self._connection.sendall(data)
+
+
+class PtyServerSocket(ServerSocket):
+def __init__(self):
+master, slave = pty.openpty()
+tty.setraw(master)
+self._master = io.FileIO(master, 'r+b')
+self._slave = io.FileIO(slave, 'r+b')
+
+def get_connect_address(self):
+libc = ctypes.CDLL(None)
+libc.ptsname.argtypes = (ctypes.c_int,)
+libc.ptsname.restype = ctypes.c_char_p
+return libc.ptsname(self._master.fileno()).decode()
+
+def get_connect_url(self):
+return "file://" + self.get_connect_address()
+
+def close_server(self):
+self._slave.close()
+self._master.close()
+
+def recv(self):
+try:
+return self._master.read(4096)
+except OSError as e:
+# closing the pty results in EIO on Linux, convert it to EOF
+if e.errno == errno.EIO:
+return b''
+raise
+
+def sendall(self, data):
+return self._master.write(data)
+
+
 class MockGDBServer:
 """
 A simple TCP-based GDB server that can test client behavior by receiving
@@ -343,48 +451,37 @@
 
 responder = None
 _socket = None
-_client = None
 _thread = None
 _receivedData = None
 _receivedDataOffset = None
 _shouldSendAck = True
 
-def __init__(self):
+def __init__(self, socket_class):
+self._socket_class = socket_class
 self.responder = MockGDBServerResponder()
 
 def start(self):
-family, type, proto, _, addr = socket.getaddrinfo("localhost", 0,
-proto=socket.IPPROTO_TCP)[0]
-self._socket = socket.socket(family, type, proto)
-
-
-self._socket.bind(addr)
-self._socket.listen(1

[Lldb-commits] [PATCH] D110914: [lldb] Remove "dwarf dynamic register size expressions" from RegisterInfo

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

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

Thanks for doing this. The baseline code LGTM but I haven't verified the 
register enumerations ;-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110914

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

[Lldb-commits] [lldb] 8fa2394 - [lldb] Add a gdb_remote_client test for connecting to pty

2021-10-01 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-10-01T14:31:40+02:00
New Revision: 8fa2394bad433558f3083cee158043e2fb66d781

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

LOG: [lldb] Add a gdb_remote_client test for connecting to pty

Add a minimal mock server utilizing a pty, and add a client test
connecting to that server.

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

Added: 
lldb/test/API/functionalities/gdb_remote_client/TestPty.py

Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py
lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py
index 30262fce74808..ff32a6e0093a5 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py
@@ -50,7 +50,7 @@ def qsProcessInfo(self):
 
 try:
 self.runCmd("platform select remote-linux")
-self.runCmd("platform connect connect://" + 
self.server.get_connect_address())
+self.runCmd("platform connect " + self.server.get_connect_url())
 self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
 self.expect("platform process list -x",
 substrs=["2 matching processes were found", 
"test_process", "another_test_process"])
@@ -84,8 +84,8 @@ def test_no_timeout(self):
 self.runCmd("settings set plugin.process.gdb-remote.packet-timeout 30")
 plat = lldb.SBPlatform("remote-linux")
 try:
-
self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions("connect://"
-+ self.server.get_connect_address(
+
self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions(
+self.server.get_connect_url(
 self.assertEqual(plat.GetWorkingDirectory(), "/foo/bar")
 finally:
 plat.DisconnectRemote()
@@ -98,8 +98,8 @@ def test_timeout(self):
 self.runCmd("settings set plugin.process.gdb-remote.packet-timeout 3")
 plat = lldb.SBPlatform("remote-linux")
 try:
-
self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions("connect://"
-+ self.server.get_connect_address(
+
self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions(
+self.server.get_connect_url(
 self.assertIsNone(plat.GetWorkingDirectory())
 finally:
 plat.DisconnectRemote()

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
index 9ad498de15d6e..6f99c939562b6 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -39,8 +39,7 @@ def test_process_connect_sync(self):
 self.dbg.SetAsync(False)
 self.expect("platform select remote-gdb-server",
 substrs=['Platform: remote-gdb-server', 'Connected: 
no'])
-self.expect("process connect connect://" +
-self.server.get_connect_address(),
+self.expect("process connect " + self.server.get_connect_url(),
 substrs=['Process', 'stopped'])
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
@@ -52,8 +51,7 @@ def test_process_connect_async(self):
 self.dbg.SetAsync(True)
 self.expect("platform select remote-gdb-server",
 substrs=['Platform: remote-gdb-server', 'Connected: 
no'])
-self.expect("process connect connect://" +
-self.server.get_connect_address(),
+self.expect("process connect " + self.server.get_connect_url(),
 matching=False,
 substrs=['Process', 'stopped'])
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),

diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestPty.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestPty.py
new file mode 100644
index 0..46078b7a0adb3
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestPty.py
@@ -0,0 +1,35 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+@skipIfWindows
+class TestPty(GDBRemoteTestBase):
+mydir = TestBase.compute_mydir(_

[Lldb-commits] [PATCH] D110878: [lldb] Add a gdb_remote_client test for connecting to pty

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8fa2394bad43: [lldb] Add a gdb_remote_client test for 
connecting to pty (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110878

Files:
  lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
  lldb/test/API/functionalities/gdb_remote_client/TestPty.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -1,8 +1,12 @@
+import ctypes
 import errno
+import io
 import os
 import os.path
+import pty
 import threading
 import socket
+import tty
 import lldb
 import binascii
 import traceback
@@ -332,6 +336,110 @@
 pass
 
 
+class ServerSocket:
+"""
+A wrapper class for TCP or pty-based server.
+"""
+
+def get_connect_address(self):
+"""Get address for the client to connect to."""
+
+def get_connect_url(self):
+"""Get URL suitable for process connect command."""
+
+def close_server(self):
+"""Close all resources used by the server."""
+
+def accept(self):
+"""Accept a single client connection to the server."""
+
+def close_connection(self):
+"""Close all resources used by the accepted connection."""
+
+def recv(self):
+"""Receive a data packet from the connected client."""
+
+def sendall(self, data):
+"""Send the data to the connected client."""
+
+
+class TCPServerSocket(ServerSocket):
+def __init__(self):
+family, type, proto, _, addr = socket.getaddrinfo(
+"localhost", 0, proto=socket.IPPROTO_TCP)[0]
+self._server_socket = socket.socket(family, type, proto)
+self._connection = None
+
+self._server_socket.bind(addr)
+self._server_socket.listen(1)
+
+def get_connect_address(self):
+return "[{}]:{}".format(*self._server_socket.getsockname())
+
+def get_connect_url(self):
+return "connect://" + self.get_connect_address()
+
+def close_server(self):
+self._server_socket.close()
+
+def accept(self):
+assert self._connection is None
+# accept() is stubborn and won't fail even when the socket is
+# shutdown, so we'll use a timeout
+self._server_socket.settimeout(30.0)
+client, client_addr = self._server_socket.accept()
+# The connected client inherits its timeout from self._socket,
+# but we'll use a blocking socket for the client
+client.settimeout(None)
+self._connection = client
+
+def close_connection(self):
+assert self._connection is not None
+self._connection.close()
+self._connection = None
+
+def recv(self):
+assert self._connection is not None
+return self._connection.recv(4096)
+
+def sendall(self, data):
+assert self._connection is not None
+return self._connection.sendall(data)
+
+
+class PtyServerSocket(ServerSocket):
+def __init__(self):
+master, slave = pty.openpty()
+tty.setraw(master)
+self._master = io.FileIO(master, 'r+b')
+self._slave = io.FileIO(slave, 'r+b')
+
+def get_connect_address(self):
+libc = ctypes.CDLL(None)
+libc.ptsname.argtypes = (ctypes.c_int,)
+libc.ptsname.restype = ctypes.c_char_p
+return libc.ptsname(self._master.fileno()).decode()
+
+def get_connect_url(self):
+return "file://" + self.get_connect_address()
+
+def close_server(self):
+self._slave.close()
+self._master.close()
+
+def recv(self):
+try:
+return self._master.read(4096)
+except OSError as e:
+# closing the pty results in EIO on Linux, convert it to EOF
+if e.errno == errno.EIO:
+return b''
+raise
+
+def sendall(self, data):
+return self._master.write(data)
+
+
 class MockGDBServer:
 """
 A simple TCP-based GDB server that can test client behavior by receiving
@@ -343,48 +451,37 @@
 
 responder = None
 _socket = None
-_client = None
 _thread = None
 _receivedData = None
 _receivedDataOffset = None
 _shouldSendAck = True
 
-def __init__(self):
+def __init__(self, socket_class):
+self._socket_class = socket_class
 self.responder = MockGDBServerResponder()
 
 def start(self):
-family, type, proto, _, addr = socket.getaddrinfo("localhost", 0,
-proto=socket.IPPROTO_TCP)[0]
-self._socket =

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

2021-10-01 Thread Jaroslav Sevcik via Phabricator via lldb-commits

jarin updated this revision to Diff 376501.
jarin marked 5 inline comments as done.
jarin added a comment.

Addressed reviewer comments, separated merging of the abstract parameters into 
a function, prevented interleaving of parameters with locals.


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

https://reviews.llvm.org/D110571

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/API/functionalities/unused-inlined-parameters/Makefile
  
lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py
  lldb/test/API/functionalities/unused-inlined-parameters/main.c
  lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/unused-inlined-params.s
  lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

Index: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
@@ -0,0 +1,48 @@
+# RUN: llvm-mc -filetype=obj %S/Inputs/unused-inlined-params.s \
+# RUN: -triple x86_64-pc-linux -o %t.o
+# RUN: %lldb %t.o -s %s -o exit | FileCheck %s
+
+
+# In this test we verify that inlined functions still mention
+# all their parameters in `frame variable`, even when those
+# parameters were completely optimized away from the concrete
+# instance of the inlined function in the debug info.
+# The debugger should look up the parameters in the abstract
+# origin of the concrete instance.
+
+# Let us check that unused parameters of an inlined function are listed
+# at the inlined function entry.
+image lookup -v -s break_at_inlined_f_in_main
+# CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main
+# CHECK: name = "unused1", type = "void *", location = 
+# CHECK: name = "used", type = "int", location = DW_OP_consts +42
+# CHECK: name = "unused2", type = "int", location = 
+# CHECK: name = "partial", type = "int", location = DW_OP_reg4 RSI
+# CHECK: name = "unused3", type = "int", location = 
+
+# Show variables outsid of the live range of the 'partial' parameter
+# and verify that the output is as expected.
+image lookup -v -s break_at_inlined_f_in_main_between_printfs
+# CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main_between_printfs
+# CHECK: name = "unused1", type = "void *", location = 
+# CHECK: name = "used", type = "int", location = DW_OP_reg3 RBX
+# CHECK: name = "unused2", type = "int", location = 
+# Note: image lookup does not show variables outside of their
+#   location, so |partial| is missing here.
+# CHECK-NOT: partial
+# CHECK: name = "unused3", type = "int", location = 
+
+# Check that we show parameters even if all of them are compiled away.
+image lookup -v -s  break_at_inlined_g_in_main
+# CHECK-LABEL: image lookup -v -s  break_at_inlined_g_in_main
+# CHECK: name = "unused", type = "int", location = 
+
+# Check that even the other inlined instance of f displays the correct
+# parameters.
+image lookup -v -s  break_at_inlined_f_in_other
+# CHECK-LABEL: image lookup -v -s  break_at_inlined_f_in_other
+# CHECK: name = "unused1", type = "void *", location = 
+# CHECK: name = "used", type = "int", location = DW_OP_consts +1
+# CHECK: name = "unused2", type = "int", location = 
+# CHECK: name = "partial", type = "int", location =  DW_OP_consts +2
+# CHECK: name = "unused3", type = "int", location = 
Index: lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/unused-inlined-params.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/unused-inlined-params.s
@@ -0,0 +1,458 @@
+# The below program is roughly derived from the following C program.
+# To see the annotated debug info, look for the section 
+# '.section.debug_info' below.
+#
+# __attribute__((always_inline))
+# void f(void* unused1, int used, int unused2, int partial, int unused3) {
+#   used += partial;
+#   printf("f %i", partial);
+#   printf("f %i", used);   // |partial| is not live at this line.
+# }
+#
+# void g(int unused) {
+#   printf("Hello");
+# }
+#
+# __attribute__((noinline))
+# void other() {
+#   f(nullptr, 1, 0, 2, 0);
+# }
+#
+# int main(int argc, char** argv) {
+#   f(argv, 42, 1, argc, 2);
+#   g(1);
+#   other();
+#   return 0;
+# }
+
+.text
+.file"unused-inlined-params.c"
+
+.Lcu_begin:
+
+.globlother
+other:
+nop
+.Linlined_f_in_other:
+break_at_inlined_f_in_other:
+callqprintf# Omitted the setup of arguments.
+.Linlined_f_in_other_between_printfs:
+callqprintf# Omitted the setup of arguments.
+.Linlined_f_in_other_end:
+retq
+.Lother_end:
+.sizeother, .Lother_end-other
+
+.globlmain
+main:
+.file1 "/example" "unused-inlined-params.c"
+movl$1, %esi
+.Linlined_f:
+break_at_inlined_f_in_main:
+leal42(%rsi), %ebx
+.Linlined_f_before_printf:
+callqprint

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

2021-10-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib marked 7 inline comments as done.
mib added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp:105
+const char *ScriptedThread::GetName() {
+  if (m_name.empty()) {
+CheckInterpreterAndScriptObject();

JDevlieghere wrote:
> We're caching the name and the queue, is this an optimization (to avoid 
> calling into Python over and over) or are there assumptions that will break 
> if these are not stable? If so, should that be something we call out on the 
> Python side or alternatively, should we allow this to change and not cache 
> it? 
This meant to be an optimization to avoid calling the Python interface but I 
guess we can rid of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

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

[Lldb-commits] [lldb] 12ee4c9 - [lldb] [test] Delay pty/tty imports to fix Windows builds

2021-10-01 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-10-01T15:25:35+02:00
New Revision: 12ee4c9ad87e5c144cde2e8ff8a4787da0ed71df

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

LOG: [lldb] [test] Delay pty/tty imports to fix Windows builds

Delay pty/tty module imports until they are actually used, in order
to prevent their import failures on Windows.

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Removed: 




diff  --git a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py 
b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
index c395bac2de70..082b935885fb 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -3,10 +3,8 @@
 import io
 import os
 import os.path
-import pty
 import threading
 import socket
-import tty
 import lldb
 import binascii
 import traceback
@@ -409,6 +407,8 @@ def sendall(self, data):
 
 class PtyServerSocket(ServerSocket):
 def __init__(self):
+import pty
+import tty
 master, slave = pty.openpty()
 tty.setraw(master)
 self._master = io.FileIO(master, 'r+b')



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

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

2021-10-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 376502.
mib added a comment.

Address @JDevlieghere comments:

- Remove "caching"
- Refactor `error_with_message` lambda and `StructuredData` objects sanity 
checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -223,6 +223,12 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSwigPythonCreateScriptedThread(
+const char *python_class_name, const char *session_dictionary_name,
+const lldb::TargetSP &target_sp, std::string &error_string) {
+  return nullptr;
+}
+
 extern "C" void *
 LLDBSWIGPython_CreateFrameRecognizer(const char *python_class_name,
  const char *session_dictionary_name) {
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -72,6 +72,28 @@
 self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
 self.assertEqual(hello_world, memory_read)
 
+self.assertEqual(process.GetNumThreads(), 1)
+
+thread = process.GetSelectedThread()
+self.assertTrue(thread, "Invalid thread.")
+self.assertEqual(thread.GetThreadID(), 0x19)
+self.assertEqual(thread.GetName(), "MyScriptedThread.thread-1")
+self.assertEqual(thread.GetStopReason(), lldb.eStopReasonSignal)
+
+self.assertGreater(thread.GetNumFrames(), 0)
+
+frame = thread.GetFrameAtIndex(0)
+register_set = frame.registers # Returns an SBValueList.
+for regs in register_set:
+if 'GPR' in regs.name:
+registers  = regs
+break
+
+self.assertTrue(registers, "Invalid General Purpose Registers Set")
+self.assertEqual(registers.GetNumChildren(), 21)
+for idx, reg in enumerate(registers, start=1):
+self.assertEqual(idx, int(reg.value, 16))
+
 def test_launch_scripted_process_cli(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
@@ -0,0 +1,48 @@
+//===-- ScriptedThreadPythonInterface.h *- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "ScriptedPythonInterface.h"
+#include "lldb/Interpreter/ScriptedProcessInterface.h"
+
+namespace lldb_private {
+class ScriptedThreadPythonInterface : public ScriptedThreadInterface,
+

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

2021-10-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 376504.
mib added a comment.

Reformat code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -223,6 +223,12 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSwigPythonCreateScriptedThread(
+const char *python_class_name, const char *session_dictionary_name,
+const lldb::TargetSP &target_sp, std::string &error_string) {
+  return nullptr;
+}
+
 extern "C" void *
 LLDBSWIGPython_CreateFrameRecognizer(const char *python_class_name,
  const char *session_dictionary_name) {
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -72,6 +72,28 @@
 self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
 self.assertEqual(hello_world, memory_read)
 
+self.assertEqual(process.GetNumThreads(), 1)
+
+thread = process.GetSelectedThread()
+self.assertTrue(thread, "Invalid thread.")
+self.assertEqual(thread.GetThreadID(), 0x19)
+self.assertEqual(thread.GetName(), "MyScriptedThread.thread-1")
+self.assertEqual(thread.GetStopReason(), lldb.eStopReasonSignal)
+
+self.assertGreater(thread.GetNumFrames(), 0)
+
+frame = thread.GetFrameAtIndex(0)
+register_set = frame.registers # Returns an SBValueList.
+for regs in register_set:
+if 'GPR' in regs.name:
+registers  = regs
+break
+
+self.assertTrue(registers, "Invalid General Purpose Registers Set")
+self.assertEqual(registers.GetNumChildren(), 21)
+for idx, reg in enumerate(registers, start=1):
+self.assertEqual(idx, int(reg.value, 16))
+
 def test_launch_scripted_process_cli(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
@@ -0,0 +1,48 @@
+//===-- ScriptedThreadPythonInterface.h *- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "ScriptedPythonInterface.h"
+#include "lldb/Interpreter/ScriptedProcessInterface.h"
+
+namespace lldb_private {
+class ScriptedThreadPythonInterface : public ScriptedThreadInterface,
+  public ScriptedPythonInterface {
+public:
+  ScriptedThreadPythonInterface(ScriptInterpreterPythonImpl &in

[Lldb-commits] [PATCH] D110571: [lldb] Add omitted abstract formal parameters in DWARF symbol files

2021-10-01 Thread Jaroslav Sevcik via Phabricator via lldb-commits

jarin added a comment.

Thank you for the great comments, Pavel. I took a stab at merging the 
parameters without interleaving them with the locals. Let me know what you 
think; I can certainly put this back to the original state if you think this is 
a change for the worse.

(I am sorry for the churn, but I feel the code is fairly subtle and would like 
to leave it in a state we are all happy with.)




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3561-3562
+const lldb::addr_t func_low_pc, VariableList &variable_list,
+llvm::SmallVectorImpl &abstract_formal_parameters,
+size_t &abstract_parameter_index) {
   size_t vars_added = 0;

labath wrote:
> I'm wondering if one could pass a single `ArrayRef &` argument 
> instead of the array+index pair. FWICS, you're processing the list in a 
> left-to-right fashion, which seems ideal for `ArrayRef::drop_front`. WDYT?
I have replaced the in place-merging with a separate merging function, so this 
should not be relevant anymore.



Comment at: 
lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py:17
+
+# For the unused parameters, only check the types.
+self.assertEqual("void *", 
self.frame().FindVariable("unused1").GetType().GetName())

labath wrote:
> Maybe we could check something else as well... Do `GetDescription` or 
> `str(value)` return something reasonable here?
Actually, the most important thing is the type here, so this was quite 
deliberate.

GetDescription returns `(void *) unused1 = \n\n`, but I am not sure if we can safely require that all future 
versions/platforms optimize the parameter out.


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

https://reviews.llvm.org/D110571

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

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

2021-10-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

ping @JDevlieghere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108953

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

[Lldb-commits] [PATCH] D107585: [lldb/Plugins] Add support for ScriptedThread in ScriptedProcess

2021-10-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 376507.
mib added a comment.

Pass `__PRETTY_FUNCTION__` to `CheckStructuredDataObject` to log the caller 
function name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107585

Files:
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/Process/scripted/CMakeLists.txt
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -223,6 +223,12 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSwigPythonCreateScriptedThread(
+const char *python_class_name, const char *session_dictionary_name,
+const lldb::TargetSP &target_sp, std::string &error_string) {
+  return nullptr;
+}
+
 extern "C" void *
 LLDBSWIGPython_CreateFrameRecognizer(const char *python_class_name,
  const char *session_dictionary_name) {
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -72,6 +72,28 @@
 self.assertTrue(error.Success(), "Failed to read memory from scripted process.")
 self.assertEqual(hello_world, memory_read)
 
+self.assertEqual(process.GetNumThreads(), 1)
+
+thread = process.GetSelectedThread()
+self.assertTrue(thread, "Invalid thread.")
+self.assertEqual(thread.GetThreadID(), 0x19)
+self.assertEqual(thread.GetName(), "MyScriptedThread.thread-1")
+self.assertEqual(thread.GetStopReason(), lldb.eStopReasonSignal)
+
+self.assertGreater(thread.GetNumFrames(), 0)
+
+frame = thread.GetFrameAtIndex(0)
+register_set = frame.registers # Returns an SBValueList.
+for regs in register_set:
+if 'GPR' in regs.name:
+registers  = regs
+break
+
+self.assertTrue(registers, "Invalid General Purpose Registers Set")
+self.assertEqual(registers.GetNumChildren(), 21)
+for idx, reg in enumerate(registers, start=1):
+self.assertEqual(idx, int(reg.value, 16))
+
 def test_launch_scripted_process_cli(self):
 """Test that we can launch an lldb scripted process from the command
 line, check its process ID and read string from memory."""
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
===
--- /dev/null
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
@@ -0,0 +1,48 @@
+//===-- ScriptedThreadPythonInterface.h *- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+#define LLDB_PLUGINS_SCRIPTINTERPRETER_PYTHON_SCRIPTEDTHREADPYTHONINTERFACE_H
+
+#include "lldb/Host/Config.h"
+
+#if LLDB_ENABLE_PYTHON
+
+#include "ScriptedPythonInterface.h"
+#include "lldb/Interpreter/ScriptedProcessInterface.h"
+
+namespace lldb_private {
+class ScriptedThreadPythonInterface : public ScriptedThreadInterface,
+  public ScriptedPythonInterfac

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

2021-10-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 376508.
mib added a comment.

Use `ScriptedInterface::ErrorWithMessage` helper function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108953

Files:
  lldb/bindings/interface/SBMemoryRegionInfo.i
  lldb/bindings/interface/SBMemoryRegionInfoList.i
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/main.stack-dump
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/include/lldb/API/SBMemoryRegionInfo.h
  lldb/include/lldb/API/SBMemoryRegionInfoList.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/main.c
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -166,6 +166,10 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo(void *data) {
+  return nullptr;
+}
+
 extern lldb::ValueObjectSP
 LLDBSWIGPython_GetValueObjectSPFromSBValue(void *data) {
   return nullptr;
Index: lldb/test/API/functionalities/scripted_process/main.c
===
--- lldb/test/API/functionalities/scripted_process/main.c
+++ lldb/test/API/functionalities/scripted_process/main.c
@@ -1,5 +1,8 @@
-#include 
-
-int main() {
-  return 0; // break here
+int bar(int i) {
+  int j = i * i;
+  return j; // break here
 }
+
+int foo(int i) { return bar(i); }
+
+int main() { return foo(42); }
Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -0,0 +1,90 @@
+import os,struct, signal
+
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class DummyScriptedProcess(ScriptedProcess):
+def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
+super().__init__(target, args)
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+return self.memory_regions[0]
+
+def get_thread_with_id(self, tid: int):
+return {}
+
+def get_registers_for_thread(self, tid: int):
+return {}
+
+def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+data = lldb.SBData().CreateDataFromCString(
+self.target.GetByteOrder(),
+self.target.GetCodeByteSize(),
+"Hello, world!")
+return data
+
+def get_loaded_images(self):
+return self.loaded_images
+
+def get_process_id(self) -> int:
+return 42
+
+def should_stop(self) -> bool:
+return True
+
+def is_alive(self) -> bool:
+return True
+
+def get_scripted_thread_plugin(self):
+return DummyScriptedThread.__module__ + "." + DummyScriptedThread.__name__
+
+
+class DummyScriptedThread(ScriptedThread):
+def __init__(self, target):
+super().__init__(target)
+
+def get_thread_id(self) -> int:
+return 0x19
+
+def get_name(self) -> str:
+return DummyScriptedThread.__name__ + ".thread-1"
+
+def get_state(self) -> int:
+return lldb.eStateStopped
+
+def get_stop_reason(self) -> Dict[str, Any]:
+return { "type": lldb.eStopReasonSignal, "data": {
+"signal": signal.SIGINT
+} }
+
+def get_stackframes(self):
+class ScriptedStackFrame:
+def __init__(idx, cfa, pc, symbol_ctx):
+self.idx = idx
+self.cfa = cfa
+self.pc = pc
+self.symbol_ctx = symbol_ctx
+
+
+symbol_ctx = lldb.SBSymbolContext()
+frame_zero = ScriptedStackFrame(0, 0x42424242, 0x500, symbol_ctx)
+self.frames.append(frame_zero)
+
+return self.frame_zero[0:0]
+
+def get_register_context(

[Lldb-commits] [PATCH] D108953: [lldb/Plugins] Add memory region support in ScriptedProcess

2021-10-01 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 376509.
mib added a comment.

Update error message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108953

Files:
  lldb/bindings/interface/SBMemoryRegionInfo.i
  lldb/bindings/interface/SBMemoryRegionInfoList.i
  lldb/bindings/python/python-wrapper.swig
  lldb/examples/python/scripted_process/main.stack-dump
  lldb/examples/python/scripted_process/my_scripted_process.py
  lldb/include/lldb/API/SBMemoryRegionInfo.h
  lldb/include/lldb/API/SBMemoryRegionInfoList.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/source/API/SBMemoryRegionInfo.cpp
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/main.c
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -166,6 +166,10 @@
   return nullptr;
 }
 
+extern "C" void *LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo(void *data) {
+  return nullptr;
+}
+
 extern lldb::ValueObjectSP
 LLDBSWIGPython_GetValueObjectSPFromSBValue(void *data) {
   return nullptr;
Index: lldb/test/API/functionalities/scripted_process/main.c
===
--- lldb/test/API/functionalities/scripted_process/main.c
+++ lldb/test/API/functionalities/scripted_process/main.c
@@ -1,5 +1,8 @@
-#include 
-
-int main() {
-  return 0; // break here
+int bar(int i) {
+  int j = i * i;
+  return j; // break here
 }
+
+int foo(int i) { return bar(i); }
+
+int main() { return foo(42); }
Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -0,0 +1,90 @@
+import os,struct, signal
+
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class DummyScriptedProcess(ScriptedProcess):
+def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
+super().__init__(target, args)
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+return self.memory_regions[0]
+
+def get_thread_with_id(self, tid: int):
+return {}
+
+def get_registers_for_thread(self, tid: int):
+return {}
+
+def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+data = lldb.SBData().CreateDataFromCString(
+self.target.GetByteOrder(),
+self.target.GetCodeByteSize(),
+"Hello, world!")
+return data
+
+def get_loaded_images(self):
+return self.loaded_images
+
+def get_process_id(self) -> int:
+return 42
+
+def should_stop(self) -> bool:
+return True
+
+def is_alive(self) -> bool:
+return True
+
+def get_scripted_thread_plugin(self):
+return DummyScriptedThread.__module__ + "." + DummyScriptedThread.__name__
+
+
+class DummyScriptedThread(ScriptedThread):
+def __init__(self, target):
+super().__init__(target)
+
+def get_thread_id(self) -> int:
+return 0x19
+
+def get_name(self) -> str:
+return DummyScriptedThread.__name__ + ".thread-1"
+
+def get_state(self) -> int:
+return lldb.eStateStopped
+
+def get_stop_reason(self) -> Dict[str, Any]:
+return { "type": lldb.eStopReasonSignal, "data": {
+"signal": signal.SIGINT
+} }
+
+def get_stackframes(self):
+class ScriptedStackFrame:
+def __init__(idx, cfa, pc, symbol_ctx):
+self.idx = idx
+self.cfa = cfa
+self.pc = pc
+self.symbol_ctx = symbol_ctx
+
+
+symbol_ctx = lldb.SBSymbolContext()
+frame_zero = ScriptedStackFrame(0, 0x42424242, 0x500, symbol_ctx)
+self.frames.append(frame_zero)
+
+return self.frame_zero[0:0]
+
+def get_register_context(self) -> str:
+return struct.

[Lldb-commits] [lldb] 633ac51 - [lldb] Simplify TestCompletion.py

2021-10-01 Thread Pavel Labath via lldb-commits


Author: Pavel Labath
Date: 2021-10-01T15:49:23+02:00
New Revision: 633ac5170996da6a80a2236bed99913b66ed1d27

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

LOG: [lldb] Simplify TestCompletion.py

Added: 


Modified: 
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index 2bba1bfcb7dd6..8332048313a9d 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -233,7 +233,7 @@ def test_target_symbols_add_shlib(self):
 
 def test_log_file(self):
 # Complete in our source directory which contains a 'main.cpp' file.
-src_dir =  os.path.dirname(os.path.realpath(__file__)) + '/'
+src_dir =  self.getSourceDir() + '/'
 self.complete_from_to('log enable lldb expr -f ' + src_dir,
   ['main.cpp'])
 



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

[Lldb-commits] [PATCH] D110942: [lldb] Move DynamicRegisterInfo to public Target library

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

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

Move DynamicRegisterInfo from the internal lldbPluginProcessUtility
library to the public lldbTarget library.  This is a prerequisite
towards ABI plugin changes that are going to pass DynamicRegisterInfo
parameters.


https://reviews.llvm.org/D110942

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/DynamicRegisterInfo.cpp
  lldb/unittests/Process/Utility/CMakeLists.txt
  lldb/unittests/Process/Utility/DynamicRegisterInfoTest.cpp
  lldb/unittests/Target/CMakeLists.txt
  lldb/unittests/Target/DynamicRegisterInfoTest.cpp

Index: lldb/unittests/Target/DynamicRegisterInfoTest.cpp
===
--- lldb/unittests/Target/DynamicRegisterInfoTest.cpp
+++ lldb/unittests/Target/DynamicRegisterInfoTest.cpp
@@ -9,8 +9,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
-#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
-
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
Index: lldb/unittests/Target/CMakeLists.txt
===
--- lldb/unittests/Target/CMakeLists.txt
+++ lldb/unittests/Target/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(TargetTests
   ABITest.cpp
+  DynamicRegisterInfoTest.cpp
   ExecutionContextTest.cpp
   MemoryRegionInfoTest.cpp
   ModuleCacheTest.cpp
Index: lldb/unittests/Process/Utility/CMakeLists.txt
===
--- lldb/unittests/Process/Utility/CMakeLists.txt
+++ lldb/unittests/Process/Utility/CMakeLists.txt
@@ -15,7 +15,6 @@
   ${NETBSD_SOURCES})
 
 add_lldb_unittest(ProcessUtilityTests
-  DynamicRegisterInfoTest.cpp
   LinuxProcMapsTest.cpp
   MemoryTagManagerAArch64MTETest.cpp
   RegisterContextTest.cpp
Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -6,8 +6,7 @@
 //
 //===--===//
 
-#include "DynamicRegisterInfo.h"
-
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Interpreter/OptionArgParser.h"
Index: lldb/source/Target/CMakeLists.txt
===
--- lldb/source/Target/CMakeLists.txt
+++ lldb/source/Target/CMakeLists.txt
@@ -9,6 +9,7 @@
 add_lldb_library(lldbTarget
   ABI.cpp
   AssertFrameRecognizer.cpp
+  DynamicRegisterInfo.cpp
   ExecutionContext.cpp
   InstrumentationRuntime.cpp
   InstrumentationRuntimeStopInfo.cpp
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
@@ -11,7 +11,7 @@
 
 #include 
 
-#include "Plugins/Process/Utility/DynamicRegisterInfo.h"
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataExtractor.h"
Index: lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
===
--- lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
+++ lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
@@ -11,17 +11,16 @@
 
 #include 
 
+#include "lldb/Target/DynamicRegisterInfo.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/lldb-private.h"
 
-class DynamicRegisterInfo;
-
 class RegisterContextMemory : public lldb_private::RegisterContext {
 public:
   RegisterContextMemory(lldb_private::Thread &thread,
 uint32_t concrete_frame_idx,
-DynamicRegisterInfo ®_info,
+lldb_private::DynamicRegisterInfo ®_info,
 lldb::addr_t reg_data_addr);
 
   ~RegisterContextMemory() override;
@@ -60,7 +59,7 @@
 protected:
   void SetAllRegisterValid(bool b);
 
-  DynamicRegisterInfo &m_reg_infos;
+  lldb_private::DynamicRegisterInfo &m_

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

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 376549.
mgorny retitled this revision from "[lldb] [ABI/AArch64] Add pseudo-regs if 
missing [WIP]" to "[lldb] [ABI/AArch64] Add pseudo-regs if missing".
mgorny added a reviewer: teemperor.
mgorny added a comment.

Update for DynamicRegisterInfo move.


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

https://reviews.llvm.org/D109876

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

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

[Lldb-commits] [lldb] bd21257 - [lldb] [Host] Fix flipped logic in TerminalState::Save()

2021-10-01 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-10-01T18:23:54+02:00
New Revision: bd21257bf5af211c4d269ddbec0911c76c3b6120

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

LOG: [lldb] [Host] Fix flipped logic in TerminalState::Save()

Added: 


Modified: 
lldb/source/Host/common/Terminal.cpp

Removed: 




diff  --git a/lldb/source/Host/common/Terminal.cpp 
b/lldb/source/Host/common/Terminal.cpp
index 6adcfa6a92c4..2be9a6d6b0ec 100644
--- a/lldb/source/Host/common/Terminal.cpp
+++ b/lldb/source/Host/common/Terminal.cpp
@@ -110,7 +110,7 @@ bool TerminalState::Save(Terminal term, bool 
save_process_group) {
 m_tflags = ::fcntl(fd, F_GETFL, 0);
 #if LLDB_ENABLE_TERMIOS
 std::unique_ptr new_data{new Data()};
-if (::tcgetattr(fd, &new_data->m_termios) != 0)
+if (::tcgetattr(fd, &new_data->m_termios) == 0)
   m_data = std::move(new_data);
 #endif // LLDB_ENABLE_TERMIOS
 if (save_process_group)



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

[Lldb-commits] [lldb] a7b4ce9 - [NFC][AttributeList] Replace index_begin/end with an iterator

2021-10-01 Thread Arthur Eubanks via lldb-commits


Author: Arthur Eubanks
Date: 2021-10-01T10:17:41-07:00
New Revision: a7b4ce9cfd110db56d89fa8d76b39023d038de11

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

LOG: [NFC][AttributeList] Replace index_begin/end with an iterator

We expose the fact that we rely on unsigned wrapping to iterate through
all indexes. This can be confusing. Rather, keeping it as an
implementation detail through an iterator is less confusing and is less
code.

Reviewed By: rnk

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

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
llvm/include/llvm/IR/Attributes.h
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
llvm/lib/IR/Attributes.cpp
llvm/lib/Transforms/Utils/FunctionComparator.cpp
llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
index edfadc94debea..bb09b5e171f00 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
@@ -237,8 +237,7 @@ bool fixupRSAllocationStructByValCalls(llvm::Module 
&module) {
 llvm::AttributeList call_attribs = call_inst->getAttributes();
 
 // iterate over the argument attributes
-for (unsigned I = call_attribs.index_begin(); I != 
call_attribs.index_end();
- I++) {
+for (unsigned I : call_attribs.indexes()) {
   // if this argument is passed by val
   if (call_attribs.hasAttributeAtIndex(I, llvm::Attribute::ByVal)) {
 // strip away the byval attribute

diff  --git a/llvm/include/llvm/IR/Attributes.h 
b/llvm/include/llvm/IR/Attributes.h
index 24588faf6ffa5..6e830b3ebbbe2 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -851,9 +851,32 @@ class AttributeList {
 
   unsigned getNumAttrSets() const;
 
-  /// Use these to iterate over the valid attribute indices.
-  unsigned index_begin() const { return AttributeList::FunctionIndex; }
-  unsigned index_end() const { return getNumAttrSets() - 1; }
+  // Implementation of indexes(). Produces iterators that wrap an index. Mostly
+  // to hide the awkwardness of unsigned wrapping when iterating over valid
+  // indexes.
+  struct index_iterator {
+unsigned NumAttrSets;
+index_iterator(int NumAttrSets) : NumAttrSets(NumAttrSets) {}
+struct int_wrapper {
+  int_wrapper(unsigned i) : i(i) {}
+  unsigned i;
+  unsigned operator*() { return i; }
+  bool operator!=(const int_wrapper &Other) { return i != Other.i; }
+  int_wrapper &operator++() {
+// This is expected to undergo unsigned wrapping since FunctionIndex is
+// ~0 and that's where we start.
+++i;
+return *this;
+  }
+};
+
+int_wrapper begin() { return int_wrapper(AttributeList::FunctionIndex); }
+
+int_wrapper end() { return int_wrapper(NumAttrSets - 1); }
+  };
+
+  /// Use this to iterate over the valid attribute indexes.
+  index_iterator indexes() const { return index_iterator(getNumAttrSets()); }
 
   /// operator==/!= - Provide equality predicates.
   bool operator==(const AttributeList &RHS) const { return pImpl == RHS.pImpl; 
}

diff  --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp 
b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index a915d478890ef..23d6132f185f9 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -835,7 +835,7 @@ void ModuleBitcodeWriter::writeAttributeTable() {
   SmallVector Record;
   for (unsigned i = 0, e = Attrs.size(); i != e; ++i) {
 AttributeList AL = Attrs[i];
-for (unsigned i = AL.index_begin(), e = AL.index_end(); i != e; ++i) {
+for (unsigned i : AL.indexes()) {
   AttributeSet AS = AL.getAttributes(i);
   if (AS.hasAttributes())
 Record.push_back(VE.getAttributeGroupID({i, AS}));

diff  --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp 
b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index 0c6e7462e1242..9465a3b11c8fe 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -1039,7 +1039,7 @@ void ValueEnumerator::EnumerateAttributes(AttributeList 
PAL) {
   }
 
   // Do lookups for all attribute groups.
-  for (unsigned i = PAL.index_begin(), e = PAL.index_end(); i != e; ++i) {
+  for (unsigned i : PAL.indexes()) {
 AttributeSet AS = PAL.getAttributes(i);
 if (!AS.hasAttri

[Lldb-commits] [PATCH] D110885: [NFC][AttributeList] Replace index_begin/end with an iterator

2021-10-01 Thread Arthur Eubanks via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7b4ce9cfd11: [NFC][AttributeList] Replace index_begin/end 
with an iterator (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110885

Files:
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp

Index: llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
===
--- llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
+++ llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp
@@ -84,8 +84,7 @@
   AttrPtrVecVecTy &AttributeSetsToPreserve) {
 assert(AttributeSetsToPreserve.empty() && "Should not be sharing vectors.");
 AttributeSetsToPreserve.reserve(AL.getNumAttrSets());
-for (unsigned SetIdx = AL.index_begin(), SetEndIdx = AL.index_end();
- SetIdx != SetEndIdx; ++SetIdx) {
+for (unsigned SetIdx : AL.indexes()) {
   AttrPtrIdxVecVecTy AttributesToPreserve;
   AttributesToPreserve.first = SetIdx;
   visitAttributeSet(AL.getAttributes(AttributesToPreserve.first),
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -110,7 +110,7 @@
   if (int Res = cmpNumbers(L.getNumAttrSets(), R.getNumAttrSets()))
 return Res;
 
-  for (unsigned i = L.index_begin(), e = L.index_end(); i != e; ++i) {
+  for (unsigned i : L.indexes()) {
 AttributeSet LAS = L.getAttributes(i);
 AttributeSet RAS = R.getAttributes(i);
 AttributeSet::iterator LI = LAS.begin(), LE = LAS.end();
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -1527,7 +1527,7 @@
 void AttributeList::print(raw_ostream &O) const {
   O << "AttributeList[\n";
 
-  for (unsigned i = index_begin(), e = index_end(); i != e; ++i) {
+  for (unsigned i : indexes()) {
 if (!getAttributes(i).hasAttributes())
   continue;
 O << "  { ";
Index: llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
===
--- llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -1039,7 +1039,7 @@
   }
 
   // Do lookups for all attribute groups.
-  for (unsigned i = PAL.index_begin(), e = PAL.index_end(); i != e; ++i) {
+  for (unsigned i : PAL.indexes()) {
 AttributeSet AS = PAL.getAttributes(i);
 if (!AS.hasAttributes())
   continue;
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -835,7 +835,7 @@
   SmallVector Record;
   for (unsigned i = 0, e = Attrs.size(); i != e; ++i) {
 AttributeList AL = Attrs[i];
-for (unsigned i = AL.index_begin(), e = AL.index_end(); i != e; ++i) {
+for (unsigned i : AL.indexes()) {
   AttributeSet AS = AL.getAttributes(i);
   if (AS.hasAttributes())
 Record.push_back(VE.getAttributeGroupID({i, AS}));
Index: llvm/include/llvm/IR/Attributes.h
===
--- llvm/include/llvm/IR/Attributes.h
+++ llvm/include/llvm/IR/Attributes.h
@@ -851,9 +851,32 @@
 
   unsigned getNumAttrSets() const;
 
-  /// Use these to iterate over the valid attribute indices.
-  unsigned index_begin() const { return AttributeList::FunctionIndex; }
-  unsigned index_end() const { return getNumAttrSets() - 1; }
+  // Implementation of indexes(). Produces iterators that wrap an index. Mostly
+  // to hide the awkwardness of unsigned wrapping when iterating over valid
+  // indexes.
+  struct index_iterator {
+unsigned NumAttrSets;
+index_iterator(int NumAttrSets) : NumAttrSets(NumAttrSets) {}
+struct int_wrapper {
+  int_wrapper(unsigned i) : i(i) {}
+  unsigned i;
+  unsigned operator*() { return i; }
+  bool operator!=(const int_wrapper &Other) { return i != Other.i; }
+  int_wrapper &operator++() {
+// This is expected to undergo unsigned wrapping since FunctionIndex is
+// ~0 and that's where we start.
+++i;
+return *this;
+  }
+};
+
+int_wrapper begin() { return int_wrapper(AttributeList::FunctionIndex); }
+
+int_wrapper end() { return int_wrapper(NumAttrSets - 1); }
+  };
+
+  /// Use this to iterate over the valid attribute indexes.
+  ind

[Lldb-commits] [lldb] f853789 - [lldb] [Host] Sync TerminalState::Data to struct type

2021-10-01 Thread Michał Górny via lldb-commits


Author: Michał Górny
Date: 2021-10-01T20:34:11+02:00
New Revision: f8537895b9a63caec31a02b90b82fb367b852ae8

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

LOG: [lldb] [Host] Sync TerminalState::Data to struct type

Added: 


Modified: 
lldb/include/lldb/Host/Terminal.h

Removed: 




diff  --git a/lldb/include/lldb/Host/Terminal.h 
b/lldb/include/lldb/Host/Terminal.h
index 081a427fdb50..ba70acf720d2 100644
--- a/lldb/include/lldb/Host/Terminal.h
+++ b/lldb/include/lldb/Host/Terminal.h
@@ -45,7 +45,7 @@ class Terminal {
 /// This class can be used to remember the terminal state for a file
 /// descriptor and later restore that state as it originally was.
 class TerminalState {
-  class Data;
+  struct Data;
 
 public:
   /// Construct a new instance and optionally save terminal state.



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

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

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

https://reviews.llvm.org/D110962

Files:
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/TerminalTest.cpp

Index: lldb/unittests/Host/TerminalTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/TerminalTest.cpp
@@ -0,0 +1,135 @@
+//===-- TerminalTest.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 "lldb/Host/Terminal.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#if defined(__linux__)
+#include 
+#elif defined(__FreeBSD__)
+#include 
+#else // Darwin, NetBSD, OpenBSD
+#include 
+#endif
+#include 
+#include 
+
+using namespace lldb_private;
+
+class TerminalTest : public ::testing::Test {
+protected:
+  int m_master;
+  int m_slave;
+
+  void SetUp() override {
+ASSERT_EQ(openpty(&m_master, &m_slave, nullptr, nullptr, nullptr), 0);
+  }
+
+  void TearDown() override {
+close(m_slave);
+close(m_master);
+  }
+};
+
+TEST_F(TerminalTest, PtyIsATerminal) {
+  Terminal term{m_slave};
+  EXPECT_EQ(term.IsATerminal(), true);
+}
+
+TEST_F(TerminalTest, PipeIsNotATerminal) {
+  int pipefd[2];
+  ASSERT_EQ(pipe(pipefd), 0);
+  Terminal pipeterm{pipefd[0]};
+  EXPECT_EQ(pipeterm.IsATerminal(), false);
+  close(pipefd[0]);
+  close(pipefd[1]);
+}
+
+TEST_F(TerminalTest, SetEcho) {
+  struct termios terminfo;
+  Terminal term{m_slave};
+
+  ASSERT_EQ(term.SetEcho(true), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_NE(terminfo.c_lflag & ECHO, 0U);
+
+  ASSERT_EQ(term.SetEcho(false), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_EQ(terminfo.c_lflag & ECHO, 0U);
+}
+
+TEST_F(TerminalTest, SetCanonical) {
+  struct termios terminfo;
+  Terminal term{m_slave};
+
+  ASSERT_EQ(term.SetCanonical(true), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_NE(terminfo.c_lflag & ICANON, 0U);
+
+  ASSERT_EQ(term.SetCanonical(false), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
+}
+
+TEST_F(TerminalTest, SaveRestoreRAII) {
+  struct termios orig_terminfo;
+  struct termios terminfo;
+  ASSERT_EQ(tcgetattr(m_slave, &orig_terminfo), 0);
+
+  Terminal term{m_slave};
+
+  {
+TerminalState term_state{term};
+terminfo = orig_terminfo;
+
+// make some arbitrary changes
+terminfo.c_iflag ^= IGNPAR | INLCR;
+terminfo.c_oflag ^= OPOST | OCRNL;
+terminfo.c_cflag ^= PARENB | PARODD;
+terminfo.c_lflag ^= ICANON | ECHO;
+terminfo.c_cc[VEOF] ^= 8;
+terminfo.c_cc[VEOL] ^= 4;
+cfsetispeed(&terminfo, B9600);
+cfsetospeed(&terminfo, B9600);
+
+ASSERT_EQ(tcsetattr(m_slave, TCSANOW, &terminfo), 0);
+  }
+
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  ASSERT_EQ(memcmp(&terminfo, &orig_terminfo, sizeof(terminfo)), 0);
+}
+
+TEST_F(TerminalTest, SaveRestore) {
+  TerminalState term_state;
+
+  struct termios orig_terminfo;
+  struct termios terminfo;
+  ASSERT_EQ(tcgetattr(m_slave, &orig_terminfo), 0);
+
+  Terminal term{m_slave};
+  term_state.Save(term, false);
+  terminfo = orig_terminfo;
+
+  // make some arbitrary changes
+  terminfo.c_iflag ^= IGNPAR | INLCR;
+  terminfo.c_oflag ^= OPOST | OCRNL;
+  terminfo.c_cflag ^= PARENB | PARODD;
+  terminfo.c_lflag ^= ICANON | ECHO;
+  terminfo.c_cc[VEOF] ^= 8;
+  terminfo.c_cc[VEOL] ^= 4;
+  cfsetispeed(&terminfo, B9600);
+  cfsetospeed(&terminfo, B9600);
+
+  ASSERT_EQ(tcsetattr(m_slave, TCSANOW, &terminfo), 0);
+
+  term_state.Restore();
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  ASSERT_EQ(memcmp(&terminfo, &orig_terminfo, sizeof(terminfo)), 0);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -14,6 +14,7 @@
   SocketTestUtilities.cpp
   XMLTest.cpp
 )
+set (EXTRA_LIBS)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   list(APPEND FILES
@@ -22,10 +23,25 @@
   )
 endif()
 
+# TODO: support more systems
+if (LLDB_ENABLE_TERMIOS AND
+CMAKE_SYSTEM_NAME MATCHES "Darwin|FreeBSD|Linux|NetBSD|OpenBSD")
+  list(APPEND FILES
+TerminalTest.cpp
+  )
+
+  if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
+list(APPEND EXTRA_LIBS
+  util
+)
+  endif()
+endif()
+
 add_lldb_unittest(HostTests
   ${FILES}
   LINK_LIBS
 lldbHost
 lldbUtilityHelpers
 LLVMTestingSupport
+${EXTRA_LIBS}
   )
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/lis

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny updated this revision to Diff 376607.
mgorny added a comment.

Remove obsolete TODO.


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

https://reviews.llvm.org/D110962

Files:
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/TerminalTest.cpp

Index: lldb/unittests/Host/TerminalTest.cpp
===
--- /dev/null
+++ lldb/unittests/Host/TerminalTest.cpp
@@ -0,0 +1,135 @@
+//===-- TerminalTest.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 "lldb/Host/Terminal.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#if defined(__linux__)
+#include 
+#elif defined(__FreeBSD__)
+#include 
+#else // Darwin, NetBSD, OpenBSD
+#include 
+#endif
+#include 
+#include 
+
+using namespace lldb_private;
+
+class TerminalTest : public ::testing::Test {
+protected:
+  int m_master;
+  int m_slave;
+
+  void SetUp() override {
+ASSERT_EQ(openpty(&m_master, &m_slave, nullptr, nullptr, nullptr), 0);
+  }
+
+  void TearDown() override {
+close(m_slave);
+close(m_master);
+  }
+};
+
+TEST_F(TerminalTest, PtyIsATerminal) {
+  Terminal term{m_slave};
+  EXPECT_EQ(term.IsATerminal(), true);
+}
+
+TEST_F(TerminalTest, PipeIsNotATerminal) {
+  int pipefd[2];
+  ASSERT_EQ(pipe(pipefd), 0);
+  Terminal pipeterm{pipefd[0]};
+  EXPECT_EQ(pipeterm.IsATerminal(), false);
+  close(pipefd[0]);
+  close(pipefd[1]);
+}
+
+TEST_F(TerminalTest, SetEcho) {
+  struct termios terminfo;
+  Terminal term{m_slave};
+
+  ASSERT_EQ(term.SetEcho(true), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_NE(terminfo.c_lflag & ECHO, 0U);
+
+  ASSERT_EQ(term.SetEcho(false), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_EQ(terminfo.c_lflag & ECHO, 0U);
+}
+
+TEST_F(TerminalTest, SetCanonical) {
+  struct termios terminfo;
+  Terminal term{m_slave};
+
+  ASSERT_EQ(term.SetCanonical(true), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_NE(terminfo.c_lflag & ICANON, 0U);
+
+  ASSERT_EQ(term.SetCanonical(false), true);
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
+}
+
+TEST_F(TerminalTest, SaveRestoreRAII) {
+  struct termios orig_terminfo;
+  struct termios terminfo;
+  ASSERT_EQ(tcgetattr(m_slave, &orig_terminfo), 0);
+
+  Terminal term{m_slave};
+
+  {
+TerminalState term_state{term};
+terminfo = orig_terminfo;
+
+// make some arbitrary changes
+terminfo.c_iflag ^= IGNPAR | INLCR;
+terminfo.c_oflag ^= OPOST | OCRNL;
+terminfo.c_cflag ^= PARENB | PARODD;
+terminfo.c_lflag ^= ICANON | ECHO;
+terminfo.c_cc[VEOF] ^= 8;
+terminfo.c_cc[VEOL] ^= 4;
+cfsetispeed(&terminfo, B9600);
+cfsetospeed(&terminfo, B9600);
+
+ASSERT_EQ(tcsetattr(m_slave, TCSANOW, &terminfo), 0);
+  }
+
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  ASSERT_EQ(memcmp(&terminfo, &orig_terminfo, sizeof(terminfo)), 0);
+}
+
+TEST_F(TerminalTest, SaveRestore) {
+  TerminalState term_state;
+
+  struct termios orig_terminfo;
+  struct termios terminfo;
+  ASSERT_EQ(tcgetattr(m_slave, &orig_terminfo), 0);
+
+  Terminal term{m_slave};
+  term_state.Save(term, false);
+  terminfo = orig_terminfo;
+
+  // make some arbitrary changes
+  terminfo.c_iflag ^= IGNPAR | INLCR;
+  terminfo.c_oflag ^= OPOST | OCRNL;
+  terminfo.c_cflag ^= PARENB | PARODD;
+  terminfo.c_lflag ^= ICANON | ECHO;
+  terminfo.c_cc[VEOF] ^= 8;
+  terminfo.c_cc[VEOL] ^= 4;
+  cfsetispeed(&terminfo, B9600);
+  cfsetospeed(&terminfo, B9600);
+
+  ASSERT_EQ(tcsetattr(m_slave, TCSANOW, &terminfo), 0);
+
+  term_state.Restore();
+  ASSERT_EQ(tcgetattr(m_slave, &terminfo), 0);
+  ASSERT_EQ(memcmp(&terminfo, &orig_terminfo, sizeof(terminfo)), 0);
+}
Index: lldb/unittests/Host/CMakeLists.txt
===
--- lldb/unittests/Host/CMakeLists.txt
+++ lldb/unittests/Host/CMakeLists.txt
@@ -14,6 +14,7 @@
   SocketTestUtilities.cpp
   XMLTest.cpp
 )
+set (EXTRA_LIBS)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   list(APPEND FILES
@@ -22,10 +23,24 @@
   )
 endif()
 
+if (LLDB_ENABLE_TERMIOS AND
+CMAKE_SYSTEM_NAME MATCHES "Darwin|FreeBSD|Linux|NetBSD|OpenBSD")
+  list(APPEND FILES
+TerminalTest.cpp
+  )
+
+  if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
+list(APPEND EXTRA_LIBS
+  util
+)
+  endif()
+endif()
+
 add_lldb_unittest(HostTests
   ${FILES}
   LINK_LIBS
 lldbHost
 lldbUtilityHelpers
 LLVMTestingSupport
+${EXTRA_LIBS}
   )
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

2021-10-01 Thread Michał Górny via Phabricator via lldb-commits

mgorny added inline comments.



Comment at: lldb/unittests/Host/CMakeLists.txt:27
+if (LLDB_ENABLE_TERMIOS AND
+CMAKE_SYSTEM_NAME MATCHES "Darwin|FreeBSD|Linux|NetBSD|OpenBSD")
+  list(APPEND FILES

FTR, I've tested it on FreeBSD and Linux so far. NetBSD in progress. Darwin and 
OpenBSD support is based on what I've found in gnulib but explicit testing 
would be helpful.


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

https://reviews.llvm.org/D110962

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

[Lldb-commits] [PATCH] D110893: [lldb] Refactor statistics (NFC)

2021-10-01 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

In D110893#3035753 , @labath wrote:

> Some general thoughts:
>
> - instead of throwing patches around in might be good to have a discussion 
> thread to determine all the requirements and figure out the general direction

Got to agree with Pavel here :-)

[Sorry, I missed the context in https://reviews.llvm.org/D110804, I should read 
through my reviews in chronological order.]


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

https://reviews.llvm.org/D110893

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

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

2021-10-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Thanks Greg! Following Pavel's suggestion in D110893 
, could you split up this patch into smaller 
ones? I think that will make it a lot easier to review and discuss.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110804

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

[Lldb-commits] [PATCH] D110981: [lldb] Improve help for platform put-file

2021-10-01 Thread Keith Smiley via Phabricator via lldb-commits

keith created this revision.
keith added a reviewer: kastiglione.
keith requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Herald added a subscriber: JDevlieghere.

Previously it was not clear what arguments this required, or what it would do 
if you didn't pass the destination argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110981

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp


Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1067,7 +1067,16 @@
   CommandObjectPlatformPutFile(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "platform put-file",
-"Transfer a file from this system to the remote end.", nullptr, 0) 
{
+"Transfer a file from this system to the remote end.",
+"platform put-file  []", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform put-file /source/foo.txt /destination/bar.txt
+
+(lldb) platform put-file /source/foo.txt
+
+Omitting the destination places the file in the debuggers current working 
directory.)");
   }
 
   ~CommandObjectPlatformPutFile() override = default;


Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1067,7 +1067,16 @@
   CommandObjectPlatformPutFile(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "platform put-file",
-"Transfer a file from this system to the remote end.", nullptr, 0) {
+"Transfer a file from this system to the remote end.",
+"platform put-file  []", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform put-file /source/foo.txt /destination/bar.txt
+
+(lldb) platform put-file /source/foo.txt
+
+Omitting the destination places the file in the debuggers current working directory.)");
   }
 
   ~CommandObjectPlatformPutFile() override = default;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

42 matches

Mail list logo