Skip to content

[Test][JITLink] Correctly generate the R_X86_64_PC8 relocation. #109283

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

Merged
merged 1 commit into from
Sep 22, 2024

Conversation

higuoxing
Copy link
Member

Previously, ELF_R_X86_64_PC8.s doesn't produce the R_X86_64_PC8 relocation. This patch helps fix it.

Before:

llvm-mc -triple=x86_64-unknown-linux -position-independent -filetype=obj -o - ELF_R_X86_64_PC8.s | llvm-readobj -r -

File: <stdin>
Format: elf64-x86-64
Arch: x86_64
AddressSize: 64bit
LoadName: <Not found>
Relocations [
]

After:

llvm-mc -triple=x86_64-unknown-linux -position-independent -filetype=obj -o - ELF_R_X86_64_PC8.s | llvm-readobj -r -

File: <stdin>
Format: elf64-x86-64
Arch: x86_64
AddressSize: 64bit
LoadName: <Not found>
Relocations [
  Section (3) .rela.text {
    0x4 R_X86_64_PC8 main 0xFFFFFFFFFFFFFFFF
  }
]

@higuoxing higuoxing requested review from lhames and maksfb September 19, 2024 13:44
@@ -10,10 +10,4 @@
main:
xorl %eax, %eax
retq
.size main, .-main

.type P,@object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting the issue and providing the fix. I'm not sure why loope would make llvm-mc emit relocation and .byte main-. would not. I suggest not to rely on the loope behavior and issue .byte main-. in a different section, e.g. .rodata. I.e. if you insert .rodata before .globl P, it will force llvm-mc to always emit a relocation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! I fixed it by emitting a byte main - . to the .rodata section.

Previously, ELF_R_X86_64_PC8.s doesn't produce the R_X86_64_PC8
relocation. This patch helps fix it by emitting a byte `main - .` to the
.rodata section.

Before:

```
llvm-mc -triple=x86_64-unknown-linux -position-independent -filetype=obj -o - ELF_R_X86_64_PC8.s | llvm-readobj -r -

File: <stdin>
Format: elf64-x86-64
Arch: x86_64
AddressSize: 64bit
LoadName: <Not found>
Relocations [
]
```

After:

```
llvm-mc -triple=x86_64-unknown-linux -position-independent -filetype=obj -o - ELF_R_X86_64_PC8.s | llvm-readobj -r -

File: <stdin>
Format: elf64-x86-64
Arch: x86_64
AddressSize: 64bit
LoadName: <Not found>
Relocations [
  Section (4) .rela.rodata {
    0x0 R_X86_64_PC8 main 0x0
  }
]
```
Copy link
Contributor

@lhames lhames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @higuoxing and @maksfb!

@higuoxing higuoxing changed the title [Test][JitLink] Correctly generate the R_X86_64_PC8 relocation. [Test][JITLink] Correctly generate the R_X86_64_PC8 relocation. Sep 22, 2024
@higuoxing higuoxing merged commit 38666e6 into llvm:main Sep 22, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants