From a132000868a18772dbb6e2ed663b9d045268c104 Mon Sep 17 00:00:00 2001 From: Artem Savkov Date: Tue, 3 Aug 2021 15:46:32 +0200 Subject: [PATCH 1/7] test/integration/rhel-8.0: disable unneeded test gcc-static-local-var-4.patch is disabled on this distribution, disable the test as well as it will always fail during 'slow' integration test runs. Signed-off-by: Artem Savkov --- ...atic-local-var-4.test => gcc-static-local-var-4.test.disabled} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/integration/rhel-8.0/{gcc-static-local-var-4.test => gcc-static-local-var-4.test.disabled} (100%) diff --git a/test/integration/rhel-8.0/gcc-static-local-var-4.test b/test/integration/rhel-8.0/gcc-static-local-var-4.test.disabled similarity index 100% rename from test/integration/rhel-8.0/gcc-static-local-var-4.test rename to test/integration/rhel-8.0/gcc-static-local-var-4.test.disabled From db442d14053c1c9fea66ad120701bd6ed0ac3634 Mon Sep 17 00:00:00 2001 From: Artem Savkov Date: Mon, 2 Aug 2021 11:22:32 +0200 Subject: [PATCH 2/7] Make lookup_symbol() accept struct symbol as an argument At the moment lookup_symbol() takes symbol name as an argument which might not be enough in some cases (e.g. for objectfiles built from multiple source files). Make it accept full symbol structure. Signed-off-by: Artem Savkov --- kpatch-build/create-diff-object.c | 8 ++++---- kpatch-build/lookup.c | 20 ++++++++++++-------- kpatch-build/lookup.h | 3 ++- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 1ea587f08..e57fab95c 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -2280,7 +2280,7 @@ static bool should_keep_jump_label(struct lookup_table *lookup, * jump label init. */ - if (lookup_symbol(lookup, key->sym->name, &symbol) && + if (lookup_symbol(lookup, key->sym, &symbol) && strcmp(symbol.objname, "vmlinux")) { /* The static key lives in a module -- not supported */ @@ -2935,7 +2935,7 @@ static void kpatch_create_patches_sections(struct kpatch_elf *kelf, sym->parent) continue; - if (!lookup_symbol(table, sym->name, &symbol)) + if (!lookup_symbol(table, sym, &symbol)) ERROR("can't find symbol '%s' in symbol table", sym->name); if (sym->bind == STB_LOCAL && symbol.global) @@ -3096,7 +3096,7 @@ static bool need_dynrela(struct lookup_table *table, const struct rela *rela) !strchr(toc_rela(rela)->sym->name, '.'); } - if (!lookup_symbol(table, rela->sym->name, &symbol)) { + if (!lookup_symbol(table, rela->sym, &symbol)) { /* * Assume the symbol lives in another .o in the patch module. * A normal rela should work. @@ -3282,7 +3282,7 @@ static void kpatch_create_intermediate_sections(struct kpatch_elf *kelf, ERROR("unsupported dynrela reference to symbol '%s' in module-specific special section '%s'", rela->sym->name, sec->base->name); - if (!lookup_symbol(table, rela->sym->name, &symbol)) + if (!lookup_symbol(table, rela->sym, &symbol)) ERROR("can't find symbol '%s' in symbol table", rela->sym->name); diff --git a/kpatch-build/lookup.c b/kpatch-build/lookup.c index 5c89c1800..d9a35607b 100644 --- a/kpatch-build/lookup.c +++ b/kpatch-build/lookup.c @@ -407,7 +407,8 @@ void lookup_close(struct lookup_table *table) free(table); } -static bool lookup_local_symbol(struct lookup_table *table, char *name, +static bool lookup_local_symbol(struct lookup_table *table, + struct symbol *lookup_sym, struct lookup_result *result) { struct object_symbol *sym; @@ -419,7 +420,8 @@ static bool lookup_local_symbol(struct lookup_table *table, char *name, memset(result, 0, sizeof(*result)); for_each_obj_symbol(i, sym, table) { - if (sym->bind == STB_LOCAL && !strcmp(sym->name, name)) + if (sym->bind == STB_LOCAL && !strcmp(sym->name, + lookup_sym->name)) sympos++; if (table->local_syms == sym) { @@ -433,10 +435,12 @@ static bool lookup_local_symbol(struct lookup_table *table, char *name, if (sym->type == STT_FILE) break; - if (sym->bind == STB_LOCAL && !strcmp(sym->name, name)) { + if (sym->bind == STB_LOCAL && !strcmp(sym->name, + lookup_sym->name)) { if (result->objname) - ERROR("duplicate local symbol found for %s", name); + ERROR("duplicate local symbol found for %s", + lookup_sym->name); result->objname = table->objname; result->addr = sym->addr; @@ -511,14 +515,14 @@ static bool lookup_global_symbol(struct lookup_table *table, char *name, return !!result->objname; } -bool lookup_symbol(struct lookup_table *table, char *name, +bool lookup_symbol(struct lookup_table *table, struct symbol *sym, struct lookup_result *result) { - if (lookup_local_symbol(table, name, result)) + if (lookup_local_symbol(table, sym, result)) return true; - if (lookup_global_symbol(table, name, result)) + if (lookup_global_symbol(table, sym->name, result)) return true; - return lookup_exported_symbol(table, name, result); + return lookup_exported_symbol(table, sym->name, result); } diff --git a/kpatch-build/lookup.h b/kpatch-build/lookup.h index 8d299008c..ae92880e4 100644 --- a/kpatch-build/lookup.h +++ b/kpatch-build/lookup.h @@ -2,6 +2,7 @@ #define _LOOKUP_H_ #include +#include "kpatch-elf.h" struct lookup_table; @@ -22,7 +23,7 @@ struct lookup_table *lookup_open(char *symtab_path, char *objname, char *symvers_path, char *hint, struct sym_compare_type *locals); void lookup_close(struct lookup_table *table); -bool lookup_symbol(struct lookup_table *table, char *name, +bool lookup_symbol(struct lookup_table *table, struct symbol *sym, struct lookup_result *result); #endif /* _LOOKUP_H_ */ From 720768767d9df5bd7c14aec41404e79d50056f6b Mon Sep 17 00:00:00 2001 From: Artem Savkov Date: Mon, 2 Aug 2021 11:32:23 +0200 Subject: [PATCH 3/7] Switch to per-file lookup table pointers. So far create-diff-object worked only with objectfiles built from a single source file. To support object-files built from multiple sources such as linked vmlinux.o, we call locals_match() for each of STT_FILE symbols and store the pointer to the beginning of appropriate symbol block in lookup table in each symbol. Signed-off-by: Artem Savkov --- kpatch-build/create-diff-object.c | 67 ++---------------- kpatch-build/kpatch-elf.h | 1 + kpatch-build/lookup.c | 111 +++++++++++++++++++----------- kpatch-build/lookup.h | 8 +-- 4 files changed, 79 insertions(+), 108 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index e57fab95c..96a92c0a6 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -926,6 +926,10 @@ static void __kpatch_correlate_section(struct section *sec1, struct section *sec static void kpatch_correlate_symbol(struct symbol *sym1, struct symbol *sym2) { CORRELATE_ELEMENT(sym1, sym2, "symbol"); + if (sym1->lookup_table_file_sym && !sym2->lookup_table_file_sym) + sym2->lookup_table_file_sym = sym1->lookup_table_file_sym; + else if (!sym1->lookup_table_file_sym && sym2->lookup_table_file_sym) + sym1->lookup_table_file_sym = sym2->lookup_table_file_sym; } static void kpatch_correlate_static_local(struct symbol *sym1, struct symbol *sym2) @@ -2858,43 +2862,6 @@ static void kpatch_process_special_sections(struct kpatch_elf *kelf, kpatch_regenerate_orc_sections(kelf); } -static struct sym_compare_type *kpatch_elf_locals(struct kpatch_elf *kelf) -{ - struct symbol *sym; - int i = 0, sym_num = 0; - struct sym_compare_type *sym_array; - - list_for_each_entry(sym, &kelf->symbols, list) { - if (sym->bind != STB_LOCAL) - continue; - if (sym->type != STT_FUNC && sym->type != STT_OBJECT) - continue; - - sym_num++; - } - - if (!sym_num) - return NULL; - - sym_array = malloc((sym_num + 1) * sizeof(struct sym_compare_type)); - if (!sym_array) - ERROR("malloc"); - - list_for_each_entry(sym, &kelf->symbols, list) { - if (sym->bind != STB_LOCAL) - continue; - if (sym->type != STT_FUNC && sym->type != STT_OBJECT) - continue; - - sym_array[i].type = sym->type; - sym_array[i++].name = strdup(sym->name); - } - sym_array[i].type = 0; - sym_array[i].name = NULL; - - return sym_array; -} - static void kpatch_create_patches_sections(struct kpatch_elf *kelf, struct lookup_table *table, char *objname) @@ -3724,10 +3691,8 @@ int main(int argc, char *argv[]) int num_changed, callbacks_exist, new_globals_exist; struct lookup_table *lookup; struct section *sec, *symtab; - struct symbol *sym; - char *hint = NULL, *orig_obj, *patched_obj, *parent_name; + char *orig_obj, *patched_obj, *parent_name; char *parent_symtab, *mod_symvers, *patch_name, *output_obj; - struct sym_compare_type *base_locals, *sym_comp; memset(&arguments, 0, sizeof(arguments)); argp_parse (&argp, argc, argv, 0, NULL, &arguments); @@ -3761,21 +3726,7 @@ int main(int argc, char *argv[]) kpatch_detect_child_functions(kelf_base); kpatch_detect_child_functions(kelf_patched); - list_for_each_entry(sym, &kelf_base->symbols, list) { - if (sym->type == STT_FILE) { - hint = strdup(sym->name); - break; - } - } - if (!hint) { - log_normal("WARNING: FILE symbol not found in base. Stripped object file or assembly source?\n"); - return EXIT_STATUS_NO_CHANGE; - } - - base_locals = kpatch_elf_locals(kelf_base); - - lookup = lookup_open(parent_symtab, parent_name, mod_symvers, hint, - base_locals); + lookup = lookup_open(parent_symtab, parent_name, mod_symvers, kelf_base); kpatch_mark_grouped_sections(kelf_patched); kpatch_replace_sections_syms(kelf_base); @@ -3816,7 +3767,6 @@ int main(int argc, char *argv[]) log_debug("no changed functions were found, but callbacks exist\n"); else { log_debug("no changed functions were found\n"); - free(hint); return EXIT_STATUS_NO_CHANGE; } } @@ -3832,11 +3782,6 @@ int main(int argc, char *argv[]) */ kpatch_elf_teardown(kelf_patched); - for (sym_comp = base_locals; sym_comp && sym_comp->name; sym_comp++) - free(sym_comp->name); - free(base_locals); - free(hint); - kpatch_no_sibling_calls_ppc64le(kelf_out); /* create strings, patches, and dynrelas sections */ diff --git a/kpatch-build/kpatch-elf.h b/kpatch-build/kpatch-elf.h index 36f500aab..244866b30 100644 --- a/kpatch-build/kpatch-elf.h +++ b/kpatch-build/kpatch-elf.h @@ -76,6 +76,7 @@ struct symbol { struct section *sec; GElf_Sym sym; char *name; + struct object_symbol *lookup_table_file_sym; unsigned int index; unsigned char bind, type; enum status status; diff --git a/kpatch-build/lookup.c b/kpatch-build/lookup.c index d9a35607b..49e15614c 100644 --- a/kpatch-build/lookup.c +++ b/kpatch-build/lookup.c @@ -56,7 +56,6 @@ struct lookup_table { int obj_nr, exp_nr; struct object_symbol *obj_syms; struct export_symbol *exp_syms; - struct object_symbol *local_syms; char *objname; }; @@ -92,25 +91,31 @@ static int maybe_discarded_sym(const char *name) } static int locals_match(struct lookup_table *table, int idx, - struct sym_compare_type *child_locals) + struct symbol *file_sym, struct list_head *sym_list) { - struct sym_compare_type *child; - struct object_symbol *sym; + struct symbol *sym; + struct object_symbol *table_sym; int i, found; i = idx + 1; - for_each_obj_symbol_continue(i, sym, table) { - if (sym->type == STT_FILE) + for_each_obj_symbol_continue(i, table_sym, table) { + if (table_sym->type == STT_FILE) break; - if (sym->bind != STB_LOCAL) + if (table_sym->bind != STB_LOCAL) continue; - if (sym->type != STT_FUNC && sym->type != STT_OBJECT) + if (table_sym->type != STT_FUNC && table_sym->type != STT_OBJECT) continue; found = 0; - for (child = child_locals; child->name; child++) { - if (child->type == sym->type && - !strcmp(child->name, sym->name)) { + sym = file_sym; + list_for_each_entry_continue(sym, sym_list, list) { + if (sym->type == STT_FILE) + break; + if (sym->bind != STB_LOCAL) + continue; + + if (sym->type == table_sym->type && + !strcmp(sym->name, table_sym->name)) { found = 1; break; } @@ -120,27 +125,33 @@ static int locals_match(struct lookup_table *table, int idx, return 0; } - for (child = child_locals; child->name; child++) { + sym = file_sym; + list_for_each_entry_continue(sym, sym_list, list) { + if (sym->type == STT_FILE) + break; + if (sym->bind != STB_LOCAL) + continue; + if (sym->type != STT_FUNC && table_sym->type != STT_OBJECT) + continue; /* * Symbols which get discarded at link time are missing from * the lookup table, so skip them. */ - if (maybe_discarded_sym(child->name)) + if (maybe_discarded_sym(sym->name)) continue; found = 0; i = idx + 1; - for_each_obj_symbol_continue(i, sym, table) { - if (sym->type == STT_FILE) + for_each_obj_symbol_continue(i, table_sym, table) { + if (table_sym->type == STT_FILE) break; - if (sym->bind != STB_LOCAL) - continue; - if (sym->type != STT_FUNC && sym->type != STT_OBJECT) + if (table_sym->bind != STB_LOCAL) continue; - if (maybe_discarded_sym(sym->name)) + if (maybe_discarded_sym(table_sym->name)) continue; - if (!strcmp(child->name, sym->name)) { + if (sym->type == table_sym->type && + !strcmp(sym->name, table_sym->name)) { found = 1; break; } @@ -153,32 +164,56 @@ static int locals_match(struct lookup_table *table, int idx, return 1; } -static void find_local_syms(struct lookup_table *table, char *hint, - struct sym_compare_type *child_locals) +static void find_local_syms(struct lookup_table *table, struct symbol *file_sym, + struct list_head *sym_list) { struct object_symbol *sym; + struct object_symbol *lookup_table_file_sym = NULL; int i; - if (!child_locals) - return; - for_each_obj_symbol(i, sym, table) { if (sym->type != STT_FILE) continue; - if (strcmp(hint, sym->name)) + if (strcmp(file_sym->name, sym->name)) continue; - if (!locals_match(table, i, child_locals)) + if (!locals_match(table, i, file_sym, sym_list)) continue; - if (table->local_syms) + if (lookup_table_file_sym) ERROR("found duplicate matches for %s local symbols in %s symbol table", - hint, table->objname); + file_sym->name, table->objname); - table->local_syms = sym; + lookup_table_file_sym = sym; } - if (!table->local_syms) + if (!lookup_table_file_sym) ERROR("couldn't find matching %s local symbols in %s symbol table", - hint, table->objname); + file_sym->name, table->objname); + + list_for_each_entry_continue(file_sym, sym_list, list) { + if (file_sym->type == STT_FILE) + break; + file_sym->lookup_table_file_sym = lookup_table_file_sym; + } +} + +/* + * Because there can be duplicate symbols and duplicate filenames we need to + * correlate each symbol from the elf file to it's corresponding symbol in + * lookup table. Both the elf file and the lookup table can be split on + * STT_FILE symbols into blocks of symbols originating from a single source + * file. We then compare local symbol lists from both blocks and store the + * pointer to STT_FILE symbol in lookup table for later use in + * lookup_local_symbol(). + */ +static void find_local_syms_multiple(struct lookup_table *table, + struct kpatch_elf *kelf) +{ + struct symbol *sym; + + list_for_each_entry(sym, &kelf->symbols, list) { + if (sym->type == STT_FILE) + find_local_syms(table, sym, &kelf->symbols); + } } /* Strip the path and replace '-' with '_' */ @@ -371,8 +406,7 @@ static void symvers_read(struct lookup_table *table, char *path) } struct lookup_table *lookup_open(char *symtab_path, char *objname, - char *symvers_path, char *hint, - struct sym_compare_type *locals) + char *symvers_path, struct kpatch_elf *kelf) { struct lookup_table *table; @@ -384,7 +418,8 @@ struct lookup_table *lookup_open(char *symtab_path, char *objname, table->objname = objname; symtab_read(table, symtab_path); symvers_read(table, symvers_path); - find_local_syms(table, hint, locals); + + find_local_syms_multiple(table, kelf); return table; } @@ -415,16 +450,13 @@ static bool lookup_local_symbol(struct lookup_table *table, unsigned long sympos = 0; int i, in_file = 0; - if (!table->local_syms) - return false; - memset(result, 0, sizeof(*result)); for_each_obj_symbol(i, sym, table) { if (sym->bind == STB_LOCAL && !strcmp(sym->name, lookup_sym->name)) sympos++; - if (table->local_syms == sym) { + if (lookup_sym->lookup_table_file_sym == sym) { in_file = 1; continue; } @@ -437,7 +469,6 @@ static bool lookup_local_symbol(struct lookup_table *table, if (sym->bind == STB_LOCAL && !strcmp(sym->name, lookup_sym->name)) { - if (result->objname) ERROR("duplicate local symbol found for %s", lookup_sym->name); diff --git a/kpatch-build/lookup.h b/kpatch-build/lookup.h index ae92880e4..e1277f15a 100644 --- a/kpatch-build/lookup.h +++ b/kpatch-build/lookup.h @@ -14,14 +14,8 @@ struct lookup_result { bool global, exported; }; -struct sym_compare_type { - char *name; - int type; -}; - struct lookup_table *lookup_open(char *symtab_path, char *objname, - char *symvers_path, char *hint, - struct sym_compare_type *locals); + char *symvers_path, struct kpatch_elf *kelf); void lookup_close(struct lookup_table *table); bool lookup_symbol(struct lookup_table *table, struct symbol *sym, struct lookup_result *result); From e2b50d7b6685a46376e8c1e0a60144cadd0c2b88 Mon Sep 17 00:00:00 2001 From: Artem Savkov Date: Wed, 11 Aug 2021 10:33:44 +0200 Subject: [PATCH 4/7] create-diff-object: make locals_match() and maybe_discarded_sym() return bool Change the return type of locals_match() and maybe_discarded_sym() to better reflect their usage. Signed-off-by: Artem Savkov --- kpatch-build/lookup.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/kpatch-build/lookup.c b/kpatch-build/lookup.c index 49e15614c..0323c6142 100644 --- a/kpatch-build/lookup.c +++ b/kpatch-build/lookup.c @@ -68,10 +68,10 @@ struct lookup_table { #define for_each_exp_symbol(ndx, iter, table) \ for (ndx = 0, iter = table->exp_syms; ndx < table->exp_nr; ndx++, iter++) -static int maybe_discarded_sym(const char *name) +static bool maybe_discarded_sym(const char *name) { if (!name) - return 0; + return false; /* * Sometimes these symbols are discarded during linking, and sometimes @@ -85,12 +85,12 @@ static int maybe_discarded_sym(const char *name) strstr(name, "__addressable_") || strstr(name, "__UNIQUE_ID_") || !strncmp(name, ".L.str", 6)) - return 1; + return true; - return 0; + return false; } -static int locals_match(struct lookup_table *table, int idx, +static bool locals_match(struct lookup_table *table, int idx, struct symbol *file_sym, struct list_head *sym_list) { struct symbol *sym; @@ -122,7 +122,7 @@ static int locals_match(struct lookup_table *table, int idx, } if (!found) - return 0; + return false; } sym = file_sym; @@ -158,10 +158,10 @@ static int locals_match(struct lookup_table *table, int idx, } if (!found) - return 0; + return false; } - return 1; + return true; } static void find_local_syms(struct lookup_table *table, struct symbol *file_sym, From 22e16619e0a7f03100fe31f27dd2da0648e4fdd4 Mon Sep 17 00:00:00 2001 From: Artem Savkov Date: Mon, 16 Aug 2021 10:51:24 +0200 Subject: [PATCH 5/7] create-diff-object: base->orig renames Rename "base" to "orig" when referencing object files and their contents to be consistent with temporary directory structure. Signed-off-by: Artem Savkov --- kpatch-build/create-diff-object.c | 44 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index 96a92c0a6..a7e13f641 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -24,7 +24,7 @@ * This file contains the heart of the ELF object differencing engine. * * The tool takes two ELF objects from two versions of the same source - * file; a "base" object and a "patched" object. These object need to have + * file; a "orig" object and a "patched" object. These object need to have * been compiled with the -ffunction-sections and -fdata-sections GCC options. * * The tool compares the objects at a section level to determine what @@ -1255,14 +1255,14 @@ static struct rela *kpatch_find_static_twin_ref(struct section *rela_sec, struct * with the same name to be used in the same function if they * have different scopes. (We have to assume that in such * cases, the order in which they're referenced remains the - * same between the base and patched objects, as there's no + * same between the orig and patched objects, as there's no * other way to distinguish them.) * * - Static locals are usually referenced by functions, but * they can occasionally be referenced by data sections as * well. */ -static void kpatch_correlate_static_local_variables(struct kpatch_elf *base, +static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, struct kpatch_elf *patched) { struct symbol *sym, *patched_sym; @@ -1272,10 +1272,10 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *base, /* * First undo the correlations for all static locals. Two static - * locals can have the same numbered suffix in the base and patched + * locals can have the same numbered suffix in the orig and patched * objects by coincidence. */ - list_for_each_entry(sym, &base->symbols, list) { + list_for_each_entry(sym, &orig->symbols, list) { if (!kpatch_is_normal_static_local(sym)) continue; @@ -1299,7 +1299,7 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *base, * Do the correlations: for each section reference to a static local, * look for a corresponding reference in the section's twin. */ - list_for_each_entry(sec, &base->sections, list) { + list_for_each_entry(sec, &orig->sections, list) { if (!is_rela_section(sec) || is_debug_section(sec) || @@ -1355,14 +1355,14 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *base, /* * Make sure that: * - * 1. all the base object's referenced static locals have been + * 1. all the orig object's referenced static locals have been * correlated; and * - * 2. each reference to a static local in the base object has a + * 2. each reference to a static local in the orig object has a * corresponding reference in the patched object (because a static * local can be referenced by more than one section). */ - list_for_each_entry(sec, &base->sections, list) { + list_for_each_entry(sec, &orig->sections, list) { if (!is_rela_section(sec) || is_debug_section(sec)) @@ -3686,7 +3686,7 @@ static struct argp argp = { options, parse_opt, args_doc, NULL }; int main(int argc, char *argv[]) { - struct kpatch_elf *kelf_base, *kelf_patched, *kelf_out; + struct kpatch_elf *kelf_orig, *kelf_patched, *kelf_out; struct arguments arguments; int num_changed, callbacks_exist, new_globals_exist; struct lookup_table *lookup; @@ -3713,30 +3713,30 @@ int main(int argc, char *argv[]) childobj = basename(orig_obj); - kelf_base = kpatch_elf_open(orig_obj); + kelf_orig = kpatch_elf_open(orig_obj); kelf_patched = kpatch_elf_open(patched_obj); - kpatch_compare_elf_headers(kelf_base->elf, kelf_patched->elf); - kpatch_check_program_headers(kelf_base->elf); + kpatch_compare_elf_headers(kelf_orig->elf, kelf_patched->elf); + kpatch_check_program_headers(kelf_orig->elf); kpatch_check_program_headers(kelf_patched->elf); - kpatch_bundle_symbols(kelf_base); + kpatch_bundle_symbols(kelf_orig); kpatch_bundle_symbols(kelf_patched); - kpatch_detect_child_functions(kelf_base); + kpatch_detect_child_functions(kelf_orig); kpatch_detect_child_functions(kelf_patched); - lookup = lookup_open(parent_symtab, parent_name, mod_symvers, kelf_base); + lookup = lookup_open(parent_symtab, parent_name, mod_symvers, kelf_orig); kpatch_mark_grouped_sections(kelf_patched); - kpatch_replace_sections_syms(kelf_base); + kpatch_replace_sections_syms(kelf_orig); kpatch_replace_sections_syms(kelf_patched); - kpatch_correlate_elfs(kelf_base, kelf_patched); - kpatch_correlate_static_local_variables(kelf_base, kelf_patched); + kpatch_correlate_elfs(kelf_orig, kelf_patched); + kpatch_correlate_static_local_variables(kelf_orig, kelf_patched); /* - * After this point, we don't care about kelf_base anymore. + * After this point, we don't care about kelf_orig anymore. * We access its sections via the twin pointers in the * section, symbol, and rela lists of kelf_patched. */ @@ -3745,8 +3745,8 @@ int main(int argc, char *argv[]) kpatch_mark_ignored_functions_same(kelf_patched); kpatch_mark_ignored_sections_same(kelf_patched); kpatch_check_func_profiling_calls(kelf_patched); - kpatch_elf_teardown(kelf_base); - kpatch_elf_free(kelf_base); + kpatch_elf_teardown(kelf_orig); + kpatch_elf_free(kelf_orig); kpatch_include_standard_elements(kelf_patched); num_changed = kpatch_include_changed_functions(kelf_patched); From 99542e864ebf9010537f4c62e2bb0b2b14fcdec8 Mon Sep 17 00:00:00 2001 From: Artem Savkov Date: Wed, 11 Aug 2021 09:58:46 +0200 Subject: [PATCH 6/7] create-diff-object: rename arguments in most correlate/compare functions While theoretically most of these functions can work both ways right now all calls follow the same pattern: first argument is orig element and second is patched element. Rename the arguments so that these functions are used in the same fashion going forward. This allows us to cut some corners such as removing the elseif statement in kpatch_correlate_symbol(). Signed-off-by: Artem Savkov --- kpatch-build/create-diff-object.c | 141 ++++++++++++++++-------------- 1 file changed, 73 insertions(+), 68 deletions(-) diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c index a7e13f641..046b8f293 100644 --- a/kpatch-build/create-diff-object.c +++ b/kpatch-build/create-diff-object.c @@ -918,91 +918,95 @@ do { \ elem->twin = NULL; \ } while (0) -static void __kpatch_correlate_section(struct section *sec1, struct section *sec2) +static void __kpatch_correlate_section(struct section *sec_orig, + struct section *sec_patched) { - CORRELATE_ELEMENT(sec1, sec2, "section"); + CORRELATE_ELEMENT(sec_orig, sec_patched, "section"); } -static void kpatch_correlate_symbol(struct symbol *sym1, struct symbol *sym2) +static void kpatch_correlate_symbol(struct symbol *sym_orig, + struct symbol *sym_patched) { - CORRELATE_ELEMENT(sym1, sym2, "symbol"); - if (sym1->lookup_table_file_sym && !sym2->lookup_table_file_sym) - sym2->lookup_table_file_sym = sym1->lookup_table_file_sym; - else if (!sym1->lookup_table_file_sym && sym2->lookup_table_file_sym) - sym1->lookup_table_file_sym = sym2->lookup_table_file_sym; + CORRELATE_ELEMENT(sym_orig, sym_patched, "symbol"); + if (sym_orig->lookup_table_file_sym && !sym_patched->lookup_table_file_sym) + sym_patched->lookup_table_file_sym = sym_orig->lookup_table_file_sym; } -static void kpatch_correlate_static_local(struct symbol *sym1, struct symbol *sym2) +static void kpatch_correlate_static_local(struct symbol *sym_orig, + struct symbol *sym_patched) { - CORRELATE_ELEMENT(sym1, sym2, "static local"); + CORRELATE_ELEMENT(sym_orig, sym_patched, "static local"); } -static void kpatch_correlate_section(struct section *sec1, struct section *sec2) +static void kpatch_correlate_section(struct section *sec_orig, + struct section *sec_patched) { - __kpatch_correlate_section(sec1, sec2); + __kpatch_correlate_section(sec_orig, sec_patched); - if (is_rela_section(sec1)) { - __kpatch_correlate_section(sec1->base, sec2->base); - sec1 = sec1->base; - sec2 = sec2->base; - } else if (sec1->rela) { - __kpatch_correlate_section(sec1->rela, sec2->rela); + if (is_rela_section(sec_orig)) { + __kpatch_correlate_section(sec_orig->base, sec_patched->base); + sec_orig = sec_orig->base; + sec_patched = sec_patched->base; + } else if (sec_orig->rela) { + __kpatch_correlate_section(sec_orig->rela, sec_patched->rela); } - if (sec1->secsym) - kpatch_correlate_symbol(sec1->secsym, sec2->secsym); - if (sec1->sym) - kpatch_correlate_symbol(sec1->sym, sec2->sym); + if (sec_orig->secsym) + kpatch_correlate_symbol(sec_orig->secsym, sec_patched->secsym); + if (sec_orig->sym) + kpatch_correlate_symbol(sec_orig->sym, sec_patched->sym); } -static void kpatch_correlate_sections(struct list_head *seclist1, struct list_head *seclist2) +static void kpatch_correlate_sections(struct list_head *seclist_orig, + struct list_head *seclist_patched) { - struct section *sec1, *sec2; + struct section *sec_orig, *sec_patched; - list_for_each_entry(sec1, seclist1, list) { - if (sec1->twin) + list_for_each_entry(sec_orig, seclist_orig, list) { + if (sec_orig->twin) continue; - list_for_each_entry(sec2, seclist2, list) { - if (kpatch_mangled_strcmp(sec1->name, sec2->name) || - sec2->twin) + list_for_each_entry(sec_patched, seclist_patched, list) { + if (kpatch_mangled_strcmp(sec_orig->name, sec_patched->name) || + sec_patched->twin) continue; - if (is_special_static(is_rela_section(sec1) ? - sec1->base->secsym : - sec1->secsym)) + if (is_special_static(is_rela_section(sec_orig) ? + sec_orig->base->secsym : + sec_orig->secsym)) continue; /* * Group sections must match exactly to be correlated. * Changed group sections are currently not supported. */ - if (sec1->sh.sh_type == SHT_GROUP) { - if (sec1->data->d_size != sec2->data->d_size) + if (sec_orig->sh.sh_type == SHT_GROUP) { + if (sec_orig->data->d_size != sec_patched->data->d_size) continue; - if (memcmp(sec1->data->d_buf, sec2->data->d_buf, - sec1->data->d_size)) + if (memcmp(sec_orig->data->d_buf, sec_patched->data->d_buf, + sec_orig->data->d_size)) continue; } - kpatch_correlate_section(sec1, sec2); + kpatch_correlate_section(sec_orig, sec_patched); break; } } } -static void kpatch_correlate_symbols(struct list_head *symlist1, struct list_head *symlist2) +static void kpatch_correlate_symbols(struct list_head *symlist_orig, + struct list_head *symlist_patched) { - struct symbol *sym1, *sym2; + struct symbol *sym_orig, *sym_patched; - list_for_each_entry(sym1, symlist1, list) { - if (sym1->twin) + list_for_each_entry(sym_orig, symlist_orig, list) { + if (sym_orig->twin) continue; - list_for_each_entry(sym2, symlist2, list) { - if (kpatch_mangled_strcmp(sym1->name, sym2->name) || - sym1->type != sym2->type || sym2->twin) + list_for_each_entry(sym_patched, symlist_patched, list) { + if (kpatch_mangled_strcmp(sym_orig->name, sym_patched->name) || + sym_orig->type != sym_patched->type || sym_patched->twin) continue; - if (is_special_static(sym1)) + if (is_special_static(sym_orig)) continue; /* @@ -1019,42 +1023,42 @@ static void kpatch_correlate_symbols(struct list_head *symlist1, struct list_hea * sure if this actually happens anywhere), any string * changes will be detected elsewhere in rela_equal(). */ - if (sym1->type == STT_NOTYPE && - !strncmp(sym1->name, ".LC", 3)) + if (sym_orig->type == STT_NOTYPE && + !strncmp(sym_orig->name, ".LC", 3)) continue; /* group section symbols must have correlated sections */ - if (sym1->sec && - sym1->sec->sh.sh_type == SHT_GROUP && - sym1->sec->twin != sym2->sec) + if (sym_orig->sec && + sym_orig->sec->sh.sh_type == SHT_GROUP && + sym_orig->sec->twin != sym_patched->sec) continue; - kpatch_correlate_symbol(sym1, sym2); + kpatch_correlate_symbol(sym_orig, sym_patched); break; } } } -static void kpatch_compare_elf_headers(Elf *elf1, Elf *elf2) +static void kpatch_compare_elf_headers(Elf *elf_orig, Elf *elf_patched) { - GElf_Ehdr eh1, eh2; + GElf_Ehdr ehdr_orig, ehdr_patched; - if (!gelf_getehdr(elf1, &eh1)) + if (!gelf_getehdr(elf_orig, &ehdr_orig)) ERROR("gelf_getehdr"); - if (!gelf_getehdr(elf2, &eh2)) + if (!gelf_getehdr(elf_patched, &ehdr_patched)) ERROR("gelf_getehdr"); - if (memcmp(eh1.e_ident, eh2.e_ident, EI_NIDENT) || - eh1.e_type != eh2.e_type || - eh1.e_machine != eh2.e_machine || - eh1.e_version != eh2.e_version || - eh1.e_entry != eh2.e_entry || - eh1.e_phoff != eh2.e_phoff || - eh1.e_flags != eh2.e_flags || - eh1.e_ehsize != eh2.e_ehsize || - eh1.e_phentsize != eh2.e_phentsize || - eh1.e_shentsize != eh2.e_shentsize) + if (memcmp(ehdr_orig.e_ident, ehdr_patched.e_ident, EI_NIDENT) || + ehdr_orig.e_type != ehdr_patched.e_type || + ehdr_orig.e_machine != ehdr_patched.e_machine || + ehdr_orig.e_version != ehdr_patched.e_version || + ehdr_orig.e_entry != ehdr_patched.e_entry || + ehdr_orig.e_phoff != ehdr_patched.e_phoff || + ehdr_orig.e_flags != ehdr_patched.e_flags || + ehdr_orig.e_ehsize != ehdr_patched.e_ehsize || + ehdr_orig.e_phentsize != ehdr_patched.e_phentsize || + ehdr_orig.e_shentsize != ehdr_patched.e_shentsize) DIFF_FATAL("ELF headers differ"); } @@ -1423,10 +1427,11 @@ static void kpatch_correlate_static_local_variables(struct kpatch_elf *orig, } } -static void kpatch_correlate_elfs(struct kpatch_elf *kelf1, struct kpatch_elf *kelf2) +static void kpatch_correlate_elfs(struct kpatch_elf *kelf_orig, + struct kpatch_elf *kelf_patched) { - kpatch_correlate_sections(&kelf1->sections, &kelf2->sections); - kpatch_correlate_symbols(&kelf1->symbols, &kelf2->symbols); + kpatch_correlate_sections(&kelf_orig->sections, &kelf_patched->sections); + kpatch_correlate_symbols(&kelf_orig->symbols, &kelf_patched->symbols); } static void kpatch_compare_correlated_elements(struct kpatch_elf *kelf) From 941a96742ddb8abba79f5c9417c3b0b216e66e80 Mon Sep 17 00:00:00 2001 From: Artem Savkov Date: Tue, 17 Aug 2021 09:38:30 +0200 Subject: [PATCH 7/7] Include unit-tests for multifile case Update unit-test submodule pointer to include a multi-file unit-test. Signed-off-by: Artem Savkov --- test/unit/objs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/objs b/test/unit/objs index 32ab24dc5..0d75b8137 160000 --- a/test/unit/objs +++ b/test/unit/objs @@ -1 +1 @@ -Subproject commit 32ab24dc590c955f50f794afc0ccc2d8d9a92aa2 +Subproject commit 0d75b8137a08d62d89acf3358df8f44b1b86ff7a