[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

--- Comment #9 from Aldy Hernandez  ---
Author: aldyh
Date: Wed Sep 13 16:50:00 2017
New Revision: 252389

URL: https://gcc.gnu.org/viewcvs?rev=252389&root=gcc&view=rev
Log:
PR c++/81586 - valgrind error in output_buffer_append_r with -Wall

gcc/ChangeLog:

PR c++/81586
* pretty-print.c (pp_format): Correct the handling of %s precision.

Modified:
branches/range-gen2/gcc/ChangeLog
branches/range-gen2/gcc/pretty-print.c

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

--- Comment #8 from Martin Sebor  ---
Author: msebor
Date: Thu Aug 10 17:40:11 2017
New Revision: 251029

URL: https://gcc.gnu.org/viewcvs?rev=251029&root=gcc&view=rev
Log:
PR c++/81586 - valgrind error in output_buffer_append_r with -Wall

gcc/ChangeLog:

PR c++/81586
* pretty-print.c (pp_format): Correct the handling of %s precision.


Modified:
trunk/gcc/ChangeLog
trunk/gcc/pretty-print.c

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

Martin Sebor  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #7 from Martin Sebor  ---
Fixed in r251029.

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

--- Comment #6 from David Binderman  ---
(In reply to Martin Sebor from comment #5)
> Patch: https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01866.html

Did this patch ever get into trunk gcc ?

I have some evidence that gcc trunk revision 250947
doesn't have it.

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

Martin Sebor  changed:

   What|Removed |Added

   Keywords||patch

--- Comment #5 from Martin Sebor  ---
Patch: https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01866.html

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

Martin Sebor  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |msebor at gcc dot 
gnu.org

--- Comment #4 from Martin Sebor  ---
Let me fix it.

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

--- Comment #3 from Martin Sebor  ---
I don't see a problem with the code in maybe_warn.  It does this:

  /* Buffer for the directive in the host character set (used when
 the source character set is different).  */
  char hostdir[32];
  ...
  return fmtwarn (dirloc, pargrange, NULL,
  info.warnopt (), fmtstr, dir.len,
  target_to_host (hostdir, sizeof hostdir, dir.beg),
  res.min, navail);

The call being made has fmtstr = "%<%.*s%> directive output truncated writing
%wu bytes into a region of size %wu", dir.len = 73, and hostdir =
"af_get_next_segv end of file..." (with strlen (hostdir) == 31).  With that,
while the "%.*s" directive in fmtstr says to read at most 73 bytes from
hostdir, since hostdir is only 31 characters long, the directive should read
exactly that many and no more than that.

I think the bug is actually in pp_format where the "%.*s" directive is handled:

case '.':
  {
int n;
const char *s;

/* We handle '%.Ns' and '%.*s' or '%M$.*N$s'
   (where M == N + 1).  The format string should be verified
   already from the first phase.  */
p++;
if (ISDIGIT (*p))
  {
char *end;
n = strtoul (p, &end, 10);
p = end;
gcc_assert (*p == 's');
  }
else
  {
gcc_assert (*p == '*');
p++;
gcc_assert (*p == 's');
n = va_arg (*text->args_ptr, int);
^

Here n is extracted from the variable argument list (the corresponding argument
is dir.len).

/* This consumes a second entry in the formatters array.  */
gcc_assert (formatters[argno] == formatters[argno+1]);
argno++;
  }

s = va_arg (*text->args_ptr, const char *);
pp_append_text (pp, s, s + n);
^

Here s points to hostdir and n is 73, but strlen(s) is just 31.  This code
doesn't handle the precision correctly.  The precision is the maximum number of
non-nul characters to print.  The fix is to set n to be no greater than
strlen(s):

if (strlen (s) < n)
  n = strlen (s);
pp_append_text (pp, s, s + n);

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

Martin Liška  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-07-27
 CC||marxin at gcc dot gnu.org,
   ||msebor at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #2 from Martin Liška  ---
Confirmed, following patch will be needed:

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 644cf7e33b1..72ec10a77b9 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -2500,7 +2500,7 @@ maybe_warn (substring_loc &dirloc, source_range
*pargrange,
  : G_("%<%.*s%> directive writing %wu bytes "
   "into a region of size %wu")));
  return fmtwarn (dirloc, pargrange, NULL,
- info.warnopt (), fmtstr, dir.len,
+ info.warnopt (), fmtstr, MIN (dir.len, 31),
  target_to_host (hostdir, sizeof hostdir, dir.beg),
  res.min, navail);
}

I'm not sure about the constant (maybe 32) and there are multiple invocations
of the fmtwarn function. Leaving to Martin.

[Bug c++/81586] valgrind error in output_buffer_append_r with -Wall

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81586

--- Comment #1 from David Binderman  ---
The following much reduced C++ code seems to demonstrate the bug:

extern "C" int snprintf(char *, unsigned long, const char *...) ;
struct  S {
char * a;
};
void f( S * af)
{
snprintf(af->a, sizeof(af), "af_get_next_segv end of file reading
segmen
t tail AFF file is truncated 0");
}