RFA (symtab): PATCH to symtab_node::nonzero_address DECL_COMDAT handling for c++/80485

Message ID CADzB+2nDLRbDS2p6gZGuTS0vygYOpVkJkRRsT1sStcsTcmfG-A@mail.gmail.com
State New
Headers show
Series
  • RFA (symtab): PATCH to symtab_node::nonzero_address DECL_COMDAT handling for c++/80485
Related show

Commit Message

Jason Merrill May 19, 2018, 1:07 p.m.
A comment earlier in in nonzero_address says, "Important case of WEAK
we want to do well are comdats. Those are handled by later check for
definition."  But in this case we aren't handling this comdat function
well, we return false because it is DECL_WEAK and DECL_EXTERNAL
(because we aren't at EOF yet) and so we fail to fold the comparison.

This patch fixes the testcase by checking DECL_COMDAT directly.

Tested x86_64-pc-linux-gnu.  OK for trunk?

Comments

Jeff Law May 24, 2018, 9:36 p.m. | #1
On 05/19/2018 07:07 AM, Jason Merrill wrote:
> A comment earlier in in nonzero_address says, "Important case of WEAK

> we want to do well are comdats. Those are handled by later check for

> definition."  But in this case we aren't handling this comdat function

> well, we return false because it is DECL_WEAK and DECL_EXTERNAL

> (because we aren't at EOF yet) and so we fail to fold the comparison.

> 

> This patch fixes the testcase by checking DECL_COMDAT directly.

> 

> Tested x86_64-pc-linux-gnu.  OK for trunk?

> 

> 

> 80485.diff

> 

> 

> commit a1a0c12db660aa94a625771a9f2fa9db30a552fe

> Author: Jason Merrill <jason@redhat.com>

> Date:   Fri May 18 20:20:05 2018 -0400

> 

>             PR c++/80485 - inline function non-zero address.

>     

>             * symtab.c (nonzero_address): Check DECL_COMDAT.

OK
jeff
Jason Merrill June 5, 2018, 6:56 a.m. | #2
On Thu, May 24, 2018 at 11:36 PM, Jeff Law <law@redhat.com> wrote:
> On 05/19/2018 07:07 AM, Jason Merrill wrote:

>> A comment earlier in in nonzero_address says, "Important case of WEAK

>> we want to do well are comdats. Those are handled by later check for

>> definition."  But in this case we aren't handling this comdat function

>> well, we return false because it is DECL_WEAK and DECL_EXTERNAL

>> (because we aren't at EOF yet) and so we fail to fold the comparison.

>>

>> This patch fixes the testcase by checking DECL_COMDAT directly.

>>

>> Tested x86_64-pc-linux-gnu.  OK for trunk?

>>

>>

>> 80485.diff

>>

>>

>> commit a1a0c12db660aa94a625771a9f2fa9db30a552fe

>> Author: Jason Merrill <jason@redhat.com>

>> Date:   Fri May 18 20:20:05 2018 -0400

>>

>>             PR c++/80485 - inline function non-zero address.

>>

>>             * symtab.c (nonzero_address): Check DECL_COMDAT.

> OK


For GCC 8 as well?

Jason
Jeff Law June 11, 2018, 6:54 p.m. | #3
On 06/05/2018 12:56 AM, Jason Merrill wrote:
> On Thu, May 24, 2018 at 11:36 PM, Jeff Law <law@redhat.com> wrote:

>> On 05/19/2018 07:07 AM, Jason Merrill wrote:

>>> A comment earlier in in nonzero_address says, "Important case of WEAK

>>> we want to do well are comdats. Those are handled by later check for

>>> definition."  But in this case we aren't handling this comdat function

>>> well, we return false because it is DECL_WEAK and DECL_EXTERNAL

>>> (because we aren't at EOF yet) and so we fail to fold the comparison.

>>>

>>> This patch fixes the testcase by checking DECL_COMDAT directly.

>>>

>>> Tested x86_64-pc-linux-gnu.  OK for trunk?

>>>

>>>

>>> 80485.diff

>>>

>>>

>>> commit a1a0c12db660aa94a625771a9f2fa9db30a552fe

>>> Author: Jason Merrill <jason@redhat.com>

>>> Date:   Fri May 18 20:20:05 2018 -0400

>>>

>>>             PR c++/80485 - inline function non-zero address.

>>>

>>>             * symtab.c (nonzero_address): Check DECL_COMDAT.

>> OK

> 

> For GCC 8 as well?

Yea.
jeff

Patch

commit a1a0c12db660aa94a625771a9f2fa9db30a552fe
Author: Jason Merrill <jason@redhat.com>
Date:   Fri May 18 20:20:05 2018 -0400

            PR c++/80485 - inline function non-zero address.
    
            * symtab.c (nonzero_address): Check DECL_COMDAT.

diff --git a/gcc/symtab.c b/gcc/symtab.c
index c1533083573..faf0ccf3e61 100644
--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -1952,11 +1952,11 @@  symtab_node::nonzero_address ()
       return true;
     }
 
-  /* If target is defined and not extern, we know it will be output and thus
-     it will bind to non-NULL.
-     Play safe for flag_delete_null_pointer_checks where weak definition maye
+  /* If target is defined and either comdat or not extern, we know it will be
+     output and thus it will bind to non-NULL.
+     Play safe for flag_delete_null_pointer_checks where weak definition may
      be re-defined by NULL.  */
-  if (definition && !DECL_EXTERNAL (decl)
+  if (definition && (!DECL_EXTERNAL (decl) || DECL_COMDAT (decl))
       && (flag_delete_null_pointer_checks || !DECL_WEAK (decl)))
     {
       if (!DECL_WEAK (decl))
diff --git a/gcc/testsuite/g++.dg/expr/pmf-3.C b/gcc/testsuite/g++.dg/expr/pmf-3.C
new file mode 100644
index 00000000000..fac42fc23a5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/pmf-3.C
@@ -0,0 +1,15 @@ 
+// PR c++/80485
+// { dg-do compile { target c++11 } }
+
+struct dummy {
+  void nonnull() {};
+  void nonnull2();
+};
+
+typedef void (dummy::*safe_bool)();
+
+constexpr safe_bool a = &dummy::nonnull;
+constexpr safe_bool b = &dummy::nonnull2;
+
+static_assert( static_cast<bool>( a ), "" );
+static_assert( static_cast<bool>( b ), "" );