public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: gcc-patches@gcc.gnu.org, Jonathan Wakely <jwakely@redhat.com>
Subject: [PATCH] c++: Allow indeterminate unsigned char or std::byte in bit_cast - P1272R4
Date: Mon, 29 Nov 2021 17:31:42 +0100	[thread overview]
Message-ID: <20211129163142.GG2646553@tucnak> (raw)

Hi!

P1272R4 has added to the std::byteswap new stuff to me quite unrelated
clarification for std::bit_cast.
std::bit_cast is already in C++20 though and std::byteswap is a new
C++23 feature, it is unclear to me if that bit_cast addition in there
is really meant for C++23 and later only (e.g. related to only
C++23 actually allowing non-literal vars and thus other spots where
indeterminate values can appear during constant evaluation), or if
it was meant as a DR and should apply to C++20 too.

The following so far only lightly tested patch (dg.exp=bit-cast*.C
testing) implements it as applying to C++23 only.

Also, it is unclear to me how bitfields should behave, whether having
indeterminate bits in
struct S { unsigned char a : 8, b : 5, c : 3, d, e; unsigned int f : 8, g : 24; };
struct T1 { unsigned char a : 1, : 7, b : 5, c : 3, d, e; unsigned int f : 8, g : 24; };
struct T2 { unsigned char a : 8, b : 1, : 4, c : 3, d, e; unsigned int f : 8, g : 24; };
struct T3 { unsigned char a : 8, b : 5, c : 1, : 2, d, e; unsigned int f : 8, g : 24; };
struct T4 { unsigned char a : 8, b : 5, c : 3, d, e; unsigned int f : 1, : 7, g : 24; };
constexpr bool f1 () { T1 t = { 0, 0, 0, 0, 0, 0, 0 }; S s = __builtin_bit_cast (S, t); return true; }
constexpr bool f2 () { T2 t = { 0, 0, 0, 0, 0, 0, 0 }; S s = __builtin_bit_cast (S, t); return true; }
constexpr bool f3 () { T3 t = { 0, 0, 0, 0, 0, 0, 0 }; S s = __builtin_bit_cast (S, t); return true; }
constexpr bool f4 () { T4 t = { 0, 0, 0, 0, 0, 0, 0 }; S s = __builtin_bit_cast (S, t); return true; }
constexpr bool a = f1 ();
constexpr bool b = f2 ();
constexpr bool c = f3 ();
constexpr bool d = f4 ();
The patch currently uses TREE_TYPE and doesn't check DECL_BIT_FIELD,
so it accepts f1 and f4 cases where the TREE_TYPE is unsigned char
and doesn't accept f2 and f3 where the type is some 5 or 3 bit precision
unsigned type.  But possibly either it should look at the declared
type of the bitfield and don't allow f4 and allow f1-f3, or don't allow
any of the f1-f4 cases.

2021-11-29  Jakub Jelinek <jakub@redhat.com>

	* constexpr.c (clear_uchar_or_std_byte_in_mask): New function.
	(cxx_eval_bit_cast): Don't error about padding bits if target
	type is unsigned char or std::byte, instead return no clearing
	ctor.  Use clear_uchar_or_std_byte_in_mask.

	* g++.dg/cpp2a/bit-cast11.C: New test.
	* g++.dg/cpp2a/bit-cast12.C: New test.
	* g++.dg/cpp2a/bit-cast13.C: New test.

--- gcc/cp/constexpr.c.jj	2021-11-22 10:13:17.156882931 +0100
+++ gcc/cp/constexpr.c	2021-11-29 15:41:54.611361669 +0100
@@ -4268,6 +4268,59 @@ check_bit_cast_type (const constexpr_ctx
   return false;
 }
 
+/* Helper function for cxx_eval_bit_cast.  For unsigned char or
+   std::byte members of CONSTRUCTOR (recursively) if they contain
+   some indeterminate bits (as set in MASK), remove the ctor elts,
+   mark the CONSTRUCTOR as CONSTRUCTOR_NO_CLEARING and clear the
+   bits in MASK.  */
+
+static void
+clear_uchar_or_std_byte_in_mask (tree t, unsigned char *mask)
+{
+  if (TREE_CODE (t) != CONSTRUCTOR)
+    return;
+
+  unsigned i, j = 0;
+  tree index, value;
+  FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (t), i, index, value)
+    {
+      tree type = TREE_TYPE (value);
+      if (is_byte_access_type (type)
+	  && TYPE_MAIN_VARIANT (type) != char_type_node)
+	{
+	  HOST_WIDE_INT pos;
+	  if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE)
+	    pos = tree_to_shwi (index);
+	  else
+	    pos = int_byte_position (index);
+	  if (mask[pos])
+	    {
+	      CONSTRUCTOR_NO_CLEARING (t) = 1;
+	      mask[pos] = 0;
+	      continue;
+	    }
+	}
+      if (TREE_CODE (value) == CONSTRUCTOR)
+	{
+	  HOST_WIDE_INT pos;
+	  if (TREE_CODE (TREE_TYPE (t)) == ARRAY_TYPE)
+	    pos = tree_to_shwi (index)
+		  * tree_to_shwi (TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (t))));
+	  else
+	    pos = int_byte_position (index);
+	  clear_uchar_or_std_byte_in_mask (value, mask + pos);
+	}
+      if (i != j)
+	{
+	  CONSTRUCTOR_ELT (t, j)->index = index;
+	  CONSTRUCTOR_ELT (t, j)->value = value;
+	}
+      ++j;
+    }
+  if (CONSTRUCTOR_NELTS (t) != j)
+    vec_safe_truncate (CONSTRUCTOR_ELTS (t), j);
+}
+
 /* Subroutine of cxx_eval_constant_expression.
    Attempt to evaluate a BIT_CAST_EXPR.  */
 
@@ -4344,12 +4397,30 @@ cxx_eval_bit_cast (const constexpr_ctx *
 
   tree r = NULL_TREE;
   if (can_native_interpret_type_p (TREE_TYPE (t)))
-    r = native_interpret_expr (TREE_TYPE (t), ptr, len);
+    {
+      r = native_interpret_expr (TREE_TYPE (t), ptr, len);
+      if (cxx_dialect >= cxx23
+	  && is_byte_access_type (TREE_TYPE (t))
+	  && TYPE_MAIN_VARIANT (TREE_TYPE (t)) != char_type_node)
+	{
+	  gcc_assert (len == 1);
+	  if (mask[0])
+	    {
+	      memset (mask, 0, len);
+	      r = build_constructor (TREE_TYPE (r), NULL);
+	      CONSTRUCTOR_NO_CLEARING (r) = 1;
+	    }
+	}
+    }
   else if (TREE_CODE (TREE_TYPE (t)) == RECORD_TYPE)
     {
       r = native_interpret_aggregate (TREE_TYPE (t), ptr, 0, len);
       if (r != NULL_TREE)
-	clear_type_padding_in_mask (TREE_TYPE (t), mask);
+	{
+	  clear_type_padding_in_mask (TREE_TYPE (t), mask);
+	  if (cxx_dialect >= cxx23)
+	    clear_uchar_or_std_byte_in_mask (r, mask);
+	}
     }
 
   if (r != NULL_TREE)
--- gcc/testsuite/g++.dg/cpp2a/bit-cast11.C.jj	2021-11-29 16:21:38.470327372 +0100
+++ gcc/testsuite/g++.dg/cpp2a/bit-cast11.C	2021-11-29 16:23:24.536811602 +0100
@@ -0,0 +1,69 @@
+// P1272R4
+// { dg-do compile { target c++14 } }
+
+struct S { unsigned char a[2]; alignas(sizeof 0) int b; };
+struct T { char a; alignas(sizeof 0) int b; };
+struct U { char a : 1; char : 6; char b : 1; };
+struct V { int a; S b; };
+struct W { unsigned a; T b; };
+
+constexpr bool
+f1 ()
+{
+  T t = { 1, 2 };
+  S s = __builtin_bit_cast (S, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  S u = s;
+  return u.a[0] == 1;
+}
+
+constexpr bool
+f2 ()
+{
+  U u = { 0, 0 };
+  unsigned char a = __builtin_bit_cast (unsigned char, u);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  unsigned char b = a;
+  return true;
+}
+
+constexpr bool
+f3 ()
+{
+  T t = { 1, 2 };
+  S s = __builtin_bit_cast (S, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  S u = s;
+  return u.a[1] == 0;
+}
+
+constexpr bool
+f4 ()
+{
+  U u = { 0, 0 };
+  unsigned char a = __builtin_bit_cast (unsigned char, u);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  unsigned char b = a;
+  return b == 0;		// { dg-error "is not a constant expression" "" { target c++23 } }
+}
+
+constexpr bool
+f5 ()
+{
+  W t = { 1, 2 };
+  V s = __builtin_bit_cast (V, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  V u = s;
+  return u.b.a[0] == 1;
+}
+
+constexpr bool
+f6 ()
+{
+  W t = { 1, 2 };
+  V s = __builtin_bit_cast (V, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  V u = s;
+  return u.b.a[1] == 1;
+}
+
+constexpr bool a = f1 ();
+constexpr bool b = f2 ();
+constexpr bool c = f3 ();	// { dg-error "accessing uninitialized array element" "" { target c++23 } }
+constexpr bool d = f4 ();
+constexpr bool e = f5 ();
+constexpr bool f = f6 ();	// { dg-error "accessing uninitialized array element" "" { target c++23 } }
--- gcc/testsuite/g++.dg/cpp2a/bit-cast12.C.jj	2021-11-29 16:22:25.106660906 +0100
+++ gcc/testsuite/g++.dg/cpp2a/bit-cast12.C	2021-11-29 16:23:40.012590436 +0100
@@ -0,0 +1,74 @@
+// P1272R4
+// { dg-do compile { target c++14 } }
+
+namespace std
+{
+  enum class byte : unsigned char {};
+}
+
+struct S { unsigned char a[2]; alignas(sizeof 0) int b; };
+struct T { char a; alignas(sizeof 0) int b; };
+struct U { char a : 1; char : 6; char b : 1; };
+struct V { int a; S b; };
+struct W { unsigned a; T b; };
+
+constexpr bool
+f1 ()
+{
+  T t = { 1, 2 };
+  S s = __builtin_bit_cast (S, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  S u = s;
+  return u.a[0] == 1;
+}
+
+constexpr bool
+f2 ()
+{
+  U u = { 0, 0 };
+  unsigned char a = __builtin_bit_cast (unsigned char, u);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  unsigned char b = a;
+  return true;
+}
+
+constexpr bool
+f3 ()
+{
+  T t = { 1, 2 };
+  S s = __builtin_bit_cast (S, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  S u = s;
+  return u.a[1] == 0;
+}
+
+constexpr bool
+f4 ()
+{
+  U u = { 0, 0 };
+  unsigned char a = __builtin_bit_cast (unsigned char, u);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  unsigned char b = a;
+  return b == 0;		// { dg-error "is not a constant expression" "" { target c++23 } }
+}
+
+constexpr bool
+f5 ()
+{
+  W t = { 1, 2 };
+  V s = __builtin_bit_cast (V, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  V u = s;
+  return u.b.a[0] == 1;
+}
+
+constexpr bool
+f6 ()
+{
+  W t = { 1, 2 };
+  V s = __builtin_bit_cast (V, t);	// { dg-error "accessing uninitialized byte" "" { target c++20_down } }
+  V u = s;
+  return u.b.a[1] == 1;
+}
+
+constexpr bool a = f1 ();
+constexpr bool b = f2 ();
+constexpr bool c = f3 ();	// { dg-error "accessing uninitialized array element" "" { target c++23 } }
+constexpr bool d = f4 ();
+constexpr bool e = f5 ();
+constexpr bool f = f6 ();	// { dg-error "accessing uninitialized array element" "" { target c++23 } }
--- gcc/testsuite/g++.dg/cpp2a/bit-cast13.C.jj	2021-11-29 16:23:52.403413364 +0100
+++ gcc/testsuite/g++.dg/cpp2a/bit-cast13.C	2021-11-29 16:26:14.432383655 +0100
@@ -0,0 +1,69 @@
+// P1272R4
+// { dg-do compile { target c++14 } }
+
+struct S { char a[2]; alignas(sizeof 0) int b; };
+struct T { char a; alignas(sizeof 0) int b; };
+struct U { char a : 1; char : 6; char b : 1; };
+struct V { int a; S b; };
+struct W { unsigned a; T b; };
+
+constexpr bool
+f1 ()
+{
+  T t = { 1, 2 };
+  S s = __builtin_bit_cast (S, t);	// { dg-error "accessing uninitialized byte" }
+  S u = s;
+  return u.a[0] == 1;
+}
+
+constexpr bool
+f2 ()
+{
+  U u = { 0, 0 };
+  char a = __builtin_bit_cast (char, u);	// { dg-error "accessing uninitialized byte" }
+  char b = a;
+  return true;
+}
+
+constexpr bool
+f3 ()
+{
+  T t = { 1, 2 };
+  S s = __builtin_bit_cast (S, t);	// { dg-error "accessing uninitialized byte" }
+  S u = s;
+  return u.a[1] == 0;
+}
+
+constexpr bool
+f4 ()
+{
+  U u = { 0, 0 };
+  char a = __builtin_bit_cast (char, u);	// { dg-error "accessing uninitialized byte" }
+  char b = a;
+  return b == 0;
+}
+
+constexpr bool
+f5 ()
+{
+  W t = { 1, 2 };
+  V s = __builtin_bit_cast (V, t);	// { dg-error "accessing uninitialized byte" }
+  V u = s;
+  return u.b.a[0] == 1;
+}
+
+constexpr bool
+f6 ()
+{
+  W t = { 1, 2 };
+  V s = __builtin_bit_cast (V, t);	// { dg-error "accessing uninitialized byte" }
+  V u = s;
+  return u.b.a[1] == 1;
+}
+
+constexpr bool a = f1 ();
+constexpr bool b = f2 ();
+constexpr bool c = f3 ();
+constexpr bool d = f4 ();
+constexpr bool e = f5 ();
+constexpr bool f = f6 ();

	Jakub


             reply	other threads:[~2021-11-29 16:31 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-29 16:31 Jakub Jelinek [this message]
2021-11-30  3:25 ` Jason Merrill
2021-11-30 12:17   ` [PATCH] c++, v2: " Jakub Jelinek
2021-11-30 20:19     ` Jason Merrill
2021-12-01  9:46       ` Jakub Jelinek
2021-12-02 20:32         ` Jason Merrill
2021-12-02 20:56           ` Jakub Jelinek
2021-12-03 18:56             ` Jason Merrill
2021-12-04 10:12               ` Jakub Jelinek

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=20211129163142.GG2646553@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=jwakely@redhat.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).