This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Don't set the executable bit for relocatable or shared files
ClosedPublic

Authored by phosek on Dec 8 2017, 4:52 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Dec 8 2017, 4:52 PM
ruiu edited edge metadata.Dec 8 2017, 4:59 PM

So is true for .so, isn't it?

phosek updated this revision to Diff 126253.Dec 8 2017, 5:46 PM
phosek retitled this revision from [ELF] Don't set the executable bit for relocatable files to [ELF] Don't set the executable bit for relocatable or shared files.
phosek edited the summary of this revision. (Show Details)

True, that's a good point.

grimar added a subscriber: grimar.Dec 11 2017, 3:44 AM

+1 to testcase request.

ELF/Writer.cpp
1843 ↗(On Diff #126253)

I believe LLD style avoids using ternary operator where possible.
So I see this code better as:

unsigned Flags = FileOutputBuffer::F_executable;
if (Config->Relocatable || Config->Shared)
  Flags = 0;
ruiu added inline comments.Dec 11 2017, 9:20 AM
ELF/Writer.cpp
1843 ↗(On Diff #126253)

Yeah, sort of. I learned this style from Go. I'd prefer defaulting to 0 and re-setting F_executable though.

phosek updated this revision to Diff 126445.Dec 11 2017, 2:22 PM
phosek marked 2 inline comments as done.

Test added.

ruiu accepted this revision.Dec 11 2017, 2:32 PM

LGTM with this fix.

ELF/Writer.cpp
1837 ↗(On Diff #126445)

Please remove !Config->Shared as other linkers set X bit to shared libraries.

This revision is now accepted and ready to land.Dec 11 2017, 2:32 PM
phosek updated this revision to Diff 126458.Dec 11 2017, 3:16 PM
phosek marked an inline comment as done.
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.