This is an archive of the discontinued LLVM Phabricator instance.

[ELFYAML] Support mips64 relocation record format in yaml2obj/obj2yaml
ClosedPublic

Authored by atanasyan on Jan 22 2015, 10:18 PM.

Details

Summary

MIPS64 ELF file has a very specific relocation record format. Each record might specify up to three relocation operations. So the r_info field in fact consists of three relocation type sub-fields and optional code of "special" symbols.

http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
page 40

The patch implements support of the MIPS64 relocation record format in yaml2obj/obj2yaml tools by introducing new optional Relocation fields: Type2, Type3, and SpecSym. These fields are recognized only if the object/YAML file relates to the MIPS64 target.

Diff Detail

Event Timeline

atanasyan updated this revision to Diff 18650.Jan 22 2015, 10:18 PM
atanasyan retitled this revision from to [ELFYAML] Support mips64 relocation record format in yaml2obj/obj2yaml.
atanasyan updated this object.
atanasyan edited the test plan for this revision. (Show Details)
atanasyan added a reviewer: silvas.
atanasyan added a subscriber: Unknown Object (MLST).
silvas edited edge metadata.Jan 24 2015, 6:14 AM

Random: Could you please link that document into
http://llvm.org/docs/CompilerWriterInfo.html#linux alongside the other
processor supplement documents?

silvas added inline comments.Jan 24 2015, 7:41 AM
lib/Object/ELFYAML.cpp
583–595

Rather than changing the fields of ELFYAML::Relocation dynamically, I would prefer if we just treated this MIPS64 special case as just syntax sugar for filling in the bits of the usual Type field. E.g. make the Type field a union of either a single relocation value, as it "usually" is, or a sub-object with Type, Type2, Type3, SpecSym fields in the MIPS64 case.

So really the only changes to the code would end up being that we would optionally accept a subobject for the Type field with the MIPS64 special fields, and we would dump in that format when applicable.

583–595

Ignore this comment. Phab seems broken and doesn't let me delete it.

tools/yaml2obj/yaml2elf.cpp
93–100

Please don't reuse these functions for just a single use. They are just local helpers (pre-lambda days) for main to make it less gross; it defeats the purpose to have them far away from main like this.

343–344

Rather than reusing the other two predicates here, I would prefer if you open-coded all the checks into an isMips64EL helper function. That makes it immediately obvious to a reader what is being checked.

Overall this patch is horrifically ugly, but that's just the ugliness of this bizarre MIPS ELF semantics showing through. Modulo my comments earlier, LGTM. (and make sure to ignore that comment that Phab didn't let me delete, after reconsideration, it didn't seem to make sense).

This revision was automatically updated to reflect the committed changes.