diff mbox

C++ PATCH for c++/77742 (-Waligned-new and placement new)

Message ID CADzB+2=caHOhkssgXkvhiTEVMb-4GVAD9G2r1Lx10B6WcxEL9A@mail.gmail.com
State New
Headers show

Commit Message

Jason Merrill Oct. 12, 2016, 6:04 p.m. UTC 
It doesn't make sense to warn about placement new of over-aligned
types; most occurrences are likely to be doing this in order to
provide properly aligned memory by allocating it first and then using
the standard non-allocating placement new.

Also, the discussion of -Waligned-new on HPPA led me to think that
even though MALLOC_ABI_ALIGNMENT is oddly low on most targets, some
might define it to be larger than max_align_t_align, and we should
take advantage of that when it occurs.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit fb86b2acf865a283c94fea604843308b3b5ba331
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Oct 11 17:24:05 2016 -0400

            PR c++/77742 - -Waligned-new and placement new.
            * init.c (build_new_1): Don't -Waligned-new about placement new.
            (malloc_alignment): New.  Consider MALLOC_ABI_ALIGNMENT.
            * decl.c (cxx_init_decl_processing): New.

Comments

Andreas Schwab Oct. 13, 2016, 8:54 a.m. UTC | #1 
FAIL: g++.dg/cpp1z/aligned-new7.C   (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20161013/gcc/testsuite/g++.dg/cpp1z/aligned-new7.C:13:41: warning: requested alignment 8 is larger than 2 [-Wattributes]

Andreas.
Christophe Lyon Oct. 13, 2016, 2:17 p.m. UTC | #2 
Hi Jason,


On 13 October 2016 at 10:54, Andreas Schwab <schwab@suse.de> wrote:
> FAIL: g++.dg/cpp1z/aligned-new7.C   (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20161013/gcc/testsuite/g++.dg/cpp1z/aligned-new7.C:13:41: warning: requested alignment 8 is larger than 2 [-Wattributes]
>

Similarly on arm*/aarch64*:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/g++.dg/cpp1z/aligned-new7.C:13:41:
warning: requested alignment 32 is larger than 16 [-Wattributes]

Christophe

> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Eric Botcazou Oct. 20, 2016, 10:50 a.m. UTC | #3 
> Similarly on arm*/aarch64*:
> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/g++.dg/cpp1z/aligned-new7.
> C:13:41: warning: requested alignment 32 is larger than 16 [-Wattributes]

Likewise on SPARC/Solaris:

aligned-new7.C: In function 'int main()':
aligned-new7.C:13:41: warning: requested alignment 16 is larger than 8 [-
Wattributes]
   alignas(alignof(X)) char buf[sizeof(X)];

Can we reopen the PR since it's apparently not fixed everywhere?
Jason Merrill Oct. 20, 2016, 1:49 p.m. UTC | #4 
On Thu, Oct 20, 2016 at 6:50 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Similarly on arm*/aarch64*:
>> /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/g++.dg/cpp1z/aligned-new7.
>> C:13:41: warning: requested alignment 32 is larger than 16 [-Wattributes]
>
> Likewise on SPARC/Solaris:
>
> aligned-new7.C: In function 'int main()':
> aligned-new7.C:13:41: warning: requested alignment 16 is larger than 8 [-
> Wattributes]
>    alignas(alignof(X)) char buf[sizeof(X)];

Fixed the testcase.

Jason
Eric Botcazou Oct. 20, 2016, 2:06 p.m. UTC | #5 
> Fixed the testcase.

It passes now, thanks!
diff mbox

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8b0442f..88cae04 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5953,6 +5953,7 @@  extern tree build_offset_ref			(tree, tree, bool,
 						 tsubst_flags_t);
 extern tree throw_bad_array_new_length		(void);
 extern bool type_has_new_extended_alignment	(tree);
+extern unsigned malloc_alignment		(void);
 extern tree build_new				(vec<tree, va_gc> **, tree, tree,
 						 vec<tree, va_gc> **, int,
                                                  tsubst_flags_t);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 2d11aef..6240893 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4082,7 +4082,7 @@  cxx_init_decl_processing (void)
   if (aligned_new_threshold == -1)
     aligned_new_threshold = (cxx_dialect >= cxx1z) ? 1 : 0;
   if (aligned_new_threshold == 1)
-    aligned_new_threshold = max_align_t_align () / BITS_PER_UNIT;
+    aligned_new_threshold = malloc_alignment () / BITS_PER_UNIT;
 
   {
     tree newattrs, extvisattr;
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index b4b5e0a..455995a 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -2589,6 +2589,16 @@  type_has_new_extended_alignment (tree t)
 	  && TYPE_ALIGN_UNIT (t) > (unsigned)aligned_new_threshold);
 }
 
+/* Return the alignment we expect malloc to guarantee.  This should just be
+   MALLOC_ABI_ALIGNMENT, but that macro defaults to only BITS_PER_WORD for some
+   reason, so don't let the threshold be smaller than max_align_t_align.  */
+
+unsigned
+malloc_alignment ()
+{
+  return MAX (max_align_t_align(), MALLOC_ABI_ALIGNMENT);
+}
+
 /* Generate code for a new-expression, including calling the "operator
    new" function, initializing the object, and, if an exception occurs
    during construction, cleaning up.  The arguments are as for
@@ -2974,8 +2984,23 @@  build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
 
   gcc_assert (alloc_fn != NULL_TREE);
 
+  /* Now, check to see if this function is actually a placement
+     allocation function.  This can happen even when PLACEMENT is NULL
+     because we might have something like:
+
+       struct S { void* operator new (size_t, int i = 0); };
+
+     A call to `new S' will get this allocation function, even though
+     there is no explicit placement argument.  If there is more than
+     one argument, or there are variable arguments, then this is a
+     placement allocation function.  */
+  placement_allocation_fn_p
+    = (type_num_arguments (TREE_TYPE (alloc_fn)) > 1
+       || varargs_function_p (alloc_fn));
+
   if (warn_aligned_new
-      && TYPE_ALIGN (elt_type) > max_align_t_align ()
+      && !placement_allocation_fn_p
+      && TYPE_ALIGN (elt_type) > malloc_alignment ()
       && (warn_aligned_new > 1
 	  || CP_DECL_CONTEXT (alloc_fn) == global_namespace)
       && !aligned_allocation_fn_p (alloc_fn))
@@ -3033,20 +3058,6 @@  build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
   while (TREE_CODE (alloc_call) == COMPOUND_EXPR)
     alloc_call = TREE_OPERAND (alloc_call, 1);
 
-  /* Now, check to see if this function is actually a placement
-     allocation function.  This can happen even when PLACEMENT is NULL
-     because we might have something like:
-
-       struct S { void* operator new (size_t, int i = 0); };
-
-     A call to `new S' will get this allocation function, even though
-     there is no explicit placement argument.  If there is more than
-     one argument, or there are variable arguments, then this is a
-     placement allocation function.  */
-  placement_allocation_fn_p
-    = (type_num_arguments (TREE_TYPE (alloc_fn)) > 1
-       || varargs_function_p (alloc_fn));
-
   /* Preevaluate the placement args so that we don't reevaluate them for a
      placement delete.  */
   if (placement_allocation_fn_p)
diff --git a/gcc/testsuite/g++.dg/cpp1z/aligned-new7.C b/gcc/testsuite/g++.dg/cpp1z/aligned-new7.C
new file mode 100644
index 0000000..e12db9b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/aligned-new7.C
@@ -0,0 +1,15 @@ 
+// PR c++/77742
+// { dg-options "-Wall -std=c++1z" }
+
+#include <new>
+
+struct X
+{
+  alignas(2*__STDCPP_DEFAULT_NEW_ALIGNMENT__) int i;
+};
+
+int main()
+{
+  alignas(alignof(X)) char buf[sizeof(X)];
+  ::new((void*)buf) X{1};
+}