Re: [PATCH] Do not cast away const in agent_run_command

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

> While investigating something else, I noticed some weird code in
> agent_run_command (use of memcpy rather than strcpy).  Then I noticed
> that 'cmd' is used as both an in and out parameter, despite being
> const.
>
> Casting away const like this is bad.  This patch removes the const and
> fixes the memcpy.  I also added a static assert to assure myself that
> the code in gdbserver is correct -- gdbserver is passing its own
> buffer directly to agent_run_command.

LGTM.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> ---
>  gdb/linux-nat.c         |  7 ++-----
>  gdbserver/server.cc     |  5 +++++
>  gdbserver/tracepoint.cc |  2 +-
>  gdbsupport/agent.cc     | 14 ++++++++------
>  gdbsupport/agent.h      |  2 +-
>  5 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 2b206a4ec1e..d6e69e61ef5 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4114,9 +4114,7 @@ linux_nat_target::static_tracepoint_markers_by_strid (const char *strid)
>    /* Pause all */
>    target_stop (ptid);
>  
> -  memcpy (s, "qTfSTM", sizeof ("qTfSTM"));
> -  s[sizeof ("qTfSTM")] = 0;
> -
> +  strcpy (s, "qTfSTM");
>    agent_run_command (pid, s, strlen (s) + 1);
>  
>    /* Unpause all.  */
> @@ -4133,8 +4131,7 @@ linux_nat_target::static_tracepoint_markers_by_strid (const char *strid)
>  	}
>        while (*p++ == ',');	/* comma-separated list */
>  
> -      memcpy (s, "qTsSTM", sizeof ("qTsSTM"));
> -      s[sizeof ("qTsSTM")] = 0;
> +      strcpy (s, "qTsSTM");
>        agent_run_command (pid, s, strlen (s) + 1);
>        p = s;
>      }
> diff --git a/gdbserver/server.cc b/gdbserver/server.cc
> index 21fb51a45d1..46dfe70838b 100644
> --- a/gdbserver/server.cc
> +++ b/gdbserver/server.cc
> @@ -51,6 +51,11 @@
>  #include "gdbsupport/scoped_restore.h"
>  #include "gdbsupport/search.h"
>  
> +/* PBUFSIZ must also be at least as big as IPA_CMD_BUF_SIZE, because
> +   the client state data is passed directly to some agent
> +   functions.  */
> +gdb_static_assert (PBUFSIZ >= IPA_CMD_BUF_SIZE);
> +
>  #define require_running_or_return(BUF)		\
>    if (!target_running ())			\
>      {						\
> diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc
> index 37a9a8c5b7c..b59077a3896 100644
> --- a/gdbserver/tracepoint.cc
> +++ b/gdbserver/tracepoint.cc
> @@ -6820,7 +6820,7 @@ run_inferior_command (char *cmd, int len)
>    target_pause_all (false);
>    uninsert_all_breakpoints ();
>  
> -  err = agent_run_command (pid, (const char *) cmd, len);
> +  err = agent_run_command (pid, cmd, len);
>  
>    reinsert_all_breakpoints ();
>    target_unpause_all (false);
> diff --git a/gdbsupport/agent.cc b/gdbsupport/agent.cc
> index 531807be3d2..81c925dd99d 100644
> --- a/gdbsupport/agent.cc
> +++ b/gdbsupport/agent.cc
> @@ -179,14 +179,16 @@ gdb_connect_sync_socket (int pid)
>  #endif
>  }
>  
> -/* Execute an agent command in the inferior.  PID is the value of pid of the
> -   inferior.  CMD is the buffer for command.  GDB or GDBserver will store the
> -   command into it and fetch the return result from CMD.  The interaction
> -   between GDB/GDBserver and the agent is synchronized by a synchronization
> -   socket.  Return zero if success, otherwise return non-zero.  */
> +/* Execute an agent command in the inferior.  PID is the value of pid
> +   of the inferior.  CMD is the buffer for command.  It is assumed to
> +   be at least IPA_CMD_BUF_SIZE bytes long.  GDB or GDBserver will
> +   store the command into it and fetch the return result from CMD.
> +   The interaction between GDB/GDBserver and the agent is synchronized
> +   by a synchronization socket.  Return zero if success, otherwise
> +   return non-zero.  */
>  
>  int
> -agent_run_command (int pid, const char *cmd, int len)
> +agent_run_command (int pid, char *cmd, int len)
>  {
>    int fd;
>    int tid = agent_get_helper_thread_id ();
> diff --git a/gdbsupport/agent.h b/gdbsupport/agent.h
> index dceb33f6bd5..7a258e267a5 100644
> --- a/gdbsupport/agent.h
> +++ b/gdbsupport/agent.h
> @@ -22,7 +22,7 @@
>  
>  #include "gdbsupport/preprocessor.h"
>  
> -int agent_run_command (int pid, const char *cmd, int len);
> +int agent_run_command (int pid, char *cmd, int len);
>  
>  int agent_look_up_symbols (void *);
>  
> -- 
> 2.39.1