From eba390d0743d7cd134c668c9d4f41de4fb312c3e Mon Sep 17 00:00:00 2001 From: Nick Alcock Date: Wed, 24 May 2017 17:57:05 +0100 Subject: [PATCH] ctf: prevent modules on the dedup blacklist from sharing any types at all The deduplication blacklist exists to deal with a few modules, mostly old ISA sound drivers, which consist of a #define and a #include of another module, which then uses #ifdef profusely, sometimes on structure members. This leads to DWARF which has differently shaped structures in the same source location, which conflicts with the purpose of the deduplicator, which is to use the source location to identify identical structures and merge them. Before now, the blacklist worked by preventing the appearance of a structure in a module on the blacklist from making a structure shared: it could still be shared if other modules defined it. This fails to work for two reasons. Firstly, if it is used by only *one* other module, we'll hit an assertion failure in the emission phase which checks that any type it emits is either defined in the module being emitted or in the shared type repo. We could remove that assertion, but we'd still be in the wrong, because you could easily have a type in some header used by many modules that said struct trouble { #ifdef MODULE_FOO /* one definition */ #else /* a different definition */ #endif } and a module that says #define MODULE_FOO 1 #include Even if we blacklisted this module (and we would), this would still fail, because 'struct trouble' would end up in the shared type repository, and the existing code would fail to emit a new definition of it in module blah, even though it should because its definition is different. This shows that if a module is pulling tricks like this we cannot trust its use of shared types at all, since we have no visibility into preprocessor definitions: regardless of the space cost (about 40KiB per module), we cannot let it share any types whatsoever with the rest of the kernel. Rather than piling heaps of blacklist checks in all over dwarf2ctf to implement this, we do it by exploiting the fact that the deduplicator works entirely on the basis of type IDs. We add a 'blacklist type prefix' that type_id() can add to the start of its IDs (with some extra decoration, because the start of type IDs is a file path, so we want this to be an impossible path). If we set this prefix to the module name if and only if the module is blacklisted, and do not add one otherwise, then every blacklisted module will have a unique set of IDs for all its types, which can be shared within the module but not outside it, so every type in the module will be unique and none of them will end up in the shared type repository. While we're at it, add yet another ancient ISA sound driver that plays the same games to the blacklist. This fix makes blacklisting modules much more space-expensive: each such module expands the current size of the kernel module package by about 40KiB. (But there is only one blacklisted module built in current UEK, so this is a tolerable cost.) Signed-off-by: Nick Alcock Reviewed-by: vincent.lim@oracle.com Orabug: 26137220 --- scripts/dwarf2ctf/dedup.blacklist | 1 + scripts/dwarf2ctf/dwarf2ctf.c | 58 ++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/scripts/dwarf2ctf/dedup.blacklist b/scripts/dwarf2ctf/dedup.blacklist index 3f53fb4137844..5d588367556cf 100644 --- a/scripts/dwarf2ctf/dedup.blacklist +++ b/scripts/dwarf2ctf/dedup.blacklist @@ -1,3 +1,4 @@ snd-ens1371 snd-opti93x +snd-opti92x-cs4231 snd-interwave-stb diff --git a/scripts/dwarf2ctf/dwarf2ctf.c b/scripts/dwarf2ctf/dwarf2ctf.c index 7a83d49aafdd1..aed79ca06f2cf 100644 --- a/scripts/dwarf2ctf/dwarf2ctf.c +++ b/scripts/dwarf2ctf/dwarf2ctf.c @@ -181,8 +181,7 @@ static void init_builtin(const char *builtin_objects_file, * proceed to add or remove members from structures depending on which source * file they were included from. * - * These modules still share types with the rest of the kernel, but types that - * only they share with other modules will not be shared for that reason alone. + * These modules share no types whatsoever with the rest of the kernel. * * This is, of course, only used if deduplication is turned on. */ @@ -193,6 +192,12 @@ static GHashTable *dedup_blacklist; */ static void init_dedup_blacklist(const char *dedup_blacklist_file); +/* + * See if the given module is blacklisted, and update state accordingly so that + * type IDs are appropriately augmented. + */ +static void dedup_blacklisted_module(const char *module_name); + /* * The member blacklist bans fields with specific names in specifically named * structures, declared in specific source files, from being emitted. The @@ -289,6 +294,12 @@ static char *type_id(Dwarf_Die *die, die_override_t *overrides, void *data), void *data) __attribute__((__warn_unused_result__)); +/* + * If non-NULL, a prefix attached to all type ID types. This is used to ensure + * that types in blacklisted modules do not appear anywhere else. + */ +static char *blacklist_type_prefix; + /* * Process a file, calling the dwarf_process function for every type found * therein (even types in functions). Optionally call tu_init() at the start of @@ -1171,6 +1182,32 @@ static void init_dedup_blacklist(const char *dedup_blacklist_file) fclose(f); } +/* + * See if the given module is blacklisted, and update state accordingly so that + * type IDs are appropriately augmented. + */ +static void dedup_blacklisted_module(const char *module_name) +{ + if (dedup_blacklist == NULL) + return; + if (g_hash_table_lookup_extended(dedup_blacklist, module_name, + NULL, NULL)) { + /* + * The prefix goes before the DWARF file pathname, so we pick + * something that is not going to be a valid path on any POSIX + * system. + */ + free(blacklist_type_prefix); + blacklist_type_prefix = NULL; + blacklist_type_prefix = str_appendn(blacklist_type_prefix, + "/dev/null/@blacklisted: ", + module_name, "@", NULL); + } else { + free(blacklist_type_prefix); + blacklist_type_prefix = NULL; + } +} + /* * Populate the member blacklist from the member_blacklist file. */ @@ -1503,7 +1540,9 @@ static char *type_id(Dwarf_Die *die, * * Otherwise, note the location of this DIE, providing scoping * information for all types based upon this one. Location elements are - * separated by //, an element impossible in a Linux path. + * separated by //, an element impossible in a Linux path. The + * blacklist type prefix (if set) follows this (which is a name which, + * while not impossible in a Linux path, is very unlikely.) * * Array dimensions get none of this: they must be contained within * another DIE, so will always have a location attached via that DIE, @@ -1535,7 +1574,11 @@ static char *type_id(Dwarf_Die *die, snprintf(line_num, sizeof (line_num), "%i", decl_line_num); } - id = str_appendn(id, fname, "//", line_num, "//", NULL); + if (!blacklist_type_prefix) + id = str_appendn(id, fname, "//", line_num, "//", NULL); + else + id = str_appendn(id, blacklist_type_prefix, "//", fname, + "//", line_num, "//", NULL); } } @@ -1801,6 +1844,12 @@ static void process_file(const char *file_name, } + /* + * This arranges to augment type IDs appropriately for + * dedup-blacklisted modules for everything that uses + * process_file(). We reset it at the end. + */ + dedup_blacklisted_module(module_name); if (tu_init != NULL) tu_init(module_name, file_name, tu_die, data); @@ -1810,6 +1859,7 @@ static void process_file(const char *file_name, if (tu_done != NULL) tu_done(module_name, file_name, tu_die, data); } + dedup_blacklisted_module(""); free(fn_module_name); simple_dwfl_free(dwfl); -- 2.50.1