This is an archive of the discontinued LLVM Phabricator instance.

[lld] Correct forming of ARM/Thumb atoms
ClosedPublic

Authored by denis-protivensky on Jan 24 2015, 2:36 AM.

Details

Summary

Symbols addressing Thumb code have zero bit set in st_value to distinguish them from ARM instructions.
This caused wrong atoms' forming because of offset of one byte brought in by that corrected st_value.

Fixed reading of st_value & st_value-related things in ARMELFFile while forming atoms.
Symbol table generation is also fixed for Thumb atoms.

Diff Detail

Repository
rL LLVM

Event Timeline

denis-protivensky retitled this revision from to [lld] Correct forming of ARM/Thumb atoms.
denis-protivensky updated this object.
denis-protivensky edited the test plan for this revision. (Show Details)
denis-protivensky added reviewers: shankarke, ruiu.
denis-protivensky added a project: lld.
denis-protivensky added a subscriber: Unknown Object (MLST).
shankar.easwaran requested changes to this revision.Jan 24 2015, 5:17 AM
shankar.easwaran added inline comments.
lib/ReaderWriter/ELF/ARM/ARMELFFile.h
56 ↗(On Diff #18720)

You are duplicating a lot of base class functionality

86 ↗(On Diff #18720)

Why static?

141 ↗(On Diff #18720)

Once you have the base function for getSymbolValue this function can be removed.

157 ↗(On Diff #18720)

Add a base class function getSymbolValue and you can override that in ARM.

This revision now requires changes to proceed.Jan 24 2015, 5:17 AM
lib/ReaderWriter/ELF/ARM/ARMELFFile.h
56 ↗(On Diff #18720)

Okay, I'll introduce getSymbolValue as for ELFFile.

86 ↗(On Diff #18720)

This function is more like a helper which doesn't affect object's state and doesn't contain any logic connected directly with ELFFile. The same one is in ARMELFDefinedAtom and the only reason I couldn't move it out of classes is that it uses class-level Elf_Sym typedef. If you don't like that, I'll make it a class's member.

157 ↗(On Diff #18720)

Agree.

denis-protivensky edited edge metadata.

Updated:

  • Removed lots of overrides from ARMELFDefinedAtom and ARMELFFile by adding getSymbolValue method in both base classes and overriding it in the corresponding children
  • Regrouped tests by putting all symbol-related test cases into ARM- and Thumb-specific bundles
  • Added tests to check exact content generation for ARM and Thumb objects
shankarke accepted this revision.Jan 26 2015, 8:15 AM
shankarke edited edge metadata.

LGTM.

ruiu accepted this revision.Jan 26 2015, 10:32 AM
ruiu edited edge metadata.

LGTM

This revision was automatically updated to reflect the committed changes.