[PATCH] D56453: Modify InputSectionBase::getLocation to add section and offset to every location string.

[Sean Fertile via Phabricator via llvm-commits]

sfertile added inline comments.


================
Comment at: ELF/InputSection.cpp:310
   if (Defined *D = getEnclosingFunction<ELFT>(Offset))
-    return SrcFile + ":(function " + toString(*D) + ")";
+    return SrcFile + ":(function " + toString(*D) + ": " + SecAndOffset + ")";
 
----------------
ruiu wrote:
> I think that the idea here is that a function name and an object file name uniquely identifies an error location. A section name and an offset is the last resort if there's no other information we can display.
> 
> Did you find it useful to print out a section name and an offset in addition to function name? If so, in what situation?
> 
The motivation for this change comes from a range-check error message I get from a compiler generated file. Since the compiler adds the filename we get a  message that look roughly like this:
` test.c:(function  foo): relocation R_PPC64_ADDR16_DS out of range: 32808 is not in [-32768, 32767]` 

The problem is that foo could potentially have dozens of `R_PPC64_TOC16_DS` relocations. By adding the section and offset I can pinpoint exactly which relocation is overflowing/underflowing immediately. The same benefits holds for the `checkAlignment` diagnostics.

I agree for some of the error messages the extra info holds no value. For example the diagnostic for split-stack prologue adjust failure is completely characterized by the file and the function. I think I would prefer to keep the extra info even in this case so that the diagnostics are consistent and we don't need to be able to generate 2 slightly different formattings but I don't feel to strongly either way.




Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56453/new/

https://reviews.llvm.org/D56453