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

[Pavel Labath via llvm-commits]

You're welcome. I'm glad we could get to the bottom of this.

cheers.

On 10 February 2018 at 03:22, Alex Blewitt <alblue at apple.com> wrote:
> 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