[ELF] Change common diagnostics to report both object file location and source file location

[MaskRay]

Summary

Many diagnostics use getErrorPlace or getErrorLocation to report a location.
In the presence of line table debug information, getErrorPlace uses a source
file location and ignores the object file location. However, the object file
location is sometimes more useful.

This patch changes "undefined symbol" and "out of range" diagnostics to report
both object/source file locations. Other diagnostics can use similar format if
needed.

The key idea is to let InputSectionBase::getLocation report the object file
location and use getSrcMsg for source file/line information. getSrcMsg
doesn't leverage STT_FILE information yet, but I think the temporary lack of
the functionality is ok.

For the ARM "branch and link relocation" diagnostic, I arbitrarily place the
source file location at the end of the line. The diagnostic is not very common
so its formatting doesn't need to be pretty.

[peter.smith]

Overall I think the changes are going in the right direction. Just wanted to check that losing the function symbol information in the error messages was intentional?

lld/ELF/Arch/ARM.cpp

390–392

The first 4 lines of the if/else statement bodies look to be the same. Worth moving before the if/else?

lld/ELF/InputSection.cpp

271

I think the comment might be out of date. I think this is an object location now.

lld/test/ELF/ppc64-split-stack-adjust-overflow.s

23 ↗

(On Diff #382208)

I note that we're missing the (function caller) before (.text+0x8). Same in next test. Is this intentional? Although this can be derived from looking at objdump/readelf it would be a shame to lose it.

lld/test/ELF/x86-64-reloc-error2.s

5

It looks like we are no longer reporting the function, only its section. I'm sure the references func is the target of the relocation. If we no longer output the information we should update the comment.

[ayermolo]

Thanks for working on this.
Maybe add a test for debug relocation overflow?

On side note. What do you think of expanding the error reporting to print out a section in to which relocation that overflowed "points to"?

[MaskRay]

add a dedicated test

improve formatting

[MaskRay]

lld/test/ELF/ppc64-split-stack-adjust-overflow.s

23 ↗

(On Diff #382208)

I restored the function func: part. The change is reverted.

lld/test/ELF/x86-64-reloc-error2.s

5

I restored the function func: part. The change is reverted.

[peter.smith]

Thanks for the update, I don't have any more comments. No objections to this landing assuming others are happy.

[ayermolo]

same. thanks

[MaskRay]

Thanks for the comments and the positive responses.
On the related D109400 we've waited for @evgeny777 for quite some time.

I'll push this tomorrow.

lld/ELF/Arch/ARM.cpp

bool ARM::inBranchRange(RelType type, uint64_t src, uint64_t dst) const {
   }
 }
 
 // Helper to produce message text when LLD detects that a CALL relocation to
 // a non STT_FUNC symbol that may result in incorrect interworking between ARM
 // or Thumb.
 static void stateChangeWarning(uint8_t *loc, RelType relt, const Symbol &s) {
   assert(!s.isFunc());
+  const ErrorPlace place = getErrorPlace(loc);
+  std::string hint;
+  if (!place.srcLoc.empty())
+    hint = "; " + place.srcLoc;
   if (s.isSection()) {
     // Section symbols must be defined and in a section. Users cannot change
     // the type. Use the section name as getName() returns an empty string.
-    warn(getErrorLocation(loc) + "branch and link relocation: " +
-         toString(relt) + " to STT_SECTION symbol " +
-         cast<Defined>(s).section->name + " ; interworking not performed");
+    warn(place.loc + "branch and link relocation: " + toString(relt) +
+         " to STT_SECTION symbol " + cast<Defined>(s).section->name +
+         " ; interworking not performed" + hint);
   } else {
     // Warn with hint on how to alter the symbol type.
     warn(getErrorLocation(loc) + "branch and link relocation: " +
          toString(relt) + " to non STT_FUNC symbol: " + s.getName() +
          " interworking not performed; consider using directive '.type " +
          s.getName() +
-         ", %function' to give symbol type STT_FUNC if"
-         " interworking between ARM and Thumb is required");
+         ", %function' to give symbol type STT_FUNC if interworking between "
+         "ARM and Thumb is required" +
+         hint);
   }
 }
 
 // Utility functions taken from ARMAddressingModes.h, only changes are LLD
 // coding style.
 
 // Rotate a 32-bit unsigned value right by a specified amt of bits.
 static uint32_t rotr32(uint32_t val, uint32_t amt) {

lld/ELF/InputSection.cpp

Defined *InputSectionBase::getEnclosingFunction(uint64_t offset) {
   for (Symbol *b : file->getSymbols())
     if (Defined *d = dyn_cast<Defined>(b))
       if (d->section == this && d->type == STT_FUNC && d->value <= offset &&
           offset < d->value + d->size)
         return d;
   return nullptr;
 }
 
-// Returns a source location string. Used to construct an error message.
+// Returns an object file location string. Used to construct an error message.
 template <class ELFT>
 std::string InputSectionBase::getLocation(uint64_t offset) {
-  std::string secAndOffset = (name + "+0x" + utohexstr(offset)).str();
+  std::string secAndOffset =
+      (name + "+0x" + Twine::utohexstr(offset) + ")").str();
 
   // We don't have file for synthetic sections.
   if (getFile<ELFT>() == nullptr)
-    return (config->outputFile + ":(" + secAndOffset + ")")
-        .str();
-
-  // First check if we can get desired values from debugging information.
-  if (Optional<DILineInfo> info = getFile<ELFT>()->getDILineInfo(this, offset))
-    return info->FileName + ":" + std::to_string(info->Line) + ":(" +
-           secAndOffset + ")";
-
-  // File->sourceFile contains STT_FILE symbol that contains a
-  // source file name. If it's missing, we use an object file name.
-  std::string srcFile = std::string(getFile<ELFT>()->sourceFile);
-  if (srcFile.empty())
-    srcFile = toString(file);
+    return (config->outputFile + ":(" + secAndOffset).str();
 
+  std::string file = toString(getFile<ELFT>());
   if (Defined *d = getEnclosingFunction<ELFT>(offset))
-    return srcFile + ":(function " + toString(*d) + ": " + secAndOffset + ")";
+    return file + ":(function " + toString(*d) + ": " + secAndOffset;
 
-  // If there's no symbol, print out the offset in the section.
-  return (srcFile + ":(" + secAndOffset + ")");
+  return file + ":(" + secAndOffset;
 }
 
 // This function is intended to be used for constructing an error message.
 // The returned message looks like this:
 //
 //   foo.c:42 (/home/alice/possibly/very/long/path/foo.c:42)
 //
 //  Returns an empty string if there's no way to get line info.

lld/ELF/Relocations.cpp

static std::string getLocation(InputSectionBase &s, const Symbol &sym,
   return msg + s.getObjMsg(off);
 }
 
 void elf::reportRangeError(uint8_t *loc, const Relocation &rel, const Twine &v,
                            int64_t min, uint64_t max) {
   ErrorPlace errPlace = getErrorPlace(loc);
   std::string hint;
   if (rel.sym && !rel.sym->isLocal())
-    hint = "; references " + lld::toString(*rel.sym) +
-           getDefinedLocation(*rel.sym);
+    hint = "; references " + lld::toString(*rel.sym);
+  if (!errPlace.srcLoc.empty())
+    hint += "\n>>> referenced by " + errPlace.srcLoc;
+  if (rel.sym && !rel.sym->isLocal())
+    hint += getDefinedLocation(*rel.sym);
 
   if (errPlace.isec && errPlace.isec->name.startswith(".debug"))
     hint += "; consider recompiling with -fdebug-types-section to reduce size "
             "of debug sections";
 
   errorOrWarn(errPlace.loc + "relocation " + lld::toString(rel.type) +
               " out of range: " + v.str() + " is not in [" + Twine(min).str() +
               ", " + Twine(max).str() + "]" + hint);

lld/ELF/Target.h

 TargetInfo *getSPARCV9TargetInfo();
 TargetInfo *getX86TargetInfo();
 TargetInfo *getX86_64TargetInfo();
 template <class ELFT> TargetInfo *getMipsTargetInfo();
 
 struct ErrorPlace {
   InputSectionBase *isec;
   std::string loc;
+  std::string srcLoc;
 };
 
 // Returns input section and corresponding source string for the given location.
 ErrorPlace getErrorPlace(const uint8_t *loc);
 
 static inline std::string getErrorLocation(const uint8_t *loc) {
   return getErrorPlace(loc).loc;
 }

lld/ELF/Target.cpp

for (InputSectionBase *d : inputSections) {
     const uint8_t *isecLoc =
         Out::bufferStart
             ? (Out::bufferStart + isec->getParent()->offset + isec->outSecOff)
             : isec->data().data();
     if (isecLoc == nullptr) {
       assert(isa<SyntheticSection>(isec) && "No data but not synthetic?");
       continue;
     }
-    if (isecLoc <= loc && loc < isecLoc + isec->getSize())
-      return {isec, isec->template getLocation<ELFT>(loc - isecLoc) + ": "};
+    if (isecLoc <= loc && loc < isecLoc + isec->getSize()) {
+      auto objLoc = isec->template getLocation<ELFT>(loc - isecLoc);
+      // Return object file location and source file location.
+      // TODO: Refactor getSrcMsg not to take a variable.
+      Undefined dummy(nullptr, "", STB_LOCAL, 0, 0);
+      return {isec, objLoc + ": ",
+              isec->file ? isec->getSrcMsg(dummy, loc - isecLoc) : ""};
+    }
   }
   return {};
 }
 
 ErrorPlace elf::getErrorPlace(const uint8_t *loc) {
   switch (config->ekind) {
   case ELF32LEKind:
     return getErrPlace<ELF32LE>(loc);

lld/test/ELF/arm-thumb-interwork-notfunc.s

  b thumb_func_with_notype
  b thumb_func_with_explicit_notype
  bl .arm_target
  bl arm_func_with_notype
  bl arm_func_with_explicit_notype
  bl .thumb_target
  bl thumb_func_with_notype
  bl thumb_func_with_explicit_notype
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.arm_caller+0x30): branch and link relocation: R_ARM_CALL to STT_SECTION symbol .arm_target ; interworking not performed
+// WARN: {{.*}}.o:(.arm_caller+0x30): branch and link relocation: R_ARM_CALL to STT_SECTION symbol .arm_target ; interworking not performed; {{.*}}.s:[[#@LINE+1]]
  blx .arm_target
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.arm_caller+0x34): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: arm_func_with_notype interworking not performed; consider using directive '.type arm_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+// WARN: {{.*}}.o:(.arm_caller+0x34): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: arm_func_with_notype interworking not performed; consider using directive '.type arm_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  blx arm_func_with_notype
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.arm_caller+0x38): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: arm_func_with_explicit_notype interworking not performed; consider using directive '.type arm_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+// WARN: {{.*}}.o:(.arm_caller+0x38): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: arm_func_with_explicit_notype interworking not performed; consider using directive '.type arm_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  blx arm_func_with_explicit_notype
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.arm_caller+0x3C): branch and link relocation: R_ARM_CALL to STT_SECTION symbol .thumb_target ; interworking not performed
+// WARN: {{.*}}.o:(.arm_caller+0x3c): branch and link relocation: R_ARM_CALL to STT_SECTION symbol .thumb_target ; interworking not performed; {{.*}}.s:[[#@LINE+1]]
  blx .thumb_target
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.arm_caller+0x40): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: thumb_func_with_notype interworking not performed; consider using directive '.type thumb_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+// WARN: {{.*}}.o:(.arm_caller+0x40): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: thumb_func_with_notype interworking not performed; consider using directive '.type thumb_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  blx thumb_func_with_notype
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.arm_caller+0x44): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: thumb_func_with_explicit_notype interworking not performed; consider using directive '.type thumb_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+// WARN: {{.*}}.o:(.arm_caller+0x44): branch and link relocation: R_ARM_CALL to non STT_FUNC symbol: thumb_func_with_explicit_notype interworking not performed; consider using directive '.type thumb_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  blx thumb_func_with_explicit_notype
 
  .section .thumb_caller, "ax", %progbits
  .thumb
  .balign 4
  .global thumb_caller
 thumb_caller:
  b.w .arm_target
  b.w arm_func_with_notype
  b.w arm_func_with_explicit_notype
  b.w .thumb_target
  b.w thumb_func_with_notype
  b.w thumb_func_with_explicit_notype
  beq.w .arm_target
  beq.w arm_func_with_notype
  beq.w arm_func_with_explicit_notype
  beq.w .thumb_target
  beq.w thumb_func_with_notype
  beq.w thumb_func_with_explicit_notype
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.thumb_caller+0x30): branch and link relocation: R_ARM_THM_CALL to STT_SECTION symbol .arm_target ; interworking not performed
+// WARN: {{.*}}.o:(.thumb_caller+0x30): branch and link relocation: R_ARM_THM_CALL to STT_SECTION symbol .arm_target ; interworking not performed; {{.*}}.s:[[#@LINE+1]]
  bl .arm_target
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.thumb_caller+0x34): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: arm_func_with_notype interworking not performed; consider using directive '.type arm_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+// WARN: {{.*}}.o:(.thumb_caller+0x34): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: arm_func_with_notype interworking not performed; consider using directive '.type arm_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  bl arm_func_with_notype
- // WARN: {{.*}}.s:[[# @LINE+1]]:(.thumb_caller+0x38): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: arm_func_with_explicit_notype interworking not performed; consider using directive '.type arm_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+ // WARN: {{.*}}.o:(.thumb_caller+0x38): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: arm_func_with_explicit_notype interworking not performed; consider using directive '.type arm_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  bl arm_func_with_explicit_notype
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.thumb_caller+0x3C): branch and link relocation: R_ARM_THM_CALL to STT_SECTION symbol .thumb_target ; interworking not performed
+// WARN: {{.*}}.o:(.thumb_caller+0x3c): branch and link relocation: R_ARM_THM_CALL to STT_SECTION symbol .thumb_target ; interworking not performed; {{.*}}.s:[[#@LINE+1]]
  bl .thumb_target
-// WARN: {{.*}}.s:[[# @LINE+1]]:(.thumb_caller+0x40): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: thumb_func_with_notype interworking not performed; consider using directive '.type thumb_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+// WARN: {{.*}}.o:(.thumb_caller+0x40): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: thumb_func_with_notype interworking not performed; consider using directive '.type thumb_func_with_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  bl thumb_func_with_notype
-// {{.*}}.s:[[# @LINE+1]]:(.thumb_caller+0x44): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: thumb_func_with_explicit_notype interworking not performed; consider using directive '.type thumb_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required
+// {{.*}}.o:(.thumb_caller+0x44): branch and link relocation: R_ARM_THM_CALL to non STT_FUNC symbol: thumb_func_with_explicit_notype interworking not performed; consider using directive '.type thumb_func_with_explicit_notype, %function' to give symbol type STT_FUNC if interworking between ARM and Thumb is required; {{.*}}.s:[[#@LINE+1]]
  bl thumb_func_with_explicit_notype
  blx .arm_target
  blx arm_func_with_notype
  blx arm_func_with_explicit_notype
  blx .thumb_target
  blx thumb_func_with_notype
  blx thumb_func_with_explicit_notype
 

lld/test/ELF/ppc64-error-toc-local-call.s

 # REQUIRES: ppc
 # RUN: llvm-mc -filetype=obj -triple=powerpc64le %s -o %t.o
 # RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
 
 # RUN: llvm-mc -filetype=obj -triple=powerpc64 %s -o %t.o
 # RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
 
 ## This test checks that the linker produces errors when it is missing the nop
 ## after a local call to a callee with st_other=1.
 
-# CHECK: (.text+0xC): call to save_callee lacks nop, can't restore toc
-# CHECK: (.text+0x1C): call to save_callee lacks nop, can't restore toc
+# CHECK:      {{.*}}.o:(.text+0xc): call to save_callee lacks nop, can't restore toc
+# CHECK-NEXT: {{.*}}.o:(.text+0x1c): call to save_callee lacks nop, can't restore toc
 
 callee:
   .localentry callee, 1
   blr                                                # 0x0
 
 caller:
 .Lfunc_gep1:
   addis 2, 12, .TOC.-.Lfunc_gep1@ha

lld/test/ELF/x86-64-reloc-error2.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: not ld.lld --entry=func --gc-sections %t.o -o /dev/null 2>&1 | FileCheck %s
 
 ## Check we are able to find a function symbol that encloses
 ## a given location when reporting error messages.
 # CHECK: {{.*}}.o:(function func: .text.func+0x3): relocation R_X86_64_32S out of range: -281474974609120 is not in [-2147483648, 2147483647]; references func
 # CHECK-NEXT: >>> defined in {{.*}}.o
+# CHECK-EMPTY:
 
 # This mergeable section will be garbage collected. We had a crash issue in that case. Test it.
 .section .rodata.str1,"aMS",@progbits,1
 .asciz "a"
 
 .section .text.func, "ax", %progbits
 .globl func
 .type func,@function
 func:
   movq $func - 0x1000000000000, %rdx
 .size func, .-func

lld/test/ELF/x86-64-reloc-range-debug-loc.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 %s -o %t.o -triple x86_64-pc-linux -filetype=obj
 # RUN: not ld.lld %tabs %t.o -o /dev/null -shared 2>&1 | FileCheck %s
 
+# RUN: rm -f %t.a && llvm-ar rc %t.a %t.o
+# RUN: not ld.lld -shared %t.a %tabs -o /dev/null 2>&1 | FileCheck %s --check-prefix=CHECK2
+
 ## Check we are able to report file and location from debug information
 ## when reporting such kind of errors.
-# CHECK: error: test.s:3:(.text+0x1): relocation R_X86_64_32 out of range: 68719476736 is not in [0, 4294967295]
+# CHECK:      error: {{.*}}.o:(.text+0x1): relocation R_X86_64_32 out of range: 68719476736 is not in [0, 4294967295]; references big
+# CHECK-NEXT: >>> referenced by test.s:3
+# CHECK-NEXT: >>> defined in {{.*}}abs
+
+# CHECK2:      error: {{.*}}.a({{.*}}.o):(.text+0x1): relocation R_X86_64_32 out of range: 68719476736 is not in [0, 4294967295]; references big
+# CHECK2-NEXT: >>> referenced by test.s:3
+# CHECK2-NEXT: >>> defined in {{.*}}abs
 
 .section .text,"ax",@progbits
-foo:
+.globl _start
+_start:
 .file 1 "test.s"
 .loc 1 3
  movl $big, %edx
 
 .section .debug_abbrev,"",@progbits
 .byte 1                       # Abbreviation Code
 .byte 17                      # DW_TAG_compile_unit
 .byte 0                       # DW_CHILDREN_no