From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9856 invoked by alias); 20 Jan 2010 17:38:03 -0000 Received: (qmail 9834 invoked by uid 22791); 20 Jan 2010 17:38:01 -0000 X-Spam-Check-By: sourceware.org Received: from pool-173-76-48-109.bstnma.east.verizon.net (HELO cgf.cx) (173.76.48.109) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 Jan 2010 17:37:56 +0000 Received: from ednor.cgf.cx (ednor.casa.cgf.cx [192.168.187.5]) by cgf.cx (Postfix) with ESMTP id 6A12113C0C7 for ; Wed, 20 Jan 2010 12:37:46 -0500 (EST) Received: by ednor.cgf.cx (Postfix, from userid 201) id 586A12B35A; Wed, 20 Jan 2010 12:37:46 -0500 (EST) Date: Wed, 20 Jan 2010 17:38:00 -0000 From: Christopher Faylor To: gdb-patches ml Subject: Re: [RFA] windows: do not crash if inferior Message-ID: <20100120173746.GA27289@ednor.casa.cgf.cx> Mail-Followup-To: gdb-patches ml References: <1C9A707A-AE7C-4947-A9DA-F105674F81AE@adacore.com> <20100119092227.GN17397@adacore.com> <20100120161549.GA24063@ednor.casa.cgf.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100120161549.GA24063@ednor.casa.cgf.cx> User-Agent: Mutt/1.5.20 (2009-06-14) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-01/txt/msg00504.txt.bz2 On Wed, Jan 20, 2010 at 11:15:49AM -0500, Christopher Faylor wrote: >On Tue, Jan 19, 2010 at 01:22:27PM +0400, Joel Brobecker wrote: >>> 2010-01-13 gingold >>> >>> * windows-nat.c (do_initial_windows_stuff): Call error() if the >>> inferior doesn't exist anymore. >> >>Just some thoughts, as Chris usually reviews Windows patches... >> >>I don't see a "generic" way of dealing with this situation. So the type >>of approach you took (returning early from do_initial_windows_stuff) >>seems to be the only approach I can see. I was initially a little >>reluctant about throwing an error: As far as I can tell from the code, >>the debugger should have already printed an error message such as >>"Inferior exited with code ..." (is that correct?) - and so an extra >>"inferior exited early" message could be considered superfluous. However, >>if you do not error-out now, core GDB will assume that target_create_inferior >>succeeded and thus possibly do something unexpected as well. >> >>Bottom line - I cannot propose a better approach short of revamping >>a bit the target_create_inferior routine to add error-handling, >>I think the patch is fine. >> >>I would suggest a different wording, to explain that we were trying >>to run the program when the error occured. >> >> error (_("cannot run program, inferior exited prematurely during startup")); > >Does inferior_thread really need the assert()? If not, we could jsut test >tp for NULL. > >Otherwise, I agree with Joel's assessment. Actually, how about something like this instead? I used the same wording as fork-child.c after seeing Pedro's note. cgf Index: windows-nat.c =================================================================== RCS file: /cvs/src/src/gdb/windows-nat.c,v retrieving revision 1.196.4.1 diff -d -u -r1.196.4.1 windows-nat.c --- windows-nat.c 30 Sep 2009 07:40:10 -0000 1.196.4.1 +++ windows-nat.c 20 Jan 2010 17:35:47 -0000 @@ -41,6 +41,7 @@ #include #ifdef __CYGWIN__ #include +#include #endif #include @@ -123,6 +128,8 @@ static uintptr_t dr[8]; static int debug_registers_changed; static int debug_registers_used; + +static int windows_initialization_done; #define DR6_CLEAR_VALUE 0xffff0ff0 /* The string sent by cygwin when it processes a signal. @@ -1399,6 +1407,8 @@ (unsigned) current_event.dwProcessId, (unsigned) current_event.dwThreadId, "EXIT_PROCESS_DEBUG_EVENT")); + if (!windows_initialization_done) + error (_("During startup program exited with code 0x%x."), (unsigned int) current_event.u.ExitProcess.dwExitCode); if (saw_create != 1) break; ourstatus->kind = TARGET_WAITKIND_EXITED; @@ -1597,6 +1607,7 @@ terminal_init_inferior_with_pgrp (pid); target_terminal_inferior (); + windows_initialization_done = 0; inf->stop_soon = STOP_QUIETLY; while (1) { @@ -1609,6 +1620,7 @@ break; } + windows_initialization_done = 1; inf->stop_soon = NO_STOP_QUIETLY; stop_after_trap = 0; return;