This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Change way how we handle --noinhibit-exec
ClosedPublic

Authored by grimar on Jul 24 2017, 4:12 AM.

Details

Summary

It is splitted from D35724, which uses this change,

Previously we handled this option implicitly, only for infering
unresolved symbols handling policy.
ld man says: "--noinhibit-exec Retain the executable output file whenever
it is still usable", we we may want to handle other cases too.

Diff Detail

Event Timeline

grimar created this revision.Jul 24 2017, 4:12 AM
ruiu added inline comments.Jul 24 2017, 10:05 AM
ELF/Relocations.cpp
915

This is the only place you are using errorOrWarn, so don't add that function just for -noinbhiti-exec.

ruiu edited edge metadata.Jul 24 2017, 3:28 PM

I think preventing almost all errors is too much. Most errors are still checked even if --noinhibit-exec is specified. For example, if you give nonexistent file to the linker, it is still an error.

grimar updated this revision to Diff 108022.Jul 25 2017, 2:15 AM
  • Addressed comments.
ELF/Relocations.cpp
915

Added more places using errorOrWarn

grimar edited the summary of this revision. (Show Details)Jul 25 2017, 2:16 AM
ruiu accepted this revision.Jul 25 2017, 11:24 AM

Yes, this looks fine. LGTM

ELF/Driver.cpp
646–648

Sort

This revision is now accepted and ready to land.Jul 25 2017, 11:24 AM
grimar added inline comments.Jul 26 2017, 2:48 AM
ELF/Driver.cpp
646–648

Done (I hope I got sorting rule correctly now).
But isn't it easier/more usefu(for search) just to ignore character upper/lower cases
when sorting this list ?

Not sure it makes sense to have current:

Config->NoGnuUnique = Args.hasArg(OPT_no_gnu_unique);
Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);
Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
Config->Nostdlib = Args.hasArg(OPT_nostdlib);

instead of straightforward:

Config->NoGnuUnique = Args.hasArg(OPT_no_gnu_unique);
Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
Config->Nostdlib = Args.hasArg(OPT_nostdlib);
Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);
This revision was automatically updated to reflect the committed changes.
ruiu added inline comments.Jul 26 2017, 10:20 AM
lld/trunk/ELF/Config.h
131 ↗(On Diff #108242)

Sort. In general, please sort in ASCIIbetical order unless there's other local rule there.