diff mbox series

c++: Fix crash during NRV optimization with invalid input [PR117099]

Message ID 0102019296d5f0d6-503071f0-4f6b-4ed0-80b6-9cbf5ebf7a6c-000000@eu-west-1.amazonses.com
State New
Headers show
Series c++: Fix crash during NRV optimization with invalid input [PR117099] | expand

Commit Message

Simon Martin Oct. 16, 2024, 7:37 p.m. UTC 
We ICE upon the following invalid code because we end up calling
finalize_nrv_r with a RETURN_EXPR with no operand.

=== cut here ===
struct X {
  ~X();
};
X test(bool b) {
  {
    X x;
    return x;
  }
  if (!(b)) return;
}
=== cut here ===

This patch fixes this by simply returning error_mark_node when detecting
a void return in a function returning non-void.

Successfully tested on x86_64-pc-linux-gnu.

	PR c++/117099

gcc/cp/ChangeLog:

	* typeck.cc (check_return_expr): Return error_mark_node upon
	void return for function returning non-void.

gcc/testsuite/ChangeLog:

	* g++.dg/parse/crash77.C: New test.

---
 gcc/cp/typeck.cc                     |  1 +
 gcc/testsuite/g++.dg/parse/crash77.C | 14 ++++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/parse/crash77.C

Comments

Sam James Oct. 16, 2024, 8:06 p.m. UTC | #1 
Simon Martin <simon@nasilyan.com> writes:

> We ICE upon the following invalid code because we end up calling
> finalize_nrv_r with a RETURN_EXPR with no operand.
> 
> === cut here ===
> struct X {
>   ~X();
> };
> X test(bool b) {
>   {
>     X x;
>     return x;
>   }
>   if (!(b)) return;
> }
> === cut here ===
> 
> This patch fixes this by simply returning error_mark_node when detecting
> a void return in a function returning non-void.
> 
> Successfully tested on x86_64-pc-linux-gnu.
> 
> 	PR c++/117099
> 
> gcc/cp/ChangeLog:
> 
> 	* typeck.cc (check_return_expr): Return error_mark_node upon
> 	void return for function returning non-void.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/parse/crash77.C: New test.
> 
> ---
>  gcc/cp/typeck.cc                     |  1 +
>  gcc/testsuite/g++.dg/parse/crash77.C | 14 ++++++++++++++
>  2 files changed, 15 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/parse/crash77.C
> 
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 71d879abef1..22a6ec9a185 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -11238,6 +11238,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
>  	 RETURN_EXPR to avoid control reaches end of non-void function
>  	 warnings in tree-cfg.cc.  */
>        *no_warning = true;
> +      return error_mark_node;
>      }
>    /* Check for a return statement with a value in a function that
>       isn't supposed to return a value.  */
> diff --git a/gcc/testsuite/g++.dg/parse/crash77.C b/gcc/testsuite/g++.dg/parse/crash77.C
> new file mode 100644
> index 00000000000..d3f0ae6a877
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/crash77.C
> @@ -0,0 +1,14 @@
> +// PR c++/117099
> +// { dg-compile }

dg-do compile

> +
> +struct X {
> +  ~X();
> +};
> +
> +X test(bool b) {
> +  {
> +    X x;
> +    return x;
> +  } 
> +  if (!(b)) return; // { dg-error "return-statement with no value" }
> +}
> -- 
> 2.44.0
> 

BTW, the line-endings on this seem a bit odd. Did you use git-send-email?
Simon Martin Oct. 17, 2024, 8:38 a.m. UTC | #2 
Hi Sam,

On 16 Oct 2024, at 22:06, Sam James wrote:

> Simon Martin <simon@nasilyan.com> writes:
>
>> We ICE upon the following invalid code because we end up calling
>> finalize_nrv_r with a RETURN_EXPR with no operand.
>>
>> === cut here ===
>> struct X {
>>   ~X();
>> };
>> X test(bool b) {
>>   {
>>     X x;
>>     return x;
>>   }
>>   if (!(b)) return;
>> }
>> === cut here ===
>>
>> This patch fixes this by simply returning error_mark_node when 
>> detecting
>> a void return in a function returning non-void.
>>
>> Successfully tested on x86_64-pc-linux-gnu.
>>
>> 	PR c++/117099
>>
>> gcc/cp/ChangeLog:
>>
>> 	* typeck.cc (check_return_expr): Return error_mark_node upon
>> 	void return for function returning non-void.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/parse/crash77.C: New test.
>>
>> ---
>>  gcc/cp/typeck.cc                     |  1 +
>>  gcc/testsuite/g++.dg/parse/crash77.C | 14 ++++++++++++++
>>  2 files changed, 15 insertions(+)
>>  create mode 100644 gcc/testsuite/g++.dg/parse/crash77.C
>>
>> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>> index 71d879abef1..22a6ec9a185 100644
>> --- a/gcc/cp/typeck.cc
>> +++ b/gcc/cp/typeck.cc
>> @@ -11238,6 +11238,7 @@ check_return_expr (tree retval, bool 
>> *no_warning, bool *dangling)
>>  	 RETURN_EXPR to avoid control reaches end of non-void function
>>  	 warnings in tree-cfg.cc.  */
>>        *no_warning = true;
>> +      return error_mark_node;
>>      }
>>    /* Check for a return statement with a value in a function that
>>       isn't supposed to return a value.  */
>> diff --git a/gcc/testsuite/g++.dg/parse/crash77.C 
>> b/gcc/testsuite/g++.dg/parse/crash77.C
>> new file mode 100644
>> index 00000000000..d3f0ae6a877
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/parse/crash77.C
>> @@ -0,0 +1,14 @@
>> +// PR c++/117099
>> +// { dg-compile }
>
> dg-do compile
>
Aarg, of course, thanks for spotting this! Fixed in the attached 
version.

>> +
>> +struct X {
>> +  ~X();
>> +};
>> +
>> +X test(bool b) {
>> +  {
>> +    X x;
>> +    return x;
>> +  }
>> +  if (!(b)) return; // { dg-error "return-statement with no value" }
>> +}
>> -- 
>> 2.44.0
>>
>
> BTW, the line-endings on this seem a bit odd. Did you use 
> git-send-email?
I did use git-send-email indeed. What oddities do you see with line 
endings?
cat -A over the patch file looks good.

Thanks, Simon
From 46fcb8cd0f89213b80f2815f6b7c2af064d7e86d Mon Sep 17 00:00:00 2001
From: Simon Martin <simon@nasilyan.com>
Date: Wed, 16 Oct 2024 15:47:12 +0200
Subject: [PATCH] c++: Fix crash during NRV optimization with invalid input [PR117099]

We ICE upon the following invalid code because we end up calling
finalize_nrv_r with a RETURN_EXPR with no operand.

=== cut here ===
struct X {
  ~X();
};
X test(bool b) {
  {
    X x;
    return x;
  }
  if (!(b)) return;
}
=== cut here ===

This patch fixes this by simply returning error_mark_node when detecting
a void return in a function returning non-void.

Successfully tested on x86_64-pc-linux-gnu.

	PR c++/117099

gcc/cp/ChangeLog:

	* typeck.cc (check_return_expr): Return error_mark_node upon
	void return for function returning non-void.

gcc/testsuite/ChangeLog:

	* g++.dg/parse/crash77.C: New test.

---
 gcc/cp/typeck.cc                     |  1 +
 gcc/testsuite/g++.dg/parse/crash77.C | 14 ++++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/parse/crash77.C

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 71d879abef1..22a6ec9a185 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11238,6 +11238,7 @@ check_return_expr (tree retval, bool *no_warning, bool *dangling)
 	 RETURN_EXPR to avoid control reaches end of non-void function
 	 warnings in tree-cfg.cc.  */
       *no_warning = true;
+      return error_mark_node;
     }
   /* Check for a return statement with a value in a function that
      isn't supposed to return a value.  */
diff --git a/gcc/testsuite/g++.dg/parse/crash77.C b/gcc/testsuite/g++.dg/parse/crash77.C
new file mode 100644
index 00000000000..912b997177c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/crash77.C
@@ -0,0 +1,14 @@
+// PR c++/117099
+// { dg-do compile }
+
+struct X {
+  ~X();
+};
+
+X test(bool b) {
+  {
+    X x;
+    return x;
+  } 
+  if (!(b)) return; // { dg-error "return-statement with no value" }
+}
Sam James Oct. 18, 2024, 8:55 a.m. UTC | #3 
Simon Martin <simon@nasilyan.com> writes:

> Hi Sam,

Hi Simon,

>
> On 16 Oct 2024, at 22:06, Sam James wrote:
>
>> Simon Martin <simon@nasilyan.com> writes:
>>
>>> We ICE upon the following invalid code because we end up calling
>>> finalize_nrv_r with a RETURN_EXPR with no operand.
>>>
>>> === cut here ===
>>> struct X {
>>>   ~X();
>>> };
>>> X test(bool b) {
>>>   {
>>>     X x;
>>>     return x;
>>>   }
>>>   if (!(b)) return;
>>> }
>>> === cut here ===
>>>
>>> This patch fixes this by simply returning error_mark_node when 
>>> detecting
>>> a void return in a function returning non-void.
>>>
>>> Successfully tested on x86_64-pc-linux-gnu.
>>>
>>> 	PR c++/117099
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* typeck.cc (check_return_expr): Return error_mark_node upon
>>> 	void return for function returning non-void.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/parse/crash77.C: New test.
>>>
>>> ---
>>>  gcc/cp/typeck.cc                     |  1 +
>>>  gcc/testsuite/g++.dg/parse/crash77.C | 14 ++++++++++++++
>>>  2 files changed, 15 insertions(+)
>>>  create mode 100644 gcc/testsuite/g++.dg/parse/crash77.C
>>>
>>> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
>>> index 71d879abef1..22a6ec9a185 100644
>>> --- a/gcc/cp/typeck.cc
>>> +++ b/gcc/cp/typeck.cc
>>> @@ -11238,6 +11238,7 @@ check_return_expr (tree retval, bool 
>>> *no_warning, bool *dangling)
>>>  	 RETURN_EXPR to avoid control reaches end of non-void function
>>>  	 warnings in tree-cfg.cc.  */
>>>        *no_warning = true;
>>> +      return error_mark_node;
>>>      }
>>>    /* Check for a return statement with a value in a function that
>>>       isn't supposed to return a value.  */
>>> diff --git a/gcc/testsuite/g++.dg/parse/crash77.C 
>>> b/gcc/testsuite/g++.dg/parse/crash77.C
>>> new file mode 100644
>>> index 00000000000..d3f0ae6a877
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/parse/crash77.C
>>> @@ -0,0 +1,14 @@
>>> +// PR c++/117099
>>> +// { dg-compile }
>>
>> dg-do compile
>>
> Aarg, of course, thanks for spotting this! Fixed in the attached 
> version.
>
>>> +
>>> +struct X {
>>> +  ~X();
>>> +};
>>> +
>>> +X test(bool b) {
>>> +  {
>>> +    X x;
>>> +    return x;
>>> +  }
>>> +  if (!(b)) return; // { dg-error "return-statement with no value" }
>>> +}
>>> -- 
>>> 2.44.0
>>>
>>
>> BTW, the line-endings on this seem a bit odd. Did you use 
>> git-send-email?
> I did use git-send-email indeed. What oddities do you see with line 
> endings?
> cat -A over the patch file looks good.
>

Weird -- if I open your original email in mu4e, I see a bunch of ^M at
the end of the lines.


> Thanks, Simon
>
> [2. text/plain; 0001-c-Fix-crash-during-NRV-optimization-with-invalid-inp.patch]...
diff mbox series

Patch

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 71d879abef1..22a6ec9a185 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11238,6 +11238,7 @@  check_return_expr (tree retval, bool *no_warning, bool *dangling)
 	 RETURN_EXPR to avoid control reaches end of non-void function
 	 warnings in tree-cfg.cc.  */
       *no_warning = true;
+      return error_mark_node;
     }
   /* Check for a return statement with a value in a function that
      isn't supposed to return a value.  */
diff --git a/gcc/testsuite/g++.dg/parse/crash77.C b/gcc/testsuite/g++.dg/parse/crash77.C
new file mode 100644
index 00000000000..d3f0ae6a877
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/crash77.C
@@ -0,0 +1,14 @@ 
+// PR c++/117099
+// { dg-compile }
+
+struct X {
+  ~X();
+};
+
+X test(bool b) {
+  {
+    X x;
+    return x;
+  } 
+  if (!(b)) return; // { dg-error "return-statement with no value" }
+}