[RFA] Fix PR gdb/11702, printing of static const member variables

[dje at google dot com (Doug Evans)]

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  <dje@google.com>

	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;