* [PATCH] tree-object-size: Support strndup and strdup
@ 2022-08-15 19:23 Siddhesh Poyarekar
2022-08-29 14:16 ` Siddhesh Poyarekar
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-08-15 19:23 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub
Use string length of input to strdup to determine the usable size of the
resulting object. Avoid doing the same for strndup since there's a
chance that the input may be too large, resulting in an unnecessary
overhead or worse, the input may not be NULL terminated, resulting in a
crash where there would otherwise have been none.
gcc/ChangeLog:
* tree-object-size.cc (get_whole_object): New function.
(addr_object_size): Use it.
(strdup_object_size): New function.
(call_object_size): Use it.
(pass_data_object_sizes, pass_data_early_object_sizes): Set
todo_flags_finish to TODO_update_ssa_no_phi.
gcc/testsuite/ChangeLog:
* gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
test_strndup, test_strdup_min, test_strndup_min): New tests.
(main): Call them.
* gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
warnings.
* gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
* gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
* gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
* gcc.dg/builtin-object-size-1.c: Silence overread warnings.
Declare free, strdup and strndup.
(test11): New test.
(main): Call it.
* gcc.dg/builtin-object-size-2.c: Silence overread warnings.
Declare free, strdup and strndup.
(test9): New test.
(main): Call it.
* gcc.dg/builtin-object-size-3.c: Silence overread warnings.
Declare free, strdup and strndup.
(test11): New test.
(main): Call it.
* gcc.dg/builtin-object-size-4.c: Silence overread warnings.
Declare free, strdup and strndup.
(test9): New test.
(main): Call it.
---
.../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++++
.../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
.../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
.../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
.../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
gcc/testsuite/gcc.dg/builtin-object-size-1.c | 64 +++++++++++++++-
gcc/testsuite/gcc.dg/builtin-object-size-2.c | 63 ++++++++++++++-
gcc/testsuite/gcc.dg/builtin-object-size-3.c | 63 ++++++++++++++-
gcc/testsuite/gcc.dg/builtin-object-size-4.c | 63 ++++++++++++++-
gcc/tree-object-size.cc | 76 +++++++++++++++++--
10 files changed, 366 insertions(+), 14 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
index 01a280b2d7b..7f023708b15 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
@@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start, size_t end, int incr)
return __builtin_dynamic_object_size (ptr, 0);
}
+/* strdup/strndup. */
+
+size_t
+__attribute__ ((noinline))
+test_strdup (const char *in)
+{
+ char *res = __builtin_strdup (in);
+ return __builtin_dynamic_object_size (res, 0);
+}
+
+size_t
+__attribute__ ((noinline))
+test_strndup (const char *in, size_t bound)
+{
+ char *res = __builtin_strndup (in, bound);
+ return __builtin_dynamic_object_size (res, 0);
+}
+
+size_t
+__attribute__ ((noinline))
+test_strdup_min (const char *in)
+{
+ char *res = __builtin_strdup (in);
+ return __builtin_dynamic_object_size (res, 2);
+}
+
+size_t
+__attribute__ ((noinline))
+test_strndup_min (const char *in, size_t bound)
+{
+ char *res = __builtin_strndup (in, bound);
+ return __builtin_dynamic_object_size (res, 2);
+}
+
/* Other tests. */
struct TV4
@@ -651,6 +685,15 @@ main (int argc, char **argv)
int *t = test_pr105736 (&val3);
if (__builtin_dynamic_object_size (t, 0) != -1)
FAIL ();
+ const char *str = "hello world";
+ if (test_strdup (str) != __builtin_strlen (str) + 1)
+ FAIL ();
+ if (test_strndup (str, 4) != 5)
+ FAIL ();
+ if (test_strdup_min (str) != __builtin_strlen (str) + 1)
+ FAIL ();
+ if (test_strndup_min (str, 4) != 0)
+ FAIL ();
if (nfails > 0)
__builtin_abort ();
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
index 7cc8b1c9488..8f17c8edcaf 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
index 267dbf48ca7..3677782ff1c 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
index fb9dc56da7e..5b6987b7773 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
index 870548b4206..9d796224e96 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
index b772e2da9b9..4fbd372d97a 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -629,6 +632,64 @@ test10 (void)
}
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test11 (void)
+{
+ int i = 0;
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 0) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 0) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 0) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 0) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 0) != 33)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 0) != 64)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 0) != 27)
+#else
+ if (__builtin_object_size (res, 0) != 64)
+#endif
+ abort ();
+
+ free (res);
+ free (ptr2);
+}
+
int
main (void)
{
@@ -644,5 +705,6 @@ main (void)
test8 ();
test9 (1);
test10 ();
+ test11 ();
exit (0);
}
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
index 2729538da17..beb271c5afc 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -544,6 +547,63 @@ test8 (unsigned cond)
#endif
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test9 (void)
+{
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 1) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 1) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 1) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 1) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 1) != 33)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 1) != 64)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 1) != 27)
+#else
+ if (__builtin_object_size (res, 1) != 64)
+#endif
+ abort ();
+
+ free (res);
+ free (ptr2);
+}
+
int
main (void)
{
@@ -557,5 +617,6 @@ main (void)
test6 ();
test7 ();
test8 (1);
+ test9 ();
exit (0);
}
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
index 44a99189776..5c878a14647 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -636,6 +639,63 @@ test10 (void)
}
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test11 (void)
+{
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 2) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 2) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 2) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 2) != 0)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 2) != 0)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 2) != 0)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 2) != 27)
+#else
+ if (__builtin_object_size (res, 2) != 0)
+#endif
+ abort ();
+
+ free (res);
+ free (ptr2);
+}
+
int
main (void)
{
@@ -651,5 +711,6 @@ main (void)
test8 ();
test9 (1);
test10 ();
+ test11 ();
exit (0);
}
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
index b9fddfed036..0b1cb1e528a 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -517,6 +520,63 @@ test8 (unsigned cond)
#endif
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test9 (void)
+{
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 3) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 3) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 3) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 3) != 0)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 3) != 0)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 3) != 0)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 3) != 27)
+#else
+ if (__builtin_object_size (res, 3) != 0)
+#endif
+ abort ();
+
+ free (res);
+ free (ptr2);
+}
+
int
main (void)
{
@@ -530,5 +590,6 @@ main (void)
test6 ();
test7 ();
test8 (1);
+ test9 ();
exit (0);
}
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 4eb454a4a33..c075b71db56 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min)
return size;
}
+/* Get the outermost object that PTR may point into. */
+
+static tree
+get_whole_object (const_tree ptr)
+{
+ tree pt_var = TREE_OPERAND (ptr, 0);
+ while (handled_component_p (pt_var))
+ pt_var = TREE_OPERAND (pt_var, 0);
+
+ return pt_var;
+}
+
/* Compute __builtin_object_size for PTR, which is a ADDR_EXPR.
OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
If unknown, return size_unknown (object_size_type). */
@@ -514,9 +526,7 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
if (pwholesize)
*pwholesize = size_unknown (object_size_type);
- pt_var = TREE_OPERAND (ptr, 0);
- while (handled_component_p (pt_var))
- pt_var = TREE_OPERAND (pt_var, 0);
+ pt_var = get_whole_object (ptr);
if (!pt_var)
return false;
@@ -789,6 +799,53 @@ alloc_object_size (const gcall *call, int object_size_type)
return bytes ? bytes : size_unknown (object_size_type);
}
+/* Compute __builtin_object_size for CALL, which is a call to either
+ BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which it is.
+ OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
+ If unknown, return size_unknown (object_size_type). */
+
+static tree
+strdup_object_size (const gcall *call, int object_size_type, bool is_strndup)
+{
+ tree src = gimple_call_arg (call, 0);
+ tree sz = size_unknown (object_size_type);
+ tree n = is_strndup ? gimple_call_arg (call, 1) : NULL_TREE;
+
+ /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer fold it the
+ way it likes. */
+ if (!is_strndup)
+ {
+ tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
+ if (strlen_fn)
+ sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
+ build_call_expr (strlen_fn, 1, src));
+ }
+
+ /* In all other cases, return the size of SRC since the object size cannot
+ exceed that. We cannot do this for OST_MINIMUM unless SRC points into a
+ string constant since otherwise the object size could go all the way down
+ to zero. */
+ if (!size_valid_p (sz, object_size_type)
+ || size_unknown_p (sz, object_size_type))
+ {
+ tree wholesrc = NULL_TREE;
+ if (TREE_CODE (src) == ADDR_EXPR)
+ wholesrc = get_whole_object (src);
+
+ if (!(object_size_type & OST_MINIMUM)
+ || (wholesrc && TREE_CODE (wholesrc) == STRING_CST))
+ compute_builtin_object_size (src, object_size_type, &sz);
+ }
+
+ if (!n)
+ return sz;
+
+ /* Factor in the N. Note that with OST_MINIMUM, even if N is known we return
+ 0 since the size could be less than N. */
+ return fold_build2 (MIN_EXPR, sizetype,
+ fold_build2 (PLUS_EXPR, sizetype, size_one_node, n),
+ sz);
+}
/* If object size is propagated from one of function's arguments directly
to its return value, return that argument for GIMPLE_CALL statement CALL.
@@ -1235,12 +1292,19 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
{
int object_size_type = osi->object_size_type;
unsigned int varno = SSA_NAME_VERSION (ptr);
+ tree bytes = NULL_TREE;
gcc_assert (is_gimple_call (call));
gcc_assert (!object_sizes_unknown_p (object_size_type, varno));
gcc_assert (osi->pass == 0);
- tree bytes = alloc_object_size (call, object_size_type);
+
+ bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
+ bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
+ if (is_strdup || is_strndup)
+ bytes = strdup_object_size (call, object_size_type, is_strndup);
+ else
+ bytes = alloc_object_size (call, object_size_type);
if (!size_valid_p (bytes, object_size_type))
bytes = size_unknown (object_size_type);
@@ -2113,7 +2177,7 @@ const pass_data pass_data_object_sizes =
PROP_objsz, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- 0, /* todo_flags_finish */
+ TODO_update_ssa_no_phi, /* todo_flags_finish */
};
class pass_object_sizes : public gimple_opt_pass
@@ -2153,7 +2217,7 @@ const pass_data pass_data_early_object_sizes =
0, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
- 0, /* todo_flags_finish */
+ TODO_update_ssa_no_phi, /* todo_flags_finish */
};
class pass_early_object_sizes : public gimple_opt_pass
--
2.37.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tree-object-size: Support strndup and strdup
2022-08-15 19:23 [PATCH] tree-object-size: Support strndup and strdup Siddhesh Poyarekar
@ 2022-08-29 14:16 ` Siddhesh Poyarekar
2022-09-07 19:21 ` Siddhesh Poyarekar
2022-09-22 13:02 ` Jakub Jelinek
2022-11-04 12:48 ` [PATCH v2] " Siddhesh Poyarekar
2 siblings, 1 reply; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-08-29 14:16 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub
Ping!
On 2022-08-15 15:23, Siddhesh Poyarekar wrote:
> Use string length of input to strdup to determine the usable size of the
> resulting object. Avoid doing the same for strndup since there's a
> chance that the input may be too large, resulting in an unnecessary
> overhead or worse, the input may not be NULL terminated, resulting in a
> crash where there would otherwise have been none.
>
> gcc/ChangeLog:
>
> * tree-object-size.cc (get_whole_object): New function.
> (addr_object_size): Use it.
> (strdup_object_size): New function.
> (call_object_size): Use it.
> (pass_data_object_sizes, pass_data_early_object_sizes): Set
> todo_flags_finish to TODO_update_ssa_no_phi.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
> test_strndup, test_strdup_min, test_strndup_min): New tests.
> (main): Call them.
> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
> warnings.
> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
> ---
> .../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++++
> .../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
> gcc/testsuite/gcc.dg/builtin-object-size-1.c | 64 +++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-2.c | 63 ++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-3.c | 63 ++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-4.c | 63 ++++++++++++++-
> gcc/tree-object-size.cc | 76 +++++++++++++++++--
> 10 files changed, 366 insertions(+), 14 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> index 01a280b2d7b..7f023708b15 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> @@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start, size_t end, int incr)
> return __builtin_dynamic_object_size (ptr, 0);
> }
>
> +/* strdup/strndup. */
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strdup (const char *in)
> +{
> + char *res = __builtin_strdup (in);
> + return __builtin_dynamic_object_size (res, 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strndup (const char *in, size_t bound)
> +{
> + char *res = __builtin_strndup (in, bound);
> + return __builtin_dynamic_object_size (res, 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strdup_min (const char *in)
> +{
> + char *res = __builtin_strdup (in);
> + return __builtin_dynamic_object_size (res, 2);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strndup_min (const char *in, size_t bound)
> +{
> + char *res = __builtin_strndup (in, bound);
> + return __builtin_dynamic_object_size (res, 2);
> +}
> +
> /* Other tests. */
>
> struct TV4
> @@ -651,6 +685,15 @@ main (int argc, char **argv)
> int *t = test_pr105736 (&val3);
> if (__builtin_dynamic_object_size (t, 0) != -1)
> FAIL ();
> + const char *str = "hello world";
> + if (test_strdup (str) != __builtin_strlen (str) + 1)
> + FAIL ();
> + if (test_strndup (str, 4) != 5)
> + FAIL ();
> + if (test_strdup_min (str) != __builtin_strlen (str) + 1)
> + FAIL ();
> + if (test_strndup_min (str, 4) != 0)
> + FAIL ();
>
> if (nfails > 0)
> __builtin_abort ();
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> index 7cc8b1c9488..8f17c8edcaf 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> index 267dbf48ca7..3677782ff1c 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> index fb9dc56da7e..5b6987b7773 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> index 870548b4206..9d796224e96 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> index b772e2da9b9..4fbd372d97a 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -629,6 +632,64 @@ test10 (void)
> }
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test11 (void)
> +{
> + int i = 0;
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 0) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 0) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 0) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 0) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 0) != 33)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 0) != 64)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 0) != 27)
> +#else
> + if (__builtin_object_size (res, 0) != 64)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +}
> +
> int
> main (void)
> {
> @@ -644,5 +705,6 @@ main (void)
> test8 ();
> test9 (1);
> test10 ();
> + test11 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> index 2729538da17..beb271c5afc 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -544,6 +547,63 @@ test8 (unsigned cond)
> #endif
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test9 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 1) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 1) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 1) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 1) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 1) != 33)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 1) != 64)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 1) != 27)
> +#else
> + if (__builtin_object_size (res, 1) != 64)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +}
> +
> int
> main (void)
> {
> @@ -557,5 +617,6 @@ main (void)
> test6 ();
> test7 ();
> test8 (1);
> + test9 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> index 44a99189776..5c878a14647 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -636,6 +639,63 @@ test10 (void)
> }
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test11 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 2) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 2) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 2) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 2) != 0)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 2) != 0)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 2) != 0)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 2) != 27)
> +#else
> + if (__builtin_object_size (res, 2) != 0)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +}
> +
> int
> main (void)
> {
> @@ -651,5 +711,6 @@ main (void)
> test8 ();
> test9 (1);
> test10 ();
> + test11 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> index b9fddfed036..0b1cb1e528a 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -517,6 +520,63 @@ test8 (unsigned cond)
> #endif
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test9 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 3) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 3) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 3) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 3) != 0)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 3) != 0)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 3) != 0)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 3) != 27)
> +#else
> + if (__builtin_object_size (res, 3) != 0)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +}
> +
> int
> main (void)
> {
> @@ -530,5 +590,6 @@ main (void)
> test6 ();
> test7 ();
> test8 (1);
> + test9 ();
> exit (0);
> }
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 4eb454a4a33..c075b71db56 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min)
> return size;
> }
>
> +/* Get the outermost object that PTR may point into. */
> +
> +static tree
> +get_whole_object (const_tree ptr)
> +{
> + tree pt_var = TREE_OPERAND (ptr, 0);
> + while (handled_component_p (pt_var))
> + pt_var = TREE_OPERAND (pt_var, 0);
> +
> + return pt_var;
> +}
> +
> /* Compute __builtin_object_size for PTR, which is a ADDR_EXPR.
> OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
> If unknown, return size_unknown (object_size_type). */
> @@ -514,9 +526,7 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
> if (pwholesize)
> *pwholesize = size_unknown (object_size_type);
>
> - pt_var = TREE_OPERAND (ptr, 0);
> - while (handled_component_p (pt_var))
> - pt_var = TREE_OPERAND (pt_var, 0);
> + pt_var = get_whole_object (ptr);
>
> if (!pt_var)
> return false;
> @@ -789,6 +799,53 @@ alloc_object_size (const gcall *call, int object_size_type)
> return bytes ? bytes : size_unknown (object_size_type);
> }
>
> +/* Compute __builtin_object_size for CALL, which is a call to either
> + BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which it is.
> + OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
> + If unknown, return size_unknown (object_size_type). */
> +
> +static tree
> +strdup_object_size (const gcall *call, int object_size_type, bool is_strndup)
> +{
> + tree src = gimple_call_arg (call, 0);
> + tree sz = size_unknown (object_size_type);
> + tree n = is_strndup ? gimple_call_arg (call, 1) : NULL_TREE;
> +
> + /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer fold it the
> + way it likes. */
> + if (!is_strndup)
> + {
> + tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
> + if (strlen_fn)
> + sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
> + build_call_expr (strlen_fn, 1, src));
> + }
> +
> + /* In all other cases, return the size of SRC since the object size cannot
> + exceed that. We cannot do this for OST_MINIMUM unless SRC points into a
> + string constant since otherwise the object size could go all the way down
> + to zero. */
> + if (!size_valid_p (sz, object_size_type)
> + || size_unknown_p (sz, object_size_type))
> + {
> + tree wholesrc = NULL_TREE;
> + if (TREE_CODE (src) == ADDR_EXPR)
> + wholesrc = get_whole_object (src);
> +
> + if (!(object_size_type & OST_MINIMUM)
> + || (wholesrc && TREE_CODE (wholesrc) == STRING_CST))
> + compute_builtin_object_size (src, object_size_type, &sz);
> + }
> +
> + if (!n)
> + return sz;
> +
> + /* Factor in the N. Note that with OST_MINIMUM, even if N is known we return
> + 0 since the size could be less than N. */
> + return fold_build2 (MIN_EXPR, sizetype,
> + fold_build2 (PLUS_EXPR, sizetype, size_one_node, n),
> + sz);
> +}
>
> /* If object size is propagated from one of function's arguments directly
> to its return value, return that argument for GIMPLE_CALL statement CALL.
> @@ -1235,12 +1292,19 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
> {
> int object_size_type = osi->object_size_type;
> unsigned int varno = SSA_NAME_VERSION (ptr);
> + tree bytes = NULL_TREE;
>
> gcc_assert (is_gimple_call (call));
>
> gcc_assert (!object_sizes_unknown_p (object_size_type, varno));
> gcc_assert (osi->pass == 0);
> - tree bytes = alloc_object_size (call, object_size_type);
> +
> + bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
> + bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
> + if (is_strdup || is_strndup)
> + bytes = strdup_object_size (call, object_size_type, is_strndup);
> + else
> + bytes = alloc_object_size (call, object_size_type);
>
> if (!size_valid_p (bytes, object_size_type))
> bytes = size_unknown (object_size_type);
> @@ -2113,7 +2177,7 @@ const pass_data pass_data_object_sizes =
> PROP_objsz, /* properties_provided */
> 0, /* properties_destroyed */
> 0, /* todo_flags_start */
> - 0, /* todo_flags_finish */
> + TODO_update_ssa_no_phi, /* todo_flags_finish */
> };
>
> class pass_object_sizes : public gimple_opt_pass
> @@ -2153,7 +2217,7 @@ const pass_data pass_data_early_object_sizes =
> 0, /* properties_provided */
> 0, /* properties_destroyed */
> 0, /* todo_flags_start */
> - 0, /* todo_flags_finish */
> + TODO_update_ssa_no_phi, /* todo_flags_finish */
> };
>
> class pass_early_object_sizes : public gimple_opt_pass
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tree-object-size: Support strndup and strdup
2022-08-29 14:16 ` Siddhesh Poyarekar
@ 2022-09-07 19:21 ` Siddhesh Poyarekar
2022-09-15 14:00 ` Siddhesh Poyarekar
0 siblings, 1 reply; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-09-07 19:21 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub
Ping!
On 2022-08-29 10:16, Siddhesh Poyarekar wrote:
> Ping!
>
> On 2022-08-15 15:23, Siddhesh Poyarekar wrote:
>> Use string length of input to strdup to determine the usable size of the
>> resulting object. Avoid doing the same for strndup since there's a
>> chance that the input may be too large, resulting in an unnecessary
>> overhead or worse, the input may not be NULL terminated, resulting in a
>> crash where there would otherwise have been none.
>>
>> gcc/ChangeLog:
>>
>> * tree-object-size.cc (get_whole_object): New function.
>> (addr_object_size): Use it.
>> (strdup_object_size): New function.
>> (call_object_size): Use it.
>> (pass_data_object_sizes, pass_data_early_object_sizes): Set
>> todo_flags_finish to TODO_update_ssa_no_phi.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
>> test_strndup, test_strdup_min, test_strndup_min): New tests.
>> (main): Call them.
>> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
>> warnings.
>> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
>> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
>> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
>> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test11): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test9): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test11): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test9): New test.
>> (main): Call it.
>> ---
>> .../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++++
>> .../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
>> .../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
>> .../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
>> .../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
>> gcc/testsuite/gcc.dg/builtin-object-size-1.c | 64 +++++++++++++++-
>> gcc/testsuite/gcc.dg/builtin-object-size-2.c | 63 ++++++++++++++-
>> gcc/testsuite/gcc.dg/builtin-object-size-3.c | 63 ++++++++++++++-
>> gcc/testsuite/gcc.dg/builtin-object-size-4.c | 63 ++++++++++++++-
>> gcc/tree-object-size.cc | 76 +++++++++++++++++--
>> 10 files changed, 366 insertions(+), 14 deletions(-)
>>
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>> index 01a280b2d7b..7f023708b15 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>> @@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start,
>> size_t end, int incr)
>> return __builtin_dynamic_object_size (ptr, 0);
>> }
>> +/* strdup/strndup. */
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strdup (const char *in)
>> +{
>> + char *res = __builtin_strdup (in);
>> + return __builtin_dynamic_object_size (res, 0);
>> +}
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strndup (const char *in, size_t bound)
>> +{
>> + char *res = __builtin_strndup (in, bound);
>> + return __builtin_dynamic_object_size (res, 0);
>> +}
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strdup_min (const char *in)
>> +{
>> + char *res = __builtin_strdup (in);
>> + return __builtin_dynamic_object_size (res, 2);
>> +}
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strndup_min (const char *in, size_t bound)
>> +{
>> + char *res = __builtin_strndup (in, bound);
>> + return __builtin_dynamic_object_size (res, 2);
>> +}
>> +
>> /* Other tests. */
>> struct TV4
>> @@ -651,6 +685,15 @@ main (int argc, char **argv)
>> int *t = test_pr105736 (&val3);
>> if (__builtin_dynamic_object_size (t, 0) != -1)
>> FAIL ();
>> + const char *str = "hello world";
>> + if (test_strdup (str) != __builtin_strlen (str) + 1)
>> + FAIL ();
>> + if (test_strndup (str, 4) != 5)
>> + FAIL ();
>> + if (test_strdup_min (str) != __builtin_strlen (str) + 1)
>> + FAIL ();
>> + if (test_strndup_min (str, 4) != 0)
>> + FAIL ();
>> if (nfails > 0)
>> __builtin_abort ();
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>> index 7cc8b1c9488..8f17c8edcaf 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>> index 267dbf48ca7..3677782ff1c 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>> index fb9dc56da7e..5b6987b7773 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>> index 870548b4206..9d796224e96 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>> b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>> index b772e2da9b9..4fbd372d97a 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>> struct A
>> {
>> @@ -629,6 +632,64 @@ test10 (void)
>> }
>> }
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test11 (void)
>> +{
>> + int i = 0;
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 0) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 0) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 0) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 0) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 0) != 33)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 0) != 64)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 0) != 27)
>> +#else
>> + if (__builtin_object_size (res, 0) != 64)
>> +#endif
>> + abort ();
>> +
>> + free (res);
>> + free (ptr2);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -644,5 +705,6 @@ main (void)
>> test8 ();
>> test9 (1);
>> test10 ();
>> + test11 ();
>> exit (0);
>> }
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>> b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>> index 2729538da17..beb271c5afc 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>> struct A
>> {
>> @@ -544,6 +547,63 @@ test8 (unsigned cond)
>> #endif
>> }
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test9 (void)
>> +{
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 1) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 1) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 1) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 1) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 1) != 33)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 1) != 64)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 1) != 27)
>> +#else
>> + if (__builtin_object_size (res, 1) != 64)
>> +#endif
>> + abort ();
>> +
>> + free (res);
>> + free (ptr2);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -557,5 +617,6 @@ main (void)
>> test6 ();
>> test7 ();
>> test8 (1);
>> + test9 ();
>> exit (0);
>> }
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>> b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>> index 44a99189776..5c878a14647 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>> struct A
>> {
>> @@ -636,6 +639,63 @@ test10 (void)
>> }
>> }
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test11 (void)
>> +{
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 2) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 2) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 2) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 2) != 0)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 2) != 0)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 2) != 0)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 2) != 27)
>> +#else
>> + if (__builtin_object_size (res, 2) != 0)
>> +#endif
>> + abort ();
>> +
>> + free (res);
>> + free (ptr2);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -651,5 +711,6 @@ main (void)
>> test8 ();
>> test9 (1);
>> test10 ();
>> + test11 ();
>> exit (0);
>> }
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>> b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>> index b9fddfed036..0b1cb1e528a 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>> struct A
>> {
>> @@ -517,6 +520,63 @@ test8 (unsigned cond)
>> #endif
>> }
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test9 (void)
>> +{
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 3) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 3) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 3) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 3) != 0)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 3) != 0)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 3) != 0)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 3) != 27)
>> +#else
>> + if (__builtin_object_size (res, 3) != 0)
>> +#endif
>> + abort ();
>> +
>> + free (res);
>> + free (ptr2);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -530,5 +590,6 @@ main (void)
>> test6 ();
>> test7 ();
>> test8 (1);
>> + test9 ();
>> exit (0);
>> }
>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>> index 4eb454a4a33..c075b71db56 100644
>> --- a/gcc/tree-object-size.cc
>> +++ b/gcc/tree-object-size.cc
>> @@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min)
>> return size;
>> }
>> +/* Get the outermost object that PTR may point into. */
>> +
>> +static tree
>> +get_whole_object (const_tree ptr)
>> +{
>> + tree pt_var = TREE_OPERAND (ptr, 0);
>> + while (handled_component_p (pt_var))
>> + pt_var = TREE_OPERAND (pt_var, 0);
>> +
>> + return pt_var;
>> +}
>> +
>> /* Compute __builtin_object_size for PTR, which is a ADDR_EXPR.
>> OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>> If unknown, return size_unknown (object_size_type). */
>> @@ -514,9 +526,7 @@ addr_object_size (struct object_size_info *osi,
>> const_tree ptr,
>> if (pwholesize)
>> *pwholesize = size_unknown (object_size_type);
>> - pt_var = TREE_OPERAND (ptr, 0);
>> - while (handled_component_p (pt_var))
>> - pt_var = TREE_OPERAND (pt_var, 0);
>> + pt_var = get_whole_object (ptr);
>> if (!pt_var)
>> return false;
>> @@ -789,6 +799,53 @@ alloc_object_size (const gcall *call, int
>> object_size_type)
>> return bytes ? bytes : size_unknown (object_size_type);
>> }
>> +/* Compute __builtin_object_size for CALL, which is a call to either
>> + BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which it
>> is.
>> + OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>> + If unknown, return size_unknown (object_size_type). */
>> +
>> +static tree
>> +strdup_object_size (const gcall *call, int object_size_type, bool
>> is_strndup)
>> +{
>> + tree src = gimple_call_arg (call, 0);
>> + tree sz = size_unknown (object_size_type);
>> + tree n = is_strndup ? gimple_call_arg (call, 1) : NULL_TREE;
>> +
>> + /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer
>> fold it the
>> + way it likes. */
>> + if (!is_strndup)
>> + {
>> + tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
>> + if (strlen_fn)
>> + sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
>> + build_call_expr (strlen_fn, 1, src));
>> + }
>> +
>> + /* In all other cases, return the size of SRC since the object size
>> cannot
>> + exceed that. We cannot do this for OST_MINIMUM unless SRC
>> points into a
>> + string constant since otherwise the object size could go all the
>> way down
>> + to zero. */
>> + if (!size_valid_p (sz, object_size_type)
>> + || size_unknown_p (sz, object_size_type))
>> + {
>> + tree wholesrc = NULL_TREE;
>> + if (TREE_CODE (src) == ADDR_EXPR)
>> + wholesrc = get_whole_object (src);
>> +
>> + if (!(object_size_type & OST_MINIMUM)
>> + || (wholesrc && TREE_CODE (wholesrc) == STRING_CST))
>> + compute_builtin_object_size (src, object_size_type, &sz);
>> + }
>> +
>> + if (!n)
>> + return sz;
>> +
>> + /* Factor in the N. Note that with OST_MINIMUM, even if N is known
>> we return
>> + 0 since the size could be less than N. */
>> + return fold_build2 (MIN_EXPR, sizetype,
>> + fold_build2 (PLUS_EXPR, sizetype, size_one_node, n),
>> + sz);
>> +}
>> /* If object size is propagated from one of function's arguments
>> directly
>> to its return value, return that argument for GIMPLE_CALL
>> statement CALL.
>> @@ -1235,12 +1292,19 @@ call_object_size (struct object_size_info
>> *osi, tree ptr, gcall *call)
>> {
>> int object_size_type = osi->object_size_type;
>> unsigned int varno = SSA_NAME_VERSION (ptr);
>> + tree bytes = NULL_TREE;
>> gcc_assert (is_gimple_call (call));
>> gcc_assert (!object_sizes_unknown_p (object_size_type, varno));
>> gcc_assert (osi->pass == 0);
>> - tree bytes = alloc_object_size (call, object_size_type);
>> +
>> + bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
>> + bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
>> + if (is_strdup || is_strndup)
>> + bytes = strdup_object_size (call, object_size_type, is_strndup);
>> + else
>> + bytes = alloc_object_size (call, object_size_type);
>> if (!size_valid_p (bytes, object_size_type))
>> bytes = size_unknown (object_size_type);
>> @@ -2113,7 +2177,7 @@ const pass_data pass_data_object_sizes =
>> PROP_objsz, /* properties_provided */
>> 0, /* properties_destroyed */
>> 0, /* todo_flags_start */
>> - 0, /* todo_flags_finish */
>> + TODO_update_ssa_no_phi, /* todo_flags_finish */
>> };
>> class pass_object_sizes : public gimple_opt_pass
>> @@ -2153,7 +2217,7 @@ const pass_data pass_data_early_object_sizes =
>> 0, /* properties_provided */
>> 0, /* properties_destroyed */
>> 0, /* todo_flags_start */
>> - 0, /* todo_flags_finish */
>> + TODO_update_ssa_no_phi, /* todo_flags_finish */
>> };
>> class pass_early_object_sizes : public gimple_opt_pass
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tree-object-size: Support strndup and strdup
2022-09-07 19:21 ` Siddhesh Poyarekar
@ 2022-09-15 14:00 ` Siddhesh Poyarekar
0 siblings, 0 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-09-15 14:00 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub
Ping!
On 2022-09-07 15:21, Siddhesh Poyarekar wrote:
> Ping!
>
> On 2022-08-29 10:16, Siddhesh Poyarekar wrote:
>> Ping!
>>
>> On 2022-08-15 15:23, Siddhesh Poyarekar wrote:
>>> Use string length of input to strdup to determine the usable size of the
>>> resulting object. Avoid doing the same for strndup since there's a
>>> chance that the input may be too large, resulting in an unnecessary
>>> overhead or worse, the input may not be NULL terminated, resulting in a
>>> crash where there would otherwise have been none.
>>>
>>> gcc/ChangeLog:
>>>
>>> * tree-object-size.cc (get_whole_object): New function.
>>> (addr_object_size): Use it.
>>> (strdup_object_size): New function.
>>> (call_object_size): Use it.
>>> (pass_data_object_sizes, pass_data_early_object_sizes): Set
>>> todo_flags_finish to TODO_update_ssa_no_phi.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
>>> test_strndup, test_strdup_min, test_strndup_min): New tests.
>>> (main): Call them.
>>> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
>>> warnings.
>>> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
>>> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
>>> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
>>> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test11): New test.
>>> (main): Call it.
>>> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test9): New test.
>>> (main): Call it.
>>> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test11): New test.
>>> (main): Call it.
>>> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test9): New test.
>>> (main): Call it.
>>> ---
>>> .../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++++
>>> .../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
>>> .../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
>>> .../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
>>> .../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
>>> gcc/testsuite/gcc.dg/builtin-object-size-1.c | 64 +++++++++++++++-
>>> gcc/testsuite/gcc.dg/builtin-object-size-2.c | 63 ++++++++++++++-
>>> gcc/testsuite/gcc.dg/builtin-object-size-3.c | 63 ++++++++++++++-
>>> gcc/testsuite/gcc.dg/builtin-object-size-4.c | 63 ++++++++++++++-
>>> gcc/tree-object-size.cc | 76 +++++++++++++++++--
>>> 10 files changed, 366 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>>> index 01a280b2d7b..7f023708b15 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>>> @@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start,
>>> size_t end, int incr)
>>> return __builtin_dynamic_object_size (ptr, 0);
>>> }
>>> +/* strdup/strndup. */
>>> +
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test_strdup (const char *in)
>>> +{
>>> + char *res = __builtin_strdup (in);
>>> + return __builtin_dynamic_object_size (res, 0);
>>> +}
>>> +
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test_strndup (const char *in, size_t bound)
>>> +{
>>> + char *res = __builtin_strndup (in, bound);
>>> + return __builtin_dynamic_object_size (res, 0);
>>> +}
>>> +
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test_strdup_min (const char *in)
>>> +{
>>> + char *res = __builtin_strdup (in);
>>> + return __builtin_dynamic_object_size (res, 2);
>>> +}
>>> +
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test_strndup_min (const char *in, size_t bound)
>>> +{
>>> + char *res = __builtin_strndup (in, bound);
>>> + return __builtin_dynamic_object_size (res, 2);
>>> +}
>>> +
>>> /* Other tests. */
>>> struct TV4
>>> @@ -651,6 +685,15 @@ main (int argc, char **argv)
>>> int *t = test_pr105736 (&val3);
>>> if (__builtin_dynamic_object_size (t, 0) != -1)
>>> FAIL ();
>>> + const char *str = "hello world";
>>> + if (test_strdup (str) != __builtin_strlen (str) + 1)
>>> + FAIL ();
>>> + if (test_strndup (str, 4) != 5)
>>> + FAIL ();
>>> + if (test_strdup_min (str) != __builtin_strlen (str) + 1)
>>> + FAIL ();
>>> + if (test_strndup_min (str, 4) != 0)
>>> + FAIL ();
>>> if (nfails > 0)
>>> __builtin_abort ();
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>>> index 7cc8b1c9488..8f17c8edcaf 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> #define __builtin_object_size __builtin_dynamic_object_size
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>>> index 267dbf48ca7..3677782ff1c 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> #define __builtin_object_size __builtin_dynamic_object_size
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>>> index fb9dc56da7e..5b6987b7773 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> #define __builtin_object_size __builtin_dynamic_object_size
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>>> b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>>> index 870548b4206..9d796224e96 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> #define __builtin_object_size __builtin_dynamic_object_size
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>>> b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>>> index b772e2da9b9..4fbd372d97a 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> typedef __SIZE_TYPE__ size_t;
>>> @@ -7,10 +7,13 @@ extern void abort (void);
>>> extern void exit (int);
>>> extern void *malloc (size_t);
>>> extern void *calloc (size_t, size_t);
>>> +extern void free (void *);
>>> extern void *alloca (size_t);
>>> extern void *memcpy (void *, const void *, size_t);
>>> extern void *memset (void *, int, size_t);
>>> extern char *strcpy (char *, const char *);
>>> +extern char *strdup (const char *);
>>> +extern char *strndup (const char *, size_t);
>>> struct A
>>> {
>>> @@ -629,6 +632,64 @@ test10 (void)
>>> }
>>> }
>>> +/* Tests for strdup/strndup. */
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test11 (void)
>>> +{
>>> + int i = 0;
>>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>>> + char *res = strndup (ptr, 21);
>>> + if (__builtin_object_size (res, 0) != 22)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr, 32);
>>> + if (__builtin_object_size (res, 0) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr);
>>> + if (__builtin_object_size (res, 0) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + char *ptr2 = malloc (64);
>>> + strcpy (ptr2, ptr);
>>> +
>>> + res = strndup (ptr2, 21);
>>> + if (__builtin_object_size (res, 0) != 22)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 32);
>>> + if (__builtin_object_size (res, 0) != 33)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 128);
>>> + if (__builtin_object_size (res, 0) != 64)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr2);
>>> +#ifdef __builtin_object_size
>>> + if (__builtin_object_size (res, 0) != 27)
>>> +#else
>>> + if (__builtin_object_size (res, 0) != 64)
>>> +#endif
>>> + abort ();
>>> +
>>> + free (res);
>>> + free (ptr2);
>>> +}
>>> +
>>> int
>>> main (void)
>>> {
>>> @@ -644,5 +705,6 @@ main (void)
>>> test8 ();
>>> test9 (1);
>>> test10 ();
>>> + test11 ();
>>> exit (0);
>>> }
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>>> b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>>> index 2729538da17..beb271c5afc 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> typedef __SIZE_TYPE__ size_t;
>>> @@ -7,10 +7,13 @@ extern void abort (void);
>>> extern void exit (int);
>>> extern void *malloc (size_t);
>>> extern void *calloc (size_t, size_t);
>>> +extern void free (void *);
>>> extern void *alloca (size_t);
>>> extern void *memcpy (void *, const void *, size_t);
>>> extern void *memset (void *, int, size_t);
>>> extern char *strcpy (char *, const char *);
>>> +extern char *strdup (const char *);
>>> +extern char *strndup (const char *, size_t);
>>> struct A
>>> {
>>> @@ -544,6 +547,63 @@ test8 (unsigned cond)
>>> #endif
>>> }
>>> +/* Tests for strdup/strndup. */
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test9 (void)
>>> +{
>>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>>> + char *res = strndup (ptr, 21);
>>> + if (__builtin_object_size (res, 1) != 22)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr, 32);
>>> + if (__builtin_object_size (res, 1) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr);
>>> + if (__builtin_object_size (res, 1) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + char *ptr2 = malloc (64);
>>> + strcpy (ptr2, ptr);
>>> +
>>> + res = strndup (ptr2, 21);
>>> + if (__builtin_object_size (res, 1) != 22)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 32);
>>> + if (__builtin_object_size (res, 1) != 33)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 128);
>>> + if (__builtin_object_size (res, 1) != 64)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr2);
>>> +#ifdef __builtin_object_size
>>> + if (__builtin_object_size (res, 1) != 27)
>>> +#else
>>> + if (__builtin_object_size (res, 1) != 64)
>>> +#endif
>>> + abort ();
>>> +
>>> + free (res);
>>> + free (ptr2);
>>> +}
>>> +
>>> int
>>> main (void)
>>> {
>>> @@ -557,5 +617,6 @@ main (void)
>>> test6 ();
>>> test7 ();
>>> test8 (1);
>>> + test9 ();
>>> exit (0);
>>> }
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>>> b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>>> index 44a99189776..5c878a14647 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> typedef __SIZE_TYPE__ size_t;
>>> @@ -7,10 +7,13 @@ extern void abort (void);
>>> extern void exit (int);
>>> extern void *malloc (size_t);
>>> extern void *calloc (size_t, size_t);
>>> +extern void free (void *);
>>> extern void *alloca (size_t);
>>> extern void *memcpy (void *, const void *, size_t);
>>> extern void *memset (void *, int, size_t);
>>> extern char *strcpy (char *, const char *);
>>> +extern char *strdup (const char *);
>>> +extern char *strndup (const char *, size_t);
>>> struct A
>>> {
>>> @@ -636,6 +639,63 @@ test10 (void)
>>> }
>>> }
>>> +/* Tests for strdup/strndup. */
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test11 (void)
>>> +{
>>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>>> + char *res = strndup (ptr, 21);
>>> + if (__builtin_object_size (res, 2) != 22)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr, 32);
>>> + if (__builtin_object_size (res, 2) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr);
>>> + if (__builtin_object_size (res, 2) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + char *ptr2 = malloc (64);
>>> + strcpy (ptr2, ptr);
>>> +
>>> + res = strndup (ptr2, 21);
>>> + if (__builtin_object_size (res, 2) != 0)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 32);
>>> + if (__builtin_object_size (res, 2) != 0)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 128);
>>> + if (__builtin_object_size (res, 2) != 0)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr2);
>>> +#ifdef __builtin_object_size
>>> + if (__builtin_object_size (res, 2) != 27)
>>> +#else
>>> + if (__builtin_object_size (res, 2) != 0)
>>> +#endif
>>> + abort ();
>>> +
>>> + free (res);
>>> + free (ptr2);
>>> +}
>>> +
>>> int
>>> main (void)
>>> {
>>> @@ -651,5 +711,6 @@ main (void)
>>> test8 ();
>>> test9 (1);
>>> test10 ();
>>> + test11 ();
>>> exit (0);
>>> }
>>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>>> b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>>> index b9fddfed036..0b1cb1e528a 100644
>>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>>> @@ -1,5 +1,5 @@
>>> /* { dg-do run } */
>>> -/* { dg-options "-O2" } */
>>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>>> /* { dg-require-effective-target alloca } */
>>> typedef __SIZE_TYPE__ size_t;
>>> @@ -7,10 +7,13 @@ extern void abort (void);
>>> extern void exit (int);
>>> extern void *malloc (size_t);
>>> extern void *calloc (size_t, size_t);
>>> +extern void free (void *);
>>> extern void *alloca (size_t);
>>> extern void *memcpy (void *, const void *, size_t);
>>> extern void *memset (void *, int, size_t);
>>> extern char *strcpy (char *, const char *);
>>> +extern char *strdup (const char *);
>>> +extern char *strndup (const char *, size_t);
>>> struct A
>>> {
>>> @@ -517,6 +520,63 @@ test8 (unsigned cond)
>>> #endif
>>> }
>>> +/* Tests for strdup/strndup. */
>>> +size_t
>>> +__attribute__ ((noinline))
>>> +test9 (void)
>>> +{
>>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>>> + char *res = strndup (ptr, 21);
>>> + if (__builtin_object_size (res, 3) != 22)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr, 32);
>>> + if (__builtin_object_size (res, 3) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr);
>>> + if (__builtin_object_size (res, 3) != 27)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + char *ptr2 = malloc (64);
>>> + strcpy (ptr2, ptr);
>>> +
>>> + res = strndup (ptr2, 21);
>>> + if (__builtin_object_size (res, 3) != 0)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 32);
>>> + if (__builtin_object_size (res, 3) != 0)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strndup (ptr2, 128);
>>> + if (__builtin_object_size (res, 3) != 0)
>>> + abort ();
>>> +
>>> + free (res);
>>> +
>>> + res = strdup (ptr2);
>>> +#ifdef __builtin_object_size
>>> + if (__builtin_object_size (res, 3) != 27)
>>> +#else
>>> + if (__builtin_object_size (res, 3) != 0)
>>> +#endif
>>> + abort ();
>>> +
>>> + free (res);
>>> + free (ptr2);
>>> +}
>>> +
>>> int
>>> main (void)
>>> {
>>> @@ -530,5 +590,6 @@ main (void)
>>> test6 ();
>>> test7 ();
>>> test8 (1);
>>> + test9 ();
>>> exit (0);
>>> }
>>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>>> index 4eb454a4a33..c075b71db56 100644
>>> --- a/gcc/tree-object-size.cc
>>> +++ b/gcc/tree-object-size.cc
>>> @@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min)
>>> return size;
>>> }
>>> +/* Get the outermost object that PTR may point into. */
>>> +
>>> +static tree
>>> +get_whole_object (const_tree ptr)
>>> +{
>>> + tree pt_var = TREE_OPERAND (ptr, 0);
>>> + while (handled_component_p (pt_var))
>>> + pt_var = TREE_OPERAND (pt_var, 0);
>>> +
>>> + return pt_var;
>>> +}
>>> +
>>> /* Compute __builtin_object_size for PTR, which is a ADDR_EXPR.
>>> OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>>> If unknown, return size_unknown (object_size_type). */
>>> @@ -514,9 +526,7 @@ addr_object_size (struct object_size_info *osi,
>>> const_tree ptr,
>>> if (pwholesize)
>>> *pwholesize = size_unknown (object_size_type);
>>> - pt_var = TREE_OPERAND (ptr, 0);
>>> - while (handled_component_p (pt_var))
>>> - pt_var = TREE_OPERAND (pt_var, 0);
>>> + pt_var = get_whole_object (ptr);
>>> if (!pt_var)
>>> return false;
>>> @@ -789,6 +799,53 @@ alloc_object_size (const gcall *call, int
>>> object_size_type)
>>> return bytes ? bytes : size_unknown (object_size_type);
>>> }
>>> +/* Compute __builtin_object_size for CALL, which is a call to either
>>> + BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which
>>> it is.
>>> + OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>>> + If unknown, return size_unknown (object_size_type). */
>>> +
>>> +static tree
>>> +strdup_object_size (const gcall *call, int object_size_type, bool
>>> is_strndup)
>>> +{
>>> + tree src = gimple_call_arg (call, 0);
>>> + tree sz = size_unknown (object_size_type);
>>> + tree n = is_strndup ? gimple_call_arg (call, 1) : NULL_TREE;
>>> +
>>> + /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer
>>> fold it the
>>> + way it likes. */
>>> + if (!is_strndup)
>>> + {
>>> + tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
>>> + if (strlen_fn)
>>> + sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
>>> + build_call_expr (strlen_fn, 1, src));
>>> + }
>>> +
>>> + /* In all other cases, return the size of SRC since the object
>>> size cannot
>>> + exceed that. We cannot do this for OST_MINIMUM unless SRC
>>> points into a
>>> + string constant since otherwise the object size could go all
>>> the way down
>>> + to zero. */
>>> + if (!size_valid_p (sz, object_size_type)
>>> + || size_unknown_p (sz, object_size_type))
>>> + {
>>> + tree wholesrc = NULL_TREE;
>>> + if (TREE_CODE (src) == ADDR_EXPR)
>>> + wholesrc = get_whole_object (src);
>>> +
>>> + if (!(object_size_type & OST_MINIMUM)
>>> + || (wholesrc && TREE_CODE (wholesrc) == STRING_CST))
>>> + compute_builtin_object_size (src, object_size_type, &sz);
>>> + }
>>> +
>>> + if (!n)
>>> + return sz;
>>> +
>>> + /* Factor in the N. Note that with OST_MINIMUM, even if N is
>>> known we return
>>> + 0 since the size could be less than N. */
>>> + return fold_build2 (MIN_EXPR, sizetype,
>>> + fold_build2 (PLUS_EXPR, sizetype, size_one_node, n),
>>> + sz);
>>> +}
>>> /* If object size is propagated from one of function's arguments
>>> directly
>>> to its return value, return that argument for GIMPLE_CALL
>>> statement CALL.
>>> @@ -1235,12 +1292,19 @@ call_object_size (struct object_size_info
>>> *osi, tree ptr, gcall *call)
>>> {
>>> int object_size_type = osi->object_size_type;
>>> unsigned int varno = SSA_NAME_VERSION (ptr);
>>> + tree bytes = NULL_TREE;
>>> gcc_assert (is_gimple_call (call));
>>> gcc_assert (!object_sizes_unknown_p (object_size_type, varno));
>>> gcc_assert (osi->pass == 0);
>>> - tree bytes = alloc_object_size (call, object_size_type);
>>> +
>>> + bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
>>> + bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
>>> + if (is_strdup || is_strndup)
>>> + bytes = strdup_object_size (call, object_size_type, is_strndup);
>>> + else
>>> + bytes = alloc_object_size (call, object_size_type);
>>> if (!size_valid_p (bytes, object_size_type))
>>> bytes = size_unknown (object_size_type);
>>> @@ -2113,7 +2177,7 @@ const pass_data pass_data_object_sizes =
>>> PROP_objsz, /* properties_provided */
>>> 0, /* properties_destroyed */
>>> 0, /* todo_flags_start */
>>> - 0, /* todo_flags_finish */
>>> + TODO_update_ssa_no_phi, /* todo_flags_finish */
>>> };
>>> class pass_object_sizes : public gimple_opt_pass
>>> @@ -2153,7 +2217,7 @@ const pass_data pass_data_early_object_sizes =
>>> 0, /* properties_provided */
>>> 0, /* properties_destroyed */
>>> 0, /* todo_flags_start */
>>> - 0, /* todo_flags_finish */
>>> + TODO_update_ssa_no_phi, /* todo_flags_finish */
>>> };
>>> class pass_early_object_sizes : public gimple_opt_pass
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tree-object-size: Support strndup and strdup
2022-08-15 19:23 [PATCH] tree-object-size: Support strndup and strdup Siddhesh Poyarekar
2022-08-29 14:16 ` Siddhesh Poyarekar
@ 2022-09-22 13:02 ` Jakub Jelinek
2022-09-22 15:26 ` Siddhesh Poyarekar
2022-11-04 12:48 ` [PATCH v2] " Siddhesh Poyarekar
2 siblings, 1 reply; 16+ messages in thread
From: Jakub Jelinek @ 2022-09-22 13:02 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: gcc-patches
On Mon, Aug 15, 2022 at 03:23:11PM -0400, Siddhesh Poyarekar wrote:
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min)
> return size;
> }
>
> +/* Get the outermost object that PTR may point into. */
> +
> +static tree
> +get_whole_object (const_tree ptr)
> +{
> + tree pt_var = TREE_OPERAND (ptr, 0);
> + while (handled_component_p (pt_var))
> + pt_var = TREE_OPERAND (pt_var, 0);
> +
> + return pt_var;
> +}
Not sure why you want a new function for this.
This is essentially get_base_address (TREE_OPERAND (ptr, 0)).
> /* Compute __builtin_object_size for PTR, which is a ADDR_EXPR.
> OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
> If unknown, return size_unknown (object_size_type). */
> + if (!size_valid_p (sz, object_size_type)
> + || size_unknown_p (sz, object_size_type))
> + {
> + tree wholesrc = NULL_TREE;
> + if (TREE_CODE (src) == ADDR_EXPR)
> + wholesrc = get_whole_object (src);
> +
> + if (!(object_size_type & OST_MINIMUM)
> + || (wholesrc && TREE_CODE (wholesrc) == STRING_CST))
Is this safe? I mean get_whole_object will also skip ARRAY_REFs with
variable indexes etc. and the STRING_CST could have embedded '\0's
in it.
Even if c_strlen (src, 1) is constant, I don't see what you can assume
for object size of strndup ("abcd\0efgh", n); for minimum, except 1.
But on the other side, 1 is a safe minimum for OST_MINIMUM of both
strdup and strndup if you don't find anything more specific (exact strlen
for strndup) because the terminating '\0' will be always there.
Other than that you'd need to consider INTEGER_CST second strndup argument
or ranges of the second argument etc.
E.g. maximum for OST_DYNAMIC could be for strndup (src, n)
MIN (__bdos (src, ?), n + 1).
> @@ -2113,7 +2177,7 @@ const pass_data pass_data_object_sizes =
> PROP_objsz, /* properties_provided */
> 0, /* properties_destroyed */
> 0, /* todo_flags_start */
> - 0, /* todo_flags_finish */
> + TODO_update_ssa_no_phi, /* todo_flags_finish */
> };
>
> class pass_object_sizes : public gimple_opt_pass
> @@ -2153,7 +2217,7 @@ const pass_data pass_data_early_object_sizes =
> 0, /* properties_provided */
> 0, /* properties_destroyed */
> 0, /* todo_flags_start */
> - 0, /* todo_flags_finish */
> + TODO_update_ssa_no_phi, /* todo_flags_finish */
> };
This is quite expensive. Do you really need full ssa update, or just
TODO_update_ssa_only_virtuals would be enough (is it for the missing
vuse on the strlen call if you emit it)?
In any case, would be better not to do that always, but only if you
really need it (emitted the strlen call somewhere; e.g. if __bdos is
never used, only __bos, it is certainly not needed), todo flags
can be both in todo_flags_finish and in return value from execute method.
Jakub
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tree-object-size: Support strndup and strdup
2022-09-22 13:02 ` Jakub Jelinek
@ 2022-09-22 15:26 ` Siddhesh Poyarekar
2022-09-23 13:02 ` Jakub Jelinek
0 siblings, 1 reply; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-09-22 15:26 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 2022-09-22 09:02, Jakub Jelinek wrote:
> On Mon, Aug 15, 2022 at 03:23:11PM -0400, Siddhesh Poyarekar wrote:
>> --- a/gcc/tree-object-size.cc
>> +++ b/gcc/tree-object-size.cc
>> @@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min)
>> return size;
>> }
>>
>> +/* Get the outermost object that PTR may point into. */
>> +
>> +static tree
>> +get_whole_object (const_tree ptr)
>> +{
>> + tree pt_var = TREE_OPERAND (ptr, 0);
>> + while (handled_component_p (pt_var))
>> + pt_var = TREE_OPERAND (pt_var, 0);
>> +
>> + return pt_var;
>> +}
>
> Not sure why you want a new function for this.
> This is essentially get_base_address (TREE_OPERAND (ptr, 0)).
Oh, so can addr_object_size be simplified to use get_base_address too?
>> /* Compute __builtin_object_size for PTR, which is a ADDR_EXPR.
>> OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>> If unknown, return size_unknown (object_size_type). */
>> + if (!size_valid_p (sz, object_size_type)
>> + || size_unknown_p (sz, object_size_type))
>> + {
>> + tree wholesrc = NULL_TREE;
>> + if (TREE_CODE (src) == ADDR_EXPR)
>> + wholesrc = get_whole_object (src);
>> +
>> + if (!(object_size_type & OST_MINIMUM)
>> + || (wholesrc && TREE_CODE (wholesrc) == STRING_CST))
>
> Is this safe? I mean get_whole_object will also skip ARRAY_REFs with
> variable indexes etc. and the STRING_CST could have embedded '\0's
> in it.
> Even if c_strlen (src, 1) is constant, I don't see what you can assume
> for object size of strndup ("abcd\0efgh", n); for minimum, except 1.
Can't we assume MIN(5, n) for STRING_CST?
For ARRAY_REFs, it may end up being MIN(array_size, n) and not account
for the NUL termination but I was thinking of that as being a better
option than bailing out. Should we try harder here and return, e.g.
strlen or some equivalent?
> But on the other side, 1 is a safe minimum for OST_MINIMUM of both
> strdup and strndup if you don't find anything more specific (exact strlen
> for strndup) because the terminating '\0' will be always there.
OK, I can return size_one_node as the final return value for OST_MINIMUM
if we don't find a suitable expression.
> Other than that you'd need to consider INTEGER_CST second strndup argument
> or ranges of the second argument etc.
> E.g. maximum for OST_DYNAMIC could be for strndup (src, n)
> MIN (__bdos (src, ?), n + 1).
Yeah, that's what I return in the end:
return fold_build2 (MIN_EXPR, sizetype,
fold_build2 (PLUS_EXPR, sizetype, size_one_node,n),
sz);
where sz is __bdos(src)
>
>> @@ -2113,7 +2177,7 @@ const pass_data pass_data_object_sizes =
>> PROP_objsz, /* properties_provided */
>> 0, /* properties_destroyed */
>> 0, /* todo_flags_start */
>> - 0, /* todo_flags_finish */
>> + TODO_update_ssa_no_phi, /* todo_flags_finish */
>> };
>>
>> class pass_object_sizes : public gimple_opt_pass
>> @@ -2153,7 +2217,7 @@ const pass_data pass_data_early_object_sizes =
>> 0, /* properties_provided */
>> 0, /* properties_destroyed */
>> 0, /* todo_flags_start */
>> - 0, /* todo_flags_finish */
>> + TODO_update_ssa_no_phi, /* todo_flags_finish */
>> };
>
> This is quite expensive. Do you really need full ssa update, or just
> TODO_update_ssa_only_virtuals would be enough (is it for the missing
> vuse on the strlen call if you emit it)?
> In any case, would be better not to do that always, but only if you
> really need it (emitted the strlen call somewhere; e.g. if __bdos is
> never used, only __bos, it is certainly not needed), todo flags
> can be both in todo_flags_finish and in return value from execute method.
Thanks, I'll find a cheaper way to do this.
Thanks,
Sid
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tree-object-size: Support strndup and strdup
2022-09-22 15:26 ` Siddhesh Poyarekar
@ 2022-09-23 13:02 ` Jakub Jelinek
2022-11-02 22:30 ` Siddhesh Poyarekar
0 siblings, 1 reply; 16+ messages in thread
From: Jakub Jelinek @ 2022-09-23 13:02 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: gcc-patches
On Thu, Sep 22, 2022 at 11:26:29AM -0400, Siddhesh Poyarekar wrote:
> On 2022-09-22 09:02, Jakub Jelinek wrote:
> > On Mon, Aug 15, 2022 at 03:23:11PM -0400, Siddhesh Poyarekar wrote:
> > > --- a/gcc/tree-object-size.cc
> > > +++ b/gcc/tree-object-size.cc
> > > @@ -495,6 +495,18 @@ decl_init_size (tree decl, bool min)
> > > return size;
> > > }
> > > +/* Get the outermost object that PTR may point into. */
> > > +
> > > +static tree
> > > +get_whole_object (const_tree ptr)
> > > +{
> > > + tree pt_var = TREE_OPERAND (ptr, 0);
> > > + while (handled_component_p (pt_var))
> > > + pt_var = TREE_OPERAND (pt_var, 0);
> > > +
> > > + return pt_var;
> > > +}
> >
> > Not sure why you want a new function for this.
> > This is essentially get_base_address (TREE_OPERAND (ptr, 0)).
>
> Oh, so can addr_object_size be simplified to use get_base_address too?
You can try. As you can see in get_base_address, that function
handles something that the above doesn't (looking through some MEM_REFs too).
> > Even if c_strlen (src, 1) is constant, I don't see what you can assume
> > for object size of strndup ("abcd\0efgh", n); for minimum, except 1.
>
> Can't we assume MIN(5, n) for STRING_CST?
If you mean MIN(5, n + 1), for c_strlen constant yes, but say if you have
strndup (&"abcd\0efgh"[i], n); you can't just from seeing a base address
being a STRING_CST with certain length assume anything than 1.
> For ARRAY_REFs, it may end up being MIN(array_size, n) and not account for
No, for non-OST_MINIMUM array size of objects (or string literals) containing
the strings is relevant and you can indeed use MIN(__b{d}os (src), n + 1)
as maximum. But for the minimum, the object size is irrelevant, you don't
know where in the string there are '\0's and they could appear anywhere
(unless you do string length range analysis). With c_strlen on src
returning constant you know the exact string length and so you can use
MIN (c_strlen (src, 1) + 1, n + 1) as both minimum and maximum, but in all
other cases, 1 is the safe answer.
Jakub
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] tree-object-size: Support strndup and strdup
2022-09-23 13:02 ` Jakub Jelinek
@ 2022-11-02 22:30 ` Siddhesh Poyarekar
0 siblings, 0 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-11-02 22:30 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 2022-09-23 09:02, Jakub Jelinek wrote:
>> Oh, so can addr_object_size be simplified to use get_base_address too?
>
> You can try. As you can see in get_base_address, that function
> handles something that the above doesn't (looking through some MEM_REFs too).
>
I went down this rabbithole and it actually simplifies some cases but
got sucked into flex array related issues that I need more time to
figure out. I'll stick to using get_base_address for now since I want
to make sure this makes the stage 1 deadline.
Thanks,
Sid
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] tree-object-size: Support strndup and strdup
2022-08-15 19:23 [PATCH] tree-object-size: Support strndup and strdup Siddhesh Poyarekar
2022-08-29 14:16 ` Siddhesh Poyarekar
2022-09-22 13:02 ` Jakub Jelinek
@ 2022-11-04 12:48 ` Siddhesh Poyarekar
2022-11-04 13:43 ` Prathamesh Kulkarni
` (2 more replies)
2 siblings, 3 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-11-04 12:48 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub
Use string length of input to strdup to determine the usable size of the
resulting object. Avoid doing the same for strndup since there's a
chance that the input may be too large, resulting in an unnecessary
overhead or worse, the input may not be NULL terminated, resulting in a
crash where there would otherwise have been none.
gcc/ChangeLog:
* tree-object-size.cc (todo): New variable.
(object_sizes_execute): Use it.
(strdup_object_size): New function.
(call_object_size): Use it.
gcc/testsuite/ChangeLog:
* gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
test_strndup, test_strdup_min, test_strndup_min): New tests.
(main): Call them.
* gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
warnings.
* gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
* gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
* gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
* gcc.dg/builtin-object-size-1.c: Silence overread warnings.
Declare free, strdup and strndup.
(test11): New test.
(main): Call it.
* gcc.dg/builtin-object-size-2.c: Silence overread warnings.
Declare free, strdup and strndup.
(test9): New test.
(main): Call it.
* gcc.dg/builtin-object-size-3.c: Silence overread warnings.
Declare free, strdup and strndup.
(test11): New test.
(main): Call it.
* gcc.dg/builtin-object-size-4.c: Silence overread warnings.
Declare free, strdup and strndup.
(test9): New test.
(main): Call it.
---
Tested:
- x86_64 bootstrap and testsuite run
- i686 build and testsuite run
- ubsan bootstrap
.../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++
.../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
.../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
.../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
.../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
gcc/testsuite/gcc.dg/builtin-object-size-1.c | 94 +++++++++++++++++-
gcc/testsuite/gcc.dg/builtin-object-size-2.c | 94 +++++++++++++++++-
gcc/testsuite/gcc.dg/builtin-object-size-3.c | 95 ++++++++++++++++++-
gcc/testsuite/gcc.dg/builtin-object-size-4.c | 94 +++++++++++++++++-
gcc/tree-object-size.cc | 84 +++++++++++++++-
10 files changed, 502 insertions(+), 10 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
index 01a280b2d7b..4f1606a486b 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
@@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start, size_t end, int incr)
return __builtin_dynamic_object_size (ptr, 0);
}
+/* strdup/strndup. */
+
+size_t
+__attribute__ ((noinline))
+test_strdup (const char *in)
+{
+ char *res = __builtin_strdup (in);
+ return __builtin_dynamic_object_size (res, 0);
+}
+
+size_t
+__attribute__ ((noinline))
+test_strndup (const char *in, size_t bound)
+{
+ char *res = __builtin_strndup (in, bound);
+ return __builtin_dynamic_object_size (res, 0);
+}
+
+size_t
+__attribute__ ((noinline))
+test_strdup_min (const char *in)
+{
+ char *res = __builtin_strdup (in);
+ return __builtin_dynamic_object_size (res, 2);
+}
+
+size_t
+__attribute__ ((noinline))
+test_strndup_min (const char *in, size_t bound)
+{
+ char *res = __builtin_strndup (in, bound);
+ return __builtin_dynamic_object_size (res, 2);
+}
+
/* Other tests. */
struct TV4
@@ -651,6 +685,15 @@ main (int argc, char **argv)
int *t = test_pr105736 (&val3);
if (__builtin_dynamic_object_size (t, 0) != -1)
FAIL ();
+ const char *str = "hello world";
+ if (test_strdup (str) != __builtin_strlen (str) + 1)
+ FAIL ();
+ if (test_strndup (str, 4) != 5)
+ FAIL ();
+ if (test_strdup_min (str) != __builtin_strlen (str) + 1)
+ FAIL ();
+ if (test_strndup_min (str, 4) != 1)
+ FAIL ();
if (nfails > 0)
__builtin_abort ();
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
index 7cc8b1c9488..8f17c8edcaf 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
index 267dbf48ca7..3677782ff1c 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
index fb9dc56da7e..5b6987b7773 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
index 870548b4206..9d796224e96 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
#define __builtin_object_size __builtin_dynamic_object_size
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
index b772e2da9b9..c6e5b4c29f8 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -629,6 +632,94 @@ test10 (void)
}
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test11 (void)
+{
+ int i = 0;
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 0) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 0) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 0) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 0) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 0) != 33)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 0) != 64)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 0) != 27)
+#else
+ if (__builtin_object_size (res, 0) != (size_t) -1)
+#endif
+ abort ();
+ free (res);
+ free (ptr2);
+
+ ptr = "abcd\0efghijklmnopqrstuvwxyz";
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 0) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 24);
+ if (__builtin_object_size (res, 0) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 2);
+ if (__builtin_object_size (res, 0) != 3)
+ abort ();
+ free (res);
+
+ res = strdup (&ptr[4]);
+ if (__builtin_object_size (res, 0) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 4);
+ if (__builtin_object_size (res, 0) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 1);
+ if (__builtin_object_size (res, 0) != 1)
+ abort ();
+ free (res);
+}
+
int
main (void)
{
@@ -644,5 +735,6 @@ main (void)
test8 ();
test9 (1);
test10 ();
+ test11 ();
exit (0);
}
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
index 2729538da17..639a83cfd39 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -544,6 +547,94 @@ test8 (unsigned cond)
#endif
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test9 (void)
+{
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 1) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 1) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 1) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 1) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 1) != 33)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 1) != 64)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 1) != 27)
+#else
+ if (__builtin_object_size (res, 1) != (size_t) -1)
+#endif
+ abort ();
+
+ free (res);
+ free (ptr2);
+
+ ptr = "abcd\0efghijklmnopqrstuvwxyz";
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 1) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 24);
+ if (__builtin_object_size (res, 1) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 2);
+ if (__builtin_object_size (res, 1) != 3)
+ abort ();
+ free (res);
+
+ res = strdup (&ptr[4]);
+ if (__builtin_object_size (res, 1) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 4);
+ if (__builtin_object_size (res, 1) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 1);
+ if (__builtin_object_size (res, 1) != 1)
+ abort ();
+ free (res);
+}
+
int
main (void)
{
@@ -557,5 +648,6 @@ main (void)
test6 ();
test7 ();
test8 (1);
+ test9 ();
exit (0);
}
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
index 44a99189776..ff4f1747334 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -636,6 +639,95 @@ test10 (void)
}
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test11 (void)
+{
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 2) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 2) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 2) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 2) != 1)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 2) != 1)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 2) != 1)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 2) != 27)
+#else
+ if (__builtin_object_size (res, 2) != 1)
+#endif
+ abort ();
+
+ free (res);
+ free (ptr2);
+
+ ptr = "abcd\0efghijklmnopqrstuvwxyz";
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 2) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 24);
+ if (__builtin_object_size (res, 2) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 2);
+ if (__builtin_object_size (res, 2) != 3)
+ abort ();
+ free (res);
+
+ res = strdup (&ptr[4]);
+ if (__builtin_object_size (res, 2) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 4);
+ if (__builtin_object_size (res, 2) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 1);
+ if (__builtin_object_size (res, 2) != 1)
+ abort ();
+ free (res);
+}
+
int
main (void)
{
@@ -651,5 +743,6 @@ main (void)
test8 ();
test9 (1);
test10 ();
+ test11 ();
exit (0);
}
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
index b9fddfed036..4c007c364b7 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -Wno-stringop-overread" } */
/* { dg-require-effective-target alloca } */
typedef __SIZE_TYPE__ size_t;
@@ -7,10 +7,13 @@ extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
extern void *calloc (size_t, size_t);
+extern void free (void *);
extern void *alloca (size_t);
extern void *memcpy (void *, const void *, size_t);
extern void *memset (void *, int, size_t);
extern char *strcpy (char *, const char *);
+extern char *strdup (const char *);
+extern char *strndup (const char *, size_t);
struct A
{
@@ -517,6 +520,94 @@ test8 (unsigned cond)
#endif
}
+/* Tests for strdup/strndup. */
+size_t
+__attribute__ ((noinline))
+test9 (void)
+{
+ const char *ptr = "abcdefghijklmnopqrstuvwxyz";
+ char *res = strndup (ptr, 21);
+ if (__builtin_object_size (res, 3) != 22)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr, 32);
+ if (__builtin_object_size (res, 3) != 27)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 3) != 27)
+ abort ();
+
+ free (res);
+
+ char *ptr2 = malloc (64);
+ strcpy (ptr2, ptr);
+
+ res = strndup (ptr2, 21);
+ if (__builtin_object_size (res, 3) != 1)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 32);
+ if (__builtin_object_size (res, 3) != 1)
+ abort ();
+
+ free (res);
+
+ res = strndup (ptr2, 128);
+ if (__builtin_object_size (res, 3) != 1)
+ abort ();
+
+ free (res);
+
+ res = strdup (ptr2);
+#ifdef __builtin_object_size
+ if (__builtin_object_size (res, 3) != 27)
+#else
+ if (__builtin_object_size (res, 3) != 1)
+#endif
+ abort ();
+
+ free (res);
+ free (ptr2);
+
+ ptr = "abcd\0efghijklmnopqrstuvwxyz";
+ res = strdup (ptr);
+ if (__builtin_object_size (res, 3) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 24);
+ if (__builtin_object_size (res, 3) != 5)
+ abort ();
+ free (res);
+
+ res = strndup (ptr, 2);
+ if (__builtin_object_size (res, 3) != 3)
+ abort ();
+ free (res);
+
+ res = strdup (&ptr[4]);
+ if (__builtin_object_size (res, 3) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 4);
+ if (__builtin_object_size (res, 3) != 1)
+ abort ();
+ free (res);
+
+ res = strndup (&ptr[4], 1);
+ if (__builtin_object_size (res, 3) != 1)
+ abort ();
+ free (res);
+}
+
int
main (void)
{
@@ -530,5 +621,6 @@ main (void)
test6 ();
test7 ();
test8 (1);
+ test9 ();
exit (0);
}
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 1f04cb80fd0..08e5731617d 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -89,6 +89,10 @@ static bitmap computed[OST_END];
/* Maximum value of offset we consider to be addition. */
static unsigned HOST_WIDE_INT offset_limit;
+/* Tell the generic SSA updater what kind of update is needed after the pass
+ executes. */
+static unsigned todo;
+
/* Return true if VAL represents an initial size for OBJECT_SIZE_TYPE. */
static inline bool
@@ -787,6 +791,73 @@ alloc_object_size (const gcall *call, int object_size_type)
return bytes ? bytes : size_unknown (object_size_type);
}
+/* Compute __builtin_object_size for CALL, which is a call to either
+ BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which it is.
+ OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
+ If unknown, return size_unknown (object_size_type). */
+
+static tree
+strdup_object_size (const gcall *call, int object_size_type, bool is_strndup)
+{
+ tree src = gimple_call_arg (call, 0);
+ tree sz = size_unknown (object_size_type);
+ tree n = NULL_TREE;
+
+ if (is_strndup)
+ n = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
+ gimple_call_arg (call, 1));
+
+
+ /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer fold it the
+ way it likes. */
+ if (!is_strndup)
+ {
+ tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
+ if (strlen_fn)
+ {
+ sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
+ build_call_expr (strlen_fn, 1, src));
+ todo = TODO_update_ssa_only_virtuals;
+ }
+ }
+
+ /* In all other cases, return the size of SRC since the object size cannot
+ exceed that. We cannot do this for OST_MINIMUM unless SRC points into a
+ string constant since otherwise the object size could go all the way down
+ to zero. */
+ if (!size_valid_p (sz, object_size_type)
+ || size_unknown_p (sz, object_size_type))
+ {
+ tree wholesrc = NULL_TREE;
+ if (TREE_CODE (src) == ADDR_EXPR)
+ wholesrc = get_base_address (TREE_OPERAND (src, 0));
+
+ /* If the source points within a string constant, we try to get its
+ length. */
+ if (wholesrc && TREE_CODE (wholesrc) == STRING_CST)
+ {
+ tree len = c_strlen (src, 0);
+ if (len)
+ sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node, len);
+ }
+
+ /* For maximum estimate, our next best guess is the object size of the
+ source. */
+ if (size_unknown_p (sz, object_size_type)
+ && !(object_size_type & OST_MINIMUM))
+ compute_builtin_object_size (src, object_size_type, &sz);
+ }
+
+ /* String duplication allocates at least one byte, so we should never fail
+ for OST_MINIMUM. */
+ if ((!size_valid_p (sz, object_size_type)
+ || size_unknown_p (sz, object_size_type))
+ && (object_size_type & OST_MINIMUM))
+ sz = size_one_node;
+
+ /* Factor in the N. */
+ return n ? fold_build2 (MIN_EXPR, sizetype, n, sz) : sz;
+}
/* If object size is propagated from one of function's arguments directly
to its return value, return that argument for GIMPLE_CALL statement CALL.
@@ -1233,12 +1304,19 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
{
int object_size_type = osi->object_size_type;
unsigned int varno = SSA_NAME_VERSION (ptr);
+ tree bytes = NULL_TREE;
gcc_assert (is_gimple_call (call));
gcc_assert (!object_sizes_unknown_p (object_size_type, varno));
gcc_assert (osi->pass == 0);
- tree bytes = alloc_object_size (call, object_size_type);
+
+ bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
+ bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
+ if (is_strdup || is_strndup)
+ bytes = strdup_object_size (call, object_size_type, is_strndup);
+ else
+ bytes = alloc_object_size (call, object_size_type);
if (!size_valid_p (bytes, object_size_type))
bytes = size_unknown (object_size_type);
@@ -1998,6 +2076,8 @@ dynamic_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
static unsigned int
object_sizes_execute (function *fun, bool early)
{
+ todo = 0;
+
basic_block bb;
FOR_EACH_BB_FN (bb, fun)
{
@@ -2094,7 +2174,7 @@ object_sizes_execute (function *fun, bool early)
}
fini_object_sizes ();
- return 0;
+ return todo;
}
/* Simple pass to optimize all __builtin_object_size () builtins. */
--
2.37.3
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] tree-object-size: Support strndup and strdup
2022-11-04 12:48 ` [PATCH v2] " Siddhesh Poyarekar
@ 2022-11-04 13:43 ` Prathamesh Kulkarni
2022-11-04 13:47 ` Siddhesh Poyarekar
2022-11-17 19:47 ` Siddhesh Poyarekar
2022-11-20 15:42 ` Jeff Law
2 siblings, 1 reply; 16+ messages in thread
From: Prathamesh Kulkarni @ 2022-11-04 13:43 UTC (permalink / raw)
To: Siddhesh Poyarekar; +Cc: gcc-patches, jakub
On Fri, 4 Nov 2022 at 18:18, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> Use string length of input to strdup to determine the usable size of the
> resulting object. Avoid doing the same for strndup since there's a
> chance that the input may be too large, resulting in an unnecessary
> overhead or worse, the input may not be NULL terminated, resulting in a
> crash where there would otherwise have been none.
>
> gcc/ChangeLog:
>
> * tree-object-size.cc (todo): New variable.
> (object_sizes_execute): Use it.
> (strdup_object_size): New function.
> (call_object_size): Use it.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
> test_strndup, test_strdup_min, test_strndup_min): New tests.
> (main): Call them.
> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
> warnings.
> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
> ---
> Tested:
>
> - x86_64 bootstrap and testsuite run
> - i686 build and testsuite run
> - ubsan bootstrap
>
> .../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++
> .../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
> gcc/testsuite/gcc.dg/builtin-object-size-1.c | 94 +++++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-2.c | 94 +++++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-3.c | 95 ++++++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-4.c | 94 +++++++++++++++++-
> gcc/tree-object-size.cc | 84 +++++++++++++++-
> 10 files changed, 502 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> index 01a280b2d7b..4f1606a486b 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> @@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start, size_t end, int incr)
> return __builtin_dynamic_object_size (ptr, 0);
> }
>
> +/* strdup/strndup. */
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strdup (const char *in)
> +{
> + char *res = __builtin_strdup (in);
> + return __builtin_dynamic_object_size (res, 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strndup (const char *in, size_t bound)
> +{
> + char *res = __builtin_strndup (in, bound);
> + return __builtin_dynamic_object_size (res, 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strdup_min (const char *in)
> +{
> + char *res = __builtin_strdup (in);
> + return __builtin_dynamic_object_size (res, 2);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strndup_min (const char *in, size_t bound)
> +{
> + char *res = __builtin_strndup (in, bound);
> + return __builtin_dynamic_object_size (res, 2);
> +}
> +
> /* Other tests. */
>
> struct TV4
> @@ -651,6 +685,15 @@ main (int argc, char **argv)
> int *t = test_pr105736 (&val3);
> if (__builtin_dynamic_object_size (t, 0) != -1)
> FAIL ();
> + const char *str = "hello world";
> + if (test_strdup (str) != __builtin_strlen (str) + 1)
> + FAIL ();
> + if (test_strndup (str, 4) != 5)
> + FAIL ();
> + if (test_strdup_min (str) != __builtin_strlen (str) + 1)
> + FAIL ();
> + if (test_strndup_min (str, 4) != 1)
> + FAIL ();
>
> if (nfails > 0)
> __builtin_abort ();
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> index 7cc8b1c9488..8f17c8edcaf 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> index 267dbf48ca7..3677782ff1c 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> index fb9dc56da7e..5b6987b7773 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> index 870548b4206..9d796224e96 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> index b772e2da9b9..c6e5b4c29f8 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -629,6 +632,94 @@ test10 (void)
> }
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test11 (void)
> +{
> + int i = 0;
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 0) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 0) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 0) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 0) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 0) != 33)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 0) != 64)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 0) != 27)
> +#else
> + if (__builtin_object_size (res, 0) != (size_t) -1)
> +#endif
> + abort ();
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 0) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 0) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 0) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 0) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 0) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 0) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -644,5 +735,6 @@ main (void)
> test8 ();
> test9 (1);
> test10 ();
> + test11 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> index 2729538da17..639a83cfd39 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -544,6 +547,94 @@ test8 (unsigned cond)
> #endif
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test9 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 1) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 1) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 1) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 1) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 1) != 33)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 1) != 64)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 1) != 27)
> +#else
> + if (__builtin_object_size (res, 1) != (size_t) -1)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 1) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 1) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 1) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 1) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 1) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 1) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -557,5 +648,6 @@ main (void)
> test6 ();
> test7 ();
> test8 (1);
> + test9 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> index 44a99189776..ff4f1747334 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -636,6 +639,95 @@ test10 (void)
> }
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test11 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 2) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 2) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 2) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 2) != 27)
> +#else
> + if (__builtin_object_size (res, 2) != 1)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 2) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 2) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 2) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -651,5 +743,6 @@ main (void)
> test8 ();
> test9 (1);
> test10 ();
> + test11 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> index b9fddfed036..4c007c364b7 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -517,6 +520,94 @@ test8 (unsigned cond)
> #endif
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test9 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 3) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 3) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 3) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 3) != 27)
> +#else
> + if (__builtin_object_size (res, 3) != 1)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 3) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 3) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 3) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -530,5 +621,6 @@ main (void)
> test6 ();
> test7 ();
> test8 (1);
> + test9 ();
> exit (0);
> }
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 1f04cb80fd0..08e5731617d 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -89,6 +89,10 @@ static bitmap computed[OST_END];
> /* Maximum value of offset we consider to be addition. */
> static unsigned HOST_WIDE_INT offset_limit;
>
> +/* Tell the generic SSA updater what kind of update is needed after the pass
> + executes. */
> +static unsigned todo;
> +
> /* Return true if VAL represents an initial size for OBJECT_SIZE_TYPE. */
>
> static inline bool
> @@ -787,6 +791,73 @@ alloc_object_size (const gcall *call, int object_size_type)
> return bytes ? bytes : size_unknown (object_size_type);
> }
>
> +/* Compute __builtin_object_size for CALL, which is a call to either
> + BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which it is.
> + OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
> + If unknown, return size_unknown (object_size_type). */
> +
> +static tree
> +strdup_object_size (const gcall *call, int object_size_type, bool is_strndup)
> +{
> + tree src = gimple_call_arg (call, 0);
> + tree sz = size_unknown (object_size_type);
> + tree n = NULL_TREE;
> +
> + if (is_strndup)
> + n = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
> + gimple_call_arg (call, 1));
> +
> +
> + /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer fold it the
> + way it likes. */
> + if (!is_strndup)
Sorry to nitpick, should this just be under else ? Since this
condition will be false if the above if (is_strndup)
is true.
Thanks,
Prathamesh
> + {
> + tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
> + if (strlen_fn)
> + {
> + sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
> + build_call_expr (strlen_fn, 1, src));
> + todo = TODO_update_ssa_only_virtuals;
> + }
> + }
> +
> + /* In all other cases, return the size of SRC since the object size cannot
> + exceed that. We cannot do this for OST_MINIMUM unless SRC points into a
> + string constant since otherwise the object size could go all the way down
> + to zero. */
> + if (!size_valid_p (sz, object_size_type)
> + || size_unknown_p (sz, object_size_type))
> + {
> + tree wholesrc = NULL_TREE;
> + if (TREE_CODE (src) == ADDR_EXPR)
> + wholesrc = get_base_address (TREE_OPERAND (src, 0));
> +
> + /* If the source points within a string constant, we try to get its
> + length. */
> + if (wholesrc && TREE_CODE (wholesrc) == STRING_CST)
> + {
> + tree len = c_strlen (src, 0);
> + if (len)
> + sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node, len);
> + }
> +
> + /* For maximum estimate, our next best guess is the object size of the
> + source. */
> + if (size_unknown_p (sz, object_size_type)
> + && !(object_size_type & OST_MINIMUM))
> + compute_builtin_object_size (src, object_size_type, &sz);
> + }
> +
> + /* String duplication allocates at least one byte, so we should never fail
> + for OST_MINIMUM. */
> + if ((!size_valid_p (sz, object_size_type)
> + || size_unknown_p (sz, object_size_type))
> + && (object_size_type & OST_MINIMUM))
> + sz = size_one_node;
> +
> + /* Factor in the N. */
> + return n ? fold_build2 (MIN_EXPR, sizetype, n, sz) : sz;
> +}
>
> /* If object size is propagated from one of function's arguments directly
> to its return value, return that argument for GIMPLE_CALL statement CALL.
> @@ -1233,12 +1304,19 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
> {
> int object_size_type = osi->object_size_type;
> unsigned int varno = SSA_NAME_VERSION (ptr);
> + tree bytes = NULL_TREE;
>
> gcc_assert (is_gimple_call (call));
>
> gcc_assert (!object_sizes_unknown_p (object_size_type, varno));
> gcc_assert (osi->pass == 0);
> - tree bytes = alloc_object_size (call, object_size_type);
> +
> + bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
> + bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
> + if (is_strdup || is_strndup)
> + bytes = strdup_object_size (call, object_size_type, is_strndup);
> + else
> + bytes = alloc_object_size (call, object_size_type);
>
> if (!size_valid_p (bytes, object_size_type))
> bytes = size_unknown (object_size_type);
> @@ -1998,6 +2076,8 @@ dynamic_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
> static unsigned int
> object_sizes_execute (function *fun, bool early)
> {
> + todo = 0;
> +
> basic_block bb;
> FOR_EACH_BB_FN (bb, fun)
> {
> @@ -2094,7 +2174,7 @@ object_sizes_execute (function *fun, bool early)
> }
>
> fini_object_sizes ();
> - return 0;
> + return todo;
> }
>
> /* Simple pass to optimize all __builtin_object_size () builtins. */
> --
> 2.37.3
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] tree-object-size: Support strndup and strdup
2022-11-04 13:43 ` Prathamesh Kulkarni
@ 2022-11-04 13:47 ` Siddhesh Poyarekar
0 siblings, 0 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-11-04 13:47 UTC (permalink / raw)
To: Prathamesh Kulkarni; +Cc: gcc-patches, jakub
On 2022-11-04 09:43, Prathamesh Kulkarni wrote:
> On Fri, 4 Nov 2022 at 18:18, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> Use string length of input to strdup to determine the usable size of the
>> resulting object. Avoid doing the same for strndup since there's a
>> chance that the input may be too large, resulting in an unnecessary
>> overhead or worse, the input may not be NULL terminated, resulting in a
>> crash where there would otherwise have been none.
>>
>> gcc/ChangeLog:
>>
>> * tree-object-size.cc (todo): New variable.
>> (object_sizes_execute): Use it.
>> (strdup_object_size): New function.
>> (call_object_size): Use it.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
>> test_strndup, test_strdup_min, test_strndup_min): New tests.
>> (main): Call them.
>> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
>> warnings.
>> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
>> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
>> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
>> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test11): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test9): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test11): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test9): New test.
>> (main): Call it.
>> ---
>> Tested:
>>
>> - x86_64 bootstrap and testsuite run
>> - i686 build and testsuite run
>> - ubsan bootstrap
>>
>> .../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++
>> .../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
>> .../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
>> .../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
>> .../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
>> gcc/testsuite/gcc.dg/builtin-object-size-1.c | 94 +++++++++++++++++-
>> gcc/testsuite/gcc.dg/builtin-object-size-2.c | 94 +++++++++++++++++-
>> gcc/testsuite/gcc.dg/builtin-object-size-3.c | 95 ++++++++++++++++++-
>> gcc/testsuite/gcc.dg/builtin-object-size-4.c | 94 +++++++++++++++++-
>> gcc/tree-object-size.cc | 84 +++++++++++++++-
>> 10 files changed, 502 insertions(+), 10 deletions(-)
>>
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>> index 01a280b2d7b..4f1606a486b 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
>> @@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start, size_t end, int incr)
>> return __builtin_dynamic_object_size (ptr, 0);
>> }
>>
>> +/* strdup/strndup. */
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strdup (const char *in)
>> +{
>> + char *res = __builtin_strdup (in);
>> + return __builtin_dynamic_object_size (res, 0);
>> +}
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strndup (const char *in, size_t bound)
>> +{
>> + char *res = __builtin_strndup (in, bound);
>> + return __builtin_dynamic_object_size (res, 0);
>> +}
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strdup_min (const char *in)
>> +{
>> + char *res = __builtin_strdup (in);
>> + return __builtin_dynamic_object_size (res, 2);
>> +}
>> +
>> +size_t
>> +__attribute__ ((noinline))
>> +test_strndup_min (const char *in, size_t bound)
>> +{
>> + char *res = __builtin_strndup (in, bound);
>> + return __builtin_dynamic_object_size (res, 2);
>> +}
>> +
>> /* Other tests. */
>>
>> struct TV4
>> @@ -651,6 +685,15 @@ main (int argc, char **argv)
>> int *t = test_pr105736 (&val3);
>> if (__builtin_dynamic_object_size (t, 0) != -1)
>> FAIL ();
>> + const char *str = "hello world";
>> + if (test_strdup (str) != __builtin_strlen (str) + 1)
>> + FAIL ();
>> + if (test_strndup (str, 4) != 5)
>> + FAIL ();
>> + if (test_strdup_min (str) != __builtin_strlen (str) + 1)
>> + FAIL ();
>> + if (test_strndup_min (str, 4) != 1)
>> + FAIL ();
>>
>> if (nfails > 0)
>> __builtin_abort ();
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>> index 7cc8b1c9488..8f17c8edcaf 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>> index 267dbf48ca7..3677782ff1c 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>> index fb9dc56da7e..5b6987b7773 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>> index 870548b4206..9d796224e96 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> #define __builtin_object_size __builtin_dynamic_object_size
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>> index b772e2da9b9..c6e5b4c29f8 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>>
>> struct A
>> {
>> @@ -629,6 +632,94 @@ test10 (void)
>> }
>> }
>>
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test11 (void)
>> +{
>> + int i = 0;
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 0) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 0) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 0) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 0) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 0) != 33)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 0) != 64)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 0) != 27)
>> +#else
>> + if (__builtin_object_size (res, 0) != (size_t) -1)
>> +#endif
>> + abort ();
>> + free (res);
>> + free (ptr2);
>> +
>> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 0) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 24);
>> + if (__builtin_object_size (res, 0) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 2);
>> + if (__builtin_object_size (res, 0) != 3)
>> + abort ();
>> + free (res);
>> +
>> + res = strdup (&ptr[4]);
>> + if (__builtin_object_size (res, 0) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 4);
>> + if (__builtin_object_size (res, 0) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 1);
>> + if (__builtin_object_size (res, 0) != 1)
>> + abort ();
>> + free (res);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -644,5 +735,6 @@ main (void)
>> test8 ();
>> test9 (1);
>> test10 ();
>> + test11 ();
>> exit (0);
>> }
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>> index 2729538da17..639a83cfd39 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>>
>> struct A
>> {
>> @@ -544,6 +547,94 @@ test8 (unsigned cond)
>> #endif
>> }
>>
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test9 (void)
>> +{
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 1) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 1) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 1) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 1) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 1) != 33)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 1) != 64)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 1) != 27)
>> +#else
>> + if (__builtin_object_size (res, 1) != (size_t) -1)
>> +#endif
>> + abort ();
>> +
>> + free (res);
>> + free (ptr2);
>> +
>> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 1) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 24);
>> + if (__builtin_object_size (res, 1) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 2);
>> + if (__builtin_object_size (res, 1) != 3)
>> + abort ();
>> + free (res);
>> +
>> + res = strdup (&ptr[4]);
>> + if (__builtin_object_size (res, 1) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 4);
>> + if (__builtin_object_size (res, 1) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 1);
>> + if (__builtin_object_size (res, 1) != 1)
>> + abort ();
>> + free (res);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -557,5 +648,6 @@ main (void)
>> test6 ();
>> test7 ();
>> test8 (1);
>> + test9 ();
>> exit (0);
>> }
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>> index 44a99189776..ff4f1747334 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>>
>> struct A
>> {
>> @@ -636,6 +639,95 @@ test10 (void)
>> }
>> }
>>
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test11 (void)
>> +{
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 2) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 2) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 2) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 2) != 1)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 2) != 1)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 2) != 1)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 2) != 27)
>> +#else
>> + if (__builtin_object_size (res, 2) != 1)
>> +#endif
>> + abort ();
>> +
>> + free (res);
>> + free (ptr2);
>> +
>> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 2) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 24);
>> + if (__builtin_object_size (res, 2) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 2);
>> + if (__builtin_object_size (res, 2) != 3)
>> + abort ();
>> + free (res);
>> +
>> + res = strdup (&ptr[4]);
>> + if (__builtin_object_size (res, 2) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 4);
>> + if (__builtin_object_size (res, 2) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 1);
>> + if (__builtin_object_size (res, 2) != 1)
>> + abort ();
>> + free (res);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -651,5 +743,6 @@ main (void)
>> test8 ();
>> test9 (1);
>> test10 ();
>> + test11 ();
>> exit (0);
>> }
>> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>> index b9fddfed036..4c007c364b7 100644
>> --- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
>> @@ -1,5 +1,5 @@
>> /* { dg-do run } */
>> -/* { dg-options "-O2" } */
>> +/* { dg-options "-O2 -Wno-stringop-overread" } */
>> /* { dg-require-effective-target alloca } */
>>
>> typedef __SIZE_TYPE__ size_t;
>> @@ -7,10 +7,13 @@ extern void abort (void);
>> extern void exit (int);
>> extern void *malloc (size_t);
>> extern void *calloc (size_t, size_t);
>> +extern void free (void *);
>> extern void *alloca (size_t);
>> extern void *memcpy (void *, const void *, size_t);
>> extern void *memset (void *, int, size_t);
>> extern char *strcpy (char *, const char *);
>> +extern char *strdup (const char *);
>> +extern char *strndup (const char *, size_t);
>>
>> struct A
>> {
>> @@ -517,6 +520,94 @@ test8 (unsigned cond)
>> #endif
>> }
>>
>> +/* Tests for strdup/strndup. */
>> +size_t
>> +__attribute__ ((noinline))
>> +test9 (void)
>> +{
>> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
>> + char *res = strndup (ptr, 21);
>> + if (__builtin_object_size (res, 3) != 22)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr, 32);
>> + if (__builtin_object_size (res, 3) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 3) != 27)
>> + abort ();
>> +
>> + free (res);
>> +
>> + char *ptr2 = malloc (64);
>> + strcpy (ptr2, ptr);
>> +
>> + res = strndup (ptr2, 21);
>> + if (__builtin_object_size (res, 3) != 1)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 32);
>> + if (__builtin_object_size (res, 3) != 1)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strndup (ptr2, 128);
>> + if (__builtin_object_size (res, 3) != 1)
>> + abort ();
>> +
>> + free (res);
>> +
>> + res = strdup (ptr2);
>> +#ifdef __builtin_object_size
>> + if (__builtin_object_size (res, 3) != 27)
>> +#else
>> + if (__builtin_object_size (res, 3) != 1)
>> +#endif
>> + abort ();
>> +
>> + free (res);
>> + free (ptr2);
>> +
>> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
>> + res = strdup (ptr);
>> + if (__builtin_object_size (res, 3) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 24);
>> + if (__builtin_object_size (res, 3) != 5)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (ptr, 2);
>> + if (__builtin_object_size (res, 3) != 3)
>> + abort ();
>> + free (res);
>> +
>> + res = strdup (&ptr[4]);
>> + if (__builtin_object_size (res, 3) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 4);
>> + if (__builtin_object_size (res, 3) != 1)
>> + abort ();
>> + free (res);
>> +
>> + res = strndup (&ptr[4], 1);
>> + if (__builtin_object_size (res, 3) != 1)
>> + abort ();
>> + free (res);
>> +}
>> +
>> int
>> main (void)
>> {
>> @@ -530,5 +621,6 @@ main (void)
>> test6 ();
>> test7 ();
>> test8 (1);
>> + test9 ();
>> exit (0);
>> }
>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>> index 1f04cb80fd0..08e5731617d 100644
>> --- a/gcc/tree-object-size.cc
>> +++ b/gcc/tree-object-size.cc
>> @@ -89,6 +89,10 @@ static bitmap computed[OST_END];
>> /* Maximum value of offset we consider to be addition. */
>> static unsigned HOST_WIDE_INT offset_limit;
>>
>> +/* Tell the generic SSA updater what kind of update is needed after the pass
>> + executes. */
>> +static unsigned todo;
>> +
>> /* Return true if VAL represents an initial size for OBJECT_SIZE_TYPE. */
>>
>> static inline bool
>> @@ -787,6 +791,73 @@ alloc_object_size (const gcall *call, int object_size_type)
>> return bytes ? bytes : size_unknown (object_size_type);
>> }
>>
>> +/* Compute __builtin_object_size for CALL, which is a call to either
>> + BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which it is.
>> + OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
>> + If unknown, return size_unknown (object_size_type). */
>> +
>> +static tree
>> +strdup_object_size (const gcall *call, int object_size_type, bool is_strndup)
>> +{
>> + tree src = gimple_call_arg (call, 0);
>> + tree sz = size_unknown (object_size_type);
>> + tree n = NULL_TREE;
>> +
>> + if (is_strndup)
>> + n = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
>> + gimple_call_arg (call, 1));
>> +
>> +
>> + /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer fold it the
>> + way it likes. */
>> + if (!is_strndup)
> Sorry to nitpick, should this just be under else ? Since this
> condition will be false if the above if (is_strndup)
> is true.
Fixed locally. I'll resend once I have more comments on the rest of the
patch.
Thanks,
Sid
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] tree-object-size: Support strndup and strdup
2022-11-04 12:48 ` [PATCH v2] " Siddhesh Poyarekar
2022-11-04 13:43 ` Prathamesh Kulkarni
@ 2022-11-17 19:47 ` Siddhesh Poyarekar
2022-11-20 15:42 ` Jeff Law
2 siblings, 0 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-11-17 19:47 UTC (permalink / raw)
To: gcc-patches; +Cc: jakub
Ping!
On 2022-11-04 08:48, Siddhesh Poyarekar wrote:
> Use string length of input to strdup to determine the usable size of the
> resulting object. Avoid doing the same for strndup since there's a
> chance that the input may be too large, resulting in an unnecessary
> overhead or worse, the input may not be NULL terminated, resulting in a
> crash where there would otherwise have been none.
>
> gcc/ChangeLog:
>
> * tree-object-size.cc (todo): New variable.
> (object_sizes_execute): Use it.
> (strdup_object_size): New function.
> (call_object_size): Use it.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
> test_strndup, test_strdup_min, test_strndup_min): New tests.
> (main): Call them.
> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
> warnings.
> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
> ---
> Tested:
>
> - x86_64 bootstrap and testsuite run
> - i686 build and testsuite run
> - ubsan bootstrap
>
> .../gcc.dg/builtin-dynamic-object-size-0.c | 43 +++++++++
> .../gcc.dg/builtin-dynamic-object-size-1.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-2.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-3.c | 2 +-
> .../gcc.dg/builtin-dynamic-object-size-4.c | 2 +-
> gcc/testsuite/gcc.dg/builtin-object-size-1.c | 94 +++++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-2.c | 94 +++++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-3.c | 95 ++++++++++++++++++-
> gcc/testsuite/gcc.dg/builtin-object-size-4.c | 94 +++++++++++++++++-
> gcc/tree-object-size.cc | 84 +++++++++++++++-
> 10 files changed, 502 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> index 01a280b2d7b..4f1606a486b 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-0.c
> @@ -479,6 +479,40 @@ test_loop (int *obj, size_t sz, size_t start, size_t end, int incr)
> return __builtin_dynamic_object_size (ptr, 0);
> }
>
> +/* strdup/strndup. */
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strdup (const char *in)
> +{
> + char *res = __builtin_strdup (in);
> + return __builtin_dynamic_object_size (res, 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strndup (const char *in, size_t bound)
> +{
> + char *res = __builtin_strndup (in, bound);
> + return __builtin_dynamic_object_size (res, 0);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strdup_min (const char *in)
> +{
> + char *res = __builtin_strdup (in);
> + return __builtin_dynamic_object_size (res, 2);
> +}
> +
> +size_t
> +__attribute__ ((noinline))
> +test_strndup_min (const char *in, size_t bound)
> +{
> + char *res = __builtin_strndup (in, bound);
> + return __builtin_dynamic_object_size (res, 2);
> +}
> +
> /* Other tests. */
>
> struct TV4
> @@ -651,6 +685,15 @@ main (int argc, char **argv)
> int *t = test_pr105736 (&val3);
> if (__builtin_dynamic_object_size (t, 0) != -1)
> FAIL ();
> + const char *str = "hello world";
> + if (test_strdup (str) != __builtin_strlen (str) + 1)
> + FAIL ();
> + if (test_strndup (str, 4) != 5)
> + FAIL ();
> + if (test_strdup_min (str) != __builtin_strlen (str) + 1)
> + FAIL ();
> + if (test_strndup_min (str, 4) != 1)
> + FAIL ();
>
> if (nfails > 0)
> __builtin_abort ();
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> index 7cc8b1c9488..8f17c8edcaf 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-1.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> index 267dbf48ca7..3677782ff1c 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> index fb9dc56da7e..5b6987b7773 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-3.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> index 870548b4206..9d796224e96 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-4.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> #define __builtin_object_size __builtin_dynamic_object_size
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> index b772e2da9b9..c6e5b4c29f8 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -629,6 +632,94 @@ test10 (void)
> }
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test11 (void)
> +{
> + int i = 0;
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 0) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 0) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 0) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 0) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 0) != 33)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 0) != 64)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 0) != 27)
> +#else
> + if (__builtin_object_size (res, 0) != (size_t) -1)
> +#endif
> + abort ();
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 0) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 0) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 0) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 0) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 0) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 0) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -644,5 +735,6 @@ main (void)
> test8 ();
> test9 (1);
> test10 ();
> + test11 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-2.c b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> index 2729538da17..639a83cfd39 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-2.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -544,6 +547,94 @@ test8 (unsigned cond)
> #endif
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test9 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 1) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 1) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 1) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 1) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 1) != 33)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 1) != 64)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 1) != 27)
> +#else
> + if (__builtin_object_size (res, 1) != (size_t) -1)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 1) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 1) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 1) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 1) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 1) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 1) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -557,5 +648,6 @@ main (void)
> test6 ();
> test7 ();
> test8 (1);
> + test9 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> index 44a99189776..ff4f1747334 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -636,6 +639,95 @@ test10 (void)
> }
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test11 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 2) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 2) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 2) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 2) != 27)
> +#else
> + if (__builtin_object_size (res, 2) != 1)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 2) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 2) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 2) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 2) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -651,5 +743,6 @@ main (void)
> test8 ();
> test9 (1);
> test10 ();
> + test11 ();
> exit (0);
> }
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> index b9fddfed036..4c007c364b7 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-O2" } */
> +/* { dg-options "-O2 -Wno-stringop-overread" } */
> /* { dg-require-effective-target alloca } */
>
> typedef __SIZE_TYPE__ size_t;
> @@ -7,10 +7,13 @@ extern void abort (void);
> extern void exit (int);
> extern void *malloc (size_t);
> extern void *calloc (size_t, size_t);
> +extern void free (void *);
> extern void *alloca (size_t);
> extern void *memcpy (void *, const void *, size_t);
> extern void *memset (void *, int, size_t);
> extern char *strcpy (char *, const char *);
> +extern char *strdup (const char *);
> +extern char *strndup (const char *, size_t);
>
> struct A
> {
> @@ -517,6 +520,94 @@ test8 (unsigned cond)
> #endif
> }
>
> +/* Tests for strdup/strndup. */
> +size_t
> +__attribute__ ((noinline))
> +test9 (void)
> +{
> + const char *ptr = "abcdefghijklmnopqrstuvwxyz";
> + char *res = strndup (ptr, 21);
> + if (__builtin_object_size (res, 3) != 22)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr, 32);
> + if (__builtin_object_size (res, 3) != 27)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 3) != 27)
> + abort ();
> +
> + free (res);
> +
> + char *ptr2 = malloc (64);
> + strcpy (ptr2, ptr);
> +
> + res = strndup (ptr2, 21);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 32);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strndup (ptr2, 128);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> +
> + free (res);
> +
> + res = strdup (ptr2);
> +#ifdef __builtin_object_size
> + if (__builtin_object_size (res, 3) != 27)
> +#else
> + if (__builtin_object_size (res, 3) != 1)
> +#endif
> + abort ();
> +
> + free (res);
> + free (ptr2);
> +
> + ptr = "abcd\0efghijklmnopqrstuvwxyz";
> + res = strdup (ptr);
> + if (__builtin_object_size (res, 3) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 24);
> + if (__builtin_object_size (res, 3) != 5)
> + abort ();
> + free (res);
> +
> + res = strndup (ptr, 2);
> + if (__builtin_object_size (res, 3) != 3)
> + abort ();
> + free (res);
> +
> + res = strdup (&ptr[4]);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 4);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> + free (res);
> +
> + res = strndup (&ptr[4], 1);
> + if (__builtin_object_size (res, 3) != 1)
> + abort ();
> + free (res);
> +}
> +
> int
> main (void)
> {
> @@ -530,5 +621,6 @@ main (void)
> test6 ();
> test7 ();
> test8 (1);
> + test9 ();
> exit (0);
> }
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 1f04cb80fd0..08e5731617d 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -89,6 +89,10 @@ static bitmap computed[OST_END];
> /* Maximum value of offset we consider to be addition. */
> static unsigned HOST_WIDE_INT offset_limit;
>
> +/* Tell the generic SSA updater what kind of update is needed after the pass
> + executes. */
> +static unsigned todo;
> +
> /* Return true if VAL represents an initial size for OBJECT_SIZE_TYPE. */
>
> static inline bool
> @@ -787,6 +791,73 @@ alloc_object_size (const gcall *call, int object_size_type)
> return bytes ? bytes : size_unknown (object_size_type);
> }
>
> +/* Compute __builtin_object_size for CALL, which is a call to either
> + BUILT_IN_STRDUP or BUILT_IN_STRNDUP; IS_STRNDUP indicates which it is.
> + OBJECT_SIZE_TYPE is the second argument from __builtin_object_size.
> + If unknown, return size_unknown (object_size_type). */
> +
> +static tree
> +strdup_object_size (const gcall *call, int object_size_type, bool is_strndup)
> +{
> + tree src = gimple_call_arg (call, 0);
> + tree sz = size_unknown (object_size_type);
> + tree n = NULL_TREE;
> +
> + if (is_strndup)
> + n = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
> + gimple_call_arg (call, 1));
> +
> +
> + /* For strdup, simply emit strlen (SRC) + 1 and let the optimizer fold it the
> + way it likes. */
> + if (!is_strndup)
> + {
> + tree strlen_fn = builtin_decl_implicit (BUILT_IN_STRLEN);
> + if (strlen_fn)
> + {
> + sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node,
> + build_call_expr (strlen_fn, 1, src));
> + todo = TODO_update_ssa_only_virtuals;
> + }
> + }
> +
> + /* In all other cases, return the size of SRC since the object size cannot
> + exceed that. We cannot do this for OST_MINIMUM unless SRC points into a
> + string constant since otherwise the object size could go all the way down
> + to zero. */
> + if (!size_valid_p (sz, object_size_type)
> + || size_unknown_p (sz, object_size_type))
> + {
> + tree wholesrc = NULL_TREE;
> + if (TREE_CODE (src) == ADDR_EXPR)
> + wholesrc = get_base_address (TREE_OPERAND (src, 0));
> +
> + /* If the source points within a string constant, we try to get its
> + length. */
> + if (wholesrc && TREE_CODE (wholesrc) == STRING_CST)
> + {
> + tree len = c_strlen (src, 0);
> + if (len)
> + sz = fold_build2 (PLUS_EXPR, sizetype, size_one_node, len);
> + }
> +
> + /* For maximum estimate, our next best guess is the object size of the
> + source. */
> + if (size_unknown_p (sz, object_size_type)
> + && !(object_size_type & OST_MINIMUM))
> + compute_builtin_object_size (src, object_size_type, &sz);
> + }
> +
> + /* String duplication allocates at least one byte, so we should never fail
> + for OST_MINIMUM. */
> + if ((!size_valid_p (sz, object_size_type)
> + || size_unknown_p (sz, object_size_type))
> + && (object_size_type & OST_MINIMUM))
> + sz = size_one_node;
> +
> + /* Factor in the N. */
> + return n ? fold_build2 (MIN_EXPR, sizetype, n, sz) : sz;
> +}
>
> /* If object size is propagated from one of function's arguments directly
> to its return value, return that argument for GIMPLE_CALL statement CALL.
> @@ -1233,12 +1304,19 @@ call_object_size (struct object_size_info *osi, tree ptr, gcall *call)
> {
> int object_size_type = osi->object_size_type;
> unsigned int varno = SSA_NAME_VERSION (ptr);
> + tree bytes = NULL_TREE;
>
> gcc_assert (is_gimple_call (call));
>
> gcc_assert (!object_sizes_unknown_p (object_size_type, varno));
> gcc_assert (osi->pass == 0);
> - tree bytes = alloc_object_size (call, object_size_type);
> +
> + bool is_strdup = gimple_call_builtin_p (call, BUILT_IN_STRDUP);
> + bool is_strndup = gimple_call_builtin_p (call, BUILT_IN_STRNDUP);
> + if (is_strdup || is_strndup)
> + bytes = strdup_object_size (call, object_size_type, is_strndup);
> + else
> + bytes = alloc_object_size (call, object_size_type);
>
> if (!size_valid_p (bytes, object_size_type))
> bytes = size_unknown (object_size_type);
> @@ -1998,6 +2076,8 @@ dynamic_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
> static unsigned int
> object_sizes_execute (function *fun, bool early)
> {
> + todo = 0;
> +
> basic_block bb;
> FOR_EACH_BB_FN (bb, fun)
> {
> @@ -2094,7 +2174,7 @@ object_sizes_execute (function *fun, bool early)
> }
>
> fini_object_sizes ();
> - return 0;
> + return todo;
> }
>
> /* Simple pass to optimize all __builtin_object_size () builtins. */
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] tree-object-size: Support strndup and strdup
2022-11-04 12:48 ` [PATCH v2] " Siddhesh Poyarekar
2022-11-04 13:43 ` Prathamesh Kulkarni
2022-11-17 19:47 ` Siddhesh Poyarekar
@ 2022-11-20 15:42 ` Jeff Law
2022-11-21 14:27 ` Siddhesh Poyarekar
2 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2022-11-20 15:42 UTC (permalink / raw)
To: Siddhesh Poyarekar, gcc-patches; +Cc: jakub
On 11/4/22 06:48, Siddhesh Poyarekar wrote:
> Use string length of input to strdup to determine the usable size of the
> resulting object. Avoid doing the same for strndup since there's a
> chance that the input may be too large, resulting in an unnecessary
> overhead or worse, the input may not be NULL terminated, resulting in a
> crash where there would otherwise have been none.
>
> gcc/ChangeLog:
>
> * tree-object-size.cc (todo): New variable.
> (object_sizes_execute): Use it.
> (strdup_object_size): New function.
> (call_object_size): Use it.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
> test_strndup, test_strdup_min, test_strndup_min): New tests.
> (main): Call them.
> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
> warnings.
> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test11): New test.
> (main): Call it.
> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
> Declare free, strdup and strndup.
> (test9): New test.
> (main): Call it.
I'm struggling to see how the SSA updating is correct. Yes we need to
update the virtuals due to the introduction of the call to strlen,
particularly when SRC is not a string constant. But do we need to do more?
Don't we end up gimplifying the 1 + strlenfn (src) expression? Can that
possibly create new SSA_NAMEs? Do those need to be put into SSA form?
I feel like I'm missing something here...
jeff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] tree-object-size: Support strndup and strdup
2022-11-20 15:42 ` Jeff Law
@ 2022-11-21 14:27 ` Siddhesh Poyarekar
2022-11-22 20:43 ` Jeff Law
0 siblings, 1 reply; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-11-21 14:27 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: jakub
On 2022-11-20 10:42, Jeff Law wrote:
>
> On 11/4/22 06:48, Siddhesh Poyarekar wrote:
>> Use string length of input to strdup to determine the usable size of the
>> resulting object. Avoid doing the same for strndup since there's a
>> chance that the input may be too large, resulting in an unnecessary
>> overhead or worse, the input may not be NULL terminated, resulting in a
>> crash where there would otherwise have been none.
>>
>> gcc/ChangeLog:
>>
>> * tree-object-size.cc (todo): New variable.
>> (object_sizes_execute): Use it.
>> (strdup_object_size): New function.
>> (call_object_size): Use it.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
>> test_strndup, test_strdup_min, test_strndup_min): New tests.
>> (main): Call them.
>> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
>> warnings.
>> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
>> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
>> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
>> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test11): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test9): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test11): New test.
>> (main): Call it.
>> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
>> Declare free, strdup and strndup.
>> (test9): New test.
>> (main): Call it.
>
> I'm struggling to see how the SSA updating is correct. Yes we need to
> update the virtuals due to the introduction of the call to strlen,
> particularly when SRC is not a string constant. But do we need to do more?
>
> Don't we end up gimplifying the 1 + strlenfn (src) expression? Can that
> possibly create new SSA_NAMEs? Do those need to be put into SSA form? I
> feel like I'm missing something here...
We do all of that manually in gimplify_size_expressions, the only thing
left to do is updating virtuals AFAICT.
Thanks,
Sid
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] tree-object-size: Support strndup and strdup
2022-11-21 14:27 ` Siddhesh Poyarekar
@ 2022-11-22 20:43 ` Jeff Law
2022-11-22 23:13 ` Siddhesh Poyarekar
0 siblings, 1 reply; 16+ messages in thread
From: Jeff Law @ 2022-11-22 20:43 UTC (permalink / raw)
To: Siddhesh Poyarekar, gcc-patches; +Cc: jakub
On 11/21/22 07:27, Siddhesh Poyarekar wrote:
> On 2022-11-20 10:42, Jeff Law wrote:
>>
>> On 11/4/22 06:48, Siddhesh Poyarekar wrote:
>>> Use string length of input to strdup to determine the usable size of
>>> the
>>> resulting object. Avoid doing the same for strndup since there's a
>>> chance that the input may be too large, resulting in an unnecessary
>>> overhead or worse, the input may not be NULL terminated, resulting in a
>>> crash where there would otherwise have been none.
>>>
>>> gcc/ChangeLog:
>>>
>>> * tree-object-size.cc (todo): New variable.
>>> (object_sizes_execute): Use it.
>>> (strdup_object_size): New function.
>>> (call_object_size): Use it.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
>>> test_strndup, test_strdup_min, test_strndup_min): New tests.
>>> (main): Call them.
>>> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
>>> warnings.
>>> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
>>> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
>>> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
>>> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test11): New test.
>>> (main): Call it.
>>> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test9): New test.
>>> (main): Call it.
>>> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test11): New test.
>>> (main): Call it.
>>> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
>>> Declare free, strdup and strndup.
>>> (test9): New test.
>>> (main): Call it.
>>
>> I'm struggling to see how the SSA updating is correct. Yes we need
>> to update the virtuals due to the introduction of the call to strlen,
>> particularly when SRC is not a string constant. But do we need to do
>> more?
>>
>> Don't we end up gimplifying the 1 + strlenfn (src) expression? Can
>> that possibly create new SSA_NAMEs? Do those need to be put into SSA
>> form? I feel like I'm missing something here...
>
> We do all of that manually in gimplify_size_expressions, the only
> thing left to do is updating virtuals AFAICT.
I guess it's actually buried down in force_gimple_operand and I guess
for temporaries they're not going to be alive across the new gimple
sequence and each destination gets its own SSA_NAME, so it ought to be
safe. Just had to work a bit further through things.
OK for the trunk.
Thanks,
jeff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] tree-object-size: Support strndup and strdup
2022-11-22 20:43 ` Jeff Law
@ 2022-11-22 23:13 ` Siddhesh Poyarekar
0 siblings, 0 replies; 16+ messages in thread
From: Siddhesh Poyarekar @ 2022-11-22 23:13 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: jakub
On 2022-11-22 15:43, Jeff Law wrote:
>
> On 11/21/22 07:27, Siddhesh Poyarekar wrote:
>> On 2022-11-20 10:42, Jeff Law wrote:
>>>
>>> On 11/4/22 06:48, Siddhesh Poyarekar wrote:
>>>> Use string length of input to strdup to determine the usable size of
>>>> the
>>>> resulting object. Avoid doing the same for strndup since there's a
>>>> chance that the input may be too large, resulting in an unnecessary
>>>> overhead or worse, the input may not be NULL terminated, resulting in a
>>>> crash where there would otherwise have been none.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> * tree-object-size.cc (todo): New variable.
>>>> (object_sizes_execute): Use it.
>>>> (strdup_object_size): New function.
>>>> (call_object_size): Use it.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
>>>> test_strndup, test_strdup_min, test_strndup_min): New tests.
>>>> (main): Call them.
>>>> * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
>>>> warnings.
>>>> * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
>>>> * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
>>>> * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
>>>> * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
>>>> Declare free, strdup and strndup.
>>>> (test11): New test.
>>>> (main): Call it.
>>>> * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
>>>> Declare free, strdup and strndup.
>>>> (test9): New test.
>>>> (main): Call it.
>>>> * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
>>>> Declare free, strdup and strndup.
>>>> (test11): New test.
>>>> (main): Call it.
>>>> * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
>>>> Declare free, strdup and strndup.
>>>> (test9): New test.
>>>> (main): Call it.
>>>
>>> I'm struggling to see how the SSA updating is correct. Yes we need
>>> to update the virtuals due to the introduction of the call to strlen,
>>> particularly when SRC is not a string constant. But do we need to do
>>> more?
>>>
>>> Don't we end up gimplifying the 1 + strlenfn (src) expression? Can
>>> that possibly create new SSA_NAMEs? Do those need to be put into SSA
>>> form? I feel like I'm missing something here...
>>
>> We do all of that manually in gimplify_size_expressions, the only
>> thing left to do is updating virtuals AFAICT.
>
> I guess it's actually buried down in force_gimple_operand and I guess
> for temporaries they're not going to be alive across the new gimple
> sequence and each destination gets its own SSA_NAME, so it ought to be
> safe. Just had to work a bit further through things.
>
> OK for the trunk.
Thanks, pushed with the trivial fixup that Prathamesh suggested, i.e.
replaced 'if (!strndup)' with 'else'.
Sid
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-11-22 23:13 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-15 19:23 [PATCH] tree-object-size: Support strndup and strdup Siddhesh Poyarekar
2022-08-29 14:16 ` Siddhesh Poyarekar
2022-09-07 19:21 ` Siddhesh Poyarekar
2022-09-15 14:00 ` Siddhesh Poyarekar
2022-09-22 13:02 ` Jakub Jelinek
2022-09-22 15:26 ` Siddhesh Poyarekar
2022-09-23 13:02 ` Jakub Jelinek
2022-11-02 22:30 ` Siddhesh Poyarekar
2022-11-04 12:48 ` [PATCH v2] " Siddhesh Poyarekar
2022-11-04 13:43 ` Prathamesh Kulkarni
2022-11-04 13:47 ` Siddhesh Poyarekar
2022-11-17 19:47 ` Siddhesh Poyarekar
2022-11-20 15:42 ` Jeff Law
2022-11-21 14:27 ` Siddhesh Poyarekar
2022-11-22 20:43 ` Jeff Law
2022-11-22 23:13 ` Siddhesh Poyarekar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).