This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Linkerscript: set correct synamic tag entries values when LS is used.
ClosedPublic

Authored by grimar on Aug 18 2016, 8:18 AM.

Details

Summary

Previously DT_PREINIT_ARRAYSZ, DT_INIT_ARRAYSZ and DT_FINI_ARRAYSZ
were set to zero when lincerscript was used becase sections sizes are
calculated later that were taken.

Patch delays values calculation for these entries. Testcase is provided.

Diff Detail

Event Timeline

grimar updated this revision to Diff 68538.Aug 18 2016, 8:18 AM
grimar retitled this revision from to [ELF] - Linkerscript: set correct synamic tag entries values when LS is used..
grimar updated this object.
grimar added a reviewer: ruiu.
grimar added subscribers: llvm-commits, grimar, davide, evgeny777.
evgeny777 added inline comments.Aug 18 2016, 8:34 AM
ELF/OutputSections.cpp
755

Can we use OutSec field from Entry?

case Entry::SecSize:
      P->d_un.d_val = E.OutSec->getSize();
      break;
emaste added a subscriber: emaste.Aug 18 2016, 8:49 AM
grimar added inline comments.Aug 18 2016, 8:54 AM
ELF/OutputSections.cpp
755

OutSec field is not set, see:

Entry(int32_t Tag) : Tag(Tag), Kind(SecSize) {}

Problem is that constructor that takes
output section is already busy with SecAddr:

Entry(int32_t Tag, OutputSectionBase<ELFT> *OutSec)
    : Tag(Tag), OutSec(OutSec), Kind(SecAddr) {}

I`ll check what can I do here.

evgeny777 added inline comments.Aug 18 2016, 8:56 AM
ELF/OutputSections.cpp
755

May be just add extra parameter (Kind) ?

grimar updated this revision to Diff 68550.Aug 18 2016, 8:59 AM
  • Addressed review comments.
ruiu added inline comments.Aug 18 2016, 12:29 PM
ELF/OutputSections.h
591–596

You have added three new constructors (a constructor with an optional parameter actually defines two ctors.) It's probably too much. Please remove the default argument at least.

594

Why do you need this?

grimar updated this revision to Diff 68662.Aug 19 2016, 1:59 AM
  • Addressed review comments.
ELF/OutputSections.h
594

Oops sorry, I forgot to remove it after addressing comments for first diff. I do not need it, what I was need is first constructor with default argument. I know you don't like to use them, though in this case it seems nicer for me because separates sections size taking case (which is special) from other ones. And also it brings much less changes.

You can see how version without default argument looks like: D23710. I would prever version with default argument (this patch). What do you think ?

ruiu accepted this revision.Aug 19 2016, 8:01 AM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Aug 19 2016, 8:01 AM
This revision was automatically updated to reflect the committed changes.