| Message ID | 20100729202053.GO18378@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/29/2010 01:20 PM, Jakub Jelinek wrote: > + /* In DWARF3+ the default is DW_ACCESS_private only in DW_TAG_class_type > + children, otherwise the default is DW_ACCESS_public. In DWARF2 > + the default has always been DW_ACCESS_public. */ > 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); > + { > + if (dwarf_version == 2 > + || die->die_parent == NULL > + || die->die_parent->die_tag != DW_TAG_class_type > + || (die->die_tag != DW_TAG_member > + && die->die_tag != DW_TAG_subprogram)) > + add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_private); > + } > + else if (dwarf_version > 2 > + && die->die_parent > + && die->die_parent->die_tag == DW_TAG_class_type > + && (die->die_tag == DW_TAG_member > + || die->die_tag == DW_TAG_subprogram)) > + add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_public); Your code matches the comment, but the comment is wrong: DWARF2 default is private. You said it correctly in the email, but not in the code. r~
On Fri, Jul 30, 2010 at 11:33:51AM -0700, Richard Henderson wrote: > On 07/29/2010 01:20 PM, Jakub Jelinek wrote: > > + /* In DWARF3+ the default is DW_ACCESS_private only in DW_TAG_class_type > > + children, otherwise the default is DW_ACCESS_public. In DWARF2 > > + the default has always been DW_ACCESS_public. */ > > 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); > > + { > > + if (dwarf_version == 2 > > + || die->die_parent == NULL > > + || die->die_parent->die_tag != DW_TAG_class_type > > + || (die->die_tag != DW_TAG_member > > + && die->die_tag != DW_TAG_subprogram)) > > + add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_private); > > + } > > + else if (dwarf_version > 2 > > + && die->die_parent > > + && die->die_parent->die_tag == DW_TAG_class_type > > + && (die->die_tag == DW_TAG_member > > + || die->die_tag == DW_TAG_subprogram)) > > + add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_public); > > Your code matches the comment, but the comment is wrong: DWARF2 default is private. DWARF2 default for DW_TAG_inheritance if DW_AT_accessibility is missing is private, yes (and the other hunk which handles DW_TAG_inheritance implements that). DWARF2 doesn't seem to talk at all about default for DW_TAG_member or DW_TAG_subprogram, so the patch above just kept status quo in that case for DWARF2 and only changed to what DWARF3 says for dwarf_version > 2. Other options are emit DW_AT_accessibility always for DW_TAG_member/DW_TAG_subprogram for dwarf_version == 2, or pretend the DWARF3+ rules (private in DW_TAG_class_type, public otherwise) apply even for DWARF2. Not sure what is best. Jakub
On 07/29/2010 04:20 PM, Jakub Jelinek wrote: > Actually, given the response on Dwarf-Discuss, I believe we should > limit this only to DW_TAG_member and DW_TAG_subprogram (to match the spec; > DW_TAG_inheritance handled in the other routine), as for say types and other > children of DW_TAG_class_type no DECL_PRIVATE doesn't mean we need to emit > DW_AT_accessibility DW_ACCESS_public, but instead just that accessibility > is not meaningful for it. All class members have accessibility; the accessibility of a nested type or enumerator has the same meaning as for a data or function member. > DWARF2 default for DW_TAG_inheritance if DW_AT_accessibility is missing is > private, yes (and the other hunk which handles DW_TAG_inheritance implements > that). > DWARF2 doesn't seem to talk at all about default for DW_TAG_member or > DW_TAG_subprogram, so the patch above just kept status quo in that case for > DWARF2 and only changed to what DWARF3 says for dwarf_version > 2. > Other options are emit DW_AT_accessibility always for > DW_TAG_member/DW_TAG_subprogram for dwarf_version == 2, or pretend the > DWARF3+ rules (private in DW_TAG_class_type, public otherwise) apply even > for DWARF2. Not sure what is best. Sticking with the status quo for DWARF2 is fine. Jason
On Fri, Sep 17, 2010 at 10:05:18AM -0400, Jason Merrill wrote: > On 07/29/2010 04:20 PM, Jakub Jelinek wrote: > >Actually, given the response on Dwarf-Discuss, I believe we should > >limit this only to DW_TAG_member and DW_TAG_subprogram (to match the spec; > >DW_TAG_inheritance handled in the other routine), as for say types and other > >children of DW_TAG_class_type no DECL_PRIVATE doesn't mean we need to emit > >DW_AT_accessibility DW_ACCESS_public, but instead just that accessibility > >is not meaningful for it. > > All class members have accessibility; the accessibility of a nested > type or enumerator has the same meaning as for a data or function > member. So, should GCC assume for DWARF3+ that the default is always what the language (C++ in this case) says is the default (and adjust the standard)? > >DWARF2 default for DW_TAG_inheritance if DW_AT_accessibility is missing is > >private, yes (and the other hunk which handles DW_TAG_inheritance implements > >that). > >DWARF2 doesn't seem to talk at all about default for DW_TAG_member or > >DW_TAG_subprogram, so the patch above just kept status quo in that case for > >DWARF2 and only changed to what DWARF3 says for dwarf_version > 2. > >Other options are emit DW_AT_accessibility always for > >DW_TAG_member/DW_TAG_subprogram for dwarf_version == 2, or pretend the > >DWARF3+ rules (private in DW_TAG_class_type, public otherwise) apply even > >for DWARF2. Not sure what is best. > > Sticking with the status quo for DWARF2 is fine. Ok. Jakub class A { typedef int AD1; enum AD2 { ad21, ad22 }; struct AD3 { typedef int C1; C1 i; }; class AD4 { typedef int C2; C2 i; }; int AD5; static int AD6; int AD7 () {} static int AD8 () {} private: typedef int AP1; enum AP2 { ap21, ap22 }; struct AP3 { typedef int C3; C3 i; }; class AP4 { typedef int C4; C4 i; }; int AP5; static int AP6; int AP7 () {} static int AP8 () {} protected: typedef int AR1; enum AR2 { ar21, ar22 }; struct AR3 { typedef int C5; C5 i; }; class AR4 { typedef int C6; C6 i; }; int AR5; static int AR6; int AR7 () {} static int AR8 () {} public: typedef int AU1; enum AU2 { au21, au22 }; struct AU3 { typedef int C7; C7 i; }; class AU4 { typedef int C8; C8 i; }; int AU5; static int AU6; int AU7 () {} static int AU8 () {} public: AD1 ad1; AD2 ad2; AP1 ap1; AP2 ap2; AR1 ar1; AR2 ar2; AU1 au1; AU2 au2; AD3 ad3; AD4 ad4; AP3 ap3; AP4 ap4; AR3 ar3; AR4 ar4; AU3 au3; AU4 au4; }; struct B { typedef int BD1; enum BD2 { bd21, bd22 }; struct BD3 { typedef int C9; C9 i; }; class BD4 { typedef int C10; C10 i; }; int BD5; static int BD6; int BD7 () {} static int BD8 () {} private: typedef int BP1; enum BP2 { bp21, bp22 }; struct BP3 { typedef int C11; C11 i; }; class BP4 { typedef int C12; C12 i; }; int BP5; static int BP6; int BP7 () {} static int BP8 () {} protected: typedef int BR1; enum BR2 { br21, br22 }; struct BR3 { typedef int C13; C13 i; }; class BR4 { typedef int C14; C14 i; }; int BR5; static int BR6; int BR7 () {} static int BR8 () {} public: typedef int BU1; enum BU2 { bu21, bu22 }; struct BU3 { typedef int C15; C15 i; }; class BU4 { typedef int C16; C16 i; }; int BU5; static int BU6; int BU7 () {} static int BU8 () {} public: BD1 bd1; BD2 bd2; BP1 bp1; BP2 bp2; BR1 br1; BR2 br2; BU1 bu1; BU2 bu2; BD3 bd3; BD4 bd4; BP3 bp3; BP4 bp4; BR3 br3; BR4 br4; BU3 bu3; BU4 bu4; }; A a; B b;
On 09/17/2010 01:44 PM, Jakub Jelinek wrote: > So, should GCC assume for DWARF3+ that the default is always what the > language (C++ in this case) says is the default (and adjust the standard)? Yes. Jason
--- gcc/dwarf2out.c.jj 2010-07-29 21:49:03.827604473 +0200 +++ gcc/dwarf2out.c 2010-07-29 22:14:28.231354703 +0200 @@ -15834,10 +15834,26 @@ add_AT_location_description (dw_die_ref static void add_accessibility_attribute (dw_die_ref die, tree decl) { + /* In DWARF3+ the default is DW_ACCESS_private only in DW_TAG_class_type + children, otherwise the default is DW_ACCESS_public. In DWARF2 + the default has always been DW_ACCESS_public. */ 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); + { + if (dwarf_version == 2 + || die->die_parent == NULL + || die->die_parent->die_tag != DW_TAG_class_type + || (die->die_tag != DW_TAG_member + && die->die_tag != DW_TAG_subprogram)) + add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_private); + } + else if (dwarf_version > 2 + && die->die_parent + && die->die_parent->die_tag == DW_TAG_class_type + && (die->die_tag == DW_TAG_member + || die->die_tag == DW_TAG_subprogram)) + add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_public); } /* Attach the specialized form of location attribute used for data members of @@ -19567,10 +19583,20 @@ gen_inheritance_die (tree binfo, tree ac if (BINFO_VIRTUAL_P (binfo)) add_AT_unsigned (die, DW_AT_virtuality, DW_VIRTUALITY_virtual); + /* In DWARF3+ the default is DW_ACCESS_private only in DW_TAG_class_type + children, otherwise the default is DW_ACCESS_public. In DWARF2 + the default has always been DW_ACCESS_private. */ if (access == access_public_node) - add_AT_unsigned (die, DW_AT_accessibility, DW_ACCESS_public); + { + if (dwarf_version == 2 + || context_die->die_tag == DW_TAG_class_type) + 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 (dwarf_version > 2 + && 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. */
On Thu, Jul 29, 2010 at 01:13:25PM -0400, Jason Merrill wrote: > OK. Actually, given the response on Dwarf-Discuss, I believe we should limit this only to DW_TAG_member and DW_TAG_subprogram (to match the spec; DW_TAG_inheritance handled in the other routine), as for say types and other children of DW_TAG_class_type no DECL_PRIVATE doesn't mean we need to emit DW_AT_accessibility DW_ACCESS_public, but instead just that accessibility is not meaningful for it. 2010-07-29 Jakub Jelinek <jakub@redhat.com> PR debug/45124 * dwarf2out.c (add_accessibility_attribute): Assume DW_ACCESS_private as the default for dwarf_version > 2 and DW_TAG_class_type parent. (gen_inheritance_die): Assume DW_ACCESS_public as the default for dwarf_version > 2 and parent other than DW_TAG_class_type. Jakub