[Lldb-commits] [PATCH] D87333: [lldb/ipv6] Support running lldb tests in an ipv6-only environment.

Tue Sep 15 09:30:20 PDT 2020

rupprecht added a comment.

In D87333#2273506 <https://reviews.llvm.org/D87333#2273506>, @labath wrote:

> (Sorry about the delay.) Given the current requirements, I think this patch is fine (excellent even).
>
> That said, I'm not sure whether the original motivation for this requirement (avoiding dns lookups) is still relevant. These days, we communicate with the local debug server via `socketpair(2)` sockets (which wasn't the case back then), which does not require any dns lookups and is immune to a misconfigured hosts file.

The initial dns lookup may still fail, I think? e.g. my initial version of this patch had:

  if (llvm::Error error =
          listen_socket.Listen("localhost:0", backlog).ToError())

If the hosts file has a bad value for localhost, that will create a bad socket. That said, I don't know how to reproduce the original issue -- maybe it isn't an issue for some other reason.

> Given that none of the things touched by this patch is extremely critical (`GDBRemoteCommunication::ConnectLocally` is used for reproducers and the rest is test code), maybe we could use this to test the water and see whether we can start using the network stack the way it's supposed to be used ?

Many non-reproducers tests fail w/o the change to `GDBRemoteCommunication::ConnectLocally`, so it's at least used outside of that.

I can go either way with using `localhost` or using hard-coded localhost IPs. Whichever way we go, I have a slight preference for keeping the tests and code consistent, e.g. if we use hard-coded IPs in code linked into lldb to allow people with a bad hosts file to use lldb, we should use hard-coded IPs in the tests to allow those same users to develop on lldb.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87333