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

rbp clobber in inline assembly is not respected for functions with a frame pointer #58528

Open
aykevl opened this issue Oct 21, 2022 · 2 comments

Comments

@aykevl
Copy link
Contributor

aykevl commented Oct 21, 2022

The rbp register can be clobbered by inline assembly, like so:

void simplenop(void) {
    asm volatile("nop" : : : "rbp");
}

This results in the following expected assembly:

simplenop():                          # @simplenop()
        push    rbp
        nop
        pop     rbp
        ret

However, when a frame pointer is forced, this clobber constraint is not respected:

void buffernop(int size) {
    char buf[size];
    asm volatile("nop" : : "r"(buf): "rbp");
}
buffernop(int):                          # @buffernop(int)
        push    rbp
        mov     rbp, rsp
        mov     eax, edi
        mov     rcx, rsp
        add     rax, 15
        and     rax, -16
        sub     rcx, rax
        mov     rsp, rcx
        nop
        mov     rsp, rbp
        pop     rbp
        ret

As you can see, the generated code clearly expects the rbp to not be clobbered after the inline assembly.
Godbolt link: https://godbolt.org/z/osMEb1Kcj

This resulted in an actual bug in tinygo-org/tinygo#3103 (comment) where I try to call functions with a custom ABI via inline assembly.
Ideally the X86 backend should save the rbp register somewhere else (for example, on the stack: rsp is not clobbered) or just throw an error. Silently miscompiling results in bugs.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 21, 2022

@llvm/issue-subscribers-backend-x86

@phoebewang
Copy link
Contributor

rbp is not allowed in such case, GCC reports error for it https://godbolt.org/z/ar376adeY
We may need add it for Clang too.

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

No branches or pull requests

4 participants