* -fpatchable-function-entry should set SHF_WRITE and create one __patchable_function_entries per function
@ 2020-01-07 6:06 Fangrui Song
2020-01-07 7:26 ` Fangrui Song
0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2020-01-07 6:06 UTC (permalink / raw)
To: gcc
Cc: Martin Liška, Alexander Monakov, Torsten Duwe,
Maxim Kuvyrkov, Nick Clifton
The addresses of NOPs are collected in a section named __patchable_function_entries.
A __patchable_function_entries entry is relocated by a symbolic relocation (e.g. R_X86_64_64, R_AARCH64_ABS64, R_PPC64_ADDR64).
In -shared or -pie mode, the linker will create a dynamic relocation (non-preemptible: relative relocation (e.g. R_X86_64_RELATIVE); preemptible: symbolic relocation (e.g. R_X86_64_64)).
In either case, the section contents will be modified at runtime.
Thus, the section should have the SHF_WRITE flag to avoid text relocations (DF_TEXTREL).
When -ffunction-sections is used, ideally GCC should emit one __patchable_function_entries (SHF_LINK_ORDER) per .text.foo .
If the corresponding .text.foo is discarded (--gc-sections, COMDAT, /DISCARD/), the linker can discard the associated __patchable_function_entries. This can be seen as a lightweight COMDAT section group. (A section group adds an extra section and costs 3 words)
Currently lld (LLVM linker) has implemented such SHF_LINK_ORDER collecting features. GNU ld and gold don't have the features.
I have summarized the feature requests in this post https://sourceware.org/ml/binutils/2019-11/msg00266.html
gcc -fpatchable-function-entry=2 -ffunction-sections -c a.c
[ 4] .text.f0 PROGBITS 0000000000000000 000040 000009 00 AX 0 0 1
### No W flag
### One __patchable_function_entries instead of 3.
[ 5] __patchable_function_entries PROGBITS 0000000000000000 000049 000018 00 A 0 0 1
[ 6] .rela__patchable_function_entries RELA 0000000000000000 000280 000048 18 I 13 5 8
[ 7] .text.f1 PROGBITS 0000000000000000 000061 000009 00 AX 0 0 1
[ 8] .text.f2 PROGBITS 0000000000000000 00006a 000009 00 AX 0 0 1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: -fpatchable-function-entry should set SHF_WRITE and create one __patchable_function_entries per function
2020-01-07 6:06 -fpatchable-function-entry should set SHF_WRITE and create one __patchable_function_entries per function Fangrui Song
@ 2020-01-07 7:26 ` Fangrui Song
2020-01-07 10:32 ` Szabolcs Nagy
0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2020-01-07 7:26 UTC (permalink / raw)
To: gcc
Cc: Martin Liška, Alexander Monakov, Torsten Duwe,
Maxim Kuvyrkov, Nick Clifton
On 2020-01-06, Fangrui Song wrote:
>The addresses of NOPs are collected in a section named __patchable_function_entries.
>A __patchable_function_entries entry is relocated by a symbolic relocation (e.g. R_X86_64_64, R_AARCH64_ABS64, R_PPC64_ADDR64).
>In -shared or -pie mode, the linker will create a dynamic relocation (non-preemptible: relative relocation (e.g. R_X86_64_RELATIVE); preemptible: symbolic relocation (e.g. R_X86_64_64)).
>
>In either case, the section contents will be modified at runtime.
>Thus, the section should have the SHF_WRITE flag to avoid text relocations (DF_TEXTREL).
>
>
>
>When -ffunction-sections is used, ideally GCC should emit one __patchable_function_entries (SHF_LINK_ORDER) per .text.foo .
>If the corresponding .text.foo is discarded (--gc-sections, COMDAT, /DISCARD/), the linker can discard the associated __patchable_function_entries. This can be seen as a lightweight COMDAT section group. (A section group adds an extra section and costs 3 words)
>Currently lld (LLVM linker) has implemented such SHF_LINK_ORDER collecting features. GNU ld and gold don't have the features.
>
>I have summarized the feature requests in this post https://sourceware.org/ml/binutils/2019-11/msg00266.html
>
>gcc -fpatchable-function-entry=2 -ffunction-sections -c a.c
>
> [ 4] .text.f0 PROGBITS 0000000000000000 000040 000009 00 AX 0 0 1
> ### No W flag
> ### One __patchable_function_entries instead of 3.
> [ 5] __patchable_function_entries PROGBITS 0000000000000000 000049 000018 00 A 0 0 1
> [ 6] .rela__patchable_function_entries RELA 0000000000000000 000280 000048 18 I 13 5 8
> [ 7] .text.f1 PROGBITS 0000000000000000 000061 000009 00 AX 0 0 1
> [ 8] .text.f2 PROGBITS 0000000000000000 00006a 000009 00 AX 0 0 1
Emitting a __patchable_function_entries for each function may waste
object file sizes (64 bytes per function on ELF64). If zeros are
allowed, emitting a single __patchable_function_entries should be fine.
If we do want to emit unique sections, the condition should be either
-ffunction-sections or COMDAT is used.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: -fpatchable-function-entry should set SHF_WRITE and create one __patchable_function_entries per function
2020-01-07 7:26 ` Fangrui Song
@ 2020-01-07 10:32 ` Szabolcs Nagy
2020-01-08 9:16 ` __patchable_function_entries is flawed Fangrui Song
0 siblings, 1 reply; 6+ messages in thread
From: Szabolcs Nagy @ 2020-01-07 10:32 UTC (permalink / raw)
To: Fangrui Song, gcc
Cc: nd, Martin Liška, Alexander Monakov, Torsten Duwe,
Maxim Kuvyrkov, nickc
On 07/01/2020 07:25, Fangrui Song wrote:
> On 2020-01-06, Fangrui Song wrote:
>> The addresses of NOPs are collected in a section named __patchable_function_entries.
>> A __patchable_function_entries entry is relocated by a symbolic relocation (e.g. R_X86_64_64, R_AARCH64_ABS64, R_PPC64_ADDR64).
>> In -shared or -pie mode, the linker will create a dynamic relocation (non-preemptible: relative relocation (e.g. R_X86_64_RELATIVE);
>> preemptible: symbolic relocation (e.g. R_X86_64_64)).
>>
>> In either case, the section contents will be modified at runtime.
>> Thus, the section should have the SHF_WRITE flag to avoid text relocations (DF_TEXTREL).
pie/pic should either imply writable __patchable_function_entries,
or __patchable_function_entries should be documented to be offsets
from some base address in the module: the users of it have to modify
.text and do lowlevel hacks so they should be able to handle such
arithmetics.
i think it's worth opening a gcc bug report.
>> When -ffunction-sections is used, ideally GCC should emit one __patchable_function_entries (SHF_LINK_ORDER) per .text.foo .
>> If the corresponding .text.foo is discarded (--gc-sections, COMDAT, /DISCARD/), the linker can discard the associated
>> __patchable_function_entries. This can be seen as a lightweight COMDAT section group. (A section group adds an extra section and costs 3 words)
>> Currently lld (LLVM linker) has implemented such SHF_LINK_ORDER collecting features. GNU ld and gold don't have the features.
>>
>> I have summarized the feature requests in this post https://sourceware.org/ml/binutils/2019-11/msg00266.html
>>
>> gcc -fpatchable-function-entry=2 -ffunction-sections -c a.c
>>
>> [ 4] .text.f0 PROGBITS 0000000000000000 000040 000009 00 AX 0 0 1
>> ### No W flag
>> ### One __patchable_function_entries instead of 3.
>> [ 5] __patchable_function_entries PROGBITS 0000000000000000 000049 000018 00 A 0 0 1
>> [ 6] .rela__patchable_function_entries RELA 0000000000000000 000280 000048 18 I 13 5 8
>> [ 7] .text.f1 PROGBITS 0000000000000000 000061 000009 00 AX 0 0 1
>> [ 8] .text.f2 PROGBITS 0000000000000000 00006a 000009 00 AX 0 0 1
>
> Emitting a __patchable_function_entries for each function may waste
> object file sizes (64 bytes per function on ELF64). If zeros are
> allowed, emitting a single __patchable_function_entries should be fine.
>
> If we do want to emit unique sections, the condition should be either
> -ffunction-sections or COMDAT is used.
again it's worth raising a gcc bug i think.
there is another known issue: aarch64 -mbranch-protect=bti
(and presumably x86_64 -fcf-protection=branch) has to add
landing pad at the begining of each indirectly called function
so the patchable nops can only come after that.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92424
no matter how this gets resolved i think this will require
documentation changes too.
^ permalink raw reply [flat|nested] 6+ messages in thread
* __patchable_function_entries is flawed
2020-01-07 10:32 ` Szabolcs Nagy
@ 2020-01-08 9:16 ` Fangrui Song
2020-01-10 3:53 ` Fangrui Song
0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2020-01-08 9:16 UTC (permalink / raw)
To: gcc
Cc: nd, Szabolcs Nagy, Martin Liška, Alexander Monakov,
Torsten Duwe, Maxim Kuvyrkov, nickc
On 2020-01-07, Szabolcs Nagy wrote:
>On 07/01/2020 07:25, Fangrui Song wrote:
>> On 2020-01-06, Fangrui Song wrote:
>>> The addresses of NOPs are collected in a section named __patchable_function_entries.
>>> A __patchable_function_entries entry is relocated by a symbolic relocation (e.g. R_X86_64_64, R_AARCH64_ABS64, R_PPC64_ADDR64).
>>> In -shared or -pie mode, the linker will create a dynamic relocation (non-preemptible: relative relocation (e.g. R_X86_64_RELATIVE);
>>> preemptible: symbolic relocation (e.g. R_X86_64_64)).
>>>
>>> In either case, the section contents will be modified at runtime.
>>> Thus, the section should have the SHF_WRITE flag to avoid text relocations (DF_TEXTREL).
>
>pie/pic should either imply writable __patchable_function_entries,
>or __patchable_function_entries should be documented to be offsets
>from some base address in the module: the users of it have to modify
>.text and do lowlevel hacks so they should be able to handle such
>arithmetics.
>
>i think it's worth opening a gcc bug report.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93194 patch posted to gcc-patches.
PC-relative relocation types (R_X86_64_64, R_AARCH64_PREL64,
R_PPC64_REL64) can avoid dynamic relocations, and avoid the need for
SHF_WRITE. Unfortunately, it seems we cannot re-interpret
__patchable_function_entries. I have found 2 other problems with the
current __patchable_function_entries. Perhaps we need a new design.
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93197
__patchable_function_entries will always be collected by --gc-sections
(.tmp_vmlinux1 is not linked with --gc-sections, so it appears that
the Linux kernel is not affected.)
I think the solution requires a GNU ld and gold side fix:
https://sourceware.org/bugzilla/show_bug.cgi?id=24526
If you clang>=8, you can play with clang -fstack-size-section -c a.cc
Basically I think __patchable_function_entry has to behave like .stack_sizes .
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195
__patchable_function_entries should consider comdat groups.
It can cause linker failures (GNU ld, gold and lld). It is fairly
easy to trigger with C++ inline.
(OK, the Linux kernel does not speak C++.)
(Clang's function multi-versioning implementation uses comdat, even in
C. I don't have an example with GCC. My GCC __attribute__((target(..))) is broken.)
The two issues make -fpatchable-function-entry useless for user
applications. I still hope we can make the solution more general, not
restricted to the Linux kernel. We've already implemented enough GCC
options for dynamic ftrace (with regs) and live patching: -mfentry
-mnop-mcount -mrecord-mcount -mhotpatch -fpatchable-function-entry.
FWIW I am working on a clang/llvm patch https://reviews.llvm.org/D72215
I have tried resolving these problems.
("o" (SHF_LINK_ORDER+sh_link) is not recognized by GNU as.)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: __patchable_function_entries is flawed
2020-01-08 9:16 ` __patchable_function_entries is flawed Fangrui Song
@ 2020-01-10 3:53 ` Fangrui Song
2020-01-10 6:28 ` Fangrui Song
0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2020-01-10 3:53 UTC (permalink / raw)
To: gcc
Cc: nd, Szabolcs Nagy, Martin Liška, Alexander Monakov,
Torsten Duwe, Maxim Kuvyrkov, nickc
On 2020-01-08, Fangrui Song wrote:
>On 2020-01-07, Szabolcs Nagy wrote:
>>On 07/01/2020 07:25, Fangrui Song wrote:
>>>On 2020-01-06, Fangrui Song wrote:
>>>>The addresses of NOPs are collected in a section named __patchable_function_entries.
>>>>A __patchable_function_entries entry is relocated by a symbolic relocation (e.g. R_X86_64_64, R_AARCH64_ABS64, R_PPC64_ADDR64).
>>>>In -shared or -pie mode, the linker will create a dynamic relocation (non-preemptible: relative relocation (e.g. R_X86_64_RELATIVE);
>>>>preemptible: symbolic relocation (e.g. R_X86_64_64)).
>>>>
>>>>In either case, the section contents will be modified at runtime.
>>>>Thus, the section should have the SHF_WRITE flag to avoid text relocations (DF_TEXTREL).
>>
>>pie/pic should either imply writable __patchable_function_entries,
>>or __patchable_function_entries should be documented to be offsets
>>from some base address in the module: the users of it have to modify
>>.text and do lowlevel hacks so they should be able to handle such
>>arithmetics.
>>
>>i think it's worth opening a gcc bug report.
>
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93194 patch posted to gcc-patches.
>
>PC-relative relocation types (R_X86_64_64, R_AARCH64_PREL64,
>R_PPC64_REL64) can avoid dynamic relocations, and avoid the need for
>SHF_WRITE. Unfortunately, it seems we cannot re-interpret
>__patchable_function_entries. I have found 2 other problems with the
>current __patchable_function_entries. Perhaps we need a new design.
>
>
>* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93197
> __patchable_function_entries will always be collected by --gc-sections
> (.tmp_vmlinux1 is not linked with --gc-sections, so it appears that
> the Linux kernel is not affected.)
> I think the solution requires a GNU ld and gold side fix:
> https://sourceware.org/bugzilla/show_bug.cgi?id=24526
>
> If you clang>=8, you can play with clang -fstack-size-section -c a.cc
> Basically I think __patchable_function_entry has to behave like .stack_sizes .
>
>* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195
> __patchable_function_entries should consider comdat groups.
>
> It can cause linker failures (GNU ld, gold and lld). It is fairly
> easy to trigger with C++ inline.
> (OK, the Linux kernel does not speak C++.)
> (Clang's function multi-versioning implementation uses comdat, even in
> C. I don't have an example with GCC. My GCC __attribute__((target(..))) is broken.)
>
>The two issues make -fpatchable-function-entry useless for user
>applications. I still hope we can make the solution more general, not
>restricted to the Linux kernel. We've already implemented enough GCC
>options for dynamic ftrace (with regs) and live patching: -mfentry
>-mnop-mcount -mrecord-mcount -mhotpatch -fpatchable-function-entry.
>
>FWIW I am working on a clang/llvm patch https://reviews.llvm.org/D72215
>I have tried resolving these problems.
>("o" (SHF_LINK_ORDER+sh_link) is not recognized by GNU as.)
The LLVM 10.0 release branch is scheduled next week. I want to
contribute my clang -fpatchable-function-entry patches before that
(already approved), so that clang built Linux people can play with it
with Clang 10.0 .
Can we agree on the syntax of the new option before 2020-01-15?
We can reuse the option name, just overload the value by adding the
third argument, "pcrel":
-fpatchable-function-entry=2,0,pcrel
If "pcrel" is seen, GCC should emit the __patchable_function_entries
section which contains PC-relative addresses, instead of absolute
addresses. This is backward compatible.
% stage1-gcc/xgcc -B stage1-gcc -fpatchable-function-entry=1,a -xc /dev/null
cc1: error: invalid arguments for ‘-fpatchable_function_entry’
(OK, another bug: incorrect option name in the diagnostic.)
I can teach Clang to only recognize the pcrel form.
The function attribute does not need any change: __attribute__((patchable_function_entry(N,M)))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: __patchable_function_entries is flawed
2020-01-10 3:53 ` Fangrui Song
@ 2020-01-10 6:28 ` Fangrui Song
0 siblings, 0 replies; 6+ messages in thread
From: Fangrui Song @ 2020-01-10 6:28 UTC (permalink / raw)
To: gcc
Cc: nd, Szabolcs Nagy, Martin Liška, Alexander Monakov,
Torsten Duwe, Maxim Kuvyrkov, nickc
On 2020-01-09, Fangrui Song wrote:
>On 2020-01-08, Fangrui Song wrote:
>>On 2020-01-07, Szabolcs Nagy wrote:
>>>On 07/01/2020 07:25, Fangrui Song wrote:
>>>>On 2020-01-06, Fangrui Song wrote:
>>>>>The addresses of NOPs are collected in a section named __patchable_function_entries.
>>>>>A __patchable_function_entries entry is relocated by a symbolic relocation (e.g. R_X86_64_64, R_AARCH64_ABS64, R_PPC64_ADDR64).
>>>>>In -shared or -pie mode, the linker will create a dynamic relocation (non-preemptible: relative relocation (e.g. R_X86_64_RELATIVE);
>>>>>preemptible: symbolic relocation (e.g. R_X86_64_64)).
>>>>>
>>>>>In either case, the section contents will be modified at runtime.
>>>>>Thus, the section should have the SHF_WRITE flag to avoid text relocations (DF_TEXTREL).
>>>
>>>pie/pic should either imply writable __patchable_function_entries,
>>>or __patchable_function_entries should be documented to be offsets
>>>from some base address in the module: the users of it have to modify
>>>.text and do lowlevel hacks so they should be able to handle such
>>>arithmetics.
>>>
>>>i think it's worth opening a gcc bug report.
>>
>>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93194 patch posted to gcc-patches.
>>
>>PC-relative relocation types (R_X86_64_64, R_AARCH64_PREL64,
>>R_PPC64_REL64) can avoid dynamic relocations, and avoid the need for
>>SHF_WRITE. Unfortunately, it seems we cannot re-interpret
>>__patchable_function_entries. I have found 2 other problems with the
>>current __patchable_function_entries. Perhaps we need a new design.
>>
>>
>>* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93197
>> __patchable_function_entries will always be collected by --gc-sections
>> (.tmp_vmlinux1 is not linked with --gc-sections, so it appears that
>> the Linux kernel is not affected.)
>> I think the solution requires a GNU ld and gold side fix:
>> https://sourceware.org/bugzilla/show_bug.cgi?id=24526
>>
>> If you clang>=8, you can play with clang -fstack-size-section -c a.cc
>> Basically I think __patchable_function_entry has to behave like .stack_sizes .
>>
>>* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93195
>> __patchable_function_entries should consider comdat groups.
>>
>> It can cause linker failures (GNU ld, gold and lld). It is fairly
>> easy to trigger with C++ inline.
>> (OK, the Linux kernel does not speak C++.)
>> (Clang's function multi-versioning implementation uses comdat, even in
>> C. I don't have an example with GCC. My GCC __attribute__((target(..))) is broken.)
>>
>>The two issues make -fpatchable-function-entry useless for user
>>applications. I still hope we can make the solution more general, not
>>restricted to the Linux kernel. We've already implemented enough GCC
>>options for dynamic ftrace (with regs) and live patching: -mfentry
>>-mnop-mcount -mrecord-mcount -mhotpatch -fpatchable-function-entry.
>>
>>FWIW I am working on a clang/llvm patch https://reviews.llvm.org/D72215
>>I have tried resolving these problems.
>>("o" (SHF_LINK_ORDER+sh_link) is not recognized by GNU as.)
>
>The LLVM 10.0 release branch is scheduled next week. I want to
>contribute my clang -fpatchable-function-entry patches before that
>(already approved), so that clang built Linux people can play with it
>with Clang 10.0 .
>
>Can we agree on the syntax of the new option before 2020-01-15?
>
>We can reuse the option name, just overload the value by adding the
>third argument, "pcrel":
>
>-fpatchable-function-entry=2,0,pcrel
>
>If "pcrel" is seen, GCC should emit the __patchable_function_entries
>section which contains PC-relative addresses, instead of absolute
>addresses. This is backward compatible.
>
> % stage1-gcc/xgcc -B stage1-gcc -fpatchable-function-entry=1,a -xc /dev/null
> cc1: error: invalid arguments for ‘-fpatchable_function_entry’
>
>(OK, another bug: incorrect option name in the diagnostic.)
>
>I can teach Clang to only recognize the pcrel form.
>
>
>The function attribute does not need any change: __attribute__((patchable_function_entry(N,M)))
Some architectures do not have a PC-relative relocation type of pointer
size, e.g. MIPS has R_MIPS_PC32 (GNU extension) but not R_MIPS_PC64, and
.quad foo-. is not representable.
Making -fpatchable-function-entry=2,0 absolute by default is fine (I
will teach Clang to emit such __patchable_function_entries table). Just
wanted to mention that on most other architectures PC-relative is
better.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-10 6:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07 6:06 -fpatchable-function-entry should set SHF_WRITE and create one __patchable_function_entries per function Fangrui Song
2020-01-07 7:26 ` Fangrui Song
2020-01-07 10:32 ` Szabolcs Nagy
2020-01-08 9:16 ` __patchable_function_entries is flawed Fangrui Song
2020-01-10 3:53 ` Fangrui Song
2020-01-10 6:28 ` Fangrui Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).