llvm dev - Nov 2020 - [llvm-mc] FreeBSD kernel module performance impact when upgrading clang

Hi,

I'm in the process of migrating from clang5 to clang10. Unfortunately
clang10 introduced a negative performance impact. The cause is an increase of
PLT entries from this patch (first released in clang7):

https://bugs.llvm.org/show_bug.cgi?id=36370
https://reviews.llvm.org/D43383

If I revert that clang patch locally, the additional PLT entries and the
performance impact disappear.

This occurs in the context of FreeBSD kernel modules. Using the example code
from this page:

https://www.freebsd.org/doc/en_US.ISO8859-1/books/arch-handbook/driverbasics-kld.html

...I can explain what I'm seeing. If I compare the objects generated by
clang5 and clang10 for the example:

clang5:

	$ objdump -r skeleton.o
	
	skeleton.o:     file format elf64-x86-64
	
	RELOCATION RECORDS FOR [.text]:
	OFFSET           TYPE              VALUE
	0000000000000019 R_X86_64_32S      .rodata.str1.1+0x0000000000000015
	0000000000000024 R_X86_64_32S      .rodata.str1.1+0x000000000000002b
	000000000000002b R_X86_64_PC32     uprintf-0x0000000000000004
	[...]

clang10:

	$ objdump -r skeleton.o
	
	skeleton.o:     file format elf64-x86-64
	
	RELOCATION RECORDS FOR [.text]:
	OFFSET           TYPE              VALUE
	0000000000000017 R_X86_64_32S      .rodata.str1.1+0x000000000000002b
	0000000000000020 R_X86_64_32S      .rodata.str1.1+0x0000000000000015
	0000000000000029 R_X86_64_PLT32    uprintf-0x0000000000000004
	[...]

The relocation for the external uprintf call is changed from R_X86_64_PC32 to
R_X86_64_PLT32.

Normally, amd64/x86 kernel modules are relocatable object files (via ld -r).
Because of that, D43383 typically has no impact as the FreeBSD loader sees the
relocations directly and treats R_X86_64_PC32 and R_X86_64_PLT32 the same:

https://github.com/freebsd/freebsd/blob/master/sys/amd64/amd64/elf_machdep.c#L321

But in my case, the kernel objects are created as shared objects. Using shared
objects is atypical for amd64, but done for every other architecture except
mips:

https://github.com/freebsd/freebsd/blob/master/sys/conf/kmod.mk#L81

The comments in the D43383 review suggest that a modern linker should reduce the
PLT32 relocations to PC32 for local calls. But I do not see that reduction even
when testing this and other examples with lld 10. My understanding is this is
due to the shared kernel objects. The relocations are being processed (and left
as PLT) prior to the kernel loader ever seeing them. Unfortunately this means
many calls that previously did not go through the PLT now do.

Note that allowing R_X86_64_PC32 within shared objects (without -fPIC) requires
a linker patch. This works within a kernel environment even if it should be
disallowed elsewhere. But it reveals the larger question raised by the patch and
its impact: whose responsibility should this behavior be?

It seems the linker/lld should supply an equivalent of -mcmodel=kernel, e.g.
indicating 64-bit pointers will fit in a 32-bit address space. (Stated another
way: it seems appropriate to allow users to 32-bit sign extend relocations in
shared libraries if they specify some sort of kernel mode.)

From there though, is the linker the place to eliminate PLT relocations for this
use case?

Or should the compiler be the one to specify the "right" relocations,
meaning the D43383 patch should be modified to emit a different relocation for
-mcmodel=kernel?

In summary:

1. Could you please clarify for me the conditions under which the PLT->PC
relocation reduction should occur?
2. Given the goal of eliminating unneeded PLT entries from shared kernel
objects: should the linker or the compiler be responsible for doing the right
thing?

Thanks,

Justin

Hi Justin,

I can answer your first question
> Could you please clarify for me the conditions under which the PLT->PC
relocation reduction should occur?
Below there is a description of LLD linker behavior with links to source code,
which also contains some usefull comments.

1) A symbol should be not preemtible. The responsible function that is used to
set
   this flag is `bool elf::computeIsPreemptible(const Symbol &sym)`
   (https://github.com/llvm/llvm-project/blob/master/lld/ELF/Symbols.cpp#L353)

   When the configuration is `--shared` it can be one of the reasons why a
symbol becomes
   preemtible. But `--Bsymbolic-functions`/`--Bsymbolic` options can
   be used to force it be non-preemtible.

2) Then when we know that a PLT entry will be resolved within the same ELF
module, we
   can skip PLT access and directly jump to the destination function.
   The responsible code that performs PLT->PC relaxation is the "expr =
fromPlt(expr);"
   line in `scanReloc(...)`
  
(https://github.com/llvm/llvm-project/blob/master/lld/ELF/Relocations.cpp#L1377)
   
Hope that clarifies the situation a bit.

Best regards,
George | Developer | Access Softek, Inc

________________________________________
От: Justin Cady <desk at justincady.com>
Отправлено: 2 ноября 2020 г. 22:00
Кому: llvm-dev at lists.llvm.org
Копия: George Rimar
Тема: [EXTERNAL] [llvm-mc] FreeBSD kernel module performance impact when
upgrading clang

CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you recognize the sender and know the content
is safe.  If you suspect potential phishing or spam email, report it to
ReportSpam at accesssoftek.com

Hi,

I'm in the process of migrating from clang5 to clang10. Unfortunately
clang10 introduced a negative performance impact. The cause is an increase of
PLT entries from this patch (first released in clang7):

https://bugs.llvm.org/show_bug.cgi?id=36370
https://reviews.llvm.org/D43383

If I revert that clang patch locally, the additional PLT entries and the
performance impact disappear.

This occurs in the context of FreeBSD kernel modules. Using the example code
from this page:

https://www.freebsd.org/doc/en_US.ISO8859-1/books/arch-handbook/driverbasics-kld.html

...I can explain what I'm seeing. If I compare the objects generated by
clang5 and clang10 for the example:

clang5:

        $ objdump -r skeleton.o

        skeleton.o:     file format elf64-x86-64

        RELOCATION RECORDS FOR [.text]:
        OFFSET           TYPE              VALUE
        0000000000000019 R_X86_64_32S      .rodata.str1.1+0x0000000000000015
        0000000000000024 R_X86_64_32S      .rodata.str1.1+0x000000000000002b
        000000000000002b R_X86_64_PC32     uprintf-0x0000000000000004
        [...]

clang10:

        $ objdump -r skeleton.o

        skeleton.o:     file format elf64-x86-64

        RELOCATION RECORDS FOR [.text]:
        OFFSET           TYPE              VALUE
        0000000000000017 R_X86_64_32S      .rodata.str1.1+0x000000000000002b
        0000000000000020 R_X86_64_32S      .rodata.str1.1+0x0000000000000015
        0000000000000029 R_X86_64_PLT32    uprintf-0x0000000000000004
        [...]

The relocation for the external uprintf call is changed from R_X86_64_PC32 to
R_X86_64_PLT32.

Normally, amd64/x86 kernel modules are relocatable object files (via ld -r).
Because of that, D43383 typically has no impact as the FreeBSD loader sees the
relocations directly and treats R_X86_64_PC32 and R_X86_64_PLT32 the same:

https://github.com/freebsd/freebsd/blob/master/sys/amd64/amd64/elf_machdep.c#L321

But in my case, the kernel objects are created as shared objects. Using shared
objects is atypical for amd64, but done for every other architecture except
mips:

https://github.com/freebsd/freebsd/blob/master/sys/conf/kmod.mk#L81

The comments in the D43383 review suggest that a modern linker should reduce the
PLT32 relocations to PC32 for local calls. But I do not see that reduction even
when testing this and other examples with lld 10. My understanding is this is
due to the shared kernel objects. The relocations are being processed (and left
as PLT) prior to the kernel loader ever seeing them. Unfortunately this means
many calls that previously did not go through the PLT now do.

Note that allowing R_X86_64_PC32 within shared objects (without -fPIC) requires
a linker patch. This works within a kernel environment even if it should be
disallowed elsewhere. But it reveals the larger question raised by the patch and
its impact: whose responsibility should this behavior be?

It seems the linker/lld should supply an equivalent of -mcmodel=kernel, e.g.
indicating 64-bit pointers will fit in a 32-bit address space. (Stated another
way: it seems appropriate to allow users to 32-bit sign extend relocations in
shared libraries if they specify some sort of kernel mode.)
>From there though, is the linker the place to eliminate PLT relocations for
this use case?
Or should the compiler be the one to specify the "right" relocations,
meaning the D43383 patch should be modified to emit a different relocation for
-mcmodel=kernel?

In summary:

1. Could you please clarify for me the conditions under which the PLT->PC
relocation reduction should occur?
2. Given the goal of eliminating unneeded PLT entries from shared kernel
objects: should the linker or the compiler be responsible for doing the right
thing?

Thanks,

Justin

On 2020-11-03, George Rimar wrote:
>Hi Justin,
>
>I can answer your first question
I can answer some other questions
>> Could you please clarify for me the conditions under which the
PLT->PC relocation reduction should occur?
>
>Below there is a description of LLD linker behavior with links to source
code,
>which also contains some usefull comments.
>
>1) A symbol should be not preemtible. The responsible function that is used
to set
>   this flag is `bool elf::computeIsPreemptible(const Symbol &sym)`
>  
(https://github.com/llvm/llvm-project/blob/master/lld/ELF/Symbols.cpp#L353)
>
>   When the configuration is `--shared` it can be one of the reasons why a
symbol becomes
>   preemtible. But `--Bsymbolic-functions`/`--Bsymbolic` options can
>   be used to force it be non-preemtible.
>
>2) Then when we know that a PLT entry will be resolved within the same ELF
module, we
>   can skip PLT access and directly jump to the destination function.
>   The responsible code that performs PLT->PC relaxation is the
"expr = fromPlt(expr);"
>   line in `scanReloc(...)`
>  
(https://github.com/llvm/llvm-project/blob/master/lld/ELF/Relocations.cpp#L1377)
>
>Hope that clarifies the situation a bit.
>
>Best regards,
>George | Developer | Access Softek, Inc
>
>________________________________________
>От: Justin Cady <desk at justincady.com>
>Отправлено: 2 ноября 2020 г. 22:00
>Кому: llvm-dev at lists.llvm.org
>Копия: George Rimar
>Тема: [EXTERNAL] [llvm-mc] FreeBSD kernel module performance impact when
upgrading clang
>
>CAUTION: This email originated from outside of the organization. Do not
click links or open attachments unless you recognize the sender and know the
content is safe.  If you suspect potential phishing or spam email, report it to
ReportSpam at accesssoftek.com
>
>Hi,
>
>I'm in the process of migrating from clang5 to clang10. Unfortunately
clang10 introduced a negative performance impact. The cause is an increase of
PLT entries from this patch (first released in clang7):
>
>https://bugs.llvm.org/show_bug.cgi?id=36370
>https://reviews.llvm.org/D43383
>
>If I revert that clang patch locally, the additional PLT entries and the
performance impact disappear.
>
>This occurs in the context of FreeBSD kernel modules. Using the example code
from this page:
>
>https://www.freebsd.org/doc/en_US.ISO8859-1/books/arch-handbook/driverbasics-kld.html
>
>...I can explain what I'm seeing. If I compare the objects generated by
clang5 and clang10 for the example:
>
>clang5:
>
>        $ objdump -r skeleton.o
>
>        skeleton.o:     file format elf64-x86-64
>
>        RELOCATION RECORDS FOR [.text]:
>        OFFSET           TYPE              VALUE
>        0000000000000019 R_X86_64_32S      .rodata.str1.1+0x0000000000000015
>        0000000000000024 R_X86_64_32S      .rodata.str1.1+0x000000000000002b
>        000000000000002b R_X86_64_PC32     uprintf-0x0000000000000004
>        [...]
>
>clang10:
>
>        $ objdump -r skeleton.o
>
>        skeleton.o:     file format elf64-x86-64
>
>        RELOCATION RECORDS FOR [.text]:
>        OFFSET           TYPE              VALUE
>        0000000000000017 R_X86_64_32S      .rodata.str1.1+0x000000000000002b
>        0000000000000020 R_X86_64_32S      .rodata.str1.1+0x0000000000000015
>        0000000000000029 R_X86_64_PLT32    uprintf-0x0000000000000004
>        [...]
>
>The relocation for the external uprintf call is changed from R_X86_64_PC32
to R_X86_64_PLT32.
The GNU as change is (binutils-gdb):

commit bd7ab16b4537788ad53521c45469a1bdae84ad4a
Author: H.J. Lu
Date:   Tue Feb 13 07:34:36 2018

     x86-64: Generate branch with PLT32 relocation

R_X86_64_PLT32 is a bit of a misnomer. The actual intention it conveys
is that this is a function call/jump operation and the address of the
target symbol is **insignificant**: which means the jump target can be
the actual function or a PLT entry.

For

   .globl _start, foo
   _start:
     .byte 0xe8
     .reloc ., R_X86_64_PC32, foo - 4
     .long foo - .
   
   foo:
     ret

as a.s -o a.o
ld.lld -shared a.o # error: relocation R_X86_64_PC32 cannot be used against
symbol foo; recompil

If you are using an older linker (either GNU ld or LLD), it may not have such a
diagnostic.
>Normally, amd64/x86 kernel modules are relocatable object files (via ld -r).
Because of that, D43383 typically has no impact as the FreeBSD loader sees the
relocations directly and treats R_X86_64_PC32 and R_X86_64_PLT32 the same:
>
>https://github.com/freebsd/freebsd/blob/master/sys/amd64/amd64/elf_machdep.c#L321
>
>But in my case, the kernel objects are created as shared objects. Using
shared objects is atypical for amd64, but done for every other architecture
except mips:
>
>https://github.com/freebsd/freebsd/blob/master/sys/conf/kmod.mk#L81
>
>The comments in the D43383 review suggest that a modern linker should reduce
the PLT32 relocations to PC32 for local calls. But I do not see that reduction
even when testing this and other examples with lld 10. My understanding is this
is due to the shared kernel objects. The relocations are being processed (and
left as PLT) prior to the kernel loader ever seeing them. Unfortunately this
means many calls that previously did not go through the PLT now do.
>
>Note that allowing R_X86_64_PC32 within shared objects (without -fPIC)
requires a linker patch. This works within a kernel environment even if it
should be disallowed elsewhere. But it reveals the larger question raised by the
patch and its impact: whose responsibility should this behavior be?
The user code's responsibility. See George's reply about
computeIsPreemptible. There are many ways to make a defined symbol in -shared
mode non-preemptible:

* visibility (usually via STV_HIDDEN; for a function, if you don't take the
address in any -fno-pic translation unit, STV_PROTECTED can be used as well if
you do want to export the function)
* -Bsymbolic, -Bsymbolic-functions
* --dynamic-list
* local: in --version-script
>It seems the linker/lld should supply an equivalent of -mcmodel=kernel, e.g.
indicating 64-bit pointers will fit in a 32-bit address space. (Stated another
way: it seems appropriate to allow users to 32-bit sign extend relocations in
shared libraries if they specify some sort of kernel mode.)
There is no such a need for a new option. These desired characteristics can be
achieved with existing mechanisms.
>From there though, is the linker the place to eliminate PLT relocations for
this use case?
>
>Or should the compiler be the one to specify the "right"
relocations, meaning the D43383 patch should be modified to emit a different
relocation for -mcmodel=kernel?
>
>In summary:
>
>1. Could you please clarify for me the conditions under which the PLT->PC
relocation reduction should occur?
>2. Given the goal of eliminating unneeded PLT entries from shared kernel
objects: should the linker or the compiler be responsible for doing the right
thing?
>
>Thanks,
>
>Justin