public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] gimple-ssa-sprintf: Use [0, 1] range for %lc with (wint_t) 0 argument [PR114876]
Date: Tue, 30 Apr 2024 09:25:42 +0200	[thread overview]
Message-ID: <ZjCc9teLQ2NWcZQF@tucnak> (raw)

Hi!

Seems when Martin S. implemented this, he coded there strict reading
of the standard, which said that %lc with (wint_t) 0 argument is handled
as wchar_t[2] temp = { arg, 0 }; %ls with temp arg and so shouldn't print
any values.  But, most of the libc implementations actually handled that
case like %c with '\0' argument, adding a single NUL character, the only
known exception is musl.
Recently, C23 changed this in response to GB-141 and POSIX in
https://austingroupbugs.net/view.php?id=1647
so that it should have the same behavior as %c with '\0'.

Because there is implementation divergence, the following patch uses
a range rather than hardcoding it to all 1s (i.e. the %c behavior),
though the likely case is still 1 (forward looking plus most of
implementations).
The res.knownrange = true; assignment removed is redundant due to
the same assignment done unconditionally before the if statement,
rest is formatting fixes.

I don't think the min >= 0 && min < 128 case is right either, I'd think
it should be min >= 0 && max < 128, otherwise it is just some possible
inputs are (maybe) ASCII and there can be others, but this code is a total
mess anyway, with the min, max, likely (somewhere in [min, max]?) and then
unlikely possibly larger than max, dunno, perhaps for at least some chars
in the ASCII range the likely case could be for the ascii case; so perhaps
just the one_2_one_ascii shouldn't set max to 1 and mayfail should be true
for max >= 128.  Anyway, didn't feel I should touch that right now.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Shall it go to 14.1, or wait for 14.2?

2024-04-30  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/114876
	* gimple-ssa-sprintf.cc (format_character): For min == 0 && max == 0,
	set max, likely and unlikely members to 1 rather than 0.  Remove
	useless res.knownrange = true;.  Formatting fixes.

	* gcc.dg/pr114876.c: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Adjust expected
	diagnostics.

--- gcc/gimple-ssa-sprintf.cc.jj	2024-01-03 11:51:22.225860346 +0100
+++ gcc/gimple-ssa-sprintf.cc	2024-04-29 12:52:59.760668894 +0200
@@ -2177,8 +2177,7 @@ format_character (const directive &dir,
 
   res.knownrange = true;
 
-  if (dir.specifier == 'C'
-      || dir.modifier == FMT_LEN_l)
+  if (dir.specifier == 'C' || dir.modifier == FMT_LEN_l)
     {
       /* A wide character can result in as few as zero bytes.  */
       res.range.min = 0;
@@ -2189,10 +2188,13 @@ format_character (const directive &dir,
 	{
 	  if (min == 0 && max == 0)
 	    {
-	      /* The NUL wide character results in no bytes.  */
-	      res.range.max = 0;
-	      res.range.likely = 0;
-	      res.range.unlikely = 0;
+	      /* In strict reading of older ISO C or POSIX, this required
+		 no characters to be emitted.  ISO C23 changes that, so
+		 does POSIX, to match what has been implemented in most of the
+		 implementations, namely emitting a single NUL character.
+		 Let's use 0 for minimum and 1 for all the other values.  */
+	      res.range.max = 1;
+	      res.range.likely = res.range.unlikely = 1;
 	    }
 	  else if (min >= 0 && min < 128)
 	    {
@@ -2200,11 +2202,12 @@ format_character (const directive &dir,
 		 is not a 1-to-1 mapping to the source character set or
 		 if the source set is not ASCII.  */
 	      bool one_2_one_ascii
-		= (target_to_host_charmap[0] == 1 && target_to_host ('a') == 97);
+		= (target_to_host_charmap[0] == 1
+		   && target_to_host ('a') == 97);
 
 	      /* A wide character in the ASCII range most likely results
 		 in a single byte, and only unlikely in up to MB_LEN_MAX.  */
-	      res.range.max = one_2_one_ascii ? 1 : target_mb_len_max ();;
+	      res.range.max = one_2_one_ascii ? 1 : target_mb_len_max ();
 	      res.range.likely = 1;
 	      res.range.unlikely = target_mb_len_max ();
 	      res.mayfail = !one_2_one_ascii;
@@ -2235,7 +2238,6 @@ format_character (const directive &dir,
       /* A plain '%c' directive.  Its output is exactly 1.  */
       res.range.min = res.range.max = 1;
       res.range.likely = res.range.unlikely = 1;
-      res.knownrange = true;
     }
 
   /* Bump up the byte counters if WIDTH is greater.  */
--- gcc/testsuite/gcc.dg/pr114876.c.jj	2024-04-29 12:26:45.774965158 +0200
+++ gcc/testsuite/gcc.dg/pr114876.c	2024-04-29 12:51:37.863777055 +0200
@@ -0,0 +1,34 @@
+/* PR tree-optimization/114876 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-not "return \[01\];" "optimized" } } */
+/* { dg-final { scan-tree-dump "return 3;" "optimized" } } */
+/* { dg-final { scan-tree-dump "return 4;" "optimized" } } */
+
+int
+foo (void)
+{
+  char buf[64];
+  return __builtin_sprintf (buf, "%lc%lc%lc", (__WINT_TYPE__) 0, (__WINT_TYPE__) 0, (__WINT_TYPE__) 0);
+}
+
+int
+bar (void)
+{
+  char buf[64];
+  return __builtin_sprintf (buf, "%c%c%c", 0, 0, 0);
+}
+
+int
+baz (void)
+{
+  char buf[64];
+  return __builtin_sprintf (buf, "%lc%lc%lca", (__WINT_TYPE__) 0, (__WINT_TYPE__) 0, (__WINT_TYPE__) 0);
+}
+
+int
+qux (void)
+{
+  char buf[64];
+  return __builtin_sprintf (buf, "%c%c%ca", 0, 0, 0);
+}
--- gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c.jj	2020-12-03 10:04:35.888092988 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c	2024-04-29 12:49:14.452717581 +0200
@@ -200,11 +200,11 @@ void test_sprintf_chk_c_const (void)
   T (3, "%c%c", '1', '2');
 
   /* Wide characters.  */
-  T (0, "%lc",     (wint_t)0);   /* { dg-warning "nul past the end" } */
-  T (1, "%lc",     (wint_t)0);
-  T (1, "%lc%lc",  (wint_t)0, (wint_t)0);
+  T (0, "%lc",     (wint_t)0);   /* { dg-warning ".%lc. directive writing up to 1 bytes into a region of size 0" } */
+  T (1, "%lc",     (wint_t)0);   /* { dg-warning "nul past the end" } */
+  T (1, "%lc%lc",  (wint_t)0, (wint_t)0);   /* { dg-warning ".%lc. directive writing up to 1 bytes into a region of size between 0 and 1" } */
   T (2, "%lc",     (wint_t)0);
-  T (2, "%lc%lc",  (wint_t)0, (wint_t)0);
+  T (2, "%lc%lc",  (wint_t)0, (wint_t)0);   /* { dg-warning "nul past the end" } */
 
   /* The following could result in as few as no bytes and in as many as
      MB_CUR_MAX, but since the MB_CUR_MAX value is a runtime property
@@ -1550,7 +1550,7 @@ void test_snprintf_c_const (char *d)
 
   /* Wide characters.  */
   T (0, "%lc",  (wint_t)0);
-  T (1, "%lc",  (wint_t)0);
+  T (1, "%lc",  (wint_t)0);      /* { dg-warning "output may be truncated before the last format character" } */
   T (2, "%lc",  (wint_t)0);
 
   /* The following could result in as few as a single byte and in as many
@@ -1603,7 +1603,7 @@ void test_snprintf_chk_c_const (void)
 
   /* Wide characters.  */
   T (0, "%lc",  (wint_t)0);
-  T (1, "%lc",  (wint_t)0);
+  T (1, "%lc",  (wint_t)0);      /* { dg-warning "output may be truncated before the last format character" } */
   T (2, "%lc",  (wint_t)0);
 
   /* The following could result in as few as a single byte and in as many

	Jakub


             reply	other threads:[~2024-04-30  7:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-30  7:25 Jakub Jelinek [this message]
2024-04-30  9:01 ` Richard Biener

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=ZjCc9teLQ2NWcZQF@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /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).