[lldb] Add serial:// protocol for connecting to serial port

[mgorny]

Summary

Add a new serial:// protocol along with SerialPort that provides a new
API to open serial ports. The URL consists of serial device path
followed by URL-style options, e.g.:

serial:///dev/ttyS0?baud=115200&parity=even

If no options are provided, the serial port is only set to raw mode
and the other attributes remain unchanged. Attributes provided via
options are modified to the specified values. Upon closing the serial
port, its original attributes are restored.

[labath]

This looks reasonable to me. In case the url does not specify port parameters, it might be better to set them to known sane defaults instead of leaving them unchanged. That way lldb and lldb-server could "just connect". However, I don't know much about serial ports so I'm leaving that up to you.

[mgorny]

I don't really have a strong opinion about this. I think we should leave the option of leaving port settings unchanged. I'll check what gdb does, as I suppose interoperability with gdbserver is also useful. I guess people are more likely to connect to other servers than lldb over serial port after all.

[mgorny]

Move serial:// URL parsing to SerialPort class. Improve error handling.

[labath]

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

790–792

It would be better to hide this by making the constructor private and have a static factory function returning Expected<unique_ptr<SerialPort>>. Bonus points if you can move all the fallible operations into the factory function so that the (now private) constructor does not need the Error argument at all

[mgorny]

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

790–792

I know but I couldn't make this work — the compiler just won't find a way to convert the new SerialPort instance into Expected<SerialPort>. I suspect I would have to add move ctors and assignment operators all the way up the class hierarchy.

[labath]

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

790–792

That's where the unique_ptr part comes in. :)

[mgorny]

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

790–792

I presume you mean shared_ptr here, or know some dirty trick.

[mgorny]

Use a fallible ctor approach.

[labath]

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

790–792

a unique_ptr is convertible to a shared_ptr, but not the other way around, so it's better to return the former

[mgorny]

Use unique_ptr.

[mgorny]

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

790–792

Ah, indeed. I was missing std::move.

[labath]

lldb/source/Host/common/File.cpp

847

Empty line here

lldb/include/lldb/Host/File.h

 //===-- File.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_HOST_FILE_H
 #define LLDB_HOST_FILE_H
 
 #include "lldb/Host/PosixApi.h"
+#include "lldb/Host/Terminal.h"
 #include "lldb/Utility/IOObject.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
 #include "llvm/ADT/BitmaskEnum.h"
 
 #include <cstdarg>
 #include <cstdio>
 #include <mutex>
protected:
   bool m_own_stream = false;
   std::mutex offset_access_mutex;
 
 private:
   NativeFile(const NativeFile &) = delete;
   const NativeFile &operator=(const NativeFile &) = delete;
 };
 
+class SerialPort : public NativeFile {
+public:
+  struct Options {
+    llvm::Optional<unsigned int> BaudRate = llvm::None;
+    llvm::Optional<TerminalParity> Parity = llvm::None;
+    llvm::Optional<unsigned int> StopBits = llvm::None;
+  };
+
+  SerialPort() {}
+  SerialPort(int fd, OpenOptions options, Options serial_options,
+             bool transfer_ownership);
+
+  bool IsValid() const override {
+    return NativeFile::IsValid() && m_is_interactive == eLazyBoolYes;
+  }
+
+  Status Close() override;
+
+  static char ID;
+  virtual bool isA(const void *classID) const override {
+    return classID == &ID || File::isA(classID);
+  }
+  static bool classof(const File *file) { return file->isA(&ID); }
+
+private:
+  SerialPort(const SerialPort &) = delete;
+  const SerialPort &operator=(const SerialPort &) = delete;
+
+  TerminalState m_state;
+};
+
 } // namespace lldb_private
 
 #endif // LLDB_HOST_FILE_H

lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h

protected:
 
   lldb::ConnectionStatus UnixAbstractSocketConnect(llvm::StringRef socket_name,
                                                    Status *error_ptr);
 
   lldb::ConnectionStatus ConnectFD(llvm::StringRef args, Status *error_ptr);
 
   lldb::ConnectionStatus ConnectFile(llvm::StringRef args, Status *error_ptr);
 
+  lldb::ConnectionStatus ConnectSerialPort(llvm::StringRef args,
+                                           Status *error_ptr);
+
   lldb::IOObjectSP m_read_sp;
   lldb::IOObjectSP m_write_sp;
 
   Predicate<uint16_t>
       m_port_predicate; // Used when binding to port zero to wait for the thread
                         // that creates the socket, binds and listens to
                         // resolve the port number.
 

lldb/source/Host/common/File.cpp

mode_t File::ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options) {
   if (open_options & eOpenOptionCanCreateNewOnly)
     mode |= O_CREAT | O_EXCL;
   else if (open_options & eOpenOptionCanCreate)
     mode |= O_CREAT;
 
   return mode;
 }
 
+SerialPort::SerialPort(int fd, OpenOptions options,
+                       SerialPort::Options serial_options,
+                       bool transfer_ownership)
+    : NativeFile(fd, options, transfer_ownership), m_state(fd) {
+  if (GetIsInteractive()) {
+    Terminal term{fd};
+    // TODO: error handling?
+    term.SetRaw();
+    if (serial_options.BaudRate)
+      term.SetBaudRate(serial_options.BaudRate.getValue());
+    if (serial_options.Parity)
+      term.SetParity(serial_options.Parity.getValue());
+    if (serial_options.StopBits)
+      term.SetStopBits(serial_options.StopBits.getValue());
+  }
+}
+
+Status SerialPort::Close() {
+  m_state.Restore();
+  return NativeFile::Close();
+}
+
 char File::ID = 0;
 char NativeFile::ID = 0;
+char SerialPort::ID = 0;

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

if (!path.empty()) {
               .Case("udp", &ConnectionFileDescriptor::ConnectUDP)
               .Case("unix-connect",
                     &ConnectionFileDescriptor::NamedSocketConnect)
               .Case("unix-abstract-connect",
                     &ConnectionFileDescriptor::UnixAbstractSocketConnect)
 #if LLDB_ENABLE_POSIX
               .Case("fd", &ConnectionFileDescriptor::ConnectFD)
               .Case("file", &ConnectionFileDescriptor::ConnectFile)
+              .Case("serial", &ConnectionFileDescriptor::ConnectSerialPort)
 #endif
               .Default(nullptr);
 
       if (method)
         return method(*this, path, error_ptr);
     }
 
     if (error_ptr)
#if LLDB_ENABLE_POSIX
   m_read_sp =
       std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, true);
   m_write_sp = m_read_sp;
   return eConnectionStatusSuccess;
 #endif // LLDB_ENABLE_POSIX
   llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
 }
 
+ConnectionStatus
+ConnectionFileDescriptor::ConnectSerialPort(llvm::StringRef s,
+                                            Status *error_ptr) {
+#if LLDB_ENABLE_POSIX
+  llvm::StringRef path, params;
+  // serial:///PATH?k1=v1&k2=v2...
+  std::tie(path, params) = s.split('?');
+  int fd = llvm::sys::RetryAfterSignal(-1, ::open, path.str().c_str(), O_RDWR);
+  if (fd == -1) {
+    if (error_ptr)
+      error_ptr->SetErrorToErrno();
+    return eConnectionStatusError;
+  }
+
+  int flags = ::fcntl(fd, F_GETFL, 0);
+  if (flags >= 0) {
+    if ((flags & O_NONBLOCK) == 0) {
+      flags |= O_NONBLOCK;
+      ::fcntl(fd, F_SETFL, flags);
+    }
+  }
+
+  SerialPort::Options serial_options;
+  for (auto x : llvm::Split(params, '&')) {
+    if (x.consume_front("baud=")) {
+      unsigned int baud_rate;
+      if (!llvm::to_integer(x, baud_rate, 10)) {
+        if (error_ptr)
+          error_ptr->SetErrorStringWithFormat("invalid baud rate: \"%s\"",
+                                              x.str().c_str());
+        return eConnectionStatusError;
+      }
+      serial_options.BaudRate = baud_rate;
+    } else if (x.consume_front("parity=")) {
+      serial_options.Parity =
+          llvm::StringSwitch<llvm::Optional<TerminalParity>>(x)
+              .Case("no", TerminalParity::No)
+              .Case("even", TerminalParity::Even)
+              .Case("odd", TerminalParity::Odd)
+              .Case("mark", TerminalParity::Mark)
+              .Case("space", TerminalParity::Space)
+              .Default(llvm::None);
+      if (!serial_options.Parity) {
+        if (error_ptr)
+          error_ptr->SetErrorStringWithFormat(
+              "invalid parity (must be no, even, odd, mark or space): \"%s\"",
+              x.str().c_str());
+        return eConnectionStatusError;
+      }
+    } else if (x.consume_front("stop-bits=")) {
+      unsigned int stop_bits;
+      if (!llvm::to_integer(x, stop_bits, 10) ||
+          (stop_bits != 1 && stop_bits != 2)) {
+        if (error_ptr)
+          error_ptr->SetErrorStringWithFormat(
+              "invalid stop bits (must be 1 or 2): \"%s\"", x.str().c_str());
+        return eConnectionStatusError;
+      }
+      serial_options.StopBits = stop_bits;
+    } else {
+      if (error_ptr)
+        error_ptr->SetErrorStringWithFormat("unknown parameter: \"%s\"",
+                                            x.str().c_str());
+      return eConnectionStatusError;
+    }
+  }
+
+  m_read_sp = std::make_shared<SerialPort>(fd, File::eOpenOptionReadWrite,
+                                           serial_options, true);
+  if (!m_read_sp->IsValid()) {
+    m_read_sp.reset();
+    if (error_ptr)
+      error_ptr->SetErrorStringWithFormat("path is not a tty: \"%s\"",
+                                          path.str().c_str());
+    return eConnectionStatusError;
+  }
+  m_write_sp = m_read_sp;
+
+  return eConnectionStatusSuccess;
+#endif // LLDB_ENABLE_POSIX
+  llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
+}
+
 uint16_t
 ConnectionFileDescriptor::GetListeningPort(const Timeout<std::micro> &timeout) {
   auto Result = m_port_predicate.WaitForValueNotEqualTo(0, timeout);
   return Result ? *Result : 0;
 }
 
 bool ConnectionFileDescriptor::GetChildProcessesInherit() const {
   return m_child_processes_inherit;