[PATCH] D42845: Add an option 'allow-all-hosts' to permit lldb debugging inside a Docker container

[Alex Blewitt via llvm-commits]

> On 8 Feb 2018, at 02:35, Pavel Labath <labath at google.com> wrote:
> 
> Thanks for checking this. This confirms to me that it should be
> possible to make things "just work". Why it does not work is not clear
> to me, but I think we should figure that out. I made some experiments
> 
> Can you dig a bit deeper into why things are failing _without_ your
> patch. The two things I would check is:
> - is LLGS being passed the correct address on the command line (172.17.0.2)
> - does that address make it all the way to the address-checking code
> (TCPSocket::Accept)

So, running this again without the flag, I get the following inside the container:

lldb-server platform --server --listen *:4000 --min-gdbserver-port 4001 --max-gdbserver-port 4010
\- /usr/bin/lldb-server gdbserver tcp://172.17.0.1:4001 --native-regs

So it's passing an IP address that makes sense inside the container.

Looking at netstat inside the container:

# netstat -n | grep 4000
tcp        0      0 172.17.0.2:4000         172.17.0.1:39840        ESTABLISHED

and outside:

$ netstat -n | grep 400
tcp6       0      0  ::1.4001               ::1.60819              ESTABLISHED
tcp6       0      0  ::1.60819              ::1.4001               ESTABLISHED
tcp6       0      0  ::1.4000               ::1.60810              ESTABLISHED
tcp6       0      0  ::1.60810              ::1.4000               ESTABLISHED

I'm connecting via the symbolic name 'localhost' here, which Docker looks like it's helpfully translating the IPv6 to IPv4.

If I connect with connect://127.0.0.1 instead, it seems to do the sensible thing as well:

$ netstat -n | grep 400
tcp4       0      0  127.0.0.1.4001         127.0.0.1.60827        ESTABLISHED
tcp4       0      0  127.0.0.1.60827        127.0.0.1.4001         ESTABLISHED
tcp4       0      0  127.0.0.1.4000         127.0.0.1.60826        ESTABLISHED
tcp4       0      0  127.0.0.1.60826        127.0.0.1.4000         ESTABLISHED

# netstat -n | grep 400
tcp        0      0 172.17.0.2:4001         172.17.0.1:60814        ESTABLISHED
tcp        0      0 172.17.0.2:4000         172.17.0.1:39850        ESTABLISHED

However, if I run it as is, it seems to work as expected. So my patch now seems unnecessary :(

The only thing I can think of is that the original derivation of this patch was back in 2016, which would have been before the last change to that line in April 2017, so maybe there was a difference in behaviour of the code used before then, or that there has been some change in the Docker engine for the type of network it defaulted to at the time (which may have been bridge, but I don't have the proof of that) which prevented the getpeername from correctly identifying the code. I've been trying to reproduce the original issue based on what I recall using at the time, but I can't get it to fail in the same way.

So, I think the code works as expected and the patch is unnecessary any more. Thanks for your time in reviewing and making me recheck my assumptions!

Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180209/89919879/attachment.html>