libctf, ld: prohibit getting the size or alignment of forwards
authorNick Alcock <nick.alcock@oracle.com>
Tue, 5 Jan 2021 13:25:56 +0000 (13:25 +0000)
committerNick Alcock <nick.alcock@oracle.com>
Tue, 5 Jan 2021 14:53:39 +0000 (14:53 +0000)
C allows you to do only a very few things with entities of incomplete
type (as opposed to pointers to them): make pointers to them and give
them cv-quals, roughly. In particular you can't sizeof them and you
can't get their alignment.

We cannot impose all the requirements the standard imposes on CTF users,
because the deduplicator can transform any structure type into a forward
for the purposes of breaking cycles: so CTF type graphs can easily
contain things like arrays of forward type (if you want to figure out
their size or alignment, you need to chase down the types this forward
might be a forward to in child TU dicts: we will soon add API functions
to make doing this much easier).

Nonetheless, it is still meaningless to ask for the size or alignment of
forwards: but libctf didn't prohibit this and returned nonsense from
internal implementation details when you asked (it returned the kind of
the pointed-to type as both the size and alignment, because forwards
reuse ctt_type as a type kind, and ctt_type and ctt_size overlap).  So
introduce a new error, ECTF_INCOMPLETE, which is returned when you try
to get the size or alignment of forwards: we also return it when you try
to do things that require libctf itself to get the size or alignment of
a forward, notably using a forward as an array index type (which C
should never do in any case) or adding forwards to structures without
specifying their offset explicitly.

The dumper will not emit size or alignment info for forwards any more.

(This should not be an API break since ctf_type_size and ctf_type_align
could both return errors before now: any code that isn't expecting error
returns is already potentially broken.)

include/ChangeLog
2021-01-05  Nick Alcock  <nick.alcock@oracle.com>

* ctf-api.h (ECTF_INCOMPLETE): New.
(ECTF_NERR): Adjust.

ld/ChangeLog
2021-01-05  Nick Alcock  <nick.alcock@oracle.com>

* testsuite/ld-ctf/conflicting-cycle-1.parent.d: Adjust for dumper
changes.
* testsuite/ld-ctf/cross-tu-cyclic-conflicting.d: Likewise.
* testsuite/ld-ctf/forward.c: New test...
* testsuite/ld-ctf/forward.d: ... and results.

libctf/ChangeLog
2021-01-05  Nick Alcock  <nick.alcock@oracle.com>

* ctf-types.c (ctf_type_resolve): Improve comment.
(ctf_type_size): Yield ECTF_INCOMPLETE when applied to forwards.
Emit errors into the right dict.
(ctf_type_align): Likewise.
* ctf-create.c (ctf_add_member_offset): Yield ECTF_INCOMPLETE
when adding a member without explicit offset when this member, or
the previous member, is incomplete.
* ctf-dump.c (ctf_dump_format_type): Do not try to print the size of
forwards.
(ctf_dump_member): Do not try to print their alignment.

include/ChangeLog
include/ctf-api.h
ld/ChangeLog
ld/testsuite/ld-ctf/conflicting-cycle-1.parent.d
ld/testsuite/ld-ctf/cross-tu-cyclic-conflicting.d
ld/testsuite/ld-ctf/forward.c [new file with mode: 0644]
ld/testsuite/ld-ctf/forward.d [new file with mode: 0644]
libctf/ChangeLog
libctf/ctf-create.c
libctf/ctf-dump.c
libctf/ctf-types.c

index 048ba6113f95d63c315a3e2197fd1e3c5d275bce..1b56987ea9b57735aa40d01913c6266b48f66b0c 100644 (file)
@@ -1,3 +1,8 @@
+2021-01-05  Nick Alcock  <nick.alcock@oracle.com>
+
+       * ctf-api.h (ECTF_INCOMPLETE): New.
+       (ECTF_NERR): Adjust.
+
 2021-01-01  Nicolas Boulenguez  <nicolas@debian.org>
 
        * coff/internal.h: Correct comment spelling.
index f221d17f33e6b003a56abbba3a5efd5eed31b639..b3cfd391506badcfb5460eb0b58b6637cc6037db 100644 (file)
@@ -230,7 +230,8 @@ typedef struct ctf_snapshot_id
   _CTF_ITEM (ECTF_NEXT_WRONGFUN, "Wrong iteration function called.") \
   _CTF_ITEM (ECTF_NEXT_WRONGFP, "Iteration entity changed in mid-iterate.") \
   _CTF_ITEM (ECTF_FLAGS, "CTF header contains flags unknown to libctf.") \
-  _CTF_ITEM (ECTF_NEEDSBFD, "This feature needs a libctf with BFD support.")
+  _CTF_ITEM (ECTF_NEEDSBFD, "This feature needs a libctf with BFD support.") \
+  _CTF_ITEM (ECTF_INCOMPLETE, "Type is not a complete type.")
 
 #define        ECTF_BASE       1000    /* Base value for libctf errnos.  */
 
@@ -243,7 +244,7 @@ _CTF_ERRORS
 #undef _CTF_FIRST
   };
 
-#define ECTF_NERR (ECTF_NEEDSBFD - ECTF_BASE + 1) /* Count of CTF errors.  */
+#define ECTF_NERR (ECTF_INCOMPLETE - ECTF_BASE + 1) /* Count of CTF errors.  */
 
 /* The CTF data model is inferred to be the caller's data model or the data
    model of the given object, unless ctf_setmodel is explicitly called.  */
index a130c6c06d26cfc36159c06e79d19a982f3cb6ca..1497fc40e702a9acbfc93b19e90c148130a1d718 100644 (file)
@@ -1,3 +1,11 @@
+2021-01-05  Nick Alcock  <nick.alcock@oracle.com>
+
+       * testsuite/ld-ctf/conflicting-cycle-1.parent.d: Adjust for dumper
+       changes.
+       * testsuite/ld-ctf/cross-tu-cyclic-conflicting.d: Likewise.
+       * testsuite/ld-ctf/forward.c: New test...
+       * testsuite/ld-ctf/forward.d: ... and results.
+
 2021-01-05  Nick Alcock  <nick.alcock@oracle.com>
 
        * testsuite/ld-ctf/array.d: Adjust for dumper changes.
index 4cbe9b61f3c26f9809640eed7699a586e34b8b52..5da66fda14c99371b59720ee68c1c7ef785776c8 100644 (file)
@@ -29,8 +29,8 @@ Contents of CTF section .ctf:
 #...
   Types:
 #...
-     0x[0-9a-f]*: struct B \(.*
-           *\[0x0\] \(ID 0x[0-9a-f]*\) \(kind 9\) struct B \(.*
+     0x[0-9a-f]*: struct B
+           *\[0x0\] \(ID 0x[0-9a-f]*\) \(kind 9\) struct B
 #...
 CTF archive member: .*:
 #...
index 3c975ebaa510824538eda4880360db2f72bab713..eff295edd308612f010aac9e478da1d740879d57 100644 (file)
@@ -23,8 +23,8 @@ Contents of CTF section \.ctf:
      0x[0-9a-f]*: int \[0x0:0x[0-9a-f]*\] \(size 0x[0-9a-f]*\)
            *\[0x0\] \(ID 0x[0-9a-f]*\) \(kind 1\) int:[0-9]* \(aligned at 0x[0-9a-f]*, format 0x1, offset:bits 0x0:0x[0-9a-f]*\)
 #...
-     0x[0-9a-f]*: struct A .*
-           *\[0x0\] \(ID 0x[0-9a-f]*\) \(kind 9\) struct A .*
+     0x[0-9a-f]*: struct A
+           *\[0x0\] \(ID 0x[0-9a-f]*\) \(kind 9\) struct A
 #...
      0x[0-9a-f]*: struct C .*
            *\[0x0\] \(ID 0x[0-9a-f]*\) \(kind 6\) struct C .*
diff --git a/ld/testsuite/ld-ctf/forward.c b/ld/testsuite/ld-ctf/forward.c
new file mode 100644 (file)
index 0000000..e41a7ae
--- /dev/null
@@ -0,0 +1,2 @@
+struct foo;
+struct foo *bar __attribute__((used));
diff --git a/ld/testsuite/ld-ctf/forward.d b/ld/testsuite/ld-ctf/forward.d
new file mode 100644 (file)
index 0000000..9ff0dd2
--- /dev/null
@@ -0,0 +1,23 @@
+#as:
+#source: forward.c
+#objdump: --ctf=.ctf
+#ld: -shared
+#name: Forwards
+
+.*: +file format .*
+
+Contents of CTF section .ctf:
+
+  Header:
+    Magic number: 0xdff2
+    Version: 4 \(CTF_VERSION_3\)
+#...
+    Type section:      .* \(0x18 bytes\)
+#...
+  Types:
+
+     0x[0-9a-f]: struct foo
+          *\[0x0\] \(ID 0x[0-9a-f]*\) \(kind 9\) struct foo
+     0x[0-9a-f]: struct foo \* \(size 0x[0-9a-f]*\) -> 0x[0-9a-f]: struct foo
+          *\[0x0\] \(ID 0x[0-9a-f]\) \(kind 3\) struct foo \* \(aligned at 0x[0-9a-f]*\)
+#...
index 80f05e90ff6920e6c98068dcfc12e7e7243488de..b9142541063d4a617128bde7a441fd3540f0de90 100644 (file)
@@ -1,3 +1,16 @@
+2021-01-05  Nick Alcock  <nick.alcock@oracle.com>
+
+       * ctf-types.c (ctf_type_resolve): Improve comment.
+       (ctf_type_size): Yield ECTF_INCOMPLETE when applied to forwards.
+       Emit errors into the right dict.
+       (ctf_type_align): Likewise.
+       * ctf-create.c (ctf_add_member_offset): Yield ECTF_INCOMPLETE
+       when adding a member without explicit offset when this member, or
+       the previous member, is incomplete.
+       * ctf-dump.c (ctf_dump_format_type): Do not try to print the size of
+       forwards.
+       (ctf_dump_member): Do not try to print their alignment.
+
 2021-01-05  Nick Alcock  <nick.alcock@oracle.com>
 
        * ctf-dump.c (ctf_dump_objts): Dump by calling ctf_dump_format_type.
index 0af46d3c28a81091e2493643157789611f5bb0ca..6fe7461c527854d97db0f6cb1ccc8c0f1946813a 100644 (file)
@@ -1690,6 +1690,14 @@ ctf_add_array (ctf_dict_t *fp, uint32_t flag, const ctf_arinfo_t *arp)
   if (ctf_lookup_by_id (&tmp, arp->ctr_index) == NULL)
     return CTF_ERR;            /* errno is set for us.  */
 
+  if (ctf_type_kind (fp, arp->ctr_index) == CTF_K_FORWARD)
+    {
+      ctf_err_warn (fp, 1, ECTF_INCOMPLETE,
+                   _("ctf_add_array: index type %lx is incomplete"),
+                   arp->ctr_contents);
+      return (ctf_set_errno (fp, ECTF_INCOMPLETE));
+    }
+
   if ((type = ctf_add_generic (fp, flag, NULL, CTF_K_ARRAY, &dtd)) == CTF_ERR)
     return CTF_ERR;            /* errno is set for us.  */
 
@@ -2040,6 +2048,7 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
   ssize_t msize, malign, ssize;
   uint32_t kind, vlen, root;
   char *s = NULL;
+  int is_incomplete = 0;
 
   if (!(fp->ctf_flags & LCTF_RDWR))
     return (ctf_set_errno (fp, ECTF_RDONLY));
@@ -2075,14 +2084,19 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
     {
       /* The unimplemented type, and any type that resolves to it, has no size
         and no alignment: it can correspond to any number of compiler-inserted
-        types.  */
-
+        types.  We allow incomplete types through since they are routinely
+        added to the ends of structures, and can even be added elsewhere in
+        structures by the deduplicator.  They are assumed to be zero-size with
+        no alignment: this is often wrong, but problems can be avoided in this
+        case by explicitly specifying the size of the structure via the _sized
+        functions.  The deduplicator always does this.  */
+
+      msize = 0;
+      malign = 0;
       if (ctf_errno (fp) == ECTF_NONREPRESENTABLE)
-       {
-         msize = 0;
-         malign = 0;
-         ctf_set_errno (fp, 0);
-       }
+       ctf_set_errno (fp, 0);
+      else if (ctf_errno (fp) == ECTF_INCOMPLETE)
+       is_incomplete = 1;
       else
        return -1;              /* errno is set for us.  */
     }
@@ -2123,10 +2137,32 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
              return -1;        /* errno is set for us.  */
            }
 
+         if (is_incomplete)
+           {
+             ctf_err_warn (fp, 1, ECTF_INCOMPLETE,
+                           _("ctf_add_member_offset: cannot add member %s of "
+                             "incomplete type %lx to struct %lx without "
+                             "specifying explicit offset\n"),
+                           name ? name : _("(unnamed member)"), type, souid);
+             return (ctf_set_errno (fp, ECTF_INCOMPLETE));
+           }
+
          if (ctf_type_encoding (fp, ltype, &linfo) == 0)
            off += linfo.cte_bits;
          else if ((lsize = ctf_type_size (fp, ltype)) > 0)
            off += lsize * CHAR_BIT;
+         else if (lsize == -1 && ctf_errno (fp) == ECTF_INCOMPLETE)
+           {
+             ctf_err_warn (fp, 1, ECTF_INCOMPLETE,
+                           _("ctf_add_member_offset: cannot add member %s of "
+                             "type %lx to struct %lx without specifying "
+                             "explicit offset after member %s of type %lx, "
+                             "which is an incomplete type\n"),
+                           name ? name : _("(unnamed member)"), type, souid,
+                           lmd->dmd_name ? lmd->dmd_name
+                           : _("(unnamed member)"), ltype);
+             return -1;                        /* errno is set for us.  */
+           }
 
          /* Round up the offset of the end of the last member to
             the next byte boundary, convert 'off' to bytes, and
index f01f225d4326e4100e93e1c1b07117e453dbd3fa..abed48310f6c70c2a7ac28853655e92fa698198a 100644 (file)
@@ -151,7 +151,7 @@ ctf_dump_format_type (ctf_dict_t *fp, ctf_id_t id, int flag)
       free (bit);
       bit = NULL;
 
-      if (kind != CTF_K_FUNCTION)
+      if (kind != CTF_K_FUNCTION && kind != CTF_K_FORWARD)
        if (asprintf (&bit, " (size 0x%lx)%s",
                      (unsigned long) ctf_type_size (fp, id),
                      nonroot_trailer) < 0)
@@ -476,6 +476,7 @@ ctf_dump_member (const char *name, ctf_id_t id, unsigned long offset,
   char *bit = NULL;
   ctf_encoding_t ep;
   int has_encoding = 0;
+  int opened_paren = 0;
 
   /* Align neatly.  */
 
@@ -520,8 +521,9 @@ ctf_dump_member (const char *name, ctf_id_t id, unsigned long offset,
                    ep.cte_bits, (unsigned long) ctf_type_align (state->cdm_fp,
                                                                 id)) < 0)
        goto oom;
+      opened_paren = 1;
     }
-  else
+  else if (ctf_type_kind (state->cdm_fp, id) != CTF_K_FORWARD)
     {
       if (asprintf (&bit, "[0x%lx] (ID 0x%lx) (kind %i) %s%s%s "
                    "(aligned at 0x%lx", offset, id,
@@ -529,6 +531,14 @@ ctf_dump_member (const char *name, ctf_id_t id, unsigned long offset,
                    (name[0] != 0 && typestr[0] != 0) ? " " : "", name,
                    (unsigned long) ctf_type_align (state->cdm_fp, id)) < 0)
        goto oom;
+      opened_paren = 1;
+    }
+  else /* Forwards have no alignment.  */
+    {
+      if (asprintf (&bit, "[0x%lx] (ID 0x%lx) (kind %i) %s%s%s\n", offset, id,
+                   ctf_type_kind (state->cdm_fp, id), typestr,
+                   (name[0] != 0 && typestr[0] != 0) ? " " : "", name) < 0)
+       goto oom;
     }
 
   *state->cdm_str = str_append (*state->cdm_str, bit);
@@ -547,7 +557,8 @@ ctf_dump_member (const char *name, ctf_id_t id, unsigned long offset,
       bit = NULL;
     }
 
-  *state->cdm_str = str_append (*state->cdm_str, ")\n");
+  if (opened_paren)
+    *state->cdm_str = str_append (*state->cdm_str, ")\n");
   return 0;
 
  oom:
index 61891d67ba2de9856018befd96be3052b016b6ba..3ba635f48bbf961e8e3d3983d499f43ecd8c16dc 100644 (file)
@@ -583,7 +583,10 @@ ctf_variable_next (ctf_dict_t *fp, ctf_next_t **it, const char **name)
    against infinite loops, we implement simplified cycle detection and check
    each link against itself, the previous node, and the topmost node.
 
-   Does not drill down through slices to their contained type.  */
+   Does not drill down through slices to their contained type.
+
+   Callers of this function must not presume that a type it returns must have a
+   valid ctt_size: forwards do not, and must be separately handled.  */
 
 ctf_id_t
 ctf_type_resolve (ctf_dict_t *fp, ctf_id_t type)
@@ -911,6 +914,7 @@ ctf_type_aname_raw (ctf_dict_t *fp, ctf_id_t type)
 ssize_t
 ctf_type_size (ctf_dict_t *fp, ctf_id_t type)
 {
+  ctf_dict_t *ofp = fp;
   const ctf_type_t *tp;
   ssize_t size;
   ctf_arinfo_t ar;
@@ -942,12 +946,16 @@ ctf_type_size (ctf_dict_t *fp, ctf_id_t type)
       if ((size = ctf_get_ctt_size (fp, tp, NULL, NULL)) > 0)
        return size;
 
-      if (ctf_array_info (fp, type, &ar) < 0
-         || (size = ctf_type_size (fp, ar.ctr_contents)) < 0)
+      if (ctf_array_info (ofp, type, &ar) < 0
+         || (size = ctf_type_size (ofp, ar.ctr_contents)) < 0)
        return -1;              /* errno is set for us.  */
 
       return size * ar.ctr_nelems;
 
+    case CTF_K_FORWARD:
+      /* Forwards do not have a meaningful size.  */
+      return (ctf_set_errno (ofp, ECTF_INCOMPLETE));
+
     default: /* including slices of enums, etc */
       return (ctf_get_ctt_size (fp, tp, NULL, NULL));
     }
@@ -981,9 +989,9 @@ ctf_type_align (ctf_dict_t *fp, ctf_id_t type)
     case CTF_K_ARRAY:
       {
        ctf_arinfo_t r;
-       if (ctf_array_info (fp, type, &r) < 0)
+       if (ctf_array_info (ofp, type, &r) < 0)
          return -1;            /* errno is set for us.  */
-       return (ctf_type_align (fp, r.ctr_contents));
+       return (ctf_type_align (ofp, r.ctr_contents));
       }
 
     case CTF_K_STRUCT:
@@ -1009,7 +1017,7 @@ ctf_type_align (ctf_dict_t *fp, ctf_id_t type)
                const ctf_member_t *mp = vmp;
                for (; n != 0; n--, mp++)
                  {
-                   ssize_t am = ctf_type_align (fp, mp->ctm_type);
+                   ssize_t am = ctf_type_align (ofp, mp->ctm_type);
                    align = MAX (align, (size_t) am);
                  }
              }
@@ -1018,7 +1026,7 @@ ctf_type_align (ctf_dict_t *fp, ctf_id_t type)
                const ctf_lmember_t *lmp = vmp;
                for (; n != 0; n--, lmp++)
                  {
-                   ssize_t am = ctf_type_align (fp, lmp->ctlm_type);
+                   ssize_t am = ctf_type_align (ofp, lmp->ctlm_type);
                    align = MAX (align, (size_t) am);
                  }
              }
@@ -1030,7 +1038,7 @@ ctf_type_align (ctf_dict_t *fp, ctf_id_t type)
              for (dmd = ctf_list_next (&dtd->dtd_u.dtu_members);
                   dmd != NULL; dmd = ctf_list_next (dmd))
                {
-                 ssize_t am = ctf_type_align (fp, dmd->dmd_type);
+                 ssize_t am = ctf_type_align (ofp, dmd->dmd_type);
                  align = MAX (align, (size_t) am);
                  if (kind == CTF_K_STRUCT)
                    break;
@@ -1043,6 +1051,10 @@ ctf_type_align (ctf_dict_t *fp, ctf_id_t type)
     case CTF_K_ENUM:
       return fp->ctf_dmodel->ctd_int;
 
+    case CTF_K_FORWARD:
+      /* Forwards do not have a meaningful alignment.  */
+      return (ctf_set_errno (ofp, ECTF_INCOMPLETE));
+
     default:  /* including slices of enums, etc */
       return (ctf_get_ctt_size (fp, tp, NULL, NULL));
     }