This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Rework debug line parsing to use llvm::Error (LLD-side)
AbandonedPublic

Authored by jhenderson on Mar 12 2018, 8:26 AM.

Details

Summary

This is a companion change to D44382. That change changes the debug line parser interface to report LLVM errors, and to improve the error/warning reporting mechanism to not use fprintf(stderr, ...).

As noted in the description on D44382, these warnings were not using the errs() stream, and so were not always being flushed properly by LLD on shutdown. To test this, I have extended the bad-debug undefined symbol message case to show that a corresponding warning is printed, if the debug line cannot be parsed.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

jhenderson created this revision.Mar 12 2018, 8:26 AM

(I will say "LTOrErr" is a slightly confusing variable name, given the
prevalence of "LTO" as a known acronym... )

ruiu added inline comments.Mar 14 2018, 2:03 PM
ELF/InputFiles.cpp
124–135

I agree with David; LTOrError is a confusing variable name.

Can you avoid using auto here?

jhenderson added inline comments.Mar 15 2018, 3:39 AM
ELF/InputFiles.cpp
124–135

It's also wrong, since it could be a LineTable and an Error! I'll fix that shortly (note: this may undergo some changes, as there's some discussion on the main review about the appropriate way of reporting the different levels of error).

Fixed use of auto and variable name. Also fixed a bug which I'd accidentally introduced - my changes to getOrParseLineTable meant that it reported an error, if an invalid offset was specified. This meant that an object with an empty or missing .debug_line section would result in a warning printed from this function whenever LLD tried to look up the debug information for reporting its own message. I've added a couple of tests to check this going forwards.

jhenderson abandoned this revision.Mar 19 2018, 2:26 AM

I've abandoned D44382, as I prefer its alternative D44560, so I'm also abandoning this one. See D44562 for the alternative.