[review] Share handle_exception

[Pedro Alves (Code Review)]

Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/425
......................................................................


Patch Set 1:

(5 comments)

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +3,20 @@ Author:     Tom Tromey <tromey@adacore.com>
| +AuthorDate: 2019-10-18 13:26:45 -0600
| +Commit:     Tom Tromey <tromey@adacore.com>
| +CommitDate: 2019-10-29 10:08:40 -0600
| +
| +Share handle_exception
| +
| +Both gdb and gdbserver have a "handle_exception" function, the bulk of
| +which is shared between the two implementations.  This patch arranges
| +for the entire thing to be moved into nat/windows-nat.c, with the
| +differences handled by callbacks.  This patch introduces one more
| +callback to make this possible.

PS1, Line 13:

That's:

> 	(windows_nat::handle_ms_vc_exception): New function.

There's no reason we couldn't just support thread names in gdbserver
as well.  It just wasn't done when the original support was added,
left forever stuck as a TODO item.  You'd just have to install a
target_ops::thread_name implementation on win32-low.c as well.

| +
| +Change-Id: I4e6e0d17b868cd51964c273fb28ec066fea6b767
| +
| +gdb/ChangeLog
| +2019-10-29  Tom Tromey  <tromey@adacore.com>
| +
| +	* windows-nat.c (MS_VC_EXCEPTION): Move to nat/windows-nat.c.
| +	(handle_exception_result): Move to nat/windows-nat.h.
| +	(DEBUG_EXCEPTION_SIMPLE): Remove.
| --- gdb/gdbserver/win32-low.c
| +++ gdb/gdbserver/win32-low.c
| @@ -1287,16 +1187,18 @@ /* Get the next event from the child.  */
|  static int
| -get_child_debug_event (struct target_waitstatus *ourstatus)
| +get_child_debug_event (DWORD *continue_status,
| +		       struct target_waitstatus *ourstatus)
|  {
|    ptid_t ptid;
|  
|    last_sig = GDB_SIGNAL_0;
|    ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
| +  *continue_status = DBG_CONTINUE;

PS1, Line 1195:

Given this...

|  
|    /* Check if GDB sent us an interrupt request.  */
|    check_remote_input_interrupt_request ();
|  
|    if (soft_interrupt_requested)
|      {
|        soft_interrupt_requested = 0;
|        fake_breakpoint_event ();
|        goto gotevent;

 ...

| @@ -1499,17 +1403,18 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
|  	 fails).  Report it now.  */
|        *ourstatus = cached_status;
|        cached_status.kind = TARGET_WAITKIND_IGNORE;
|        return debug_event_ptid (&current_event);
|      }
|  
|    while (1)
|      {
| -      if (!get_child_debug_event (ourstatus))
| +      DWORD continue_status = DBG_CONTINUE;

PS1, Line 1411:

... this here doesn't appear necessary.

| +      if (!get_child_debug_event (&continue_status, ourstatus))
|  	continue;
|  
|        switch (ourstatus->kind)
|  	{
|  	case TARGET_WAITKIND_EXITED:
|  	  OUTMSG2 (("Child exited with retcode = %x\n",
|  		    ourstatus->value.integer));
|  	  win32_clear_inferiors ();
| --- gdb/nat/windows-nat.h
| +++ gdb/nat/windows-nat.h
| @@ -123,11 +123,19 @@ extern void handle_load_dll ();
|  /* Handle a DLL unload event.
|  
|     This function assumes that this event did not occur during inferior
|     initialization.
|  
|     This function must be supplied by the embedding application.  */
|  
|  extern void handle_unload_dll ();
|  
| +/* Handle MS_VC_EXCEPTION when processing a FIXME.

PS1, Line 132:

FIXME?

| +
| +   Return true if the exception was handled; return false otherwise.
| +
| +   This function must be supplied by the embedding application.  */
| +
| +extern bool handle_ms_vc_exception (const EXCEPTION_RECORD *rec);
| +
|  
|  /* Currently executing process */

 ...

| @@ -195,4 +203,14 @@ extern const char *get_image_name (HANDLE h, void *address, int unicode);
|  
| +typedef enum
| +{
| +  HANDLE_EXCEPTION_UNHANDLED = 0,
| +  HANDLE_EXCEPTION_HANDLED,
| +  HANDLE_EXCEPTION_IGNORED
| +} handle_exception_result;
| +
| +extern handle_exception_result handle_exception
| +    (struct target_waitstatus *ourstatus, bool debug_exceptions);

PS1, Line 212:

Two leading spaces:

extern handle_exception_result handle_exception
  (struct target_waitstatus *ourstatus, bool debug_exceptions);

See e.g. extension-priv.h.

| +
|  }
|  
|  #endif

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I2efe53c78f5a8e28381e539505f1b703967933c4
Gerrit-Change-Number: 425
Gerrit-PatchSet: 1
Gerrit-Owner: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Tue, 19 Nov 2019 19:55:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment