[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 Martin Liška changed: What|Removed |Added Ever confirmed|0 |1 Summary|[10 Regression] Optimizing |[10 Regression] Optimizing |mismatched new/delete pairs |mismatched new/delete pairs ||since ||r10-2106-g6343b6bf3bb83c87 CC||redi at gcc dot gnu.org Known to work||9.3.0 Last reconfirmed||2020-03-25 Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org Status|UNCONFIRMED |ASSIGNED Known to fail||10.0 --- Comment #3 from Martin Liška --- Yes, I remember we discussed the topic about the user-provided new/delete implementations. Can please Jason or Jonathan comment about the test-case?
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #4 from Richard Biener --- (In reply to Martin Liška from comment #3) > Yes, I remember we discussed the topic about the user-provided new/delete > implementations. Can please Jason or Jonathan comment about the test-case? The testcase is certainly valid. The issue is we're matching new/delete pairs by means of dataflow (the new resulting pointer is fed to the delete) and identify new/delete by the decls flag. But that doesn't catch the case in this PR where there is a mismatch between the new/delete calls. Now - the question is if whether class-specific operator new/delete even have to "match" in this sense or how it's possible to "match" at all. I think the frontend has to provide some "link" between the new/delete decls to make this work (which is then quite heavy - an extra pointer in function decls) :/
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #5 from Marc Glisse --- I don't think we need heavy machinery linking new and delete (and if we did I'd be tempted to store it in some global table rather than in the nodes). The most important case is the global replacable functions, for which we have a finite list, and for those a few checks like not matching array with non-array versions should do. For user overloads with attribute malloc (a gcc extension), I would go with heuristics like both/neither being class members, being members of the same class, etc. Although I am not quite sure how doable that is from the middle-end, how much of that information is still available (I think it is available in the mangled name, but demangling doesn't seem like a great idea).
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #6 from Jason Merrill --- (In reply to Marc Glisse from comment #5) > I don't think we need heavy machinery linking new and delete (and if we did > I'd be tempted to store it in some global table rather than in the nodes). > The most important case is the global replacable functions, for which we > have a finite list, and for those a few checks like not matching array with > non-array versions should do. For user overloads with attribute malloc (a > gcc extension), I would go with heuristics like both/neither being class > members, being members of the same class, etc. Although I am not quite sure > how doable that is from the middle-end, how much of that information is > still available (I think it is available in the mangled name, but demangling > doesn't seem like a great idea). It should be sufficient to check whether they have the same DECL_CONTEXT.
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #7 from Martin Liška --- > It should be sufficient to check whether they have the same DECL_CONTEXT. This seems to work. I'm testing a patch candidate.
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #8 from CVS Commits --- The master branch has been updated by Martin Liska : https://gcc.gnu.org/g:70df40cab6f268ba7f05c6d1421928cca0834ee3 commit r10-7628-g70df40cab6f268ba7f05c6d1421928cca0834ee3 Author: Martin Liska Date: Wed Apr 8 17:16:55 2020 +0200 Allow new/delete operator deletion only for replaceable. PR c++/94314 * gimple.c (gimple_call_operator_delete_p): Rename to... (gimple_call_replaceable_operator_delete_p): ... this. Use DECL_IS_REPLACEABLE_OPERATOR_DELETE_P. * gimple.h (gimple_call_operator_delete_p): Rename to ... (gimple_call_replaceable_operator_delete_p): ... this. * tree-core.h (tree_function_decl): Add replaceable_operator flag. * tree-ssa-dce.c (mark_all_reaching_defs_necessary_1): Use DECL_IS_REPLACEABLE_OPERATOR_DELETE_P. (propagate_necessity): Use gimple_call_replaceable_operator_delete_p. (eliminate_unnecessary_stmts): Likewise. * tree-streamer-in.c (unpack_ts_function_decl_value_fields): Pack DECL_IS_REPLACEABLE_OPERATOR. * tree-streamer-out.c (pack_ts_function_decl_value_fields): Unpack the field here. * tree.h (DECL_IS_REPLACEABLE_OPERATOR): New. (DECL_IS_REPLACEABLE_OPERATOR_NEW_P): New. (DECL_IS_REPLACEABLE_OPERATOR_DELETE_P): New. * cgraph.c (cgraph_node::dump): Dump if an operator is replaceable. * ipa-icf.c (sem_item::compare_referenced_symbol_properties): Compare replaceable operator flags. PR c++/94314 * decl.c (duplicate_decls): Duplicate also DECL_IS_REPLACEABLE_OPERATOR. (cxx_init_decl_processing): Mark replaceable all implicitly defined operators. PR c++/94314 * lto-common.c (compare_tree_sccs_1): Compare also DECL_IS_REPLACEABLE_OPERATOR. PR c++/94314 * g++.dg/pr94314-2.C: New test. * g++.dg/pr94314-3.C: New test. * g++.dg/pr94314.C: New test.
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 Martin Liška changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #9 from Martin Liška --- Fixed.
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314
--- Comment #10 from Marc Glisse ---
I am still getting -1 at -O2 for
#include
#include
int count = 0;
__attribute__((malloc,noinline))
void* operator new[](unsigned long sz){++count;return ::operator new(sz);}
void operator delete[](void* ptr)noexcept{--count;::operator delete(ptr);}
void operator delete[](void* ptr, std::size_t sz)noexcept{--count;::operator
delete(ptr, sz);}
int main(){
delete[] new int[1];
printf("%d\n",count); // Should print 0.
}
I am not aware of any code that breaks in practice, but it still looks strange.
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #11 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:6c9a711575d8fdf9e75f01d7a0e84d558600df40 commit r10-7638-g6c9a711575d8fdf9e75f01d7a0e84d558600df40 Author: Jakub Jelinek Date: Wed Apr 8 20:04:01 2020 +0200 testsuite: Fix up pr94314*.C tests [PR94314] The test FAIL everywhere where size_t is not unsigned long. Fixed by using __SIZE_TYPE__ instead. 2020-04-08 Jakub Jelinek PR c++/94314 * g++.dg/pr94314.C (A::operator new, B::operator new, C::operator new): Use __SIZE_TYPE__ instead of unsigned long. * g++.dg/pr94314-3.C (base::operator new, B::operator new): Likewise.
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 Martin Liška changed: What|Removed |Added Resolution|FIXED |--- Status|RESOLVED|REOPENED --- Comment #12 from Martin Liška --- Let's reopen it again..
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #13 from CVS Commits --- The master branch has been updated by Martin Liska : https://gcc.gnu.org/g:d7a65edb629a010f7ef907d457343abcb569fab7 commit r10-7758-gd7a65edb629a010f7ef907d457343abcb569fab7 Author: Martin Liska Date: Thu Apr 16 15:39:22 2020 +0200 List valid pairs for new and delete operators. PR c++/94314 * cgraphclones.c (set_new_clone_decl_and_node_flags): Drop DECL_IS_REPLACEABLE_OPERATOR during cloning. * tree-ssa-dce.c (valid_new_delete_pair_p): New function. (propagate_necessity): Check operator names. PR c++/94314 * g++.dg/pr94314.C: Do not use dg-additional-options and remove not needed stdio.h include. * g++.dg/pr94314-2.C: Likewise. * g++.dg/pr94314-3.C: Likewise. * g++.dg/pr94314-4.C: New test. Co-Authored-By: Jakub Jelinek
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 Martin Liška changed: What|Removed |Added Resolution|--- |FIXED Known to work||10.0 Known to fail|10.0| Status|REOPENED|RESOLVED --- Comment #14 from Martin Liška --- Now it should be fixed.
[Bug c++/94314] [10 Regression] Optimizing mismatched new/delete pairs since r10-2106-g6343b6bf3bb83c87
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94314 --- Comment #15 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:61b58e7fa5aea3ef0c7db2d9f75d17d65dff56a3 commit r10-7766-g61b58e7fa5aea3ef0c7db2d9f75d17d65dff56a3 Author: Jakub Jelinek Date: Fri Apr 17 09:01:08 2020 +0200 testsuite: Fix up test language requirements [PR94314] With c++11 one gets: Excess errors: .../testsuite/g++.dg/pr94314-4.C:19:28: error: too many arguments to function 'void operator delete(void*)' because C++ sized deallocation is a C++14 feature. 2020-04-17 Jakub Jelinek PR c++/94314 * g++.dg/pr94314-4.C: Require c++14 rather than c++11.
