On 12/04/2016 04:55 PM, Martin Sebor wrote:
Bug 78519 points out that while the -Wformat warning flags a small
subset of sprintf calls with a null pointer argument to a %s directive
(those where the pointer is a constant) it misses the much bigger
set where the pointer is not a constant but instead is determined
to be null as a result of optimization. This is because -Wformat
runs too early, before any of the optimization passes that make it
possible to detect that non-constant pointers are null.
With the -Wformat-length warning running much later than -Wformat,
it's trivial to detect and diagnose these types of bugs with it.
The attached patch adds this warning, along with the ability to
detect a null destination pointer when it's required to be non-null
(this is in all of the {v,}sprintf functions and in {v,}snprintf
when the size argument is not zero).
Ultimately, the destination pointer argument (but not the format
string) to the {v,}sprintf functions needs to be declared nonnull
(pursuant to bug 78673) and the null-checking moved elsewhere.
I'm testing a follow-on patch that does just that but I post this
fix in the meantime since its main focus is the null %s argument.
Martin
gcc-78519.diff
PR middle-end/78519 - missing warning for sprintf %s with null pointer
gcc/ChangeLog:
PR middle-end/78519
* gimple-ssa-sprintf.c (format_string): Handle null pointers.
(format_directive): Diagnose null pointer arguments.
(pass_sprintf_length::handle_gimple_call): Diagnose null destination
pointers. Correct location of null format string in diagnostics.
gcc/testsuite/ChangeLog:
PR middle-end/78519
* gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: New test.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index e86c4dc..7004f09 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -433,7 +433,7 @@ struct result_range
struct fmtresult
{
fmtresult ()
- : argmin (), argmax (), knownrange (), bounded (), constant ()
+ : argmin (), argmax (), knownrange (), bounded (), constant (), nullp ()
{
range.min = range.max = HOST_WIDE_INT_MAX;
}
@@ -461,6 +461,9 @@ struct fmtresult
are also constant (such as determined by constant propagation,
though not value range propagation). */
bool constant;
+
+ /* True when the argument is a null pointer. */
+ bool nullp;
};
/* Description of a conversion specification. */
@@ -1624,6 +1627,20 @@ format_string (const conversion_spec &spec, tree arg)
res.range.min = 0;
}
}
+ else if (arg && integer_zerop (arg))
+ {
+ /* Handle null pointer argument. */
+
+ fmtresult res;
+ /* Set the range based on Glibc "(null)" output but leave
+ all other members at default to indicate that the range
+ isn't trustworthy. This allows the rest of the format
+ string to be checked for problems. */
By not trustworthy, I guess you mean it's only used to issue "may be"
style warnings, right? What benefit do you gain by encoding the
glib-ism vs using HOST_WIDE_INT_MAX? Presumably once you use
HOST_WIDE_INT_MAX nothing else is going to be checked?
Jeff
