On 12/20/23 18:29, Jakub Jelinek wrote:
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:

LGTM.

Jason

Reply via email to