This is an archive of the discontinued LLVM Phabricator instance.

[ELF2] - Implemented R_X86_64_GOTTPOFF relocation
ClosedPublic

Authored by grimar on Nov 12 2015, 11:01 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 40070.Nov 12 2015, 11:01 AM
grimar retitled this revision from to [ELF2] - Implemented R_X86_64_GOTTPOFF relocation.
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar.
ruiu accepted this revision.Nov 12 2015, 11:30 AM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Nov 12 2015, 11:30 AM
grimar updated this revision to Diff 40123.Nov 13 2015, 2:17 AM
grimar edited edge metadata.

I was need to upgrade it because of code changes in r252979. Could this please be reviewed again ?

rafael accepted this revision.Nov 13 2015, 6:39 AM
rafael edited edge metadata.

LGTM with a nit.

ELF/InputSection.cpp
143 ↗(On Diff #40123)

Maybe just pass body to getGotReloc?

grimar added inline comments.Nov 13 2015, 6:44 AM
ELF/InputSection.cpp
143 ↗(On Diff #40123)

Target->getGotReloc(Body) ?

Maybe just pass body to getGotReloc?

Target->getGotReloc(Body) ?

Yes.

Cheers,
Rafael

Maybe just pass body to getGotReloc?

Target->getGotReloc(Body) ?

Yes.

Cheers,
Rafael

It looks good but let me think about that a bit later please.

Sure. A second patch is fine.

This revision was automatically updated to reflect the committed changes.

LGTM with a nit.
Maybe just pass body to getGotReloc?

Problem here is that currently there are two places in code that looks very similiar:

1) Type = Body.isTLS() ? Target->getTlsGotReloc() : Target->getGotRefReloc();
...
2) unsigned GotReloc =
  Body->isTLS() ? Target->getTlsGotReloc() : Target->getGotReloc();

But getGotRefReloc() and getGotReloc() are different calls. And it seems I cant just use the body argument to know which one relocation return for them in that places.

Problem here is that currently there are two places in code that looks very similiar:

  1. Type = Body.isTLS() ? Target->getTlsGotReloc() : Target->getGotRefReloc(); ...
  2. unsigned GotReloc = Body->isTLS() ? Target->getTlsGotReloc() : Target->getGotReloc();

But getGotRefReloc() and getGotReloc() are different calls. And it seems I cant just use the body argument to know which one relocation return for them in that places.

Fair enough.