parser, xlators, printf, cg, ident: resolve forwards to real types

In contrast to libdtrace-ctf (which didn't reliably handle conflicted
structs and unions at all and often just assumed they weren't
conflicting, leading to corrupted type graphs), libctf's deduplicator
reliably identifies conflicted structs and unions, and avoids getting
caught in cycles while maximizing the number of shared types by
routinely converting conflicted structs and unions into forwards and
leaving the forward in the shared dict as a marker.

This means that forwards can and do appear in the shared graph in places
where they would be forbidden in C because there are things you can't do
with incomplete types: you can see things like arrays of forwards.  We'd
rather not inflict this on our users, so introduce a new dt_type_resolve
which is like ctf_type_resolve except that rather than returning a
forward when called on a type in the shared dict, it hunts through the
module list for the first non-forward with a matching name and suitable
kind (so a forward to 'struct foo' will only match a real 'struct foo',
not 'union foo'), returning the dict as well.  It only returns a forward
if no suitable complete type is found in any module at all.

This search can be relatively expensive, because it might involve
loading lots of modules, but it only happens when conflicted forwards
are found, which isn't all that common: and correctness trumps
performance anyway.

This is made a bit more complex because there were several places where
we weren't bothering to call ctf_type_resolve at all but where a
resolution is nonetheless required in any case (e.g. dt_idsize_type)
and a number of places where we were just *assuming* that
ctf_type_resolve succeeded (most unwise).  There are also a number of
places where we need to track both the fp of the original type and the
fp of the resolved type, notably in xlator-related code.

A couple of places have to *avoid* searching for real types, in
particular if a forward is found in the C and D dicts: dt_type_resolve
takes a flag to turn this off.  This was spotted heuristically (i.e.
a test failed): I don't *think* there are any places where this needs
doing that I missed, but I could be wrong.

(In the future some of this commit may be unwindable, if and when libctf
gains the ability to search for forwards in archives in the same way we
are doing here.  It seems like a worthwhile API addition.)

Signed-off-by: Nick Alcock <nick.alcock@oracle.com>

libdtrace/dt_cg.c

@@ -1872,8 +1872,12 @@ dt_cg_load(dt_node_t *dnp, ctf_file_t *ctfp, ctf_id_t type)
 	if ((dnp->dn_flags & DT_NF_BITFIELD) &&
 	    ctf_type_encoding(ctfp, type, &e) != CTF_ERR)
 		size = clp2(P2ROUNDUP(e.cte_bits, NBBY) / NBBY);
-	else
+	else {
+		if (ctf_type_kind(ctfp, type) == CTF_K_FORWARD)
+			type = dt_type_resolve(yypcb->pcb_hdl, &ctfp,
+			    type, 0);
 		size = ctf_type_size(ctfp, type);
+	}
 
 	if (size < 1 || size > 8 || (size & (size - 1)) != 0) {
 		xyerror(D_UNKNOWN, "internal error -- cg cannot load "
@@ -1995,7 +1999,7 @@ dt_cg_ptrsize(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 	uint_t kind;
 	ssize_t size;
 
-	type = ctf_type_resolve(ctfp, dnp->dn_type);
+	type = dt_type_resolve(yypcb->pcb_hdl, &ctfp, dnp->dn_type, 0);
 	kind = ctf_type_kind(ctfp, type);
 	assert(kind == CTF_K_POINTER || kind == CTF_K_ARRAY);
 
@@ -2098,14 +2102,14 @@ dt_cg_field_set(dt_node_t *src, dt_irlist_t *dlp,
 	assert(dst->dn_right->dn_kind == DT_NODE_IDENT);
 
 	fp = dst->dn_left->dn_ctfp;
-	type = ctf_type_resolve(fp, dst->dn_left->dn_type);
+	type = dt_type_resolve(yypcb->pcb_hdl, &fp, dst->dn_left->dn_type, 0);
 
 	if (dst->dn_op == DT_TOK_PTR) {
 		type = ctf_type_reference(fp, type);
-		type = ctf_type_resolve(fp, type);
+		type = dt_type_resolve(yypcb->pcb_hdl, &fp, type, 0);
 	}
 
-	if ((fp = dt_cg_membinfo(ofp = fp, type,
+	if ((fp = dt_cg_membinfo(ofp = dst->dn_left->dn_ctfp, type,
 	    dst->dn_right->dn_string, &m)) == NULL) {
 		yypcb->pcb_hdl->dt_ctferr = ctf_errno(ofp);
 		longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
@@ -2485,7 +2489,7 @@ dt_cg_prearith_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp, uint_t op)
 	ssize_t size = 1;
 
 	if (dt_node_is_pointer(dnp)) {
-		type = ctf_type_resolve(ctfp, dnp->dn_type);
+		type = dt_type_resolve(yypcb->pcb_hdl, &ctfp, dnp->dn_type, 0);
 		assert(ctf_type_kind(ctfp, type) == CTF_K_POINTER);
 		size = ctf_type_size(ctfp, ctf_type_reference(ctfp, type));
 	}
@@ -2533,7 +2537,7 @@ dt_cg_postarith_op(dt_node_t *dnp, dt_irlist_t *dlp,
 	int oreg, nreg;
 
 	if (dt_node_is_pointer(dnp)) {
-		type = ctf_type_resolve(ctfp, dnp->dn_type);
+		type = dt_type_resolve(yypcb->pcb_hdl, &ctfp, dnp->dn_type, 0);
 		assert(ctf_type_kind(ctfp, type) == CTF_K_POINTER);
 		size = ctf_type_size(ctfp, ctf_type_reference(ctfp, type));
 	}
@@ -3581,14 +3585,15 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 		}
 
 		ctfp = dnp->dn_left->dn_ctfp;
-		type = ctf_type_resolve(ctfp, dnp->dn_left->dn_type);
+		type = dt_type_resolve(yypcb->pcb_hdl, &ctfp,
+		    dnp->dn_left->dn_type, 0);
 
 		if (dnp->dn_op == DT_TOK_PTR) {
 			type = ctf_type_reference(ctfp, type);
-			type = ctf_type_resolve(ctfp, type);
+			type = dt_type_resolve(yypcb->pcb_hdl, &ctfp, type, 0);
 		}
 
-		if ((ctfp = dt_cg_membinfo(octfp = ctfp, type,
+		if ((ctfp = dt_cg_membinfo(octfp = dnp->dn_left->dn_ctfp, type,
 		    dnp->dn_right->dn_string, &m)) == NULL) {
 			yypcb->pcb_hdl->dt_ctferr = ctf_errno(octfp);
 			longjmp(yypcb->pcb_jmpbuf, EDT_CTF);

libdtrace/dt_decl.c

@@ -527,9 +527,10 @@ dt_decl_member(dt_node_t *dnp)
 		    "cannot have dynamic member: %s\n", ident);
 	}
 
-	base = ctf_type_resolve(dtt.dtt_ctfp, dtt.dtt_type);
-	kind = ctf_type_kind(dtt.dtt_ctfp, base);
-	size = ctf_type_size(dtt.dtt_ctfp, base);
+	ctf_file_t *fp = dtt.dtt_ctfp;
+	base = dt_type_resolve(yypcb->pcb_hdl, &fp, dtt.dtt_type, 0);
+	kind = ctf_type_kind(fp, base);
+	size = ctf_type_size(fp, base);
 
 	if (kind == CTF_K_FORWARD || ((kind == CTF_K_STRUCT ||
 	    kind == CTF_K_UNION) && size == 0)) {
@@ -567,8 +568,8 @@ dt_decl_member(dt_node_t *dnp)
 			    "expression expected as bit-field size\n");
 		}
 
-		if (ctf_type_kind(dtt.dtt_ctfp, base) != CTF_K_INTEGER ||
-		    ctf_type_encoding(dtt.dtt_ctfp, base, &cte) == CTF_ERR ||
+		if (ctf_type_kind(fp, base) != CTF_K_INTEGER ||
+		    ctf_type_encoding(fp, base, &cte) == CTF_ERR ||
 		    IS_VOID(cte)) {
 			xyerror(D_DECL_BFTYPE, "invalid type for "
 			    "bit-field: %s\n", idname);
@@ -790,6 +791,10 @@ dt_decl_enumerator(char *s, dt_node_t *dnp)
  * the underlying type names is handled by dt_type_lookup().  We build up the
  * name from the specified string and prefixes and then lookup the type.  If
  * we fail, an errmsg is saved and the caller must abort with EDT_COMPILER.
+ *
+ * Pointers to forwards in the C and D dictionaries are not chased down in the
+ * original dicts, because they will likely have been added by dt_decl_type
+ * itself, and cannot be the result of the link-time deduplicator.
  */
 int
 dt_decl_type(dt_decl_t *ddp, dtrace_typeinfo_t *tip)
@@ -852,7 +857,7 @@ dt_decl_type(dt_decl_t *ddp, dtrace_typeinfo_t *tip)
 		}
 
 		if ((rv = dt_decl_type(ddp->dd_next, tip)) == 0 &&
-		    (rv = dt_type_pointer(tip)) != 0) {
+		    (rv = dt_type_pointer(tip, 1)) != 0) {
 			xywarn(D_UNKNOWN, "cannot find type: %s*: %s\n",
 			    dt_type_name(tip->dtt_ctfp, tip->dtt_type,
 			    n, sizeof(n)), ctf_errmsg(dtp->dt_ctferr));
@@ -1015,6 +1020,7 @@ dt_decl_type(dt_decl_t *ddp, dtrace_typeinfo_t *tip)
 	}
 
 	if (type == CTF_ERR || ctf_update(dmp->dm_ctfp) == CTF_ERR) {
+		dt_dprintf ("can't update dict for module %s\n", dmp->dm_name);
 		xywarn(D_UNKNOWN, "failed to add forward tag for %s: %s\n",
 		    name, ctf_errmsg(ctf_errno(dmp->dm_ctfp)));
 		return -1;

libdtrace/dt_ident.c

@@ -552,7 +552,13 @@ dt_iddtor_difo(dt_ident_t *idp)
 static size_t
 dt_idsize_type(dt_ident_t *idp)
 {
-	return ctf_type_size(idp->di_ctfp, idp->di_type);
+	ctf_file_t *fp = idp->di_ctfp;
+	ctf_id_t type;
+
+	type = dt_type_resolve(yypcb->pcb_hdl, &fp, idp->di_type, 0);
+	if (type == CTF_ERR)
+		return type;
+	return ctf_type_size(fp, type);
 }
 
 /*ARGSUSED*/

libdtrace/dt_impl.h

@@ -489,6 +489,10 @@ struct dtrace_hdl {
 #define	DT_USYMADDR_CTFP(dtp)	((dtp)->dt_ddefs->dm_ctfp)
 #define	DT_USYMADDR_TYPE(dtp)	((dtp)->dt_type_usymaddr)
 
+#define DT_RESOLVE_CD_FORWARDS_OK 0x0001
+
+ctf_id_t dt_type_resolve(dtrace_hdl_t *, ctf_file_t **, ctf_id_t, int);
+
 /*
  * Actions and subroutines are both DT_NODE_FUNC nodes; to avoid confusing
  * an action for a subroutine (or vice versa), we assure that the DT_ACT_*
@@ -799,6 +803,8 @@ extern dt_lib_depend_t *dt_lib_depend_lookup(dt_list_t *, const char *);
 
 extern int dt_variable_read(caddr_t, size_t, uint64_t *);
 
+extern ctf_id_t dtrace_type_resolve(dtrace_hdl_t *, ctf_file_t *, ctf_id_t);
+
 extern dt_pcb_t *yypcb;		/* pointer to current parser control block */
 extern char yyintprefix;	/* int token prefix for macros (+/-) */
 extern char yyintsuffix[4];	/* int token suffix ([uUlL]*) */

libdtrace/dt_module.c

@@ -1529,8 +1529,12 @@ dtrace_lookup_by_name(dtrace_hdl_t *dtp, const char *object, const char *name,
 				symp->st_other = 0;
 				symp->st_shndx = SHN_UNDEF;
 				symp->st_value = 0;
-				symp->st_size =
-				    ctf_type_size(idp->di_ctfp, idp->di_type);
+                                if (ctf_type_size (idp->di_ctfp,
+                                        idp->di_type) >= 0)
+                                        symp->st_size =
+                                            ctf_type_size(idp->di_ctfp, idp->di_type);
+                                else
+                                        symp->st_size = 0;
 			}
 
 			if (sip != NULL) {
@@ -1718,6 +1722,127 @@ dtrace_lookup_by_type(dtrace_hdl_t *dtp, const char *object, const char *name,
 	return 0;
 }
 
+/*
+ * Resolve a type, searching for a corresponding non-forward if a forward is
+ * found in the shared dict, or (unless suppressed via the
+ * DT_RESOLVE_CD_FORWARDS_OK flag) in the C or D dicts, from which it may have
+ * been copied from the shared dict via direct use in a D definition or a
+ * translator.
+ *
+ * This function wraps ctf_type_resolve and is treated just like it by callers,
+ * so it must not affect the DTrace error state except in utterly catastrophic
+ * cases like OOM.
+ */
+ctf_id_t
+dt_type_resolve(dtrace_hdl_t *dtp, ctf_file_t **fp, ctf_id_t type, int flags)
+{
+	dt_module_t *dmp = dt_list_next(&dtp->dt_modlist);
+	ctf_id_t forwarded_type = -1;
+	ssize_t namelen = 255;
+	char *name;
+	int err = 0;
+	int old_errno = dtp->dt_errno;
+	ctf_id_t oldtype = type;
+
+	/*
+	 * Getting a name in a libdtrace-ctf-compatible way is horribly
+	 * inconvenient.  DTrace v2 should use ctf_type_aname.
+	 */
+	if ((name = malloc(namelen)) == 0)
+		return (dt_set_errno(dtp, EDT_NOMEM));
+
+	if ((type = ctf_type_resolve(*fp, type)) == CTF_ERR ||
+	    ctf_type_kind(*fp, type) != CTF_K_FORWARD ||
+	    (*fp != dtp->dt_shared_ctf &&
+		((flags & DT_RESOLVE_CD_FORWARDS_OK) ||
+		    (*fp != dtp->dt_cdefs->dm_ctfp &&
+			*fp != dtp->dt_ddefs->dm_ctfp))))
+	{
+		dt_dprintf("dt_type_resolve: error %s, type kind %i: "
+		    "resolved %lx to %lx\n", (type == CTF_ERR ?
+			ctf_errmsg (ctf_errno (*fp)) : "(no error)"),
+		    ctf_type_kind (*fp, type), oldtype, type);
+		goto ret;
+	}
+
+	namelen = ctf_type_lname(*fp, type, name, namelen) + 1;
+
+	if (namelen > 255) {
+		char *newname;
+		if ((newname = realloc(name, namelen)) == NULL) {
+			free(name);
+			return (dt_set_errno(dtp, EDT_NOMEM));
+		}
+		name = newname;
+		ctf_type_lname(*fp, type, name, namelen);
+	}
+
+	dt_dprintf("dt_type_resolve: %s is a forward in the shared dict, "
+	    "hunting in sub-dictionaries\n", name);
+
+	/*
+	 * Forward in the shared dict or a translator.  Search modules for a
+	 * corresponding concrete type.  Any error terminates the check.
+	 * CTF_NOTYPE is impossible if this type is in the shared dict; if it is
+	 * *not* in the shared dict but was copied into the C or D dict from a
+	 * child dict, this cannot be a dedup-introduced ambiguous forward and
+	 * thus is not something we need to hunt for: so if we get a ECTF_NOTYPE
+	 * at any stage, return the original type unchanged, since the forward
+	 * is really the right thing to return.
+	 */
+	do {
+		if (dt_module_getctf(dtp, dmp) == NULL)
+			continue;
+
+		/*
+		 * We are only interested in searching the modules: i.e., the
+		 * shared dict, and dicts which have the shared dict as a
+		 * parent.
+		 */
+		if (dmp->dm_ctfp != dtp->dt_shared_ctf &&
+		    ctf_parent_file (dmp->dm_ctfp) != dtp->dt_shared_ctf)
+			continue;
+
+		forwarded_type = ctf_lookup_by_name(dmp->dm_ctfp, name);
+		if (forwarded_type == -1)
+			err = ctf_errno (dmp->dm_ctfp);
+
+		/*
+		 * Errors, or a successfully-found non-forward, terminate the
+		 * search.
+		 */
+		if (forwarded_type == -1 ||
+		    ctf_type_kind(dmp->dm_ctfp, forwarded_type) != CTF_K_FORWARD)
+		{
+			dt_dprintf ("Found real type, of kind %i in %s\n",
+			    ctf_type_kind (dmp->dm_ctfp, forwarded_type),
+			    dmp->dm_name);
+			break;
+		}
+	} while ((dmp = dt_list_next (dmp)) != NULL);
+
+	if (forwarded_type != -1 && dmp &&
+	    ctf_type_kind(dmp->dm_ctfp, forwarded_type) != CTF_K_FORWARD)
+	{
+		*fp = dmp->dm_ctfp;
+		type = forwarded_type;
+		goto ret;
+	}
+	/*
+	 * Pass back errors other than ECTF_NOTYPE untouched in the ctfp state;
+	 * otherwise, return the original type.
+	 */
+	if (err != 0 && err != ECTF_NOTYPE)
+	{
+		type = -1;
+		goto ret;
+	}
+
+ret:
+	dtp->dt_errno = old_errno;
+	return type;
+}
+
 int
 dtrace_symbol_type(dtrace_hdl_t *dtp, const GElf_Sym *symp,
     const dtrace_syminfo_t *sip, dtrace_typeinfo_t *tip)