Page MenuHomePhabricator

Error instead of allocating a header bellow address 0
ClosedPublic

Authored by espindola on Feb 26 2018, 6:36 PM.

Details

Summary

With the current code if the script has a PHDRS we always obey and try to allocate a header. This can cause Min - HeaderSize to underflow.

It looks like bfd actually prints an error for this case. With this patch we do the same.

Found while looking at pr36515

Diff Detail

Event Timeline

espindola created this revision.Feb 26 2018, 6:36 PM
ruiu added a comment.Feb 26 2018, 7:12 PM

If you can detect an underflow (or actually an overflow towards negative infinity), how hard is it to report it as an error, instead of not emitting a header without any message?

jhenderson added inline comments.Feb 27 2018, 1:36 AM
test/ELF/linkerscript/header-phdr2.s
4–5

I think it's been mentioned before that it's better to use multiple echoes and ">>" appending, to get line breaks in the script, making it easier to debug with.

This looks fine for me.

espindola updated this revision to Diff 136086.Feb 27 2018, 8:51 AM
espindola retitled this revision from Don't allocate a header bellow address 0 to Error instead of allocating a header bellow address 0.
espindola edited the summary of this revision. (Show Details)

This now matches bfd and produces an error.

grimar added inline comments.Feb 28 2018, 1:07 AM
ELF/LinkerScript.cpp
921

Doesn't seem you need else if here, it could be:

if (Script->AllocateHeaders)
  error("Could not allocate headers");

Out::ElfHeader->PtLoad = nullptr;
Out::ProgramHeaders->PtLoad = nullptr;
...
ELF/LinkerScript.h
277 ↗(On Diff #136086)

I think you can avoid introducing this flag and
just iterate over Script->PhdrsCommands to find
Cmd.HasPhdrs || Cmd.HasFilehdr instead ?

No else after return.

Use any_of instead of a new member variable.

grimar accepted this revision.Mar 1 2018, 5:19 AM

This LGTM with few nits.

ELF/LinkerScript.cpp
909

Allocateheaders -> AllocateHeaders ?
Or may be HasExplicitHeaders would be better.

923

Could -> could.

This revision is now accepted and ready to land.Mar 1 2018, 5:19 AM
espindola accepted this revision.Mar 1 2018, 7:30 AM

r326441