This patch provides generation of .ARM.exidx & .ARM.extab sections which are used for unwinding. The patch adds new content type typeARMExidx for atoms from .ARM.exidx section and integration of atoms with such type to the ELF ReaderWriter. Most of changes were done for only ARM component. However, some changes have to be implemented in common headers.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
LGTM with nits
lib/ReaderWriter/ELF/ARM/ARMELFFile.h | ||
---|---|---|
29 ↗ | (On Diff #24552) | nit: I'd remove this blank line (and other two lines below after dangling if). |
lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h | ||
70 ↗ | (On Diff #24552) | Can you keep and update the comment, so that it's clear what we are doing here? |
lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h | ||
23 ↗ | (On Diff #24552) | A brief comment about what this class is for would be needed. |
81 ↗ | (On Diff #24552) | Does just "TargetLayout" (without <>) work? |
90 ↗ | (On Diff #24552) | Ditto |
lib/Core/DefinedAtom.cpp | ||
---|---|---|
46 ↗ | (On Diff #24552) | Just a general note to think about: Maybe it's a good time to introduce doPermissions method for descendants and handle specific types there? |
lib/ReaderWriter/ELF/ARM/ARMELFFile.h | ||
33 ↗ | (On Diff #24552) | This should work as ELFDefinedAtom::permissions(). Please, check it. |
41 ↗ | (On Diff #24552) | Ditto. |
lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h | ||
74 ↗ | (On Diff #24552) | There's startEnd lambda function in the ExecutableWriter::finalizeDefaultAtomValues() that does *almost* the same thing, you may adapt it to be a proper function and use here. |
lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h | ||
38 ↗ | (On Diff #24552) | Remove this blank line. |
39 ↗ | (On Diff #24552) | asserts should have description of a failure provided. |
42 ↗ | (On Diff #24552) | This variable is not used in this context. Please, move it to the place where it's actually needed. |
71 ↗ | (On Diff #24552) | TargetLayout without <> should work here as well. |
- implementations of permissions() and doContentType() were moved to base class ELFDefinedAtom;
- ARMELFDefinedAtomBase was removed;
- assert message was added;
- minor code changes, comments and formatting after review.
lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h | ||
---|---|---|
76 ↗ | (On Diff #24952) | That's now copy'n'pasting of the functionality. I actually wanted you to convert base class's lambda into member method and call it from here. Please, remove code duplication. |
lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h | ||
23 ↗ | (On Diff #24952) | It should be a doxygen-style comment. See here: http://llvm.org/docs/CodingStandards.html#commenting |