Commit Graph

98 Commits

Author SHA1 Message Date
Seth Jennings
207c237e48 fixup review comments
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-05-09 23:42:11 -05:00
Seth Jennings
20c7882d93 remove temporary debug message
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-05-07 11:18:54 -05:00
Seth Jennings
5d36dd1c32 convert from arrays to linked-lists
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-05-07 10:06:41 -05:00
Seth Jennings
16221237dd Revert "create-diff-object: support for __jump_table"
This reverts commit 5852ddb6a2.

The __jump_table section is more complex than the initial analysis
determined.  The __jump_table has three relocs per entry that must
be pulled in together and one of the relocs is to symbols contained
in the __tracepoints section whose rela section references the
__tracepoint_strings section.  So it's more complex and should just
fail rather than appear that it is being handled properly.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-30 14:23:03 -05:00
Josh Poimboeuf
5852ddb6a2 create-diff-object: support for __jump_table
Almost a line-for-line copy/paste of the smp locks function.  The only
differences are the section name, and an offset increment of 8 instead
of 4.

Fixes #157.
2014-04-29 16:21:03 -05:00
Seth Jennings
ad87448c9f Merge pull request #151 from jpoimboe/cdo-error-order
create-diff-object: better reachability error message
2014-04-29 12:26:28 -05:00
Josh Poimboeuf
a397818257 create-diff-object: better reachability error message
If a patch changes a single function which is in a special section that
we don't support, create-diff-object reports "no changed functions were
found".  Give a clearer error message in that case, by checking
reachability errors before unchanged errors and by printing all
reachability errors errors instead of the first one it encounters.

Fixes #150.
2014-04-29 12:24:01 -05:00
Josh Poimboeuf
d07fe1d8b1 Revert "fail on changed init section"
This reverts commit 7b15e23149.

It seems that there was no bug to start with, so apparently the commit
didn't fix anything.

Fixes #103.
2014-04-29 11:08:12 -05:00
Seth Jennings
6ea92f1fc5 add .parainstructions support
Following in the same solution, regenerate [.rela].parainstructions
sections if table entries contain relocations that reference changed
functions (if any).

Fixes #135

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-25 16:00:22 -05:00
Seth Jennings
e444b2cbb8 fix smp_lock support
The initial commit had a bug where the offset field of the
.rela.smp_locks entries was not updated to reflect the correct
offset in the truncated .smp_locks section.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-25 14:07:51 -05:00
Seth Jennings
30757f027f add smp_locks support
This commit uses the same approach as the bug table support,
mangling the .smp_locks and .rela.smp_locks sections so that
they only contain entries for changed functions (if any).

Fixes #107

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-25 10:46:24 -05:00
Seth Jennings
47d4109f7e fixup review comments
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-22 11:01:01 -05:00
Seth Jennings
ab07805166 add info to expected rela sym error
While debugging the code for the bug table logic, I found it useful to
know which rela section and entry the error occurred on.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-21 17:40:38 -05:00
Seth Jennings
7cfcce1ed6 add bug table support
This commit adds a new function to properly handle the bug table.
It works by going through .rela__bug_table, after the changed
function symbols have already been marked, and rewrites the section
including only the relocations pertaining to bug entries for
changed functions.

The __bug_table section itself is not modified resulting in
"blank" bug entries: ones whose IP and filename pointers will
not be relocated and, therefore, will be zero.  While a waste
of space, it simplifies the code not to remove these blank
entries. They do no harm.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-21 17:40:38 -05:00
Seth Jennings
3753f06de4 use d_size instead of sh_size
The section header size is calculated at output time by libelf
and we use it as a read-only value from read files.

With the next patch we are changing the size of the .rela__bug_table
section.  Lets use d_size instead since it is the value that tells
libelf how to calculate sh_size at output time.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-21 17:40:38 -05:00
Seth Jennings
09262d4d67 support .bss.* bundling
Allow bundling of .bss.* sections that are the result of -fdata-sections
so that rela sections referencing data in bss sections by section symbol
can be replaced with the object symbol so it can be linked to the existing
data object in the kernel.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-21 17:40:38 -05:00
Josh Poimboeuf
9b038039dd create-diff-object: removed unused function
kpatch_find_changed_functions isn't called by anybody, so remove it.
2014-04-15 19:17:40 -05:00
Josh Poimboeuf
37a66ee57c create-diff-object: show object name in log messages
For log_normal and DIFF_FATAL messages, prefix them with the object name
to give more context, which is useful for patches which change multiple
objects.  Also, no need to add the function and line number to
DIFF_FATAL messages, as the error strings already give enough
information.

Example messages:

  meminfo.o: changed function: meminfo_proc_show
  cmdline.o: no changed functions were found
2014-04-15 16:49:59 -05:00
Seth Jennings
7b15e23149 fail on changed init section
fixes issue #103

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-08 12:45:55 -05:00
Seth Jennings
6626db1ad8 add debug message when section has changed
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-04-08 12:40:23 -05:00
Seth Jennings
ead06280df check for changed sections that are not included
There are many cases where a section may have
changed due to soure-level change but the inclusion
logic has not selected it for output.  Some of these
cases are real no-go situations like changing data
structures.  Some are just situations that
create-diff-object isn't smart enough to figure out
(yet).

Either way, it should be considered fatal when a
changed section hasn't been selected for output.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-27 17:38:02 -05:00
Seth Jennings
026362fab6 fix conditional in find_section_by_index()
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-27 10:24:28 -05:00
Seth Jennings
634b9cee78 improve find_[symbol|section]_by_index
The indexes are in order when being read from the
table.  Just index directly into the table; a benefit
of using an array for this structure instead of a linked
list.

Removes another hot path during the rela table initialization.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-24 17:56:21 -05:00
Seth Jennings
24894d263c change for_each_symbol to start at 1 (avoid ugh)
remove "ughs" by changing macro to start at symbol
index 1.  new for_each_symbol_zero will start at zero
for rare cases that need it.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-24 17:19:16 -05:00
Seth Jennings
6567762937 refactor rela section scanning
Upon realizing that there is no point in correlating rela entries,
I also realized that tracking the status of rela entries is also
not needed.

Additionally, the rela section correlation path (really misnamed
as it is the rela section _comparison path) is VERY hot.  Particularly
on files like fs/ext4/ext4.o (which create-diff-object currently can't
successfully parse entirely):

Samples: 40K of event 'cycles', Event count (approx.): 36516578362
49.49%  create-diff-obj  create-diff-object  [.] rela_equal
31.85%  create-diff-obj  create-diff-object  [.] kpatch_correlate_relas
16.22%  create-diff-obj  create-diff-object  [.] find_symbol_by_index

The refactor does a few things:
- replaces nested for loops with single for loop when comparing rela entries
- removes status field for rela entires
- compares rela and nonrela sections in the same path
- removes unnecessary setting of status fields as the inclusion tree
  will include them even if the section status isn't set to CHANGED. This is
  even better as unchanged sections won't appear as CHANGED just because
  their partner .text or .rela section is CHANGED.

This drastically reduced runtime for larger objects and cooled the rela
comparison path:

87.64%  create-diff-obj  create-diff-object  [.] find_symbol_by_index
6.98%  create-diff-obj  libc-2.18.so        [.] __GI___strcmp_ssse3
1.33%  create-diff-obj  create-diff-object  [.] find_section_by_index
1.16%  create-diff-obj  create-diff-object  [.] kpatch_correlate_symbols
0.61%  create-diff-obj  create-diff-object  [.] kpatch_create_rela_table
0.52%  create-diff-obj  create-diff-object  [.] kpatch_correlate_sections

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-24 16:29:40 -05:00
Seth Jennings
57a6b12d9d remove rela twin field, never used
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-24 14:43:40 -05:00
Seth Jennings
1813d54d58 bundle with inclusive logic rather than exclusive
I've created a test designed to exercise the ability
of create-diff-object to parse, compare, and return
"no changed functions" for the entire kernel source
tree one file at a time by passing the same file as
both the original and patched file.

Through this process, I realized that excluding every
special case from being bundled it not feasible.  There
are many sections in the kernel that don't honor
-ffunction|data-sections, not just __ksymtab_strings and
.init.text.

Plus, excluding situations is not the best way.  We are
really only looking for sections that _were_ the result
of -f[function|data]-sections for bundling.

To that end, this commit looks to bundle only symbol/section
pairs that should be bundled ensuringthe .text/.data suffix
and the FUNC/OBJECT symbol name match.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-24 14:14:46 -05:00
Seth Jennings
eb915722cd do not proceed with output generation if no changed funcs
If there are no changed functions, print notice and exit.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-24 11:27:29 -05:00
Seth Jennings
8c487ad6a8 print "changed function" notice for all changed functions
A local changed function will not appear with a "changed function"
notice if it is a dependency of another changed function and that
function occurs before it in the symbol table.

Rearrange some logic to print the notice regardless of whether or not
the function symbols has already been selected for inclusion.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-24 10:38:32 -05:00
Seth Jennings
a15c6eca61 ignore init functions during bundling
When linking function bundles (.text/.rela/section sym/func sym)
ignore __init functions as they do not honor -ffunction-sections
and violate the one-to-one func/section assumption of the
function bundling.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-21 15:11:59 -05:00
Seth Jennings
fdf83f21c3 refactor copying symbols to output
Create a new function kpatch_copy_symbols() that copies symbols from
one kelf to another if the "select" function to return true for the
symbol.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-20 16:03:43 -05:00
Seth Jennings
e51fd3d28b order local FUNC syms before SECTION syms
ld drops FUNC syms that appear after the last SECTION
sym in the symbol table.

Make sure we order the FUNC syms before the SECTION syms.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-20 14:04:02 -05:00
Josh Poimboeuf
bfad3b1880 kmod/core: update API and use hash table
My apologies for the size of this commit.  I combined these two features
(updating API and using a hash table) into a single commit because their
implementations are tightly coupled and I didn't want to have to add
support for the old kpatch_funcs array with the new API just for the
sake of splitting up the commit :-)

- Update the core module API to get a more clear separation between core
  module and patch module.  This is cleaner and will help our case for
  getting the core module merged upstream into the kernel.
- Convert the old kpatch_funcs array into a hash table.  This is so much
  nicer performance-wise and everything-else-wise than that ugly old
  array.
- Do the incremental patching in stop machine.  This ensures that the
  funcs hash is up to date and we don't miss anything.
- Disable preemption in the ftrace handler when accessing the func hash.
  That way we don't get conflicts with the stop_machine handler updating
  the hash.
2014-03-18 13:34:15 -05:00
Josh Poimboeuf
72af45502f make the changed function output more readable
This makes it much easier to spot which functions have changed,
especially when there are more than one of them.
2014-03-13 12:22:01 -05:00
Seth Jennings
e8fb5b1c9f fixup comment
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-10 15:11:23 -05:00
Seth Jennings
8b9de30581 add error for sym value != 0
For a local object or function symbol, we expect that
the section offset, sym.st_value, be 0 because we used
-ffunction-sections and -fdata-section during compile.

If value != 0, it undermines assumptions we make and
should return an error.  Exceptions should be handled
on a case by case basis, like __ksymtab_strings.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-10 15:11:23 -05:00
Seth Jennings
582f2b27e0 add comment for __ksymtab_strings case
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-10 14:49:56 -05:00
Seth Jennings
8eafea114a add inventory file creation to create-object-diff
In preparation for adding an automated test framework,
add an ability to create-object-diff that will create
a human readable list of included sections and symbols
with type and bind information so that the test framework
can compare against a known-good reference list with the
expected set of sections and symbols.

The file is created when the -i/--inventory option is
used.  The inventory filename is the user supplied output
file name suffixed by .inventory

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-07 17:02:45 -06:00
Seth Jennings
6550dd74f6 replace DEBUG compiler define with -d/--debug runtime option
I'm tired of setting CFLAGS and people shouldn't have to
recompile to get debug output.  This lays the foundations
proper option handling and logging levels.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-07 16:06:32 -06:00
Seth Jennings
159214a4fc fixup printf formats with -DDEBUG
Fix -Wformat warnings for printf formats with -DDEBUG

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-07 12:07:56 -06:00
Josh Poimboeuf
693426b5ef enable gcc warnings and fix all warnings 2014-03-05 09:46:10 -06:00
Josh Poimboeuf
81cc4a3542 Merge pull request #37 from spartacus06/fixes
refactor and fix multiple issues
2014-03-05 09:42:22 -06:00
Josh Poimboeuf
330a08dd0d add GPLv2 headers to source files 2014-03-04 21:34:19 -06:00
Seth Jennings
f6efd53541 refactor and fix multiple issues
- Fixup debug messages
- Remove dead code
- No more DEPENDENCY state
- Reachability test is now the "Inclusion tree" for determining
  which syms/sections will be included in the output
- 'reachable' field is now and 'include' and is the sole
  consideration in including sections/symbols (no more complex
  conditional checks)
- Order LOCAL before GLOBAL in the symbol table.  Apparently, after
  a FILE sym, all LOCAL symbols should precede GLOBAL syms or readelf
  shows <corrupt>
- Handle __ksymtab_strings section and __ksymtab_* syms

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-03-04 15:56:41 -06:00
Seth Jennings
62bb62f409 s/sybmols/symbols/ in create-diff-object
Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-02-17 13:30:30 -06:00
Seth Jennings
d32ca30052 don't compare data on NOBITS sections
NOBITS section may have a non-zero size, however, the have no data and
the data descriptor will have d_buf set to NULL.

This commit fixes as segfault that occurs from trying to compare the
data of such sections.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-02-13 11:18:25 -06:00
Seth Jennings
d927215dda check for one-to-one section-symbol relationship
There are cases in which the compiler will create symbols with NOTYPE
that map to a non-zero offset inside an .rodata section.  In that case, there
may not be a one-to-one relationship between that symbol and section as
the section may contains the data for multiple NOTYPE symbols.

This commit checks for this case and does not assign the symbol pointer of the
section that contains its data to avoid multiple symbols referring to the same
section from overwriting one another.  It also adds a check ensuring that all
symbols whose type is !NOTYPE start at offset 0 within the section.  This
should be guarenteed by the -ffunction-sections and -fdata-sections options
compiler options.

Signed-off-by: Seth Jennings <sjenning@redhat.com>
2014-02-13 11:18:25 -06:00
Josh Poimboeuf
4f27b9ae31 functional reorganization
Organize the files functionally:
- kmod/core: core kmod source
- kmod/patch: patch kmod source
- kpatch: kpatch script
- kpatch-build: kpatch build script and supporting tools
- contrib: distro-related files
2014-02-13 11:00:06 -06:00