[PATCH] Enhance array types debug info. for Ada

[Pierre-Marie de Rodat]

Hi!

I'm currently working on improving the debug information output for GNAT
(the Ada frontend in GCC), which currently uses non-standard DWARF to
describe complex types. Lately, I focused on debug information for
arrays and the attached inter-dependent patches are an attempt to do so:

- they enhance the existing "array_descr_info" language hook;
- they adjust the Fortran front-end accordingly (it's the only
array_descr_info user currently);
- they make the Ada front-end use this hook.

Here are more details about what motivated each patch:


1. This first patch enhances the array_descr_info language hook so
that front-end can pass more information about array types to the DWARF
backend:

- Array ordering (column/row major) so that the information that
Fortran arrays are column major ordered comes from the Fortran front-end
and so that later GNAT can decide itself for each array type (they can
be both column and row major ordered).

- Bounds type: Ada arrays can be indexed by integers but also
characters, enumerated types, etc. However it seems that the middle-end
makes the assumption that every array index is sizetype so this
information is needed here for accurate debug info.

It also makes the language hook generate "GNAT descriptive type"
attributes for array types, just as the regular array types handling in
dwarf2out.c does.

Finally, it makes the DWARF back-end initialize the
"array_descr_info" structure so that new fields can be added to it later
without affecting existing front-ends that use this hook.


2. Currently, this language hook is enabled only when (dwarf_version

= 3 || !dwarf_strict). The hook generates information that is mostly

valid for strict DWARFv2, though. The second patch enables this language
hook every time and instead prevents the emission for some attributes
when needed.


3. This one enables the array_descr_info hook in GNAT.


4. This one enhances debug helpers in dwarf2out.c to ease location
descriptions (DWARF expressions) bugs investigation.


5. The array_descr_info hook has its own circuitry in dwarf2out.c to
generate location description: add_descr_info_field. It is a duplicate
of loc_list_from_tree and less powerful except that it handles
"self-referencial attributes". This final patch is an attempt to merge
these two circuitries so that this hook can generate more complex DWARF
expressions. It also adjusts the Fortran front-end accordingly.


These patches were tested on x86_64-pc-linux-gnu. They trigger no
regression in the GCC DejaGNU testsuite nor in the GDB one (they fix
some failures however).

Ok for trunk?

Thank you very much for reading until this point and thank you in
advance for your review! ;-)

[Pierre-Marie de Rodat]

Ping for https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00206.html

Adding a few maintainers in copy... Thanks in advance!

[Pierre-Marie de Rodat]

Post by Pierre-Marie de Rodat
Ping for https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00206.html
Adding a few maintainers in copy... Thanks in advance!

Should I enhance something in this patch set in order to make the review
easier? Thanks!

[Jason Merrill]

On 09/17/2014 10:38 AM, Pierre-Marie de Rodat wrote:

Patches 1-4 are OK.

+ bool pell_conversions = true;

I don't understand "pell". Do you mean "strip"?

Jason

[Pierre-Marie de Rodat]

Post by Jason Merrill
Patches 1-4 are OK.

+ bool pell_conversions = true;

I don't understand "pell". Do you mean "strip"?

Absolutely: I though it was correct English. I replaced all occurences
of "pell" with "strip". Updates patches will follow...

Thank you very much for your review! :-)

[Jakub Jelinek]

--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -17359,18 +17359,36 @@ static void
gen_descr_array_type_die (tree type, struct array_descr_info *info,
dw_die_ref context_die)
{
- dw_die_ref scope_die = scope_die_for (type, context_die);
+ dw_die_ref scope_die;
dw_die_ref array_die;
int dim;
+ /* Instead of producing a dedicated DW_TAG_array_type DIE for this type, let
+ the circuitry wrap the main variant with DIEs for qualifiers (for
+ instance: DW_TAG_const_type, ...). */
+ if (type != TYPE_MAIN_VARIANT (type))
+ {
+ gen_type_die (TYPE_MAIN_VARIANT (type), context_die);
+ return;
+ }

I don't like this, can you explain why? I'd say that if you only want
to see TYPE_MAIN_VARIANT here, it should be responsibility of the callers
to ensure that.

@@ -19941,7 +19991,8 @@ gen_type_die_with_usage (tree type, dw_die_ref context_die,
/* If this is an array type with hidden descriptor, handle it first. */
if (!TREE_ASM_WRITTEN (type)
&& lang_hooks.types.get_array_descr_info
- && lang_hooks.types.get_array_descr_info (type, &info)
+ && lang_hooks.types.get_array_descr_info (type,
+ init_array_descr_info (&info))

Just memset it to 0 instead?

+ enum array_descr_ordering ordering;
tree element_type;
tree base_decl;
tree data_location;
tree allocated;
tree associated;
+

Why the extra vertical space?

struct array_descr_dimen
{

From 0d683ca8c1fcf8d780928f1cd629e7a99651c9c0 Mon Sep 17 00:00:00 2001

Date: Wed, 3 Sep 2014 09:46:25 +0200
Subject: [PATCH 2/5] Enable the array descr language hook for all DWARF
versions
* dwarf2out.c (gen_type_die_with_usage): Enable the array lang-hook
even when (dwarf_version < 3 && dwarf_strict).
(gen_descr_array_die): Do not output DW_AT_data_locationn,
DW_AT_associated, DW_AT_allocated and DW_AT_byte_stride DWARF
attributes when (dwarf_version < 3 && dwarf_strict).

This patch sounds very wrong. DW_OP_push_object_address is not in DWARF2
either, and that is the basis of all the fields, so there is really nothing
you can really output correctly for DWARF2. It isn't the default on sane
targets, where GCC defaults to DWARF4 these days, so why bother?

#include "real.h"
#include "function.h" /* For pass_by_reference. */
+#include "dwarf2out.h"
#include "ada.h"
#include "adadecode.h"
@@ -626,6 +627,64 @@ gnat_type_max_size (const_tree gnu_type)
return max_unitsize;
}
+/* Provide information in INFO for debug output about the TYPE array type.
+ Return whether TYPE is handled. */
+
+static bool
+gnat_get_array_descr_info (const_tree type, struct array_descr_info *info)
+{
+ bool convention_fortran_p;
+ tree index_type;
+
+ const_tree dimen, last_dimen;
+ int i;
+
+ if (TREE_CODE (type) != ARRAY_TYPE
+ || !TYPE_DOMAIN (type)
+ || !TYPE_INDEX_TYPE (TYPE_DOMAIN (type)))
+ return false;
+
+ /* Count how many dimentions this array has. */
+ for (i = 0, dimen = type; ; ++i, dimen = TREE_TYPE (dimen))
+ if (i > 0
+ && (TREE_CODE (dimen) != ARRAY_TYPE
+ || !TYPE_MULTI_ARRAY_P (dimen)))
+ break;
+ info->ndimensions = i;
+ convention_fortran_p = TYPE_CONVENTION_FORTRAN_P (type);
+
+ /* TODO??? For row major ordering, we probably want to emit nothing and
+ instead specify it as the default in Dw_TAG_compile_unit. */
+ info->ordering = (convention_fortran_p
+ ? array_descr_ordering_column_major
+ : array_descr_ordering_row_major);
+ info->base_decl = NULL_TREE;
+ info->data_location = NULL_TREE;
+ info->allocated = NULL_TREE;
+ info->associated = NULL_TREE;
+
+ for (i = (convention_fortran_p ? info->ndimensions - 1 : 0),
+ dimen = type;
+
+ 0 <= i && i < info->ndimensions;
+
+ i += (convention_fortran_p ? -1 : 1),
+ dimen = TREE_TYPE (dimen))
+ {
+ /* We are interested in the stored bounds for the debug info. */
+ index_type = TYPE_INDEX_TYPE (TYPE_DOMAIN (dimen));
+
+ info->dimen[i].bounds_type = index_type;
+ info->dimen[i].lower_bound = TYPE_MIN_VALUE (index_type);
+ info->dimen[i].upper_bound = TYPE_MAX_VALUE (index_type);
+ last_dimen = dimen;
+ }
+
+ info->element_type = TREE_TYPE (last_dimen);
+
+ return true;
+}
+
/* GNU_TYPE is a subtype of an integral type. Set LOWVAL to the low bound
and HIGHVAL to the high bound, respectively. */
@@ -916,6 +975,8 @@ gnat_init_ts (void)
#define LANG_HOOKS_TYPE_FOR_SIZE gnat_type_for_size
#undef LANG_HOOKS_TYPES_COMPATIBLE_P
#define LANG_HOOKS_TYPES_COMPATIBLE_P gnat_types_compatible_p
+#undef LANG_HOOKS_GET_ARRAY_DESCR_INFO
+#define LANG_HOOKS_GET_ARRAY_DESCR_INFO gnat_get_array_descr_info
#undef LANG_HOOKS_GET_SUBRANGE_BOUNDS
#define LANG_HOOKS_GET_SUBRANGE_BOUNDS gnat_get_subrange_bounds
#undef LANG_HOOKS_DESCRIPTIVE_TYPE
--
2.1.0

From 166fcbad8529818e492c57b7b9091799bf3ae72d Mon Sep 17 00:00:00 2001

Date: Wed, 3 Sep 2014 09:46:29 +0200
Subject: [PATCH 4/5] Add a few debug utilities for DWARF expressions
* dwarf2out.c (print_loc_descr): New.
(print_dw_val): New.
(print_attribute): New.
(print_loc_descr): New.
(print_die): Use print_dw_val.
(debug_dwarf_loc_descr): New.
* dwarf2out.h (debug_dwarf_loc_descr): New declaration.
---
gcc/dwarf2out.c | 277 +++++++++++++++++++++++++++++++++++---------------------
gcc/dwarf2out.h | 1 +
2 files changed, 176 insertions(+), 102 deletions(-)
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 78a470f..1638da4 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -5337,6 +5337,172 @@ print_signature (FILE *outfile, char *sig)
fprintf (outfile, "%02x", sig[i] & 0xff);
}
+static void print_loc_descr (dw_loc_descr_ref, FILE *);
+
+/* Print the value associated to the VAL DWARF value node to OUTFILE. If
+ RECURSE, output location descriptor operations. */
+
+static void
+print_dw_val (dw_val_node *val, bool recurse, FILE *outfile)
+{
+ switch (val->val_class)
+ {
+ fprintf (outfile, "address");
+ break;
+ fprintf (outfile, "offset");
+ break;
+ fprintf (outfile, "location descriptor");
+ if (val->v.val_loc == NULL)
+ fprintf (outfile, " -> <null>\n");
+ else if (recurse)
+ {
+ fprintf (outfile, ":\n");
+ print_indent += 4;
+ print_loc_descr (val->v.val_loc, outfile);
+ print_indent -= 4;
+ }
+ else
+ fprintf (outfile, " (%p)\n", (void *) val->v.val_loc);
+ break;
+ fprintf (outfile, "location list -> label:%s",
+ val->v.val_loc_list->ll_symbol);
+ break;
+ fprintf (outfile, "range list");
+ break;
+ fprintf (outfile, HOST_WIDE_INT_PRINT_DEC, val->v.val_int);
+ break;
+ fprintf (outfile, HOST_WIDE_INT_PRINT_UNSIGNED, val->v.val_unsigned);
+ break;
+ fprintf (outfile, "constant ("HOST_WIDE_INT_PRINT_DEC","\
+ HOST_WIDE_INT_PRINT_UNSIGNED")",
+ val->v.val_double.high,
+ val->v.val_double.low);
+ break;
+ {
+ int i = val->v.val_wide->get_len ();
+ fprintf (outfile, "constant (");
+ gcc_assert (i > 0);
+ if (val->v.val_wide->elt (i - 1) == 0)
+ fprintf (outfile, "0x");
+ fprintf (outfile, HOST_WIDE_INT_PRINT_HEX,
+ val->v.val_wide->elt (--i));
+ while (--i >= 0)
+ fprintf (outfile, HOST_WIDE_INT_PRINT_PADDED_HEX,
+ val->v.val_wide->elt (i));
+ fprintf (outfile, ")");
+ break;
+ }
+ fprintf (outfile, "floating-point or vector constant");
+ break;
+ fprintf (outfile, "%u", val->v.val_flag);
+ break;
+ if (val->v.val_die_ref.die != NULL)
+ {
+ dw_die_ref die = val->v.val_die_ref.die;
+
+ if (die->comdat_type_p)
+ {
+ fprintf (outfile, "die -> signature: ");
+ print_signature (outfile,
+ die->die_id.die_type_node->signature);
+ }
+ else if (die->die_id.die_symbol)
+ fprintf (outfile, "die -> label: %s", die->die_id.die_symbol);
+ else
+ fprintf (outfile, "die -> %ld", die->die_offset);
+ fprintf (outfile, " (%p)", (void *) die);
+ }
+ else
+ fprintf (outfile, "die -> <null>");
+ break;
+ val->v.val_vms_delta.lbl2, val->v.val_vms_delta.lbl1);
+ break;
+ fprintf (outfile, "label: %s", val->v.val_lbl_id);
+ break;
+ if (val->v.val_str->str != NULL)
+ fprintf (outfile, "\"%s\"", val->v.val_str->str);
+ else
+ fprintf (outfile, "<null>");
+ break;
+ fprintf (outfile, "\"%s\" (%d)", val->v.val_file->filename,
+ val->v.val_file->emitted_number);
+ break;
+ {
+ int i;
+
+ for (i = 0; i < 8; i++)
+ fprintf (outfile, "%02x", val->v.val_data8[i]);
+ break;
+ }
+ break;
+ }
+}
+
+/* Likewise, for a DIE attribute. */
+
+static void
+print_attribute (dw_attr_ref a, bool recurse, FILE *outfile)
+{
+ print_dw_val (&a->dw_attr_val, recurse, outfile);
+}
+
+/* Print the list of operands in the LOC location description to OUTFILE. This
+ routine is a debugging aid only. */
+
+static void
+print_loc_descr (dw_loc_descr_ref loc, FILE *outfile)
+{
+ dw_loc_descr_ref l = loc;
+
+ if (loc == NULL)
+ {
+ print_spaces (outfile);
+ fprintf (outfile, "<null>\n");
+ return;
+ }
+
+ for (l = loc; l != NULL; l = l->dw_loc_next)
+ {
+ print_spaces (outfile);
+ fprintf (outfile, "(%p) %s",
+ (void *) l,
+ dwarf_stack_op_name (l->dw_loc_opc));
+ if (l->dw_loc_oprnd1.val_class != dw_val_class_none)
+ {
+ fprintf (outfile, " ");
+ print_dw_val (&l->dw_loc_oprnd1, false, outfile);
+ }
+ if (l->dw_loc_oprnd2.val_class != dw_val_class_none)
+ {
+ fprintf (outfile, ", ");
+ print_dw_val (&l->dw_loc_oprnd2, false, outfile);
+ }
+ fprintf (outfile, "\n");
+ }
+}
+
/* Print the information associated with a given DIE, and its children.
This routine is a debugging aid only. */
@@ -5369,108 +5535,7 @@ print_die (dw_die_ref die, FILE *outfile)
print_spaces (outfile);
fprintf (outfile, " %s: ", dwarf_attr_name (a->dw_attr));
- switch (AT_class (a))
- {
- fprintf (outfile, "address");
- break;
- fprintf (outfile, "offset");
- break;
- fprintf (outfile, "location descriptor");
- break;
- fprintf (outfile, "location list -> label:%s",
- AT_loc_list (a)->ll_symbol);
- break;
- fprintf (outfile, "range list");
- break;
- fprintf (outfile, HOST_WIDE_INT_PRINT_DEC, AT_int (a));
- break;
- fprintf (outfile, HOST_WIDE_INT_PRINT_UNSIGNED, AT_unsigned (a));
- break;
- fprintf (outfile, "constant ("HOST_WIDE_INT_PRINT_DEC","\
- HOST_WIDE_INT_PRINT_UNSIGNED")",
- a->dw_attr_val.v.val_double.high,
- a->dw_attr_val.v.val_double.low);
- break;
- {
- int i = a->dw_attr_val.v.val_wide->get_len ();
- fprintf (outfile, "constant (");
- gcc_assert (i > 0);
- if (a->dw_attr_val.v.val_wide->elt (i - 1) == 0)
- fprintf (outfile, "0x");
- fprintf (outfile, HOST_WIDE_INT_PRINT_HEX,
- a->dw_attr_val.v.val_wide->elt (--i));
- while (--i >= 0)
- fprintf (outfile, HOST_WIDE_INT_PRINT_PADDED_HEX,
- a->dw_attr_val.v.val_wide->elt (i));
- fprintf (outfile, ")");
- break;
- }
- fprintf (outfile, "floating-point or vector constant");
- break;
- fprintf (outfile, "%u", AT_flag (a));
- break;
- if (AT_ref (a) != NULL)
- {
- if (AT_ref (a)->comdat_type_p)
- {
- fprintf (outfile, "die -> signature: ");
- print_signature (outfile,
- AT_ref (a)->die_id.die_type_node->signature);
- }
- else if (AT_ref (a)->die_id.die_symbol)
- fprintf (outfile, "die -> label: %s",
- AT_ref (a)->die_id.die_symbol);
- else
- fprintf (outfile, "die -> %ld", AT_ref (a)->die_offset);
- fprintf (outfile, " (%p)", (void *) AT_ref (a));
- }
- else
- fprintf (outfile, "die -> <null>");
- break;
- AT_vms_delta2 (a), AT_vms_delta1 (a));
- break;
- fprintf (outfile, "label: %s", AT_lbl (a));
- break;
- if (AT_string (a) != NULL)
- fprintf (outfile, "\"%s\"", AT_string (a));
- else
- fprintf (outfile, "<null>");
- break;
- fprintf (outfile, "\"%s\" (%d)", AT_file (a)->filename,
- AT_file (a)->emitted_number);
- break;
- {
- int i;
-
- for (i = 0; i < 8; i++)
- fprintf (outfile, "%02x", a->dw_attr_val.v.val_data8[i]);
- break;
- }
- break;
- }
-
+ print_attribute (a, true, outfile);
fprintf (outfile, "\n");
}
@@ -5484,6 +5549,14 @@ print_die (dw_die_ref die, FILE *outfile)
fprintf (outfile, "\n");
}
+/* Print the list of operations in the LOC location description. */
+
+DEBUG_FUNCTION void
+debug_dwarf_loc_descr (dw_loc_descr_ref loc)
+{
+ print_loc_descr (loc, stderr);
+}
+
/* Print the information collected for a given DIE. */
DEBUG_FUNCTION void
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index 8b03e78..fbcb70a 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -254,6 +254,7 @@ extern void dwarf2out_emit_cfi (dw_cfi_ref cfi);
extern void debug_dwarf (void);
struct die_struct;
extern void debug_dwarf_die (struct die_struct *);
+extern void debug_dwarf_loc_descr (dw_loc_descr_ref);
extern void debug (die_struct &ref);
extern void debug (die_struct *ptr);
extern void dwarf2out_set_demangle_name_func (const char *(*) (const char *));
--
2.1.0

From e029b9300c58a0ffbfa1b7f81381a937a60b27fd Mon Sep 17 00:00:00 2001

Date: Wed, 3 Sep 2014 09:46:32 +0200
Subject: [PATCH 5/5] dwarf2out.c: do not short-circuit add_bound_info in array
lang-hook
gcc/
* dwarf2out.h (struct array_descr_info): Remove the base_decl field.
* dwarf2out.c (init_array_descr_info): Update accordingly.
(enum dw_scalar_form): New.
(add_scalar_info): New.
(loc_list_from_tree): Handle PLACEHOLDER_EXPR nodes for
type-related expressions.
(add_bound_info): Use add_scalar_info.
(descr_info_loc): Remove.
(add_descr_info_field): Remove.
(gen_descr_array_type_die): Switch add_descr_info_field calls
into add_scalar_info/add_bound_info ones.
gcc/ada/
* gcc-interface/misc.c (gnat_get_array_descr_info): Remove base_decl
initialization.
gcc/fortran/
* trans-types.c (gfc_get_array_descr_info): Use PLACEHOLDER_EXPR nodes
instead of VAR_DECL ones in type-related expressions. Remove base_decl
initialization.

Ugh, I must say I don't like PLACEHOLDER_EXPRs at all.

Jakub

[Jakub Jelinek]

Post by Jakub Jelinek

gcc/fortran/
* trans-types.c (gfc_get_array_descr_info): Use PLACEHOLDER_EXPR nodes
instead of VAR_DECL ones in type-related expressions. Remove base_decl
initialization.

Ugh, I must say I don't like PLACEHOLDER_EXPRs at all.

What kind of more complex expressions do you need and why?

Jakub

[Pierre-Marie de Rodat]

Jakub,

First, thank you very much for reviewing this set of patches.

Post by Jakub Jelinek
What kind of more complex expressions do you need and why?

GNAT can produce array types that make sense only as part of a record
type and whose bounds are equal to members of this record type. Such
ARRAY_TYPE nodes get generated from the kind of example you could see on
the Dwarf-Discuss mailing list:

type Array_Type is array (Integer range <>) of Integer;
type Record_Type (N : Integer) is record
A : Array_Type (1 .. N);
end record;

In this case, the "A" field's type is an ARRAY_TYPE node whose upper
bound is:

COMPONENT_REF (PLACEHOLDER_EXPR (<Record_Type>),
FIELD_DECL("N"))

Upcoming patches will actually extend the need to handle more complex
expressions: Ada arrays can contain dynamically-sized objects (their
size is bounded, though). As a consequence, debuggers need these arrays
to have a DW_AT_byte_stride attribute in order to decode them. The size
expressions that describe the array stride in GCC can contain fairly
complex operations such as unsigned divisions, unsigned comparisons,
bitwise operations, calls to size functions (see
stor-layout.c:self_referencial_size).

Post by Jakub Jelinek

+ /* Instead of producing a dedicated DW_TAG_array_type DIE for this type, let
+ the circuitry wrap the main variant with DIEs for qualifiers (for
+ instance: DW_TAG_const_type, ...). */
+ if (type != TYPE_MAIN_VARIANT (type))
+ {
+ gen_type_die (TYPE_MAIN_VARIANT (type), context_die);
+ return;
+ }

I don't like this, can you explain why? I'd say that if you only
want to see TYPE_MAIN_VARIANT here, it should be responsibility of
the callers to ensure that.

Agreed. I have updated the patch to:

1. remove this hunk;
2. in gen_type_die_with_usage, which is the only caller, move the
type_main_variant call on "type" right before the array descriptors
handling.

Post by Jakub Jelinek

@@ -19941,7 +19991,8 @@ gen_type_die_with_usage (tree type, dw_die_ref context_die,
/* If this is an array type with hidden descriptor, handle itfirst. */
if (!TREE_ASM_WRITTEN (type)
&& lang_hooks.types.get_array_descr_info
- && lang_hooks.types.get_array_descr_info (type, &info)
+ && lang_hooks.types.get_array_descr_info (type,
+ init_array_descr_info (&info))

Just memset it to 0 instead?

Sure. I was not sure about whether is was considered good style, but
it's done, now.

Post by Jakub Jelinek

+ enum array_descr_ordering ordering;
tree element_type;
tree base_decl;
tree data_location;
tree allocated;
tree associated;
+

Why the extra vertical space?

struct array_descr_dimen
{

It made the separation between "global" and "dimension-local"
information clearer to me. As I suppose you don't like it and as there
is already one indentation level, I removed it.

Post by Jakub Jelinek

* dwarf2out.c (gen_type_die_with_usage): Enable the array lang-hook
even when (dwarf_version < 3 && dwarf_strict).
(gen_descr_array_die): Do not output DW_AT_data_locationn,
DW_AT_associated, DW_AT_allocated and DW_AT_byte_stride DWARF
attributes when (dwarf_version < 3 && dwarf_strict).

This patch sounds very wrong. DW_OP_push_object_address is not in DWARF2
either, and that is the basis of all the fields, so there is reallynothing
you can really output correctly for DWARF2. It isn't the default on sane
targets, where GCC defaults to DWARF4 these days, so why bother?

Generating DW_OP_push_object_address in strict DWARF2 mode is indeed a
bug (patch is adjusted). However, if I understand correctly all
fields/attributes don't have to rely on it.

In the case of the first Ada example I quoted above, such an operation
would not be emitted: instead, add_bound_info/add_scalar_info are going
to output a DW_AT_upper_bound attribute that is a reference to another
DIE. This is valid DWARF2 and, I think, justifies enabling the language
hook in this case.

We have several platforms whose default to strict DWARF2. These are
quite used platforms on which some DWARF consumers crash when provided
DIEs and tags they do not handle.

Post by Jakub Jelinek

gcc/fortran/
* trans-types.c (gfc_get_array_descr_info): Use PLACEHOLDER_EXPR nodes
instead of VAR_DECL ones in type-related expressions. Remove base_decl
initialization.

Ugh, I must say I don't like PLACEHOLDER_EXPRs at all.

Why so? I know that as far as supported front-ends are concerned,
PLACEHOLDE_EXPR nodes are used only in GNAT, but it seems to me they
describe the best what object the bound/stride/allocated/associated
expressions (self-)reference.

I have attached to this mail the 3 patches that are updated thanks to
your (Jakub and Jason's) comments and run successfuly the GCC testsuite
on x86_64-pc-linux-gnu.

Thanks again for revewing!

[Jakub Jelinek]

Post by Pierre-Marie de Rodat

Post by Jakub Jelinek

gcc/fortran/
* trans-types.c (gfc_get_array_descr_info): Use PLACEHOLDER_EXPR nodes
instead of VAR_DECL ones in type-related expressions. Remove base_decl
initialization.

Ugh, I must say I don't like PLACEHOLDER_EXPRs at all.

Why so? I know that as far as supported front-ends are concerned,
PLACEHOLDE_EXPR nodes are used only in GNAT, but it seems to me they
describe the best what object the bound/stride/allocated/associated
expressions (self-)reference.

But isn't there a risk that you will have PLACEHOLDER_EXPRs (likely for Ada
only) in some trees not constructed by the langhook?
I mean, DW_OP_push_object_address isn't meaningful in all DWARF contexts,
in some it is forbidden, in others there is really no object to push, and as
implemented, you emit DW_OP_push_object_address (which emits the address of
a context related particular object) for any kind of PLACEHOLDER_EXPR with
RECORD_TYPE.

Thus, I'd feel safer, even if you decide to use a PLACEHOLDER_EXPR, that
the translation of that to DW_OP_push_object_address would be done only
if the PLACEHOLDER_EXPR is equal to some global variable, normally NULL,
and only changed temporarily while emitting loc for the array descriptor.
But then IMHO a DEBUG_EXPR_DECL is better.

That said, if Jason is fine with the patchset as is, I can live with it,
as other FEs don't use PLACEHOLDER_EXPRs, worst case it will affect Ada
only.
Also, please verify that with your patch the generated debug info for some
Fortran arrays is the same.

Jakub

[Pierre-Marie de Rodat]

Post by Jakub Jelinek
But isn't there a risk that you will have PLACEHOLDER_EXPRs (likely for Ada
only) in some trees not constructed by the langhook?
I mean, DW_OP_push_object_address isn't meaningful in all DWARF contexts,
in some it is forbidden, in others there is really no object to push, and as
implemented, you emit DW_OP_push_object_address (which emits the address of
a context related particular object) for any kind of PLACEHOLDER_EXPR with
RECORD_TYPE.

Even with GNAT, this is not _supposed_ to happen. However during the
development (for instance with LTO) I noticed cases where
PLACEHOLDER_EXPR nodes were incorrectly used. Thanks to current work on
the early debug info pass, such cases are doomed to disappear, but I
completely agree with your point, so thank you for raising it. :-)

Post by Jakub Jelinek
Thus, I'd feel safer, even if you decide to use a PLACEHOLDER_EXPR, that
the translation of that to DW_OP_push_object_address would be done only
if the PLACEHOLDER_EXPR is equal to some global variable, normally NULL,
and only changed temporarily while emitting loc for the array descriptor.

This is what the updated (and attached) patch does. Note that upcoming
patches will enhance loc_list_from_tree (adding another parameter to
loc_list_from_tree) and make it recurse to generate sub-expressions as
DWARF procedures. Because of this kind recursion, I added a composite
argument instead of relying on a global variable (so that "nested"
contexts can exist at the same time).

Post by Jakub Jelinek
But then IMHO a DEBUG_EXPR_DECL is better.
[...]
Also, please verify that with your patch the generated debug info for some
Fortran arrays is the same.

It's fortunate that you asked this since I wrongly assumed there was the
corresponding testing in the GDB testsuite. As a matter of fact, support
for Fortran's variable length arrays in GDB is still a work in progress
so tests are not commited yet. So I used the Fortran example I could
find there <http://intel-gdb.github.io/> instead and discovered that my
patch did break debugging information for Fortran array types.

Fixing it while keeping a PLACEHOLDER_EXPR-based implementation seems a
too heavy task for my little experience in the Fortran front-end and
after having a closer look I agree with you: it seems less adapted to
how things are currently done, there. So I finally leveraged this new
composite argument to re-introduce the base_decl mechanism.
DEBUG_EXPR_DECL is back in the Fortran front-end. ;-) Now, the same
example keeps the same debugging information.

The latest patches bootstrapped well and passed successfully the GCC
testsuite on x86_64-pc-linux-gnu.