[llvm-readelf]Don't lose negative-ness of negative addends for no symbol relocations

[jhenderson]

Summary

llvm-readelf prints relocation addends as "<symbol value>[+-]<absolute addend>" where [+-] is determined from whether addend is less than zero or not. However, it does not print the +/- if there is no symbol, which meant that negative addends became their positive value with no indication that this had happened. This patch stops the absolute conversion when addends are negative and there is no associated symbol.

Before:
0000000000000008 0000000000000000 R_X86_64_NONE 1
With patch:
0000000000000008 0000000000000000 R_X86_64_NONE ffffffffffffffff

[mattd]

Makes sense. LGTM.

[Higuoxing]

This comment was removed by Higuoxing.

tools/llvm-readobj/ELFDumper.cpp

3401 ↗

(On Diff #189717)

Can we use to_hexString(RelAddend, false) here? I think it's better to have consistent code style.

[Higuoxing]

LGTM, but a small nit

[MaskRay]

tools/llvm-readobj/ELFDumper.cpp

2735 ↗

(On Diff #189717)

If r_addend = LLONG_MIN, abs(r_addend) is ub...

"The behavior is undefined if the result cannot be represented by the return type."

[jhenderson]

Thanks all. I am already planning a number of follow-up changes to fix the issues re. consistent formatting and the LLONG_MIN comment. This particular issue however is something I needed more urgently, so wanted to fix first.

tools/llvm-readobj/ELFDumper.cpp

2735 ↗

(On Diff #189717)

Yes, it is. But it's already a problem, so I'd like to fix this separately, if that's okay?

3401 ↗

(On Diff #189717)

Would you mind if I do this in a later patch? I'm looking to try to share the code for printRelocation and printDynamicRelocation, since they're almost identical anyway, and this would be one of the changes I'm planning on then.

[MaskRay]

tools/llvm-readobj/ELFDumper.cpp

2735 ↗

(On Diff #189717)

That's totally fine :)

[Higuoxing]

tools/llvm-readobj/ELFDumper.cpp

3401 ↗

(On Diff #189717)

Never mind :)

llvm/trunk/test/tools/llvm-readobj/elf-reloc-negative-addend-no-sym.test

+# Show that llvm-readelf properly prints the addend for relocations that do not
+# have an associated symbol but have a negative addend.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readelf --relocations %t | FileCheck %s --check-prefix=REL
+# RUN: llvm-readelf --dyn-relocations %t | FileCheck %s --check-prefix=DYN
+
+# REL:      Relocation section '.rela.text' at offset {{.*}} contains 1 entries:
+# REL-NEXT:    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
+# REL-NEXT: 0000000000000000  0000000000000000 R_X86_64_NONE                     ffffffffffffffff
+# REL-EMPTY:
+# REL-NEXT: Relocation section '.rela.dyn' at offset {{.*}} contains 1 entries:
+# REL-NEXT:    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
+# REL-NEXT: 0000000000000008  0000000000000000 R_X86_64_NONE                     ffffffffffffffff
+
+# DYN:      'RELA' relocation section at offset {{.*}} contains 24 bytes:
+# DYN-NEXT:     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
+# DYN-NEXT: 0000000000000008  0000000000000000 R_X86_64_NONE                     ffffffffffffffff
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+    Size: 0x10
+  - Name: .rela.text
+    Type: SHT_RELA
+    Link: .symtab
+    Info: .text
+    Relocations:
+      - Offset: 0
+        Type: R_X86_64_NONE
+        Addend: -1
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Flags: [SHF_ALLOC]
+    Address: 0x1000
+    AddressAlign: 0x1000
+    Entries:
+      - Tag: DT_RELA
+        Value: 0x1100
+      - Tag: DT_RELASZ
+        Value: 24
+      - Tag: DT_RELAENT
+        Value: 24
+      - Tag: DT_NULL
+        Value: 0
+  - Name: .rela.dyn
+    Type: SHT_RELA
+    Flags: [SHF_ALLOC]
+    Info: .text
+    Address: 0x1100
+    AddressAlign: 0x100
+    Relocations:
+      - Offset: 8
+        Type: R_X86_64_NONE
+        Addend: -1
+DynamicSymbols:
+  Global:
+    - Name: force_dynsym
+ProgramHeaders:
+  - Type: PT_LOAD
+    VAddr: 0x1000
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+  - Type: PT_DYNAMIC
+    VAddr: 0x1000
+    Sections:
+      - Section: .dynamic

llvm/trunk/tools/llvm-readobj/ELFDumper.cpp

void GNUStyle<ELFT>::printRelocation(const ELFO *Obj, const Elf_Shdr *SymTab,
   Fields[2].Str = RelocName.str();
   if (Sym)
     Fields[3].Str = to_string(format_hex_no_prefix(Sym->getValue(), Width));
   Fields[4].Str = TargetName;
   for (auto &F : Fields)
     printField(F);
 
   std::string Addend;
-  if (Sym && IsRela) {
-    if (R.r_addend < 0)
+  if (IsRela) {
+    int64_t RelAddend = R.r_addend;
+    if (Sym) {
+      if (R.r_addend < 0) {
         Addend = " - ";
-    else
+        RelAddend = std::abs(RelAddend);
+      } else
         Addend = " + ";
     }
 
-  if (IsRela)
-    Addend += to_hexString(std::abs(R.r_addend), false);
+    Addend += to_hexString(RelAddend, false);
+  }
   OS << Addend << "\n";
 }
 
 template <class ELFT> void GNUStyle<ELFT>::printRelocHeader(unsigned SType) {
   bool IsRela = SType == ELF::SHT_RELA || SType == ELF::SHT_ANDROID_RELA;
   bool IsRelr = SType == ELF::SHT_RELR || SType == ELF::SHT_ANDROID_RELR;
   if (ELFT::Is64Bits)
     OS << "    ";
void GNUStyle<ELFT>::printDynamicRelocation(const ELFO *Obj, Elf_Rela R,
 
   if (!SymbolName.empty() || Sym->getValue() != 0)
     Fields[3].Str = to_string(format_hex_no_prefix(Sym->getValue(), Width));
 
   Fields[4].Str = SymbolName;
   for (auto &Field : Fields)
     printField(Field);
 
-  int64_t RelAddend = R.r_addend;
   std::string Addend;
-  if (!SymbolName.empty() && IsRela) {
-    if (R.r_addend < 0)
+  if (IsRela) {
+    int64_t RelAddend = R.r_addend;
+    if (!SymbolName.empty()) {
+      if (R.r_addend < 0) {
         Addend = " - ";
-    else
+        RelAddend = std::abs(RelAddend);
+      } else
         Addend = " + ";
     }
-
-  if (IsRela)
-    Addend += to_string(format_hex_no_prefix(std::abs(RelAddend), 1));
+    Addend += to_string(format_hex_no_prefix(RelAddend, 1));
+  }
   OS << Addend << "\n";
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printDynamicRelocations(const ELFO *Obj) {
   const DynRegionInfo &DynRelRegion = this->dumper()->getDynRelRegion();
   const DynRegionInfo &DynRelaRegion = this->dumper()->getDynRelaRegion();
   const DynRegionInfo &DynRelrRegion = this->dumper()->getDynRelrRegion();