dynamic: Fix -U for -fpatchable-function-entry #1799
Conversation
honggyukim
force-pushed
the
check/unpatch-for-patchable-func
branch
from
582f6a0
to
f1150cf
Compare
| @@ -623,6 +623,7 @@ int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol * | |||
|
|
|||
| switch (mdi->type) { | |||
| case DYNAMIC_FENTRY: | |||
| case DYNAMIC_PATCHABLE: | |||
There was a problem hiding this comment.
But there's nothing to unpatch for -fpatchable-function-entry as they are already NOPs.
There was a problem hiding this comment.
It does its job in the following example.
$ g++ -fpatchable-function-entry=5 example.cpp
$ uftrace record -P. -U ^std:: a.out
Since -P. asks patching all the available functions, we need a way to suppress the previous patching request.
There was a problem hiding this comment.
Ok, multiple updates also make it useful.
| else if (match == 1) | ||
| mcount_patch_func_with_stats(mdi, sym); | ||
| else | ||
| mcount_unpatch_func(mdi, sym, NULL); |
There was a problem hiding this comment.
IIRC I saw this change in the Ciena's work.
There was a problem hiding this comment.
Right, it looks 151fb61 missed fixing patch_patchable_func_matched along with patch_normal_func_matched.
There was a problem hiding this comment.
As it has other problems, I think I can merge this one first.
We have missed handling -U option to functions compiled with -fpatchable-function-entry. This patch fixes the problem as follows. Before: $ ./runtest.py "265|266" Start 2 tests with 2 worker Compiler gcc clang Runtime test case pg finstrument-fu fpatchable-fun pg finstrument-fu fpatchable-fun ------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os 265 patchable_dynamic3 : NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG 266 patchable_dynamic4 : NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG NG After: $ ./runtest.py "265|266" Start 2 tests with 2 worker Compiler gcc clang Runtime test case pg finstrument-fu fpatchable-fun pg finstrument-fu fpatchable-fun ------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os 265 patchable_dynamic3 : OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK 266 patchable_dynamic4 : OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK This is verified in both x86_64 and aarch64. Fixes: 151fb61 ("dynamic: Refactor 'match_pattern_list'") Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>
honggyukim
force-pushed
the
check/unpatch-for-patchable-func
branch
from
f1150cf
to
782e0bc
Compare
|
I just added the following line at the commit message. |
| @@ -623,6 +623,7 @@ int mcount_unpatch_func(struct mcount_dynamic_info *mdi, struct uftrace_symbol * | |||
|
|
|||
| switch (mdi->type) { | |||
| case DYNAMIC_FENTRY: | |||
| case DYNAMIC_PATCHABLE: | |||
There was a problem hiding this comment.
Ok, multiple updates also make it useful.
| else if (match == 1) | ||
| mcount_patch_func_with_stats(mdi, sym); | ||
| else | ||
| mcount_unpatch_func(mdi, sym, NULL); |
There was a problem hiding this comment.
As it has other problems, I think I can merge this one first.
We have missed handling -U option to functions compiled with -fpatchable-function-entry.
This patch fixes the problem as follows.
Before:
After:
This is verified in both x86_64 and aarch64.
Fixes: 151fb61 ("dynamic: Refactor 'match_pattern_list'")