On Wed, Dec 20, 2023 at 05:45:05PM -0500, Jason Merrill wrote:
> On 12/20/23 14:20, Jakub Jelinek wrote:
> > + if ((warn_sizeof_pointer_memaccess || alloc_size_attr)
> > && (complain & tf_warning)
> > && !vec_safe_is_empty (*args)
> > && !processing_template_decl)
> > {
> > - location_t sizeof_arg_loc[3];
> > - tree sizeof_arg[3];
> > + location_t sizeof_arg_loc[6];
> > + tree sizeof_arg[6];
>
> Why do we need to check 6 args for calloc? The patch is OK, just wondering.
The 3 for warn_sizeof_pointer_memaccess comes from the fact that we only
warn on certain builtins and don't really need it on 4th and later
arguments.
For the calloc case it warns about any 2 argument alloc_size attribute, so
generally it could be even INT_MAX or so; while for the C++ FE which
still has SIZEOF_EXPRs around it could be implementable if the function is
adjusted to be called with different arguments, for the C FE we need to
remember whether each argument was a sizeof or not because we fold
everything immediately.
I've grepped my /usr/include/ with what arguments alloc_size attribute is
used and found some 1,2 and 2,3 and 3,4 cases, so I went for 6 as a
compromise between not wasting too much compile time on it vs. covering
majority of functions with alloc_size 2 argument attributes.
It is unlikely allocation functions would need dozens of arguments...
Anyway, bootstrap fails with the patch due to:
../../gcc/gimple-fold.cc:7089:15: error: allocation of insufficient size ‘72’ for type ‘tree_node’ with size ‘216’ [-Werror=alloc-size]
../../gcc/gimple-fold.cc:7096:15: error: allocation of insufficient size ‘72’ for type ‘tree_node’ with size ‘216’ [-Werror=alloc-size]
This is on
/* Allocate SSA names(lhs1) on the stack. */
tree lhs1 = (tree)XALLOCA (tree_ssa_name);
memset (lhs1, 0, sizeof (tree_ssa_name));
TREE_SET_CODE (lhs1, SSA_NAME);
TREE_TYPE (lhs1) = type;
init_ssa_name_imm_use (lhs1);
where tree is a union and we don't allocate enough memory for the
whole union, just for one member of it.
Guess that is a case the warning is meant to warn about,
so we need some workaround. I find it really weird to allocate
constant 72 bytes using alloca...
Then there is also:
../../gcc/collect2.cc:625:39: error: ‘void* xcalloc(size_t, size_t)’ sizes specified with ‘sizeof’ in the earlier argument and not in the later argument [-Werror=calloc-transposed-args]
which is what the warning is meant to warn about.
So perhaps incremental:
2023-12-21 Jakub Jelinek <jakub@redhat.com>
* gimple-fold.cc (maybe_fold_comparisons_from_match_pd):
Use unsigned char buffers for lhs1 and lhs2 instead of allocating
them through XALLOCA.
* collect2.cc (maybe_run_lto_and_relink): Swap xcalloc arguments.
--- gcc/gimple-fold.cc.jj 2023-12-17 22:50:01.077589250 +0100
+++ gcc/gimple-fold.cc 2023-12-21 00:25:26.254807466 +0100
@@ -7086,14 +7086,16 @@ maybe_fold_comparisons_from_match_pd (tr
gimple_set_bb (stmt2, NULL);
/* Allocate SSA names(lhs1) on the stack. */
- tree lhs1 = (tree)XALLOCA (tree_ssa_name);
+ alignas (tree_node) unsigned char lhs1buf[sizeof (tree_ssa_name)];
+ tree lhs1 = (tree) &lhs1buf[0];
memset (lhs1, 0, sizeof (tree_ssa_name));
TREE_SET_CODE (lhs1, SSA_NAME);
TREE_TYPE (lhs1) = type;
init_ssa_name_imm_use (lhs1);
/* Allocate SSA names(lhs2) on the stack. */
- tree lhs2 = (tree)XALLOCA (tree_ssa_name);
+ alignas (tree_node) unsigned char lhs2buf[sizeof (tree_ssa_name)];
+ tree lhs2 = (tree) &lhs2buf[0];
memset (lhs2, 0, sizeof (tree_ssa_name));
TREE_SET_CODE (lhs2, SSA_NAME);
TREE_TYPE (lhs2) = type;
--- gcc/collect2.cc.jj 2023-11-10 22:48:01.356867230 +0100
+++ gcc/collect2.cc 2023-12-21 00:18:31.821159336 +0100
@@ -622,7 +622,7 @@ maybe_run_lto_and_relink (char **lto_ld_
LTO object replaced by all partitions and other LTO
objects removed. */
- lto_c_argv = (char **) xcalloc (sizeof (char *), num_lto_c_args);
+ lto_c_argv = (char **) xcalloc (num_lto_c_args, sizeof (char *));
lto_c_ptr = CONST_CAST2 (const char **, char **, lto_c_argv);
*lto_c_ptr++ = lto_wrapper;
Jakub
@@ -332,7 +332,7 @@ C ObjC C++ ObjC++ Var(warn_alloca) Warni
Warn on any use of alloca.
Walloc-size
-C ObjC Var(warn_alloc_size) Warning LangEnabledBy(C ObjC, Wextra)
+C ObjC C++ ObjC++ Var(warn_alloc_size) Warning LangEnabledBy(C ObjC C++ ObjC++, Wextra)
Warn when allocating insufficient storage for the target type of the assigned pointer.
Walloc-size-larger-than=
@@ -2048,6 +2048,25 @@ cp_genericize_r (tree *stmt_p, int *walk
case NOP_EXPR:
*stmt_p = predeclare_vla (*stmt_p);
+
+ /* Warn of new allocations that are not big enough for the target
+ type. */
+ if (warn_alloc_size
+ && TREE_CODE (TREE_OPERAND (stmt, 0)) == CALL_EXPR
+ && POINTER_TYPE_P (TREE_TYPE (stmt)))
+ {
+ if (tree fndecl = get_callee_fndecl (TREE_OPERAND (stmt, 0)))
+ if (DECL_IS_MALLOC (fndecl))
+ {
+ tree attrs = TYPE_ATTRIBUTES (TREE_TYPE (fndecl));
+ tree alloc_size = lookup_attribute ("alloc_size", attrs);
+ if (alloc_size)
+ warn_for_alloc_size (EXPR_LOCATION (stmt),
+ TREE_TYPE (TREE_TYPE (stmt)),
+ TREE_OPERAND (stmt, 0), alloc_size);
+ }
+ }
+
if (!wtd->no_sanitize_p
&& sanitize_flags_p (SANITIZE_NULL | SANITIZE_ALIGNMENT)
&& TYPE_REF_P (TREE_TYPE (stmt)))
@@ -2939,15 +2939,24 @@ finish_call_expr (tree fn, vec<tree, va_
if (!result)
{
- if (warn_sizeof_pointer_memaccess
+ tree alloc_size_attr = NULL_TREE;
+ if (warn_calloc_transposed_args
+ && TREE_CODE (fn) == FUNCTION_DECL
+ && (alloc_size_attr
+ = lookup_attribute ("alloc_size",
+ TYPE_ATTRIBUTES (TREE_TYPE (fn)))))
+ if (TREE_VALUE (alloc_size_attr) == NULL_TREE
+ || TREE_CHAIN (TREE_VALUE (alloc_size_attr)) == NULL_TREE)
+ alloc_size_attr = NULL_TREE;
+ if ((warn_sizeof_pointer_memaccess || alloc_size_attr)
&& (complain & tf_warning)
&& !vec_safe_is_empty (*args)
&& !processing_template_decl)
{
- location_t sizeof_arg_loc[3];
- tree sizeof_arg[3];
+ location_t sizeof_arg_loc[6];
+ tree sizeof_arg[6];
unsigned int i;
- for (i = 0; i < 3; i++)
+ for (i = 0; i < (alloc_size_attr ? 6 : 3); i++)
{
tree t;
@@ -2964,9 +2973,15 @@ finish_call_expr (tree fn, vec<tree, va_
sizeof_arg[i] = TREE_OPERAND (t, 0);
sizeof_arg_loc[i] = EXPR_LOCATION (t);
}
- sizeof_pointer_memaccess_warning
- (sizeof_arg_loc, fn, *args,
- sizeof_arg, same_type_ignoring_top_level_qualifiers_p);
+ if (warn_sizeof_pointer_memaccess)
+ {
+ auto same_p = same_type_ignoring_top_level_qualifiers_p;
+ sizeof_pointer_memaccess_warning (sizeof_arg_loc, fn, *args,
+ sizeof_arg, same_p);
+ }
+ if (alloc_size_attr)
+ warn_for_calloc (sizeof_arg_loc, fn, *args, sizeof_arg,
+ alloc_size_attr);
}
if ((complain & tf_warning)
@@ -0,0 +1,54 @@
+// { dg-do compile }
+// { dg-options "-Wcalloc-transposed-args" }
+
+typedef __SIZE_TYPE__ size_t;
+extern "C" void free (void *);
+extern "C" void *calloc (size_t, size_t);
+void *myfree (void *, int, int);
+void *mycalloc (int, int, size_t, size_t) __attribute__((malloc, malloc (myfree), alloc_size (3, 4)));
+
+void
+foo (int n)
+{
+ void *p;
+ p = __builtin_calloc (1, sizeof (int));
+ __builtin_free (p);
+ p = __builtin_calloc (n, sizeof (int));
+ __builtin_free (p);
+ p = __builtin_calloc (sizeof (int), 1); // { dg-warning "'\[^']*__builtin_calloc\[^']*' sizes specified with 'sizeof' in the earlier argument and not in the later argument" }
+ __builtin_free (p); // { dg-message "earlier argument should specify number of elements, later size of each element" "" { target *-*-* } .-1 }
+ p = __builtin_calloc (sizeof (int), n); // { dg-warning "'\[^']*__builtin_calloc\[^']*' sizes specified with 'sizeof' in the earlier argument and not in the later argument" }
+ __builtin_free (p); // { dg-message "earlier argument should specify number of elements, later size of each element" "" { target *-*-* } .-1 }
+ p = __builtin_calloc ((sizeof (int)), 1); // { dg-warning "'\[^']*__builtin_calloc\[^']*' sizes specified with 'sizeof' in the earlier argument and not in the later argument" }
+ __builtin_free (p); // { dg-message "earlier argument should specify number of elements, later size of each element" "" { target *-*-* } .-1 }
+ p = __builtin_calloc (sizeof (int) + 0, 1);
+ __builtin_free (p);
+ p = __builtin_calloc (sizeof (int), sizeof (char));
+ __builtin_free (p);
+ p = __builtin_calloc (1 * sizeof (int), 1);
+ __builtin_free (p);
+ p = calloc (1, sizeof (int));
+ free (p);
+ p = calloc (n, sizeof (int));
+ free (p);
+ p = calloc (sizeof (int), 1); // { dg-warning "'\[^']*calloc\[^']*' sizes specified with 'sizeof' in the earlier argument and not in the later argument" }
+ free (p); // { dg-message "earlier argument should specify number of elements, later size of each element" "" { target *-*-* } .-1 }
+ p = calloc (sizeof (int), n); // { dg-warning "'\[^']*calloc\[^']*' sizes specified with 'sizeof' in the earlier argument and not in the later argument" }
+ free (p); // { dg-message "earlier argument should specify number of elements, later size of each element" "" { target *-*-* } .-1 }
+ p = calloc (sizeof (int), sizeof (char));
+ free (p);
+ p = calloc (1 * sizeof (int), 1);
+ free (p);
+ p = mycalloc (42, 42, 1, sizeof (int));
+ myfree (p, 42, 42);
+ p = mycalloc (42, 42, n, sizeof (int));
+ myfree (p, 42, 42);
+ p = mycalloc (42, 42, sizeof (int), 1); // { dg-warning "'\[^']*mycalloc\[^']*' sizes specified with 'sizeof' in the earlier argument and not in the later argument" }
+ myfree (p, 42, 42); // { dg-message "earlier argument should specify number of elements, later size of each element" "" { target *-*-* } .-1 }
+ p = mycalloc (42, 42, sizeof (int), n); // { dg-warning "'\[^']*mycalloc\[^']*' sizes specified with 'sizeof' in the earlier argument and not in the later argument" }
+ myfree (p, 42, 42); // { dg-message "earlier argument should specify number of elements, later size of each element" "" { target *-*-* } .-1 }
+ p = mycalloc (42, 42, sizeof (int), sizeof (char));
+ myfree (p, 42, 42);
+ p = mycalloc (42, 42, 1 * sizeof (int), 1);
+ myfree (p, 42, 42);
+}
@@ -0,0 +1,52 @@
+// Tests the warnings for insufficient allocation size.
+// { dg-do compile }
+// { dg-options "-Walloc-size -Wno-calloc-transposed-args" }
+
+struct S { int x[10]; };
+void bar (S *);
+typedef __SIZE_TYPE__ size_t;
+void *myfree (void *, int, int);
+void *mymalloc (int, int, size_t) __attribute__((malloc, malloc (myfree), alloc_size (3)));
+void *mycalloc (int, int, size_t, size_t) __attribute__((malloc, malloc (myfree), alloc_size (3, 4)));
+
+void
+foo (void)
+{
+ S *p = (S *) __builtin_malloc (sizeof *p);
+ __builtin_free (p);
+ p = (S *) __builtin_malloc (sizeof p); // { dg-warning "allocation of insufficient size" }
+ __builtin_free (p);
+ p = (S *) __builtin_alloca (sizeof p); // { dg-warning "allocation of insufficient size" }
+ bar (p);
+ p = (S *) __builtin_calloc (1, sizeof p); // { dg-warning "allocation of insufficient size" }
+ __builtin_free (p);
+ bar ((S *) __builtin_malloc (4)); // { dg-warning "allocation of insufficient size" }
+ __builtin_free (p);
+ p = (S *) __builtin_calloc (sizeof *p, 1);
+ __builtin_free (p);
+}
+
+void
+baz (void)
+{
+ S *p = (S *) mymalloc (42, 42, sizeof *p);
+ myfree (p, 42, 42);
+ p = (S *) mymalloc (42, 42, sizeof p); // { dg-warning "allocation of insufficient size" }
+ myfree (p, 42, 42);
+ p = (S *) mycalloc (42, 42, 1, sizeof p); // { dg-warning "allocation of insufficient size" }
+ myfree (p, 42, 42);
+ bar ((S *) mymalloc (42, 42, 4)); // { dg-warning "allocation of insufficient size" }
+ myfree (p, 42, 42);
+ p = (S *) mycalloc (42, 42, sizeof *p, 1);
+ myfree (p, 42, 42);
+ p = static_cast <S *> (mycalloc (42, 42, sizeof *p, 1));
+ myfree (p, 42, 42);
+ p = static_cast <S *> (mymalloc (42, 42, sizeof *p));
+ myfree (p, 42, 42);
+ p = static_cast <S *> (mymalloc (42, 42, sizeof p)); // { dg-warning "allocation of insufficient size" }
+ myfree (p, 42, 42);
+ p = static_cast <S *> (mycalloc (42, 42, 1, sizeof p)); // { dg-warning "allocation of insufficient size" }
+ myfree (p, 42, 42);
+ bar (static_cast <S *> (mymalloc (42, 42, 4))); // { dg-warning "allocation of insufficient size" }
+ myfree (p, 42, 42);
+}