RepoMirrors/kpatch
1
0
Fork 0
mirror of https://github.com/dynup/kpatch synced 2025-02-19 19:26:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

create-diff-object: more static local variable rework

Browse Source 
Refine the static local variable handling again.  This builds on a
previous patch by Zhou Chengming.

This fixes the following bugs reported by Zhou:

1.          xxx.123 ---> xxx.123 (previous correlation by coincidence)
            xxx.256 ---> xxx.256 (previous correlation by coincidence)
   But real xxx.123 ---> xxx.256

   In this case, the code doesn't work. Because when find patched_sym for
   xxx.123, the xxx.256 in patched_object hasn't been de-correlated.

2. old-object | new-object
        func1 | func1
      xxx.123 | xxx.123 (inline)
        func2 | func2
      xxx.256 | xxx.256
      xxx.123 | xxx.123 (inline)

   When find patched_sym for xxx.123, first find xxx.123 in func1 of new-object,
   But then find xxx.256 in func2 of new-object.
   So I think should not iterate the base-sections, when find one, just go out to next symbol.

Both of these problems can be fixed by splitting the code up into
multiple passes:

  1. uncorrelate all static locals
  2. correlate all static locals
  3. ensure each static local is referenced by all the same sections in
     both objects
  4. print warning on any new static locals

Fixes: #545
This commit is contained in:
Josh Poimboeuf 2015-11-13 13:56:13 -06:00
parent eb54876936
commit ac9020af20
1 changed files with 148 additions and 135 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

283
kpatch-build/create-diff-object.c
View File

@ -1086,193 +1086,206 @@ static struct symbol *kpatch_find_static_twin(struct section *sec,
return NULL;
}
static int kpatch_is_normal_static_local(struct symbol *sym)
{
if (sym->type != STT_OBJECT || sym->bind != STB_LOCAL)
return 0;
if (!strchr(sym->name, '.'))
return 0;
if (is_special_static(sym))
return 0;
return 1;
}
/*
* 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. Correlate them by comparing which functions
* reference them, and rename the patched symbols to match the base symbol
* names.
*
* 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.)
*
* - Static locals are usually referenced by functions, but
* they can occasionally be referenced by data sections as
* well.
*/
void kpatch_correlate_static_local_variables(struct kpatch_elf *base,
struct kpatch_elf *patched)
{
struct symbol *sym, *patched_sym, *tmpsym;
struct symbol *sym, *patched_sym;
struct section *sec;
struct rela *rela;
int bundled, patched_bundled;
struct rela *rela, *rela2;
int bundled, patched_bundled, found;
/*
* First undo the correlations for all static locals. Two static
* locals can have the same numbered suffix in the base and patched
* objects by coincidence.
*/
list_for_each_entry(sym, &base->symbols, list) {
if (sym->type != STT_OBJECT || sym->bind != STB_LOCAL)
if (!kpatch_is_normal_static_local(sym))
continue;
if (!strchr(sym->name, '.'))
continue;
if (is_special_static(sym))
continue;
/*
* 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.)
*
* - 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.
*/
patched_sym = NULL;
list_for_each_entry(sec, &base->sections, list) {
/*
* 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) {
if (!is_rela_section(sec) ||
is_debug_section(sec))
if (!is_rela_section(sec) ||
is_debug_section(sec))
continue;
list_for_each_entry(rela, &sec->relas, list) {
sym = rela->sym;
if (!kpatch_is_normal_static_local(sym))
continue;
list_for_each_entry(rela, &sec->relas, list) {
if (sym->twin)
continue;
if (rela->sym != sym)
continue;
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;
break;
bundled = sym == sym->sec->sym;
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);
continue;
}
}
/*
* 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;
}
patched_sym = kpatch_find_static_twin(sec, sym);
if (!patched_sym)
DIFF_FATAL("reference to static local variable %s in %s was removed",
sym->name,
kpatch_section_function_name(sec));
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 != patched_sym->sec)
ERROR("sections %s and %s aren't correlated",
sym->sec->name, patched_sym->sec->name);
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 != patched_sym->sec)
ERROR("sections %s and %s aren't correlated",
sym->sec->name, patched_sym->sec->name);
log_debug("renaming and correlating static local %s to %s\n",
patched_sym->name, sym->name);
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;
patched_sym->name = strdup(sym->name);
sym->twin = patched_sym;
patched_sym->twin = sym;
if (bundled) {
sym->sec->twin = patched_sym->sec;
patched_sym->sec->twin = sym->sec;
if (bundled) {
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;
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.
* Make sure that:
*
* 1. all the base object's referenced static locals have been
* correlated; and
*
* 2. each reference to a static local in the base 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(sym, &patched->symbols, list) {
list_for_each_entry(sec, &base->sections, list) {
if (sym->type != STT_OBJECT || sym->bind != STB_LOCAL ||
sym->twin)
if (!is_rela_section(sec) ||
is_debug_section(sec))
continue;
if (!strchr(sym->name, '.'))
continue;
list_for_each_entry(rela, &sec->relas, list) {
if (is_special_static(sym))
continue;
list_for_each_entry(sec, &patched->sections, list) {
if (!is_rela_section(sec) ||
is_debug_section(sec))
sym = rela->sym;
if (!kpatch_is_normal_static_local(sym))
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",
if (!sym->twin || !sec->twin)
DIFF_FATAL("reference to static local variable %s in %s was removed",
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);
found = 0;
list_for_each_entry(rela2, &sec->twin->relas, list) {
if (rela2->sym == sym->twin) {
found = 1;
break;
}
}
if (!found)
DIFF_FATAL("static local %s has been correlated with %s, but patched %s is missing a reference to it",
sym->name, sym->twin->name,
kpatch_section_function_name(sec->twin));
}
}
/*
* Now go through the patched object and look for any uncorrelated
* static locals to see if we need to print any warnings about new
* variables.
*/
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) {
sym = rela->sym;
if (!kpatch_is_normal_static_local(sym))
continue;
if (sym->twin)
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;
}
}
}