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

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D87333#2274572 <https://reviews.llvm.org/D87333#2274572>, @rupprecht wrote:

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

Switched back to the original approach of using `localhost` when possible [1], though I still have the git branch so I can always switch back. I don't have a personal preference either way, but would like to land one of the two approaches as this creates a strange amount of test flakiness internally when we need to support ipv4.

[1] IIUC, when connecting via hostname, attempting "localhost" will try both AF_INET and AF_INET6, but there is no corresponding way to construct a raw socket with automatic fallback to AF_INET6 if AF_INET does not work, so that part is the same as before (try to create AF_INET and catch the error manually)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87333