]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
c: Split -Wcalloc-transposed-args warning from -Walloc-size, -Walloc-size fixes
authorJakub Jelinek <jakub@redhat.com>
Wed, 20 Dec 2023 10:31:18 +0000 (11:31 +0100)
committerJakub Jelinek <jakub@redhat.com>
Wed, 20 Dec 2023 10:31:18 +0000 (11:31 +0100)
The following patch changes -Walloc-size warning to no longer warn
about int *p = calloc (1, sizeof (int));, because as discussed earlier,
the size is IMNSHO sufficient in that case, for alloc_size with 2
arguments warns if the product of the 2 arguments is insufficiently small.

Also, it warns also for explicit casts of malloc/calloc etc. calls
rather than just implicit, so not just
  int *p = malloc (1);
but also
  int *p = (int *) malloc (1);

It also fixes some ICEs where the code didn't verify the alloc_size
arguments properly (Walloc-size-5.c testcase ICEs with vanilla trunk).

And lastly, it introduces a coding style warning, -Wcalloc-transposed-args
to warn for calloc (sizeof (struct S), 1) and similar calls (regardless
of what they are cast to, warning whenever first argument is sizeof and
the second is not).

2023-12-20  Jakub Jelinek  <jakub@redhat.com>

gcc/
* doc/invoke.texi (-Walloc-size): Add to the list of
warning options, remove unnecessary line-break.
(-Wcalloc-transposed-args): Document new warning.
gcc/c-family/
* c.opt (Wcalloc-transposed-args): New warning.
* c-common.h (warn_for_calloc, warn_for_alloc_size): Declare.
* c-warn.cc (warn_for_calloc, warn_for_alloc_size): New functions.
gcc/c/
* c-parser.cc (c_parser_postfix_expression_after_primary): Grow
sizeof_arg and sizeof_arg_loc arrays to 6 elements.  Call
warn_for_calloc if warn_calloc_transposed_args for functions with
alloc_size type attribute with 2 arguments.
(c_parser_expr_list): Use 6 instead of 3.
* c-typeck.cc (build_c_cast): Call warn_for_alloc_size for casts
of calls to functions with alloc_size type attribute.
(convert_for_assignment): Likewise.
gcc/testsuite/
* gcc.dg/Walloc-size-4.c: New test.
* gcc.dg/Walloc-size-5.c: New test.
* gcc.dg/Wcalloc-transposed-args-1.c: New test.

gcc/c-family/c-common.h
gcc/c-family/c-warn.cc
gcc/c-family/c.opt
gcc/c/c-parser.cc
gcc/c/c-typeck.cc
gcc/doc/invoke.texi
gcc/testsuite/gcc.dg/Walloc-size-4.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/Walloc-size-5.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/Wcalloc-transposed-args-1.c [new file with mode: 0644]

index b8bd56c1a4dff146f5f0015ddae90bf5ddc11189..925e7afc910e0a00ea5a676879b86d92fa7056f2 100644 (file)
@@ -1599,6 +1599,9 @@ extern void warn_about_parentheses (location_t,
 extern void warn_for_unused_label (tree label);
 extern void warn_for_div_by_zero (location_t, tree divisor);
 extern void warn_for_memset (location_t, tree, tree, int);
+extern void warn_for_calloc (location_t *, tree, vec<tree, va_gc> *,
+                            tree *, tree);
+extern void warn_for_alloc_size (location_t, tree, tree, tree);
 extern void warn_for_sign_compare (location_t,
                                   tree orig_op0, tree orig_op1,
                                   tree op0, tree op1,
index abe66dd3030e361cc53fb09694b5f66fe4177d8e..69621bfa181b18f8413aeaefc542af12706a5165 100644 (file)
@@ -2263,6 +2263,76 @@ warn_for_memset (location_t loc, tree arg0, tree arg2,
     }
 }
 
+/* Warn for calloc (sizeof (X), n).  */
+
+void
+warn_for_calloc (location_t *sizeof_arg_loc, tree callee,
+                vec<tree, va_gc> *params, tree *sizeof_arg, tree attr)
+{
+  if (!TREE_VALUE (attr) || !TREE_CHAIN (TREE_VALUE (attr)))
+    return;
+
+  int arg1 = TREE_INT_CST_LOW (TREE_VALUE (TREE_VALUE (attr))) - 1;
+  int arg2
+    = TREE_INT_CST_LOW (TREE_VALUE (TREE_CHAIN (TREE_VALUE (attr)))) - 1;
+  if (arg1 < 0
+      || (unsigned) arg1 >= vec_safe_length (params)
+      || arg1 >= 6
+      || arg2 < 0
+      || (unsigned) arg2 >= vec_safe_length (params)
+      || arg2 >= 6
+      || arg1 >= arg2)
+    return;
+
+  if (sizeof_arg[arg1] == NULL_TREE || sizeof_arg[arg2] != NULL_TREE)
+    return;
+
+  if (warning_at (sizeof_arg_loc[arg1], OPT_Wcalloc_transposed_args,
+                 "%qD sizes specified with %<sizeof%> in the earlier "
+                 "argument and not in the later argument", callee))
+    inform (sizeof_arg_loc[arg1], "earlier argument should specify number "
+           "of elements, later size of each element");
+}
+
+/* Warn for allocator calls where the constant allocated size is smaller
+   than sizeof (TYPE).  */
+
+void
+warn_for_alloc_size (location_t loc, tree type, tree call, tree alloc_size)
+{
+  if (!TREE_VALUE (alloc_size))
+    return;
+
+  tree arg1 = TREE_VALUE (TREE_VALUE (alloc_size));
+  int idx1 = TREE_INT_CST_LOW (arg1) - 1;
+  if (idx1 < 0 || idx1 >= call_expr_nargs (call))
+    return;
+  arg1 = CALL_EXPR_ARG (call, idx1);
+  if (TREE_CODE (arg1) != INTEGER_CST)
+    return;
+  if (TREE_CHAIN (TREE_VALUE (alloc_size)))
+    {
+      tree arg2 = TREE_VALUE (TREE_CHAIN (TREE_VALUE (alloc_size)));
+      int idx2 = TREE_INT_CST_LOW (arg2) - 1;
+      if (idx2 < 0 || idx2 >= call_expr_nargs (call))
+       return;
+      arg2 = CALL_EXPR_ARG (call, idx2);
+      if (TREE_CODE (arg2) != INTEGER_CST)
+       return;
+      arg1 = int_const_binop (MULT_EXPR, fold_convert (sizetype, arg1),
+                             fold_convert (sizetype, arg2));
+      if (TREE_CODE (arg1) != INTEGER_CST)
+       return;
+    }
+  if (!VOID_TYPE_P (type)
+      && TYPE_SIZE_UNIT (type)
+      && TREE_CODE (TYPE_SIZE_UNIT (type)) == INTEGER_CST
+      && tree_int_cst_lt (arg1, TYPE_SIZE_UNIT (type)))
+    warning_at (loc, OPT_Walloc_size,
+               "allocation of insufficient size %qE for type %qT with "
+               "size %qE", arg1, type, TYPE_SIZE_UNIT (type));
+}
+
 /* Subroutine of build_binary_op. Give warnings for comparisons
    between signed and unsigned quantities that may fail. Do the
    checking based on the original operand trees ORIG_OP0 and ORIG_OP1,
index 03b64d536fa3d4e797313c0b6f67e26f1814bfe4..3faab7a891861d564480327f221228cf45842f9f 100644 (file)
@@ -502,6 +502,10 @@ Wc++26-extensions
 C++ ObjC++ Var(warn_cxx26_extensions) Warning Init(1)
 Warn about C++26 constructs in code compiled with an older standard.
 
+Wcalloc-transposed-args
+C ObjC C++ ObjC++ Var(warn_calloc_transposed_args) Warning LangEnabledBy(C ObjC C++ ObjC++,Wextra)
+Warn about suspicious calls to calloc-like functions where sizeof expression is the earlier size argument and not the latter.
+
 Wcast-function-type
 C ObjC C++ ObjC++ Var(warn_cast_function_type) Warning EnabledBy(Wextra)
 Warn about casts between incompatible function types.
index 4f1e0662c72b5a0477da8f4bc4c9581ee930768b..c3724304580cf54f52655e10d2697c68966b9a17 100644 (file)
@@ -12478,8 +12478,8 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
 {
   struct c_expr orig_expr;
   tree ident, idx;
-  location_t sizeof_arg_loc[3], comp_loc;
-  tree sizeof_arg[3];
+  location_t sizeof_arg_loc[6], comp_loc;
+  tree sizeof_arg[6];
   unsigned int literal_zero_mask;
   unsigned int i;
   vec<tree, va_gc> *exprlist;
@@ -12512,7 +12512,7 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
          {
            matching_parens parens;
            parens.consume_open (parser);
-           for (i = 0; i < 3; i++)
+           for (i = 0; i < 6; i++)
              {
                sizeof_arg[i] = NULL_TREE;
                sizeof_arg_loc[i] = UNKNOWN_LOCATION;
@@ -12577,6 +12577,13 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
                                      "not permitted in intervening code");
                  parser->omp_for_parse_state->fail = true;
                }
+             if (warn_calloc_transposed_args)
+               if (tree attr = lookup_attribute ("alloc_size",
+                                                 TYPE_ATTRIBUTES
+                                                   (TREE_TYPE (expr.value))))
+                 if (TREE_VALUE (attr) && TREE_CHAIN (TREE_VALUE (attr)))
+                   warn_for_calloc (sizeof_arg_loc, expr.value, exprlist,
+                                    sizeof_arg, attr);
            }
 
          start = expr.get_start ();
@@ -12861,7 +12868,7 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
        vec_safe_push (orig_types, expr.original_type);
       if (locations)
        locations->safe_push (expr.get_location ());
-      if (++idx < 3
+      if (++idx < 6
          && sizeof_arg != NULL
          && (expr.original_code == SIZEOF_EXPR
              || expr.original_code == PAREN_SIZEOF_EXPR))
index 022e3c6386ba4e31c6dce6db95ef0adcbc4e7882..a2164d9d2263a0a58cfccd891f8b068e3c5ece00 100644 (file)
@@ -6054,6 +6054,19 @@ build_c_cast (location_t loc, tree type, tree expr)
                            c_addr_space_name (as_to),
                            c_addr_space_name (as_from));
            }
+
+         /* Warn of new allocations that are not big enough for the target
+            type.  */
+         if (warn_alloc_size && TREE_CODE (value) == CALL_EXPR)
+           if (tree fndecl = get_callee_fndecl (value))
+             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 (loc, TREE_TYPE (type), value,
+                                        alloc_size);
+               }
        }
 
       /* Warn about possible alignment problems.  */
@@ -7277,32 +7290,15 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
 
       /* Warn of new allocations that are not big enough for the target
         type.  */
-      tree fndecl;
-      if (warn_alloc_size
-         && TREE_CODE (rhs) == CALL_EXPR
-         && (fndecl = get_callee_fndecl (rhs)) != NULL_TREE
-         && DECL_IS_MALLOC (fndecl))
-       {
-         tree fntype = TREE_TYPE (fndecl);
-         tree fntypeattrs = TYPE_ATTRIBUTES (fntype);
-         tree alloc_size = lookup_attribute ("alloc_size", fntypeattrs);
-         if (alloc_size)
+      if (warn_alloc_size && TREE_CODE (rhs) == CALL_EXPR)
+       if (tree fndecl = get_callee_fndecl (rhs))
+         if (DECL_IS_MALLOC (fndecl))
            {
-             tree args = TREE_VALUE (alloc_size);
-             int idx = TREE_INT_CST_LOW (TREE_VALUE (args)) - 1;
-             /* For calloc only use the second argument.  */
-             if (TREE_CHAIN (args))
-               idx = TREE_INT_CST_LOW (TREE_VALUE (TREE_CHAIN (args))) - 1;
-             tree arg = CALL_EXPR_ARG (rhs, idx);
-             if (TREE_CODE (arg) == INTEGER_CST
-                 && !VOID_TYPE_P (ttl) && TYPE_SIZE_UNIT (ttl)
-                 && INTEGER_CST == TREE_CODE (TYPE_SIZE_UNIT (ttl))
-                 && tree_int_cst_lt (arg, TYPE_SIZE_UNIT (ttl)))
-                warning_at (location, OPT_Walloc_size, "allocation of "
-                            "insufficient size %qE for type %qT with "
-                            "size %qE", arg, ttl, TYPE_SIZE_UNIT (ttl));
+             tree attrs = TYPE_ATTRIBUTES (TREE_TYPE (fndecl));
+             tree alloc_size = lookup_attribute ("alloc_size", attrs);
+             if (alloc_size)
+               warn_for_alloc_size (location, ttl, rhs, alloc_size);
            }
-       }
 
       /* See if the pointers point to incompatible address spaces.  */
       asl = TYPE_ADDR_SPACE (ttl);
index 940adbce1bd11ca4667e4c9429a3747692ff752a..5af978b0a670920cca9fad6ef4d8468e20bdc3b2 100644 (file)
@@ -328,7 +328,7 @@ Objective-C and Objective-C++ Dialects}.
 -pedantic-errors -fpermissive
 -w  -Wextra  -Wall  -Wabi=@var{n}
 -Waddress  -Wno-address-of-packed-member  -Waggregate-return
--Walloc-size-larger-than=@var{byte-size}  -Walloc-zero
+-Walloc-size  -Walloc-size-larger-than=@var{byte-size}  -Walloc-zero
 -Walloca  -Walloca-larger-than=@var{byte-size}
 -Wno-aggressive-loop-optimizations
 -Warith-conversion
@@ -344,6 +344,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wc++20-compat
 -Wno-c++11-extensions  -Wno-c++14-extensions -Wno-c++17-extensions
 -Wno-c++20-extensions  -Wno-c++23-extensions
+-Wcalloc-transposed-args
 -Wcast-align  -Wcast-align=strict  -Wcast-function-type  -Wcast-qual
 -Wchar-subscripts
 -Wclobbered  -Wcomment
@@ -8260,8 +8261,7 @@ Warn about calls to allocation functions decorated with attribute
 @code{alloc_size} that specify insufficient size for the target type of
 the pointer the result is assigned to, including those to the built-in
 forms of the functions @code{aligned_alloc}, @code{alloca},
-@code{calloc},
-@code{malloc}, and @code{realloc}.
+@code{calloc}, @code{malloc}, and @code{realloc}.
 
 @opindex Wno-alloc-zero
 @opindex Walloc-zero
@@ -8274,6 +8274,21 @@ when called with a zero size differs among implementations (and in the case
 of @code{realloc} has been deprecated) relying on it may result in subtle
 portability bugs and should be avoided.
 
+@opindex Wcalloc-transposed-args
+@opindex Wno-calloc-transposed-args
+@item -Wcalloc-transposed-args
+Warn about calls to allocation functions decorated with attribute
+@code{alloc_size} with two arguments, which use @code{sizeof} operator
+as the earlier size argument and don't use it as the later size argument.
+This is a coding style warning.  The first argument to @code{calloc} is
+documented to be number of elements in array, while the second argument
+is size of each element, so @code{calloc (@var{n}, sizeof (int))} is preferred
+over @code{calloc (sizeof (int), @var{n})}.  If @code{sizeof} in the earlier
+argument and not the latter is intentional, the warning can be suppressed
+by using @code{calloc (sizeof (struct @var{S}) + 0, n)} or
+@code{calloc (1 * sizeof (struct @var{S}), 4)} or using @code{sizeof} in the
+later argument as well.
+
 @opindex Walloc-size-larger-than=
 @opindex Wno-alloc-size-larger-than
 @item -Walloc-size-larger-than=@var{byte-size}
diff --git a/gcc/testsuite/gcc.dg/Walloc-size-4.c b/gcc/testsuite/gcc.dg/Walloc-size-4.c
new file mode 100644 (file)
index 0000000..e069d12
--- /dev/null
@@ -0,0 +1,54 @@
+/* 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 (struct 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)
+{
+  struct S *p = (struct S *) __builtin_malloc (sizeof *p);
+  __builtin_free (p);
+  p = (struct S *) __builtin_malloc (sizeof p);                /* { dg-warning "allocation of insufficient size" } */
+  __builtin_free (p);
+  p = (struct S *) __builtin_alloca (sizeof p);                /* { dg-warning "allocation of insufficient size" } */
+  bar (p);
+  p = (struct S *) __builtin_calloc (1, sizeof p);     /* { dg-warning "allocation of insufficient size" } */
+  __builtin_free (p);
+  bar ((struct S *) __builtin_malloc (4));             /* { dg-warning "allocation of insufficient size" } */
+  __builtin_free (p);
+  p = (struct S *) __builtin_calloc (sizeof *p, 1);
+  __builtin_free (p);
+  p = __builtin_calloc (sizeof *p, 1);
+  __builtin_free (p);
+}
+
+void
+baz (void)
+{
+  struct S *p = (struct S *) mymalloc (42, 42, sizeof *p);
+  myfree (p, 42, 42);
+  p = (struct S *) mymalloc (42, 42, sizeof p);                /* { dg-warning "allocation of insufficient size" } */
+  myfree (p, 42, 42);
+  p = (struct S *) mycalloc (42, 42, 1, sizeof p);     /* { dg-warning "allocation of insufficient size" } */
+  myfree (p, 42, 42);
+  bar ((struct S *) mymalloc (42, 42, 4));             /* { dg-warning "allocation of insufficient size" } */
+  myfree (p, 42, 42);
+  p = (struct S *) mycalloc (42, 42, sizeof *p, 1);
+  myfree (p, 42, 42);
+  p = mycalloc (42, 42, sizeof *p, 1);
+  myfree (p, 42, 42);
+  p = mymalloc (42, 42, sizeof *p);
+  myfree (p, 42, 42);
+  p = mymalloc (42, 42, sizeof p);                     /* { dg-warning "allocation of insufficient size" } */
+  myfree (p, 42, 42);
+  p = mycalloc (42, 42, 1, sizeof p);                  /* { dg-warning "allocation of insufficient size" } */
+  myfree (p, 42, 42);
+  bar (mymalloc (42, 42, 4));                          /* { dg-warning "allocation of insufficient size" } */
+  myfree (p, 42, 42);
+}
diff --git a/gcc/testsuite/gcc.dg/Walloc-size-5.c b/gcc/testsuite/gcc.dg/Walloc-size-5.c
new file mode 100644 (file)
index 0000000..b675cdf
--- /dev/null
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Walloc-size -std=gnu11" } */
+
+struct S { int x[10]; };
+void myfree ();
+void *mymalloc () __attribute__((malloc, alloc_size (16)));
+void *mycalloc () __attribute__((malloc, alloc_size (16, 17)));
+
+void
+foo (void)
+{
+  struct S *p = mymalloc (1);
+  myfree (p);
+  p = mycalloc (1, 1);
+  myfree (p);
+  p = (struct S *) mymalloc (1);
+  myfree (p);
+  p = (struct S *) mycalloc (1, 1);
+  myfree (p);
+}
diff --git a/gcc/testsuite/gcc.dg/Wcalloc-transposed-args-1.c b/gcc/testsuite/gcc.dg/Wcalloc-transposed-args-1.c
new file mode 100644 (file)
index 0000000..ed14116
--- /dev/null
@@ -0,0 +1,54 @@
+/* { dg-do compile } */
+/* { dg-options "-Wcalloc-transposed-args" } */
+
+typedef __SIZE_TYPE__ size_t;
+void free (void *);
+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);
+}