On 4/29/21 11:17 PM, Richard Biener wrote:
On Thu, 29 Apr 2021, Indu Bhagat wrote:
Hello,
On 4/29/21 5:10 AM, Richard Biener wrote:
On Thu, 29 Apr 2021, Jose E. Marchesi wrote:
This commit introduces support for generating CTF debugging
information and BTF debugging information from GCC.
Comments inline.
Thanks for your reviews.
My responses and questions inline at respective points.
Indu
>>>> +#ifndef GCC_CTFC_H
+#define GCC_CTFC_H 1
+
+#include "config.h"
+#include "system.h"
+#include "tree.h"
+#include "fold-const.h"
+#include "dwarf2ctf.h"
+#include "ctf.h"
+#include "btf.h"
+
+/* Invalid CTF type ID definition. */
+
+#define CTF_NULL_TYPEID 0
+
+/* Value to start generating the CTF type ID from. */
+
+#define CTF_INIT_TYPEID 1
+
+/* CTF type ID. */
+
+typedef unsigned long ctf_id_t;
+
+/* CTF string table element (list node). */
+
+typedef struct GTY ((chain_next ("%h.cts_next"))) ctf_string
I know that DWARF takes the lead here but do all these have to
live in GC memory? The reason the DWARF bits do is that they
point to 'tree' and that trees point to DIEs.
Not entirely sure what you mean here ? Do you mean to not tag it as GC root
and avoid traversal for GC marking the individual strings ?
Basically think of what part of the CTF data structures can live on the
heap (since you should know lifetime pretty well).
OK. I have added code to release the memory for CTF strings for now when
the CTF container is finalized. This change will be included in the next
patch series.
+{
+ dw_die_ref type_die = get_AT_ref (die, DW_AT_type);
+ return (type_die ? type_die : ctf_void_die);
+}
+
+/* Some data member DIEs have location specified as a DWARF expression
+ (specifically in DWARF2). Luckily, the expression is a simple
+ DW_OP_plus_uconst with one operand set to zero.
+
+ Sometimes the data member location may also be negative. In yet some
other
+ cases (specifically union data members), the data member location is
+ non-existent. Handle all these scenarios here to abstract this. */
+
+static HOST_WIDE_INT ctf_get_AT_data_member_location (dw_die_ref die)
likewise.
+{
+ HOST_WIDE_INT field_location = 0;
+ dw_attr_node * attr;
+
+ /* The field location (in bits) can be determined from
+ either a DW_AT_data_member_location attribute or a
+ DW_AT_data_bit_offset attribute. */
+ if (get_AT (die, DW_AT_data_bit_offset))
+ field_location = get_AT_unsigned (die, DW_AT_data_bit_offset);
+ else
+ {
+ attr = get_AT (die, DW_AT_data_member_location);
+ if (attr && AT_class (attr) == dw_val_class_loc)
+ {
+ dw_loc_descr_ref descr = AT_loc (attr);
+ /* Operand 2 must be zero; the structure is assumed to be on the
+ stack in DWARF2. */
+ gcc_assert (!descr->dw_loc_oprnd2.v.val_unsigned);
+ gcc_assert (descr->dw_loc_oprnd2.val_class
+ == dw_val_class_unsigned_const);
+ field_location = descr->dw_loc_oprnd1.v.val_unsigned;
+ }
+ else
+ {
+ attr = get_AT (die, DW_AT_data_member_location);
+ if (attr && AT_class (attr) == dw_val_class_const)
+ field_location = AT_int (attr);
+ else
+ field_location = (get_AT_unsigned (die,
+ DW_AT_data_member_location)
+ * 8);
+ }
+ }
so when neither of the above we return 0? Maybe we should ICE here
instead. Ada for example has variable location fields.
Yes, adding gcc_unreachable is sensible. Will do.
Hmm... I have to correct myself and say that we should not ICE here when
neither DW_AT_data_bit_offset nor DW_AT_data_member_location attributes
are available. There are valid C constructs when we will hit that case.
For these cases, we piggyback on the get_AT_unsigned () API as it
returns 0 when the requested attribute is NULL.
union c
{
int c1;
int c2;
} my_u_c;
DIE 0: DW_TAG_union_type (0x7ffff0f49190)
abbrev id: 0 offset: 0 mark: 0
DW_AT_name: "c"
DW_AT_byte_size: 4
DW_AT_decl_file: "test-union-1.c" (0)
DW_AT_decl_line: 8
DW_AT_decl_column: 7
DIE 0: DW_TAG_member (0x7ffff0f491e0)
abbrev id: 0 offset: 0 mark: 0
DW_AT_name: "c1"
DW_AT_decl_file: "test-union-1.c" (0)
DW_AT_decl_line: 10
DW_AT_decl_column: 7
DW_AT_type: die -> 0 (0x7ffff0f49230)
DIE 0: DW_TAG_member (0x7ffff0f49280)
abbrev id: 0 offset: 0 mark: 0
DW_AT_name: "c2"
DW_AT_decl_file: "test-union-1.c" (0)
DW_AT_decl_line: 11
DW_AT_decl_column: 7
DW_AT_type: die -> 0 (0x7ffff0f49230)
As for Ada, CTF is not supported.
So I think we are OK here.
Overall I think this is fine with the suggested changes. You may want
to refactor the debug info kind into a flag based one (I've seen you
suggested that on IRC).
Richard.
Thanks again for reviewing. Yes, I have started tinkering around to make the
write_symbols into a bitmap.
Indu
I posted a patch for write_symbols to use bitmask earlier this week.
Thanks
Indu
