[PATCH] ARM: generation of .ARM.exidx/.ARM.extab sections

Denis Protivensky dprotivensky at accesssoftek.com
Wed Apr 29 02:47:31 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/Core/DefinedAtom.cpp:46
@@ -45,2 +45,3 @@
   case typeNoAlloc:
+  case typeARMExidx:
     return permR__;
----------------
Just a general note to think about:

Maybe it's a good time to introduce `doPermissions` method for descendants and handle specific types there?

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMELFFile.h:33
@@ +32,3 @@
+
+    return ELFDefinedAtom<ELF32LE>::permissions();
+  }
----------------
This should work as `ELFDefinedAtom::permissions()`. Please, check it.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMELFFile.h:41
@@ +40,3 @@
+
+    return ELFDefinedAtom<ELF32LE>::doContentType();
+  }
----------------
Ditto.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h:74
@@ +73,3 @@
+
+  auto exidxStart = _armLayout.findAbsoluteAtom("__exidx_start");
+  auto exidxEnd = _armLayout.findAbsoluteAtom("__exidx_end");
----------------
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.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h:38
@@ +37,3 @@
+    const DefinedAtom *definedAtom = cast<DefinedAtom>(atom);
+
+    assert(atom->contentType() == DefinedAtom::typeARMExidx);
----------------
Remove this blank line.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h:39
@@ +38,3 @@
+
+    assert(atom->contentType() == DefinedAtom::typeARMExidx);
+
----------------
`assert`s should have description of a failure provided.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h:42
@@ +41,3 @@
+    DefinedAtom::Alignment atomAlign = definedAtom->alignment();
+    uint64_t alignment = atomAlign.value;
+    uint64_t fOffset = alignOffset(this->fileSize(), atomAlign);
----------------
This variable is not used in this context. Please, move it to the place where it's actually needed.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMTargetHandler.h:71
@@ +70,3 @@
+    default:
+      return TargetLayout<ELF32LE>::getSectionOrder(name, contentType,
+                                                    contentPermissions);
----------------
`TargetLayout` without <> should work here as well.

http://reviews.llvm.org/D9324

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list