[AMD Official Use Only] Hi Jakub, Thanks for the review. Please find the attached patch with some of the changes you suggested. Please take a look at it. Regards, Nitika -----Original Message----- From: Jakub Jelinek Sent: Friday, June 11, 2021 4:25 PM To: Achra, Nitika Cc: dwz@sourceware.org; George, Jini Susan Subject: Re: [PATCH] DWARFv5: Support for unit type DW_UT_type and DW_TAG_type_unit. [CAUTION: External Email] On Fri, Jun 11, 2021 at 09:37:18AM +0000, Achra, Nitika via Dwz wrote: > The attached patch handles the DW_UT_type and DW_TAG_type_unit for DWARFv5. Requesting everyone to please review this. Ideally, dwz should rewrite all those DW_TAG_type_units into DW_TAG_partial_unit, after a library or binary is linked, nothing from outside can refer to its type units. The DW_FORM_ref_sig8 references are too large, DW_FORM_ref_addr is (for 32-bit DWARF) half the size and cheaper for the consumer which doesn't have to look up the type id in some hash table. Furthermore one can save some bytes from the unit header too. --- a/dwz.c +++ b/dwz.c @@ -903,6 +903,13 @@ struct dw_cu unsigned int cu_offset; /* DWARF version of the CU. */ unsigned int cu_version; + /* True if the CU unit_type is DW_UT_type inside the debug_info. */ + bool isUTType; Why can't this be in cu_kind instead, just another kind? And, dwz doesn't use this kind of variable/field names. + /* A unique 8-byte signature of the type described in this type unit. + */ uint64_t cu_type_signature; + /* A 4-byte unsigned offset relative to the beginning of the type unit + header. */ + unsigned int cu_type_offset; This will create unnecessary padding in the struct on 64-bit arches. They should be moved somewhere where that doesn't happen. /* Cached DW_AT_comp_dir value from DW_TAG_*_unit cu_die, or NULL if that attribute is not present. */ char *cu_comp_dir; @@ -3245,6 +3252,7 @@ checksum_die (DSO *dso, dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die) die->u.p1.die_hash = 0; if (die->die_tag == DW_TAG_compile_unit || die->die_tag == DW_TAG_partial_unit + || die->die_tag == DW_TAG_type_unit || die->die_tag == DW_TAG_namespace || die->die_tag == DW_TAG_module || die->die_tag == DW_TAG_imported_unit) @@ -4065,6 +4073,7 @@ checksum_ref_die (dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die, while (!reft->die_root && reft->die_parent->die_tag != DW_TAG_compile_unit && reft->die_parent->die_tag != DW_TAG_partial_unit + && reft->die_parent->die_tag != DW_TAG_type_unit && !reft->die_parent->die_named_namespace) reft = reft->die_parent; if (reft->die_ck_state != CK_KNOWN || reft->die_root) @@ -4580,9 +4589,11 @@ die_eq_1 (dw_cu_ref cu1, dw_cu_ref cu2, { const char *name1, *name2; if ((ref1->die_tag == DW_TAG_compile_unit - || ref1->die_tag == DW_TAG_partial_unit) + || ref1->die_tag == DW_TAG_partial_unit + || ref1->die_tag == DW_TAG_type_unit) && (ref2->die_tag == DW_TAG_compile_unit - || ref2->die_tag == DW_TAG_partial_unit)) + || ref2->die_tag == DW_TAG_partial_unit + || ref2->die_tag == DW_TAG_type_unit)) break; if (ref1->die_tag != ref2->die_tag) return 0; @@ -6023,6 +6034,7 @@ mark_refs (dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die, int mode) while (!reft->die_root && reft->die_parent->die_tag != DW_TAG_compile_unit && reft->die_parent->die_tag != DW_TAG_partial_unit + && reft->die_parent->die_tag != DW_TAG_type_unit && !reft->die_parent->die_named_namespace) reft = reft->die_parent; if ((mode & MARK_REFS_FOLLOW_DUPS) && reft->die_dup != NULL) @@ -6342,7 +6354,7 @@ try_debug_info (DSO *dso) if (cu_version == 5) { value = read_8 (ptr); - if (value != DW_UT_compile && value != DW_UT_partial) + if (value != DW_UT_compile && value != DW_UT_partial && value + != DW_UT_type) error (0, 0, "%s: DWARF CU type %s unhandled", dso->filename, get_DW_UT_str (value)); } @@ -6571,6 +6583,8 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) bool present; unsigned int debug_line_off; unsigned int type_offset = 0; + uint64_t type_signature = 0; + bool isUTType = false; /* Note header is one bigger with DWARF version 5. */ if (ptr + (kind == DEBUG_TYPES ? 23 : 11) > endsec) @@ -6613,12 +6627,13 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) if (cu_version == 5) { value = read_8 (ptr); - if (value != DW_UT_compile && value != DW_UT_partial) + if (value != DW_UT_compile && value != DW_UT_partial && value + != DW_UT_type) { error (0, 0, "%s: DWARF CU type %s unhandled", dso->filename, get_DW_UT_str (value)); goto fail; } + isUTType = (value == DW_UT_type); } else { @@ -6760,9 +6775,9 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) } last_abbrev_offset = value; - if (unlikely (kind == DEBUG_TYPES)) + if (unlikely (kind == DEBUG_TYPES) || isUTType) { - ptr += 8; + type_signature = read_64 (ptr); type_offset = read_32 (ptr); } @@ -6775,6 +6790,10 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) cu->cu_offset = cu_offset; cu->cu_version = cu_version; cu->cu_chunk = cu_chunk; + cu->cu_type_offset = type_offset; + cu->cu_type_signature = type_signature; + cu->isUTType = isUTType; + if (unlikely (op_multifile || low_mem)) cu->cu_abbrev = abbrev; diep = &cu->cu_die; @@ -6955,14 +6974,16 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) case DW_FORM_implicit_const: if (lang_p && (die->die_tag == DW_TAG_compile_unit - || die->die_tag == DW_TAG_partial_unit) + || die->die_tag == DW_TAG_partial_unit + || die->die_tag == DW_TAG_type_unit) && t->attr[i].attr == DW_AT_language) cu->lang = t->values[i]; break; case DW_FORM_data1: if (lang_p && (die->die_tag == DW_TAG_compile_unit - || die->die_tag == DW_TAG_partial_unit) + || die->die_tag == DW_TAG_partial_unit + || die->die_tag == DW_TAG_type_unit) && t->attr[i].attr == DW_AT_language) cu->lang = *ptr; /* FALLTHRU */ @@ -6973,7 +6994,8 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) case DW_FORM_data2: if (lang_p && (die->die_tag == DW_TAG_compile_unit - || die->die_tag == DW_TAG_partial_unit) + || die->die_tag == DW_TAG_partial_unit + || die->die_tag == DW_TAG_type_unit) && t->attr[i].attr == DW_AT_language) cu->lang = do_read_16 (ptr); /* FALLTHRU */ @@ -6983,7 +7005,8 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) case DW_FORM_data4: if (lang_p && (die->die_tag == DW_TAG_compile_unit - || die->die_tag == DW_TAG_partial_unit) + || die->die_tag == DW_TAG_partial_unit + || die->die_tag == DW_TAG_type_unit) && t->attr[i].attr == DW_AT_language) read_lang (ptr, form, &cu->lang); /* FALLTHRU */ @@ -6994,7 +7017,8 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) case DW_FORM_data8: if (lang_p && (die->die_tag == DW_TAG_compile_unit - || die->die_tag == DW_TAG_partial_unit) + || die->die_tag == DW_TAG_partial_unit + || die->die_tag == DW_TAG_type_unit) && t->attr[i].attr == DW_AT_language) read_lang (ptr, form, &cu->lang); /* FALLTHRU */ @@ -7009,7 +7033,8 @@ read_debug_info (DSO *dso, int kind, unsigned int *die_count) case DW_FORM_udata: if (lang_p && (die->die_tag == DW_TAG_compile_unit - || die->die_tag == DW_TAG_partial_unit) + || die->die_tag == DW_TAG_partial_unit + || die->die_tag == DW_TAG_type_unit) && t->attr[i].attr == DW_AT_language) { ptr = read_lang (ptr, form, &cu->lang); @@ -7559,6 +7584,7 @@ mark_singletons (dw_cu_ref cu, dw_die_ref top_die, dw_die_ref die, while (!reft->die_root && reft->die_parent->die_tag != DW_TAG_compile_unit && reft->die_parent->die_tag != DW_TAG_partial_unit + && reft->die_parent->die_tag != DW_TAG_type_unit && !reft->die_parent->die_named_namespace) reft = reft->die_parent; if (reft->die_dup != NULL || reft->die_nextdup != NULL) @@ -11001,6 +11027,7 @@ build_abbrevs_for_die (htab_t h, dw_cu_ref cu, dw_die_ref die, { case DW_TAG_partial_unit: case DW_TAG_compile_unit: + case DW_TAG_type_unit: t->nattr = 0; die->die_size = 0; if (origin == NULL) @@ -11014,7 +11041,7 @@ build_abbrevs_for_die (htab_t h, dw_cu_ref cu, dw_die_ref die, die->die_size += 4; t->nattr++; } - if (uni_lang_p || cu->cu_die->die_tag == DW_TAG_compile_unit) + if (uni_lang_p || cu->cu_die->die_tag == DW_TAG_compile_unit || + cu->cu_die->die_tag == DW_TAG_type_unit) { unsigned int lang_size = nr_bytes_for (cu->lang); die->die_size += lang_size; @@ -11421,7 +11448,7 @@ compute_abbrevs (DSO *dso) enum dwarf_form intracuform = DW_FORM_ref4; dw_die_ref child, *lastotr, child_next, *last; unsigned int headersz = (cu->cu_kind == CU_TYPES - ? 23 : (cu->cu_version >= 5 ? 12 : 11)); + ? 23 : (cu->cu_version >= 5 ? + (cu->isUTType ? 24 : 12) : 11)); if (unlikely (fi_multifile) && cu->cu_die->die_remove) continue; @@ -12639,6 +12666,7 @@ write_die (unsigned char *ptr, dw_cu_ref cu, dw_die_ref die, { case DW_TAG_partial_unit: case DW_TAG_compile_unit: + case DW_TAG_type_unit: ptr = write_unit_die (ptr, die, origin); break; case DW_TAG_namespace: @@ -12737,7 +12765,7 @@ static void recompute_abbrevs (dw_cu_ref cu, unsigned int cu_size) { unsigned int headersz = (cu->cu_kind == CU_TYPES - ? 23 : (cu->cu_version >= 5 ? 12 : 11)); + ? 23 : (cu->cu_version >= 5 ? (cu->isUTType ? + 24 : 12) : 11)); struct abbrev_tag *t; unsigned int ndies = 0, intracusize, off, i; dw_die_ref *intracuarr, *intracuvec; @@ -12846,13 +12874,23 @@ write_info (unsigned int *die_count) write_16 (ptr, cu->cu_version); if (cu->cu_version >= 5) { - *ptr++ = (cu->cu_die->die_tag == DW_TAG_compile_unit - ? DW_UT_compile : DW_UT_partial); + if (cu->cu_die->die_tag == DW_TAG_compile_unit) + *ptr++ = DW_UT_compile; + else if (cu->cu_die->die_tag == DW_TAG_type_unit) + *ptr++ = DW_UT_type; + else + *ptr++ = DW_UT_partial; write_8 (ptr, ptr_size); } write_32 (ptr, cu->u2.cu_new_abbrev_offset); if (cu->cu_version < 5) write_8 (ptr, ptr_size); + if (cu->cu_die->die_tag == DW_TAG_type_unit) + { + write_64 (ptr, cu->cu_type_signature); + write_32 (ptr, cu->cu_type_offset); + } + ptr = write_die (ptr, cu, cu->cu_die, NULL, NULL, die_count); assert (info + (next_off - (wr_multifile ? multi_info_off : 0)) == ptr); if (unlikely (low_mem) && cu->cu_kind != CU_PU) @@ -14470,6 +14508,7 @@ propagate_multifile_refs_backward (dw_cu_ref cu, dw_die_ref top_die, while (!reft->die_root && reft->die_parent->die_tag != DW_TAG_compile_unit && reft->die_parent->die_tag != DW_TAG_partial_unit + && reft->die_parent->die_tag != DW_TAG_type_unit && !reft->die_parent->die_named_namespace) reft = reft->die_parent; if (reft->die_root) -- 2.17.1 Jakub