Page MenuHomePhabricator

[ELF] - Define special symbols _etext and _edata
ClosedPublic

Authored by grimar on Feb 25 2016, 4:29 AM.

Details

Summary

Description of symbols is avalable here:
https://docs.oracle.com/cd/E53394_01/html/E54766/u-etext-3c.html

It is said that:
_etext - The address of _etext is the first location after the last read-only loadable segment.
_edata - The address of _edata is the first location after the last read-write loadable segment.
_end - If the address of _edata is greater than the address of _etext, the address of _end is same as the address of _edata.

In real life _end and _edata has different values for that case.
Both gold/bfd set _edata to the end of the last non SHT_NOBITS section.
This patch do the same for consistency.

It should fix the https://llvm.org/bugs/show_bug.cgi?id=26729.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 49032.Feb 25 2016, 4:29 AM
grimar retitled this revision from to [ELF] - Define special symbols _etext and _edata.
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar.
grimar updated this object.Feb 25 2016, 4:32 AM
rafael added inline comments.Feb 25 2016, 6:50 AM
test/ELF/edata-etext.s
4 ↗(On Diff #49032)

You are not running FileCheck.

rafael added inline comments.Feb 25 2016, 6:59 AM
test/ELF/edata-etext.s
79 ↗(On Diff #49032)

I don't this this is correct. In this case they are the same because there is no padding before .bss.

To see the difference, add a

.align 4

after the .bss and before the .space in this test.

Also, is .bss really special? My guess would be that _edata actually points to the end of the last non NOBITS section.

grimar updated this revision to Diff 49065.Feb 25 2016, 7:58 AM
grimar marked 2 inline comments as done.

Addressed Rafael's comments.

test/ELF/edata-etext.s
4 ↗(On Diff #49032)

That keeps my nerves healthy..
Fixed, thanks.

79 ↗(On Diff #49032)

That seems to be true. I was confused by output results which showed equality to .bss address in addition to such line of gold code:

{
  "_edata",			// name
  cgccpp::PT_LOAD,		// segment_type
  cgccpp::PF_W,		// segment_flags_set
  cgccpp::PF(0),		// segment_flags_clear
  0,				// value
  0,				// size
  cgccpp::STT_NOTYPE,		// type
  cgccpp::STB_GLOBAL,		// binding
  cgccpp::STV_DEFAULT,	// visibility
  0,				// nonvis
  Symbol::SEGMENT_BSS,	// offset_from_base <-------------------------------
  false			// only_if_ref
},

Fixed.

ruiu added inline comments.Feb 25 2016, 9:07 AM
ELF/Writer.cpp
906 ↗(On Diff #49065)

Local variable names need to start with a uppercase letter. Maybe just Define should suffice.

914–921 ↗(On Diff #49065)

Need to update this comment.

1357 ↗(On Diff #49065)

Can you define the variables at once here?

bool Alloc = Sec->getFlags() & SHF_ALLOC;
bool NoBits = Sec->getFlags() & SHF_NOBITS;
1379–1382 ↗(On Diff #49065)

Both symbols get the same address. Is this correct?

Is there any way to move this piece of code to fixAbsoluteSymbols? I don't want to add more code to this function.

grimar updated this revision to Diff 49162.Feb 26 2016, 1:11 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
ELF/Writer.cpp
906 ↗(On Diff #49065)

Done.

914–921 ↗(On Diff #49065)

Done.

1357 ↗(On Diff #49065)

That code is not needed anymore as I moved it to
fixAbsoluteSymbols() after your hint, but
(just in case it will be returned back):

HasBits IMO better than NoBits:

bool HasBits = Sec->getType() != SHT_NOBITS;

because double negation not looking good:

if (!NoBits)
  FileOff = alignTo(FileOff, Align);
Sec->setFileOffset(FileOff);
if (!NoBits)
  FileOff += Sec->getSize();
1379–1382 ↗(On Diff #49065)

Yes it is expected code here. Finally they will get different address. You can see that _etext and _edata are different in testcase.
Values for them will be updated on each iteration and stop updating at different moments.

// _etext points to location after the last read-only loadable segment.
// _edata points to the end of the last non SHT_NOBITS section.

moved that code to fixAbsoluteSymbols.

ruiu accepted this revision.Feb 26 2016, 5:17 AM
ruiu edited edge metadata.

LGTM

ELF/Writer.cpp
1453 ↗(On Diff #49162)
if (Sec->getFlags() & SHF_ALLOC)
  continue;
1454–1455 ↗(On Diff #49162)

Move this comment before this for loop.

This revision is now accepted and ready to land.Feb 26 2016, 5:17 AM
grimar updated this object.Feb 26 2016, 6:34 AM
grimar edited edge metadata.
This revision was automatically updated to reflect the committed changes.
grimar marked 2 inline comments as done.