[PATCH] Preliminary support for dynamically loadable coff objects

[Andy Ayers]

Lang --

I'd be happy to add a test -- would checking in an object file be ok, or would you rather have the test input be an assembly file?

If the latter, I'll have to figure out how to express what I want in your assembly syntax.

I'll probably hold off refactoring for architecture unless you feel strongly we should do it now.


REPOSITORY
  rL LLVM

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp:187
@@ +186,3 @@
+  switch (RelType) {
+  case COFF::RelocationTypeAMD64::IMAGE_REL_AMD64_ADDR32NB: {
+    uint32_t *TargetAddress = (uint32_t *)Target;
----------------
majnemer wrote:
> Any reason you chose to include the `RelocationTypeAMD64` scope qualifier?
Just force of habit... have trimmed this back.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp:236-237
@@ +235,4 @@
+  switch (RelType) {
+  case COFF::RelocationTypeAMD64::IMAGE_REL_AMD64_ADDR64:
+  case COFF::RelocationTypeAMD64::IMAGE_REL_AMD64_ADDR32NB:
+  {
----------------
majnemer wrote:
> Could you remove the extra scope qualifiers? I think the enumeration value is pretty specific on its own.
Yep.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp:261-262
@@ +260,4 @@
+  // Look for and record the EH frame sections.
+  ObjSectionToIDMap::iterator i, e;
+  for (i = SectionMap.begin(), e = SectionMap.end(); i != e; ++i) {
+    const SectionRef &Section = i->first;
----------------
majnemer wrote:
> This might be a little simpler as a range-based for loop.
Made it so.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp:265
@@ +264,3 @@
+    StringRef Name;
+    Section.getName(Name);
+    // Note unwind info is split across .pdata and .xdata, so this
----------------
majnemer wrote:
> Is it not possible for this to fail in practice?
I don't think coff allows nameless sections, but added a Check here anyways.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.h:29
@@ +28,3 @@
+// Helper for extensive error checking in debug builds.
+std::error_code Check1(std::error_code Err) {
+  if (Err) {
----------------
majnemer wrote:
> Any reason why you chose to make this different from ELF? They use `Check`.
Thanks for reminding me about this. I had a name collision and just renamed it as a workaround.

I'll moved Check into RuntimeDyldImpl.h so that both ELF and COFF can share.

================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.h:46-61
@@ +45,18 @@
+
+  unsigned getMaxStubSize() override {
+    if (Arch == Triple::aarch64 || Arch == Triple::aarch64_be)
+      return 20; // movz; movk; movk; movk; br
+    if (Arch == Triple::arm || Arch == Triple::thumb)
+      return 8; // 32-bit instruction and 32-bit address
+    else if (Arch == Triple::mipsel || Arch == Triple::mips)
+      return 16;
+    else if (Arch == Triple::ppc64 || Arch == Triple::ppc64le)
+      return 44;
+    else if (Arch == Triple::x86_64)
+      return 6; // 2-byte jmp instruction + 32-bit relative address
+    else if (Arch == Triple::systemz)
+      return 16;
+    else
+      return 0;
+  }
+
----------------
majnemer wrote:
> Something tells me you aren't trying to support SystemZ ;)
> 
> I'd remove support for everything but the ARM and x86 variants.
Yep. Thanks.

================
Comment at: lib/Object/COFFObjectFile.cpp:376
@@ -366,3 +375,3 @@
   const coff_section *Sec = toSec(Ref);
-  return Sec->Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA;
+  return Sec->SizeOfRawData == 0;
 }
----------------
majnemer wrote:
> What's the rationale for this change?
I thought it better matched the intent of the comment in ObjectFile.h:

  // A section is 'virtual' if its contents aren't present in the object image.

For coff this means the section has no raw data, regardless of section characteristics. In practice it probably doesn't matter so I'd be happy to revert this if you prefer.

http://reviews.llvm.org/D7793

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