[Bug c/80354] Poor support to silence -Wformat-truncation=1

[msebor at gcc dot gnu.org]

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

--- Comment #11 from Martin Sebor  ---
(In reply to Alejandro Colomar from comment #10)
> Many other warnings are supressed with (void), why is this one so special?

Not too many warnings can be suppressed by casts.  Those that can must be at
least partially implemented in the front end where otherwise pointless casts
are still visible.  -Wformat-truncation (and other warnings like it) is
implemented in the middle-end to benefit from optimizations such as constant
propagation or value range optimization.  Semantically pointless casts have
long been removed from expressions by the time the warning sees the program.

It might be possible to use a cast as a suppression mechanism for this warning,
but I'm not convinced it would be helpful.  Casting value-returning function
calls to void is a fairly common style used to denote that the return value is
unused.  In the case of snprintf, that doesn't necessarily imply that the
program is prepared to deal correctly with the truncated output.  In my
experience very few programs are.  Most assume it simply doesn't happen.  I
don't recall coming across an example involving a non-trivial format string
with arguments, especially of numeric types, where truncation would not have
caused some sort of an unexpected condition later on in the program.

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[colomar.6.4.3 at gmail dot com]

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

Alejandro Colomar  changed:

   What|Removed |Added

 CC||colomar.6.4.3 at gmail dot com

--- Comment #10 from Alejandro Colomar  ---
In some cases, that warning saved me from writing dangerous code (using file
paths), and it's been very useful.  But casting to void means one and only one
thing:  _I DO know what I'm doing and I do NOT care, so STFU!_
And if the casting to void is dangerous (ie. casting to void the result of
storing a path), that's my fault.

I currently have a program where I want, for some reason, to truncate the
output to an 80-char line:


#include 

#define LINE_SIZE   (80)

int main(int argc, char *argv[])
{
chartruncated[LINE_SIZE];

(void)snprintf(truncated, LINE_SIZE, "I DO want to truncate this line "
"to a 80 char line, and discard any remaining text so "
"that it is not displayed or stored anywhere because I
"
"don't care about it. "
" Usually one would check for the return value of "
"snprintf, but in this precise situation, I do not "
"care about it, so the explicit cast to (void) should "
"silence the warning, just like it does in other "
"warnings such as -Wunused-variable.");

printf("%s\n", truncated);

return 0;
}


But in the same project I have paths and that kind of uses of snprintf where I
still want to have the warnings enabled (and even more, -Werror so that they
never compile):


if (snprintf(file_name, FILENAME_MAX, "%s/%s", file_path, saved_name))
{
goto err_path;
}


Many other warnings are supressed with (void), why is this one so special?


Anyways, I'm reporting a new bug, because this one is "RESOLVED INVALID".

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[msharov at users dot sourceforge.net]

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

--- Comment #9 from Mike Sharov  ---
(In reply to Martin Sebor from comment #8)
> A simple way to avoid the warning while also avoiding bugs resulting from
> unhandled truncation is to detect it and abort if it happens, e.g.

First of all, you might want to mention this in the error message. The way it
is presently worded gives the impression that the only way to remove the
warning is to increase the buffer size. I guarantee you that most people will
just turn off the warning in this case. And then come here to complain, because
the kind of warning that is wrong in most cases (if only in our opinion) should
not be in -Wall.

Secondly, this is precisely the annoying part about it: you are making the
decision that allowing truncation to happen is always a bug and forcing it to
be handled as one. I do not consider it a problem to pass a truncated filename
to open and having it fail there. There are, naturally, some cases where this
could cause a security problem, but I am the one who should determine whether
each particular snprintf is one of those cases, and consequently I should also
have the option to tell the compiler that it is not. If I was ok with bloating
my program due to an excessive concern with safety, I'd be using Java, not C.

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[msebor at gcc dot gnu.org]

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

--- Comment #8 from Martin Sebor  ---
The problem in the pathname example is one of the bugs the warning is meant to
prevent.  Allowing a pathname to be silently truncated can lead to bugs -- see
CWE 22 for some background and CVE-2002-0499 for an example of a vulnerability
that can result from it.

A simple way to avoid the warning while also avoiding bugs resulting from
unhandled truncation is to detect it and abort if it happens, e.g., like so:

  struct Path { char a[256]; };

  void f (struct Path *d, const struct Path *s, int i)
  {
int n = snprintf (d->a, sizeof d->a, "%s%d", s->a, i);
if ((size_t)n > sizeof d->a)
  abort ();

// use d->a
  }

Many warnings have a non-zero false positive rate, certainly all those that
depend on data or flow analysis, but the vast majority of them, certainly all
those in -Wall and -Wextra, try to strike a reasonable balance between false
and true positives, based on building entire Linux distributions.  If you're
confident that the rate of GCC warnings for your code is 100% then the
appropriate mechanism to let the compiler know that "you know what you're
doing" and don't need its help in detecting bugs is to turn warnings off,
either individually of wholesale via -w.

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[msharov at users dot sourceforge.net]

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

Mike Sharov  changed:

   What|Removed |Added

 CC||msharov at users dot 
sourceforge.n
   ||et

--- Comment #7 from Mike Sharov  ---
I really do have to add my complaint about this one. Can't we have another
override option here? Have the compiler parse "truncates" in a comment, for
example, like it does for fallthrough. Doing format precision is not a good
workaround because it hardcodes the size of the buffer into the format string,
creating a maintenance problem in case the buffer size is increased later. Not
to mention unnecessarily creating multiple format strings where previously a
single one could have been shared. Why make us all create unnecessarily larger
executables?

Worse, truncation is always going to be a false positive here. Nobody wants to
choose buffer size based on worst case output. Sometimes it is merely useless,
such as when writing diagnostic messages. 8k of text won't fit in a message box
anyway and will be truncated. Other times it is distinctly wrong. For example,
if building a path from multiple components in PATH_MAX sized buffers, the
result must not be larger than PATH_MAX anyway, and must be truncated. Another
example is when you are trying to get a prefix from a large string. snprintf is
a great way of doing that, but your warning may now lead people to rewrite the
code with strncpy and its insecure behavior, possibly forgetting that it always
requires explicitly terminating the buffer.

Sure, it is just another warning to fix. I've had to fix some new warning with
every gcc release. Not a single one of them was an actual problem with the
code. It's always just "the way we've got to do things from now on", having to
write each code construct in a particular way to avoid a warning. A 100% false
positive rate is annoying, isn't it? Yet, I keep all warnings on, for some
strange reason. Can't we all be friends and always have a polite way of saying
"I know what I am doing here"?

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[toojays at toojays dot net]

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

John Steele Scott  changed:

   What|Removed |Added

 CC||toojays at toojays dot net

--- Comment #6 from John Steele Scott  ---
(In reply to Stephan Bergmann from comment #5)
> (In reply to Martin Sebor from comment #3)
> > The warning does just what it's designed to do: point out the potential
> > unhandled truncation.
> 
> But it is unusable in practice if there is no reliable way to silence false
> positives.

^- This!

A simple cast-to-void is the conventional way to indicate to the compiler that
it should consider the value as having been consumed even though it wasn't
really. It's a real shame if this information is not available to the code that
generates this warning.

(In reply to Martin Sebor from comment #1)
> Besides actually handling the truncation (e.g., branching on
> it and taking some action that does affect the behavior), storing the return
> value in a volatile variable and reading it should suppress it.

Thanks for the tip. For isolated instances this may be less invasive than what
I have been contemplating (checking the return value and calling some no-op
function). It would be better if there were some solution that didn't generate
unnecessary code though.

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[sbergman at redhat dot com]

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

--- Comment #5 from Stephan Bergmann  ---
(In reply to Martin Sebor from comment #3)
> The warning does just what it's designed to do: point out the potential
> unhandled truncation.

But it is unusable in practice if there is no reliable way to silence false
positives.

> If the argument values are such that the truncation
> cannot occur then using snprintf is unnecessary and sprintf can be used
> instead.  Otherwise, if there is a combination of argument values that can
> result in truncation a warning is issued.  Note that the length of output
> produced by each directive can be constrained by specifying a precision for
> %s (e.g., "%.24s" if arena->m_name in the LibreOffice code cannot be longer
> than 24 characters), or by asserting that an integer argument is in some
> limited range of its type (or by using a narrower type to store it).

None of that applies in the case I mentioned, where an at-most 31-character
prefix of "%s_%lu" shall be produced, where the %s argument is known to be a
string of 0..31 characters and the %lu argument is an effectively unconstrained
value of type unsigned long.

> Like all warnings that depend on data flow analysis it is subject to false
> positives but there is no evidence to suggest that on balance it's unhelpful
> or difficult to use.  Quite the contrary.

One cannot even silence the warning locally with a #pragma GCC diagnostic
push/ignored "-Wformat-truncation"/pop just around that call to snprintf:  The
warning is reported on the first line of the function definition containing the
call (see below), and the pragma is only effective if the push/ignored
"-Wformat-truncation" part precedes that first line of the whole function
definition.

> /data/sbergman/lo-gcc/core/sal/rtl/alloc_arena.cxx: In function 
> ‘rtl_arena_type* {anonymous}::rtl_arena_activate(rtl_arena_type*, const 
> char*, sal_Size, sal_Size, rtl_arena_type*, void* (*)(rtl_arena_type*, 
> sal_Size*), void (*)(rtl_arena_type*, void*, sal_Size))’:
> /data/sbergman/lo-gcc/core/sal/rtl/alloc_arena.cxx:672:1: error: ‘%lu’ 
> directive output may be truncated writing between 1 and 20 bytes into a 
> region of size between 0 and 31 [-Werror=format-truncation=]
>  rtl_arena_activate (
>  ^~
> /data/sbergman/lo-gcc/core/sal/rtl/alloc_arena.cxx:717:17: note: ‘snprintf’ 
> output between 3 and 53 bytes into a destination of size 32
>  (void) snprintf (namebuf, sizeof(namebuf), "%s_%" 
> SAL_PRIuUINTPTR, arena->m_name, size);
>  
> ^~~

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[egall at gwmail dot gwu.edu]

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

Eric Gallager  changed:

   What|Removed |Added

 CC||egall at gwmail dot gwu.edu

--- Comment #4 from Eric Gallager  ---
(In reply to Martin Sebor from comment #3)
> The warning does just what it's designed to do: point out the potential
> unhandled truncation.  If the argument values are such that the truncation
> cannot occur then using snprintf is unnecessary and sprintf can be used
> instead.

There's other code checking tools (e.g. splint) that say to never use sprintf
and to always use snprintf instead; the manpage on my computer for sprintf also
says to always use snprintf instead. For this reason some projects do
#pragma GCC poison sprintf
in a header file, so sprintf can't actually be used instead in those cases.

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[msebor at gcc dot gnu.org]

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

--- Comment #3 from Martin Sebor  ---
The warning does just what it's designed to do: point out the potential
unhandled truncation.  If the argument values are such that the truncation
cannot occur then using snprintf is unnecessary and sprintf can be used
instead.  Otherwise, if there is a combination of argument values that can
result in truncation a warning is issued.  Note that the length of output
produced by each directive can be constrained by specifying a precision for %s
(e.g., "%.24s" if arena->m_name in the LibreOffice code cannot be longer than
24 characters), or by asserting that an integer argument is in some limited
range of its type (or by using a narrower type to store it).

Like all warnings that depend on data flow analysis it is subject to false
positives but there is no evidence to suggest that on balance it's unhelpful or
difficult to use.  Quite the contrary.

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[sbergman at redhat dot com]

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

--- Comment #2 from Stephan Bergmann  ---
But that makes this warning extremely hard to use.  Is it really useful for
-Wall in that case?

I came across this with a real-world use-case in the LibreOffice code base,
where some code deliberately uses snprintf to produce a fixed-size prefix, see
 "WaE: ‘%lu’ directive output may be
truncated".  (And older builds of GCC towards GCC 7 did not emit that unhelpful
warning with -Wall => -Wformat-truncation=1, but would only have emitted it
with an explicit -Wformat-truncation=2.)

Addressing false positives for this warning thus becomes an unpleasant
whack-a-mole (given different builds of the code are done with different
optimization switches), unpleasant enough so that we'll likely have to use
-Wall -Wformat-truncation=0 for LibreOffice.  Which is unfortunate, given that
casting-to-void would be a well-understood and well-accepted way to silence
false positives (but one hard to implement in GCC, if I understand you
correctly).

[Bug c/80354] Poor support to silence -Wformat-truncation=1

[msebor at gcc dot gnu.org]

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

Martin Sebor  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |INVALID

--- Comment #1 from Martin Sebor  ---
This is the expected behavior.  Whether or not a function's return value is
considered used depends on optimization.  Simply assigning it to a variable and
doing nothing with it that affects the behavior of the program is not in this
situation considered using it.  This applies to all the cases here.  This isn't
a property of the warning itself but rather that of its dependency on prior
optimizations (which eliminate code that doesn't actually use the value).

The idea is that programs that allow output truncation to go undetected are
likely buggy.  Besides actually handling the truncation (e.g., branching on it
and taking some action that does affect the behavior), storing the return value
in a volatile variable and reading it should suppress it.