This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Teach LLD to hint about -fdebug-types-section.
ClosedPublic

Authored by grimar on Dec 7 2017, 7:27 AM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Dec 7 2017, 7:27 AM
grimar planned changes to this revision.Dec 7 2017, 7:31 AM
grimar updated this revision to Diff 125956.Dec 7 2017, 7:34 AM
  • Changed message format.
ruiu added inline comments.Dec 7 2017, 3:28 PM
ELF/Arch/AArch64.cpp
340 ↗(On Diff #125956)

getErrorLocation(Loc).Loc looks odd by itself. getErrorLocation should, well, return an error location. If you need to access Loc member of its return value, that implies that the function does return something more than a location. Please revert this and other lines.

ELF/Target.cpp
90 ↗(On Diff #125956)

If you need to get an input section, you can just define a different function, instead of making the return type more complex.

grimar planned changes to this revision.Dec 8 2017, 1:54 AM

Thanks for comments, Rui. I'll update it after D40962 be landed (I think it will be and my patch intersects with it).

grimar updated this revision to Diff 128081.Dec 23 2017, 9:40 AM
  • Addressed comments, rebased.
grimar updated this revision to Diff 138528.Mar 15 2018, 4:28 AM
grimar edited reviewers, added: espindola; removed: rafael.
  • Reimplemented.
nhaehnle removed a subscriber: nhaehnle.Mar 17 2018, 5:22 AM

Most callers want just the location, so maybe leave a getErrorLocation
inline helper that returns getErrorPlace(Loc).Loc?

With that the implementation is fine, but is someone really hitting this
issue?

Cheers,
Rafael

George Rimar via Phabricator via llvm-commits
<llvm-commits@lists.llvm.org> writes:

grimar updated this revision to Diff 138528.
grimar edited reviewers, added: espindola; removed: rafael.
grimar added a comment.
Herald added a subscriber: arichardson.

  • Reimplemented.

https://reviews.llvm.org/D40954

Files:

ELF/Arch/AArch64.cpp
ELF/Arch/AMDGPU.cpp
ELF/Arch/ARM.cpp
ELF/Arch/AVR.cpp
ELF/Arch/Mips.cpp
ELF/Arch/PPC.cpp
ELF/Arch/PPC64.cpp
ELF/Arch/SPARCV9.cpp
ELF/Arch/X86.cpp
ELF/Arch/X86_64.cpp
ELF/Target.cpp
ELF/Target.h
test/ELF/x86-64-reloc-debug-overflow.s

Index: test/ELF/x86-64-reloc-debug-overflow.s

  • test/ELF/x86-64-reloc-debug-overflow.s

+++ test/ELF/x86-64-reloc-debug-overflow.s
@@ -0,0 +1,12 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/x86-64-reloc-error.s -o %tabs
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+# RUN: not ld.lld -shared %tabs %t -o %t2 2>&1 | FileCheck %s
+
+# CHECK: (.debug_info+0x1): relocation R_X86_64_32 out of range: 68719476736 is not in [0, 4294967295]; consider recompiling with -fdebug-types-section to reduce size of debug sections
+
+# CHECK: (.debug_info+0x8): relocation R_X86_64_32S out of range: -281474976710656 is not in [-2147483648, 2147483647]; consider recompiling with -fdebug-types-section to reduce size of debug sections
+
+.section .debug_info,"",@progbits
+ movl $big, %edx
+ movq $foo - 0x1000000000000, %rdx

Index: ELF/Target.h

  • ELF/Target.h

+++ ELF/Target.h
@@ -139,7 +139,13 @@
TargetInfo *getX86_64TargetInfo();
template <class ELFT> TargetInfo *getMipsTargetInfo();

-std::string getErrorLocation(const uint8_t *Loc);
+struct ErrorPlace {
+ InputSectionBase *IS;
+ std::string Loc;
+};
+
+// Returns input section and corresponding source string for the given location.
+ErrorPlace getErrorPlace(const uint8_t *Loc);

uint64_t getPPC64TocBase();
uint64_t getAArch64Page(uint64_t Expr);
@@ -151,9 +157,15 @@

static inline void reportRangeError(uint8_t *Loc, RelType Type, const Twine &V,

int64_t Min, uint64_t Max) {
  • error(getErrorLocation(Loc) + "relocation " + lld::toString(Type) +
  • " out of range: " + V + " is not in [" + Twine(Min) + ", " +
  • Twine(Max) + "]");

+ ErrorPlace ErrPlace = getErrorPlace(Loc);
+ StringRef Hint;
+ if (ErrPlace.IS && ErrPlace.IS->Name.startswith(".debug"))
+ Hint = "; consider recompiling with -fdebug-types-section to reduce size "
+ "of debug sections";
+
+ error(ErrPlace.Loc + "relocation " + lld::toString(Type) +
+ " out of range: " + V.str() + " is not in [" + Twine(Min).str() + ", " +
+ Twine(Max).str() + "]" + Hint);
}

template <unsigned N>
@@ -180,7 +192,7 @@
template <unsigned N>
static void checkAlignment(uint8_t *Loc, uint64_t V, RelType Type) {

if ((V & (N - 1)) != 0)
  • error(getErrorLocation(Loc) + "improper alignment for relocation " +

+ error(getErrorPlace(Loc).Loc + "improper alignment for relocation " +

lld::toString(Type) + ": 0x" + llvm::utohexstr(V) +
" is not aligned to " + Twine(N) + " bytes");

}

Index: ELF/Target.cpp

  • ELF/Target.cpp

+++ ELF/Target.cpp
@@ -87,29 +87,29 @@

fatal("unknown target machine");

}

-template <class ELFT> static std::string getErrorLoc(const uint8_t *Loc) {
+template <class ELFT> static ErrorPlace getErrPlace(const uint8_t *Loc) {

for (InputSectionBase *D : InputSections) {
  auto *IS = dyn_cast<InputSection>(D);
  if (!IS || !IS->getParent())
    continue;
 
  uint8_t *ISLoc = IS->getParent()->Loc + IS->OutSecOff;
  if (ISLoc <= Loc && Loc < ISLoc + IS->getSize())
  • return IS->template getLocation<ELFT>(Loc - ISLoc) + ": ";

+ return {IS, IS->template getLocation<ELFT>(Loc - ISLoc) + ": "};

}
  • return "";

+ return {};
}

-std::string elf::getErrorLocation(const uint8_t *Loc) {
+ErrorPlace elf::getErrorPlace(const uint8_t *Loc) {

switch (Config->EKind) {
case ELF32LEKind:
  • return getErrorLoc<ELF32LE>(Loc);

+ return getErrPlace<ELF32LE>(Loc);

case ELF32BEKind:
  • return getErrorLoc<ELF32BE>(Loc);

+ return getErrPlace<ELF32BE>(Loc);

case ELF64LEKind:
  • return getErrorLoc<ELF64LE>(Loc);

+ return getErrPlace<ELF64LE>(Loc);

case ELF64BEKind:
  • return getErrorLoc<ELF64BE>(Loc);

+ return getErrPlace<ELF64BE>(Loc);

default:
  llvm_unreachable("unknown ELF type");
}

Index: ELF/Arch/X86_64.cpp

  • ELF/Arch/X86_64.cpp

+++ ELF/Arch/X86_64.cpp
@@ -242,7 +242,7 @@

  memcpy(Inst, "\x48\xc7", 2);
  *RegSlot = 0xc0 | Reg;
} else {
  • error(getErrorLocation(Loc - 3) +

+ error(getErrorPlace(Loc - 3).Loc +

        "R_X86_64_GOTTPOFF must be used in MOVQ or ADDQ instructions only");
}

@@ -320,7 +320,7 @@

  write64le(Loc, Val);
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/X86.cpp

  • ELF/Arch/X86.cpp

+++ ELF/Arch/X86.cpp
@@ -304,7 +304,7 @@

  write32le(Loc, Val);
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/SPARCV9.cpp

  • ELF/Arch/SPARCV9.cpp

+++ ELF/Arch/SPARCV9.cpp
@@ -118,7 +118,7 @@

  write64be(Loc, Val);
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/PPC64.cpp

  • ELF/Arch/PPC64.cpp

+++ ELF/Arch/PPC64.cpp
@@ -206,7 +206,7 @@

  break;
}
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/PPC.cpp

  • ELF/Arch/PPC.cpp

+++ ELF/Arch/PPC.cpp
@@ -66,7 +66,7 @@

  write32be(Loc, read32be(Loc) | (Val & 0x3FFFFFC));
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/Mips.cpp

  • ELF/Arch/Mips.cpp

+++ ELF/Arch/Mips.cpp
@@ -463,7 +463,7 @@

if (Type2 == R_MICROMIPS_SUB &&
    (Type3 == R_MICROMIPS_HI16 || Type3 == R_MICROMIPS_LO16))
  return std::make_pair(Type3, -Val);
  • error(getErrorLocation(Loc) + "unsupported relocations combination " +

+ error(getErrorPlace(Loc).Loc + "unsupported relocations combination " +

      Twine(Type));
return std::make_pair(Type & 0xff, Val);

}
@@ -650,7 +650,7 @@

  writeShuffleValue<E>(Loc, Val, 23, 2);
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/AVR.cpp

  • ELF/Arch/AVR.cpp

+++ ELF/Arch/AVR.cpp
@@ -64,7 +64,7 @@

  break;
}
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + toString(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + toString(Type));

}

}

Index: ELF/Arch/ARM.cpp

  • ELF/Arch/ARM.cpp

+++ ELF/Arch/ARM.cpp
@@ -499,7 +499,7 @@

                (Val & 0x00ff));           // imm8
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/AMDGPU.cpp

  • ELF/Arch/AMDGPU.cpp

+++ ELF/Arch/AMDGPU.cpp
@@ -73,7 +73,7 @@

  write32le(Loc, Val >> 32);
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}

Index: ELF/Arch/AArch64.cpp

  • ELF/Arch/AArch64.cpp

+++ ELF/Arch/AArch64.cpp
@@ -337,7 +337,7 @@

  or32AArch64Imm(Loc, Val);
  break;
default:
  • error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));

+ error(getErrorPlace(Loc).Loc + "unrecognized reloc " + Twine(Type));

}

}


llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

Most callers want just the location, so maybe leave a getErrorLocation
inline helper that returns getErrorPlace(Loc).Loc?

With that the implementation is fine, but is someone really hitting this
issue?

Cheers,
Rafael

Yes.
We have LLD bug: "lld fails with "relocation R_X86_64_32 out of range" (https://bugs.llvm.org//show_bug.cgi?id=31109).

-fdebug-types-section flag is disabled by default, but it can help to reduce size of output a lot.
For example one of the best results for llvm binaries for me was:

"I compared clang binaries to see the difference with and without the linker side optimisation:

  1. Clang built with -g has size of 1.7 GB, .debug_info section size is 894.5 Mb.
  2. Clang built with -g -fdebug-types-section has size of 1.0 GB. .debug_types size is 26.267 MB, .debug_info size is 227.7 MB."

With that .debug_info is significantly reduced in size, so such hint might help to solve the out of range
failure sometimes I believe.

George.

grimar updated this revision to Diff 139109.Mar 20 2018, 5:17 AM
  • Introduced getErrorLocation helper.
nhaehnle removed a subscriber: nhaehnle.Mar 20 2018, 7:49 AM
espindola accepted this revision.Mar 20 2018, 4:10 PM

LGTM with a test nit.

test/ELF/x86-64-reloc-debug-overflow.s
12 ↗(On Diff #139109)

Having code in a .debug_info section is not very realistic.

This revision is now accepted and ready to land.Mar 20 2018, 4:10 PM
This revision was automatically updated to reflect the committed changes.