diff mbox

[C++] For -gdwarf-5 emit DW_TAG_variable instead of DW_TAG_member for C++ static data members

Message ID 20170217185230.GQ1849@tucnak
State New
Headers show

Commit Message

Jakub Jelinek Feb. 17, 2017, 6:52 p.m. UTC 
Hi!

DWARF5 that has been released recently had a fairly late change where
it says that C++ static data members should use DW_TAG_variable tag
instead of DW_TAG_member inside of the class DIE.

The following patch implements just that, not any further changes e.g.
to inline static data members (i.e. it still emits a declaration in
the class and specification right after it, just both will now be
DW_TAG_variable instead DW_TAG_member followed by DW_TAG_variable).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-02-17  Jakub Jelinek  <jakub@redhat.com>

	* dwarf2out.c (add_linkage_name): Handle DW_TAG_variable with
	class_scope_p parent like DW_TAG_member.
	(gen_variable_die): For -gdwarf-5, use DW_TAG_variable instead of
	DW_TAG_member for static data member declarations.
	(gen_member_die): For -gdwarf-5 don't change DW_TAG_variable
	to DW_TAG_member.


	Jakub

Comments

Jason Merrill Feb. 18, 2017, 2:37 a.m. UTC | #1 
On Fri, Feb 17, 2017 at 1:52 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> -      && die->die_tag != DW_TAG_member)
> +      && die->die_tag != DW_TAG_member
> +      && (die->die_tag != DW_TAG_variable || !class_scope_p (die->die_parent)))

How about we only check class_scope_p (die->die_parent), and don't
consider the TAG at all?  DW_TAG_member should only appear at class
scope.

> -         if (old_die->die_tag == DW_TAG_member)
> +         if (old_die->die_tag == DW_TAG_member
> +             || (dwarf_version >= 5 && class_scope_p (old_die->die_parent)))

Likewise here.

Jason
Jakub Jelinek Feb. 18, 2017, 8:17 a.m. UTC | #2 
On Fri, Feb 17, 2017 at 09:37:09PM -0500, Jason Merrill wrote:
> On Fri, Feb 17, 2017 at 1:52 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > -      && die->die_tag != DW_TAG_member)
> > +      && die->die_tag != DW_TAG_member
> > +      && (die->die_tag != DW_TAG_variable || !class_scope_p (die->die_parent)))
> 
> How about we only check class_scope_p (die->die_parent), and don't
> consider the TAG at all?  DW_TAG_member should only appear at class
> scope.

That wouldn't work, because that would avoid adding linkage attributes to
methods.  I could use:
      && (die->die_tag == DW_TAG_subprogram || !class_scope_p (die->die_parent))
(not 100% sure if some other tag than those can make it here)
or
      && ((die->die_tag != DW_TAG_member || die->die_tag != DW_TAG_variable)
	  || !class_scope_p (die->die_parent))
or just use
      && (!VAR_P (decl) || !class_scope_p (die->die_parent))
or similar.

> > -         if (old_die->die_tag == DW_TAG_member)
> > +         if (old_die->die_tag == DW_TAG_member
> > +             || (dwarf_version >= 5 && class_scope_p (old_die->die_parent)))
> 
> Likewise here.

This spot probably can be changed as you wrote, it is in gen_variable_die,
so methods shouldn't appear there.

	Jakub
Jason Merrill Feb. 21, 2017, 8:37 p.m. UTC | #3 
On Sat, Feb 18, 2017 at 12:17 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Feb 17, 2017 at 09:37:09PM -0500, Jason Merrill wrote:
>> On Fri, Feb 17, 2017 at 1:52 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > -      && die->die_tag != DW_TAG_member)
>> > +      && die->die_tag != DW_TAG_member
>> > +      && (die->die_tag != DW_TAG_variable || !class_scope_p (die->die_parent)))
>>
>> How about we only check class_scope_p (die->die_parent), and don't
>> consider the TAG at all?  DW_TAG_member should only appear at class
>> scope.
>
> That wouldn't work, because that would avoid adding linkage attributes to
> methods.  I could use:
>       && (die->die_tag == DW_TAG_subprogram || !class_scope_p (die->die_parent))
> (not 100% sure if some other tag than those can make it here)
> or
>       && ((die->die_tag != DW_TAG_member || die->die_tag != DW_TAG_variable)
>           || !class_scope_p (die->die_parent))
> or just use
>       && (!VAR_P (decl) || !class_scope_p (die->die_parent))
> or similar.
>
>> > -         if (old_die->die_tag == DW_TAG_member)
>> > +         if (old_die->die_tag == DW_TAG_member
>> > +             || (dwarf_version >= 5 && class_scope_p (old_die->die_parent)))
>>
>> Likewise here.
>
> This spot probably can be changed as you wrote, it is in gen_variable_die,
> so methods shouldn't appear there.

Hmm, let's think about the behavior we want here.  I don't see any
reason not to put AT_linkage_name on a member DW_TAG_variable; I think
the old behavior avoided putting it on DW_TAG_member just because it
isn't defined for DW_TAG_member.  So it's not clear to me that we need
any changes in add_linkage_name or its call site.

Jason
diff mbox

Patch

--- gcc/dwarf2out.c.jj	2017-02-17 11:56:13.000000000 +0100
+++ gcc/dwarf2out.c	2017-02-17 16:28:12.336632166 +0100
@@ -20226,7 +20226,8 @@  add_linkage_name (dw_die_ref die, tree d
       && VAR_OR_FUNCTION_DECL_P (decl)
       && TREE_PUBLIC (decl)
       && !(VAR_P (decl) && DECL_REGISTER (decl))
-      && die->die_tag != DW_TAG_member)
+      && die->die_tag != DW_TAG_member
+      && (die->die_tag != DW_TAG_variable || !class_scope_p (die->die_parent)))
     add_linkage_name_raw (die, decl);
 }
 
@@ -22818,9 +22819,10 @@  gen_variable_die (tree decl, tree origin
     }
 
   /* For static data members, the declaration in the class is supposed
-     to have DW_TAG_member tag; the specification should still be
-     DW_TAG_variable referencing the DW_TAG_member DIE.  */
-  if (declaration && class_scope_p (context_die))
+     to have DW_TAG_member tag in DWARF{3,4} and we emit it for compatibility
+     also in DWARF2; the specification should still be DW_TAG_variable
+     referencing the DW_TAG_member DIE.  */
+  if (declaration && class_scope_p (context_die) && dwarf_version < 5)
     var_die = new_die (DW_TAG_member, context_die, decl);
   else
     var_die = new_die (DW_TAG_variable, context_die, decl);
@@ -22858,7 +22860,8 @@  gen_variable_die (tree decl, tree origin
 		  != (unsigned) s.column))
 	    add_AT_unsigned (var_die, DW_AT_decl_column, s.column);
 
-	  if (old_die->die_tag == DW_TAG_member)
+	  if (old_die->die_tag == DW_TAG_member
+	      || (dwarf_version >= 5 && class_scope_p (old_die->die_parent)))
 	    add_linkage_name (var_die, decl);
 	}
     }
@@ -24089,7 +24092,8 @@  gen_member_die (tree type, dw_die_ref co
 	      && get_AT (child, DW_AT_specification) == NULL)
 	    {
 	      reparent_child (child, context_die);
-	      child->die_tag = DW_TAG_member;
+	      if (dwarf_version < 5)
+		child->die_tag = DW_TAG_member;
 	    }
 	  else
 	    splice_child_die (context_die, child);
@@ -24111,7 +24115,7 @@  gen_member_die (tree type, dw_die_ref co
 	}
 
       /* For C++ inline static data members emit immediately a DW_TAG_variable
-	 DIE that will refer to that DW_TAG_member through
+	 DIE that will refer to that DW_TAG_member/DW_TAG_variable through
 	 DW_AT_specification.  */
       if (TREE_STATIC (member)
 	  && (lang_hooks.decls.decl_dwarf_attribute (member, DW_AT_inline)