Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-fasm-blocks: __asm { call fptr } does not use RIP-relative addressing #73033

Closed
dmiller423 opened this issue Nov 21, 2023 · 4 comments · Fixed by #73207
Closed

-fasm-blocks: __asm { call fptr } does not use RIP-relative addressing #73033

dmiller423 opened this issue Nov 21, 2023 · 4 comments · Fixed by #73207

Comments

@dmiller423
Copy link

dmiller423 commented Nov 21, 2023

clang version 17.0.4 Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: C:\dev\llvm\bin

Target is a simple static library, contents: two functions one __weak_symbol attribute((naked)) containing assembly in intel format that calls a static global address in the same file (set by the second function).

The generated objects are not position independent code, using R_X86_64_32S for relocations of the functions containing asm blocks declared as weak.

tested TARGET list { x86_64-pc-freebsd12-elf, x86_64-pc-unknown-elf, x86_64-pc-linux-gnu }

--
reproduction:
clang-17.0.4 --target=%TARGET% --sysroot=C:/tc_sysroot/%TARGET%/ -fPIC -fPIE -fasm-blocks -MD -MT syscalls.c.o -MF syscalls.c.o.d -o syscalls.c.o -c syscalls.c

objects relocations were tested via readelf and custom tooling, all cases produce:

> readelf -r syscalls.c.o

Relocation section '.rela.text' at offset 0x120 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
000000000000000f  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4
000000000000002f  000000030000000b R_X86_64_32S           0000000000000000 .bss + 0

rebuilding the same file/params with a previous version (clang-15.0.7):

> readelf -r syscalls.c.o

Relocation section '.rela.text' at offset 0x128 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
000000000000000f  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4
000000000000002e  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4

source:

#include <stdint.h>
#include <sys/types.h>

static uint64_t syscall_addr = 0;

int syscalls_init(uint64_t addr) {
    syscall_addr = addr;
    return 0;
}

__weak_symbol 
__attribute__((naked)) 
ssize_t read(int fd, void* buf, size_t nbytes)  {
    __asm {
            push rbp
            mov rbp, rsp
            mov eax, 3
            mov r10, rcx
            call syscall_addr
            pop rbp
            ret
    }
}
@github-actions github-actions bot added the clang Clang issues not falling into any other category label Nov 21, 2023
@EugeneZelenko EugeneZelenko added clang:codegen and removed clang Clang issues not falling into any other category labels Nov 21, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 21, 2023

@llvm/issue-subscribers-clang-codegen

Author: David Miller (dmiller423)

`` clang version 17.0.4 Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: C:\dev\llvm\bin``

Target is a simple static library, contents: two functions one __weak_symbol attribute((naked)) containing assembly in intel format that calls a static global address in the same file (set by the second function).

The generated objects are not position independent code, using R_X86_64_32S for relocations of the functions containing asm blocks declared as weak.

tested TARGET list { x86_64-pc-freebsd12-elf, x86_64-pc-unknown-elf, x86_64-pc-linux-gnu }

--
reproduction:
clang-17.0.4 --target=%TARGET% --sysroot=C:/tc_sysroot/%TARGET%/ -fPIC -fPIE -fasm-blocks -MD -MT syscalls.c.o -MF syscalls.c.o.d -o syscalls.c.o -c syscalls.c

objects relocations were tested via readelf and custom tooling, all cases produce:

&gt; readelf -r syscalls.c.o

Relocation section '.rela.text' at offset 0x120 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
000000000000000f  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4
000000000000002f  000000030000000b R_X86_64_32S           0000000000000000 .bss + 0

rebuilding the same file/params with a previous version (clang-15.0.7):

&gt; readelf -r syscalls.c.o

Relocation section '.rela.text' at offset 0x128 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
000000000000000f  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4
000000000000002e  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4

source:

#include &lt;stdint.h&gt;
#include &lt;sys/types.h&gt;

static uint64_t syscall_addr = 0;

int syscalls_init(uint64_t addr) {
    syscall_addr = addr;
    return 0;
}

__weak_symbol 
__attribute__((naked)) 
ssize_t read(int fd, void* buf, size_t nbytes)  {
    __asm {
            push rbp
            mov rbp, rsp
            mov eax, 3
            mov r10, rcx
            call syscall_addr
            pop rbp
            ret
    }
}

@efriedma-quic
Copy link
Collaborator

@MaskRay
Copy link
Member

MaskRay commented Nov 22, 2023

Adding back BaseReg = BaseReg ? BaseReg : 1; will produce callq *syscall_addr(%rip) instead of callq *syscall_addr. But that would break clang/test/CodeGen/ms-inline-asm{,-64}.c fixed by https://reviews.llvm.org/D151863

MaskRay added a commit to MaskRay/llvm-project that referenced this issue Nov 23, 2023
https://reviews.llvm.org/D151863 (2023-05) removed
`BaseReg = BaseReg ? BaseReg : 1` (introduced in commit
175d0ae (2013)) and caused a
regression: ensuring a non-zero `BaseReg` was to treat
`static void (*fptr)(); __asm { call fptr }` as non-`AbsMem`
`AsmOperandClass` and generate `call $0`/`callq *fptr(%rip)` instead of
`call ${0:P}`/`callq *fptr`
(The asm template argument modifier `P` is for call targets, while
no modifier is used by other instructions like `mov rax, fptr`)

This patch reinstates the BaseReg-setting statement but places it inside
`if (IsGlobalLV)` for clarify.

The special case is unfortunate, but we also have special case in
similar places (https://reviews.llvm.org/D149920).

Fix: llvm#73033
@MaskRay MaskRay changed the title Clang 17.0.4 ignores -fPIC in certain cases -fasm-blocks: __asm { call fptr } does not use RIP-relative addressing Nov 23, 2023
MaskRay added a commit that referenced this issue Nov 27, 2023
https://reviews.llvm.org/D151863 (2023-05) removed
`BaseReg = BaseReg ? BaseReg : 1` (introduced in commit
175d0ae (2013)) and caused a
regression: ensuring a non-zero `BaseReg` was to treat
`static void (*fptr)(); __asm { call fptr }` as non-`AbsMem`
`AsmOperandClass` and generate `call $0`/`callq *fptr(%rip)` instead of
`call ${0:P}`/`callq *fptr`
(The asm template argument modifier `P` is for call targets, while
no modifier is used by other instructions like `mov rax, fptr`)

This patch reinstates the BaseReg-setting statement but places it inside
`if (IsGlobalLV)` for clarify.

The special case is unfortunate, but we also have special case in
similar places (https://reviews.llvm.org/D149920).

Fix: #73033
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 27, 2023

@llvm/issue-subscribers-backend-x86

Author: David Miller (dmiller423)

`` clang version 17.0.4 Target: x86_64-pc-windows-msvc Thread model: posix InstalledDir: C:\dev\llvm\bin``

Target is a simple static library, contents: two functions one __weak_symbol attribute((naked)) containing assembly in intel format that calls a static global address in the same file (set by the second function).

The generated objects are not position independent code, using R_X86_64_32S for relocations of the functions containing asm blocks declared as weak.

tested TARGET list { x86_64-pc-freebsd12-elf, x86_64-pc-unknown-elf, x86_64-pc-linux-gnu }

--
reproduction:
clang-17.0.4 --target=%TARGET% --sysroot=C:/tc_sysroot/%TARGET%/ -fPIC -fPIE -fasm-blocks -MD -MT syscalls.c.o -MF syscalls.c.o.d -o syscalls.c.o -c syscalls.c

objects relocations were tested via readelf and custom tooling, all cases produce:

&gt; readelf -r syscalls.c.o

Relocation section '.rela.text' at offset 0x120 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
000000000000000f  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4
000000000000002f  000000030000000b R_X86_64_32S           0000000000000000 .bss + 0

rebuilding the same file/params with a previous version (clang-15.0.7):

&gt; readelf -r syscalls.c.o

Relocation section '.rela.text' at offset 0x128 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
000000000000000f  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4
000000000000002e  0000000300000002 R_X86_64_PC32          0000000000000000 .bss - 4

source:

#include &lt;stdint.h&gt;
#include &lt;sys/types.h&gt;

static uint64_t syscall_addr = 0;

int syscalls_init(uint64_t addr) {
    syscall_addr = addr;
    return 0;
}

__weak_symbol 
__attribute__((naked)) 
ssize_t read(int fd, void* buf, size_t nbytes)  {
    __asm {
            push rbp
            mov rbp, rsp
            mov eax, 3
            mov r10, rcx
            call syscall_addr
            pop rbp
            ret
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants