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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

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?



================
Comment at: lld/ELF/Arch/ARM.cpp:386
     // 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");
+    const ErrorPlace place = getErrorPlace(loc);
+    std::string hint;
----------------
The first 4 lines of the if/else statement bodies look to be the same. Worth moving before the if/else?


================
Comment at: lld/ELF/InputSection.cpp:271
 
 // Returns a source location string. Used to construct an error message.
 template <class ELFT>
----------------
I think the comment might be out of date. I think this is an object location now.


================
Comment at: lld/test/ELF/ppc64-split-stack-adjust-overflow.s:23
 
-# OVERFLOW: error: {{.*}}.o:(function caller: .text+0x8): split-stack prologue adjustment overflows
+# OVERFLOW: error: {{.*}}.o:(.text+0x8): split-stack prologue adjustment overflows
 
----------------
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. 


================
Comment at: lld/test/ELF/x86-64-reloc-error2.s:5
 
 ## Check we are able to find a function symbol that encloses
 ## a given location when reporting error messages.
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112518