diff mbox series

[v6] C, ObjC: Add -Wunterminated-string-initialization

Message ID 20240305202039.21125-1-alx@kernel.org
State New
Headers show
Series [v6] C, ObjC: Add -Wunterminated-string-initialization | expand

Commit Message

Alejandro Colomar March 5, 2024, 8:20 p.m. UTC 
Warn about the following:

    char  s[3] = "foo";

Initializing a char array with a string literal of the same length as
the size of the array is usually a mistake.  Rarely is the case where
one wants to create a non-terminated character sequence from a string
literal.

In some cases, for writing faster code, one may want to use arrays
instead of pointers, since that removes the need for storing an array of
pointers apart from the strings themselves.

    char  *log_levels[]   = { "info", "warning", "err" };
vs.
    char  log_levels[][7] = { "info", "warning", "err" };

This forces the programmer to specify a size, which might change if a
new entry is later added.  Having no way to enforce null termination is
very dangerous, however, so it is useful to have a warning for this, so
that the compiler can make sure that the programmer didn't make any
mistakes.  This warning catches the bug above, so that the programmer
will be able to fix it and write:

    char  log_levels[][8] = { "info", "warning", "err" };

This warning already existed as part of -Wc++-compat, but this patch
allows enabling it separately.  It is also included in -Wextra, since
it may not always be desired (when unterminated character sequences are
wanted), but it's likely to be desired in most cases.

Since Wc++-compat now includes this warning, the test has to be modified
to expect the text of the new warning too, in <gcc.dg/Wcxx-compat-14.c>.

Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00059.html>
Link: <https://lists.gnu.org/archive/html/groff/2022-11/msg00063.html>
Link: <https://inbox.sourceware.org/gcc/36da94eb-1cac-5ae8-7fea-ec66160cf413@gmail.com/T/>
Acked-by: Doug McIlroy <douglas.mcilroy@dartmouth.edu>
Cc: "G. Branden Robinson" <g.branden.robinson@gmail.com>
Cc: Ralph Corderoy <ralph@inputplus.co.uk>
Cc: Dave Kemper <saint.snit@gmail.com>
Cc: Larry McVoy <lm@mcvoy.com>
Cc: Andrew Pinski <pinskia@gmail.com>
Cc: Jonathan Wakely <jwakely.gcc@gmail.com>
Cc: Andrew Clayton <andrew@digital-domain.net>
Cc: Martin Uecker <muecker@gwdg.de>
Cc: David Malcolm <dmalcolm@redhat.com>
Cc: Mike Stump <mikestump@comcast.net>
Cc: Joseph Myers <josmyers@redhat.com>
Cc: Sandra Loosemore <sloosemore@baylibre.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

Hi!

v6:
-  Small wording fix in c.opt
-  Document the option in invoke.texi

I tried again, but didn't find much alphabetic order  in there, so put
it where Mike suggested, after -Warray-bounds=n.

Have a lovely night!
Alex


Range-diff against v5:
1:  d98d1fec176 ! 1:  e8fd975bde7 C, ObjC: Add -Wunterminated-string-initialization
    @@ gcc/c-family/c.opt: Wunsuffixed-float-constants
      
     +Wunterminated-string-initialization
     +C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C ObjC,Wextra || Wc++-compat)
    -+Warn about character arrays initialized as unterminated character sequences by a string literal.
    ++Warn about character arrays initialized as unterminated character sequences with a string literal.
     +
      Wunused
      C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
    @@ gcc/c/c-typeck.cc: digest_init (location_t init_loc, tree type, tree init, tree
      		{
      		  unsigned HOST_WIDE_INT size
     
    + ## gcc/doc/invoke.texi ##
    +@@ gcc/doc/invoke.texi: Objective-C and Objective-C++ Dialects}.
    + -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs
    + -Wtrivial-auto-var-init -Wtsan -Wtype-limits  -Wundef
    + -Wuninitialized  -Wunknown-pragmas
    +--Wunsuffixed-float-constants  -Wunused
    ++-Wunsuffixed-float-constants
    ++-Wunterminated-string-initialization
    ++-Wunused
    + -Wunused-but-set-parameter  -Wunused-but-set-variable
    + -Wunused-const-variable  -Wunused-const-variable=@var{n}
    + -Wunused-function  -Wunused-label  -Wunused-local-typedefs
    +@@ gcc/doc/invoke.texi: name is still supported, but the newer name is more descriptive.)
    + -Wredundant-move @r{(only for C++)}
    + -Wtype-limits
    + -Wuninitialized
    ++-Wunterminated-string-initialization
    + -Wshift-negative-value @r{(in C++11 to C++17 and in C99 and newer)}
    + -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}
    + -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}}
    +@@ gcc/doc/invoke.texi: arithmetic that may yield out of bounds values. This warning level may
    + give a larger number of false positives and is deactivated by default.
    + @end table
    + 
    ++@opindex Wunterminated-string-initialization
    ++@opindex Wno-unterminated-string-initialization
    ++@item -Wunterminated-string-initialization
    ++Warn about character arrays
    ++initialized as unterminated character sequences
    ++with a string literal.
    ++For example:
    ++
    ++@smallexample
    ++char arr[3] = "foo";
    ++@end smallexample
    ++
    ++@option{-Wunterminated-string-initialization} is enabled by @option{-Wextra}.
    ++
    + @opindex Warray-compare
    + @opindex Wno-array-compare
    + @item -Warray-compare
    +
      ## gcc/testsuite/gcc.dg/Wcxx-compat-14.c ##
     @@
      /* { dg-options "-Wc++-compat" } */

 gcc/c-family/c.opt                            |  4 ++++
 gcc/c/c-typeck.cc                             |  6 +++---
 gcc/doc/invoke.texi                           | 19 ++++++++++++++++++-
 gcc/testsuite/gcc.dg/Wcxx-compat-14.c         |  2 +-
 .../Wunterminated-string-initialization.c     |  6 ++++++
 5 files changed, 32 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c

Comments

Alejandro Colomar March 5, 2024, 8:25 p.m. UTC | #1 
On Tue, Mar 05, 2024 at 09:20:42PM +0100, Alejandro Colomar wrote:
> Hi!
> 
> v6:
> -  Small wording fix in c.opt
> -  Document the option in invoke.texi
> 
> I tried again, but didn't find much alphabetic order  in there, so put
> it where Mike suggested, after -Warray-bounds=n.
> 
> Have a lovely night!
> Alex
> 
[...]
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 146b40414b0..f81df4de934 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -410,7 +410,9 @@ Objective-C and Objective-C++ Dialects}.
>  -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs
>  -Wtrivial-auto-var-init -Wtsan -Wtype-limits  -Wundef
>  -Wuninitialized  -Wunknown-pragmas
> --Wunsuffixed-float-constants  -Wunused
> +-Wunsuffixed-float-constants
> +-Wunterminated-string-initialization
> +-Wunused
>  -Wunused-but-set-parameter  -Wunused-but-set-variable
>  -Wunused-const-variable  -Wunused-const-variable=@var{n}
>  -Wunused-function  -Wunused-label  -Wunused-local-typedefs
> @@ -6264,6 +6266,7 @@ name is still supported, but the newer name is more descriptive.)
>  -Wredundant-move @r{(only for C++)}
>  -Wtype-limits
>  -Wuninitialized
> +-Wunterminated-string-initialization
>  -Wshift-negative-value @r{(in C++11 to C++17 and in C99 and newer)}
>  -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}
>  -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}}
> @@ -8281,6 +8284,20 @@ arithmetic that may yield out of bounds values. This warning level may
>  give a larger number of false positives and is deactivated by default.
>  @end table
>  
> +@opindex Wunterminated-string-initialization
> +@opindex Wno-unterminated-string-initialization
> +@item -Wunterminated-string-initialization
> +Warn about character arrays
> +initialized as unterminated character sequences
> +with a string literal.
> +For example:
> +
> +@smallexample
> +char arr[3] = "foo";
> +@end smallexample
> +
> +@option{-Wunterminated-string-initialization} is enabled by @option{-Wextra}.

Oops, I should also mention -Wc++-compat here.
diff mbox series

Patch

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 44b9c862c14..3837021747b 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1407,6 +1407,10 @@  Wunsuffixed-float-constants
 C ObjC Var(warn_unsuffixed_float_constants) Warning
 Warn about unsuffixed float constants.
 
+Wunterminated-string-initialization
+C ObjC Var(warn_unterminated_string_initialization) Warning LangEnabledBy(C ObjC,Wextra || Wc++-compat)
+Warn about character arrays initialized as unterminated character sequences with a string literal.
+
 Wunused
 C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wall)
 ; documented in common.opt
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index e55e887da14..7df9de819ed 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8399,11 +8399,11 @@  digest_init (location_t init_loc, tree type, tree init, tree origtype,
 		pedwarn_init (init_loc, 0,
 			      ("initializer-string for array of %qT "
 			       "is too long"), typ1);
-	      else if (warn_cxx_compat
+	      else if (warn_unterminated_string_initialization
 		       && compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
-		warning_at (init_loc, OPT_Wc___compat,
+		warning_at (init_loc, OPT_Wunterminated_string_initialization,
 			    ("initializer-string for array of %qT "
-			     "is too long for C++"), typ1);
+			     "is too long"), typ1);
 	      if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
 		{
 		  unsigned HOST_WIDE_INT size
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 146b40414b0..f81df4de934 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -410,7 +410,9 @@  Objective-C and Objective-C++ Dialects}.
 -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs
 -Wtrivial-auto-var-init -Wtsan -Wtype-limits  -Wundef
 -Wuninitialized  -Wunknown-pragmas
--Wunsuffixed-float-constants  -Wunused
+-Wunsuffixed-float-constants
+-Wunterminated-string-initialization
+-Wunused
 -Wunused-but-set-parameter  -Wunused-but-set-variable
 -Wunused-const-variable  -Wunused-const-variable=@var{n}
 -Wunused-function  -Wunused-label  -Wunused-local-typedefs
@@ -6264,6 +6266,7 @@  name is still supported, but the newer name is more descriptive.)
 -Wredundant-move @r{(only for C++)}
 -Wtype-limits
 -Wuninitialized
+-Wunterminated-string-initialization
 -Wshift-negative-value @r{(in C++11 to C++17 and in C99 and newer)}
 -Wunused-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}
 -Wunused-but-set-parameter @r{(only with} @option{-Wunused} @r{or} @option{-Wall}@r{)}}
@@ -8281,6 +8284,20 @@  arithmetic that may yield out of bounds values. This warning level may
 give a larger number of false positives and is deactivated by default.
 @end table
 
+@opindex Wunterminated-string-initialization
+@opindex Wno-unterminated-string-initialization
+@item -Wunterminated-string-initialization
+Warn about character arrays
+initialized as unterminated character sequences
+with a string literal.
+For example:
+
+@smallexample
+char arr[3] = "foo";
+@end smallexample
+
+@option{-Wunterminated-string-initialization} is enabled by @option{-Wextra}.
+
 @opindex Warray-compare
 @opindex Wno-array-compare
 @item -Warray-compare
diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c b/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
index 23783711be6..6df0ee197cc 100644
--- a/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
+++ b/gcc/testsuite/gcc.dg/Wcxx-compat-14.c
@@ -2,5 +2,5 @@ 
 /* { dg-options "-Wc++-compat" } */
 
 char a1[] = "a";
-char a2[1] = "a";	/* { dg-warning "C\[+\]\[+\]" } */
+char a2[1] = "a";	/* { dg-warning "initializer-string for array of 'char' is too long" } */
 char a3[2] = "a";
diff --git a/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
new file mode 100644
index 00000000000..13d5dbc6640
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wunterminated-string-initialization" } */
+
+char a1[] = "a";
+char a2[1] = "a";	/* { dg-warning "initializer-string for array of 'char' is too long" } */
+char a3[2] = "a";