[patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

Hi,

iFort (Intel Fortran compiler) produces DIEs like:

 <1><1dc>: Abbrev Number: 6 (DW_TAG_module)
    <1e0>   DW_AT_name        : MOD1    

But Fortran is case insensitive, therefore GDB lowercases any user input
first.  Symbols cannot match.  gfortran always produces lowercased DIE names.

I was told by Jakub Jelinek GCC frontend will never be properly case
preserving - to output "Mod1" if user stated it in such case in the sources.

Therefore GDB will now lowercase any case insensitive symbol names on their
read-in from DWARF.

ELF symbol tables are currently not touched - their matching to their CU (and
thus to their DW_AT_identifier_case / DW_AT_language) is expensive and it does
not seem to me to be needed for iFort as only the module names are uppercased
there.

For the iFort compatibility there is also needed:
	[new testcase] Regression 7.1->7.2 for iFort [Re: [RFA-v2] dwarf debug format: Support DW_AT_variable_parameter attribute]
	http://sourceware.org/ml/gdb-patches/2010-11/msg00084.html

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.


Thanks,
Jan


gdb/
2010-11-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR 11313 - case insensitive identifiers in DWARF not in lowercase.
	* dwarf2read.c: Include ctype.h.
	(struct dwarf2_cu) <case_sensitivity>: New.
	(struct attribute) <string_is_canonical>: Update the comment.
	(canonical_case): New declaration.
	(read_partial_die): Call canonical_case.
	(canonical_case): New function.
	(dwarf2_canonicalize_name): Update the function comment.  Call
	canonical_case, adjust the obstack allocation.
	(dwarf2_name): Call canonical_case, set DW_STRING_IS_CANONICAL.
	(init_one_comp_unit, prepare_one_comp_unit): Set cu->case_sensitivity.

gdb/testsuite/
2010-11-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR 11313 - case insensitive identifiers in DWARF not in lowercase.
	* gdb.dwarf2/dw2-case-insensitive-debug.S: New.
	* gdb.dwarf2/dw2-case-insensitive.c: New.
	* gdb.dwarf2/dw2-case-insensitive.exp: New.
	* gdb.mi/mi-var-child-f.exp (mi_runto): Use lowercase main__.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -57,6 +57,7 @@
 #include "vec.h"
 #include "c-lang.h"
 #include "valprint.h"
+#include <ctype.h>
 
 #include <fcntl.h>
 #include "gdb_string.h"
@@ -388,6 +389,10 @@ struct dwarf2_cu
      DIEs for namespaces, we don't need to try to infer them
      from mangled names.  */
   unsigned int has_namespace_info : 1;
+
+  /* Are identifiers in this CU case sensitive or should they be lowercased
+     before storing them into GDB symbol tables?  */
+  enum case_sensitivity case_sensitivity;
 };
 
 /* When using the index (and thus not using psymtabs), each CU has an
@@ -638,9 +643,10 @@ struct attribute
     ENUM_BITFIELD(dwarf_attribute) name : 16;
     ENUM_BITFIELD(dwarf_form) form : 15;
 
-    /* Has DW_STRING already been updated by dwarf2_canonicalize_name?  This
-       field should be in u.str (existing only for DW_STRING) but it is kept
-       here for better struct attribute alignment.  */
+    /* Has DW_STRING already been updated by dwarf2_canonicalize_name or
+       canonical_case?  This field should be in u.str (existing only for
+       DW_STRING) but it is kept here for better struct attribute alignment.
+       */
     unsigned int string_is_canonical : 1;
 
     union
@@ -1135,6 +1141,9 @@ static gdb_byte *read_full_die (const struct die_reader_specs *reader,
 
 static void process_die (struct die_info *, struct dwarf2_cu *);
 
+static char *canonical_case (char *name, struct dwarf2_cu *cu,
+			     struct obstack *obstack);
+
 static char *dwarf2_canonicalize_name (char *, struct dwarf2_cu *,
 				       struct obstack *);
 
@@ -8799,7 +8808,8 @@ read_partial_die (struct partial_die_info *part_die,
 	    case DW_TAG_enumerator:
 	      /* These tags always have simple identifiers already; no need
 		 to canonicalize them.  */
-	      part_die->name = DW_STRING (&attr);
+	      part_die->name = canonical_case (DW_STRING (&attr), cu,
+					       &cu->objfile->objfile_obstack);
 	      break;
 	    default:
 	      part_die->name
@@ -11631,7 +11641,36 @@ sibling_die (struct die_info *die)
   return die->sibling;
 }
 
-/* Get name of a die, return NULL if not found.  */
+/* Convert NAME to the lower case if CU is case_sensitive_off.  Allocate
+   memory on OBSTACK if needed, otherwise return the NAME pointer.  */
+
+static char *
+canonical_case (char *name, struct dwarf2_cu *cu,
+		struct obstack *obstack)
+{
+  char *retval;
+  int i;
+
+  if (cu->case_sensitivity == case_sensitive_on)
+    return name;
+
+  for (i = 0; name[i] != 0; i++)
+    if (tolower (name[i]) != name[i])
+      break;
+  if (name[i] == 0)
+    return name;
+
+  retval = obstack_alloc (obstack, strlen (name) + 1);
+
+  for (i = 0; name[i] != 0; i++)
+    retval[i] = tolower (name[i]);
+  retval[i] = 0;
+
+  return retval;
+}
+
+/* Get name of a die, return NULL if not found.  canonical_case is always
+   already called for the returned string.  */
 
 static char *
 dwarf2_canonicalize_name (char *name, struct dwarf2_cu *cu,
@@ -11643,14 +11682,26 @@ dwarf2_canonicalize_name (char *name, struct dwarf2_cu *cu,
 
       if (canon_name != NULL)
 	{
-	  if (strcmp (canon_name, name) != 0)
-	    name = obsavestring (canon_name, strlen (canon_name),
+	  char *cased_name;
+
+	  cased_name = canonical_case (canon_name, cu, obstack);
+
+	  /* Push CASED_NAME on OBSTACK only if it isn't already pushed there
+	     by canonical_case and if it was allocated using xmalloc by
+	     cp_canonicalize_string.  */
+	  if (strcmp (canon_name, cased_name) == 0
+	      && strcmp (canon_name, name) != 0)
+	    name = obsavestring (cased_name, strlen (cased_name),
 				 obstack);
+	  else
+	    name = cased_name;
 	  xfree (canon_name);
+
+	  return name;
 	}
     }
 
-  return name;
+  return canonical_case (name, cu, obstack);
 }
 
 /* Get name of a die, return NULL if not found.  */
@@ -11672,7 +11723,13 @@ dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
     case DW_TAG_enumeration_type:
     case DW_TAG_enumerator:
       /* These tags always have simple identifiers already; no need
-	 to canonicalize them.  */
+	 to call dwarf2_canonicalize_name for them.  */
+      if (!DW_STRING_IS_CANONICAL (attr))
+	{
+	  DW_STRING (attr) = canonical_case (DW_STRING (attr), cu,
+					     &cu->objfile->objfile_obstack);
+	  DW_STRING_IS_CANONICAL (attr) = 1;
+	}
       return DW_STRING (attr);
 
     case DW_TAG_subprogram:
@@ -14301,6 +14358,7 @@ init_one_comp_unit (struct dwarf2_cu *cu, struct objfile *objfile)
   memset (cu, 0, sizeof (*cu));
   cu->objfile = objfile;
   obstack_init (&cu->comp_unit_obstack);
+  cu->case_sensitivity = case_sensitive_on;
 }
 
 /* Initiailize basic fields of dwarf_cu CU according to DIE COMP_UNIT_DIE.  */
@@ -14316,6 +14374,13 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die)
     set_cu_language (DW_UNSND (attr), cu);
   else
     set_cu_language (language_minimal, cu);
+
+  attr = dwarf2_attr (comp_unit_die, DW_AT_identifier_case, cu);
+  if (attr)
+    cu->case_sensitivity = (DW_UNSND (attr) == DW_ID_case_sensitive
+			    ? case_sensitive_on : case_sensitive_off);
+  else
+    cu->case_sensitivity = cu->language_defn->la_case_sensitivity;
 }
 
 /* Release one cached compilation unit, CU.  We unlink it from the tree
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S
@@ -0,0 +1,122 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 2				/* Abbrev: DW_TAG_compile_unit */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	2				/* DW_AT_language (DW_LANG_C) */
+	.byte	3				/* DW_AT_identifier_case (DW_ID_case_insensitive) */
+	.4byte		FUNC_attr		/* DW_AT_low_pc */
+	.4byte		FUNC_lang 		/* DW_AT_high_pc */
+
+	.uleb128	3			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.ascii		"FUNC_attr\0"		/* DW_AT_name */
+	.4byte		FUNC_attr		/* DW_AT_low_pc */
+	.4byte		FUNC_lang 		/* DW_AT_high_pc */
+
+	.byte		0			/* End of children of CU */
+.Lcu1_end:
+
+.Lcu2_begin:
+	/* CU header */
+	.4byte	.Lcu2_end - .Lcu2_start		/* Length of Compilation Unit */
+.Lcu2_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	8				/* DW_AT_language (DW_LANG_Fortran90) */
+	.4byte		FUNC_lang		/* DW_AT_low_pc */
+	.4byte		main			/* DW_AT_high_pc */
+
+	.uleb128	3			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.ascii		"FUNC_lang\0"		/* DW_AT_name */
+	.4byte		FUNC_lang		/* DW_AT_low_pc */
+	.4byte		main			/* DW_AT_high_pc */
+
+	.byte		0			/* End of children of CU */
+.Lcu2_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x42			/* DW_AT_identifier_case */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.c
@@ -0,0 +1,36 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This one is DW_LANG_C but it uses DW_ID_case_insensitive.  */
+
+void
+FUNC_attr (void)
+{
+}
+
+/* This one has no DW_AT_identifier_case but it is DW_LANG_Fortran90.  */
+
+void
+FUNC_lang (void)
+{
+}
+
+int
+main()
+{
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
@@ -0,0 +1,41 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0
+}
+
+set testfile "dw2-case-insensitive"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} [list ${testfile}.c ${testfile}-debug.S] {nodebug}] } {
+    return -1
+}
+
+gdb_test_no_output "set language fortran"
+
+if {[gdb_breakpoint "fuNC_attr"] == 1} {
+    pass "setting breakpoint at fuNC_attr"
+}
+
+if {[gdb_breakpoint "fuNC_lang"] == 1} {
+    pass "setting breakpoint at fuNC_lang"
+}
--- a/gdb/testsuite/gdb.mi/mi-var-child-f.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-child-f.exp
@@ -36,7 +36,7 @@ if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
 mi_gdb_reinitialize_dir $srcdir/$subdir
 mi_gdb_load ${binfile}
 
-mi_runto MAIN__
+mi_runto main__
 
 mi_gdb_test "-var-create array * array" \
     "\\^done,name=\"array\",numchild=\"3\",value=\".*\",type=\"integer \\(2,-1:1\\)\"" \

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Joel Brobecker]

> Therefore GDB will now lowercase any case insensitive symbol names on
> their read-in from DWARF.

Just a thought, since I don't really know what this would entail, but
shouldn't we just fix the lookup routines instead? I think that would
be cleaner.

-- 
Joel

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Mon, 08 Nov 2010 17:35:44 +0100, Joel Brobecker wrote:
> Just a thought, since I don't really know what this would entail, but
> shouldn't we just fix the lookup routines instead? I think that would
> be cleaner.

Do you/anyone know a compiler preserving the source case for some case
insensitive language?


Thanks,
Jan

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Joel Brobecker]

> > Just a thought, since I don't really know what this would entail, but
> > shouldn't we just fix the lookup routines instead? I think that would
> > be cleaner.
> 
> Do you/anyone know a compiler preserving the source case for some case
> insensitive language?

I don't, but I'm just trying to see if we can evaluate the work that
would be required to make lookups case-insensitive, rather than
lowercasing the entity names.  If you think it's very hard, then,
OK, let's consider your approach.

Note that with Ada, one can still export entities using a name that
has mixed casing.  We allow the user to reference them from GDB using
the "<[...]>" syntax (Eg: "print "<My_Exported_Variable>").  If you
lowercase everything, you won't be able to do allow that for Fortran.

-- 
Joel

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Mon, 08 Nov 2010 19:31:33 +0100, Joel Brobecker wrote:
> trying to see if we can evaluate the work that would be required to make
> lookups case-insensitive, rather than lowercasing the entity names.

Maybe some Ada code could be cleaned up on top of that?


On Fri, 12 Nov 2010 19:50:38 +0100, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> I have prepared a second patch now to address Joel's suggestion to
> Jan> rather look up symbols case insensitively instead of lowercasing
> Jan> them on read-in.
> 
> I'll be interested to see that.


Here is a draft, not yet for review.

I guess .gdb_index's find_slot_in_mapped_hash should already use strcmp_iw.
It may currently break C++ psymtabs expansion on .gdb_index'ed files.

But I cannot test it as GDB already is broken just by the physname:
	[testcase patch] 7.2 regression on expand psymtabs
	http://sourceware.org/ml/gdb-patches/2010-11/msg00300.html


Unrelated talk:

Currently strcmp_iw does ugly skipping of whitespaces.  Thanks to the physname
canonicalization of all the symbols they should be unique enough so that no
longer should be needed.

Just one would have to canonicalize even the user input string (such as
`method(long, char *)').  As dwarf2_physname works from the internal parsed
representation the expression would need to be parsed first.  But for
tab-completion an incomplete string cannot be parsed into exp_element-s, can
it?

It is now even reproducible as a bug, GDB cannot distinguish those two:

#include <stdio.h>
class C
  {
  public:
    static void operator delete (void *p)
    { printf ("operator %p\n", operator delete); }
    static void operatordelete (void *p)
    { printf ("method %p\n", operatordelete); }
  };
int
main ()
{
  C *x = new C;
  x->operatordelete(NULL);
  delete x;
}

gdb -q -nx ./1 -ex r -ex 'p C::operator delete(void*)' -ex 'p C::operatordelete(void*)' -ex q
method 0x4005d9
operator 0x4005b7
Program exited normally.
$1 = {void (void *)} 0x4005b7 <C::operator delete(void*)>
$2 = {void (void *)} 0x4005b7 <C::operator delete(void*)>



Thanks,
Jan


--- a/gdb/dictionary.c
+++ b/gdb/dictionary.c
@@ -836,7 +836,7 @@ dict_hash (const char *string0)
 	    }
 	  /* FALL THROUGH */
 	default:
-	  hash = hash * 67 + *string - 113;
+	  hash = SYMBOL_HASH_NEXT (hash, *string);
 	  string += 1;
 	  break;
 	}
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -71,6 +71,7 @@
 #define MAP_FAILED ((void *) -1)
 #endif
 #endif
+#include <ctype.h>
 
 typedef struct symbol *symbolp;
 DEF_VEC_P (symbolp);
@@ -148,6 +149,8 @@ DEF_VEC_I (offset_type);
    a comment by the code that writes the index.  */
 struct mapped_index
 {
+  /* Index data format version.  */
+  int version;
   /* The total length of the buffer.  */
   off_t total_size;
   /* A pointer to the address table data.  */
@@ -1853,21 +1856,26 @@ create_addrmap_from_index (struct objfile *objfile, struct mapped_index *index)
   do_cleanups (cleanup);
 }
 
-/* The hash function for strings in the mapped index.  This is the
-   same as the hashtab.c hash function, but we keep a separate copy to
-   maintain control over the implementation.  This is necessary
-   because the hash function is tied to the format of the mapped index
-   file.  */
+/* The hash function for strings in the mapped index.  This is the same as
+   SYMBOL_HASH_NEXT, but we keep a separate copy to maintain control over the
+   implementation.  This is necessary because the hash function is tied to the
+   format of the mapped index file.
+   
+   Use INT_MAX for INDEX_VERSION if you generate the current index format.  */
 
 static hashval_t
-mapped_index_string_hash (const void *p)
+mapped_index_string_hash (int index_version, const void *p)
 {
   const unsigned char *str = (const unsigned char *) p;
   hashval_t r = 0;
   unsigned char c;
 
   while ((c = *str++) != 0)
-    r = r * 67 + c - 113;
+    {
+      if (index_version >= 4)
+	c = tolower (c);
+      r = r * 67 + c - 113;
+    }
 
   return r;
 }
@@ -1880,7 +1888,7 @@ static int
 find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
 			  offset_type **vec_out)
 {
-  offset_type hash = mapped_index_string_hash (name);
+  offset_type hash = mapped_index_string_hash (index->version, name);
   offset_type slot, step;
 
   slot = hash & (index->symbol_table_slots - 1);
@@ -1895,7 +1903,7 @@ find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
 	return 0;
 
       str = index->constant_pool + MAYBE_SWAP (index->symbol_table[i]);
-      if (!strcmp (name, str))
+      if (!strcmp_iw (str, name))
 	{
 	  *vec_out = (offset_type *) (index->constant_pool
 				      + MAYBE_SWAP (index->symbol_table[i + 1]));
@@ -1941,8 +1949,13 @@ dwarf2_read_index (struct objfile *objfile)
      it seems better to just ignore such indices.  */
   if (version < 3)
     return 0;
+  /* Indexes with higher version than the one supported by GDB may be no
+     longer backward compatible.  */
+  if (version > 4)
+    return 0;
 
   map = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct mapped_index);
+  map->version = version;
   map->total_size = dwarf2_per_objfile->gdb_index.size;
 
   metadata = (offset_type *) (addr + sizeof (offset_type));
@@ -14800,13 +14813,16 @@ struct strtab_entry
   const char *str;
 };
 
-/* Hash function for a strtab_entry.  */
+/* Hash function for a strtab_entry.
+
+   Function is used only during write_hash_table so no index format backward
+   compatibility is needed.  */
 
 static hashval_t
 hash_strtab_entry (const void *e)
 {
   const struct strtab_entry *entry = e;
-  return mapped_index_string_hash (entry->str);
+  return mapped_index_string_hash (INT_MAX, entry->str);
 }
 
 /* Equality function for a strtab_entry.  */
@@ -14944,12 +14960,15 @@ cleanup_mapped_symtab (void *p)
 }
 
 /* Find a slot in SYMTAB for the symbol NAME.  Returns a pointer to
-   the slot.  */
+   the slot.
+   
+   Function is used only during write_hash_table so no index format backward
+   compatibility is needed.  */
 
 static struct symtab_index_entry **
 find_slot (struct mapped_symtab *symtab, const char *name)
 {
-  offset_type index, step, hash = mapped_index_string_hash (name);
+  offset_type index, step, hash = mapped_index_string_hash (INT_MAX, name);
 
   index = hash & (symtab->size - 1);
   step = ((hash * 17) & (symtab->size - 1)) | 1;
@@ -15349,7 +15368,7 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
   total_len = size_of_contents;
 
   /* The version number.  */
-  val = MAYBE_SWAP (3);
+  val = MAYBE_SWAP (4);
   obstack_grow (&contents, &val, sizeof (val));
 
   /* The offset of the CU list from the start of the file.  */
@@ -15407,8 +15426,8 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
    1. The file header.  This is a sequence of values, of offset_type
    unless otherwise noted:
 
-   [0] The version number, currently 3.  Versions 1 and 2 are
-   obsolete.
+   [0] The version number, currently 4.  Versions 1 and 2 are
+   obsolete.  Version 3 differs by its hashing function.
    [1] The offset, from the start of the file, of the CU list.
    [2] The offset, from the start of the file, of the types CU list.
    Note that this section can be empty, in which case this offset will
@@ -15440,7 +15459,8 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
 
    5. The symbol table.  This is a hash table.  The size of the hash
    table is always a power of 2.  The initial hash and the step are
-   currently defined by the `find_slot' function.
+   currently defined by the `find_slot' function.  Index version 3 uses
+   a different hash function than index version 4 and later.
 
    Each slot in the hash table consists of a pair of offset_type
    values.  The first value is the offset of the symbol's name in the
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -89,7 +89,7 @@ msymbol_hash_iw (const char *string)
 	++string;
       if (*string && *string != '(')
 	{
-	  hash = hash * 67 + *string - 113;
+	  hash = SYMBOL_HASH_NEXT (hash, *string);
 	  ++string;
 	}
     }
@@ -104,7 +104,7 @@ msymbol_hash (const char *string)
   unsigned int hash = 0;
 
   for (; *string; ++string)
-    hash = hash * 67 + *string - 113;
+    hash = SYMBOL_HASH_NEXT (hash, *string);
   return hash;
 }
 
@@ -244,6 +244,9 @@ lookup_minimal_symbol (const char *name, const char *sfile,
 		    {
 		      match = strcmp (SYMBOL_LINKAGE_NAME (msymbol),
 				      modified_name) == 0;
+		      if (!match && case_sensitivity == case_sensitive_off)
+			match = strcasecmp (SYMBOL_LINKAGE_NAME (msymbol),
+					    modified_name) == 0;
 		    }
 		  else
 		    {
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -577,8 +577,15 @@ lookup_partial_symbol (struct partial_symtab *pst, const char *name,
       if (!(top == bottom))
 	internal_error (__FILE__, __LINE__, _("failed internal consistency check"));
 
-      while (top <= real_top
-	     && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+      /* For `case_sensitivity == case_sensitive_off' strcmp_iw_ordered will
+	 search more exactly than what matches SYMBOL_MATCHES_SEARCH_NAME.  */
+      while (top >= start && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
+	top--;
+
+      /* Fixup to have a symbol which matches SYMBOL_MATCHES_SEARCH_NAME.  */
+      top++;
+
+      while (top <= real_top && SYMBOL_MATCHES_SEARCH_NAME (*top, name))
 	{
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (*top),
 				     SYMBOL_DOMAIN (*top), domain))
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1061,19 +1061,6 @@ lookup_symbol_in_language (const char *name, const struct block *block,
 	}
     }
 
-  if (case_sensitivity == case_sensitive_off)
-    {
-      char *copy;
-      int len, i;
-
-      len = strlen (name);
-      copy = (char *) alloca (len + 1);
-      for (i= 0; i < len; i++)
-        copy[i] = tolower (name[i]);
-      copy[len] = 0;
-      modified_name = copy;
-    }
-
   returnval = lookup_symbol_aux (modified_name, block, domain, lang,
 				 is_a_field_of_this);
   do_cleanups (cleanup);
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1012,6 +1012,13 @@ extern unsigned int msymbol_hash_iw (const char *);
 
 extern unsigned int msymbol_hash (const char *);
 
+/* This is only an internally computed value with no external files
+   compatibility requirements.  Compute next hash value from string
+   character C.  */
+
+#define SYMBOL_HASH_NEXT(hash, c) \
+  ((hash) * 67 + tolower ((unsigned char) (c)) - 113)
+
 extern struct objfile * msymbol_objfile (struct minimal_symbol *sym);
 
 extern void
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fortran-sym-case.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (int argc, char **aRGv)
+{
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/fortran-sym-case.exp
@@ -0,0 +1,27 @@
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile fortran-sym-case
+if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "set language fortran" {Warning: the current language does not match this frame\.}
+
+gdb_test "frame" "aRGv=0x.*"
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive-debug.S
@@ -0,0 +1,78 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	8				/* DW_AT_language (DW_LANG_Fortran90) */
+	.4byte		FUNC_lang		/* DW_AT_low_pc */
+	.4byte		main			/* DW_AT_high_pc */
+
+	.uleb128	3			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.ascii		"FUNC_lang\0"		/* DW_AT_name */
+	.4byte		FUNC_lang		/* DW_AT_low_pc */
+	.4byte		main			/* DW_AT_high_pc */
+
+	.byte		0			/* End of children of CU */
+.Lcu1_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.c
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Use DW_LANG_Fortran90 for case insensitive DWARF.  */
+
+void
+FUNC_lang (void)
+{
+}
+
+/* Symbol is present only in ELF .symtab.  */
+
+void
+FUNC_symtab (void)
+{
+}
+
+int
+main()
+{
+  FUNC_lang ();
+  FUNC_symtab ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
@@ -0,0 +1,41 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0
+}
+
+set testfile "dw2-case-insensitive"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} [list ${testfile}.c ${testfile}-debug.S] {nodebug}] } {
+    return -1
+}
+
+gdb_test_no_output "set language fortran"
+
+if {[gdb_breakpoint "fuNC_lang"] == 1} {
+    pass "setting breakpoint at fuNC_lang"
+}
+
+if {[gdb_breakpoint "fuNC_symtab"] == 1} {
+    pass "setting breakpoint at fuNC_symtab"
+}
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2927,10 +2927,12 @@ strcmp_iw (const char *string1, const char *string2)
 	{
 	  string2++;
 	}
-      if (*string1 != *string2)
-	{
-	  break;
-	}
+      if (case_sensitivity == case_sensitive_on && *string1 != *string2)
+	break;
+      if (case_sensitivity == case_sensitive_off
+	  && (tolower ((unsigned char) *string1)
+	      != tolower ((unsigned char) *string2)))
+	break;
       if (*string1 != '\0')
 	{
 	  string1++;
@@ -2986,10 +2988,12 @@ strcmp_iw_ordered (const char *string1, const char *string2)
 	{
 	  string2++;
 	}
-      if (*string1 != *string2)
-	{
-	  break;
-	}
+      if (case_sensitivity == case_sensitive_on && *string1 != *string2)
+	break;
+      if (case_sensitivity == case_sensitive_off
+	  && (tolower ((unsigned char) *string1)
+	      != tolower ((unsigned char) *string2)))
+	break;
       if (*string1 != '\0')
 	{
 	  string1++;
@@ -3015,8 +3019,11 @@ strcmp_iw_ordered (const char *string1, const char *string2)
     default:
       if (*string2 == '(')
 	return 1;
-      else
+      else if (case_sensitivity == case_sensitive_on)
 	return *string1 - *string2;
+      else
+	return (tolower ((unsigned char) *string1)
+		- tolower ((unsigned char) *string2));
     }
 }
 

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Joel Brobecker]

> Here is a draft, not yet for review.

So - what do you think? I'm a little torn, because of the following
change which I did not anticipate:

>  	default:
> -	  hash = hash * 67 + *string - 113;
> +	  hash = SYMBOL_HASH_NEXT (hash, *string);
[...]
> +#define SYMBOL_HASH_NEXT(hash, c) \
> +  ((hash) * 67 + tolower ((unsigned char) (c)) - 113)

The lowercasing of the character impact the hash's overal performance?

Your first patch only affects the languages for which the language
is marked as case-insensitive, whereas the second one seems to affect
all languages. If the impact is sufficiently noticeable, then that
would be an argument in favor of your first patch...

(just thinking aloud)

-- 
Joel

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Mon, 22 Nov 2010 19:54:32 +0100, Joel Brobecker wrote:
> > +	  hash = SYMBOL_HASH_NEXT (hash, *string);
> [...]
> > +#define SYMBOL_HASH_NEXT(hash, c) \
> > +  ((hash) * 67 + tolower ((unsigned char) (c)) - 113)
> 
> The lowercasing of the character impact the hash's overal performance?

If there are any concerns about it (I do not think there should be any while
looking and the disassembly plus glibc's __ctype_tolower_loc) we should also
ask why GDB already uses locale-conforming tolower while gcc uses TOLOWER.

GDB could also use it, it is in libiberty.  That one has "zero" cost.


Thanks,
Jan

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Joel Brobecker]

> If there are any concerns about it (I do not think there should be any while
> looking and the disassembly plus glibc's __ctype_tolower_loc) we should also
> ask why GDB already uses locale-conforming tolower while gcc uses TOLOWER.
> 
> GDB could also use it, it is in libiberty.  That one has "zero" cost.

I was actually wondering about the change in the hash algorithm more
than the cost of calling tolower.  For instance, "tmp" and "Tmp" would
have had different hash values, but not anymore.  So, presumably, when
you start looking up for "tmp", the associated hash bucket will also
contain "Tmp" whereas it wouldn't before. I need to look at the actual
hashing parameters to see if we can figure out whether this should have
any real effect in practice...  If the number of elements in each bucket
is reasonable, a few more iterations shouldn't be an issue.

-- 
Joel

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Mon, 22 Nov 2010 20:30:41 +0100, Joel Brobecker wrote:
> I was actually wondering about the change in the hash algorithm more
> than the cost of calling tolower.  For instance, "tmp" and "Tmp" would
> have had different hash values, but not anymore.  So, presumably, when
> you start looking up for "tmp", the associated hash bucket will also
> contain "Tmp" whereas it wouldn't before. I need to look at the actual
> hashing parameters to see if we can figure out whether this should have
> any real effect in practice...  If the number of elements in each bucket
> is reasonable, a few more iterations shouldn't be an issue.

This is a more general issue.

I think (I did not measure it) most of the symbols differ even after tolower.
The symbols like tmp<->Tmp exist but rarely.  I agree the hashing function
will get worse but I did not even measure it considering the change
negligible.

There is more an issue MINIMAL_SYMBOL_HASH_SIZE is constant:
	#define MINIMAL_SYMBOL_HASH_SIZE 2039

Some objfiles have many symbols:
	libwebkit.so.debug: 54980 symbols
		/MINIMAL_SYMBOL_HASH_SIZE = 27
		log2(54980)=16
	gdb symtab: 36452 symbols
		/MINIMAL_SYMBOL_HASH_SIZE = 18
		log2(54980)=16

In such case in fact the whole hash table makes no sense and it is even
cheaper to just do binary search on objfile->msymbols which is already
qsort-ed and be done with it.

Still a hash table should be faster than a binary search but the hash table
size would need to be adaptable.

But rather than optimizations of this which reduce just the CPU load which was
in my measurements 2% during GDB startup (due to its waiting on disk).  We
could for example rather delay searching+loading any objfiles' symbols we do
not need which would do another major GDB startup time reduction like
.gdb_index did.  This is the reason I did not intend to spend any time on some
CPU discutable optimizations, they IMO do not make sense with the current
state of gdb performance.


Thanks,
Jan

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Joel Brobecker]

> This is the reason I did not intend to spend any time on some CPU
> discutable optimizations, they IMO do not make sense with the current
> state of gdb performance.

Good points :)

-- 
Joel

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> There is more an issue MINIMAL_SYMBOL_HASH_SIZE is constant:
Jan> 	#define MINIMAL_SYMBOL_HASH_SIZE 2039
Jan> Some objfiles have many symbols:
Jan> 	libwebkit.so.debug: 54980 symbols
Jan> 		/MINIMAL_SYMBOL_HASH_SIZE = 27
Jan> 		log2(54980)=16

I looked at this a long time ago, when I was trying to reduce memory use.

My recollection is that our hashing here is pretty bad.  I seem to
remember a case where we had some empty hash buckets and some buckets
with 20 symbols.

Jan> In such case in fact the whole hash table makes no sense and it is even
Jan> cheaper to just do binary search on objfile->msymbols which is already
Jan> qsort-ed and be done with it.

msymbols is sorted by address first.

Note that changing this area is trickier than it seems due to the
mst_solib_trampoline handling in lookup_minimal_symbol (maybe obvious,
but it took me a while to realize it...).

Jan> Still a hash table should be faster than a binary search but the
Jan> hash table size would need to be adaptable.

I implemented this :-).  It slowed down startup, so it didn't go in.
See the archives around 2008-08-27.  Given your other comments this hit
may not be relevant.  I still have the patch if you are interested.

Jan> But rather than optimizations of this which reduce just the CPU
Jan> load which was in my measurements 2% during GDB startup (due to its
Jan> waiting on disk).  We could for example rather delay
Jan> searching+loading any objfiles' symbols we do not need which would
Jan> do another major GDB startup time reduction like .gdb_index did.

Yeah, this would be interesting to see.  I think find_main_filename may
make this difficult, but maybe it would work ok for the attach case.

This sort of reading could maybe be done in a background thread.

Also, I wonder why we read minsyms from separate debuginfo files.
Is that ever needed?

Tom

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Wed, 24 Nov 2010 19:52:28 +0100, Tom Tromey wrote:
> msymbols is sorted by address first.

oops, ok


> This sort of reading could maybe be done in a background thread.

This is again for interactive work like .gdb_index was.

For non-interactive backtracing (ABRT) the threads have no benefits.


> Also, I wonder why we read minsyms from separate debuginfo files.
> Is that ever needed?

In the past I thought GDB should work fine just with DWARF symbols without ELF
symbols.

For example read_var_value looks up for LOC_UNRESOLVED only ELF symbols and
ignores not global DWARF symbols, there are more places like that one.
So far I have given up on the without-minsyms goal, it requires also gcc
changes how DWARF is produced.  It may be nice but it is IMO too much work
just cleaning things up without user visible effect.


Regards,
Jan

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Tom Tromey]

Jan> For non-interactive backtracing (ABRT) the threads have no benefits.

Yeah.

Maybe we can expose more controls for specialized applications like
ABRT.

Tom> Also, I wonder why we read minsyms from separate debuginfo files.
Tom> Is that ever needed?

Jan> In the past I thought GDB should work fine just with DWARF symbols
Jan> without ELF symbols.

Actually I was specifically thinking of just the separate debuginfo case.
In this case wouldn't the primary objfile always have the minsyms?

Tom

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Joel Brobecker]

> Actually I was specifically thinking of just the separate debuginfo case.
> In this case wouldn't the primary objfile always have the minsyms?

What if the primary objfile was completely stripped, though?

-- 
Joel

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Tom Tromey]

Tom> Actually I was specifically thinking of just the separate debuginfo
Tom> case.  In this case wouldn't the primary objfile always have the
Tom> minsyms?

Joel> What if the primary objfile was completely stripped, though?

We could easily check for that when we read the separate debuginfo.

Jan> Whether .symtab (=ELF symbols=minimal symbols) is present in the primary
Jan> objfile or the separate debug info objfile depends on this switch.

Jan> For almost all the Fedora packages .symtab is in the separate debug
Jan> info file.  glibc (not sure if anything else) has an exception it
Jan> has .symtab still in the primary binary file.  (I do not agree much
Jan> with such exception myself.)

I think both files have .dynsym though.  And AFAIK we end up with two
copies of these minsyms.  One or the other seems unneeded.

Tom

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Wed, 24 Nov 2010 22:37:15 +0100, Tom Tromey wrote:
> I think both files have .dynsym though.

I do not know any section would be present in both files (except .note.* like
.note.build-id and at least .shstrlab).  .dynsym is always needed in the
primary file so there is no need to have in in the .debug file:

/bin/bash
  [ 5] .dynsym           DYNSYM          0000000000403898 003898 00c588 18   A 28   1  8
/usr/lib/debug/bin/bash.debug|grep dynsym
  [ 5] .dynsym           NOBITS          0000000000403860 000248 00c588 18   A  6   1  8


> And AFAIK we end up with two
> copies of these minsyms.  One or the other seems unneeded.

There will be both .dynsym and .symtab where the first should be a subset of
the second; but GDB cannoy depend on the fact it should be a subset.


Regards,
Jan

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> I do not know any section would be present in both files (except
Jan> .note.* like .note.build-id and at least .shstrlab).

Jan>   [ 5] .dynsym           NOBITS          0000000000403860 000248 00c588 18   A  6   1  8

Aha.  Sorry about this, I got confused by objdump, which doesn't
explicitly say NOBITS (but rather just CONTENTS when there are some).
Thanks for the extended explanation.

Tom

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Wed, 24 Nov 2010 21:00:47 +0100, Tom Tromey wrote:
> Actually I was specifically thinking of just the separate debuginfo case.
> In this case wouldn't the primary objfile always have the minsyms?

/usr/lib/rpm/find-debuginfo.sh
# The -g flag says to use strip -g instead of full strip on DSOs.
strip:
  -g -S -d --strip-debug           Remove all debugging symbols & sections
eu-strip:
  -g, -d, -S, --strip-debug  Remove all debugging symbols
glibc.spec:
	find_debuginfo_args='--strict-build-id -g'

Whether .symtab (=ELF symbols=minimal symbols) is present in the primary
objfile or the separate debug info objfile depends on this switch.

For almost all the Fedora packages .symtab is in the separate debug info file.
glibc (not sure if anything else) has an exception it has .symtab still in the
primary binary file.  (I do not agree much with such exception myself.)


Regards,
Jan

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Joel Brobecker]

> For almost all the Fedora packages .symtab is in the separate debug
> info file.  glibc (not sure if anything else) has an exception it has
> .symtab still in the primary binary file.  (I do not agree much with
> such exception myself.)

I thought you needed that symbol table in order to link against a shared
library?

-- 
Joel

Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

On Wed, 24 Nov 2010 21:31:12 +0100, Joel Brobecker wrote:
> > For almost all the Fedora packages .symtab is in the separate debug
> > info file.  glibc (not sure if anything else) has an exception it has
> > .symtab still in the primary binary file.  (I do not agree much with
> > such exception myself.)
> 
> I thought you needed that symbol table in order to link against a shared
> library?

That is `.dynsym'.  GDB also reads it into minsym, though.  Yes, `.dynsym' is
always present in the primary binary file.

`.dynsym' is processed by ld.so, `.symtab' is ignored by ld.so and is provided
there only for debuggers in shared libraries.  `.dynsym' is covered by a LOAD
segment, `.symtab' is not, therefore it cannot be used during runtime.


Regards,
Jan

obsolete: Re: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Jan Kratochvil]

obsoleted by:
	[patch 0/3] case preserving, case insensitive (PR 11313+PR 11560)
	http://sourceware.org/ml/gdb-patches/2011-04/msg00123.html

RE: [patch 2/2] iFort compat.: case insensitive symbols (PR 11313)

[Pierre Muller]

  Yes, I don't like the idea of lowercas'ing everything at
read-in: 
  In Free Pascal, despite the fact that pascal is case insensitive
at source level, uses internal functions
that are lowercase as opposed to all source declared function
which are uppercased.
  If we unconditionally lowercase everything, we would
possibly not be able anymore to specifically target those internal
functions.

  A while ago, I proposed a patch that changed the
used a second copy of the pascal name
http://sourceware.org/ml/gdb-patches/2010-04/msg00967.html

  But you didn't like that proposition ...
  I didn't reply to the last message in this thread,
but I now think that your proposal of using a specific
syntax (gdb) print <My_variable>
to avoid case conversion, would not work
for info command, which was one of the
reasons of that patch.

Pierre Muller

Pascal language maintainer

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : lundi 8 novembre 2010 17:36
> À : Jan Kratochvil
> Cc : gdb-patches@sourceware.org
> Objet : Re: [patch 2/2] iFort compat.: case insensitive symbols (PR
> 11313)
> 
> > Therefore GDB will now lowercase any case insensitive symbol names on
> > their read-in from DWARF.
> 
> Just a thought, since I don't really know what this would entail, but
> shouldn't we just fix the lookup routines instead? I think that would
> be cleaner.
> 
> --
> Joel

[patch 1/2] Code cleanup: New init_one_comp_unit

[Jan Kratochvil]

Hi,

unify some code into a function, to be extended in the next patch.


Thanks,
Jan


gdb/
2010-11-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Code cleanup.
	* dwarf2read.c (alloc_one_comp_unit): Rename prototype to ...
	(init_one_comp_unit): ... this one.
	(prepare_one_comp_unit): New prototype.
	(dw2_require_line_header, process_psymtab_comp_unit): Use
	init_one_comp_unit.
	(process_psymtab_comp_unit): Use prepare_one_comp_unit.
	(load_partial_comp_unit): Remove variable attr.  Use
	init_one_comp_unit with xmalloc.  Use prepare_one_comp_unit.
	(load_full_comp_unit): Use init_one_comp_unit with xmalloc.  Use
	prepare_one_comp_unit.
	(read_signatured_type): Remove variable attr.  Use init_one_comp_unit.
	Use prepare_one_comp_unit.
	(alloc_one_comp_unit): Rename to ...
	(init_one_comp_unit): ... here and remove there calloc, new parameter
	cu.
	(prepare_one_comp_unit): New function.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1241,7 +1241,11 @@ static struct dwarf2_per_cu_data *dwarf2_find_containing_comp_unit
 static struct dwarf2_per_cu_data *dwarf2_find_comp_unit
   (unsigned int offset, struct objfile *objfile);
 
-static struct dwarf2_cu *alloc_one_comp_unit (struct objfile *objfile);
+static void init_one_comp_unit (struct dwarf2_cu *cu,
+				struct objfile *objfile);
+
+static void prepare_one_comp_unit (struct dwarf2_cu *cu,
+				   struct die_info *comp_unit_die);
 
 static void free_one_comp_unit (void *);
 
@@ -2018,10 +2022,7 @@ dw2_require_line_header (struct objfile *objfile,
     return;
   this_cu->v.quick->read_lines = 1;
 
-  memset (&cu, 0, sizeof (cu));
-  cu.objfile = objfile;
-  obstack_init (&cu.comp_unit_obstack);
-
+  init_one_comp_unit (&cu, objfile);
   cleanups = make_cleanup (free_stack_comp_unit, &cu);
 
   if (this_cu->from_debug_types)
@@ -3024,10 +3025,7 @@ process_psymtab_comp_unit (struct objfile *objfile,
   CORE_ADDR best_lowpc = 0, best_highpc = 0;
   struct die_reader_specs reader_specs;
 
-  memset (&cu, 0, sizeof (cu));
-  cu.objfile = objfile;
-  obstack_init (&cu.comp_unit_obstack);
-
+  init_one_comp_unit (&cu, objfile);
   back_to_inner = make_cleanup (free_stack_comp_unit, &cu);
 
   info_ptr = partial_read_comp_unit_head (&cu.header, info_ptr,
@@ -3081,12 +3079,7 @@ process_psymtab_comp_unit (struct objfile *objfile,
       return info_ptr;
     }
 
-  /* Set the language we're debugging.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_language, &cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), &cu);
-  else
-    set_cu_language (language_minimal, &cu);
+  prepare_one_comp_unit (&cu, comp_unit_die);
 
   /* Allocate a new partial symbol table structure.  */
   attr = dwarf2_attr (comp_unit_die, DW_AT_name, &cu);
@@ -3301,7 +3294,6 @@ load_partial_comp_unit (struct dwarf2_per_cu_data *this_cu,
   struct die_info *comp_unit_die;
   struct dwarf2_cu *cu;
   struct cleanup *free_abbrevs_cleanup, *free_cu_cleanup = NULL;
-  struct attribute *attr;
   int has_children;
   struct die_reader_specs reader_specs;
   int read_cu = 0;
@@ -3314,7 +3306,8 @@ load_partial_comp_unit (struct dwarf2_per_cu_data *this_cu,
 
   if (this_cu->cu == NULL)
     {
-      cu = alloc_one_comp_unit (objfile);
+      cu = xmalloc (sizeof (*cu));
+      init_one_comp_unit (cu, objfile);
 
       read_cu = 1;
 
@@ -3354,12 +3347,7 @@ load_partial_comp_unit (struct dwarf2_per_cu_data *this_cu,
   info_ptr = read_full_die (&reader_specs, &comp_unit_die, info_ptr,
 			    &has_children);
 
-  /* Set the language we're debugging.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_language, cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), cu);
-  else
-    set_cu_language (language_minimal, cu);
+  prepare_one_comp_unit (cu, comp_unit_die);
 
   /* Check if comp unit has_children.
      If so, read the rest of the partial symbols from this comp unit.
@@ -4314,7 +4302,8 @@ load_full_comp_unit (struct dwarf2_per_cu_data *per_cu, struct objfile *objfile)
 
   if (per_cu->cu == NULL)
     {
-      cu = alloc_one_comp_unit (objfile);
+      cu = xmalloc (sizeof (*cu));
+      init_one_comp_unit (cu, objfile);
 
       read_cu = 1;
 
@@ -4352,11 +4341,7 @@ load_full_comp_unit (struct dwarf2_per_cu_data *per_cu, struct objfile *objfile)
      all objfiles needed for references have been loaded yet, and symbol
      table processing isn't initialized.  But we have to set the CU language,
      or we won't be able to build types correctly.  */
-  attr = dwarf2_attr (cu->dies, DW_AT_language, cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), cu);
-  else
-    set_cu_language (language_minimal, cu);
+  prepare_one_comp_unit (cu, cu->dies);
 
   /* Similarly, if we do not read the producer, we can not apply
      producer-specific interpretation.  */
@@ -13220,17 +13205,15 @@ read_signatured_type (struct objfile *objfile,
   struct dwarf2_cu *cu;
   ULONGEST signature;
   struct cleanup *back_to, *free_cu_cleanup;
-  struct attribute *attr;
 
   dwarf2_read_section (objfile, &dwarf2_per_objfile->types);
   types_ptr = dwarf2_per_objfile->types.buffer + type_sig->offset;
 
   gdb_assert (type_sig->per_cu.cu == NULL);
 
-  cu = xmalloc (sizeof (struct dwarf2_cu));
-  memset (cu, 0, sizeof (struct dwarf2_cu));
-  obstack_init (&cu->comp_unit_obstack);
-  cu->objfile = objfile;
+  cu = xmalloc (sizeof (*cu));
+  init_one_comp_unit (cu, objfile);
+
   type_sig->per_cu.cu = cu;
   cu->per_cu = &type_sig->per_cu;
 
@@ -13262,11 +13245,7 @@ read_signatured_type (struct objfile *objfile,
      all objfiles needed for references have been loaded yet, and symbol
      table processing isn't initialized.  But we have to set the CU language,
      or we won't be able to build types correctly.  */
-  attr = dwarf2_attr (cu->dies, DW_AT_language, cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), cu);
-  else
-    set_cu_language (language_minimal, cu);
+  prepare_one_comp_unit (cu, cu->dies);
 
   do_cleanups (back_to);
 
@@ -14314,15 +14293,29 @@ dwarf2_find_comp_unit (unsigned int offset, struct objfile *objfile)
   return this_cu;
 }
 
-/* Malloc space for a dwarf2_cu for OBJFILE and initialize it.  */
+/* Initialize dwarf2_cu CU for OBJFILE in a pre-allocated space.  */
 
-static struct dwarf2_cu *
-alloc_one_comp_unit (struct objfile *objfile)
+static void
+init_one_comp_unit (struct dwarf2_cu *cu, struct objfile *objfile)
 {
-  struct dwarf2_cu *cu = xcalloc (1, sizeof (struct dwarf2_cu));
+  memset (cu, 0, sizeof (*cu));
   cu->objfile = objfile;
   obstack_init (&cu->comp_unit_obstack);
-  return cu;
+}
+
+/* Initiailize basic fields of dwarf_cu CU according to DIE COMP_UNIT_DIE.  */
+
+static void
+prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die)
+{
+  struct attribute *attr;
+
+  /* Set the language we're debugging.  */
+  attr = dwarf2_attr (comp_unit_die, DW_AT_language, cu);
+  if (attr)
+    set_cu_language (DW_UNSND (attr), cu);
+  else
+    set_cu_language (language_minimal, cu);
 }
 
 /* Release one cached compilation unit, CU.  We unlink it from the tree

Re: [patch 1/2] Code cleanup: New init_one_comp_unit

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> unify some code into a function, to be extended in the next patch.

Looks good to me.

Jan> +/* Initiailize basic fields of dwarf_cu CU according to DIE COMP_UNIT_DIE.  */

Typo, "Initialize".

Tom

Re: [patch 1/2] Code cleanup: New init_one_comp_unit

[Jan Kratochvil]

On Fri, 12 Nov 2010 19:40:39 +0100, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> unify some code into a function, to be extended in the next patch.
> 
> Looks good to me.

I have prepared a second patch now to address Joel's suggestion to rather look
up symbols case insensitively instead of lowercasing them on read-in.

But maybe this patch is OK anyway although the cleanup could be more complete.

(Not yet checked in.)


Thanks,
Jan

Re: [patch 1/2] Code cleanup: New init_one_comp_unit

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> I have prepared a second patch now to address Joel's suggestion to
Jan> rather look up symbols case insensitively instead of lowercasing
Jan> them on read-in.

I'll be interested to see that.

Jan> But maybe this patch is OK anyway although the cleanup could be
Jan> more complete.

Jan> (Not yet checked in.)

Either way is ok by me.

Tom

Re: [patch 1/2] Code cleanup: New init_one_comp_unit

[Jan Kratochvil]

On Fri, 12 Nov 2010 19:50:38 +0100, Tom Tromey wrote:
> Either way is ok by me.

Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-11/msg00077.html

--- src/gdb/ChangeLog	2010/11/14 12:10:55	1.12311
+++ src/gdb/ChangeLog	2010/11/16 04:36:24	1.12312
@@ -1,3 +1,23 @@
+2010-11-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Code cleanup.
+	* dwarf2read.c (alloc_one_comp_unit): Rename prototype to ...
+	(init_one_comp_unit): ... this one.
+	(prepare_one_comp_unit): New prototype.
+	(dw2_require_line_header, process_psymtab_comp_unit): Use
+	init_one_comp_unit.
+	(process_psymtab_comp_unit): Use prepare_one_comp_unit.
+	(load_partial_comp_unit): Remove variable attr.  Use
+	init_one_comp_unit with xmalloc.  Use prepare_one_comp_unit.
+	(load_full_comp_unit): Use init_one_comp_unit with xmalloc.  Use
+	prepare_one_comp_unit.
+	(read_signatured_type): Remove variable attr.  Use init_one_comp_unit.
+	Use prepare_one_comp_unit.
+	(alloc_one_comp_unit): Rename to ...
+	(init_one_comp_unit): ... here and remove there calloc, new parameter
+	cu.
+	(prepare_one_comp_unit): New function.
+
 2010-11-14  Pierre Muller  <muller@ics.u-strasbg.fr>
 
 	* arm-tdep.c (arm_in_function_epilogue_p): Fix code to avoid
--- src/gdb/dwarf2read.c	2010/11/05 14:31:27	1.475
+++ src/gdb/dwarf2read.c	2010/11/16 04:36:26	1.476
@@ -1241,7 +1241,11 @@
 static struct dwarf2_per_cu_data *dwarf2_find_comp_unit
   (unsigned int offset, struct objfile *objfile);
 
-static struct dwarf2_cu *alloc_one_comp_unit (struct objfile *objfile);
+static void init_one_comp_unit (struct dwarf2_cu *cu,
+				struct objfile *objfile);
+
+static void prepare_one_comp_unit (struct dwarf2_cu *cu,
+				   struct die_info *comp_unit_die);
 
 static void free_one_comp_unit (void *);
 
@@ -2018,10 +2022,7 @@
     return;
   this_cu->v.quick->read_lines = 1;
 
-  memset (&cu, 0, sizeof (cu));
-  cu.objfile = objfile;
-  obstack_init (&cu.comp_unit_obstack);
-
+  init_one_comp_unit (&cu, objfile);
   cleanups = make_cleanup (free_stack_comp_unit, &cu);
 
   if (this_cu->from_debug_types)
@@ -3024,10 +3025,7 @@
   CORE_ADDR best_lowpc = 0, best_highpc = 0;
   struct die_reader_specs reader_specs;
 
-  memset (&cu, 0, sizeof (cu));
-  cu.objfile = objfile;
-  obstack_init (&cu.comp_unit_obstack);
-
+  init_one_comp_unit (&cu, objfile);
   back_to_inner = make_cleanup (free_stack_comp_unit, &cu);
 
   info_ptr = partial_read_comp_unit_head (&cu.header, info_ptr,
@@ -3081,12 +3079,7 @@
       return info_ptr;
     }
 
-  /* Set the language we're debugging.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_language, &cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), &cu);
-  else
-    set_cu_language (language_minimal, &cu);
+  prepare_one_comp_unit (&cu, comp_unit_die);
 
   /* Allocate a new partial symbol table structure.  */
   attr = dwarf2_attr (comp_unit_die, DW_AT_name, &cu);
@@ -3301,7 +3294,6 @@
   struct die_info *comp_unit_die;
   struct dwarf2_cu *cu;
   struct cleanup *free_abbrevs_cleanup, *free_cu_cleanup = NULL;
-  struct attribute *attr;
   int has_children;
   struct die_reader_specs reader_specs;
   int read_cu = 0;
@@ -3314,7 +3306,8 @@
 
   if (this_cu->cu == NULL)
     {
-      cu = alloc_one_comp_unit (objfile);
+      cu = xmalloc (sizeof (*cu));
+      init_one_comp_unit (cu, objfile);
 
       read_cu = 1;
 
@@ -3354,12 +3347,7 @@
   info_ptr = read_full_die (&reader_specs, &comp_unit_die, info_ptr,
 			    &has_children);
 
-  /* Set the language we're debugging.  */
-  attr = dwarf2_attr (comp_unit_die, DW_AT_language, cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), cu);
-  else
-    set_cu_language (language_minimal, cu);
+  prepare_one_comp_unit (cu, comp_unit_die);
 
   /* Check if comp unit has_children.
      If so, read the rest of the partial symbols from this comp unit.
@@ -4314,7 +4302,8 @@
 
   if (per_cu->cu == NULL)
     {
-      cu = alloc_one_comp_unit (objfile);
+      cu = xmalloc (sizeof (*cu));
+      init_one_comp_unit (cu, objfile);
 
       read_cu = 1;
 
@@ -4352,11 +4341,7 @@
      all objfiles needed for references have been loaded yet, and symbol
      table processing isn't initialized.  But we have to set the CU language,
      or we won't be able to build types correctly.  */
-  attr = dwarf2_attr (cu->dies, DW_AT_language, cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), cu);
-  else
-    set_cu_language (language_minimal, cu);
+  prepare_one_comp_unit (cu, cu->dies);
 
   /* Similarly, if we do not read the producer, we can not apply
      producer-specific interpretation.  */
@@ -13220,17 +13205,15 @@
   struct dwarf2_cu *cu;
   ULONGEST signature;
   struct cleanup *back_to, *free_cu_cleanup;
-  struct attribute *attr;
 
   dwarf2_read_section (objfile, &dwarf2_per_objfile->types);
   types_ptr = dwarf2_per_objfile->types.buffer + type_sig->offset;
 
   gdb_assert (type_sig->per_cu.cu == NULL);
 
-  cu = xmalloc (sizeof (struct dwarf2_cu));
-  memset (cu, 0, sizeof (struct dwarf2_cu));
-  obstack_init (&cu->comp_unit_obstack);
-  cu->objfile = objfile;
+  cu = xmalloc (sizeof (*cu));
+  init_one_comp_unit (cu, objfile);
+
   type_sig->per_cu.cu = cu;
   cu->per_cu = &type_sig->per_cu;
 
@@ -13262,11 +13245,7 @@
      all objfiles needed for references have been loaded yet, and symbol
      table processing isn't initialized.  But we have to set the CU language,
      or we won't be able to build types correctly.  */
-  attr = dwarf2_attr (cu->dies, DW_AT_language, cu);
-  if (attr)
-    set_cu_language (DW_UNSND (attr), cu);
-  else
-    set_cu_language (language_minimal, cu);
+  prepare_one_comp_unit (cu, cu->dies);
 
   do_cleanups (back_to);
 
@@ -14314,15 +14293,29 @@
   return this_cu;
 }
 
-/* Malloc space for a dwarf2_cu for OBJFILE and initialize it.  */
+/* Initialize dwarf2_cu CU for OBJFILE in a pre-allocated space.  */
 
-static struct dwarf2_cu *
-alloc_one_comp_unit (struct objfile *objfile)
+static void
+init_one_comp_unit (struct dwarf2_cu *cu, struct objfile *objfile)
 {
-  struct dwarf2_cu *cu = xcalloc (1, sizeof (struct dwarf2_cu));
+  memset (cu, 0, sizeof (*cu));
   cu->objfile = objfile;
   obstack_init (&cu->comp_unit_obstack);
-  return cu;
+}
+
+/* Initialize basic fields of dwarf_cu CU according to DIE COMP_UNIT_DIE.  */
+
+static void
+prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die)
+{
+  struct attribute *attr;
+
+  /* Set the language we're debugging.  */
+  attr = dwarf2_attr (comp_unit_die, DW_AT_language, cu);
+  if (attr)
+    set_cu_language (DW_UNSND (attr), cu);
+  else
+    set_cu_language (language_minimal, cu);
 }
 
 /* Release one cached compilation unit, CU.  We unlink it from the tree