On Mar 2, 2018, Alexandre Oliva <aol...@redhat.com> wrote:
> Mark Wielaard is implementing support for LVU and IEPM in elfutils, and
> he was surprised by the encoding of DW_AT_GNU_entry_view; so was I!
> When GCC computes and outputs views internally (broken without internal
> view resets), it outputs entry_view attributes as data: it knows the
> view numbers. However, when it uses the assembler to compute the views
> symbolically, it has to output the view symbolically.
> I'd chosen lbl_id to that end, without giving it much thought, but that
> was no good: it's not just space-inefficient, it uses an addr encoding,
> indirect in some cases, for something that's not a real label, but
> rather a a small assembler data constant. Oops.
Jakub wrote (in the BZ):
> I meant to say:
> The char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view; line
> should come at the and of the union, not before the other classes.
I've moved the union field down in the revised patch below, but I don't
see the point, and I thought it would be better to keep it close to
logically-similar entries. If the point is just to make it parallel to
the order of the enum (which manh other entries don't seem to have cared
about), maybe moving the enum would be better?
> What guarantees the symview symbols always fit into 16 bits? Does
> something punt if it needs more than 65536 views?
There's no guarantee it will fit in 16 bits; the assembler will warn if
it truncates a view number. There's no real upper limit, so uleb128
would be ideal, but since that's not viable ATM, I had to pick
something, and 16 bits would cover all really useful cases than 32 or
even 64 bits would, while being more compact. I was even tempted to go
with 8 bits, but thought that was pushing it. I can make it 32 if you
consider it better. Or maybe a param?
> The FIXMEs don't really look helpful, we are not going to change the
> offset computation from compile time to assemble time, that would be
> terribly expensive.
Why do you say it would be terribly expensive to let the assembler compute
offsets in .debug_info?
[IEPM] [PR debug/84620] use constant form for DW_AT_GNU_entry_view
When outputting entry views in symbolic mode, we used to use a lbl_id,
but that outputs the view as an addr, perhaps even in an indirect one,
which is all excessive and undesirable for a small assembler-computed
constant.
Introduce a new value class for symbolic views, so that we can output
the labels as constant data, using small forms that should suffice for
any symbolic views.
Ideally, we'd use uleb128, but then the compiler would have to defer
.debug_info offset computation to the assembler. I'm not going there
for now, so a symbolic uleb128 assembler constant in an attribute is
not something GCC can deal with ATM.
for gcc/ChangeLog
PR debug/84620
* dwarf2out.h (dw_val_class): Add dw_val_class_symview.
(dw_val_node): Add val_symbolic_view.
* dwarf2out.c (dw_val_equal_p): Handle symview.
(add_AT_symview): New.
(print_dw_val): Handle symview.
(attr_checksum, attr_checksum_ordered): Likewise.
(same_dw_val_p, size_of_die): Likewise.
(value_format, output_die): Likewise.
(add_high_low_attributes): Use add_AT_symview for entry_view.
---
gcc/dwarf2out.c | 40 +++++++++++++++++++++++++++++++++++++++-
gcc/dwarf2out.h | 4 +++-
2 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 41bb11558f69..b52edee845f2 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1434,6 +1434,8 @@ dw_val_equal_p (dw_val_node *a, dw_val_node *b)
return a->v.val_die_ref.die == b->v.val_die_ref.die;
case dw_val_class_fde_ref:
return a->v.val_fde_index == b->v.val_fde_index;
+ case dw_val_class_symview:
+ return strcmp (a->v.val_symbolic_view, b->v.val_symbolic_view) == 0;
case dw_val_class_lbl_id:
case dw_val_class_lineptr:
case dw_val_class_macptr:
@@ -3600,6 +3602,7 @@ static addr_table_entry *add_addr_table_entry (void *,
enum ate_kind);
static void remove_addr_table_entry (addr_table_entry *);
static void add_AT_addr (dw_die_ref, enum dwarf_attribute, rtx, bool);
static inline rtx AT_addr (dw_attr_node *);
+static void add_AT_symview (dw_die_ref, enum dwarf_attribute, const char *);
static void add_AT_lbl_id (dw_die_ref, enum dwarf_attribute, const char *);
static void add_AT_lineptr (dw_die_ref, enum dwarf_attribute, const char *);
static void add_AT_macptr (dw_die_ref, enum dwarf_attribute, const char *);
@@ -5114,6 +5117,21 @@ add_AT_vms_delta (dw_die_ref die, enum dwarf_attribute
attr_kind,
add_dwarf_attr (die, &attr);
}
+/* Add a symbolic view identifier attribute value to a DIE. */
+
+static inline void
+add_AT_symview (dw_die_ref die, enum dwarf_attribute attr_kind,
+ const char *view_label)
+{
+ dw_attr_node attr;
+
+ attr.dw_attr = attr_kind;
+ attr.dw_attr_val.val_class = dw_val_class_symview;
+ attr.dw_attr_val.val_entry = NULL;
+ attr.dw_attr_val.v.val_symbolic_view = xstrdup (view_label);
+ add_dwarf_attr (die, &attr);
+}
+
/* Add a label identifier attribute value to a DIE. */
static inline void
@@ -6457,6 +6475,9 @@ print_dw_val (dw_val_node *val, bool recurse, FILE
*outfile)
fprintf (outfile, "delta: @slotcount(%s-%s)",
val->v.val_vms_delta.lbl2, val->v.val_vms_delta.lbl1);
break;
+ case dw_val_class_symview:
+ fprintf (outfile, "view: %s", val->v.val_symbolic_view);
+ break;
case dw_val_class_lbl_id:
case dw_val_class_lineptr:
case dw_val_class_macptr:
@@ -6828,6 +6849,7 @@ attr_checksum (dw_attr_node *at, struct md5_ctx *ctx, int
*mark)
case dw_val_class_fde_ref:
case dw_val_class_vms_delta:
+ case dw_val_class_symview:
case dw_val_class_lbl_id:
case dw_val_class_lineptr:
case dw_val_class_macptr:
@@ -7124,6 +7146,7 @@ attr_checksum_ordered (enum dwarf_tag tag, dw_attr_node
*at,
break;
case dw_val_class_fde_ref:
+ case dw_val_class_symview:
case dw_val_class_lbl_id:
case dw_val_class_lineptr:
case dw_val_class_macptr:
@@ -7624,6 +7647,9 @@ same_dw_val_p (const dw_val_node *v1, const dw_val_node
*v2, int *mark)
case dw_val_class_die_ref:
return same_die_p (v1->v.val_die_ref.die, v2->v.val_die_ref.die, mark);
+ case dw_val_class_symview:
+ return strcmp (v1->v.val_symbolic_view, v2->v.val_symbolic_view) == 0;
+
case dw_val_class_fde_ref:
case dw_val_class_vms_delta:
case dw_val_class_lbl_id:
@@ -9284,6 +9310,10 @@ size_of_die (dw_die_ref die)
size += csize;
}
break;
+ case dw_val_class_symview:
+ /* FIXME: Use uleb128 rather than data2. */
+ size += 2;
+ break;
case dw_val_class_const_implicit:
case dw_val_class_unsigned_const_implicit:
case dw_val_class_file_implicit:
@@ -9732,6 +9762,9 @@ value_format (dw_attr_node *a)
default:
return DW_FORM_block1;
}
+ case dw_val_class_symview:
+ /* FIXME: Use uleb128 rather than data2. */
+ return DW_FORM_data2;
case dw_val_class_vec:
switch (constant_size (a->dw_attr_val.v.val_vec.length
* a->dw_attr_val.v.val_vec.elt_size))
@@ -10497,6 +10530,11 @@ output_die (dw_die_ref die)
}
break;
+ case dw_val_class_symview:
+ /* FIXME: Use uleb128 rather than data2. */
+ dw2_asm_output_addr (2, a->dw_attr_val.v.val_symbolic_view, "%s",
name);
+ break;
+
case dw_val_class_const_implicit:
if (flag_debug_asm)
fprintf (asm_out_file, "\t\t\t%s %s ("
@@ -23809,7 +23847,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
indirecting them through a table might make things
easier, but even that would be more wasteful,
space-wise, than what we have now. */
- add_AT_lbl_id (die, DW_AT_GNU_entry_view, label);
+ add_AT_symview (die, DW_AT_GNU_entry_view, label);
}
}
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index a1856a5185e2..a0ba414014d4 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -161,7 +161,8 @@ enum dw_val_class
dw_val_class_const_implicit,
dw_val_class_unsigned_const_implicit,
dw_val_class_file_implicit,
- dw_val_class_view_list
+ dw_val_class_view_list,
+ dw_val_class_symview
};
/* Describe a floating point constant value, or a vector constant value. */
@@ -233,6 +234,7 @@ struct GTY(()) dw_val_node {
} GTY ((tag ("dw_val_class_vms_delta"))) val_vms_delta;
dw_discr_value GTY ((tag ("dw_val_class_discr_value"))) val_discr_value;
dw_discr_list_ref GTY ((tag ("dw_val_class_discr_list"))) val_discr_list;
+ char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view;
}
GTY ((desc ("%1.val_class"))) v;
};
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
