On 12/8/20 1:46 PM, Martin Sebor wrote:
> PR 98160 reports an ICE in pretty printer code called from the newly
> added -Wmismatched-new-delete. The ICE is just a simple oversight,
> but much more interesting is the warning issued for the test case.
> It highlights an issue I didn't consider in the initial implementation:
> that inlining one of a pair of allocation/deallocation functions but
> not the other might lead to false positives when the inlined function
> calls another allocator that the deallocator isn't associated with.
>
> In addition, tests for the changes exposed the overly simplistic
> nature of the detection of calls to mismatched forms of operator
> new and delete which fails to consider member operators, also
> resulting in false positives.
>
> Finally, in a comment on the initial implementation Jonathan notes
> that the -Wmismatched-new-delete warning should trigger not only
> in user code but also in libstdc++ functions inlined into user code.
> I thought I had done that but as it turns out, the "standard code
> sequence" I put in place isn't sufficient to make this work.
>
> The attached changes avoid the false positives a) by ignoring (with
> a warning) the new form of the malloc attribute on inline functions,
> and disabling the inlining of others by implicitly adding attribute
> noinline to their declaration, and b) by making more robust
> the detection of mismatched operators new and delete. Furthermore,
> the patch also arranges for the warning to trigger even for inlined
> calls to functions defined in system headers.
>
> To make review a little (marginally) easier the change are two files:
> 1) gcc-98166-1.diff: introduces valid_new_delete_pair_p and
> tree_inlined_location.
> 2) gcc-98166-2.diff: adjusts the atrribute/warning implementation .
>
> Tested on x86_64-linux.
>
> Martin
>
> gcc-98166-1.diff
>
> Introduce an overload of valid_new_delete_pair_p and tree_inlined_location.
>
> gcc/ChangeLog:
>
> * tree-ssa-dce.c (valid_new_delete_pair_p): Factor code out into
> valid_new_delete_pair_p in tree.c.
> * tree.c (tree_inlined_location): Define new function.
> (valid_new_delete_pair_p): Define.
> * tree.h (tree_inlined_location): Declare.
> (valid_new_delete_pair_p): Declare.
OK
>
> gcc-98166-2.diff
>
> Correct/improve maybe_emit_free_warning (PR middle-end/98166, PR c++/57111,
> PR middle-end/98160).
>
> Resolves:
> PR middle-end/98166 - bogus -Wmismatched-dealloc on user-defined allocator
> and inlining
> PR c++/57111 - 57111 - Generalize -Wfree-nonheap-object to delete
> PR middle-end/98160 - ICE in default_tree_printer at gcc/tree-diagnostic.c:270
>
> gcc/ChangeLog:
>
> PR middle-end/98166
> PR c++/57111
> PR middle-end/98160
> * builtins.c (call_dealloc_p): Remove unused function.
> (new_delete_mismatch_p): Call valid_new_delete_pair_p and rework.
> (matching_alloc_calls_p): Handle built-in deallocation functions.
> (warn_dealloc_offset): Corrct the handling of user-defined operators
> delete.
> (maybe_emit_free_warning): Avoid assuming expression is a decl.
> Simplify.
> * doc/extend.texi (attribute malloc): Update.
>
> gcc/c-family/ChangeLog:
>
> PR middle-end/98166
> PR c++/57111
> PR middle-end/98160
> * c-attribs.c (handle_malloc_attribute): Implicitly add attribute
> noinline to functions not declared inline and warn on those.
>
> libstdc++-v3/ChangeLog:
> * testsuite/ext/vstring/requirements/exception/basic.cc: Suppress
> a false positive warning.
> *
> testsuite/ext/vstring/requirements/exception/propagation_consistent.cc:
> Same.
>
> gcc/testsuite/ChangeLog:
>
> PR middle-end/98166
> PR c++/57111
> PR middle-end/98160
> * g++.dg/warn/Wmismatched-dealloc-2.C: Adjust test of expected warning.
> * g++.dg/warn/Wmismatched-new-delete.C: Same.
> * gcc.dg/Wmismatched-dealloc.c: Same.
> * c-c++-common/Wfree-nonheap-object-2.c: New test.
> * c-c++-common/Wfree-nonheap-object-3.c: New test.
> * c-c++-common/Wfree-nonheap-object.c: New test.
> * c-c++-common/Wmismatched-dealloc.c: New test.
> * g++.dg/warn/Wfree-nonheap-object-3.C: New test.
> * g++.dg/warn/Wfree-nonheap-object-4.C: New test.
> * g++.dg/warn/Wmismatched-new-delete-2.C: New test.
OK, but there's a follow-up as noted below:
>
>
> diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc-2.C
> b/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc-2.C
> index 7ecc99a325c..3aea02fa63d 100644
> --- a/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc-2.C
> +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-dealloc-2.C
Please fix the non-unique testnames in this file. This can be done as a
separate follow-up patch as they're pre-existing.
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete.C
b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete.C
> index ed1090be5c5..fc07149995d 100644
> --- a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete.C
> +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete.C
And in this file.
Jeff
