[1/2] Consolidate CU language setting

Message ID 20210608152630.4155280-2-tom@tromey.com
State New
Headers show
Series
  • Clean up language handling in the DWARF reader
Related show

Commit Message

Tom Tromey June 8, 2021, 3:26 p.m.
The DWARF reader currently sets the CU's language in two different
spots.  It is primarily done in prepare_one_comp_unit, but
read_file_scope also checks the producer and may change the language
based on the result.

This patch consolidates all language-setting into
prepare_one_comp_unit.  set_cu_language is changed not to set
language_defn; instead that is done in prepare_one_comp_unit after the
correct language enum value is chosen.

This fixes a minor latent bug, which is that read_file_scope could set
the language enum value to language_opencl, but then neglected to
reset language_defn in this case.

2021-06-06  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (read_file_scope): Don't call set_cu_language.
	(set_cu_language): Don't set language_defn.
	(prepare_one_comp_unit): Check producer and set language_defn.
---
 gdb/ChangeLog     |  6 ++++++
 gdb/dwarf2/read.c | 34 ++++++++++++++++++----------------
 2 files changed, 24 insertions(+), 16 deletions(-)

-- 
2.26.3

Comments

Metzger, Markus T via Gdb-patches June 9, 2021, 1:32 a.m. | #1
On 2021-06-08 11:26 a.m., Tom Tromey wrote:
> @@ -24413,17 +24402,30 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,

>  {

>    struct attribute *attr;

>  

> +  cu->producer = dwarf2_string_attr (comp_unit_die, DW_AT_producer, cu);

> +

>    /* Set the language we're debugging.  */

>    attr = dwarf2_attr (comp_unit_die, DW_AT_language, cu);

>    if (attr != nullptr)

>      set_cu_language (attr->constant_value (0), cu);


Not very important, but I'd even change set_cu_language to be a pure
DWARF lang value -> enum language version function, and write:

  cu->language = dwarf_language_to_gdb_language (attr->constant_value (0));

That would be consistent with the rest of the function which assigns
cu->language.

> -  else

> +  else if (cu->producer != nullptr

> +	   && strstr (cu->producer, "IBM XL C for OpenCL") != NULL)

>      {


I don't have build artifacts to test, but I'm curious whether that
compiler still puts a DW_AT_language value in the CU's DIE, just another
language than OpenCL.  Because DW_AT_language existed before, it's just
DW_LANG_OpenCL that didn't exist.  If so, I think the behavior would
change, because that branch of the condition would not be taken - the
first one would.

I also noticed that DW_LANG_OpenCL isn't handled in set_cu_language...
it probably should, so that a producer using the right value
(DW_LANG_OpenCL) in DW_AT_language will get the language set correctly.

> -      cu->language = pretend_language;

> -      cu->language_defn = language_def (cu->language);

> +      /* The XLCL doesn't generate DW_LANG_OpenCL because this

> +	 attribute is not standardised yet.  As a workaround for the

> +	 language detection we fall back to the DW_AT_producer

> +	 string.  */

> +      cu->language = language_opencl;

>      }

> -

> -  cu->producer = dwarf2_string_attr (comp_unit_die, DW_AT_producer, cu);

> +  else if (cu->producer != nullptr

> +	   && strstr (cu->producer, "GNU Go ") != NULL)


Same here, did some version of gccgo put some other language in
DW_AT_language?

Simon
Tom Tromey June 9, 2021, 8:22 p.m. | #2
Simon> Not very important, but I'd even change set_cu_language to be a pure
Simon> DWARF lang value -> enum language version function, and write:

Yeah, I'll do that.

Simon> I don't have build artifacts to test, but I'm curious whether that
Simon> compiler still puts a DW_AT_language value in the CU's DIE, just another
Simon> language than OpenCL.  Because DW_AT_language existed before, it's just
Simon> DW_LANG_OpenCL that didn't exist.  If so, I think the behavior would
Simon> change, because that branch of the condition would not be taken - the
Simon> first one would.

Thanks for noticing that.  I'll fix it.

Simon> I also noticed that DW_LANG_OpenCL isn't handled in set_cu_language...
Simon> it probably should, so that a producer using the right value
Simon> (DW_LANG_OpenCL) in DW_AT_language will get the language set correctly.

I'll add this too.

Tom

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 96009f1418f..b1a0b8bce88 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -10509,16 +10509,6 @@  read_file_scope (struct die_info *die, struct dwarf2_cu *cu)
 
   file_and_directory fnd = find_file_and_directory (die, cu);
 
-  /* The XLCL doesn't generate DW_LANG_OpenCL because this attribute is not
-     standardised yet.  As a workaround for the language detection we fall
-     back to the DW_AT_producer string.  */
-  if (cu->producer && strstr (cu->producer, "IBM XL C for OpenCL") != NULL)
-    cu->language = language_opencl;
-
-  /* Similar hack for Go.  */
-  if (cu->producer && strstr (cu->producer, "GNU Go ") != NULL)
-    set_cu_language (DW_LANG_Go, cu);
-
   cu->start_symtab (fnd.name, fnd.comp_dir, lowpc);
 
   /* Decode line number information if present.  We do this before
@@ -20407,7 +20397,6 @@  set_cu_language (unsigned int lang, struct dwarf2_cu *cu)
       cu->language = language_minimal;
       break;
     }
-  cu->language_defn = language_def (cu->language);
 }
 
 /* Return the named attribute or NULL if not there.  */
@@ -24413,17 +24402,30 @@  prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die,
 {
   struct attribute *attr;
 
+  cu->producer = dwarf2_string_attr (comp_unit_die, DW_AT_producer, cu);
+
   /* Set the language we're debugging.  */
   attr = dwarf2_attr (comp_unit_die, DW_AT_language, cu);
   if (attr != nullptr)
     set_cu_language (attr->constant_value (0), cu);
-  else
+  else if (cu->producer != nullptr
+	   && strstr (cu->producer, "IBM XL C for OpenCL") != NULL)
     {
-      cu->language = pretend_language;
-      cu->language_defn = language_def (cu->language);
+      /* The XLCL doesn't generate DW_LANG_OpenCL because this
+	 attribute is not standardised yet.  As a workaround for the
+	 language detection we fall back to the DW_AT_producer
+	 string.  */
+      cu->language = language_opencl;
     }
-
-  cu->producer = dwarf2_string_attr (comp_unit_die, DW_AT_producer, cu);
+  else if (cu->producer != nullptr
+	   && strstr (cu->producer, "GNU Go ") != NULL)
+    {
+      /* Similar hack for Go.  */
+      cu->language = language_go;
+    }
+  else
+    cu->language = pretend_language;
+  cu->language_defn = language_def (cu->language);
 }
 
 /* See read.h.  */