[RelocationResolver][Xtensa] Implement R_XTENSA_32 by yamt · Pull Request #96311 · llvm/llvm-project

yamt · 2024-06-21T14:23:30Z

This makes llvm-dwarfdump work on Xtensa ELF objects.

Reference: https://github.com/jcmvbkbc/xtensa-abi/blob/master/relocations

This makes llvm-dwarfdump work on Xtensa ELF objects. Reference: https://github.com/jcmvbkbc/xtensa-abi/blob/master/relocations

llvmbot · 2024-06-21T14:24:01Z

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-binary-utilities

Author: YAMAMOTO Takashi (yamt)

Changes

This makes llvm-dwarfdump work on Xtensa ELF objects.

Reference: https://github.com/jcmvbkbc/xtensa-abi/blob/master/relocations

Full diff: https://github.com/llvm/llvm-project/pull/96311.diff

1 Files Affected:

(modified) llvm/lib/Object/RelocationResolver.cpp (+27-2)

diff --git a/llvm/lib/Object/RelocationResolver.cpp b/llvm/lib/Object/RelocationResolver.cpp
index 564d9da78e97d..b404a79966b9d 100644
--- a/llvm/lib/Object/RelocationResolver.cpp
+++ b/llvm/lib/Object/RelocationResolver.cpp
@@ -310,6 +310,28 @@ static uint64_t resolveX86(uint64_t Type, uint64_t Offset, uint64_t S,
   }
 }
 
+static bool supportsXtensa(uint64_t Type) {
+  switch (Type) {
+  case ELF::R_XTENSA_NONE:
+  case ELF::R_XTENSA_32:
+    return true;
+  default:
+    return false;
+  }
+}
+
+static uint64_t resolveXtensa(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
+  case ELF::R_XTENSA_NONE:
+    return LocData;
+  case ELF::R_XTENSA_32:
+    return (S + Addend + LocData) & 0xFFFFFFFF;
+  default:
+    llvm_unreachable("Invalid relocation type");
+  }
+}
+
 static bool supportsPPC32(uint64_t Type) {
   switch (Type) {
   case ELF::R_PPC_ADDR32:
@@ -820,6 +842,8 @@ getRelocationResolver(const ObjectFile &Obj) {
     switch (Obj.getArch()) {
     case Triple::x86:
       return {supportsX86, resolveX86};
+    case Triple::xtensa:
+      return {supportsXtensa, resolveXtensa};
     case Triple::ppcle:
     case Triple::ppc:
       return {supportsPPC32, resolvePPC32};
@@ -885,11 +909,12 @@ uint64_t resolveRelocation(RelocationResolver Resolver, const RelocationRef &R,
 
       if (GetRelSectionType() == ELF::SHT_RELA) {
         Addend = getELFAddend(R);
-        // LoongArch and RISCV relocations use both LocData and Addend.
+        // LoongArch, RISCV, and Xtensa relocations use both LocData and Addend.
         if (Obj->getArch() != Triple::loongarch32 &&
             Obj->getArch() != Triple::loongarch64 &&
             Obj->getArch() != Triple::riscv32 &&
-            Obj->getArch() != Triple::riscv64)
+            Obj->getArch() != Triple::riscv64 &&
+            Obj->getArch() != Triple::xtensa)
           LocData = 0;
       }
     }

dwblaikie · 2024-06-22T19:14:55Z

Please add some test coverage

yamt · 2024-06-24T06:26:28Z

Please add some test coverage

i added a test

yamt · 2024-07-22T16:46:57Z

@dwblaikie any other concerns?

dwblaikie · 2024-07-22T19:31:01Z

+#  .byte 0                  # EOM(3)
+#
+# .section .debug_info,"",@progbits
+#  .4byte 2+4+1+1+4*2       # Length of Unit (UPDATE HERE)


could use assembly labels to compute the length here to make it easier to update the test in the future?

well, i just followed the existing practice: https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-dwarfdump/RISCV/riscv-relocs.yaml

dwblaikie · 2024-07-22T19:33:38Z

+# 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:


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?

Perhaps the simplest thing to do would be to have only a .debug_addr section with some relocatable addresses in it?

i'm not familiar enough with dwarf to say if it's simpler/reasonable.

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?

i'm sure the espressif fork of the llvm can produce R_XTENSA_32.
but i thought xtensa support in the stock llvm was somehow incomplete.

MaskRay · 2024-07-23T06:16:38Z

+static uint64_t resolveXtensa(uint64_t Type, uint64_t Offset, uint64_t S,
+                              uint64_t LocData, int64_t Addend) {
+  switch (Type) {
+  case ELF::R_XTENSA_NONE:


We could drop R_*_NONE. It should not appear in relocation files.

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?

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.

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.

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?

@MaskRay ping

even if it's an indication of toolchain bugs, i feel it's nicer for llvm-dwarfdump to be able to dump it.

No. DWARF sections should not contain NONE relocations.

can you please explain why it's specific to this PR and doesn't apply to R_X86_64_NONE etc?

Not specific to this PR. We could remove R_X86_64_NONE. Here R_X86_64_NONE is not the best example to follow. You should follow other architectures without NONE.

gerekon · 2026-05-21T08:04:36Z

@yamt Are still going to finish with this? Looks like we have PR (espressif#130) for the same relocation problem, but your solution seems to be preferable.

If you have no time for this do you mind if we will try to upstream your changes in another PR? Or probably you will give us the right to push to your branch?

cc @MabezDev @safronov @sstefan1

yamt · 2026-05-25T02:37:37Z

@yamt Are still going to finish with this? Looks like we have PR (espressif#130) for the same relocation problem, but your solution seems to be preferable.

If you have no time for this do you mind if we will try to upstream your changes in another PR? Or probably you will give us the right to push to your branch?

cc @MabezDev @safronov @sstefan1

i don't have time to work on this right now.
please feel free to do whatever convenient for you.

[RelocationResolver][Xtensa] Implement R_XTENSA_32

724fb17

This makes llvm-dwarfdump work on Xtensa ELF objects. Reference: https://github.com/jcmvbkbc/xtensa-abi/blob/master/relocations

llvmbot added the llvm:binary-utilities label Jun 21, 2024

yamt mentioned this pull request Jun 21, 2024

broken dwarf (LLVM-351) espressif/llvm-project#98

Open

Add a test

5c266c9

llvmbot added the debuginfo label Jun 24, 2024

dwblaikie reviewed Jul 22, 2024

View reviewed changes

MaskRay reviewed Jul 23, 2024

View reviewed changes

Conversation

yamt commented Jun 21, 2024

Uh oh!

llvmbot commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dwblaikie commented Jun 22, 2024

Uh oh!

yamt commented Jun 24, 2024

Uh oh!

yamt commented Jul 22, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MaskRay Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gerekon commented May 21, 2026

Uh oh!

yamt commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

llvmbot commented Jun 21, 2024 •

edited

Loading

MaskRay Aug 22, 2024 •

edited

Loading