This is an archive of the discontinued LLVM Phabricator instance.

Differential D40954

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

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

Details

Reviewers
ruiu
• espindola
Commits
rG89481f363a43: [ELF] - Teach LLD to hint about -fdebug-types-section.
rLLD328081: [ELF] - Teach LLD to hint about -fdebug-types-section.
rL328081: [ELF] - Teach LLD to hint about -fdebug-types-section.
Summary

Patch teaches LLD to hint user about -fdebug-types-section flag
if relocation overflow happens in debug section.

Diff Detail

Event Timeline

grimar created this revision.Dec 7 2017, 7:27 AM
Herald added subscribers: JDevlieghere, fedor.sergeev, kbarton and 7 others. · View Herald TranscriptDec 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–92

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 added a comment.Dec 30 2017, 11:55 PM

Ping.

grimar updated this revision to Diff 138528.Mar 15 2018, 4:28 AM
grimar edited reviewers, added: • espindola; removed: • rafael.
  • Reimplemented.
Herald added a subscriber: arichardson. · View Herald TranscriptMar 15 2018, 4:28 AM
nhaehnle removed a subscriber: nhaehnle.Mar 17 2018, 5:22 AM
• rafael added subscribers: nhaehnle, • rafael.Mar 19 2018, 3:23 PM

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

grimar added subscribers: • espindola, ruiu.Mar 20 2018, 2:09 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

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
13

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
Closed by commit rL328081: [ELF] - Teach LLD to hint about -fdebug-types-section. (authored by grimar). · Explain WhyMar 21 2018, 2:22 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
ELF/
15 lines
18 lines
test/
ELF/
12 lines

Diff 128081

ELF/Target.h

Show First 20 Lines • Show All 129 Lines • ▼ Show 20 Lines
TargetInfo *getPPCTargetInfo(); TargetInfo *getPPCTargetInfo();
TargetInfo *getSPARCV9TargetInfo(); TargetInfo *getSPARCV9TargetInfo();
TargetInfo *getX32TargetInfo(); TargetInfo *getX32TargetInfo();
TargetInfo *getX86TargetInfo(); TargetInfo *getX86TargetInfo();
TargetInfo *getX86_64TargetInfo(); TargetInfo *getX86_64TargetInfo();
template <class ELFT> TargetInfo *getMipsTargetInfo(); template <class ELFT> TargetInfo *getMipsTargetInfo();
std::string getErrorLocation(const uint8_t *Loc); std::string getErrorLocation(const uint8_t *Loc);
InputSectionBase *getErrorSection(const uint8_t *Loc);
uint64_t getPPC64TocBase(); uint64_t getPPC64TocBase();
uint64_t getAArch64Page(uint64_t Expr); uint64_t getAArch64Page(uint64_t Expr);
extern TargetInfo *Target; extern TargetInfo *Target;
TargetInfo *getTarget(); TargetInfo *getTarget();
template <class ELFT> bool isMipsPIC(const Defined *Sym); template <class ELFT> bool isMipsPIC(const Defined *Sym);
static inline void reportRangeError(uint8_t *Loc, RelType Type, const Twine &V, static inline void reportRangeError(uint8_t *Loc, RelType Type, const Twine &V,
int64_t Min, uint64_t Max) { int64_t Min, uint64_t Max) {
error(getErrorLocation(Loc) + "relocation " + lld::toString(Type) + std::string Msg = getErrorLocation(Loc) +
" out of range: " + V + " is not in [" + Twine(Min) + ", " + ("relocation " + lld::toString(Type) + " out of range: " +
Twine(Max) + "]"); V + " is not in [" + Twine(Min) + ", " + Twine(Max) + "]")
.str();
InputSectionBase *IS = getErrorSection(Loc);
if (IS && IS->Name.startswith(".debug"))
Msg += "; consider recompiling with -fdebug-types-section to reduce size "
"of debug sections";
error(Msg);
} }
template <unsigned N> template <unsigned N>
static void checkInt(uint8_t *Loc, int64_t V, RelType Type) { static void checkInt(uint8_t *Loc, int64_t V, RelType Type) {
if (!llvm::isInt<N>(V)) if (!llvm::isInt<N>(V))
reportRangeError(Loc, Type, Twine(V), llvm::minIntN(N), llvm::maxIntN(N)); reportRangeError(Loc, Type, Twine(V), llvm::minIntN(N), llvm::maxIntN(N));
} }
Show All 26 Lines

ELF/Target.cpp

Show First 20 Lines • Show All 81 Lines • ▼ Show 20 LinesTargetInfo *elf::getTarget() {
case EM_X86_64: case EM_X86_64:
if (Config->EKind == ELF32LEKind) if (Config->EKind == ELF32LEKind)
return getX32TargetInfo(); return getX32TargetInfo();
return getX86_64TargetInfo(); return getX86_64TargetInfo();
} }
fatal("unknown target machine"); fatal("unknown target machine");
} }
template <class ELFT> static std::string getErrorLoc(const uint8_t *Loc) { // For given location in output file returns input section and offset.
Optional<std::pair<InputSectionBase *, uint64_t>>
getErrorInfo(const uint8_t *Loc) {
ruiuUnsubmitted
Not Done
ReplyInline Actions

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

ruiu: If you need to get an input section, you can just define a different function, instead of…
for (InputSectionBase *D : InputSections) { for (InputSectionBase *D : InputSections) {
auto *IS = dyn_cast<InputSection>(D); auto *IS = dyn_cast<InputSection>(D);
if (!IS || !IS->getParent()) if (!IS || !IS->getParent())
continue; continue;
uint8_t *ISLoc = IS->getParent()->Loc + IS->OutSecOff; uint8_t *ISLoc = IS->getParent()->Loc + IS->OutSecOff;
if (ISLoc <= Loc && Loc < ISLoc + IS->getSize()) if (ISLoc <= Loc && Loc < ISLoc + IS->getSize())
return IS->template getLocation<ELFT>(Loc - ISLoc) + ": "; return std::make_pair(IS, Loc - ISLoc);
}
return None;
} }
template <class ELFT> static std::string getErrorLoc(const uint8_t *Loc) {
if (Optional<std::pair<InputSectionBase *, uint64_t>> Err = getErrorInfo(Loc))
return Err->first->template getLocation<ELFT>(Err->second) + ": ";
return ""; return "";
} }
InputSectionBase *elf::getErrorSection(const uint8_t *Loc) {
if (Optional<std::pair<InputSectionBase *, uint64_t>> Err = getErrorInfo(Loc))
return Err->first;
return nullptr;
}
std::string elf::getErrorLocation(const uint8_t *Loc) { std::string elf::getErrorLocation(const uint8_t *Loc) {
switch (Config->EKind) { switch (Config->EKind) {
case ELF32LEKind: case ELF32LEKind:
return getErrorLoc<ELF32LE>(Loc); return getErrorLoc<ELF32LE>(Loc);
case ELF32BEKind: case ELF32BEKind:
return getErrorLoc<ELF32BE>(Loc); return getErrorLoc<ELF32BE>(Loc);
case ELF64LEKind: case ELF64LEKind:
return getErrorLoc<ELF64LE>(Loc); return getErrorLoc<ELF64LE>(Loc);
▲ Show 20 Lines • Show All 63 Lines • Show Last 20 Lines

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

# 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
espindolaUnsubmitted
Not Done
ReplyInline Actions

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

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