Off-by-one error in windows-nat.c causes abort at startup

I created a gdb.ini file in my home directory, and suddenly found that
almost all my GDB binaries stopped working.  Even "gdb --version"
would crash at startup thusly:

  ./common/common-utils.c:141: internal-error: xsnprintf: Assertion `ret < size' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Quit this debugging session? (y or n) [answered Y; input not from terminal]

  This is a bug, please report it.  For instructions, see:
  <http://www.gnu.org/software/gdb/bugs/>.

  ./common/common-utils.c:141: internal-error: xsnprintf: Assertion `ret < size' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  Create a core file of GDB? (y or n) [answered Y; input not from terminal]

  This application has requested the Runtime to terminate it in an unusual way.
  Please contact the application's support team for more information.

Luckily, I still had GDB 7.5, which did work.  Using it, I found the
off-by-one gotcha below (".gdbinit" is one character longer than
"gdb.ini").  I guess no one tested this feature when we switched from
using snprintf to xsnprintf...

OK to commit (with a suitable ChangeLog entry, of course)?

On 04/30/2016 12:07 PM, Eli Zaretskii wrote:

> Luckily, I still had GDB 7.5, which did work.  Using it, I found the
> off-by-one gotcha below (".gdbinit" is one character longer than
> "gdb.ini").  I guess no one tested this feature when we switched from
> using snprintf to xsnprintf...

Sounds like gdb would corrupt memory before we switched to xsnprintf 
then.  I'd say the problem is that the feature was added without a
corresponding test case.

> OK to commit (with a suitable ChangeLog entry, of course)?

Sure.

> 
> --- gdb/windows-nat.c~	2016-02-10 05:19:39.000000000 +0200
> +++ gdb/windows-nat.c	2016-04-30 11:57:08.500000000 +0300
> @@ -2711,9 +2711,9 @@ _initialize_check_for_gdb_ini (void)
>        if (access (oldini, 0) == 0)
>  	{
>  	  int len = strlen (oldini);
> -	  char *newini = (char *) alloca (len + 1);
> +	  char *newini = (char *) alloca (len + 2);
>  
> -	  xsnprintf (newini, len + 1, "%.*s.gdbinit",
> +	  xsnprintf (newini, len + 2, "%.*s.gdbinit",
>  		     (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
>  	  warning (_("obsolete '%s' found. Rename to '%s'."), oldini, newini);

(I suspect this whole function could be rewritten in a clearer form...)

Thanks,
Pedro Alves

> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 2 May 2016 12:50:05 +0100
> 
> On 04/30/2016 12:07 PM, Eli Zaretskii wrote:
> 
> > Luckily, I still had GDB 7.5, which did work.  Using it, I found the
> > off-by-one gotcha below (".gdbinit" is one character longer than
> > "gdb.ini").  I guess no one tested this feature when we switched from
> > using snprintf to xsnprintf...
> 
> Sounds like gdb would corrupt memory before we switched to xsnprintf 
> then.  I'd say the problem is that the feature was added without a
> corresponding test case.
> 
> > OK to commit (with a suitable ChangeLog entry, of course)?
> 
> Sure.

Thanks, pushed.

> > --- gdb/windows-nat.c~	2016-02-10 05:19:39.000000000 +0200
> > +++ gdb/windows-nat.c	2016-04-30 11:57:08.500000000 +0300
> > @@ -2711,9 +2711,9 @@ _initialize_check_for_gdb_ini (void)
> >        if (access (oldini, 0) == 0)
> >  	{
> >  	  int len = strlen (oldini);
> > -	  char *newini = (char *) alloca (len + 1);
> > +	  char *newini = (char *) alloca (len + 2);
> >  
> > -	  xsnprintf (newini, len + 1, "%.*s.gdbinit",
> > +	  xsnprintf (newini, len + 2, "%.*s.gdbinit",
> >  		     (int) (len - (sizeof ("gdb.ini") - 1)), oldini);
> >  	  warning (_("obsolete '%s' found. Rename to '%s'."), oldini, newini);
> 
> (I suspect this whole function could be rewritten in a clearer form...)

Like not use xsnprintf at all, and instead use strcpy/strcat, perhaps?