Message ID | 1525458603-33351-4-git-send-email-brobecker@adacore.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 59841 invoked by alias); 4 May 2018 18:30:39 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 59316 invoked by uid 89); 4 May 2018 18:30:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=listening, our X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 04 May 2018 18:30:20 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CEA0A117C80 for <gdb-patches@sourceware.org>; Fri, 4 May 2018 14:30:06 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 3J7wM9u9iy8k for <gdb-patches@sourceware.org>; Fri, 4 May 2018 14:30:06 -0400 (EDT) Received: from tron.gnat.com (tron.gnat.com [IPv6:2620:20:4000:0:46a8:42ff:fe0e:e294]) by rock.gnat.com (Postfix) with ESMTP id C07C8117C7B for <gdb-patches@sourceware.org>; Fri, 4 May 2018 14:30:06 -0400 (EDT) Received: by tron.gnat.com (Postfix, from userid 4233) id BFB55487; Fri, 4 May 2018 14:30:06 -0400 (EDT) From: Joel Brobecker <brobecker@adacore.com> To: gdb-patches@sourceware.org Subject: [PATCH 3/3] gdbserver/Windows: crash during connection establishment phase Date: Fri, 4 May 2018 14:30:03 -0400 Message-Id: <1525458603-33351-4-git-send-email-brobecker@adacore.com> In-Reply-To: <1525458603-33351-1-git-send-email-brobecker@adacore.com> References: <1525458603-33351-1-git-send-email-brobecker@adacore.com> |
Commit Message
Joel Brobecker
May 4, 2018, 6:30 p.m. UTC
On Windows, starting a new process with GDBserver seeems to work, in the sense that the program does get started, and GDBserver confirms that it is listening for GDB to connect. However, as soon as GDB establishes the connection with GDBserver, and starts discussing with it, GDBserver crashes, with a SEGV. This SEGV occurs in remote-utils.c::prepare_resume_reply... | regp = current_target_desc ()->expedite_regs; | [...] | while (*regp) ... because, in our case, REGP is NULL. This patch fixes the problem for Windows targets based on Intel (x86 and x86_64). gdb/gdbserver/ChangeLog: * win32-i386-low.c (i386_arch_setup): set tdesc->expedite_regs. --- gdb/gdbserver/win32-i386-low.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Comments
On 05/04/2018 07:30 PM, Joel Brobecker wrote: > On Windows, starting a new process with GDBserver seeems to work, > in the sense that the program does get started, and GDBserver > confirms that it is listening for GDB to connect. However, as soon as > GDB establishes the connection with GDBserver, and starts discussing > with it, GDBserver crashes, with a SEGV. > > This SEGV occurs in remote-utils.c::prepare_resume_reply... > > | regp = current_target_desc ()->expedite_regs; > | [...] > | while (*regp) > > ... because, in our case, REGP is NULL. > > This patch fixes the problem for Windows targets based on Intel > (x86 and x86_64). > > gdb/gdbserver/ChangeLog: > > * win32-i386-low.c (i386_arch_setup): set tdesc->expedite_regs. > --- > gdb/gdbserver/win32-i386-low.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/gdb/gdbserver/win32-i386-low.c b/gdb/gdbserver/win32-i386-low.c > index a242f72..fdb95ce 100644 > --- a/gdb/gdbserver/win32-i386-low.c > +++ b/gdb/gdbserver/win32-i386-low.c > @@ -442,6 +442,16 @@ i386_arch_setup (void) > > init_target_desc (tdesc); > > +#ifndef IN_PROCESS_AGENT > +#ifdef __x86_64__ > + static const char *expedite_regs_amd64[] = { "rbp", "rsp", "rip", NULL }; > + tdesc->expedite_regs = expedite_regs_amd64; > +#else /* __x86_64__ */ > + static const char *expedite_regs_i386[] = { "ebp", "esp", "eip", NULL }; > + tdesc->expedite_regs = expedite_regs_i386; > +#endif /* __x86_64__ */ > +#endif Won't all x86 ports have the same problem? I.e., don't nto-x86-low.c:nto_x86_arch_setup and lynx-i386-low.c:lynx_i386_arch_setup need the same treatment? Should we put those arrays in some shared i386 file? Thanks, Pedro Alves
> > +#ifndef IN_PROCESS_AGENT > > +#ifdef __x86_64__ > > + static const char *expedite_regs_amd64[] = { "rbp", "rsp", "rip", NULL }; > > + tdesc->expedite_regs = expedite_regs_amd64; > > +#else /* __x86_64__ */ > > + static const char *expedite_regs_i386[] = { "ebp", "esp", "eip", NULL }; > > + tdesc->expedite_regs = expedite_regs_i386; > > +#endif /* __x86_64__ */ > > +#endif > > Won't all x86 ports have the same problem? I.e., don't > nto-x86-low.c:nto_x86_arch_setup and > lynx-i386-low.c:lynx_i386_arch_setup need the same treatment? > Should we put those arrays in some shared i386 file? They probably would, but these are platforms I cannot test at the moment, so I didn't want to take a risk and create a situation for them. I'm quite happy to adjust the way you suggest.
On 05/04/2018 07:58 PM, Joel Brobecker wrote: >>> +#ifndef IN_PROCESS_AGENT >>> +#ifdef __x86_64__ >>> + static const char *expedite_regs_amd64[] = { "rbp", "rsp", "rip", NULL }; >>> + tdesc->expedite_regs = expedite_regs_amd64; >>> +#else /* __x86_64__ */ >>> + static const char *expedite_regs_i386[] = { "ebp", "esp", "eip", NULL }; >>> + tdesc->expedite_regs = expedite_regs_i386; >>> +#endif /* __x86_64__ */ >>> +#endif >> >> Won't all x86 ports have the same problem? I.e., don't >> nto-x86-low.c:nto_x86_arch_setup and >> lynx-i386-low.c:lynx_i386_arch_setup need the same treatment? >> Should we put those arrays in some shared i386 file? > > They probably would, but these are platforms I cannot test at > the moment, so I didn't want to take a risk and create a situation > for them. I'm quite happy to adjust the way you suggest. I think I'd take the risk anyway, even without testing. (Just like the original patch that broke them didn't get testing on !Linux ports.) They're already most probably broken, so we can't be making it worse, IMO. Thanks, Pedro Alves
> From: Joel Brobecker <brobecker@adacore.com> > Date: Fri, 4 May 2018 14:30:03 -0400 > > +#ifndef IN_PROCESS_AGENT > +#ifdef __x86_64__ > + static const char *expedite_regs_amd64[] = { "rbp", "rsp", "rip", NULL }; > + tdesc->expedite_regs = expedite_regs_amd64; > +#else /* __x86_64__ */ I believe the comment here should say /* !__x86_64__ */ > + static const char *expedite_regs_i386[] = { "ebp", "esp", "eip", NULL }; > + tdesc->expedite_regs = expedite_regs_i386; > +#endif /* __x86_64__ */ Likewise here. (Yes, nit-picking, I know.) Thanks.
diff --git a/gdb/gdbserver/win32-i386-low.c b/gdb/gdbserver/win32-i386-low.c index a242f72..fdb95ce 100644 --- a/gdb/gdbserver/win32-i386-low.c +++ b/gdb/gdbserver/win32-i386-low.c @@ -442,6 +442,16 @@ i386_arch_setup (void) init_target_desc (tdesc); +#ifndef IN_PROCESS_AGENT +#ifdef __x86_64__ + static const char *expedite_regs_amd64[] = { "rbp", "rsp", "rip", NULL }; + tdesc->expedite_regs = expedite_regs_amd64; +#else /* __x86_64__ */ + static const char *expedite_regs_i386[] = { "ebp", "esp", "eip", NULL }; + tdesc->expedite_regs = expedite_regs_i386; +#endif /* __x86_64__ */ +#endif + win32_tdesc = tdesc; }