Jason Merrill <ja...@redhat.com> writes: > On 8/2/24 4:36 PM, Arsen Arsenović wrote: >> I'm not 100% clear on what the semantics of the matching here are meant >> to be - AFAICT, an operator new/delete pair matches (after falling >> through the other cases) if all their components (besides the actual >> operator name, of course) match, and the pair of actual operator names >> matches if one is a singleton new and the other a singleton delete, or, >> similarly, if one is an array new and the other an array delete. We >> also appear to ignore their argument types (or so it seems by >> experimentation - I was not able to quite discern what path those take). > > Ignoring argument types is correct, placement delete is only called for > exception handling.
ACK - good to know.
>> Stripping operator template arguments from either side of the pair
>> should have no impact on this logic, I think.
>
> Agreed.
>
>> Tested on x86_64-pc-linux-gnu, no regressions.
>> OK for trunk?
>> TIA, have a lovely evening.
>> ---------- >8 ----------
>> Template parameters on a member operator new cannot affect its member
>> status nor whether it is a singleton or array operator new, hence, we
>> can ignore it for purposes of matching. Similar logic applies to the
>> placement operator delete.
>> In the PR (and a lot of idiomatic coroutine code generally), operator
>> new is templated in order to be able to inspect (some of) the arguments
>> passed to the coroutine, to make allocation-related decisions. However,
>> the coroutine implementation will not call a placement delete form, so
>> it cannot get templated. As a result, when demangling, we have an extra
>> template DEMANGLE_COMPONENT_TEMPLATE around the actual operator new, but
>> not operator delete. This terminates new_delete_mismatch_p early.
>> PR middle-end/109224 - Wmismatched-new-delete false positive with a templated
>> operator new (common with coroutines)
>> gcc/ChangeLog:
>> PR middle-end/109224
>> * gimple-ssa-warn-access.cc (new_delete_mismatch_p): Strip
>> DEMANGLE_COMPONENT_TEMPLATE from the operator new and operator
>> after demangling.
>> gcc/testsuite/ChangeLog:
>> PR middle-end/109224
>> * g++.dg/warn/Wmismatched-new-delete-9.C: New test.
>> ---
>> gcc/gimple-ssa-warn-access.cc | 18 ++++++-
>> .../g++.dg/warn/Wmismatched-new-delete-9.C | 47 +++++++++++++++++++
>> gcc/testsuite/g++.dg/warn/pr109224.C | 25 ++++++++++
>> 3 files changed, 89 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
>> create mode 100644 gcc/testsuite/g++.dg/warn/pr109224.C
>> diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
>> index 61f9f0f3d310..e3fec5fb8e77 100644
>> --- a/gcc/gimple-ssa-warn-access.cc
>> +++ b/gcc/gimple-ssa-warn-access.cc
>> @@ -1762,7 +1762,23 @@ new_delete_mismatch_p (tree new_decl, tree
>> delete_decl)
>> void *np = NULL, *dp = NULL;
>> demangle_component *ndc = cplus_demangle_v3_components (new_str, 0, &np);
>> demangle_component *ddc = cplus_demangle_v3_components (del_str, 0, &dp);
>> - bool mismatch = ndc && ddc && new_delete_mismatch_p (*ndc, *ddc);
>> +
>> + /* Sometimes, notably quite often with coroutines, 'operator new' is
>> + templated. However, template arguments can't change whether a given
>> + new/delete is a singleton or array one, nor what it is a member of, so
>> + the template arguments can be safely ignored for the purposes of
>> checking
>> + for mismatches. */
>> +
>> + auto strip_dc_template = [] (demangle_component* dc)
>> + {
>> + if (dc->type == DEMANGLE_COMPONENT_TEMPLATE)
>> + dc = dc->u.s_binary.left;
>> + return dc;
>> + };
>> +
>> + bool mismatch = ndc && ddc
>> + && new_delete_mismatch_p (*strip_dc_template (ndc),
>> + *strip_dc_template (ddc));
>
> The && should not be left of the =; if the initializer needs to span multiple
> lines, it's usually best to wrap it in ().
Okay - done.
>> free (np);
>> free (dp);
>> return mismatch;
>> diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
>> b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
>> new file mode 100644
>> index 000000000000..fa511bbfdb4b
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
>> @@ -0,0 +1,47 @@
>> +/* { dg-do compile { target c++11 } } */
>> +/* { dg-additional-options "-Wmismatched-new-delete" } */
>> +/* PR middle-end/109224 */
>> +/* Verify that we consider templated operator new matching with its operator
>> + delete. */
>> +
>> +#include <new>
>> +
>> +struct foo
>> +{
>> + template<typename... Args>
>> + void* operator new (std::size_t sz, Args&&...);
>> + template<typename... Args>
>> + void* operator new[] (std::size_t sz, Args&&...);
>> +
>> + void operator delete (void* x);
>> + void operator delete[] (void* x);
>> +
>> + template<typename... Args>
>> + void operator delete (void* x, Args&&...);
>> + template<typename... Args>
>> + void operator delete[] (void* x, Args&&...);
>> +};
>> +
>> +void
>> +f ()
>> +{
>> + delete (new (123, true) foo);
>> + delete[] (new (123, true) foo[123]);
>> +
>> + delete (new (123, true) foo[123]);
>> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
>> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
>> + delete[] (new (123, true) foo);
>> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
>> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
>> +
>> + foo::operator delete (new (123, true) foo, 123, true);
>> + foo::operator delete[] (new (123, true) foo[123], 123, true);
>
> Instead of passing the result of a new-expression directly to operator delete,
> you should also call operator new directly.
>
>> + foo::operator delete (new (123, true) foo[123], 123, true);
>> + // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
>> + // { dg-note "returned from" "" { target *-*-* } {.-2} }
>> + foo::operator delete[] (new (123, true) foo, 123, true);
>
> Likewise.
Ah, good point - adjusted.
Here are the changes to v1 I've made as well as the full patch below
them. Does this look OK?
--8<---------------cut here---------------start------------->8---
modified gcc/gimple-ssa-warn-access.cc
@@ -1776,9 +1776,9 @@ new_delete_mismatch_p (tree new_decl, tree delete_decl)
return dc;
};
- bool mismatch = ndc && ddc
- && new_delete_mismatch_p (*strip_dc_template (ndc),
- *strip_dc_template (ddc));
+ bool mismatch = (ndc && ddc
+ && new_delete_mismatch_p (*strip_dc_template (ndc),
+ *strip_dc_template (ddc)));
free (np);
free (dp);
return mismatch;
modified gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
@@ -35,13 +35,13 @@ f ()
// { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
// { dg-note "returned from" "" { target *-*-* } {.-2} }
- foo::operator delete (new (123, true) foo, 123, true);
- foo::operator delete[] (new (123, true) foo[123], 123, true);
+ foo::operator delete (foo::operator new (1, 123, true), 123, true);
+ foo::operator delete[] (foo::operator new[] (123, 123, true), 123, true);
- foo::operator delete (new (123, true) foo[123], 123, true);
+ foo::operator delete (foo::operator new[] (123, 123, true), 123, true);
// { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
// { dg-note "returned from" "" { target *-*-* } {.-2} }
- foo::operator delete[] (new (123, true) foo, 123, true);
+ foo::operator delete[] (foo::operator new (1, 123, true), 123, true);
// { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
// { dg-note "returned from" "" { target *-*-* } {.-2} }
}
--8<---------------cut here---------------end--------------->8---
---------- >8 ----------
Template parameters on a member operator new cannot affect its member
status nor whether it is a singleton or array operator new, hence, we
can ignore it for purposes of matching. Similar logic applies to the
placement operator delete.
In the PR (and a lot of idiomatic coroutine code generally), operator
new is templated in order to be able to inspect (some of) the arguments
passed to the coroutine, to make allocation-related decisions. However,
the coroutine implementation will not call a placement delete form, so
it cannot get templated. As a result, when demangling, we have an extra
template DEMANGLE_COMPONENT_TEMPLATE around the actual operator new, but
not operator delete. This terminates new_delete_mismatch_p early.
PR middle-end/109224 - Wmismatched-new-delete false positive with a
templated operator new (common with coroutines)
gcc/ChangeLog:
* gimple-ssa-warn-access.cc (new_delete_mismatch_p): Strip
DEMANGLE_COMPONENT_TEMPLATE from the operator new and operator
after demangling.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wmismatched-new-delete-9.C: New test.
---
gcc/gimple-ssa-warn-access.cc | 18 ++++++-
.../g++.dg/warn/Wmismatched-new-delete-9.C | 47 +++++++++++++++++++
gcc/testsuite/g++.dg/warn/pr109224.C | 25 ++++++++++
3 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
create mode 100644 gcc/testsuite/g++.dg/warn/pr109224.C
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 61f9f0f3d310..fcce63ee332d 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -1762,7 +1762,23 @@ new_delete_mismatch_p (tree new_decl, tree delete_decl)
void *np = NULL, *dp = NULL;
demangle_component *ndc = cplus_demangle_v3_components (new_str, 0, &np);
demangle_component *ddc = cplus_demangle_v3_components (del_str, 0, &dp);
- bool mismatch = ndc && ddc && new_delete_mismatch_p (*ndc, *ddc);
+
+ /* Sometimes, notably quite often with coroutines, 'operator new' is
+ templated. However, template arguments can't change whether a given
+ new/delete is a singleton or array one, nor what it is a member of, so
+ the template arguments can be safely ignored for the purposes of checking
+ for mismatches. */
+
+ auto strip_dc_template = [] (demangle_component* dc)
+ {
+ if (dc->type == DEMANGLE_COMPONENT_TEMPLATE)
+ dc = dc->u.s_binary.left;
+ return dc;
+ };
+
+ bool mismatch = (ndc && ddc
+ && new_delete_mismatch_p (*strip_dc_template (ndc),
+ *strip_dc_template (ddc)));
free (np);
free (dp);
return mismatch;
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
new file mode 100644
index 000000000000..d431f4049e87
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete-9.C
@@ -0,0 +1,47 @@
+/* { dg-do compile { target c++11 } } */
+/* { dg-additional-options "-Wmismatched-new-delete" } */
+/* PR middle-end/109224 */
+/* Verify that we consider templated operator new matching with its operator
+ delete. */
+
+#include <new>
+
+struct foo
+{
+ template<typename... Args>
+ void* operator new (std::size_t sz, Args&&...);
+ template<typename... Args>
+ void* operator new[] (std::size_t sz, Args&&...);
+
+ void operator delete (void* x);
+ void operator delete[] (void* x);
+
+ template<typename... Args>
+ void operator delete (void* x, Args&&...);
+ template<typename... Args>
+ void operator delete[] (void* x, Args&&...);
+};
+
+void
+f ()
+{
+ delete (new (123, true) foo);
+ delete[] (new (123, true) foo[123]);
+
+ delete (new (123, true) foo[123]);
+ // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
+ // { dg-note "returned from" "" { target *-*-* } {.-2} }
+ delete[] (new (123, true) foo);
+ // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
+ // { dg-note "returned from" "" { target *-*-* } {.-2} }
+
+ foo::operator delete (foo::operator new (1, 123, true), 123, true);
+ foo::operator delete[] (foo::operator new[] (123, 123, true), 123, true);
+
+ foo::operator delete (foo::operator new[] (123, 123, true), 123, true);
+ // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
+ // { dg-note "returned from" "" { target *-*-* } {.-2} }
+ foo::operator delete[] (foo::operator new (1, 123, true), 123, true);
+ // { dg-warning "Wmismatched-new-delete" "" { target *-*-* } {.-1} }
+ // { dg-note "returned from" "" { target *-*-* } {.-2} }
+}
diff --git a/gcc/testsuite/g++.dg/warn/pr109224.C
b/gcc/testsuite/g++.dg/warn/pr109224.C
new file mode 100644
index 000000000000..4b6102226ffc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr109224.C
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++20 } }
+#include <coroutine>
+
+struct Task {
+ struct promise_type {
+ std::suspend_never initial_suspend() { return {}; }
+ std::suspend_never final_suspend() noexcept { return {}; }
+ void unhandled_exception() { throw; }
+ Task get_return_object() { return {}; }
+ void return_void() {}
+
+ template<class I>
+ void* operator new(std::size_t sz, I);
+
+ void operator delete(void* ptr, std::size_t);
+ };
+};
+
+Task f(int) {
+ co_return;
+}
+
+int main() {
+ f(42);
+}
--
2.46.0
signature.asc
Description: PGP signature
