[Google/main Patch] Cleanup pubnames/pubtypes and test-suite

The enclosed patch to google/main contains certain small fixes for pubnames and
pubtypes, which are now emitted completely and canonically. It also fixes the
expected output of various tests to match the canonical names of functions.

OK for google/main?


Tested:
	With full make-check and no new failures observed.

gcc/ChangeLog:

2012-01-04   Sterling Augustine  <saugustine@google.com>

	* gcc/dwarf2out.c (add_pubname): Move conditional clause from outer to
	inner if-statement.
	(dwarf2out_finish): Fix conditions to output DW_AT_GNU_pubnames and
	DW_AT_GNU_pubtypes.  Move decision to output pubnames and pubtypes from
	here...
	(output_pubnames): ...to here.
	(pubtypes_section_empty): Delete unused function.

testsuite/ChangeLog:

2012-01-04   Sterling Augustine  <saugustine@google.com>

	* gcc/testsuite/g++.dg/diagnostic/bindings1.C: Adjust expected output.
	* gcc/testsuite/g++.dg/ext/pretty3.C: Likewise.
	* gcc/testsuite/g++.dg/pr44486.C: Likewise.
	* gcc/testsuite/g++.dg/warn/Wuninitializable-member.C: Likewise.
	* gcc/testsuite/g++.dg/warn/pr35711.C: Likewise.
	* gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C: Likewise.

[saugustine, 2012-01-04 19:35:47]

The enclosed patch to google/main contains certain small fixes for pubnames and
pubtypes, which are now emitted completely and canonically. It also fixes the
expected output of various tests to match the canonical names of functions.

OK for google/main?


Tested:
	With full make-check and no new failures observed.

gcc/ChangeLog:

2012-01-04   Sterling Augustine  <saugustine@google.com>

	* gcc/dwarf2out.c (add_pubname): Move conditional clause from outer to
	inner if-statement.
	(dwarf2out_finish): Fix conditions to output DW_AT_GNU_pubnames and
	DW_AT_GNU_pubtypes.  Move decision to output pubnames and pubtypes from
	here...
	(output_pubnames): ...to here.
	(pubtypes_section_empty): Delete unused function.

gcc/testsuite/ChangeLog:

2012-01-04   Sterling Augustine  <saugustine@google.com>

	* gcc/testsuite/g++.dg/diagnostic/bindings1.C: Adjust expected output.
	* gcc/testsuite/g++.dg/ext/pretty3.C: Likewise.
	* gcc/testsuite/g++.dg/pr44486.C: Likewise.
	* gcc/testsuite/g++.dg/warn/Wuninitializable-member.C: Likewise.
	* gcc/testsuite/g++.dg/warn/pr35711.C: Likewise.
	* gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C: Likewise.

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 6512292..3788cf8 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -8659,9 +8659,7 @@ add_pubname_string (const char *str, dw_die_ref die)
 static void
 add_pubname (tree decl, dw_die_ref die)
 {
-  if (!GENERATE_MINIMUM_LINE_TABLE
-      && targetm.want_debug_pub_sections
-      && TREE_PUBLIC (decl))
+  if (!GENERATE_MINIMUM_LINE_TABLE && targetm.want_debug_pub_sections)
     {
       if ((TREE_PUBLIC (decl) && !is_class_die (die->die_parent))
           || is_cu_die (die->die_parent) || is_namespace_die (die->die_parent))
@@ -8756,10 +8754,18 @@ output_pubnames (VEC (pubname_entry, gc) * names)
   unsigned long pubnames_length = size_of_pubnames (names);
   pubname_ref pub;
 
+  if (!targetm.want_debug_pub_sections)
+    return;
   if (names == pubname_table)
-    ASM_OUTPUT_LABEL (asm_out_file, debug_pubnames_section_label);
+    {
+      switch_to_section (debug_pubnames_section);
+      ASM_OUTPUT_LABEL (asm_out_file, debug_pubnames_section_label);
+    }
   else
-    ASM_OUTPUT_LABEL (asm_out_file, debug_pubtypes_section_label);
+    {
+      switch_to_section (debug_pubtypes_section);
+      ASM_OUTPUT_LABEL (asm_out_file, debug_pubtypes_section_label);
+    }
   if (DWARF_INITIAL_LENGTH_SIZE - DWARF_OFFSET_SIZE == 4)
     dw2_asm_output_data (4, 0xffffffff,
       "Initial length escape value indicating 64-bit DWARF extension");
@@ -22462,29 +22468,6 @@ optimize_location_lists (dw_die_ref die)
 }
 
 
-/* Report if the pubtypes_section is either empty or will be pruned to
-   empty.  */
-
-static bool
-pubtypes_section_empty (void)
-{
-  if (!VEC_empty (pubname_entry, pubtype_table))
-    {
-      if (flag_eliminate_unused_debug_types)
-	{
-	  /* The pubtypes table might be emptied by pruning unused items.  */
-	  unsigned i;
-	  pubname_ref p;
-	  FOR_EACH_VEC_ELT (pubname_entry, pubtype_table, i, p)
-	    if (p->die->die_offset != 0)
-              return false;
-	}
-      return true;
-    }
-  return false;
-}
-
-
 /* Output stuff that dwarf requires at the end of every file,
    and generate the DWARF-2 debugging info.  */
 
@@ -22749,17 +22732,12 @@ dwarf2out_finish (const char *filename)
   htab_delete (comdat_type_table);
 
   /* Add the DW_AT_GNU_pubnames and DW_AT_GNU_pubtypes attributes.  */
-  if (!VEC_empty (pubname_entry, pubname_table))
+  if (targetm.want_debug_pub_sections)
     {
-      /* FIXME: Should use add_AT_pubnamesptr.  This works because
-         most targets don't care what the base section is.  */
+      /* FIXME: Should use add_AT_pubnamesptr.  This works because most targets
+         don't care what the base section is.  */
       add_AT_lineptr (comp_unit_die (), DW_AT_GNU_pubnames,
-		      debug_pubnames_section_label);
-    }
-  if (!pubtypes_section_empty ())
-    {
-      /* FIXME: Should use add_AT_pubtypesptr.  This works because
-         most targets don't care what the base section is.  */
+                      debug_pubnames_section_label);
       add_AT_lineptr (comp_unit_die (), DW_AT_GNU_pubtypes,
                       debug_pubtypes_section_label);
     }
@@ -22787,22 +22765,12 @@ dwarf2out_finish (const char *filename)
       output_location_lists (comp_unit_die ());
     }
 
-  /* Output public names table if necessary.  */
-  if (!VEC_empty (pubname_entry, pubname_table) && info_section_emitted)
-    {
-      switch_to_section (debug_pubnames_section);
-      output_pubnames (pubname_table);
-    }
-
-  /* Output public types table if necessary.  */
+  /* Output public names and types tables if necessary.  */
+  output_pubnames (pubname_table);
   /* ??? Only defined by DWARF3, but emitted by Darwin for DWARF2.
      It shouldn't hurt to emit it always, since pure DWARF2 consumers
      simply won't look for the section.  */
-  if (!pubtypes_section_empty () && info_section_emitted)
-    {
-      switch_to_section (debug_pubtypes_section);
-      output_pubnames (pubtype_table);
-    }
+  output_pubnames (pubtype_table);
 
   /* Output the address range information if a CU (.debug_info section)
      was emitted.  We output an empty table even if we had no functions
diff --git a/gcc/testsuite/g++.dg/diagnostic/bindings1.C
b/gcc/testsuite/g++.dg/diagnostic/bindings1.C
index 4972377..af2f5bd 100644
--- a/gcc/testsuite/g++.dg/diagnostic/bindings1.C
+++ b/gcc/testsuite/g++.dg/diagnostic/bindings1.C
@@ -10,7 +10,7 @@ struct x {typedef int type;};
 
 int main()
 {
-  if (strcmp (foo(x(), 3), "const char* foo(T, typename T::type) "
+  if (strcmp (foo(x(), 3), "char const* foo(T, typename T::type) "
 	      "[with T = x; typename T::type = int]") == 0)
     return 0;
   else 
diff --git a/gcc/testsuite/g++.dg/ext/pretty3.C
b/gcc/testsuite/g++.dg/ext/pretty3.C
index 01b1457..e59d77e 100644
--- a/gcc/testsuite/g++.dg/ext/pretty3.C
+++ b/gcc/testsuite/g++.dg/ext/pretty3.C
@@ -16,4 +16,4 @@ int main ()
 {
   printf ("%s\n", D<int>().foo (0));
 }
-// { dg-final { scan-assembler "const char\\* D<U>::foo\\(typename B<U>::X\\)"
} }
+// { dg-final { scan-assembler "char const\\* D<U>::foo\\(typename B<U>::X\\)"
} }
diff --git a/gcc/testsuite/g++.dg/pr44486.C b/gcc/testsuite/g++.dg/pr44486.C
index 01e8428..1a23d57 100644
--- a/gcc/testsuite/g++.dg/pr44486.C
+++ b/gcc/testsuite/g++.dg/pr44486.C
@@ -7,4 +7,4 @@ namespace { S f() { const char * s = __PRETTY_FUNCTION__; return
S(); } }
 
 int main() { f(); }
 
-// { dg-final { scan-assembler "S \{anonymous\}::f" } }
+// { dg-final { scan-assembler "S \\(anonymous namespace\\)::f" } }
diff --git a/gcc/testsuite/g++.dg/warn/Wuninitializable-member.C
b/gcc/testsuite/g++.dg/warn/Wuninitializable-member.C
index 1c37e3e..9d889e1 100644
--- a/gcc/testsuite/g++.dg/warn/Wuninitializable-member.C
+++ b/gcc/testsuite/g++.dg/warn/Wuninitializable-member.C
@@ -8,7 +8,7 @@ public:
 };
 
 class Y {
-  const int var;// { dg-warning "non-static const member 'const int Y::var' in
class without a constructor" }
+  const int var;// { dg-warning "non-static const member 'int const Y::var' in
class without a constructor" }
 public:
   int g(){ return 2*var; }
 };
diff --git a/gcc/testsuite/g++.dg/warn/pr35711.C
b/gcc/testsuite/g++.dg/warn/pr35711.C
index 653269c..7c1be1c 100644
--- a/gcc/testsuite/g++.dg/warn/pr35711.C
+++ b/gcc/testsuite/g++.dg/warn/pr35711.C
@@ -4,5 +4,5 @@
 
 int* foo (volatile int *p)
 {
-  return (int*)p; // { dg-warning "cast from type 'volatile int\\*' to type
'int\\*' casts away qualifiers" }
+  return (int*)p; // { dg-warning "cast from type 'int volatile\\*' to type
'int\\*' casts away qualifiers" }
 }
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C
b/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C
index 6dd4b54..6d7c7c4 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C
@@ -19,7 +19,7 @@ const char* S3<char>::h(int) { return __PRETTY_FUNCTION__; }
 int main()
 {
   if (strcmp (S3<double>::h(7), 
-	      "static const char* S3<T>::h(U) [with U = int; T = double]") == 0)
+	      "static char const* S3<T>::h(U) [with U = int; T = double]") == 0)
     return 0;
   else 
     return 1;

--
This patch is available for review at http://codereview.appspot.com/5514045

[Cary, 2012-01-04 20:06:58]

LGTM for google/main.

Before submitting for trunk, we'll want to take care of this FIXME:

> +      /* FIXME: Should use add_AT_pubnamesptr.  This works because most
targets
> +         don't care what the base section is.  */

-cary

[tromey_redhat.com, 2012-01-06 15:21:49]

>>>>> "Sterling" == Sterling Augustine <saugustine@google.com> writes:

Sterling> The enclosed patch to google/main contains certain small fixes
Sterling> for pubnames and pubtypes, which are now emitted completely
Sterling> and canonically.

I am curious to know how you ensure that they are canonical.

My recollection is that there are some differences between the names
emitted by g++ and those emitted by the demangler.  I can dig up some
examples if you want; I think they are in GCC bugzilla somewhere.

Tom

[saugustine, 2012-01-06 16:44:53]

On Fri, Jan 6, 2012 at 7:21 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Sterling" == Sterling Augustine <saugustine@google.com> writes:
>
> Sterling> The enclosed patch to google/main contains certain small fixes
> Sterling> for pubnames and pubtypes, which are now emitted completely
> Sterling> and canonically.
>
> I am curious to know how you ensure that they are canonical.

"Ensure" is probably a too strong a word.

But, I have been using a local python script that compares the names
emitted by this patch with those as produced by gdb in the .gdb_index.
(I suppose I should get it contributed, but it started out as a one
off and would need some cleaning up to get contributable.) I also
would have to figure out the rules on contributing python scripts.

I know of two very small differences at this point, and it is
debatable whether or not they should be fixed. First, the demangler
doesn't use the underscores around "restrict", and GCC does. Second, I
forget off-hand which way it goes, but one of them uses "short int"
and the other just "short", and similarly for the other built-in
integer types. I know of no other mismatches.

This does bring up an issue that--to my limited knowledge
anyway--isn't well addressed by the current structure of GCC vs
binutils vs GDB: There aren't good ways to run full integration tests
between the three components. Perhaps I am just ignorant.

>
> My recollection is that there are some differences between the names
> emitted by g++ and those emitted by the demangler.

Indeed, I have changed g++ in google/main to fix that. See the earlier
patch that this one cleaned up, for example.

  I can dig up some
> examples if you want; I think they are in GCC bugzilla somewhere.

I have found a lot of them (some tests have to be pretty contorted to
make it show up), but a pointer to bugzilla would be great. I expect
we will contribute these test-cases as part of a patch to Gold in the
near future.

Sterling