From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27218 invoked by alias); 27 Jun 2010 18:25:03 -0000 Received: (qmail 27193 invoked by uid 22791); 27 Jun 2010 18:25:02 -0000 X-SWARE-Spam-Status: No, hits=-1.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KAM_STOCKGEN,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 27 Jun 2010 18:24:57 +0000 Received: from hpaq1.eem.corp.google.com (hpaq1.eem.corp.google.com [172.25.149.1]) by smtp-out.google.com with ESMTP id o5RIOisk025184; Sun, 27 Jun 2010 11:24:45 -0700 Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.18.118.116]) by hpaq1.eem.corp.google.com with ESMTP id o5RIOhWc018250; Sun, 27 Jun 2010 11:24:43 -0700 Received: by ruffy.mtv.corp.google.com (Postfix, from userid 67641) id AF5CA84613; Sun, 27 Jun 2010 11:24:42 -0700 (PDT) To: gdb-patches@sourceware.org cc: jan.kratochvil@redhat.com Subject: [RFA] Fix PR gdb/11702, printing of static const member variables Message-Id: <20100627182442.AF5CA84613@ruffy.mtv.corp.google.com> Date: Sun, 27 Jun 2010 18:25:00 -0000 From: dje@google.com (Doug Evans) X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-06/txt/msg00622.txt.bz2 Hi. This patch fixes http://sourceware.org/bugzilla/show_bug.cgi?id=11702 btw, the dwarf4 standard, as I read it, says static member variables are identified by having DW_AT_external. [4.1 Data Object Entries] However, dwarf2_add_field is calling die_is_declaration. This seems wrong, but I didn't address this here, other than to add a note pointing this out (because some new code in new_symbol should match). Also, I added a case to new_symbol to handle DW_TAG_member. gcc uses DW_TAG_variable but the correct way AIUI is DW_TAG_member, so I added a case to new_symbol to handle it. I'm not sure I like putting it with DW_TAG_variable, but I assumed y'all would prefer the absence of code duplication. If you want me to make it a separate case, let me know. [Another way to go would be to try to use some code from the DW_TAG_enumerator case - but since dwarf2_add_field would be the caller for DW_TAG_variable and DW_TAG_member, I went with the current patch.] There is one outstanding issue. "info var static_const_member" should work, I *think*. Is that true? If so, then more work needs to be done (or even a different approach, but the current patch seems reasonable). symtab.c:search_symbols has this: if (file_matches (real_symtab->filename, files, nfiles) && ((regexp == NULL || re_exec (SYMBOL_NATURAL_NAME (sym)) != 0) && ((kind == VARIABLES_DOMAIN && SYMBOL_CLASS (sym) != LOC_TYPEDEF && SYMBOL_CLASS (sym) != LOC_UNRESOLVED && SYMBOL_CLASS (sym) != LOC_BLOCK >>>> && SYMBOL_CLASS (sym) != LOC_CONST) || (kind == FUNCTIONS_DOMAIN && SYMBOL_CLASS (sym) == LOC_BLOCK) || (kind == TYPES_DOMAIN && SYMBOL_CLASS (sym) == LOC_TYPEDEF)))) This prevents static const members from being found using the current implementation. Why do we care about LOC_CONST here? [And, if we do, this also needs to check LOC_CONST_BYTES ...] Comments? Ok to check in? 2010-06-27 Doug Evans PR gdb/11702 * NEWS: Add entry. * dwarf2read.c (dwarf2_add_field): If DW_AT_const_value is present, create a symbol for the field and record the value. (new_symbol): Handle DW_TAG_member. * gdbtypes.c (field_is_static): Remove FIXME. testsuite/ * gdb.cp/m-static.exp: Add testcase. * gdb.cp/m-static.h (gnu_obj_4): Add initialized static const member. Index: NEWS =================================================================== RCS file: /cvs/src/src/gdb/NEWS,v retrieving revision 1.386 diff -u -p -r1.386 NEWS --- NEWS 25 Jun 2010 18:19:31 -0000 1.386 +++ NEWS 27 Jun 2010 17:09:16 -0000 @@ -32,6 +32,11 @@ GDB now also supports proper overload resolution for all the previously mentioned flavors of operators. + ** static const class members + + Printing of static const class members that are initialized in the + class definition has been fixed. + * Windows Thread Information Block access. On Windows targets, GDB now supports displaying the Windows Thread Index: dwarf2read.c =================================================================== RCS file: /cvs/src/src/gdb/dwarf2read.c,v retrieving revision 1.402 diff -u -p -r1.402 dwarf2read.c --- dwarf2read.c 21 Jun 2010 19:49:19 -0000 1.402 +++ dwarf2read.c 27 Jun 2010 17:35:20 -0000 @@ -4518,6 +4518,11 @@ dwarf2_add_field (struct field_info *fip fp = &new_field->field; + /* NOTE: According to the dwarf standard, static data members are + indicated by having DW_AT_external. + The check here for ! die_is_declaration is historical. + This test is replicated in new_symbol. */ + if (die->tag == DW_TAG_member && ! die_is_declaration (die, cu)) { /* Data member other than a C++ static data member. */ @@ -4625,7 +4630,7 @@ dwarf2_add_field (struct field_info *fip is a declaration, but all versions of G++ as of this writing (so through at least 3.2.1) incorrectly generate DW_TAG_variable tags. */ - + char *physname; /* Get name of field. */ @@ -4633,6 +4638,14 @@ dwarf2_add_field (struct field_info *fip if (fieldname == NULL) return; + attr = dwarf2_attr (die, DW_AT_const_value, cu); + if (attr) + { + /* A static const member, not much different than an enum as far as + we're concerned, except that we can support more types. */ + new_symbol (die, NULL, cu); + } + /* Get physical name. */ physname = (char *) dwarf2_physname (fieldname, die, cu); @@ -8753,6 +8766,7 @@ new_symbol (struct die_info *die, struct BLOCK_FUNCTION from the blockvector. */ break; case DW_TAG_variable: + case DW_TAG_member: /* Compilation with minimal debug info may result in variables with missing type entries. Change the misleading `void' type to something sensible. */ @@ -8761,6 +8775,17 @@ new_symbol (struct die_info *die, struct = objfile_type (objfile)->nodebug_data_symbol; attr = dwarf2_attr (die, DW_AT_const_value, cu); + /* In the case of DW_TAG_member, we should only be called for + static const members. */ + if (die->tag == DW_TAG_member) + { + /* NOTE: This test seems wrong according to the dwarf standard. + static data members are represented by DW_AT_external. + However, dwarf2_add_field is currently calling + die_is_declaration to check, so we do the same. */ + gdb_assert (die_is_declaration (die, cu)); + gdb_assert (attr); + } if (attr) { dwarf2_const_value (attr, sym, cu); Index: gdbtypes.c =================================================================== RCS file: /cvs/src/src/gdb/gdbtypes.c,v retrieving revision 1.192 diff -u -p -r1.192 gdbtypes.c --- gdbtypes.c 14 May 2010 20:17:37 -0000 1.192 +++ gdbtypes.c 27 Jun 2010 17:09:16 -0000 @@ -2511,9 +2511,7 @@ field_is_static (struct field *f) to the address of the enclosing struct. It would be nice to have a dedicated flag that would be set for static fields when the type is being created. But in practice, checking the field - loc_kind should give us an accurate answer (at least as long as - we assume that DWARF block locations are not going to be used - for static fields). FIXME? */ + loc_kind should give us an accurate answer. */ return (FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSNAME || FIELD_LOC_KIND (*f) == FIELD_LOC_KIND_PHYSADDR); } Index: testsuite/gdb.cp/m-static.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/m-static.exp,v retrieving revision 1.12 diff -u -p -r1.12 m-static.exp --- testsuite/gdb.cp/m-static.exp 27 Jun 2010 17:19:54 -0000 1.12 +++ testsuite/gdb.cp/m-static.exp 27 Jun 2010 17:20:29 -0000 @@ -125,6 +125,9 @@ gdb_test "print test4.elsewhere" "\\$\[0 # static const int that nobody initializes. From PR gdb/635. gdb_test "print test4.nowhere" "field nowhere is nonexistent or has been optimized out" "static const int initialized nowhere" +# static const int initialized in the class definition, PR gdb/11702. +gdb_test "print test4.everywhere" "\\$\[0-9\].* = 317" "static const int initialized in class definition" + # Perhaps at some point test4 should also include a test for a static # const int that was initialized in the header file. But I'm not sure # that GDB's current behavior in such situations is either consistent Index: testsuite/gdb.cp/m-static.h =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/m-static.h,v retrieving revision 1.2 diff -u -p -r1.2 m-static.h --- testsuite/gdb.cp/m-static.h 5 May 2006 18:04:09 -0000 1.2 +++ testsuite/gdb.cp/m-static.h 27 Jun 2010 17:20:29 -0000 @@ -5,8 +5,7 @@ class gnu_obj_4 public: static const int elsewhere; static const int nowhere; - // At some point, perhaps: - // static const int everywhere = 317; + static const int everywhere = 317; // try to ensure test4 is actually allocated int dummy;