This is an archive of the discontinued LLVM Phabricator instance.

[LLD] [COFF] Link crtend.o as the last object file
ClosedPublic

Authored by mstorsjo on Apr 12 2019, 12:08 PM.

Details

Summary

When faced with command line options such as "crtbegin.o appmain.o -lsomelib crtend.o", GNU ld pulls in all necessary object files from somelib before proceeding to crtend.o.

LLD operates differently, only loading object files from any referenced static libraries after processing all input object files.

This uses a similar hack as in the ELF linker. Here, it moves crtend.o to the end of the vector of object files. This makes sure that terminator chunks for sections such as .eh_frame gets ordered last, fixing DWARF exception handling for libgcc and gcc's crtend.o.

I copied the function for identifying the crtend object file from the ELF linker - should that maybe be moved to some shared file?

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Apr 12 2019, 12:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 12 2019, 12:08 PM
Herald added a subscriber: aprantl. · View Herald Transcript
ruiu added a comment.Apr 14 2019, 11:00 PM

This patches changes the order how input files are parsed. The other approach is to parse input files normally and then sort the list of object files so that crtend.o is at the end of the input list. I wonder if the latter is simpler. What do you think?

This patches changes the order how input files are parsed. The other approach is to parse input files normally and then sort the list of object files so that crtend.o is at the end of the input list. I wonder if the latter is simpler. What do you think?

I initially thought about something like that (although I mostly thought about sorting SectionChunks even later in the pipeline). My main concern was that one would end up doing a huge std::stable_sort over O(N) entries, when all I want to do is move one single entry to the end.

But I can give that a try, out of curiosity.

ruiu added a comment.Apr 14 2019, 11:34 PM

I don't think you have to call std::stable_sort. Something like this should suffice.

for (size_t I = 0, E = InputFiles.size(); I != E; ++I) {
  InputFile *File = InputFiles[I];
  if (File.getName() != "crtend.o")
    continue;
  InputFiles.erase(i);
  InputFiles.push_back(File);
  break;
}

I don't think you have to call std::stable_sort. Something like this should suffice.

for (size_t I = 0, E = InputFiles.size(); I != E; ++I) {
  InputFile *File = InputFiles[I];
  if (File.getName() != "crtend.o")
    continue;
  InputFiles.erase(i);
  InputFiles.push_back(File);
  break;
}

Thanks, that actually is very sensible and straightforward.

mstorsjo updated this revision to Diff 195098.Apr 15 2019, 12:37 AM
mstorsjo edited the summary of this revision. (Show Details)

Reordering ObjFile::Instances instead of changing the order of parsing files, as suggested by Rui.

ruiu added inline comments.Apr 15 2019, 1:30 AM
COFF/Driver.cpp
95 ↗(On Diff #195098)

Since Filename is always "crtend", I'd hardcode that and merge this function with isCrtend.

1685 ↗(On Diff #195098)

Could you expand this comment a little to explain why that object file should be at the end of the input file list?

mstorsjo updated this revision to Diff 195105.Apr 15 2019, 1:48 AM
mstorsjo marked 2 inline comments as done.

Adjusted according to Rui's comments.

ruiu accepted this revision.Apr 15 2019, 2:16 AM

LGTM

This revision is now accepted and ready to land.Apr 15 2019, 2:16 AM
This revision was automatically updated to reflect the committed changes.