-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
[RelocationResolver][Xtensa] Implement R_XTENSA_32 #96311
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
# Tests Xtensa relocations. We provide a .debug_info section with multiple | ||
# DW_AT_high_pc entries (that's one of the attributes for which relocations are | ||
# resolved by llvm-dwarfdump) and we add a relocation for each of them. | ||
# | ||
# RUN: yaml2obj %s | llvm-dwarfdump - | FileCheck %s | ||
|
||
# To add more tests you need to modify the Content of the .debug_abbrev and | ||
# .debug_info sections. To do that create a test.s which matches the assembly | ||
# code below, run the command that follows and copy over the "Content" value of | ||
# the respective sections: | ||
Comment on lines
+7
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps the simplest thing to do would be to have only a .debug_addr section with some relocatable addresses in it? Could the assembly actually include the symbol references that would generate the relocations (can llvm-mc generate these relocations?) so they don't have to be hand-crafted in the IR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
i'm not familiar enough with dwarf to say if it's simpler/reasonable.
i'm sure the espressif fork of the llvm can produce R_XTENSA_32. |
||
# | ||
# ``` | ||
# $ cat test.s | ||
# .section .debug_abbrev,"",@progbits | ||
# .byte 1 # Abbreviation Code | ||
# .byte 0x11 # DW_TAG_compile_unit | ||
# .byte 0 # DW_CHILDREN_no | ||
# | ||
# # Add a DW_AT_high_pc for each relocation we test. | ||
# .rept 2 # (UPDATE HERE) | ||
# .byte 0x12 # DW_AT_high_pc | ||
# .byte 0x01 # DW_FORM_addr | ||
# .endr | ||
# | ||
# .byte 0 # EOM(1) | ||
# .byte 0 # EOM(2) | ||
# .byte 0 # EOM(3) | ||
# | ||
# .section .debug_info,"",@progbits | ||
# .4byte 2+4+1+1+4*2 # Length of Unit (UPDATE HERE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could use assembly labels to compute the length here to make it easier to update the test in the future? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, i just followed the existing practice: https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-dwarfdump/RISCV/riscv-relocs.yaml |
||
# .2byte 4 # DWARF version number | ||
# .4byte .debug_abbrev # Offset Into Abbrev. Section | ||
# .byte 4 # Address Size (in bytes) | ||
# .byte 1 # Abbrev 1 | ||
# .4byte 0x00000042 # Test 1 | ||
# .4byte 0x00000042 # Test 2 | ||
# $ llvm-mc test.s -filetype obj -triple xtensa -o - | obj2yaml | ||
# ``` | ||
# | ||
# Note that you may need xtensa-enabled llvm-mc. | ||
# I used a version from https://github.com/espressif/llvm-project. | ||
|
||
--- !ELF | ||
FileHeader: | ||
Class: ELFCLASS32 | ||
Data: ELFDATA2LSB | ||
Type: ET_REL | ||
Machine: EM_XTENSA | ||
SectionHeaderStringTable: .strtab | ||
Sections: | ||
- Name: .text | ||
Type: SHT_PROGBITS | ||
Flags: [ SHF_ALLOC, SHF_EXECINSTR ] | ||
AddressAlign: 0x4 | ||
- Name: .debug_abbrev | ||
Type: SHT_PROGBITS | ||
AddressAlign: 0x1 | ||
Content: '01110012011201000000' | ||
- Name: .debug_info | ||
Type: SHT_PROGBITS | ||
AddressAlign: 0x1 | ||
Content: '1000000004000000000004014200000042000000' | ||
- Name: .rela.debug_info | ||
Type: SHT_RELA | ||
Flags: [ SHF_INFO_LINK ] | ||
Link: .symtab | ||
AddressAlign: 0x4 | ||
Info: .debug_info | ||
Relocations: | ||
|
||
# Test 1 | ||
# 0x42 with R_XTENSA_NONE(0x1) = 0x42 | ||
# CHECK: DW_AT_high_pc (0x00000042) | ||
- Offset: 0x0000000C # 0xC + 4*0 | ||
Symbol: v1 | ||
Type: R_XTENSA_NONE | ||
|
||
# Test 2 | ||
# 0x42 with R_XTENSA_32(0xFFFFFFFF) = 0x00000041 | ||
# CHECK-NEXT: DW_AT_high_pc (0x00000041) | ||
- Offset: 0x00000010 # 0xC + 4*1 | ||
Symbol: vFFFFFFFF | ||
Type: R_XTENSA_32 | ||
|
||
Symbols: | ||
- Name: v1 | ||
Type: STT_SECTION | ||
Section: .debug_info | ||
Value: 0x00000001 | ||
- Name: vFFFFFFFF | ||
Type: STT_SECTION | ||
Section: .debug_info | ||
Value: 0xFFFFFFFF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could drop
R_*_NONE
. It should not appear in relocation files.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i feel it's better for this kind of tools to handle more cases as far as it's legal and easy.
anyway your comment isn't specific to this PR, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the comment is specific to this PR.
RelocationResolver is to handle static relocations. While R_NONE might be defined by a psABI, in practice it's always a toolchain bug when RelocationResolver has to resolve R_NONE. We should not handle unnecessary relocations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean that for some reasons your rationale is specific to xtensa (this PR)
and doesn't apply to other non-xtensa relocations like R_X86_64_NONE?