public inbox for dwz@sourceware.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: "Achra, Nitika" <Nitika.Achra@amd.com>
Cc: "dwz@sourceware.org" <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.
Date: Fri, 11 Jun 2021 12:54:57 +0200	[thread overview]
Message-ID: <20210611105457.GQ7746@tucnak> (raw)
In-Reply-To: <BN9PR12MB5243F2710EDBF463A14413BC9A349@BN9PR12MB5243.namprd12.prod.outlook.com>

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


  reply	other threads:[~2021-06-11 10:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11  9:37 Achra, Nitika
2021-06-11 10:54 ` Jakub Jelinek [this message]
2021-07-28 14:32   ` Achra, Nitika

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=20210611105457.GQ7746@tucnak \
    --to=jakub@redhat.com \
    --cc=JiniSusan.George@amd.com \
    --cc=Nitika.Achra@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).