[PATCH] D38721: [ELF] - Teach LLD to report line numbers for data symbols.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/InputSection.cpp:292
+  // which describes variables and their locations.
+  if (Sym.Type == STT_OBJECT) {
+    if (Optional<std::pair<std::string, unsigned>> FileLine =
----------------
grimar wrote:
> ruiu wrote:
> > Are you sure that functions would never have type STT_OBJECT?
> It's interesting. Specification says functions and other executable code should be of type STT_FUNC:
> http://www.sco.com/developers/gabi/2000-07-17/ch4.symtab.html
> So at least it does not seem correct.
> 
> At the same time it looks possible to do that:
> ```
> .section .text.fn,"ax", at progbits,unique,0
> .globl fn
> .type fn, at object
> fn:
>  nop
> 
> .section .text.fn2,"ax", at progbits,unique,0
> .globl fn2
> .type fn2, at function
> fn2:
>  callq fn at PLT
> ```
> 
> And you'll get
> ```
>      4: 0000000000400079     0 FUNC    GLOBAL DEFAULT    1 fn2
>      7: 0000000000400078     0 OBJECT  GLOBAL DEFAULT    1 fn
> ```
> 
> We probably can fallback to always use .debug_line:
> ```
>   if (Sym.Type == STT_OBJECT)
>     if (Optional<std::pair<std::string, unsigned>> FileLine =
>             File->getVariableLoc(Sym.getName()))
>       return createFileLineMsg(FileLine->first, FileLine->second);
> 
>   if (Optional<DILineInfo> Info = File->getDILineInfo(this, Offset))
>     return createFileLineMsg(Info->FileName, Info->Line);
> ```
> 
> At least such behavior does not seem harmful for me. 
> Do you think we should ? (I am not sure because with assembly it is possible
> to do many things, and particulary this one seems violates ELF spec, not sure we
> should even think about supporting that until it is reported useful).
> 
> 
Well, I think what I was trying to say is that we don't need to think about it too deeply. Why don't you just attempt both? First, try to get line info as a function, and if it fails, try as a variable.


https://reviews.llvm.org/D38721