Backport static variable correlation work from upstream

Backport 7c88c41cfe3d ("create-diff-object: rewrite static local
variable correlation logic") from kpatch.

diff --git a/create-diff-object.c b/create-diff-object.c
index a7ef04d4c0eb7f0dc55406db50e093bac8fb1b8e..686834fb54c15efbafc273690bfd606f9dd471ae 100644 (file)
--- a/create-diff-object.c
+++ b/create-diff-object.c
@@ -434,138 +434,220 @@ static char *kpatch_section_function_name(struct section *sec)
 }
 
 /*
- * Given a static local variable symbol and a section which references it in
- * the patched object, find a corresponding usage of a similarly named symbol
- * in the base object.
+ * Given a static local variable symbol and a rela section which references it
+ * in the base object, find a corresponding usage of a similarly named symbol
+ * in the patched object.
  */
 static struct symbol *kpatch_find_static_twin(struct section *sec,
                                              struct symbol *sym)
 {
        struct rela *rela;
-       struct symbol *basesym;
 
        if (!sec->twin)
                return NULL;
 
-       /*
-        * Ensure there are no other orphaned static variables with the
-        * same name in the function.  This is possible if the
-        * variables are in different scopes or if one of them is part of an
-        * inlined function.
-        */
-       list_for_each_entry(rela, &sec->relas, list) {
-               if (rela->sym == sym || rela->sym->twin)
-                       continue;
-               if (!kpatch_mangled_strcmp(rela->sym->name, sym->name))
-                       ERROR("found another static local variable matching %s in patched %s",
-                             sym->name, kpatch_section_function_name(sec));
-       }
-
-       /* find the base object's corresponding variable */
-       basesym = NULL;
+       /* find the patched object's corresponding variable */
        list_for_each_entry(rela, &sec->twin->relas, list) {
+
                if (rela->sym->twin)
                        continue;
+
                if (kpatch_mangled_strcmp(rela->sym->name, sym->name))
                        continue;
-               if (basesym && basesym != rela->sym)
-                       ERROR("found two static local variables matching %s in orig %s",
-                             sym->name, kpatch_section_function_name(sec));
 
-               basesym = rela->sym;
+               return rela->sym;
        }
 
-       return basesym;
+       return NULL;
 }
 
 /*
  * gcc renames static local variables by appending a period and a number.  For
  * example, __foo could be renamed to __foo.31452.  Unfortunately this number
- * can arbitrarily change.  Try to rename the patched version of the symbol to
- * match the base version and then correlate them.
+ * can arbitrarily change.  Correlate them by comparing which functions
+ * reference them, and rename the patched symbols to match the base symbol
+ * names.
  */
-static void kpatch_correlate_static_local_variables(struct kpatch_elf *base,
-                                                   struct kpatch_elf *patched)
+void kpatch_correlate_static_local_variables(struct kpatch_elf *base,
+                                            struct kpatch_elf *patched)
 {
-       struct symbol *sym, *basesym, *tmpsym;
-       struct section *tmpsec, *sec;
+       struct symbol *sym, *patched_sym, *tmpsym;
+       struct section *sec;
        struct rela *rela;
-       int bundled, basebundled;
+       int bundled, patched_bundled;
 
-       list_for_each_entry(sym, &patched->symbols, list) {
-               if (sym->type != STT_OBJECT || sym->bind != STB_LOCAL ||
-                   sym->twin)
-                       continue;
+       list_for_each_entry(sym, &base->symbols, list) {
 
-               if (is_special_static(sym))
+               if (sym->type != STT_OBJECT || sym->bind != STB_LOCAL)
                        continue;
 
                if (!strchr(sym->name, '.'))
                        continue;
 
+               if (is_special_static(sym))
+                       continue;
+
                /*
-                * For each function which uses the variable in the patched
-                * object, look for a corresponding use in the function's twin
-                * in the base object.
+                * Undo any previous correlation.  Two static locals can have
+                * the same numbered suffix in the base and patched objects by
+                * coincidence.
+                */
+               if (sym->twin) {
+                       sym->twin->twin = NULL;
+                       sym->twin = NULL;
+               }
+               bundled = sym == sym->sec->sym;
+               if (bundled && sym->sec->twin) {
+                       sym->sec->twin->twin = NULL;
+                       sym->sec->twin = NULL;
+               }
+
+               /*
+                *
+                * Some surprising facts about static local variable symbols:
+                *
+                * - It's possible for multiple functions to use the same
+                *   static local variable if the variable is defined in an
+                *   inlined function.
+                *
+                * - It's also possible for multiple static local variables
+                *   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
+                *   other way to distinguish them.)
                 *
-                * It's possible for multiple functions to use the same static
-                * local variable if the variable is defined in an inlined
-                * function.
+                * - Static locals are usually referenced by functions, but
+                *   they can occasionally be referenced by data sections as
+                *   well.
+                *
+                * For each section which references the variable in the base
+                * object, look for a corresponding reference in the section's
+                * twin in the patched object.
                 */
-               sec = NULL;
-               basesym = NULL;
-               list_for_each_entry(tmpsec, &patched->sections, list) {
-                       if (!is_rela_section(tmpsec) ||
-                           !is_text_section(tmpsec->base) ||
-                           is_debug_section(tmpsec))
+               patched_sym = NULL;
+               list_for_each_entry(sec, &base->sections, list) {
+
+                       if (!is_rela_section(sec) ||
+                           is_debug_section(sec))
                                continue;
-                       list_for_each_entry(rela, &tmpsec->relas, list) {
+
+                       list_for_each_entry(rela, &sec->relas, list) {
+
                                if (rela->sym != sym)
                                        continue;
 
-                               tmpsym = kpatch_find_static_twin(tmpsec, sym);
-                               if (basesym && tmpsym && basesym != tmpsym)
-                                       ERROR("found two twins for static local variable %s: %s and %s",
-                                             sym->name, basesym->name,
-                                             tmpsym->name);
-                               if (tmpsym && !basesym)
-                                       basesym = tmpsym;
+                               if (bundled && sym->sec == sec->base) {
+                                       /*
+                                        * A rare case where a static local
+                                        * data structure references itself.
+                                        * There's no reliable way to correlate
+                                        * this.  Hopefully there's another
+                                        * reference to the symbol somewhere
+                                        * that can be used.
+                                        */
+                                       log_debug("can't correlate static local %s's reference to itself\n",
+                                                 sym->name);
+                                       break;
+                               }
+
+                               tmpsym = kpatch_find_static_twin(sec, sym);
+                               if (!tmpsym)
+                                       DIFF_FATAL("reference to static local variable %s in %s was removed",
+                                                  sym->name,
+                                                  kpatch_section_function_name(sec));
+
+                               if (patched_sym && patched_sym != tmpsym)
+                                       DIFF_FATAL("found two twins for static local variable %s: %s and %s",
+                                                  sym->name, patched_sym->name,
+                                                  tmpsym->name);
+
+                               patched_sym = tmpsym;
 
-                               sec = tmpsec;
                                break;
                        }
                }
 
-               if (!sec)
-                       ERROR("static local variable %s not used", sym->name);
-
-               if (!basesym) {
-                       log_normal("WARNING: unable to correlate static local variable %s used by %s, assuming variable is new\n",
-                                  sym->name,
-                                  kpatch_section_function_name(sec));
+               /*
+                * Check if the symbol is unused.  This is possible if multiple
+                * static locals in the file refer to the same read-only data,
+                * and their symbols point to the same bundled section.  In
+                * such a case we can ignore the extra symbol.
+                */
+               if (!patched_sym) {
+                       log_debug("ignoring base unused static local %s\n",
+                                 sym->name);
                        continue;
                }
 
-
-               bundled = sym == sym->sec->sym;
-               basebundled = basesym == basesym->sec->sym;
-               if (bundled != basebundled)
+               patched_bundled = patched_sym == patched_sym->sec->sym;
+               if (bundled != patched_bundled)
                        ERROR("bundle mismatch for symbol %s", sym->name);
-               if (!bundled && sym->sec->twin != basesym->sec)
+               if (!bundled && sym->sec->twin != patched_sym->sec)
                        ERROR("sections %s and %s aren't correlated",
-                             sym->sec->name, basesym->sec->name);
+                             sym->sec->name, patched_sym->sec->name);
 
-               log_debug("renaming and correlating %s to %s\n",
-                         sym->name, basesym->name);
-               sym->name = strdup(basesym->name);
-               sym->twin = basesym;
-               basesym->twin = sym;
-               sym->status = basesym->status = SAME;
+               log_debug("renaming and correlating static local %s to %s\n",
+                         patched_sym->name, sym->name);
+
+               patched_sym->name = strdup(sym->name);
+               sym->twin = patched_sym;
+               patched_sym->twin = sym;
 
                if (bundled) {
-                       sym->sec->twin = basesym->sec;
-                       basesym->sec->twin = sym->sec;
+                       sym->sec->twin = patched_sym->sec;
+                       patched_sym->sec->twin = sym->sec;
+
+                       if (sym->sec->rela && patched_sym->sec->rela) {
+                               sym->sec->rela->twin = patched_sym->sec->rela;
+                               patched_sym->sec->rela->twin = sym->sec->rela;
+                       }
+               }
+       }
+
+       /*
+        * All the base object's static local variables have been correlated.
+        * Now go through the patched object and look for any uncorrelated
+        * static locals to see if we need to print some warnings.
+        */
+       list_for_each_entry(sym, &patched->symbols, list) {
+
+               if (sym->type != STT_OBJECT || sym->bind != STB_LOCAL ||
+                   sym->twin)
+                       continue;
+
+               if (!strchr(sym->name, '.'))
+                       continue;
+
+               if (is_special_static(sym))
+                       continue;
+
+               list_for_each_entry(sec, &patched->sections, list) {
+
+                       if (!is_rela_section(sec) ||
+                           is_debug_section(sec))
+                               continue;
+
+                       list_for_each_entry(rela, &sec->relas, list) {
+
+                               if (rela->sym != sym)
+                                       continue;
+
+                               log_normal("WARNING: unable to correlate static local variable %s used by %s, assuming variable is new\n",
+                                          sym->name,
+                                          kpatch_section_function_name(sec));
+                               return;
+                       }
                }
+
+               /*
+                * This symbol is unused by any functions.  This is possible if
+                * multiple static locals in the file refer to the same
+                * read-only data, and their symbols point to the same bundled
+                * section.  In such a case we can ignore the extra symbol.
+                */
+               log_debug("ignoring patched unused static local %s\n",
+                         sym->name);
        }
 }