This is an archive of the discontinued LLVM Phabricator instance.

Add support for GOT relocation support to Hexagon
ClosedPublic

Authored by sidneym on Oct 1 2018, 1:32 PM.

Details

Summary

Add support for R_HEX_GOT_11_X/R_HEX_GOT_32_6_X.
Create a new relocation expression, HEXAGON_GOT to handle the got offset calculation.

Diff Detail

Repository
rL LLVM

Event Timeline

sidneym created this revision.Oct 1 2018, 1:32 PM
ruiu added inline comments.Oct 1 2018, 1:49 PM
ELF/InputSection.cpp
549 ↗(On Diff #167815)

You are not using A nor P, so this expression is a constant. Is this code correct?

sidneym added inline comments.Oct 2 2018, 8:05 AM
ELF/InputSection.cpp
549 ↗(On Diff #167815)

The ABI describes the added relocations (type G) as the, "offset into global offset table for the entry of a symbol". Access to the symbol starts first by getting the base of the global offset table,
r2=add(pc,_GLOBAL_OFFSET_TABLE_@PCREL) Then the entry: r0 = memw (r2+bar@GOT)

The immediate ##bar@GOT is calculated as Sym.getGotVA() - In.GotPLT->getVA(). I checked this with some existing hexagon code and everything seemed ok.

ruiu added a comment.Oct 2 2018, 8:18 AM

Can you share the pointer to the ABI documentation?

This is the link:
https://developer.qualcomm.com/download/hexagon/hexagon-application-binary-interface-specification.zip

There is a click-thru sign-in that is required to get to the document. I could not find a copy that was otherwise accessible.

ruiu added a comment.Oct 2 2018, 10:05 AM

I need to contact a legal team before downloading the PDF since it requires me to agree with a Terms of Use to download it. I'll get back to you.

grimar added a subscriber: grimar.Oct 3 2018, 2:07 AM
ruiu accepted this revision.Oct 3 2018, 9:58 AM

OK, so looks like I cannot agree with the Terms of Use to download the spec. The exact words of the Terms of Use doesn't matter (though I'm not a lawyer), but in general, you cannot require a maintainer to sign a random legal document only to review your patch. Please consider consulting your legal team to make the spec publicly available if you can.

That said, fortunately, the only part I don't understand without the spec is the necessity of R_HEXAGON_GOT relocation type, and as per your explanation, I believe your patch is correct. So, LGTM. Please go ahead and submit.

But again, please consider contacting your legal team to publish the documentation. That makes my life as a maintainer much easier. Thanks!

This revision is now accepted and ready to land.Oct 3 2018, 9:58 AM
This revision was automatically updated to reflect the committed changes.