dynup / kpatch

kpatch - live kernel patching
GNU General Public License v2.0
1.49k stars 305 forks source link

how about add -fno-reorder-functions in kpatch-build to deal with .text.unlikely.xxx sections? #1080

Closed haoren3696 closed 3 years ago

haoren3696 commented 4 years ago

gcc can place functions to .text.unlikely and .text.hot subsections during optimizations which making some functions can not be correlated in create-diff-object. How abould just add add -fno-reorder-functions in kpatch-build to disable the compile optimization?

jpoimboe commented 4 years ago

create-diff-object is supposed to handle those sections properly. Do you have a patch which doesn't work?

Changing optimization options can be risky, there might be the risk of the original kernel not matching the rebuilt original.

haoren3696 commented 4 years ago

create-diff-object is supposed to handle those sections properly. Do you have a patch which doesn't work?

I found that sometimes function foo with static variables can be put in .text.foo section in original binary and be put in .text.unlikely.foo section in patched binary. This will result in "reference to static local variable xxx in foo was removed" problem because the .text.foo section can not be correlated to .text.unlikely.foo section by create-diff-object.

Changing optimization options can be risky, there might be the risk of the original kernel not matching the rebuilt original.

gcc just put the function in .text.unlikely.xxx section,the symbol name doesn't change which is different with other optimization such as ".constprop/.isra/.part". So disable the optimization with -fno-reorder-functions just make sure the function is placed in .text.xxx section, kernel can still find the symbol name to patch or relocate.

correlation of the unlikely section is not so easy to deal with. Sometimes, a function may have two sections: .text.xxx and .text.unlikely.xxx。

julien-thierry commented 4 years ago

This sounds similar to the issue addressed by this PR: https://github.com/dynup/kpatch/pull/1054

I still need to rebase it and take the reviews into account. Hopefully it might fix you issue as well. If you have a way to reproduce your issue I can check that your case is covered by the PR.

joe-lawrence commented 4 years ago

bump: @haoren3696 : can you provide a reproducer, or at least verify if @julien-thierry 's suggestion that #1054 might fix this issue? Thanks!

haoren3696 commented 3 years ago

I'm sorry. I circumvented this problem by using -fno-reorder-functions.