Page MenuHomePhabricator

[ELF] - Teach LLD to use information from .debug_str for error reporting.
ClosedPublic

Authored by grimar on Nov 2 2017, 3:47 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Nov 2 2017, 3:47 AM
ruiu accepted this revision.Nov 2 2017, 12:39 PM

LGTM but please update the commit message before committing.

This revision is now accepted and ready to land.Nov 2 2017, 12:39 PM
This revision was automatically updated to reflect the committed changes.
grimar edited the summary of this revision. (Show Details)Nov 3 2017, 1:06 AM
grimar reopened this revision.Nov 4 2017, 2:23 AM

It was reverted in r317378 because of https://bugs.llvm.org/show_bug.cgi?id=35199.

Something looks just broken in libDebugInfo though not sure this patch is OK, so reopening it for now.
I'll be on vacations next week, and will be able to investigate the issue after 13 nov (if nobody fix it earlier).

This revision is now accepted and ready to land.Nov 4 2017, 2:23 AM
grimar planned changes to this revision.Nov 4 2017, 2:23 AM
grimar updated this revision to Diff 123137.EditedNov 16 2017, 2:05 AM
grimar added subscribers: dblaikie, aprantl.

Reason of crash was that DWARF parsers performs an attempt to parse address ranges
from .dwo files. Previously when no .debug_str was provided by LLD, it failed to do that
and things just worked. After this patch started to provide ".debug_str", code of DWARF
parsers attempts to call DWARFObject::getFileName() to open .dwo and method was not
implemented in LLD.

I don't know why DWARF parser might want to collect address ranges from .dwo files and
wrote a mail "Collecting address ranges in DWARFUnit::collectAddressRanges." to llvm-dev with question
about that. It seems excessive logic, at least it is not covered by any testcases I found.

In this patch I suggest to implement stub returning empty string for now. That way nothing
changes for LLD, we will return empty file name and DWARF parser will fail to parse .dwo file
for collecting address ranges. But at the same time we will not crash and so can land this patch.

Added testcase which crashed with llvm_unreachable at
https://github.com/llvm-mirror/llvm/blob/master/lib/DebugInfo/DWARF/DWARFUnit.cpp#L335
in previous iteration.

This revision is now accepted and ready to land.Nov 16 2017, 2:05 AM
grimar requested review of this revision.Nov 16 2017, 2:05 AM
grimar edited edge metadata.
grimar updated this revision to Diff 123143.Nov 16 2017, 2:23 AM
ruiu accepted this revision.Nov 16 2017, 5:29 PM

LGTM

This revision is now accepted and ready to land.Nov 16 2017, 5:29 PM
This revision was automatically updated to reflect the committed changes.