This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] enable -Wformat-length for dynamically allocated buffers (pr 78245)
That said, I defer to you on how to proceed here. I'm prepared
to do the work(*) but I do worry about jeopardizing the chances
of this patch and the others making it into 7.0.
So would it make sense to just init/fini the b_o_s framework in your
pass and for builtin expansion?
I think that should work for the sprintf checking. Let me test it.
We can deal with the memxxx and strxxx patch (53562) independently
if you prefer.
Attached is a modified patch that calls {init,fini}_object_sizes()
from the gimple-ssa-sprintf pass instead.
While this works fine, I do like the approach of making the calls
in a single function better because it makes for a more robust API.
Decoupling the init/fini calls from the compute_object_size()
function that depends on them having been made makes the API easier
to accidentally misuse by calling one while forgetting to call one
or both of the other two.
Martin
PR middle-end/78245 - missing -Wformat-length on an overflow of a dynamically allocated buffer
gcc/testsuite/ChangeLog:
PR middle-end/78245
* gcc.dg/tree-ssa/builtin-sprintf-warn-3.c: Add tests.
gcc/ChangeLog:
PR middle-end/78245
* gimple-ssa-sprintf.c (get_destination_size): Call
compute_object_size.
* tree-object-size.c (addr_object_size): Adjust.
(pass_through_call): Adjust.
(internal_object_size): New function.
(compute_builtin_object_size): Call internal_object_size.
(pass_object_sizes::execute): Adjust.
* tree-object-size.h (fini_object_sizes): Declare.
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index ead8b0e..34b3723 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -2466,6 +2466,9 @@ get_destination_size (tree dest)
a member array as opposed to the whole enclosing object), otherwise
use type-zero object size to determine the size of the enclosing
object (the function fails without optimization in this type). */
+
+ init_object_sizes ();
+
int ost = optimize > 0;
unsigned HOST_WIDE_INT size;
if (compute_builtin_object_size (dest, ost, &size))
@@ -2800,6 +2803,8 @@ pass_sprintf_length::execute (function *fun)
}
}
+ fini_object_sizes ();
+
return 0;
}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
index 8d97fa8..9874332 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-3.c
@@ -1,5 +1,10 @@
/* { dg-do compile } */
-/* { dg-options "-std=c99 -O2 -Wformat -Wformat-length=1 -ftrack-macro-expansion=0" } */
+/* { dg-options "-O2 -Wformat -Wformat-length=1 -ftrack-macro-expansion=0" } */
+/* Verify that all sprintf built-ins detect overflow involving directives
+ with non-constant arguments known to be constrained by some range of
+ values, and even when writing into dynamically allocated buffers.
+ -O2 (-ftree-vrp) is necessary for the tests involving ranges to pass,
+ otherwise -O1 is sufficient. */
#ifndef LINE
# define LINE 0
@@ -7,18 +12,26 @@
#define bos(x) __builtin_object_size (x, 0)
-#define T(bufsize, fmt, ...) \
- do { \
- if (!LINE || __LINE__ == LINE) \
- { \
- char *d = (char *)__builtin_malloc (bufsize); \
- __builtin___sprintf_chk (d, 0, bos (d), fmt, __VA_ARGS__); \
- sink (d); \
- } \
- } while (0)
+/* Defined (and redefined) to the allocation function to use, either
+ malloc, or alloca, or a VLA. */
+#define ALLOC(p, n) (p) = __builtin_malloc (n)
-void
-sink (void*);
+/* Defined (and redefined) to the sprintf function to exercise. */
+#define TEST_SPRINTF(d, maxsize, objsize, fmt, ...) \
+ __builtin___sprintf_chk (d, 0, objsize, fmt, __VA_ARGS__)
+
+#define T(bufsize, fmt, ...) \
+ do { \
+ if (!LINE || __LINE__ == LINE) \
+ { \
+ char *d; \
+ ALLOC (d, bufsize); \
+ TEST_SPRINTF (d, 0, bos (d), fmt, __VA_ARGS__); \
+ sink (d); \
+ } \
+ } while (0)
+
+void sink (void*);
/* Identity function to verify that the checker figures out the value
of the operand even when it's not constant (i.e., makes use of
@@ -232,3 +245,88 @@ void test_sprintf_chk_range_sshort (signed short *a, signed short *b)
T ( 4, "%i", Ra (998, 999));
T ( 4, "%i", Ra (999, 1000)); /* { dg-warning "may write a terminating nul past the end of the destination" } */
}
+
+/* Exercise ordinary sprintf with malloc. */
+#undef TEST_SPRINTF
+#define TEST_SPRINTF(d, maxsize, objsize, fmt, ...) \
+ __builtin_sprintf (d, fmt, __VA_ARGS__)
+
+void test_sprintf_malloc (const char *s, const char *t)
+{
+#define x x ()
+
+ T (1, "%-s", x ? "" : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : ""); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? s : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : s); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? s : t);
+
+ T (2, "%-s", x ? "" : "1");
+ T (2, "%-s", x ? "" : s);
+ T (2, "%-s", x ? "1" : "");
+ T (2, "%-s", x ? s : "");
+ T (2, "%-s", x ? "1" : "2");
+ T (2, "%-s", x ? "" : "12"); /* { dg-warning "nul past the end" } */
+ T (2, "%-s", x ? "12" : ""); /* { dg-warning "nul past the end" } */
+
+ T (2, "%-s", x ? "" : "123"); /* { dg-warning "into a region" } */
+ T (2, "%-s", x ? "123" : ""); /* { dg-warning "into a region" } */
+
+#undef x
+}
+
+/* Exercise ordinary sprintf with alloca. */
+#undef ALLOC
+#define ALLOC(p, n) (p) = __builtin_alloca (n)
+
+void test_sprintf_alloca (const char *s, const char *t)
+{
+#define x x ()
+
+ T (1, "%-s", x ? "" : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : ""); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? s : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : s); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? s : t);
+
+ T (2, "%-s", x ? "" : "1");
+ T (2, "%-s", x ? "" : s);
+ T (2, "%-s", x ? "1" : "");
+ T (2, "%-s", x ? s : "");
+ T (2, "%-s", x ? "1" : "2");
+ T (2, "%-s", x ? "" : "12"); /* { dg-warning "nul past the end" } */
+ T (2, "%-s", x ? "12" : ""); /* { dg-warning "nul past the end" } */
+
+ T (2, "%-s", x ? "" : "123"); /* { dg-warning "into a region" } */
+ T (2, "%-s", x ? "123" : ""); /* { dg-warning "into a region" } */
+
+#undef x
+}
+
+/* Exercise ordinary sprintf with a VLA. */
+#undef ALLOC
+#define ALLOC(p, n) char vla [i (n)]; (p) = vla
+
+void test_sprintf_vla (const char *s, const char *t)
+{
+#define x x ()
+
+ T (1, "%-s", x ? "" : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : ""); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? s : "1"); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? "1" : s); /* { dg-warning "nul past the end" } */
+ T (1, "%-s", x ? s : t);
+
+ T (2, "%-s", x ? "" : "1");
+ T (2, "%-s", x ? "" : s);
+ T (2, "%-s", x ? "1" : "");
+ T (2, "%-s", x ? s : "");
+ T (2, "%-s", x ? "1" : "2");
+ T (2, "%-s", x ? "" : "12"); /* { dg-warning "nul past the end" } */
+ T (2, "%-s", x ? "12" : ""); /* { dg-warning "nul past the end" } */
+
+ T (2, "%-s", x ? "" : "123"); /* { dg-warning "into a region" } */
+ T (2, "%-s", x ? "123" : ""); /* { dg-warning "into a region" } */
+
+#undef x
+}
diff --git a/gcc/tree-object-size.c b/gcc/tree-object-size.c
index 1317ad7..3a2d54d 100644
--- a/gcc/tree-object-size.c
+++ b/gcc/tree-object-size.c
@@ -50,6 +50,7 @@ static const unsigned HOST_WIDE_INT unknown[4] = {
0
};
+static bool internal_object_size (tree, int, unsigned HOST_WIDE_INT *);
static tree compute_object_offset (const_tree, const_tree);
static bool addr_object_size (struct object_size_info *,
const_tree, int, unsigned HOST_WIDE_INT *);
@@ -187,7 +188,7 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
if (!osi || (object_size_type & 1) != 0
|| TREE_CODE (TREE_OPERAND (pt_var, 0)) != SSA_NAME)
{
- compute_builtin_object_size (TREE_OPERAND (pt_var, 0),
+ internal_object_size (TREE_OPERAND (pt_var, 0),
object_size_type & ~1, &sz);
}
else
@@ -491,14 +492,14 @@ pass_through_call (const gcall *call)
}
-/* Compute __builtin_object_size value for PTR and set *PSIZE to
- the resulting value. OBJECT_SIZE_TYPE is the second argument
- to __builtin_object_size. Return true on success and false
- when the object size could not be determined. */
+/* Compute the size of the object pointed to by PTR and set *PSIZE
+ to the resulting value. OBJECT_SIZE_TYPE is the second argument
+ to __builtin_object_size. Return true on success and false when
+ the object size could not be determined. */
bool
-compute_builtin_object_size (tree ptr, int object_size_type,
- unsigned HOST_WIDE_INT *psize)
+internal_object_size (tree ptr, int object_size_type,
+ unsigned HOST_WIDE_INT *psize)
{
gcc_assert (object_size_type >= 0 && object_size_type <= 3);
@@ -534,7 +535,7 @@ compute_builtin_object_size (tree ptr, int object_size_type,
ptr = gimple_assign_rhs1 (def);
if (cst_and_fits_in_hwi (offset)
- && compute_builtin_object_size (ptr, object_size_type, psize))
+ && internal_object_size (ptr, object_size_type, psize))
{
/* Return zero when the offset is out of bounds. */
unsigned HOST_WIDE_INT off = tree_to_shwi (offset);
@@ -664,6 +665,18 @@ compute_builtin_object_size (tree ptr, int object_size_type,
return *psize != unknown[object_size_type];
}
+/* Compute __builtin_object_size value for PTR and set *PSIZE to
+ the resulting value. OBJECT_SIZE_TYPE is the second argument
+ to __builtin_object_size. Return true on success and false
+ when the object size could not be determined. */
+
+bool
+compute_builtin_object_size (tree ptr, int object_size_type,
+ unsigned HOST_WIDE_INT *psize)
+{
+ return internal_object_size (ptr, object_size_type, psize);
+}
+
/* Compute object_sizes for PTR, defined to VALUE, which is not an SSA_NAME. */
static void
@@ -1230,7 +1243,7 @@ init_object_sizes (void)
/* Destroy data structures after the object size computation. */
-static void
+void
fini_object_sizes (void)
{
int object_size_type;
@@ -1325,7 +1338,7 @@ pass_object_sizes::execute (function *fun)
{
tree type = TREE_TYPE (lhs);
unsigned HOST_WIDE_INT bytes;
- if (compute_builtin_object_size (ptr, object_size_type,
+ if (internal_object_size (ptr, object_size_type,
&bytes)
&& wi::fits_to_tree_p (bytes, type))
{
diff --git a/gcc/tree-object-size.h b/gcc/tree-object-size.h
index 38c3e07..b656339 100644
--- a/gcc/tree-object-size.h
+++ b/gcc/tree-object-size.h
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see
#define GCC_TREE_OBJECT_SIZE_H
extern void init_object_sizes (void);
+extern void fini_object_sizes (void);
extern bool compute_builtin_object_size (tree, int, unsigned HOST_WIDE_INT *);
#endif // GCC_TREE_OBJECT_SIZE_H