[PATCH] D65548: Make JITDylib ignore Exported flag

[Sasha Krassovsky via Phabricator via llvm-commits]

save-buffer created this revision.
save-buffer added reviewers: compnerd, xiaobai.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

  This change addresses an issue with current compatibility with COFF.
  COFF lacks an "Exported" flag. When the COFF object file is generated,
  ORC compares the flags from the symbol table in the object file to the
  flags it expected from the IR. This was incorrect because when
  generating a COFF, the Exported flag was never set. This change ignores
  it in asserts. Further, when COFF object file is generated, some symbols
  get inserted (for example, if a float is added, it adds a symbol
  __real@<HEX OF FLOAT>. This caused issues down the line because more
  symbols were being resolved than asked for in the query. This change
  makes symbol resolution skip these extra symbols.


Repository:
  rL LLVM

https://reviews.llvm.org/D65548

Files:
  llvm/lib/ExecutionEngine/Orc/Core.cpp


Index: llvm/lib/ExecutionEngine/Orc/Core.cpp
===================================================================
--- llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -374,13 +374,20 @@
 #ifndef NDEBUG
   for (auto &KV : Symbols) {
     auto I = SymbolFlags.find(KV.first);
-    assert(I != SymbolFlags.end() &&
-           "Resolving symbol outside this responsibility set");
+
+    // Certain file formats like COFF add symbols to the symbol table
+    // that don't exist in the IR. Therefore, we just ignore it if
+    // there are extra symbols.
+    if (I == SymbolFlags.end())
+      continue;
+
+    // Mask out Exported for compatibility with COFF.
+    constexpr auto Mask = ~JITSymbolFlags::Exported;
     if (I->second.isWeak())
-      assert(I->second == (KV.second.getFlags() | JITSymbolFlags::Weak) &&
+      assert((I->second & Mask) == ((KV.second.getFlags() | JITSymbolFlags::Weak) & Mask) &&
              "Resolving symbol with incorrect flags");
     else
-      assert(I->second == KV.second.getFlags() &&
+      assert((I->second & Mask) == (KV.second.getFlags() & Mask) &&
              "Resolving symbol with incorrect flags");
   }
 #endif
@@ -864,19 +871,31 @@
 
       auto I = Symbols.find(Name);
 
-      assert(I != Symbols.end() && "Symbol not found");
+      // Certain file formats like COFF add symbols to the symbol table
+      // that don't exist in the IR. Therefore, we just ignore it if
+      // there are extra symbols.
+      if (I == Symbols.end())
+        continue;
+
       assert(!I->second.hasMaterializerAttached() &&
              "Resolving symbol with materializer attached?");
       assert(I->second.getState() == SymbolState::Materializing &&
              "Symbol should be materializing");
       assert(I->second.getAddress() == 0 && "Symbol has already been resolved");
 
-      assert((Sym.getFlags() & ~JITSymbolFlags::Weak) ==
-                 (I->second.getFlags() & ~JITSymbolFlags::Weak) &&
+      // Mask out Exported for compatibility with COFF.
+      constexpr auto Mask = ~(JITSymbolFlags::Weak | JITSymbolFlags::Exported);
+      assert((Sym.getFlags() & Mask) ==
+                 (I->second.getFlags() & Mask) &&
              "Resolved flags should match the declared flags");
 
       // Once resolved, symbols can never be weak.
-      JITSymbolFlags ResolvedFlags = Sym.getFlags();
+      // IMPORTANT: We use I->second.getFlags() because we want
+      //            to reuse the flags from the original IR. Some
+      //            object file formats like COFF don't have an
+      //            exported flag, for example, so we want to preserve
+      //            IR flags.
+      JITSymbolFlags ResolvedFlags = I->second.getFlags();
       ResolvedFlags &= ~JITSymbolFlags::Weak;
       I->second.setAddress(Sym.getAddress());
       I->second.setFlags(ResolvedFlags);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65548.212703.patch
Type: text/x-patch
Size: 2891 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190731/e11ee7e1/attachment.bin>