libctf: eliminate dtd_u, part 1: int/float/slice

This series eliminates a lot of special-case code to handle dynamic
types (types added to writable dicts and not yet serialized).

Historically, when such types have variable-length data in their final
CTF representations, libctf has always worked by adding such types to a
special union (ctf_dtdef_t.dtd_u) in the dynamic type definition
structure, then picking the members out of this structure at
serialization time and packing them into their final form.

This has the advantage that the ctf_add_* code doesn't need to know
anything about the final CTF representation, but the significant
disadvantage that all code that looks up types in any way needs two code
paths, one for dynamic types, one for all others.  Historically libctf
"handled" this by not supporting most type lookups on dynamic types at
all until ctf_update was called to do a complete reserialization of the
entire dict (it didn't emit an error, it just emitted wrong results).
Since commit 676c3ecbad6e9c4, which eliminated ctf_update in favour of
the internal-only ctf_serialize function, all the type-lookup paths
grew an extra branch to handle dynamic types.

We can eliminate this branch again by dropping the dtd_u stuff and
simply writing out the vlen in (close to) its final form at ctf_add_*
time: type lookup for types using this approach is then identical for
types in writable dicts and types that are in read-only ones, and
serialization is also simplified (we just need to write out the vlen
we already created).

The only complexity lies in type kinds for which multiple
vlen representations are valid depending on properties of the type,
e.g. structures.  But we can start simple, adjusting ints, floats,
and slices to work this way, and leaving everything else as is.

libctf/ChangeLog
2021-03-18  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-impl.h (ctf_dtdef_t) <dtd_u.dtu_enc>: Remove.
	<dtd_u.dtu_slice>: Likewise.
	<dtd_vlen>: New.
	* ctf-create.c (ctf_add_generic): Perhaps allocate it.  All
	callers adjusted.
	(ctf_dtd_delete): Free it.
	(ctf_add_slice): Use the dtd_vlen, not dtu_enc.
	(ctf_add_encoded): Likewise.  Assert that this must be an int or
	float.
	* ctf-serialize.c (ctf_emit_type_sect): Just copy the dtd_vlen.
	* ctf-dedup.c (ctf_dedup_rhash_type): Use the dtd_vlen, not
	dtu_slice.
	* ctf-types.c (ctf_type_reference): Likewise.
	(ctf_type_encoding): Remove most dynamic-type-specific code: just
	get the vlen from the right place.  Report failure to look up the
	underlying type's encoding.

diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index f64493f5012fba39db7929be8cc563f5a4aa8a5d..bf138399a393d6cc118fde3d424b95cc20b2a8e7 100644 (file)
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,22 @@
+2021-03-18  Nick Alcock  <nick.alcock@oracle.com>
+
+       * ctf-impl.h (ctf_dtdef_t) <dtd_u.dtu_enc>: Remove.
+       <dtd_u.dtu_slice>: Likewise.
+       <dtd_vlen>: New.
+       * ctf-create.c (ctf_add_generic): Perhaps allocate it.  All
+       callers adjusted.
+       (ctf_dtd_delete): Free it.
+       (ctf_add_slice): Use the dtd_vlen, not dtu_enc.
+       (ctf_add_encoded): Likewise.  Assert that this must be an int or
+       float.
+       * ctf-serialize.c (ctf_emit_type_sect): Just copy the dtd_vlen.
+       * ctf-dedup.c (ctf_dedup_rhash_type): Use the dtd_vlen, not
+       dtu_slice.
+       * ctf-types.c (ctf_type_reference): Likewise.
+       (ctf_type_encoding): Remove most dynamic-type-specific code: just
+       get the vlen from the right place.  Report failure to look up the
+       underlying type's encoding.
+
 2021-03-18  Nick Alcock  <nick.alcock@oracle.com>
 
        * ctf-archive.c (ctf_archive_next): GNU style fix for do {} while.

diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index b2e08623f2236309331b526761b4d7020ec02c2b..90db7121af6bfd4545a7bce9fc656f7144fa1d6a 100644 (file)
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -226,6 +226,7 @@ ctf_dtd_delete (ctf_dict_t *fp, ctf_dtdef_t *dtd)
   const char *name;
 
   ctf_dynhash_remove (fp->ctf_dthash, (void *) (uintptr_t) dtd->dtd_type);
+  free (dtd->dtd_vlen);
 
   switch (kind)
     {
@@ -406,7 +407,7 @@ ctf_rollback (ctf_dict_t *fp, ctf_snapshot_id_t id)
 
 static ctf_id_t
 ctf_add_generic (ctf_dict_t *fp, uint32_t flag, const char *name, int kind,
-                ctf_dtdef_t **rp)
+                size_t vlen, ctf_dtdef_t **rp)
 {
   ctf_dtdef_t *dtd;
   ctf_id_t type;
@@ -425,33 +426,42 @@ ctf_add_generic (ctf_dict_t *fp, uint32_t flag, const char *name, int kind,
 
   /* Make sure ptrtab always grows to be big enough for all types.  */
   if (ctf_grow_ptrtab (fp) < 0)
-      return CTF_ERR;          /* errno is set for us. */
+      return CTF_ERR;                          /* errno is set for us. */
 
-  if ((dtd = malloc (sizeof (ctf_dtdef_t))) == NULL)
+  if ((dtd = calloc (1, sizeof (ctf_dtdef_t))) == NULL)
     return (ctf_set_errno (fp, EAGAIN));
 
+  if (vlen > 0)
+    {
+      if ((dtd->dtd_vlen = calloc (1, vlen)) == NULL)
+       goto oom;
+    }
+  else
+    dtd->dtd_vlen = NULL;
+
   type = ++fp->ctf_typemax;
   type = LCTF_INDEX_TO_TYPE (fp, type, (fp->ctf_flags & LCTF_CHILD));
 
-  memset (dtd, 0, sizeof (ctf_dtdef_t));
   dtd->dtd_data.ctt_name = ctf_str_add_ref (fp, name, &dtd->dtd_data.ctt_name);
   dtd->dtd_type = type;
 
   if (dtd->dtd_data.ctt_name == 0 && name != NULL && name[0] != '\0')
-    {
-      free (dtd);
-      return (ctf_set_errno (fp, EAGAIN));
-    }
+    goto oom;
 
   if (ctf_dtd_insert (fp, dtd, flag, kind) < 0)
-    {
-      free (dtd);
-      return CTF_ERR;                  /* errno is set for us.  */
-    }
+    goto err;                                  /* errno is set for us.  */
+
   fp->ctf_flags |= LCTF_DIRTY;
 
   *rp = dtd;
   return type;
+
+ oom:
+  ctf_set_errno (fp, EAGAIN);
+ err:
+  free (dtd->dtd_vlen);
+  free (dtd);
+  return CTF_ERR;
 }
 
 /* When encoding integer sizes, we want to convert a byte count in the range
@@ -477,6 +487,7 @@ ctf_add_encoded (ctf_dict_t *fp, uint32_t flag,
 {
   ctf_dtdef_t *dtd;
   ctf_id_t type;
+  uint32_t encoding;
 
   if (ep == NULL)
     return (ctf_set_errno (fp, EINVAL));
@@ -484,13 +495,26 @@ ctf_add_encoded (ctf_dict_t *fp, uint32_t flag,
   if (name == NULL || name[0] == '\0')
     return (ctf_set_errno (fp, ECTF_NONAME));
 
-  if ((type = ctf_add_generic (fp, flag, name, kind, &dtd)) == CTF_ERR)
+  if (!ctf_assert (fp, kind == CTF_K_INTEGER || kind == CTF_K_FLOAT))
+    return -1;                                 /* errno is set for us.  */
+
+  if ((type = ctf_add_generic (fp, flag, name, kind, sizeof (uint32_t),
+                              &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (kind, flag, 0);
   dtd->dtd_data.ctt_size = clp2 (P2ROUNDUP (ep->cte_bits, CHAR_BIT)
                                 / CHAR_BIT);
-  dtd->dtd_u.dtu_enc = *ep;
+  switch (kind)
+    {
+    case CTF_K_INTEGER:
+      encoding = CTF_INT_DATA (ep->cte_format, ep->cte_offset, ep->cte_bits);
+      break;
+    case CTF_K_FLOAT:
+      encoding = CTF_FP_DATA (ep->cte_format, ep->cte_offset, ep->cte_bits);
+      break;
+    }
+  memcpy (dtd->dtd_vlen, &encoding, sizeof (encoding));
 
   return type;
 }
@@ -509,7 +533,7 @@ ctf_add_reftype (ctf_dict_t *fp, uint32_t flag, ctf_id_t ref, uint32_t kind)
   if (ref != 0 && ctf_lookup_by_id (&tmp, ref) == NULL)
     return CTF_ERR;            /* errno is set for us.  */
 
-  if ((type = ctf_add_generic (fp, flag, NULL, kind, &dtd)) == CTF_ERR)
+  if ((type = ctf_add_generic (fp, flag, NULL, kind, 0, &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (kind, flag, 0);
@@ -539,6 +563,7 @@ ctf_add_slice (ctf_dict_t *fp, uint32_t flag, ctf_id_t ref,
               const ctf_encoding_t *ep)
 {
   ctf_dtdef_t *dtd;
+  ctf_slice_t slice;
   ctf_id_t resolved_ref = ref;
   ctf_id_t type;
   int kind;
@@ -569,15 +594,19 @@ ctf_add_slice (ctf_dict_t *fp, uint32_t flag, ctf_id_t ref,
       && (ref != 0))
     return (ctf_set_errno (fp, ECTF_NOTINTFP));
 
-  if ((type = ctf_add_generic (fp, flag, NULL, CTF_K_SLICE, &dtd)) == CTF_ERR)
+  if ((type = ctf_add_generic (fp, flag, NULL, CTF_K_SLICE,
+                              sizeof (ctf_slice_t), &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
+  memset (&slice, 0, sizeof (ctf_slice_t));
+
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (CTF_K_SLICE, flag, 0);
   dtd->dtd_data.ctt_size = clp2 (P2ROUNDUP (ep->cte_bits, CHAR_BIT)
                                 / CHAR_BIT);
-  dtd->dtd_u.dtu_slice.cts_type = (uint32_t) ref;
-  dtd->dtd_u.dtu_slice.cts_bits = ep->cte_bits;
-  dtd->dtd_u.dtu_slice.cts_offset = ep->cte_offset;
+  slice.cts_type = (uint32_t) ref;
+  slice.cts_bits = ep->cte_bits;
+  slice.cts_offset = ep->cte_offset;
+  memcpy (dtd->dtd_vlen, &slice, sizeof (ctf_slice_t));
 
   return type;
 }
@@ -628,7 +657,8 @@ ctf_add_array (ctf_dict_t *fp, uint32_t flag, const ctf_arinfo_t *arp)
       return (ctf_set_errno (fp, ECTF_INCOMPLETE));
     }
 
-  if ((type = ctf_add_generic (fp, flag, NULL, CTF_K_ARRAY, &dtd)) == CTF_ERR)
+  if ((type = ctf_add_generic (fp, flag, NULL, CTF_K_ARRAY,
+                              0, &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (CTF_K_ARRAY, flag, 0);
@@ -700,7 +730,7 @@ ctf_add_function (ctf_dict_t *fp, uint32_t flag,
     }
 
   if ((type = ctf_add_generic (fp, flag, NULL, CTF_K_FUNCTION,
-                              &dtd)) == CTF_ERR)
+                              0, &dtd)) == CTF_ERR)
     {
       free (vdat);
       return CTF_ERR;             /* errno is set for us.  */
@@ -730,7 +760,7 @@ ctf_add_struct_sized (ctf_dict_t *fp, uint32_t flag, const char *name,
   if (type != 0 && ctf_type_kind (fp, type) == CTF_K_FORWARD)
     dtd = ctf_dtd_lookup (fp, type);
   else if ((type = ctf_add_generic (fp, flag, name, CTF_K_STRUCT,
-                                   &dtd)) == CTF_ERR)
+                                   0, &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (CTF_K_STRUCT, flag, 0);
@@ -767,7 +797,7 @@ ctf_add_union_sized (ctf_dict_t *fp, uint32_t flag, const char *name,
   if (type != 0 && ctf_type_kind (fp, type) == CTF_K_FORWARD)
     dtd = ctf_dtd_lookup (fp, type);
   else if ((type = ctf_add_generic (fp, flag, name, CTF_K_UNION,
-                                   &dtd)) == CTF_ERR)
+                                   0, &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us */
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (CTF_K_UNION, flag, 0);
@@ -803,7 +833,7 @@ ctf_add_enum (ctf_dict_t *fp, uint32_t flag, const char *name)
   if (type != 0 && ctf_type_kind (fp, type) == CTF_K_FORWARD)
     dtd = ctf_dtd_lookup (fp, type);
   else if ((type = ctf_add_generic (fp, flag, name, CTF_K_ENUM,
-                                   &dtd)) == CTF_ERR)
+                                   0, &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (CTF_K_ENUM, flag, 0);
@@ -861,7 +891,7 @@ ctf_add_forward (ctf_dict_t *fp, uint32_t flag, const char *name,
   if (type)
     return type;
 
-  if ((type = ctf_add_generic (fp, flag, name, kind, &dtd)) == CTF_ERR)
+  if ((type = ctf_add_generic (fp, flag, name, kind, 0, &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (CTF_K_FORWARD, flag, 0);
@@ -887,7 +917,7 @@ ctf_add_typedef (ctf_dict_t *fp, uint32_t flag, const char *name,
   if (ref != 0 && ctf_lookup_by_id (&tmp, ref) == NULL)
     return CTF_ERR;            /* errno is set for us.  */
 
-  if ((type = ctf_add_generic (fp, flag, name, CTF_K_TYPEDEF,
+  if ((type = ctf_add_generic (fp, flag, name, CTF_K_TYPEDEF, 0,
                               &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
@@ -1783,7 +1813,7 @@ ctf_add_type_internal (ctf_dict_t *dst_fp, ctf_dict_t *src_fp, ctf_id_t src_type
           manually so as to avoid repeated lookups in ctf_add_member
           and to ensure the exact same member offsets as in src_type.  */
 
-       dst_type = ctf_add_generic (dst_fp, flag, name, kind, &dtd);
+       dst_type = ctf_add_generic (dst_fp, flag, name, kind, 0, &dtd);
        if (dst_type == CTF_ERR)
          return CTF_ERR;                       /* errno is set for us.  */
 

diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index 9f5ba903ec486d3cf47fa57c192ef252362b263d..b8a7d49b9dbca9e93c529f9b72a0dd281d4ba7d8 100644 (file)
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -752,7 +752,7 @@ ctf_dedup_rhash_type (ctf_dict_t *fp, ctf_dict_t *input, ctf_dict_t **inputs,
        citer = hval;
 
        if ((dtd = ctf_dynamic_type (input, type)) != NULL)
-         slice = &dtd->dtd_u.dtu_slice;
+         slice = (ctf_slice_t *) dtd->dtd_vlen;
        else
          slice = (ctf_slice_t *) ((uintptr_t) tp + increment);
 

diff --git a/libctf/ctf-impl.h b/libctf/ctf-impl.h
index 5567b4c61f5106e11f0d17eb04863212e7abdb07..742b4b37affd0eea4db52ba131c709a738adf318 100644 (file)
--- a/libctf/ctf-impl.h
+++ b/libctf/ctf-impl.h
@@ -192,13 +192,12 @@ typedef struct ctf_dtdef
   ctf_list_t dtd_list;         /* List forward/back pointers.  */
   ctf_id_t dtd_type;           /* Type identifier for this definition.  */
   ctf_type_t dtd_data;         /* Type node, including name.  */
+  unsigned char *dtd_vlen;     /* Variable-length data for this type.  */
   union
   {
     ctf_list_t dtu_members;    /* struct, union, or enum */
     ctf_arinfo_t dtu_arr;      /* array */
-    ctf_encoding_t dtu_enc;    /* integer or float */
     uint32_t *dtu_argv;                /* function */
-    ctf_slice_t dtu_slice;     /* slice */
   } dtd_u;
 } ctf_dtdef_t;
 

diff --git a/libctf/ctf-serialize.c b/libctf/ctf-serialize.c
index 1e2c98b473a00a98ab9dd16ac2ac65cd58d7c585..f07cb61c42aa2bc7a7164eebfe0d8437ad69cd23 100644 (file)
--- a/libctf/ctf-serialize.c
+++ b/libctf/ctf-serialize.c
@@ -858,7 +858,6 @@ ctf_emit_type_sect (ctf_dict_t *fp, unsigned char **tptr)
       uint32_t vlen = LCTF_INFO_VLEN (fp, dtd->dtd_data.ctt_info);
 
       ctf_array_t cta;
-      uint32_t encoding;
       size_t len;
       ctf_stype_t *copied;
       const char *name;
@@ -879,24 +878,12 @@ ctf_emit_type_sect (ctf_dict_t *fp, unsigned char **tptr)
        {
        case CTF_K_INTEGER:
        case CTF_K_FLOAT:
-         if (kind == CTF_K_INTEGER)
-           {
-             encoding = CTF_INT_DATA (dtd->dtd_u.dtu_enc.cte_format,
-                                      dtd->dtd_u.dtu_enc.cte_offset,
-                                      dtd->dtd_u.dtu_enc.cte_bits);
-           }
-         else
-           {
-             encoding = CTF_FP_DATA (dtd->dtd_u.dtu_enc.cte_format,
-                                     dtd->dtd_u.dtu_enc.cte_offset,
-                                     dtd->dtd_u.dtu_enc.cte_bits);
-           }
-         memcpy (t, &encoding, sizeof (encoding));
-         t += sizeof (encoding);
+         memcpy (t, dtd->dtd_vlen, sizeof (uint32_t));
+         t += sizeof (uint32_t);
          break;
 
        case CTF_K_SLICE:
-         memcpy (t, &dtd->dtd_u.dtu_slice, sizeof (struct ctf_slice));
+         memcpy (t, dtd->dtd_vlen, sizeof (struct ctf_slice));
          t += sizeof (struct ctf_slice);
          break;
 

diff --git a/libctf/ctf-types.c b/libctf/ctf-types.c
index 28c5c7aa1e144d3791c47772b2cf67da7fd20f85..ae2438177131aa6787cf7e355c2f3c63bb854377 100644 (file)
--- a/libctf/ctf-types.c
+++ b/libctf/ctf-types.c
@@ -1168,7 +1168,7 @@ ctf_type_reference (ctf_dict_t *fp, ctf_id_t type)
            sp = (const ctf_slice_t *) ((uintptr_t) tp + increment);
          }
        else
-         sp = &dtd->dtd_u.dtu_slice;
+         sp = (const ctf_slice_t *) dtd->dtd_vlen;
 
        return sp->cts_type;
       }
@@ -1218,52 +1218,30 @@ ctf_type_encoding (ctf_dict_t *fp, ctf_id_t type, ctf_encoding_t *ep)
   ctf_dtdef_t *dtd;
   const ctf_type_t *tp;
   ssize_t increment;
+  const unsigned char *vlen;
   uint32_t data;
 
   if ((tp = ctf_lookup_by_id (&fp, type)) == NULL)
     return -1;                 /* errno is set for us.  */
 
   if ((dtd = ctf_dynamic_type (ofp, type)) != NULL)
+    vlen = dtd->dtd_vlen;
+  else
     {
-      switch (LCTF_INFO_KIND (fp, tp->ctt_info))
-       {
-       case CTF_K_INTEGER:
-       case CTF_K_FLOAT:
-         *ep = dtd->dtd_u.dtu_enc;
-         break;
-       case CTF_K_SLICE:
-         {
-           const ctf_slice_t *slice;
-           ctf_encoding_t underlying_en;
-           ctf_id_t underlying;
-
-           slice = &dtd->dtd_u.dtu_slice;
-           underlying = ctf_type_resolve (fp, slice->cts_type);
-           data = ctf_type_encoding (fp, underlying, &underlying_en);
-
-           ep->cte_format = underlying_en.cte_format;
-           ep->cte_offset = slice->cts_offset;
-           ep->cte_bits = slice->cts_bits;
-           break;
-         }
-       default:
-         return (ctf_set_errno (ofp, ECTF_NOTINTFP));
-       }
-      return 0;
+      ctf_get_ctt_size (fp, tp, NULL, &increment);
+      vlen = (const unsigned char *) ((uintptr_t) tp + increment);
     }
 
-  (void) ctf_get_ctt_size (fp, tp, NULL, &increment);
-
   switch (LCTF_INFO_KIND (fp, tp->ctt_info))
     {
     case CTF_K_INTEGER:
-      data = *(const uint32_t *) ((uintptr_t) tp + increment);
+      data = *(const uint32_t *) vlen;
       ep->cte_format = CTF_INT_ENCODING (data);
       ep->cte_offset = CTF_INT_OFFSET (data);
       ep->cte_bits = CTF_INT_BITS (data);
       break;
     case CTF_K_FLOAT:
-      data = *(const uint32_t *) ((uintptr_t) tp + increment);
+      data = *(const uint32_t *) vlen;
       ep->cte_format = CTF_FP_ENCODING (data);
       ep->cte_offset = CTF_FP_OFFSET (data);
       ep->cte_bits = CTF_FP_BITS (data);
@@ -1274,9 +1252,10 @@ ctf_type_encoding (ctf_dict_t *fp, ctf_id_t type, ctf_encoding_t *ep)
        ctf_encoding_t underlying_en;
        ctf_id_t underlying;
 
-       slice = (ctf_slice_t *) ((uintptr_t) tp + increment);
+       slice = (ctf_slice_t *) vlen;
        underlying = ctf_type_resolve (fp, slice->cts_type);
-       data = ctf_type_encoding (fp, underlying, &underlying_en);
+       if (ctf_type_encoding (fp, underlying, &underlying_en) < 0)
+         return -1;                            /* errno is set for us.  */
 
        ep->cte_format = underlying_en.cte_format;
        ep->cte_offset = slice->cts_offset;