[PATCH] Fix execution_direction's type

[PATCH] Fix execution_direction's type

[Pedro Alves]

This fixes a few build errors like these in C++ mode:

  src/gdb/reverse.c: In function ‘void exec_reverse_once(char*, char*, int)’:
  src/gdb/reverse.c:49:34: error: invalid conversion from ‘int’ to ‘exec_direction_kind’ [-fpermissive]
     enum exec_direction_kind dir = execution_direction;
				    ^
  make: *** [reverse.o] Error 1

2015-10-09  Pedro Alves  <palves@redhat.com>

	* infrun.c (restore_execution_direction): New function.
	(fetch_inferior_event): Use it instead of
	make_cleanup_restore_integer.
	(execution_direction): Change type to enum
	exec_direction_kind.
	* infrun.h (execution_direction): Likewise.
---
 gdb/infrun.c | 16 ++++++++++++++--
 gdb/infrun.h |  6 ++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 123d73f..77b9079 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3800,6 +3800,17 @@ clean_up_just_stopped_threads_fsms (struct execution_control_state *ecs)
     }
 }
 
+/* A cleanup that restores the execution direction to the value saved
+   in *ARG.  */
+
+static void
+restore_execution_direction (void *arg)
+{
+  enum exec_direction_kind *save_exec_dir = (enum exec_direction_kind *) arg;
+
+  execution_direction = *save_exec_dir;
+}
+
 /* Asynchronous version of wait_for_inferior.  It is called by the
    event loop whenever a change of state is detected on the file
    descriptor corresponding to the target.  It can be called more than
@@ -3817,6 +3828,7 @@ fetch_inferior_event (void *client_data)
   struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
   struct cleanup *ts_old_chain;
   int was_sync = sync_execution;
+  enum exec_direction_kind save_exec_dir = execution_direction;
   int cmd_done = 0;
   ptid_t waiton_ptid = minus_one_ptid;
 
@@ -3849,7 +3861,7 @@ fetch_inferior_event (void *client_data)
      event.  */
   target_dcache_invalidate ();
 
-  make_cleanup_restore_integer (&execution_direction);
+  make_cleanup (restore_execution_direction, &save_exec_dir);
   execution_direction = target_execution_direction ();
 
   ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
@@ -8939,7 +8951,7 @@ clear_exit_convenience_vars (void)
    Set exec-direction / show exec-direction commands
    (returns error unless target implements to_set_exec_direction method).  */
 
-int execution_direction = EXEC_FORWARD;
+enum exec_direction_kind execution_direction = EXEC_FORWARD;
 static const char exec_forward[] = "forward";
 static const char exec_reverse[] = "reverse";
 static const char *exec_direction = exec_forward;
diff --git a/gdb/infrun.h b/gdb/infrun.h
index ab27538..7364065 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -74,10 +74,8 @@ enum exec_direction_kind
     EXEC_REVERSE
   };
 
-/* The current execution direction.  This should only be set to enum
-   exec_direction_kind values.  It is only an int to make it
-   compatible with make_cleanup_restore_integer.  */
-extern int execution_direction;
+/* The current execution direction.  */
+extern enum exec_direction_kind execution_direction;
 
 extern void start_remote (int from_tty);
 
-- 
1.9.3

Re: [PATCH] Fix execution_direction's type

[Simon Marchi]

On 15-10-09 11:34 AM, Pedro Alves wrote:
> @@ -3817,6 +3828,7 @@ fetch_inferior_event (void *client_data)
>    struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
>    struct cleanup *ts_old_chain;
>    int was_sync = sync_execution;
> +  enum exec_direction_kind save_exec_dir = execution_direction;
>    int cmd_done = 0;
>    ptid_t waiton_ptid = minus_one_ptid;
>  
> @@ -3849,7 +3861,7 @@ fetch_inferior_event (void *client_data)
>       event.  */
>    target_dcache_invalidate ();
>  
> -  make_cleanup_restore_integer (&execution_direction);
> +  make_cleanup (restore_execution_direction, &save_exec_dir);
>    execution_direction = target_execution_direction ();
>  
>    ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,

Isn't it dangerous to record a cleanup with a pointer to a local variable?

What if something throws the exception is caught in a parent frame?  The
content of the memory location pointed by the cleanup will be undefined
at that time, won't it?

Re: [PATCH] Fix execution_direction's type

[Pedro Alves]

On 10/09/2015 06:14 PM, Simon Marchi wrote:
> On 15-10-09 11:34 AM, Pedro Alves wrote:
>> @@ -3817,6 +3828,7 @@ fetch_inferior_event (void *client_data)
>>    struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
>>    struct cleanup *ts_old_chain;
>>    int was_sync = sync_execution;
>> +  enum exec_direction_kind save_exec_dir = execution_direction;
>>    int cmd_done = 0;
>>    ptid_t waiton_ptid = minus_one_ptid;
>>  
>> @@ -3849,7 +3861,7 @@ fetch_inferior_event (void *client_data)
>>       event.  */
>>    target_dcache_invalidate ();
>>  
>> -  make_cleanup_restore_integer (&execution_direction);
>> +  make_cleanup (restore_execution_direction, &save_exec_dir);
>>    execution_direction = target_execution_direction ();
>>  
>>    ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
> 
> Isn't it dangerous to record a cleanup with a pointer to a local variable?
> 
> What if something throws the exception is caught in a parent frame?  The
> content of the memory location pointed by the cleanup will be undefined
> at that time, won't it?

No, the cleanups are run _before_ the throwing scope is left (that is,
before longjmp), from within throw_exception.

Thanks,
Pedro Alves

Re: [PATCH] Fix execution_direction's type

[Pedro Alves]

On 10/09/2015 04:34 PM, Pedro Alves wrote:
> This fixes a few build errors like these in C++ mode:
> 
>   src/gdb/reverse.c: In function ‘void exec_reverse_once(char*, char*, int)’:
>   src/gdb/reverse.c:49:34: error: invalid conversion from ‘int’ to ‘exec_direction_kind’ [-fpermissive]
>      enum exec_direction_kind dir = execution_direction;
> 				    ^
>   make: *** [reverse.o] Error 1
> 
> 2015-10-09  Pedro Alves  <palves@redhat.com>
> 
> 	* infrun.c (restore_execution_direction): New function.
> 	(fetch_inferior_event): Use it instead of
> 	make_cleanup_restore_integer.
> 	(execution_direction): Change type to enum
> 	exec_direction_kind.
> 	* infrun.h (execution_direction): Likewise.

Now pushed.

Thanks,
Pedro Alves