public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Alejandro Colomar <alx.manpages@gmail.com>
To: gcc-patches@gcc.gnu.org
Cc: Alejandro Colomar <alx@kernel.org>,
	Doug McIlroy <douglas.mcilroy@dartmouth.edu>,
	"G. Branden Robinson" <g.branden.robinson@gmail.com>,
	Ralph Corderoy <ralph@inputplus.co.uk>,
	Dave Kemper <saint.snit@gmail.com>, Larry McVoy <lm@mcvoy.com>,
	Andrew Pinski <pinskia@gmail.com>,
	Jonathan Wakely <jwakely.gcc@gmail.com>,
	Andrew Clayton <andrew@digital-domain.net>,
	Martin Uecker <muecker@gwdg.de>
Subject: [PATCH v2] C, ObjC: Add -Wunterminated-string-initialization
Date: Fri, 24 Mar 2023 14:39:28 +0100	[thread overview]
Message-ID: <20230324133928.14753-1-alx@kernel.org> (raw)

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.

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>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

Hi,

I sent v1 to the wrong list.  This time I've made sure to write to
gcc-patches@.

v2 adds some draft of a test, as suggested by Martin.  However, I don't
know yet how to write those, so the test is just a draft.  But I did
test the feature, by compiling GCC and compiling some small program with
it.

Cheers,
Alex


Range-diff against v1:
1:  61ddf1eb816 ! 1:  e40d8f54942 C, ObjC: Add -Wunterminated-string-initialization
    @@ Commit message
         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>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
      ## gcc/c-family/c.opt ##
    @@ gcc/c/c-typeck.cc: digest_init (location_t init_loc, tree type, tree init, tree
      	      if (compare_tree_int (TYPE_SIZE_UNIT (type), len) < 0)
      		{
      		  unsigned HOST_WIDE_INT size
    +
    + ## gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c (new) ##
    +@@
    ++/* { dg-do compile } */
    ++/* { dg-options "-Wunterminated-string-initialization" } */
    ++
    ++char a1[] = "a";
    ++char a2[1] = "a";	/* { dg-warning "unterminated char sequence" } */
    ++char a3[2] = "a";

 gcc/c-family/c.opt                                         | 4 ++++
 gcc/c/c-typeck.cc                                          | 6 +++---
 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c | 6 ++++++
 3 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..7f1fccfe02b 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1382,6 +1382,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 by 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 45bacc06c47..ce2750f98bb 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -8420,11 +8420,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/testsuite/gcc.dg/Wunterminated-string-initialization.c b/gcc/testsuite/gcc.dg/Wunterminated-string-initialization.c
new file mode 100644
index 00000000000..c6517702d51
--- /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 "unterminated char sequence" } */
+char a3[2] = "a";
-- 
2.39.2


             reply	other threads:[~2023-03-24 14:01 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-24 13:39 Alejandro Colomar [this message]
2023-03-24 14:53 ` David Malcolm
2023-03-24 17:45   ` Alejandro Colomar
2023-03-24 17:58     ` David Malcolm
2023-04-20 17:17       ` Ping: " Alejandro Colomar
2023-10-01  0:55   ` Alejandro Colomar
2023-10-01  7:37     ` Martin Uecker
2023-10-01 11:35       ` Alejandro Colomar
2023-10-01 11:38       ` [PATCH v3] " Alejandro Colomar
2023-10-01 11:41       ` [PATCH v4] " Alejandro Colomar
2023-10-01 16:24       ` [PATCH v5] " Alejandro Colomar
2023-10-08 13:05         ` Ping: " Alejandro Colomar
2023-11-13  9:55         ` Alejandro Colomar
2024-02-06 10:45       ` [PATCH v5 RESEND] " Alejandro Colomar
2024-02-25 18:10         ` Mike Stump
2024-02-25 19:44           ` Alejandro Colomar
2024-02-26 15:27             ` Joseph Myers
2024-02-26 15:24           ` Joseph Myers
2024-02-26 15:56             ` Alejandro Colomar
2024-02-26 16:19               ` Joseph Myers
2024-02-26 19:54                 ` Sandra Loosemore
2024-02-26 19:32               ` Mike Stump
2024-03-05 20:20                 ` [PATCH v6] " Alejandro Colomar
2024-03-05 20:25                   ` Alejandro Colomar
2024-03-05 20:33                 ` [PATCH v7] " Alejandro Colomar
2024-03-05 22:42                   ` Sandra Loosemore
2024-03-06 18:43                 ` [PATCH v8] " Alejandro Colomar
2024-05-14 18:16                   ` Alejandro Colomar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230324133928.14753-1-alx@kernel.org \
    --to=alx.manpages@gmail.com \
    --cc=alx@kernel.org \
    --cc=andrew@digital-domain.net \
    --cc=douglas.mcilroy@dartmouth.edu \
    --cc=g.branden.robinson@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jwakely.gcc@gmail.com \
    --cc=lm@mcvoy.com \
    --cc=muecker@gwdg.de \
    --cc=pinskia@gmail.com \
    --cc=ralph@inputplus.co.uk \
    --cc=saint.snit@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).