On 11/23/21 18:11, Jakub Jelinek wrote:
On Wed, Nov 10, 2021 at 12:31:31AM +0530, Siddhesh Poyarekar wrote:
Recognize the __builtin_dynamic_object_size builtin and add paths in the
object size path to deal with it, but treat it like
__builtin_object_size for now. Also add tests to provide the same
testing coverage for the new builtin name.
gcc/ChangeLog:
* builtins.def (BUILT_IN_DYNAMIC_OBJECT_SIZE): New builtin.
* tree-object-size.h (compute_builtin_object_size): Add new
argument dynamic.
* builtins.c (expand_builtin, fold_builtin_2): Handle it.
(fold_builtin_object_size): Handle new builtin and adjust for
change to compute_builtin_object_size.
* tree-object-size.c: Include builtins.h.
(OST_DYNAMIC): New enum value.
(compute_builtin_object_size): Add new argument dynamic.
(addr_object_size): Adjust.
(early_object_sizes_execute_one,
dynamic_object_sizes_execute_one): New functions.
(object_sizes_execute): Rename insert_min_max_p argument to
early. Handle BUILT_IN_DYNAMIC_OBJECT_SIZE and call the new
Two spaces after . instead of just one.
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -972,6 +972,7 @@ DEF_BUILTIN_STUB (BUILT_IN_STRNCMP_EQ,
"__builtin_strncmp_eq")
/* Object size checking builtins. */
DEF_GCC_BUILTIN (BUILT_IN_OBJECT_SIZE, "object_size",
BT_FN_SIZE_CONST_PTR_INT, ATTR_CONST_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN (BUILT_IN_DYNAMIC_OBJECT_SIZE,
"dynamic_object_size", BT_FN_SIZE_CONST_PTR_INT, ATTR_NOTHROW_LEAF_LIST)
Are you sure about the omission of CONST_ in there?
If I do:
size_t a = __builtin_dynamic_object_size (x, 0);
size_t b = __builtin_dynamic_object_size (x, 0);
I'd expect the compiler to perform it just once. While it might actually do
it eventually after objsz2 pass lowers it, with the above it won't really do
it. Perhaps const attribute isn't really safe, the function might need to
read some memory in order to compute the return value, but certainly it will
not store to any memory, so perhaps
ATTR_PURE_NOTHROW_LEAF_LIST ?
Thanks, I'll fix this.
+#define DYNAMIC_OBJECT_SIZE
Why this extra macro?
+#define __builtin_object_size __builtin_dynamic_object_size
extern char ax[];
+#ifndef DYNAMIC_OBJECT_SIZE
You can #ifndef __builtin_object_size
instead...
I'll fix this too.
@@ -371,7 +373,8 @@ addr_object_size (struct object_size_info *osi, const_tree
ptr,
|| TREE_CODE (TREE_OPERAND (pt_var, 0)) != SSA_NAME)
{
compute_builtin_object_size (TREE_OPERAND (pt_var, 0),
- object_size_type & ~OST_SUBOBJECT, &sz);
+ object_size_type & OST_MINIMUM, &sz,
+ object_size_type & OST_DYNAMIC);
}
else
{
@@ -835,9 +838,10 @@ resolve_dependency_loops (struct object_size_info *osi)
bool
compute_builtin_object_size (tree ptr, int object_size_type,
- tree *psize)
+ tree *psize, bool dynamic)
{
gcc_assert (object_size_type >= 0 && object_size_type < OST_END);
+ object_size_type |= dynamic ? OST_DYNAMIC : 0;
What's the advantage of another argument and then merging it with
object_size_type over just passing object_size_type which will have
all the bits in?
I kept the size bits as an internal detail, I can define them in
tree-object-size.h and hae builtins.c (and others) use them.
+static void
+early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
+{
+ tree ost = gimple_call_arg (call, 1);
+ tree lhs = gimple_call_lhs (call);
+ gcc_assert (lhs != NULL_TREE);
+
+ if (tree_fits_uhwi_p (ost))
+ {
+ unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
+ tree ptr = gimple_call_arg (call, 0);
+ if ((object_size_type == 1 || object_size_type == 3)
+ && (TREE_CODE (ptr) == ADDR_EXPR || TREE_CODE (ptr) == SSA_NAME))
I think it would be better to have early exits there to avoid
indenting most of the function too much, because the function doesn't
do anything otherwise. So:
if (!tree_fits_uhwi_p (ost))
return;
OK, I'll fix this.
unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
tree ptr = gimple_call_arg (call, 0);
if (object_size_type != 1 && object_size_type != 3)
return;
if (TREE_CODE (ptr) != ADDR_EXPR && TREE_CODE (ptr) != SSA_NAME)
return;
tree type = ...
+ {
+ tree type = TREE_TYPE (lhs);
+ tree bytes;
+ if (compute_builtin_object_size (ptr, object_size_type, &bytes))
+ {
+ tree tem = make_ssa_name (type);
+ gimple_call_set_lhs (call, tem);
+ enum tree_code code
+ = object_size_type & OST_MINIMUM ? MAX_EXPR : MIN_EXPR;
+ tree cst = fold_convert (type, bytes);
+ gimple *g = gimple_build_assign (lhs, code, tem, cst);
+ gsi_insert_after (i, g, GSI_NEW_STMT);
+ update_stmt (call);
+ }
+ }
+ }
+/* Attempt to fold one __builtin_dynamic_object_size call in CALL into an
+ expression and insert it at I. Return true if it succeeds. */
+
+static bool
+dynamic_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
+{
+ unsigned numargs = gimple_call_num_args (call);
+ tree *args = XALLOCAVEC (tree, numargs);
+ args[0] = gimple_call_arg (call, 0);
+ args[1] = gimple_call_arg (call, 1);
Why it would have numargs different from 2? It is a builtin, and we reject
((__SIZE_TYPE__ (*) (const void *, int, int)) &__builtin_object_size) (x, 0, 3)
etc. with error that the builtin must be directly called.
And after all, you rely on it having at least 2 arguments anyway.
So, just tree args[2]; instead of XALLOCAVEC and if you want,
gcc_assert (numargs == 2);
?
Thanks, I'll fix this too.
Siddhesh
