c++: Don't crash upon invalid placement new operator [PR117101]
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
Commit Message
We currently crash upon the following invalid code (notice the "void
void**" parameter)
=== cut here ===
using size_t = decltype(sizeof(int));
void *operator new(size_t, void void **p) noexcept { return p; }
int x;
void f() {
int y;
new (&y) int(x);
}
=== cut here ===
The problem is that in this case, we end up with a NULL_TREE parameter
list for the new operator because of the error, and (1) coerce_new_type
wrongly complains about the first parameter type not being size_t,
(2) std_placement_new_fn_p blindly accesses the parameter list, hence a
crash.
This patch does NOT address #1 since we can't easily distinguish between
a new operator declaration without parameters from one with erroneous
parameters (and it's not worth the risk to refactor and break things for
an error recovery issue) hence a dg-bogus in new52.C, but it does
address #2 and the ICE by simply checking the first parameter against
NULL_TREE.
It also adds a new testcase checking that we complain about new
operators with no or invalid first parameters, since we did not have
any.
Successfully tested on x86_64-pc-linux-gnu.
PR c++/117101
gcc/cp/ChangeLog:
* init.cc (std_placement_new_fn_p): Check first_arg against
NULL_TREE.
gcc/testsuite/ChangeLog:
* g++.dg/init/new52.C: New test.
* g++.dg/init/new53.C: New test.
---
gcc/cp/init.cc | 5 +++--
gcc/testsuite/g++.dg/init/new52.C | 14 ++++++++++++++
gcc/testsuite/g++.dg/init/new53.C | 8 ++++++++
3 files changed, 25 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/init/new52.C
create mode 100644 gcc/testsuite/g++.dg/init/new53.C
Comments
On 11/1/24 1:22 PM, Simon Martin wrote:
> We currently crash upon the following invalid code (notice the "void
> void**" parameter)
>
> === cut here ===
> using size_t = decltype(sizeof(int));
> void *operator new(size_t, void void **p) noexcept { return p; }
> int x;
> void f() {
> int y;
> new (&y) int(x);
> }
> === cut here ===
>
> The problem is that in this case, we end up with a NULL_TREE parameter
> list for the new operator because of the error, and (1) coerce_new_type
> wrongly complains about the first parameter type not being size_t,
> (2) std_placement_new_fn_p blindly accesses the parameter list, hence a
> crash.
>
> This patch does NOT address #1 since we can't easily distinguish between
> a new operator declaration without parameters from one with erroneous
> parameters (and it's not worth the risk to refactor and break things for
> an error recovery issue) hence a dg-bogus in new52.C, but it does
> address #2 and the ICE by simply checking the first parameter against
> NULL_TREE.
>
> It also adds a new testcase checking that we complain about new
> operators with no or invalid first parameters, since we did not have
> any.
>
> Successfully tested on x86_64-pc-linux-gnu.
OK.
> PR c++/117101
>
> gcc/cp/ChangeLog:
>
> * init.cc (std_placement_new_fn_p): Check first_arg against
> NULL_TREE.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/init/new52.C: New test.
> * g++.dg/init/new53.C: New test.
>
> ---
> gcc/cp/init.cc | 5 +++--
> gcc/testsuite/g++.dg/init/new52.C | 14 ++++++++++++++
> gcc/testsuite/g++.dg/init/new53.C | 8 ++++++++
> 3 files changed, 25 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/init/new52.C
> create mode 100644 gcc/testsuite/g++.dg/init/new53.C
>
> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> index 0527f53550d..12c673efb2a 100644
> --- a/gcc/cp/init.cc
> +++ b/gcc/cp/init.cc
> @@ -2980,8 +2980,9 @@ std_placement_new_fn_p (tree alloc_fn)
> if (DECL_NAMESPACE_SCOPE_P (alloc_fn))
> {
> tree first_arg = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (alloc_fn)));
> - if ((TREE_VALUE (first_arg) == ptr_type_node)
> - && TREE_CHAIN (first_arg) == void_list_node)
> + if (first_arg
> + && (TREE_VALUE (first_arg) == ptr_type_node)
> + && (TREE_CHAIN (first_arg) == void_list_node))
> return true;
> }
> return false;
> diff --git a/gcc/testsuite/g++.dg/init/new52.C b/gcc/testsuite/g++.dg/init/new52.C
> new file mode 100644
> index 00000000000..5eee2135724
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/init/new52.C
> @@ -0,0 +1,14 @@
> +// PR c++/117101
> +// { dg-do "compile" { target c++11 } }
> +
> +using size_t = decltype(sizeof(int));
> +void* operator new(size_t, // { dg-bogus "first parameter" "" { xfail *-*-* } }
> + void void **p) noexcept // { dg-error "two or more" }
> +{
> + return p; // { dg-error "not declared" }
> +}
> +int x;
> +void f() {
> + int y;
> + new (&y) int(x);
> +}
> diff --git a/gcc/testsuite/g++.dg/init/new53.C b/gcc/testsuite/g++.dg/init/new53.C
> new file mode 100644
> index 00000000000..93d49dbd6c1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/init/new53.C
> @@ -0,0 +1,8 @@
> +// Check that we reject operator new with no argument or non-size_t first
> +// argument.
> +// { dg-do "compile" }
> +
> +void* operator new(); // { dg-error "takes type .size_t." }
> +void* operator new(char); // { dg-error "takes type .size_t." }
> +void* operator new(char*); // { dg-error "takes type .size_t." }
> +void* operator new(char&); // { dg-error "takes type .size_t." }
@@ -2980,8 +2980,9 @@ std_placement_new_fn_p (tree alloc_fn)
if (DECL_NAMESPACE_SCOPE_P (alloc_fn))
{
tree first_arg = TREE_CHAIN (TYPE_ARG_TYPES (TREE_TYPE (alloc_fn)));
- if ((TREE_VALUE (first_arg) == ptr_type_node)
- && TREE_CHAIN (first_arg) == void_list_node)
+ if (first_arg
+ && (TREE_VALUE (first_arg) == ptr_type_node)
+ && (TREE_CHAIN (first_arg) == void_list_node))
return true;
}
return false;
new file mode 100644
@@ -0,0 +1,14 @@
+// PR c++/117101
+// { dg-do "compile" { target c++11 } }
+
+using size_t = decltype(sizeof(int));
+void* operator new(size_t, // { dg-bogus "first parameter" "" { xfail *-*-* } }
+ void void **p) noexcept // { dg-error "two or more" }
+{
+ return p; // { dg-error "not declared" }
+}
+int x;
+void f() {
+ int y;
+ new (&y) int(x);
+}
new file mode 100644
@@ -0,0 +1,8 @@
+// Check that we reject operator new with no argument or non-size_t first
+// argument.
+// { dg-do "compile" }
+
+void* operator new(); // { dg-error "takes type .size_t." }
+void* operator new(char); // { dg-error "takes type .size_t." }
+void* operator new(char*); // { dg-error "takes type .size_t." }
+void* operator new(char&); // { dg-error "takes type .size_t." }