public inbox for dwz@sourceware.org
 help / color / mirror / Atom feed
From: "Achra, Nitika" <Nitika.Achra@amd.com>
To: "dwz@sourceware.org" <dwz@sourceware.org>
Cc: "George, Jini Susan" <JiniSusan.George@amd.com>
Subject: RE: [PING][PATCH] DWARFv5: Support for unit type DW_UT_type and DW_TAG_type_unit.
Date: Fri, 19 Nov 2021 05:28:24 +0000	[thread overview]
Message-ID: <BN9PR12MB5243418C91081E90B097E8AC9A9C9@BN9PR12MB5243.namprd12.prod.outlook.com> (raw)
In-Reply-To: <BN9PR12MB5243983E71725D52FFCBA53F9AAD9@BN9PR12MB5243.namprd12.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 15141 bytes --]

[AMD Official Use Only]

A gentle reminder to please review this patch.

Regards,
Nitika

Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for Windows

From: Achra, Nitika<mailto:Nitika.Achra@amd.com>
Sent: Monday, October 4, 2021 4:20 AM
To: Jakub Jelinek<mailto:jakub@redhat.com>; dwz@sourceware.org<mailto:dwz@sourceware.org>
Cc: George, Jini Susan<mailto:JiniSusan.George@amd.com>
Subject: Re: [PING][PATCH] DWARFv5: Support for unit type DW_UT_type and DW_TAG_type_unit.

Requesting to please review this.

Regards,
Nitika

Get Outlook for Android<https://aka.ms/AAb9ysg>

From: Achra, Nitika
Sent: Thursday, August 26, 2021 9:47:14 AM
To: Jakub Jelinek <jakub@redhat.com>; dwz@sourceware.org <dwz@sourceware.org>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PING][PATCH] DWARFv5: Support for unit type DW_UT_type and DW_TAG_type_unit.

[AMD Official Use Only]

A gentle reminder.

Regards
Nitika

-----Original Message-----
From: Achra, Nitika
Sent: Thursday, August 12, 2021 9:50 AM
To: 'Jakub Jelinek' <jakub@redhat.com>; dwz@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PING][PATCH] DWARFv5: Support for unit type DW_UT_type and DW_TAG_type_unit.

[AMD Official Use Only]

Just a gentle reminder for review.

Regards,
Nitika

-----Original Message-----
From: Achra, Nitika
Sent: Wednesday, July 28, 2021 8:02 PM
To: Jakub Jelinek <jakub@redhat.com>
Cc: dwz@sourceware.org; George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] DWARFv5: Support for unit type DW_UT_type and DW_TAG_type_unit.

[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 <jakub@redhat.com>
Sent: Friday, June 11, 2021 4:25 PM
To: Achra, Nitika <Nitika.Achra@amd.com>
Cc: dwz@sourceware.org; George, Jini Susan <JiniSusan.George@amd.com>
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


[-- Attachment #2: 9621A344DF2947759E2F261BB231CE8A.png --]
[-- Type: image/png, Size: 159 bytes --]

      reply	other threads:[~2021-11-19  5:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12  4:20 Achra, Nitika
2021-08-26  4:17 ` Achra, Nitika
2021-10-03 22:50   ` Achra, Nitika
2021-11-19  5:28     ` Achra, Nitika [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN9PR12MB5243418C91081E90B097E8AC9A9C9@BN9PR12MB5243.namprd12.prod.outlook.com \
    --to=nitika.achra@amd.com \
    --cc=JiniSusan.George@amd.com \
    --cc=dwz@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).