Issue Type: | Bug |
---|---|
Priority: | 4 - Normal |
Status: | Open |
Created at: | 2017-11-14T15:27:57.526Z |
Updated at: | 2019-03-15T11:05:09.165Z |
Created by: | Jonathan Perkin |
---|---|
Reported by: | Jonathan Perkin |
Assigned to: | Jonathan Perkin |
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;
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: