diff mbox

Fix up DW_AT_accessibility (PR debug/45124)

Message ID 20100729130932.GF18378@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek July 29, 2010, 1:09 p.m. UTC 
Hi!

GCC (and GDB too) currently assume that DW_AT_acessibility
default is DW_ACCESS_private in DW_TAG_inheritance and
DW_ACCESS_public otherwise.

But that's not what the standard says, it says (at least for
DW_TAG_inheritance/DW_TAG_member/DW_TAG_subprogram) that
accessibility default is private in classes and public
in structs/unions/interfaces.

I've raised the issue on dwarf-discuss whether the same
applies to other DIEs and whether its meaning is how I read it
(namely, in immediate children of DW_TAG_class_type
default is DW_ACCESS_private, in immediate children
of DW_TAG_{structure,union,interface}_type default is
DW_ACCESS_public and in children of other DIEs it makes no sense.

For compatibility with GDB, I believe all we can do is add more
DW_AT_accessibility attributes and keep emitting where we were (uselessly)
emitting before.

Here is a patch that implements that.

2010-07-29  Jakub Jelinek  <jakub@redhat.com>

	PR debug/45124
	* dwarf2out.c (add_accessibility_attribute): Add DW_AT_accessibility
	DW_ACCESS_public for public children of DW_TAG_class_type.
	(gen_inheritance_die): Add DW_AT_accessibility DW_ACCESS_private
	for private children of DIEs other than DW_TAG_class_type.



	Jakub

Comments

Jason Merrill July 29, 2010, 2:01 p.m. UTC | #1 
On 07/29/2010 09:09 AM, Jakub Jelinek wrote:
> For compatibility with GDB, I believe all we can do is add more
> DW_AT_accessibility attributes and keep emitting where we were (uselessly)
> emitting before.

Is that really necessary?  What does GDB do with these attributes?

Jason
Tom Tromey July 29, 2010, 4:11 p.m. UTC | #2 
>>>>> "Jason" == Jason Merrill <jason@redhat.com> writes:

Jakub> For compatibility with GDB, I believe all we can do is add more
Jakub> DW_AT_accessibility attributes and keep emitting where we were
Jakub> (uselessly) emitting before.

Jason> Is that really necessary?  What does GDB do with these attributes?

I believe GDB only uses them when printing, to show the accessibility of
fields in a type.

Tom
diff mbox

Patch

--- gcc/dwarf2out.c.jj	2010-07-29 10:21:15.000000000 +0200
+++ gcc/dwarf2out.c	2010-07-29 10:53:49.000000000 +0200
@@ -15834,10 +15834,16 @@  add_AT_location_description (dw_die_ref 
 static void
 add_accessibility_attribute (dw_die_ref die, tree decl)
 {
+  /* In DW_TAG_class_type children DW_ACCESS_private is the default
+     instead of DW_ACCESS_public.  Emit DW_ACCESS_private anyway
+     for compatibility.  */
   if (TREE_PROTECTED (decl))
     add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_protected);
   else if (TREE_PRIVATE (decl))
     add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_private);
+  else if (die->die_parent
+	   && die->die_parent->die_tag == DW_TAG_class_type)
+    add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_public);
 }
 
 /* Attach the specialized form of location attribute used for data members of
@@ -19567,10 +19573,15 @@  gen_inheritance_die (tree binfo, tree ac
   if (BINFO_VIRTUAL_P (binfo))
     add_AT_unsigned (die, DW_AT_virtuality, DW_VIRTUALITY_virtual);
 
+  /* DW_ACCESS_private is the default accessibility only in DW_TAG_class_type
+     children, but for compatibility emit DW_ACCESS_public here even
+     when it is the default.  */
   if (access == access_public_node)
     add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_public);
   else if (access == access_protected_node)
     add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_protected);
+  else if (context_die->die_tag != DW_TAG_class_type)
+    add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_private);
 }
 
 /* Generate a DIE for a class member.  */