Re: [C++ PATCH] Aligned new option handling fixes (PR c++/77651)

[Christophe Lyon <christophe dot lyon at linaro dot org>]

Hi,


On 21 September 2016 at 17:03, Jason Merrill <jason@redhat.com> wrote:
> OK.
>
> On Wed, Sep 21, 2016 at 10:46 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> Hi!
>>
>> The following patch fixes some ICEs which were because of missing
>> RejectNegative for the *aligned-new= options - they have their "negative"
>> values as the option argument, so the only options that should allow
>> negative forms are -Wno-aligned-new and -fno-aligned-new, not
>> -Wno-aligned-new=none or -fno-aligned-new=32.
>>
>> Another thing is that sometimes the -Waligned-new warning suggests to use
>> -faligned-new even when it is already on, that looks just too weird (it can
>> be e.g. because some class has custom operator new defined in it, but not with
>> std::align_val_t argument).
>>
>> Plus, I believe aligned_new_threshhold is a spelling typo, too many h chars.
>>
>> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>>
>> 2016-09-21  Jakub Jelinek  <jakub@redhat.com>
>>
>>         PR c++/77651
>> c-family/
>>         * c.opt (Waligned-new=): Add RejectNegative.
>>         (faligned-new=): Likewise.  Spelling fix - change
>>         aligned_new_threshhold to aligned_new_threshold.
>>         * c-cppbuiltin.c (c_cpp_builtins): Change aligned_new_threshhold
>>         to aligned_new_threshold.
>> cp/
>>         * init.c (build_new_1): Don't suggest to use -faligned-new if
>>         aligned_new_threshold is non-zero.
>>         (type_has_new_extended_alignment): Change aligned_new_threshhold
>>         to aligned_new_threshold.
>>         * call.c (second_parm_is_size_t, aligned_allocation_fn_p,
>>         aligned_deallocation_fn_p, build_op_delete_call): Likewise.
>>         * decl.c (cxx_init_decl_processing): Likewise.
>> testsuite/
>>         * g++.dg/cpp1z/aligned-new6.C: New test.
>>

This new test (aligned-new6.C) fails on arm/aarch64 bare-metal targets
(using newlib):
arm-none-eabi/./libstdc++-v3/src/.libs/libstdc++.a(new_opa.o): In
function `operator new(unsigned int, std::align_val_t)':
/gccsrc/libstdc++-v3/libsupc++/new_opa.cc:76: undefined reference to
`aligned_alloc'
collect2: error: ld returned 1 exit status

Christophe

>> --- gcc/c-family/c.opt.jj       2016-09-21 08:54:14.000000000 +0200
>> +++ gcc/c-family/c.opt  2016-09-21 10:52:01.468493875 +0200
>> @@ -288,7 +288,7 @@ C++ ObjC++ Alias(Waligned-new=,global,no
>>  Warn about 'new' of type with extended alignment without -faligned-new.
>>
>>  Waligned-new=
>> -C++ ObjC++ Var(warn_aligned_new) Enum(warn_aligned_new_level) Joined Warning LangEnabledBy(C++ ObjC++,Wall,1,0)
>> +C++ ObjC++ Var(warn_aligned_new) Enum(warn_aligned_new_level) Joined RejectNegative Warning LangEnabledBy(C++ ObjC++,Wall,1,0)
>>  -Waligned-new=all Warn even if 'new' uses a class member allocation function.
>>
>>  Wall
>> @@ -1071,7 +1071,7 @@ C++ ObjC++ Alias(faligned-new=,1,0)
>>  Support C++17 allocation of over-aligned types.
>>
>>  faligned-new=
>> -C++ ObjC++ Joined Var(aligned_new_threshhold) UInteger Init(-1)
>> +C++ ObjC++ Joined RejectNegative Var(aligned_new_threshold) UInteger Init(-1)
>>  -faligned-new=<N> Use C++17 over-aligned type allocation for alignments greater than N.
>>
>>  fall-virtual
>> --- gcc/c-family/c-cppbuiltin.c.jj      2016-09-13 10:43:54.000000000 +0200
>> +++ gcc/c-family/c-cppbuiltin.c 2016-09-21 10:52:20.101258415 +0200
>> @@ -944,11 +944,11 @@ c_cpp_builtins (cpp_reader *pfile)
>>         cpp_define (pfile, "__cpp_transactional_memory=210500");
>>        if (flag_sized_deallocation)
>>         cpp_define (pfile, "__cpp_sized_deallocation=201309");
>> -      if (aligned_new_threshhold)
>> +      if (aligned_new_threshold)
>>         {
>>           cpp_define (pfile, "__cpp_aligned_new=201606");
>>           cpp_define_formatted (pfile, "__STDCPP_DEFAULT_NEW_ALIGNMENT__=%d",
>> -                               aligned_new_threshhold);
>> +                               aligned_new_threshold);
>>         }
>>      }
>>    /* Note that we define this for C as well, so that we know if
>> --- gcc/cp/init.c.jj    2016-09-14 23:49:03.000000000 +0200
>> +++ gcc/cp/init.c       2016-09-21 11:06:57.953163363 +0200
>> @@ -2574,8 +2574,8 @@ warn_placement_new_too_small (tree type,
>>  bool
>>  type_has_new_extended_alignment (tree t)
>>  {
>> -  return (aligned_new_threshhold
>> -         && TYPE_ALIGN_UNIT (t) > (unsigned)aligned_new_threshhold);
>> +  return (aligned_new_threshold
>> +         && TYPE_ALIGN_UNIT (t) > (unsigned)aligned_new_threshold);
>>  }
>>
>>  /* Generate code for a new-expression, including calling the "operator
>> @@ -3026,8 +3026,9 @@ build_new_1 (vec<tree, va_gc> **placemen
>>                "alignment %d", elt_type, TYPE_ALIGN_UNIT (elt_type));
>>        inform (input_location, "uses %qD, which does not have an alignment "
>>               "parameter", alloc_fn);
>> -      inform (input_location, "use %<-faligned-new%> to enable C++17 "
>> -             "over-aligned new support");
>> +      if (!aligned_new_threshold)
>> +       inform (input_location, "use %<-faligned-new%> to enable C++17 "
>> +                               "over-aligned new support");
>>      }
>>
>>    /* If we found a simple case of PLACEMENT_EXPR above, then copy it
>> --- gcc/cp/call.c.jj    2016-09-13 10:43:56.000000000 +0200
>> +++ gcc/cp/call.c       2016-09-21 10:53:07.607658081 +0200
>> @@ -5965,7 +5965,7 @@ second_parm_is_size_t (tree fn)
>>    t = TREE_CHAIN (t);
>>    if (t == void_list_node)
>>      return true;
>> -  if (aligned_new_threshhold && t
>> +  if (aligned_new_threshold && t
>>        && same_type_p (TREE_VALUE (t), align_type_node)
>>        && TREE_CHAIN (t) == void_list_node)
>>      return true;
>> @@ -5978,7 +5978,7 @@ second_parm_is_size_t (tree fn)
>>  bool
>>  aligned_allocation_fn_p (tree t)
>>  {
>> -  if (!aligned_new_threshhold)
>> +  if (!aligned_new_threshold)
>>      return false;
>>
>>    tree a = FUNCTION_ARG_CHAIN (t);
>> @@ -5992,7 +5992,7 @@ aligned_allocation_fn_p (tree t)
>>  static bool
>>  aligned_deallocation_fn_p (tree t)
>>  {
>> -  if (!aligned_new_threshhold)
>> +  if (!aligned_new_threshold)
>>      return false;
>>
>>    /* A template instance is never a usual deallocation function,
>> @@ -6202,7 +6202,7 @@ build_op_delete_call (enum tree_code cod
>>                selection process terminates. If more than one preferred
>>                function is found, all non-preferred functions are eliminated
>>                from further consideration.  */
>> -           if (aligned_new_threshhold)
>> +           if (aligned_new_threshold)
>>               {
>>                 bool want_align = type_has_new_extended_alignment (type);
>>                 bool fn_align = aligned_deallocation_fn_p (fn);
>> --- gcc/cp/decl.c.jj    2016-09-16 22:19:39.000000000 +0200
>> +++ gcc/cp/decl.c       2016-09-21 10:53:38.191271599 +0200
>> @@ -4132,16 +4132,16 @@ cxx_init_decl_processing (void)
>>    /* Now, C++.  */
>>    current_lang_name = lang_name_cplusplus;
>>
>> -  if (aligned_new_threshhold > 1
>> -      && !pow2p_hwi (aligned_new_threshhold))
>> +  if (aligned_new_threshold > 1
>> +      && !pow2p_hwi (aligned_new_threshold))
>>      {
>> -      error ("-faligned-new=%d is not a power of two", aligned_new_threshhold);
>> -      aligned_new_threshhold = 1;
>> +      error ("-faligned-new=%d is not a power of two", aligned_new_threshold);
>> +      aligned_new_threshold = 1;
>>      }
>> -  if (aligned_new_threshhold == -1)
>> -    aligned_new_threshhold = (cxx_dialect >= cxx1z) ? 1 : 0;
>> -  if (aligned_new_threshhold == 1)
>> -    aligned_new_threshhold = max_align_t_align () / BITS_PER_UNIT;
>> +  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;
>>
>>    {
>>      tree newattrs, extvisattr;
>> @@ -4210,7 +4210,7 @@ cxx_init_decl_processing (void)
>>         push_cp_library_fn (VEC_DELETE_EXPR, deltype, ECF_NOTHROW);
>>        }
>>
>> -    if (aligned_new_threshhold)
>> +    if (aligned_new_threshold)
>>        {
>>         push_namespace (std_identifier);
>>         tree align_id = get_identifier ("align_val_t");
>> --- gcc/testsuite/g++.dg/cpp1z/aligned-new6.C.jj        2016-09-21 10:43:40.773798919 +0200
>> +++ gcc/testsuite/g++.dg/cpp1z/aligned-new6.C   2016-09-21 10:45:55.319109667 +0200
>> @@ -0,0 +1,14 @@
>> +// PR c++/77651
>> +// { dg-do run { target { c++11 && c++14_down } } }
>> +// { dg-options "-faligned-new -W -Wall -Wno-aligned-new" }
>> +
>> +struct alignas(64) A { int i; };
>> +
>> +int
>> +main ()
>> +{
>> +  A *p = new A;
>> +  if (((__UINTPTR_TYPE__) p) % 64 != 0)
>> +    __builtin_abort ();
>> +  delete p;
>> +}
>>
>>         Jakub