OS-6457: CTF assertion failed cmp->cm_tmap[id].cmt_map == suid

Details

Issue Type:Bug
Priority:4 - Normal
Status:Open
Created at:2017-11-14T15:27:57.526Z
Updated at:2019-03-15T11:05:09.165Z

People

Created by:Jonathan Perkin
Reported by:Jonathan Perkin
Assigned to:Jonathan Perkin

Related Issues

Description

A class of failures found in the CTF pkgsrc work is the following assertion failure:

assertion failed for thread 0xfed72a40, thread-id 1: cmp->cm_tmap[id].cmt_map == suid, file /home/jperkin/smartos-live-new/projects/illumos/usr/src/lib/libctf/common/ctf_merge.c, line 511

The gist of the assertion check is this:

ctf_merge_add_sou(ctf_merge_types_t *cmp, ctf_id_t id, boolean_t forward)
{
	...
	if (kind == CTF_K_STRUCT)
		suid = ctf_add_struct(cmp->cm_out, flags, name);
	else
		suid = ctf_add_union(cmp->cm_out, flags, name);
	...
	/*
	 * If this is a forward reference then it's mapping should already
	 * exist.
	 */
	if (forward == B_FALSE) {
		VERIFY(cmp->cm_tmap[id].cmt_map == 0);
		cmp->cm_tmap[id].cmt_map = suid;
	} else {
		VERIFY(cmp->cm_tmap[id].cmt_map == suid);
	}

Taking mandoc as an example of a package which fails, this is caused by a forward declaration of a union. In mandoc's case:

./mdoc.h:union  mdoc_data {
./roff.h:union mdoc_data;
./roff.h: union mdoc_data *norm; /* Normalized arguments. */

The CTF code currently assumes that forwards are structs in at least two places.

lib/libctf/common/ctf_merge.c:

ctf_merge_add_forward(ctf_merge_types_t *cmp, ctf_id_t id)
{
	...
	/*
	 * ctf_add_forward tries to check to see if a given forward already
	 * exists in one of its hash tables. If we're here then we know that we
	 * have a forward in a container that isn't present in another.
	 * Therefore, we choose a token hash table to satisfy the API choice
	 * here.
	*/
	ret = ctf_add_forward(cmp->cm_out, flags, name, CTF_K_STRUCT);

common/ctf/ctf_create.c:

ctf_add_type(ctf_file_t *dst_fp, ctf_file_t *src_fp, ctf_id_t src_type)
{
	...
	case CTF_K_FORWARD:
		if (dst_type == CTF_ERR) {
			dst_type = ctf_add_forward(dst_fp, flag, name, CTF_K_STRUCT); /* assume STRUCT */
		}

Thus when suid is instead added correctly as a union the previous forward
definition isn't found and the mapping is incorrect. I'm still trying to figure
out how to fix this correctly by determining the correct kind when adding the
forward, but until then the following cheap hack at least makes things work:

@@ -495,7 +500,10 @@ ctf_merge_add_sou(ctf_merge_types_t *cmp, ctf_id_t id, boolean_t forward)
                ctf_dprintf("added sou \"%s\" as (%d) %d->%d\n", name, kind, id,
                    suid);
        } else {
-               VERIFY(cmp->cm_tmap[id].cmt_map == suid);
+               if (kind == CTF_K_STRUCT)
+                       VERIFY(cmp->cm_tmap[id].cmt_map == suid);
+               else
+                       cmp->cm_tmap[id].cmt_map = suid;
        }
        cmp->cm_tmap[id].cmt_fixup = B_TRUE;

Comments

Comment by Former user
Created at 2017-11-28T01:21:29.861Z

Thanks for writing this up, Jonathan. So there are a couple of different gotchas going on here. The crux of the problem is that the CTF format doesn't actually encode what type a forward is in the raw binary format. However, we do include this information while we're working with dynamic types. So while doing a conversion we should actually have this in memory.

With that in mind, I wonder if we can actually try and do something a little better possibly today. Today, we do record what kind of type this is a forward for in the dynamic data. So maybe in both of those cases we should actually try and do a ctf_dtd_lookup() and see if we can get out the type. While that will only solve the basic case, I wonder if we should allow for us to add an enum that's of type unknown.

So my token idea here is: