[RFA] Ensure GDB printf command can print convenience var strings without a target.

  Without this patch, GDB printf command calls malloc on the target,
writes the convenience var content to the target,
re-reads the content from the target, and then locally printf the string.

This implies inferior calls, and does not work when there is no inferior,
or when the inferior is a core dump.

With this patch, printf command can printf string convenience variables
without inferior function calls.
Ada string convenience variables can also be printed.

gdb/ChangeLog

2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* NEWS: Mention that GDB printf and eval commands can now print
	C-style and Ada-style convenience var strings without
	calling the inferior.
	* printcmd.c (printf_c_string): Locally print GDB internal var
	instead of transiting via the inferior.
	(printf_wide_c_string): Likewise.

2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/printcmds.exp: Test printing C strings and
	C wide strings convenience var without transiting via the inferior.
---
 gdb/NEWS                             |   7 ++
 gdb/printcmd.c                       | 143 +++++++++++++++++----------
 gdb/testsuite/gdb.base/printcmds.exp |  39 ++++++++
 3 files changed, 136 insertions(+), 53 deletions(-)
  

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Mon, 10 Jun 2019 23:16:22 +0200
> 
> Without this patch, GDB printf command calls malloc on the target,
> writes the convenience var content to the target,
> re-reads the content from the target, and then locally printf the string.
> 
> This implies inferior calls, and does not work when there is no inferior,
> or when the inferior is a core dump.
> 
> With this patch, printf command can printf string convenience variables
> without inferior function calls.
> Ada string convenience variables can also be printed.
> 
> gdb/ChangeLog
> 
> 2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* NEWS: Mention that GDB printf and eval commands can now print
> 	C-style and Ada-style convenience var strings without
> 	calling the inferior.
> 	* printcmd.c (printf_c_string): Locally print GDB internal var
> 	instead of transiting via the inferior.
> 	(printf_wide_c_string): Likewise.
> 
> 2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/printcmds.exp: Test printing C strings and
> 	C wide strings convenience var without transiting via the inferior.
> ---
>  gdb/NEWS                             |   7 ++
>  gdb/printcmd.c                       | 143 +++++++++++++++++----------
>  gdb/testsuite/gdb.base/printcmds.exp |  39 ++++++++
>  3 files changed, 136 insertions(+), 53 deletions(-)

The NEWS part is OK, thanks.
  

Ping ?

Thanks

Philippe

On Mon, 2019-06-10 at 23:16 +0200, Philippe Waroquiers wrote:
> Without this patch, GDB printf command calls malloc on the target,
> writes the convenience var content to the target,
> re-reads the content from the target, and then locally printf the string.
> 
> This implies inferior calls, and does not work when there is no inferior,
> or when the inferior is a core dump.
> 
> With this patch, printf command can printf string convenience variables
> without inferior function calls.
> Ada string convenience variables can also be printed.
> 
> gdb/ChangeLog
> 
> 2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* NEWS: Mention that GDB printf and eval commands can now print
> 	C-style and Ada-style convenience var strings without
> 	calling the inferior.
> 	* printcmd.c (printf_c_string): Locally print GDB internal var
> 	instead of transiting via the inferior.
> 	(printf_wide_c_string): Likewise.
> 
> 2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/printcmds.exp: Test printing C strings and
> 	C wide strings convenience var without transiting via the inferior.
> ---
>  gdb/NEWS                             |   7 ++
>  gdb/printcmd.c                       | 143 +++++++++++++++++----------
>  gdb/testsuite/gdb.base/printcmds.exp |  39 ++++++++
>  3 files changed, 136 insertions(+), 53 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 9e1462b6bf..9d6a2de661 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -98,6 +98,13 @@ apropos [-v] REGEXP
>    of matching commands and to use the highlight style to mark
>    the documentation parts matching REGEXP.
>  
> +printf
> +eval
> +  The GDB printf and eval commands can now print C-style and Ada-style
> +  convenience variables without calling functions in the program.
> +  This allows to do formatted printing of strings without having
> +  an inferior, or when debugging a core dump.
> +
>  show style
>    The "show style" and its subcommands are now styling
>    a style name in their output using its own style, to help
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 9e84594fe6..d7b8b9a1c1 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -23,6 +23,7 @@
>  #include "gdbtypes.h"
>  #include "value.h"
>  #include "language.h"
> +#include "c-lang.h"
>  #include "expression.h"
>  #include "gdbcore.h"
>  #include "gdbcmd.h"
> @@ -2200,91 +2201,127 @@ print_variable_and_value (const char *name, struct symbol *var,
>  
>  /* Subroutine of ui_printf to simplify it.
>     Print VALUE to STREAM using FORMAT.
> -   VALUE is a C-style string on the target.  */
> +   VALUE is a C-style string on the target or a C-style string
> +   in a GDB internal variable.  */
>  
>  static void
>  printf_c_string (struct ui_file *stream, const char *format,
>  		 struct value *value)
>  {
> -  gdb_byte *str;
> -  CORE_ADDR tem;
> -  int j;
> +  const gdb_byte *str;
>  
> -  tem = value_as_address (value);
> -  if (tem == 0)
> +  if (VALUE_LVAL (value) == lval_internalvar
> +      && c_is_string_type_p (value_type (value)))
>      {
> -      DIAGNOSTIC_PUSH
> -      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> -      fprintf_filtered (stream, format, "(null)");
> -      DIAGNOSTIC_POP
> -      return;
> -    }
> +      gdb_byte *tem_str;
> +      size_t len  = TYPE_LENGTH (value_type (value));
>  
> -  /* This is a %s argument.  Find the length of the string.  */
> -  for (j = 0;; j++)
> +      /* Copy the internal var value to tem_str and append a terminating null
> +	 character.  This protects against corrupted C-style strings that lacks
> +	 the terminating null char.  It also allows Ada style strings (not not
> +	 null terminated) to be printed without problems.  */
> +      tem_str = (gdb_byte *) alloca (len + 1);
> +      memcpy (tem_str, value_contents (value), len);
> +      tem_str [len] = 0;
> +      str = tem_str;
> +    }
> +  else
>      {
> -      gdb_byte c;
> +      int len;
> +      CORE_ADDR tem;
> +      gdb_byte *tem_str;
>  
> -      QUIT;
> -      read_memory (tem + j, &c, 1);
> -      if (c == 0)
> -	break;
> -    }
> +      tem = value_as_address (value);
> +      if (tem == 0)
> +	{
> +	  DIAGNOSTIC_PUSH
> +	    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> +	    fprintf_filtered (stream, format, "(null)");
> +	  DIAGNOSTIC_POP
> +	    return;
> +	}
>  
> -  /* Copy the string contents into a string inside GDB.  */
> -  str = (gdb_byte *) alloca (j + 1);
> -  if (j != 0)
> -    read_memory (tem, str, j);
> -  str[j] = 0;
> +      /* This is a %s argument.  Find the length of the string.  */
> +      for (len = 0;; len++)
> +	{
> +	  gdb_byte c;
> +
> +	  QUIT;
> +	  read_memory (tem + len, &c, 1);
> +	  if (c == 0)
> +	    break;
> +	}
> +
> +      /* Copy the string contents into a string inside GDB.  */
> +      tem_str = (gdb_byte *) alloca (len + 1);
> +      if (len != 0)
> +	read_memory (tem, tem_str, len);
> +      tem_str[len] = 0;
> +      str = tem_str;
> +    }
>  
>    DIAGNOSTIC_PUSH
> -  DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> -  fprintf_filtered (stream, format, (char *) str);
> +    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> +    fprintf_filtered (stream, format, (char *) str);
>    DIAGNOSTIC_POP
> -}
> +    }
>  
>  /* Subroutine of ui_printf to simplify it.
>     Print VALUE to STREAM using FORMAT.
> -   VALUE is a wide C-style string on the target.  */
> +   VALUE is a wide C-style string on the target or a wide C-style string
> +   in a GDB internal variable.  */
>  
>  static void
>  printf_wide_c_string (struct ui_file *stream, const char *format,
>  		      struct value *value)
>  {
> -  gdb_byte *str;
> -  CORE_ADDR tem;
> +  const gdb_byte *str;
>    int j;
>    struct gdbarch *gdbarch = get_type_arch (value_type (value));
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    struct type *wctype = lookup_typename (current_language, gdbarch,
>  					 "wchar_t", NULL, 0);
>    int wcwidth = TYPE_LENGTH (wctype);
> -  gdb_byte *buf = (gdb_byte *) alloca (wcwidth);
>  
> -  tem = value_as_address (value);
> -  if (tem == 0)
> +  if (VALUE_LVAL (value) == lval_internalvar
> +      && c_is_string_type_p (value_type (value)))
>      {
> -      DIAGNOSTIC_PUSH
> -      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> -      fprintf_filtered (stream, format, "(null)");
> -      DIAGNOSTIC_POP
> -      return;
> +      str = value_contents (value);
> +      j = TYPE_LENGTH (value_type (value));
>      }
> -
> -  /* This is a %s argument.  Find the length of the string.  */
> -  for (j = 0;; j += wcwidth)
> +  else
>      {
> -      QUIT;
> -      read_memory (tem + j, buf, wcwidth);
> -      if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0)
> -	break;
> -    }
> +      gdb_byte *tem_str;
> +      CORE_ADDR tem;
> +      gdb_byte *buf = (gdb_byte *) alloca (wcwidth);
>  
> -  /* Copy the string contents into a string inside GDB.  */
> -  str = (gdb_byte *) alloca (j + wcwidth);
> -  if (j != 0)
> -    read_memory (tem, str, j);
> -  memset (&str[j], 0, wcwidth);
> +      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
> +      tem = value_as_address (value);
> +      if (tem == 0)
> +	{
> +	  DIAGNOSTIC_PUSH
> +	    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> +	    fprintf_filtered (stream, format, "(null)");
> +	  DIAGNOSTIC_POP
> +	    return;
> +	}
> +
> +      /* This is a %s argument.  Find the length of the string.  */
> +      for (j = 0;; j += wcwidth)
> +	{
> +	  QUIT;
> +	  read_memory (tem + j, buf, wcwidth);
> +	  if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0)
> +	    break;
> +	}
> +
> +      /* Copy the string contents into a string inside GDB.  */
> +      tem_str = (gdb_byte *) alloca (j + wcwidth);
> +      if (j != 0)
> +	read_memory (tem, tem_str, j);
> +      memset (&tem_str[j], 0, wcwidth);
> +      str = tem_str;
> +    }
>  
>    auto_obstack output;
>  
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index f2d6ee229d..3b6562426e 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -932,6 +932,32 @@ proc test_repeat_bytes {} {
>      }
>  }
>  
> +proc test_printf_convenience_var {prefix do_wstring} {
> +
> +    with_test_prefix $prefix {
> +	gdb_test_no_output "set var \$cstr = \"abcde\"" "set \$cstr, conv var"
> +	gdb_test "printf \"cstr val = %s\\n\", \$cstr" "cstr val = abcde" \
> +	    "printf \$cstr, conv var"
> +	gdb_test_no_output "set var \$abcde = \"ABCDE\"" "set \$abcde, conv var"
> +	gdb_test "eval \"print \$%s\\n\", \$cstr" "= \"ABCDE\"" \
> +	    "indirect print abcde"
> +	gdb_test "set language ada" ".*" "set language ada, conv var"
> +	gdb_test_no_output "set var \$astr := \"fghij\"" "set \$astr, conv var"
> +	gdb_test "printf \"astr val = %s\\n\", \$astr" "astr val = fghij" \
> +	    "printf \$astr, conv var"
> +	gdb_test "set language auto" ".*" "set language auto, conv var"
> +	gdb_test "printf \"astr val = %s\\n\", \$astr" "astr val = fghij" \
> +	    "printf \$astr, conv var, auto language"
> +	if ($do_wstring) {
> +	    gdb_test_no_output "set var \$wstr = L\"facile\"" \
> +		"set \$wstr, conv var"
> +	    gdb_test "printf \"wstr val = %ls\\n\", \$wstr" \
> +		"wstr val = facile" "printf \$wstr, conv var"
> +	}
> +    }
> +}
> +
> +
>  # Start with a fresh gdb.
>  
>  gdb_exit
> @@ -948,6 +974,11 @@ gdb_test "ptype \"abc\"" " = char \\\[4\\\]"
>  gdb_test "print \$cvar = \"abc\"" " = \"abc\""
>  gdb_test "print sizeof (\$cvar)" " = 4"
>  
> +# Similarly, printf of convenience var should work without a target.
> +# At this point, we cannot create wide strings convenience var, as the
> +# type wchar_t is not yet known, so skip the wide string tests.
> +test_printf_convenience_var "no target" 0
> +
>  # GDB used to complete the explicit location options even when
>  # printing expressions.
>  gdb_test_no_output "complete p -function"
> @@ -977,6 +1008,14 @@ if ![runto_main] then {
>      return 0
>  }
>  
> +# With a target, printf convenience var should of course work.
> +test_printf_convenience_var "with target" 1
> +
> +# But it should also work when inferior function calls are forbidden.
> +gdb_test_no_output "set may-call-functions off"
> +test_printf_convenience_var "with target, may-call-functions off" 1
> +gdb_test_no_output "set may-call-functions on"
> +
>  test_integer_literals_accepted
>  test_integer_literals_rejected
>  test_float_accepted
  

Looks fine to me, with the nits below fixed.

On 6/10/19 10:16 PM, Philippe Waroquiers wrote:
> Without this patch, GDB printf command calls malloc on the target,
> writes the convenience var content to the target,
> re-reads the content from the target, and then locally printf the string.
> 
> This implies inferior calls, and does not work when there is no inferior,
> or when the inferior is a core dump.
> 
> With this patch, printf command can printf string convenience variables
> without inferior function calls.
> Ada string convenience variables can also be printed.
> 
> gdb/ChangeLog
> 
> 2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* NEWS: Mention that GDB printf and eval commands can now print
> 	C-style and Ada-style convenience var strings without
> 	calling the inferior.
> 	* printcmd.c (printf_c_string): Locally print GDB internal var
> 	instead of transiting via the inferior.
> 	(printf_wide_c_string): Likewise.
> 
> 2019-06-10  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/printcmds.exp: Test printing C strings and
> 	C wide strings convenience var without transiting via the inferior.
> ---
>  gdb/NEWS                             |   7 ++
>  gdb/printcmd.c                       | 143 +++++++++++++++++----------
>  gdb/testsuite/gdb.base/printcmds.exp |  39 ++++++++
>  3 files changed, 136 insertions(+), 53 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 9e1462b6bf..9d6a2de661 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -98,6 +98,13 @@ apropos [-v] REGEXP
>    of matching commands and to use the highlight style to mark
>    the documentation parts matching REGEXP.
>  
> +printf
> +eval
> +  The GDB printf and eval commands can now print C-style and Ada-style
> +  convenience variables without calling functions in the program.
> +  This allows to do formatted printing of strings without having
> +  an inferior, or when debugging a core dump.

Better say without having a _running_ inferior, since there's
always an inferior.

> +
>  show style
>    The "show style" and its subcommands are now styling
>    a style name in their output using its own style, to help
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 9e84594fe6..d7b8b9a1c1 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -23,6 +23,7 @@
>  #include "gdbtypes.h"
>  #include "value.h"
>  #include "language.h"
> +#include "c-lang.h"
>  #include "expression.h"
>  #include "gdbcore.h"
>  #include "gdbcmd.h"
> @@ -2200,91 +2201,127 @@ print_variable_and_value (const char *name, struct symbol *var,
>  
>  /* Subroutine of ui_printf to simplify it.
>     Print VALUE to STREAM using FORMAT.
> -   VALUE is a C-style string on the target.  */
> +   VALUE is a C-style string on the target or a C-style string
> +   in a GDB internal variable.  */

You could avoid the repetition:

   VALUE is a C-style string either on the target or in
   a GDB internal variable.  */

>  
>  static void
>  printf_c_string (struct ui_file *stream, const char *format,
>  		 struct value *value)
>  {
> -  gdb_byte *str;
> -  CORE_ADDR tem;
> -  int j;
> +  const gdb_byte *str;
>  
> -  tem = value_as_address (value);
> -  if (tem == 0)
> +  if (VALUE_LVAL (value) == lval_internalvar
> +      && c_is_string_type_p (value_type (value)))
>      {
> -      DIAGNOSTIC_PUSH
> -      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> -      fprintf_filtered (stream, format, "(null)");
> -      DIAGNOSTIC_POP
> -      return;
> -    }
> +      gdb_byte *tem_str;

You can declare tem_str at the point of initialization.

> +      size_t len  = TYPE_LENGTH (value_type (value));

Spurious double space after len.

>  
> -  /* This is a %s argument.  Find the length of the string.  */
> -  for (j = 0;; j++)
> +      /* Copy the internal var value to tem_str and append a terminating null

TEM_STR

> +	 character.  This protects against corrupted C-style strings that lacks

"strings that lack"

> +	 the terminating null char.  It also allows Ada style strings (not not

"Ada style strings" -> "Ada-style strings"

"not not" -> "not".

> +	 null terminated) to be printed without problems.  */
> +      tem_str = (gdb_byte *) alloca (len + 1);
> +      memcpy (tem_str, value_contents (value), len);
> +      tem_str [len] = 0;
> +      str = tem_str;
> +    }
> +  else
>      {
> -      gdb_byte c;
> +      int len;
> +      CORE_ADDR tem;
> +      gdb_byte *tem_str;

Ditto.


>  
> -      QUIT;
> -      read_memory (tem + j, &c, 1);
> -      if (c == 0)
> -	break;
> -    }
> +      tem = value_as_address (value);
> +      if (tem == 0)
> +	{
> +	  DIAGNOSTIC_PUSH
> +	    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> +	    fprintf_filtered (stream, format, "(null)");
> +	  DIAGNOSTIC_POP

Please align all these on the same column, like it was before.

> +	    return;
> +	}
>  
> -  /* Copy the string contents into a string inside GDB.  */
> -  str = (gdb_byte *) alloca (j + 1);
> -  if (j != 0)
> -    read_memory (tem, str, j);
> -  str[j] = 0;
> +      /* This is a %s argument.  Find the length of the string.  */
> +      for (len = 0;; len++)
> +	{
> +	  gdb_byte c;
> +
> +	  QUIT;
> +	  read_memory (tem + len, &c, 1);
> +	  if (c == 0)
> +	    break;
> +	}
> +
> +      /* Copy the string contents into a string inside GDB.  */
> +      tem_str = (gdb_byte *) alloca (len + 1);
> +      if (len != 0)
> +	read_memory (tem, tem_str, len);
> +      tem_str[len] = 0;
> +      str = tem_str;

I notice this renamed "j" -> "len", but the wide version
did not get the same treatment.

> +    }
>  
>    DIAGNOSTIC_PUSH
> -  DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> -  fprintf_filtered (stream, format, (char *) str);
> +    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> +    fprintf_filtered (stream, format, (char *) str);
>    DIAGNOSTIC_POP

Ditto.

> -}
> +    }
>  
>  /* Subroutine of ui_printf to simplify it.
>     Print VALUE to STREAM using FORMAT.
> -   VALUE is a wide C-style string on the target.  */
> +   VALUE is a wide C-style string on the target or a wide C-style string
> +   in a GDB internal variable.  */

Same comments as in the non-wide version apply.

>  
>  static void
>  printf_wide_c_string (struct ui_file *stream, const char *format,
>  		      struct value *value)
>  {
> -  gdb_byte *str;
> -  CORE_ADDR tem;
> +  const gdb_byte *str;
>    int j;
>    struct gdbarch *gdbarch = get_type_arch (value_type (value));
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    struct type *wctype = lookup_typename (current_language, gdbarch,
>  					 "wchar_t", NULL, 0);
>    int wcwidth = TYPE_LENGTH (wctype);
> -  gdb_byte *buf = (gdb_byte *) alloca (wcwidth);
>  
> -  tem = value_as_address (value);
> -  if (tem == 0)
> +  if (VALUE_LVAL (value) == lval_internalvar
> +      && c_is_string_type_p (value_type (value)))
>      {
> -      DIAGNOSTIC_PUSH
> -      DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> -      fprintf_filtered (stream, format, "(null)");
> -      DIAGNOSTIC_POP
> -      return;
> +      str = value_contents (value);
> +      j = TYPE_LENGTH (value_type (value));
>      }
> -
> -  /* This is a %s argument.  Find the length of the string.  */
> -  for (j = 0;; j += wcwidth)
> +  else
>      {
> -      QUIT;
> -      read_memory (tem + j, buf, wcwidth);
> -      if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0)
> -	break;
> -    }
> +      gdb_byte *tem_str;
> +      CORE_ADDR tem;
> +      gdb_byte *buf = (gdb_byte *) alloca (wcwidth);
>  
> -  /* Copy the string contents into a string inside GDB.  */
> -  str = (gdb_byte *) alloca (j + wcwidth);
> -  if (j != 0)
> -    read_memory (tem, str, j);
> -  memset (&str[j], 0, wcwidth);
> +      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);

You could move this after the following if block, since byte_order
won't be needed until then.

> +
> +      tem = value_as_address (value);
> +      if (tem == 0)
> +	{
> +	  DIAGNOSTIC_PUSH
> +	    DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> +	    fprintf_filtered (stream, format, "(null)");
> +	  DIAGNOSTIC_POP
> +	    return;
> +	}
> +
> +      /* This is a %s argument.  Find the length of the string.  */
> +      for (j = 0;; j += wcwidth)
> +	{
> +	  QUIT;
> +	  read_memory (tem + j, buf, wcwidth);
> +	  if (extract_unsigned_integer (buf, wcwidth, byte_order) == 0)
> +	    break;
> +	}
> +
> +      /* Copy the string contents into a string inside GDB.  */
> +      tem_str = (gdb_byte *) alloca (j + wcwidth);
> +      if (j != 0)
> +	read_memory (tem, tem_str, j);
> +      memset (&tem_str[j], 0, wcwidth);
> +      str = tem_str;
> +    }
>  
>    auto_obstack output;
>  
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index f2d6ee229d..3b6562426e 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -932,6 +932,32 @@ proc test_repeat_bytes {} {
>      }
>  }
>  
> +proc test_printf_convenience_var {prefix do_wstring} {

Needs an intro comment.

> +
> +    with_test_prefix $prefix {
> +	gdb_test_no_output "set var \$cstr = \"abcde\"" "set \$cstr, conv var"
> +	gdb_test "printf \"cstr val = %s\\n\", \$cstr" "cstr val = abcde" \
> +	    "printf \$cstr, conv var"
> +	gdb_test_no_output "set var \$abcde = \"ABCDE\"" "set \$abcde, conv var"
> +	gdb_test "eval \"print \$%s\\n\", \$cstr" "= \"ABCDE\"" \
> +	    "indirect print abcde"

Missing ", conv var" ?  But see below.

> +	gdb_test "set language ada" ".*" "set language ada, conv var"

gdb_test_no_output ?

> +	gdb_test_no_output "set var \$astr := \"fghij\"" "set \$astr, conv var"
> +	gdb_test "printf \"astr val = %s\\n\", \$astr" "astr val = fghij" \
> +	    "printf \$astr, conv var"
> +	gdb_test "set language auto" ".*" "set language auto, conv var"

gdb_test_no_output ?


> +	gdb_test "printf \"astr val = %s\\n\", \$astr" "astr val = fghij" \
> +	    "printf \$astr, conv var, auto language"
> +	if ($do_wstring) {

Use {} instead of ().

> +	    gdb_test_no_output "set var \$wstr = L\"facile\"" \
> +		"set \$wstr, conv var"
> +	    gdb_test "printf \"wstr val = %ls\\n\", \$wstr" \
> +		"wstr val = facile" "printf \$wstr, conv var"
> +	}

All these "conv var" in the test names seem redundant, given the whole
proc body is wrapped in with_test_prefix.  How about replacing all
that with:

 -    with_test_prefix $prefix {
 +    with_test_prefix "conv var: $prefix" {

> +    }
> +}
> +
> +
>  # Start with a fresh gdb.
>  
>  gdb_exit
> @@ -948,6 +974,11 @@ gdb_test "ptype \"abc\"" " = char \\\[4\\\]"
>  gdb_test "print \$cvar = \"abc\"" " = \"abc\""
>  gdb_test "print sizeof (\$cvar)" " = 4"
>  
> +# Similarly, printf of convenience var should work without a target.

"of convenience var" -> "of a convenience var" or "of convenience vars".

Or maybe even: 

  printf of a string convenience var

> +# At this point, we cannot create wide strings convenience var, as the
> +# type wchar_t is not yet known, so skip the wide string tests.

"create wide strings convenience var" -> "create a wide string convenience var"

"wchar_t type" -> "wchar_t type"

> +test_printf_convenience_var "no target" 0
> +
>  # GDB used to complete the explicit location options even when
>  # printing expressions.
>  gdb_test_no_output "complete p -function"
> @@ -977,6 +1008,14 @@ if ![runto_main] then {
>      return 0
>  }
>  
> +# With a target, printf convenience var should of course work.

"With a running target"

"printf convenience vars"

> +test_printf_convenience_var "with target" 1
> +
> +# But it should also work when inferior function calls are forbidden.

"But it" -> "It".

> +gdb_test_no_output "set may-call-functions off"
> +test_printf_convenience_var "with target, may-call-functions off" 1
> +gdb_test_no_output "set may-call-functions on"
> +
>  test_integer_literals_accepted
>  test_integer_literals_rejected
>  test_float_accepted
> 

Thanks,
Pedro Alves