From 3152b8356bf4cc82f4eab768a627e851c39bf893 Mon Sep 17 00:00:00 2001 From: Nick Alcock Date: Wed, 6 Sep 2017 11:45:51 +0100 Subject: [PATCH] ctf: automate away the deduplication blacklist The deduplication blacklist in scripts/dwarf2ctf/dedup.blacklist is a great bit kludge. It contains a list of modules that cannot be deduplicated because they contain structures which are defined in the same location in different ways different kernel modules (usually because the structure is modified by preprocessor conditionals). But augmenting the blacklist is a pig, involving lots of poring over debugging output to find the structure to focus on. So automate the problem away, by augmenting type IDs for structures with the sizeof() the structure in a new component (separated from the others by //, a component invalid in POSIX pathnames, as usual). Helpfully this is made available to us in the DW_AT_byte_size attribute, so it's fast to obtain. (The component is optional because opaque structure declarations obviously cannot include it.) We adjust the one place that transforms transparent structure IDs into opaque ones to take this tag into account. This will still break for structures that are modified by preprocessor conditionals in such a way that one member is replaced by another with a different type but which has the same size as the one it replaces (perhaps one pointer to a structure being replaced by a pointer to a different structure), but in the interests of dwarf2ctf performance I'm avoiding solving this for now, since we are not hitting it, and solving it would require annotating structure IDs with some sort of hash of their member names: the overhead of recursing over all members every time we get an ID for a structure seems likely to be quite high, given how often we look up type_id()s. This change has no detectable effect on dwarf2ctf runtime, and shrinks the CTF output by about 40KiB. Signed-off-by: Nick Alcock Reviewed-by: Tomas Jedlicka Orabug: 26765112 --- scripts/dwarf2ctf/dedup.blacklist | 4 -- scripts/dwarf2ctf/dwarf2ctf.c | 76 ++++++++++++++++++++++++++++--- 2 files changed, 70 insertions(+), 10 deletions(-) delete mode 100644 scripts/dwarf2ctf/dedup.blacklist diff --git a/scripts/dwarf2ctf/dedup.blacklist b/scripts/dwarf2ctf/dedup.blacklist deleted file mode 100644 index 4898611d45b1..000000000000 --- a/scripts/dwarf2ctf/dedup.blacklist +++ /dev/null @@ -1,4 +0,0 @@ -snd_ens1371 -snd_opti93x -snd_opti92x_cs4231 -snd_interwave_stb diff --git a/scripts/dwarf2ctf/dwarf2ctf.c b/scripts/dwarf2ctf/dwarf2ctf.c index 6b993ad86b6f..8cd015802484 100644 --- a/scripts/dwarf2ctf/dwarf2ctf.c +++ b/scripts/dwarf2ctf/dwarf2ctf.c @@ -1683,11 +1683,65 @@ static char *type_id(Dwarf_Die *die, id = str_appendn(id, "enum ", dwarf_diename(die), " ", NULL); break; case DW_TAG_structure_type: - id = str_appendn(id, "struct ", dwarf_diename(die), " ", NULL); - break; - case DW_TAG_union_type: - id = str_appendn(id, "union ", dwarf_diename(die), " ", NULL); + case DW_TAG_union_type: { + /* + * Incorporate the sizeof() the structure, if statically known + * (the offset of the last member in the DWARF) so that most + * structures which are redefined on the fly by preprocessor + * defines are disambiguated despite being defined in the same + * place. + * + * Only do this if this is a non-opaque structure/union + * definition: opaque definitions cannot have a size, but if + * they do by some mischance get one, notating it will mess up + * the several other places that manually construct opaque + * structure identifiers (and cannot incorporate a size, since + * they don't know it). + */ + const char *sou; + + if (strncmp(id, "////", 4) != 0 && + private_dwarf_hasattr(die, DW_AT_byte_size)) { + Dwarf_Attribute size_attr; + long long size; + char byte_size[24]; + + private_dwarf_attr(die, DW_AT_byte_size, &size_attr); + + switch (dwarf_whatform(&size_attr)) { + case DW_FORM_data1: + case DW_FORM_data2: + case DW_FORM_data4: + case DW_FORM_data8: + case DW_FORM_udata: + case DW_FORM_sdata: + + if (dwarf_whatform(&size_attr) == + DW_FORM_sdata) { + Dwarf_Sword dw_size; + + dwarf_formsdata(&size_attr, &dw_size); + size = dw_size; + } else { + Dwarf_Word dw_size; + + dwarf_formudata(&size_attr, &dw_size); + size = dw_size; + } + + sprintf(byte_size, "%lli", size); + id = str_appendn(id, byte_size, "//", NULL); + } + } + + if (dwarf_tag(die) == DW_TAG_union_type) + sou = "union "; + else + sou = "struct "; + + id = str_appendn(id, sou, dwarf_diename(die), " ", NULL); break; + } case DW_TAG_variable: id = str_appendn(id, "var ", dwarf_diename(die), " ", NULL); break; @@ -2619,6 +2673,7 @@ static int detect_duplicates_alias_fixup(void *id_file_data, void *data) char *opaque_id; const char *line_num; + const char *type_size; const char *type_name; /* @@ -2635,12 +2690,21 @@ static int detect_duplicates_alias_fixup(void *id_file_data, void *data) exit(1); } - type_name = strstr(line_num + 1, "//"); - if (!type_name) { + type_size = strstr(line_num + 2, "//"); + if (!type_size) { fprintf(stderr, "Internal error: type ID %s is corrupt.\n", id_file->id); exit(1); } + + type_name = strstr(type_size + 2, "//"); + if (!type_name) { + /* + * That's OK: the type size is optional, so what we thought was + * the type size is actually the type name. + */ + type_name = type_size; + } type_name += 2; opaque_id = xstrdup("////"); -- 2.50.1