public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Reject __builtin_clear_padding on non-trivially-copyable types with one exception [PR102586]
@ 2022-02-11 18:55 Jakub Jelinek
  2022-03-12  4:45 ` Jason Merrill
  2022-04-06 14:41 ` [PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586] Jakub Jelinek
  0 siblings, 2 replies; 4+ messages in thread
From: Jakub Jelinek @ 2022-02-11 18:55 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

As mentioned by Jason in the PR, non-trivially-copyable types (or non-POD
for purposes of layout?) types can be base classes of derived classes in
which the padding in those non-trivially-copyable types can be redused for
some real data members or even the layout can change and data members can
be moved to other positions.
__builtin_clear_padding is right now used for multiple purposes,
in <atomic> where it isn't used yet but was planned as the main spot
it can be used for trivially copyable types only, ditto for std::bit_cast
where we also use it.  It is used for OpenMP long double atomics too but
long double is trivially copyable, and lastly for -ftrivial-auto-var-init=.

The following patch restricts the builtin to pointers to trivially-copyable
types, with the exception when it is called directly on an address of a
variable, in that case already the FE can verify it is the complete object
type and so it is safe to clear all the paddings in it.

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

Something like the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c16
will still be needed with adjusted testcase from
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15 such that
__builtin_clear_padding is called directly on var addresses rather than
in separate functions.

2022-02-11  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/102586
gcc/
	* doc/extend.texi (__builtin_clear_padding): Clearify that for C++
	argument type should be pointer to trivially-copyable type unless it
	is address of a variable or parameter.
gcc/cp/
	* call.cc (build_cxx_call): Diagnose __builtin_clear_padding where
	first argument's type is pointer to non-trivially-copyable type unless
	it is address of a variable or parameter.
gcc/testsuite/
	* g++.dg/cpp2a/builtin-clear-padding1.C: New test.

--- gcc/doc/extend.texi.jj	2022-02-09 15:16:03.336783697 +0100
+++ gcc/doc/extend.texi	2022-02-11 13:22:39.846157538 +0100
@@ -13993,6 +13993,11 @@ bits that are padding bits for all the u
 This built-in-function is useful if the padding bits of an object might
 have intederminate values and the object representation needs to be
 bitwise compared to some other object, for example for atomic operations.
+
+For C++, @var{ptr} argument type should be pointer to trivially-copyable
+type, unless the argument is address of a variable or parameter, because
+otherwise it isn't known if the type isn't just a base class whose padding
+bits are reused or laid out differently in a derived class.
 @end deftypefn
 
 @deftypefn {Built-in Function} @var{type} __builtin_bit_cast (@var{type}, @var{arg})
--- gcc/cp/call.cc.jj	2022-02-09 20:13:51.523305107 +0100
+++ gcc/cp/call.cc	2022-02-11 12:58:19.168301395 +0100
@@ -10398,6 +10398,27 @@ build_cxx_call (tree fn, int nargs, tree
       if (!check_builtin_function_arguments (EXPR_LOCATION (fn), vNULL, fndecl,
 					     orig_fndecl, nargs, argarray))
 	return error_mark_node;
+      else if (fndecl_built_in_p (fndecl, BUILT_IN_CLEAR_PADDING))
+	{
+	  tree arg0 = argarray[0];
+	  STRIP_NOPS (arg0);
+	  if (TREE_CODE (arg0) == ADDR_EXPR
+	      && DECL_P (TREE_OPERAND (arg0, 0))
+	      && same_type_ignoring_top_level_qualifiers_p
+			(TREE_TYPE (TREE_TYPE (argarray[0])),
+			 TREE_TYPE (TREE_TYPE (arg0))))
+	    /* For __builtin_clear_padding (&var) we know the type
+	       is for a complete object, so there is no risk in clearing
+	       padding that is reused in some derived class member.  */;
+	  else if (!trivially_copyable_p (TREE_TYPE (TREE_TYPE (argarray[0]))))
+	    {
+	      error_at (EXPR_LOC_OR_LOC (argarray[0], input_location),
+			"argument %u in call to function %qE "
+			"has pointer to a non-trivially-copyable type (%qT)",
+			1, fndecl, TREE_TYPE (argarray[0]));
+	      return error_mark_node;
+	    }
+	}
     }
 
   if (VOID_TYPE_P (TREE_TYPE (fn)))
--- gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C.jj	2022-02-11 13:13:49.125471991 +0100
+++ gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C	2022-02-11 13:13:43.403550851 +0100
@@ -0,0 +1,50 @@
+// PR tree-optimization/102586
+// { dg-do compile }
+// { dg-options "-Wno-inaccessible-base" }
+
+struct C0 {};
+struct C1 {};
+struct C2 : C1, virtual C0 {};
+struct C3 : virtual C2, C1 {};
+struct C4 : virtual C3, C1 {};
+struct C5 : C4 {};
+struct C6 { char c; };
+struct C7 : virtual C6, virtual C3, C1 {};
+struct C8 : C7 {};
+
+void
+foo (C0 *c0, C1 *c1, C2 *c2, C3 *c3, C4 *c4, C5 *c5, C6 *c6, C7 *c7, C8 *c8)
+{
+  __builtin_clear_padding (c0);
+  __builtin_clear_padding (c1);
+  __builtin_clear_padding (c2);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C2\\\*'\\\)" }
+  __builtin_clear_padding (c3);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C3\\\*'\\\)" }
+  __builtin_clear_padding (c4);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C4\\\*'\\\)" }
+  __builtin_clear_padding (c5);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C5\\\*'\\\)" }
+  __builtin_clear_padding (c6);
+  __builtin_clear_padding (c7);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C7\\\*'\\\)" }
+  __builtin_clear_padding (c8);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C8\\\*'\\\)" }
+}
+
+void
+bar ()
+{
+  C0 c0;
+  C1 c1;
+  C2 c2;
+  C3 c3;
+  C4 c4;
+  C5 c5;
+  C6 c6;
+  C7 c7;
+  C8 c8;
+  __builtin_clear_padding (&c0);
+  __builtin_clear_padding (&c1);
+  __builtin_clear_padding (&c2);
+  __builtin_clear_padding (&c3);
+  __builtin_clear_padding (&c4);
+//  __builtin_clear_padding (&c5);
+  __builtin_clear_padding (&c6);
+  __builtin_clear_padding (&c7);
+  __builtin_clear_padding (&c8);
+}

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Reject __builtin_clear_padding on non-trivially-copyable types with one exception [PR102586]
  2022-02-11 18:55 [PATCH] c++: Reject __builtin_clear_padding on non-trivially-copyable types with one exception [PR102586] Jakub Jelinek
@ 2022-03-12  4:45 ` Jason Merrill
  2022-04-06 14:41 ` [PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586] Jakub Jelinek
  1 sibling, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2022-03-12  4:45 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 2/11/22 14:55, Jakub Jelinek wrote:
> Hi!
> 
> As mentioned by Jason in the PR, non-trivially-copyable types (or non-POD
> for purposes of layout?) types can be base classes of derived classes in
> which the padding in those non-trivially-copyable types can be redused for
> some real data members or even the layout can change and data members can
> be moved to other positions.
> __builtin_clear_padding is right now used for multiple purposes,
> in <atomic> where it isn't used yet but was planned as the main spot
> it can be used for trivially copyable types only, ditto for std::bit_cast
> where we also use it.  It is used for OpenMP long double atomics too but
> long double is trivially copyable, and lastly for -ftrivial-auto-var-init=.
> 
> The following patch restricts the builtin to pointers to trivially-copyable
> types, with the exception when it is called directly on an address of a
> variable, in that case already the FE can verify it is the complete object
> type and so it is safe to clear all the paddings in it.
> 
> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

OK.

> Something like the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c16
> will still be needed with adjusted testcase from
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15 such that
> __builtin_clear_padding is called directly on var addresses rather than
> in separate functions.
> 
> 2022-02-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/102586
> gcc/
> 	* doc/extend.texi (__builtin_clear_padding): Clearify that for C++
> 	argument type should be pointer to trivially-copyable type unless it
> 	is address of a variable or parameter.
> gcc/cp/
> 	* call.cc (build_cxx_call): Diagnose __builtin_clear_padding where
> 	first argument's type is pointer to non-trivially-copyable type unless
> 	it is address of a variable or parameter.
> gcc/testsuite/
> 	* g++.dg/cpp2a/builtin-clear-padding1.C: New test.
> 
> --- gcc/doc/extend.texi.jj	2022-02-09 15:16:03.336783697 +0100
> +++ gcc/doc/extend.texi	2022-02-11 13:22:39.846157538 +0100
> @@ -13993,6 +13993,11 @@ bits that are padding bits for all the u
>   This built-in-function is useful if the padding bits of an object might
>   have intederminate values and the object representation needs to be
>   bitwise compared to some other object, for example for atomic operations.
> +
> +For C++, @var{ptr} argument type should be pointer to trivially-copyable
> +type, unless the argument is address of a variable or parameter, because
> +otherwise it isn't known if the type isn't just a base class whose padding
> +bits are reused or laid out differently in a derived class.
>   @end deftypefn
>   
>   @deftypefn {Built-in Function} @var{type} __builtin_bit_cast (@var{type}, @var{arg})
> --- gcc/cp/call.cc.jj	2022-02-09 20:13:51.523305107 +0100
> +++ gcc/cp/call.cc	2022-02-11 12:58:19.168301395 +0100
> @@ -10398,6 +10398,27 @@ build_cxx_call (tree fn, int nargs, tree
>         if (!check_builtin_function_arguments (EXPR_LOCATION (fn), vNULL, fndecl,
>   					     orig_fndecl, nargs, argarray))
>   	return error_mark_node;
> +      else if (fndecl_built_in_p (fndecl, BUILT_IN_CLEAR_PADDING))
> +	{
> +	  tree arg0 = argarray[0];
> +	  STRIP_NOPS (arg0);
> +	  if (TREE_CODE (arg0) == ADDR_EXPR
> +	      && DECL_P (TREE_OPERAND (arg0, 0))
> +	      && same_type_ignoring_top_level_qualifiers_p
> +			(TREE_TYPE (TREE_TYPE (argarray[0])),
> +			 TREE_TYPE (TREE_TYPE (arg0))))
> +	    /* For __builtin_clear_padding (&var) we know the type
> +	       is for a complete object, so there is no risk in clearing
> +	       padding that is reused in some derived class member.  */;
> +	  else if (!trivially_copyable_p (TREE_TYPE (TREE_TYPE (argarray[0]))))
> +	    {
> +	      error_at (EXPR_LOC_OR_LOC (argarray[0], input_location),
> +			"argument %u in call to function %qE "
> +			"has pointer to a non-trivially-copyable type (%qT)",
> +			1, fndecl, TREE_TYPE (argarray[0]));
> +	      return error_mark_node;
> +	    }
> +	}
>       }
>   
>     if (VOID_TYPE_P (TREE_TYPE (fn)))
> --- gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C.jj	2022-02-11 13:13:49.125471991 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C	2022-02-11 13:13:43.403550851 +0100
> @@ -0,0 +1,50 @@
> +// PR tree-optimization/102586
> +// { dg-do compile }
> +// { dg-options "-Wno-inaccessible-base" }
> +
> +struct C0 {};
> +struct C1 {};
> +struct C2 : C1, virtual C0 {};
> +struct C3 : virtual C2, C1 {};
> +struct C4 : virtual C3, C1 {};
> +struct C5 : C4 {};
> +struct C6 { char c; };
> +struct C7 : virtual C6, virtual C3, C1 {};
> +struct C8 : C7 {};
> +
> +void
> +foo (C0 *c0, C1 *c1, C2 *c2, C3 *c3, C4 *c4, C5 *c5, C6 *c6, C7 *c7, C8 *c8)
> +{
> +  __builtin_clear_padding (c0);
> +  __builtin_clear_padding (c1);
> +  __builtin_clear_padding (c2);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C2\\\*'\\\)" }
> +  __builtin_clear_padding (c3);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C3\\\*'\\\)" }
> +  __builtin_clear_padding (c4);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C4\\\*'\\\)" }
> +  __builtin_clear_padding (c5);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C5\\\*'\\\)" }
> +  __builtin_clear_padding (c6);
> +  __builtin_clear_padding (c7);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C7\\\*'\\\)" }
> +  __builtin_clear_padding (c8);	// { dg-error "argument 1 in call to function '__builtin_clear_padding' has pointer to a non-trivially-copyable type \\\('C8\\\*'\\\)" }
> +}
> +
> +void
> +bar ()
> +{
> +  C0 c0;
> +  C1 c1;
> +  C2 c2;
> +  C3 c3;
> +  C4 c4;
> +  C5 c5;
> +  C6 c6;
> +  C7 c7;
> +  C8 c8;
> +  __builtin_clear_padding (&c0);
> +  __builtin_clear_padding (&c1);
> +  __builtin_clear_padding (&c2);
> +  __builtin_clear_padding (&c3);
> +  __builtin_clear_padding (&c4);
> +//  __builtin_clear_padding (&c5);
> +  __builtin_clear_padding (&c6);
> +  __builtin_clear_padding (&c7);
> +  __builtin_clear_padding (&c8);
> +}
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586]
  2022-02-11 18:55 [PATCH] c++: Reject __builtin_clear_padding on non-trivially-copyable types with one exception [PR102586] Jakub Jelinek
  2022-03-12  4:45 ` Jason Merrill
@ 2022-04-06 14:41 ` Jakub Jelinek
  2022-04-06 15:11   ` Jason Merrill
  1 sibling, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-04-06 14:41 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Fri, Feb 11, 2022 at 07:55:50PM +0100, Jakub Jelinek via Gcc-patches wrote:
> Something like the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c16
> will still be needed with adjusted testcase from
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15 such that
> __builtin_clear_padding is called directly on var addresses rather than
> in separate functions.

Here is an updated version of the
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15
patch which uses FIELD_DECL in the langhook instead of its TREE_TYPE,
and the testcases have been adjusted for the builtin accepting
pointers to non-trivially-copyable types only if it is address of a
declaration.

Bootstrapped/regtested on powerpc64le-linux, ok for trunk?

2022-04-06  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/102586
gcc/
	* langhooks.h (struct lang_hooks_for_types): Add classtype_as_base
	langhook.
	* langhooks-def.h (LANG_HOOKS_CLASSTYPE_AS_BASE): Define.
	(LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it.
	* gimple-fold.cc (clear_padding_type): Use ftype instead of
	TREE_TYPE (field) some more.  For artificial FIELD_DECLs without
	name try the lang_hooks.types.classtype_as_base langhook and
	if it returns non-NULL, use that instead of ftype for recursive call.
gcc/cp/
	* cp-objcp-common.h (cp_classtype_as_base): Declare.
	(LANG_HOOKS_CLASSTYPE_AS_BASE): Redefine.
	* cp-objcp-common.cc (cp_classtype_as_base): New function.
gcc/testsuite/
	* g++.dg/torture/builtin-clear-padding-5.C: New test.
	* g++.dg/cpp2a/builtin-clear-padding1.C (bar): Uncomment one
	call that is now accepted.

--- gcc/langhooks.h.jj	2022-02-11 00:18:54.909437559 +0100
+++ gcc/langhooks.h	2022-04-06 12:34:43.312087323 +0200
@@ -188,6 +188,11 @@ struct lang_hooks_for_types
   /* Returns a tree for the unit size of T excluding tail padding that
      might be used by objects inheriting from T.  */
   tree (*unit_size_without_reusable_padding) (tree);
+
+  /* Returns type corresponding to FIELD's type when FIELD is a C++ base class
+     i.e., type without virtual base classes or tail padding.  Returns
+     NULL_TREE otherwise.  */
+  tree (*classtype_as_base) (const_tree);
 };
 
 /* Language hooks related to decls and the symbol table.  */
--- gcc/langhooks-def.h.jj	2022-02-11 00:18:54.887437859 +0100
+++ gcc/langhooks-def.h	2022-04-06 12:31:39.149670170 +0200
@@ -216,6 +216,7 @@ extern tree lhd_unit_size_without_reusab
 #define LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO NULL
 #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE	lhd_type_dwarf_attribute
 #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING lhd_unit_size_without_reusable_padding
+#define LANG_HOOKS_CLASSTYPE_AS_BASE	hook_tree_const_tree_null
 
 #define LANG_HOOKS_FOR_TYPES_INITIALIZER { \
   LANG_HOOKS_MAKE_TYPE, \
@@ -243,7 +244,8 @@ extern tree lhd_unit_size_without_reusab
   LANG_HOOKS_GET_DEBUG_TYPE, \
   LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO, \
   LANG_HOOKS_TYPE_DWARF_ATTRIBUTE, \
-  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING \
+  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING, \
+  LANG_HOOKS_CLASSTYPE_AS_BASE \
 }
 
 /* Declaration hooks.  */
--- gcc/gimple-fold.cc.jj	2022-04-06 09:59:32.744654454 +0200
+++ gcc/gimple-fold.cc	2022-04-06 12:35:29.413440758 +0200
@@ -4747,7 +4747,7 @@ clear_padding_type (clear_padding_struct
 				      "have well defined padding bits for %qs",
 			    field, "__builtin_clear_padding");
 	      }
-	    else if (is_empty_type (TREE_TYPE (field)))
+	    else if (is_empty_type (ftype))
 	      continue;
 	    else
 	      {
@@ -4758,8 +4758,9 @@ clear_padding_type (clear_padding_struct
 		gcc_assert (pos >= 0 && fldsz >= 0 && pos >= cur_pos);
 		clear_padding_add_padding (buf, pos - cur_pos);
 		cur_pos = pos;
-		clear_padding_type (buf, TREE_TYPE (field),
-				    fldsz, for_auto_init);
+		if (tree asbase = lang_hooks.types.classtype_as_base (field))
+		  ftype = asbase;
+		clear_padding_type (buf, ftype, fldsz, for_auto_init);
 		cur_pos += fldsz;
 	      }
 	  }
--- gcc/cp/cp-objcp-common.h.jj	2022-02-11 00:18:54.730439994 +0100
+++ gcc/cp/cp-objcp-common.h	2022-04-06 12:31:39.151670141 +0200
@@ -31,6 +31,7 @@ extern int cp_decl_dwarf_attribute (cons
 extern int cp_type_dwarf_attribute (const_tree, int);
 extern void cp_common_init_ts (void);
 extern tree cp_unit_size_without_reusable_padding (tree);
+extern tree cp_classtype_as_base (const_tree);
 extern tree cp_get_global_decls ();
 extern tree cp_pushdecl (tree);
 extern void cp_register_dumps (gcc::dump_manager *);
@@ -167,6 +168,8 @@ extern tree cxx_simulate_record_decl (lo
 #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE cp_type_dwarf_attribute
 #undef LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING
 #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING cp_unit_size_without_reusable_padding
+#undef LANG_HOOKS_CLASSTYPE_AS_BASE
+#define LANG_HOOKS_CLASSTYPE_AS_BASE cp_classtype_as_base
 
 #undef LANG_HOOKS_OMP_PREDETERMINED_SHARING
 #define LANG_HOOKS_OMP_PREDETERMINED_SHARING cxx_omp_predetermined_sharing
--- gcc/cp/cp-objcp-common.cc.jj	2022-02-11 00:18:54.720440130 +0100
+++ gcc/cp/cp-objcp-common.cc	2022-04-06 13:24:43.543051969 +0200
@@ -280,6 +280,23 @@ cp_unit_size_without_reusable_padding (t
   return TYPE_SIZE_UNIT (type);
 }
 
+/* Returns type corresponding to FIELD's type when FIELD is a C++ base class
+   i.e., type without virtual base classes or tail padding.  Returns
+   NULL_TREE otherwise.  */
+
+tree
+cp_classtype_as_base (const_tree field)
+{
+  tree type = TREE_TYPE (field);
+  if (!DECL_ARTIFICIAL (field)
+      || DECL_NAME (field) != NULL_TREE
+      || TREE_CODE (type) != RECORD_TYPE)
+    return NULL_TREE;
+  if (!TYPE_LANG_SPECIFIC (type))
+    return NULL_TREE;
+  return CLASSTYPE_AS_BASE (type);
+}
+
 /* Stubs to keep c-opts.cc happy.  */
 void
 push_file_scope (void)
--- gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C.jj	2022-04-06 12:31:39.151670141 +0200
+++ gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C	2022-04-06 13:15:36.674717019 +0200
@@ -0,0 +1,44 @@
+// PR tree-optimization/102586
+// { dg-options "-Wno-inaccessible-base" }
+
+struct C0 {};
+struct C1 {};
+struct C2 : C1, virtual C0 {};
+struct C3 : virtual C2, C1 { virtual int foo () { return 1; } };
+struct C4 : virtual C3, C1 { virtual int foo () { return 2; } };
+struct C5 : C4 { virtual int foo () { return 3; } };
+struct C6 { char c; };
+struct C7 : virtual C6, virtual C3, C1 { virtual int foo () { return 4; } };
+struct C8 : C7 { virtual int foo () { return 5; } };
+
+__attribute__((noipa)) int
+bar (C5 *p)
+{
+  return p->foo ();
+}
+
+__attribute__((noipa)) int
+baz (C3 *p)
+{
+  return p->foo ();
+}
+
+__attribute__((noipa)) int
+qux (C8 *p)
+{
+  return p->foo ();
+}
+
+int
+main ()
+{
+  C5 c5;
+  C8 c8;
+  c8.c = 42;
+  __builtin_clear_padding (&c5);
+  __builtin_clear_padding (&c8);
+  if (bar (&c5) != 3 || baz (&c5) != 3)
+    __builtin_abort ();
+  if (qux (&c8) != 5 || baz (&c8) != 5 || c8.c != 42)
+    __builtin_abort ();
+}
--- gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C.jj	2022-03-14 10:46:42.021766309 +0100
+++ gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C	2022-04-06 13:14:31.614628521 +0200
@@ -43,7 +43,7 @@ bar ()
   __builtin_clear_padding (&c2);
   __builtin_clear_padding (&c3);
   __builtin_clear_padding (&c4);
-//  __builtin_clear_padding (&c5);
+  __builtin_clear_padding (&c5);
   __builtin_clear_padding (&c6);
   __builtin_clear_padding (&c7);
   __builtin_clear_padding (&c8);


	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586]
  2022-04-06 14:41 ` [PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586] Jakub Jelinek
@ 2022-04-06 15:11   ` Jason Merrill
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2022-04-06 15:11 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 4/6/22 10:41, Jakub Jelinek wrote:
> On Fri, Feb 11, 2022 at 07:55:50PM +0100, Jakub Jelinek via Gcc-patches wrote:
>> Something like the https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c16
>> will still be needed with adjusted testcase from
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15 such that
>> __builtin_clear_padding is called directly on var addresses rather than
>> in separate functions.
> 
> Here is an updated version of the
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102586#c15
> patch which uses FIELD_DECL in the langhook instead of its TREE_TYPE,
> and the testcases have been adjusted for the builtin accepting
> pointers to non-trivially-copyable types only if it is address of a
> declaration.
> 
> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
> 
> 2022-04-06  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/102586
> gcc/
> 	* langhooks.h (struct lang_hooks_for_types): Add classtype_as_base
> 	langhook.
> 	* langhooks-def.h (LANG_HOOKS_CLASSTYPE_AS_BASE): Define.
> 	(LANG_HOOKS_FOR_TYPES_INITIALIZER): Add it.
> 	* gimple-fold.cc (clear_padding_type): Use ftype instead of
> 	TREE_TYPE (field) some more.  For artificial FIELD_DECLs without
> 	name try the lang_hooks.types.classtype_as_base langhook and
> 	if it returns non-NULL, use that instead of ftype for recursive call.
> gcc/cp/
> 	* cp-objcp-common.h (cp_classtype_as_base): Declare.
> 	(LANG_HOOKS_CLASSTYPE_AS_BASE): Redefine.
> 	* cp-objcp-common.cc (cp_classtype_as_base): New function.
> gcc/testsuite/
> 	* g++.dg/torture/builtin-clear-padding-5.C: New test.
> 	* g++.dg/cpp2a/builtin-clear-padding1.C (bar): Uncomment one
> 	call that is now accepted.
> 
> --- gcc/langhooks.h.jj	2022-02-11 00:18:54.909437559 +0100
> +++ gcc/langhooks.h	2022-04-06 12:34:43.312087323 +0200
> @@ -188,6 +188,11 @@ struct lang_hooks_for_types
>     /* Returns a tree for the unit size of T excluding tail padding that
>        might be used by objects inheriting from T.  */
>     tree (*unit_size_without_reusable_padding) (tree);
> +
> +  /* Returns type corresponding to FIELD's type when FIELD is a C++ base class
> +     i.e., type without virtual base classes or tail padding.  Returns
> +     NULL_TREE otherwise.  */
> +  tree (*classtype_as_base) (const_tree);
>   };
>   
>   /* Language hooks related to decls and the symbol table.  */
> --- gcc/langhooks-def.h.jj	2022-02-11 00:18:54.887437859 +0100
> +++ gcc/langhooks-def.h	2022-04-06 12:31:39.149670170 +0200
> @@ -216,6 +216,7 @@ extern tree lhd_unit_size_without_reusab
>   #define LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO NULL
>   #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE	lhd_type_dwarf_attribute
>   #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING lhd_unit_size_without_reusable_padding
> +#define LANG_HOOKS_CLASSTYPE_AS_BASE	hook_tree_const_tree_null
>   
>   #define LANG_HOOKS_FOR_TYPES_INITIALIZER { \
>     LANG_HOOKS_MAKE_TYPE, \
> @@ -243,7 +244,8 @@ extern tree lhd_unit_size_without_reusab
>     LANG_HOOKS_GET_DEBUG_TYPE, \
>     LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO, \
>     LANG_HOOKS_TYPE_DWARF_ATTRIBUTE, \
> -  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING \
> +  LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING, \
> +  LANG_HOOKS_CLASSTYPE_AS_BASE \
>   }
>   
>   /* Declaration hooks.  */
> --- gcc/gimple-fold.cc.jj	2022-04-06 09:59:32.744654454 +0200
> +++ gcc/gimple-fold.cc	2022-04-06 12:35:29.413440758 +0200
> @@ -4747,7 +4747,7 @@ clear_padding_type (clear_padding_struct
>   				      "have well defined padding bits for %qs",
>   			    field, "__builtin_clear_padding");
>   	      }
> -	    else if (is_empty_type (TREE_TYPE (field)))
> +	    else if (is_empty_type (ftype))
>   	      continue;
>   	    else
>   	      {
> @@ -4758,8 +4758,9 @@ clear_padding_type (clear_padding_struct
>   		gcc_assert (pos >= 0 && fldsz >= 0 && pos >= cur_pos);
>   		clear_padding_add_padding (buf, pos - cur_pos);
>   		cur_pos = pos;
> -		clear_padding_type (buf, TREE_TYPE (field),
> -				    fldsz, for_auto_init);
> +		if (tree asbase = lang_hooks.types.classtype_as_base (field))
> +		  ftype = asbase;
> +		clear_padding_type (buf, ftype, fldsz, for_auto_init);
>   		cur_pos += fldsz;
>   	      }
>   	  }
> --- gcc/cp/cp-objcp-common.h.jj	2022-02-11 00:18:54.730439994 +0100
> +++ gcc/cp/cp-objcp-common.h	2022-04-06 12:31:39.151670141 +0200
> @@ -31,6 +31,7 @@ extern int cp_decl_dwarf_attribute (cons
>   extern int cp_type_dwarf_attribute (const_tree, int);
>   extern void cp_common_init_ts (void);
>   extern tree cp_unit_size_without_reusable_padding (tree);
> +extern tree cp_classtype_as_base (const_tree);
>   extern tree cp_get_global_decls ();
>   extern tree cp_pushdecl (tree);
>   extern void cp_register_dumps (gcc::dump_manager *);
> @@ -167,6 +168,8 @@ extern tree cxx_simulate_record_decl (lo
>   #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE cp_type_dwarf_attribute
>   #undef LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING
>   #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING cp_unit_size_without_reusable_padding
> +#undef LANG_HOOKS_CLASSTYPE_AS_BASE
> +#define LANG_HOOKS_CLASSTYPE_AS_BASE cp_classtype_as_base
>   
>   #undef LANG_HOOKS_OMP_PREDETERMINED_SHARING
>   #define LANG_HOOKS_OMP_PREDETERMINED_SHARING cxx_omp_predetermined_sharing
> --- gcc/cp/cp-objcp-common.cc.jj	2022-02-11 00:18:54.720440130 +0100
> +++ gcc/cp/cp-objcp-common.cc	2022-04-06 13:24:43.543051969 +0200
> @@ -280,6 +280,23 @@ cp_unit_size_without_reusable_padding (t
>     return TYPE_SIZE_UNIT (type);
>   }
>   
> +/* Returns type corresponding to FIELD's type when FIELD is a C++ base class
> +   i.e., type without virtual base classes or tail padding.  Returns
> +   NULL_TREE otherwise.  */
> +
> +tree
> +cp_classtype_as_base (const_tree field)
> +{
> +  tree type = TREE_TYPE (field);
> +  if (!DECL_ARTIFICIAL (field)
> +      || DECL_NAME (field) != NULL_TREE
> +      || TREE_CODE (type) != RECORD_TYPE)
> +    return NULL_TREE;
> +  if (!TYPE_LANG_SPECIFIC (type))
> +    return NULL_TREE;

Now that this is in the front-end, we can replace the above checks with 
DECL_FIELD_IS_BASE.  OK with that change.

> +  return CLASSTYPE_AS_BASE (type);
> +}
> +
>   /* Stubs to keep c-opts.cc happy.  */
>   void
>   push_file_scope (void)
> --- gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C.jj	2022-04-06 12:31:39.151670141 +0200
> +++ gcc/testsuite/g++.dg/torture/builtin-clear-padding-5.C	2022-04-06 13:15:36.674717019 +0200
> @@ -0,0 +1,44 @@
> +// PR tree-optimization/102586
> +// { dg-options "-Wno-inaccessible-base" }
> +
> +struct C0 {};
> +struct C1 {};
> +struct C2 : C1, virtual C0 {};
> +struct C3 : virtual C2, C1 { virtual int foo () { return 1; } };
> +struct C4 : virtual C3, C1 { virtual int foo () { return 2; } };
> +struct C5 : C4 { virtual int foo () { return 3; } };
> +struct C6 { char c; };
> +struct C7 : virtual C6, virtual C3, C1 { virtual int foo () { return 4; } };
> +struct C8 : C7 { virtual int foo () { return 5; } };
> +
> +__attribute__((noipa)) int
> +bar (C5 *p)
> +{
> +  return p->foo ();
> +}
> +
> +__attribute__((noipa)) int
> +baz (C3 *p)
> +{
> +  return p->foo ();
> +}
> +
> +__attribute__((noipa)) int
> +qux (C8 *p)
> +{
> +  return p->foo ();
> +}
> +
> +int
> +main ()
> +{
> +  C5 c5;
> +  C8 c8;
> +  c8.c = 42;
> +  __builtin_clear_padding (&c5);
> +  __builtin_clear_padding (&c8);
> +  if (bar (&c5) != 3 || baz (&c5) != 3)
> +    __builtin_abort ();
> +  if (qux (&c8) != 5 || baz (&c8) != 5 || c8.c != 42)
> +    __builtin_abort ();
> +}
> --- gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C.jj	2022-03-14 10:46:42.021766309 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/builtin-clear-padding1.C	2022-04-06 13:14:31.614628521 +0200
> @@ -43,7 +43,7 @@ bar ()
>     __builtin_clear_padding (&c2);
>     __builtin_clear_padding (&c3);
>     __builtin_clear_padding (&c4);
> -//  __builtin_clear_padding (&c5);
> +  __builtin_clear_padding (&c5);
>     __builtin_clear_padding (&c6);
>     __builtin_clear_padding (&c7);
>     __builtin_clear_padding (&c8);
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-06 15:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-11 18:55 [PATCH] c++: Reject __builtin_clear_padding on non-trivially-copyable types with one exception [PR102586] Jakub Jelinek
2022-03-12  4:45 ` Jason Merrill
2022-04-06 14:41 ` [PATCH] c++: Handle __builtin_clear_padding on non-trivially-copyable types [PR102586] Jakub Jelinek
2022-04-06 15:11   ` Jason Merrill

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).