This is an archive of the discontinued LLVM Phabricator instance.

[mac/lld] Include archive name in diagnostics
ClosedPublic

Authored by thakis on Dec 1 2020, 4:32 PM.

Details

Summary

Also, for .o files, include full path as given on link command line.

Before:

lld: error: undefined symbol [...], referenced from sandbox_logging.o

After:

lld: error: undefined symbol [...], referenced from libseatbelt.a(sandbox_logging.o)

Move archiveName up to InputFile so we can consistently use toString()
to print InputFiles in diags, and pass it to the ObjFile ctor. This
matches the ELF and COFF ports.

Diff Detail

Event Timeline

thakis created this revision.Dec 1 2020, 4:32 PM
thakis requested review of this revision.Dec 1 2020, 4:32 PM

Lots of merge conflicts... :)

lld/MachO/InputFiles.cpp
419

Since the arg here is called err, was this supposed to call error() instead of warn()?

MaskRay added a subscriber: MaskRay.Dec 1 2020, 4:39 PM

Reasonable

lld/MachO/InputFiles.h
72

Suggest: "If not empty, this stores the name of the archive containing this file. ..."

thakis updated this revision to Diff 308825.Dec 1 2020, 5:34 PM
thakis marked an inline comment as done.

tweak comment

thakis added a comment.Dec 1 2020, 5:35 PM
Thanks!
lld/MachO/InputFiles.cpp
419

Asked this on D89257 like I should have in the first place.

MaskRay accepted this revision as: MaskRay.Dec 1 2020, 5:52 PM

Thanks! This is what lld/ELF does...

This revision is now accepted and ready to land.Dec 1 2020, 5:52 PM
int3 added a subscriber: int3.Dec 1 2020, 7:29 PM

Thanks! I was actually thinking about this missing feature while implementing D91318: [lld-macho] Add archive name and file modtime to STABS output...

lld/MachO/Driver.cpp
284–287

nit: If we're going to add braces, I'd prefer we add them around all 3 levels of nesting

lld/MachO/InputFiles.h
74

why std::string over StringRef?

thakis updated this revision to Diff 308855.Dec 1 2020, 7:51 PM
thakis marked an inline comment as done.

more braces

Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2020, 7:51 PM
thakis added inline comments.Dec 1 2020, 7:51 PM
lld/MachO/InputFiles.h
74

Because it's a potential foot gun: addFile() sets this to path, which is a StringRef param and there's no guarantee that that stays alive. In fact, OPT_l calls findLibrary() which seems to return a temp string and pass that in. Since there are relatively few InputFiles, might as well not have a footgun here.

thakis updated this revision to Diff 308856.Dec 1 2020, 7:52 PM

fix rebase weirdness

int3 accepted this revision.Dec 1 2020, 7:56 PM

sgtm

lld/MachO/InputFiles.h
74

bang bang yeah good point

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2020, 8:03 PM