[llvm] 8172689 - [ORC] Add errors for missing and extraneous symbol definitions.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 22 13:11:33 PST 2020


Author: Lang Hames
Date: 2020-02-22T11:49:14-08:00
New Revision: 81726894d3c8af556eb86007c8c26d7e2d9639f3

URL: https://github.com/llvm/llvm-project/commit/81726894d3c8af556eb86007c8c26d7e2d9639f3
DIFF: https://github.com/llvm/llvm-project/commit/81726894d3c8af556eb86007c8c26d7e2d9639f3.diff

LOG: [ORC] Add errors for missing and extraneous symbol definitions.

This patch adds new errors and error checking to the ObjectLinkingLayer to
catch cases where a compiled or loaded object either:
(1) Contains definitions not covered by its responsibility set, or
(2) Is missing definitions that are covered by its responsibility set.

Proir to this patch providing the correct set of definitions was treated as
an API contract requirement, however this requires that the client be confident
in the correctness of the whole compiler / object-cache pipeline and results
in difficult-to-debug assertions upon failure. Treating this as a recoverable
error results in clearer diagnostics.

The performance overhead of this check is one comparison of densemap keys
(symbol string pointers) per linking object, which is minimal. If this overhead
ever becomes a problem we can add the check under a flag that can be turned off
if the client fully trusts the rest of the pipeline.

Added: 
    

Modified: 
    llvm/include/llvm/ExecutionEngine/Orc/Core.h
    llvm/include/llvm/ExecutionEngine/Orc/OrcError.h
    llvm/lib/ExecutionEngine/Orc/Core.cpp
    llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
    llvm/lib/ExecutionEngine/OrcError/OrcError.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/Core.h b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
index 94600f2286b8..0d2cdc57aa5a 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/Core.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/Core.h
@@ -424,6 +424,44 @@ class SymbolsCouldNotBeRemoved : public ErrorInfo<SymbolsCouldNotBeRemoved> {
   SymbolNameSet Symbols;
 };
 
+/// Errors of this type should be returned if a module fails to include
+/// definitions that are claimed by the module's associated
+/// MaterializationResponsibility. If this error is returned it is indicative of
+/// a broken transformation / compiler / object cache.
+class MissingSymbolDefinitions : public ErrorInfo<MissingSymbolDefinitions> {
+public:
+  static char ID;
+
+  MissingSymbolDefinitions(std::string ModuleName, SymbolNameVector Symbols)
+    : ModuleName(std::move(ModuleName)), Symbols(std::move(Symbols)) {}
+  std::error_code convertToErrorCode() const override;
+  void log(raw_ostream &OS) const override;
+  const std::string &getModuleName() const { return ModuleName; }
+  const SymbolNameVector &getSymbols() const { return Symbols; }
+private:
+  std::string ModuleName;
+  SymbolNameVector Symbols;
+};
+
+/// Errors of this type should be returned if a module contains definitions for
+/// symbols that are not claimed by the module's associated
+/// MaterializationResponsibility. If this error is returned it is indicative of
+/// a broken transformation / compiler / object cache.
+class UnexpectedSymbolDefinitions : public ErrorInfo<UnexpectedSymbolDefinitions> {
+public:
+  static char ID;
+
+  UnexpectedSymbolDefinitions(std::string ModuleName, SymbolNameVector Symbols)
+    : ModuleName(std::move(ModuleName)), Symbols(std::move(Symbols)) {}
+  std::error_code convertToErrorCode() const override;
+  void log(raw_ostream &OS) const override;
+  const std::string &getModuleName() const { return ModuleName; }
+  const SymbolNameVector &getSymbols() const { return Symbols; }
+private:
+  std::string ModuleName;
+  SymbolNameVector Symbols;
+};
+
 /// Tracks responsibility for materialization, and mediates interactions between
 /// MaterializationUnits and JDs.
 ///

diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h b/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h
index 61e2e49a872a..9b0d941f5459 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h
@@ -37,7 +37,9 @@ enum class OrcErrorCode : int {
   UnexpectedRPCCall,
   UnexpectedRPCResponse,
   UnknownErrorCodeFromRemote,
-  UnknownResourceHandle
+  UnknownResourceHandle,
+  MissingSymbolDefinitions,
+  UnexpectedSymbolDefinitions,
 };
 
 std::error_code orcError(OrcErrorCode ErrCode);

diff  --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
index e0725730f62b..992389ada6fa 100644
--- a/llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -144,6 +144,8 @@ namespace orc {
 char FailedToMaterialize::ID = 0;
 char SymbolsNotFound::ID = 0;
 char SymbolsCouldNotBeRemoved::ID = 0;
+char MissingSymbolDefinitions::ID = 0;
+char UnexpectedSymbolDefinitions::ID = 0;
 
 RegisterDependenciesFunction NoDependenciesToRegister =
     RegisterDependenciesFunction();
@@ -352,6 +354,24 @@ void SymbolsCouldNotBeRemoved::log(raw_ostream &OS) const {
   OS << "Symbols could not be removed: " << Symbols;
 }
 
+std::error_code MissingSymbolDefinitions::convertToErrorCode() const {
+  return orcError(OrcErrorCode::MissingSymbolDefinitions);
+}
+
+void MissingSymbolDefinitions::log(raw_ostream &OS) const {
+  OS << "Missing definitions in module " << ModuleName
+     << ": " << Symbols;
+}
+
+std::error_code UnexpectedSymbolDefinitions::convertToErrorCode() const {
+  return orcError(OrcErrorCode::UnexpectedSymbolDefinitions);
+}
+
+void UnexpectedSymbolDefinitions::log(raw_ostream &OS) const {
+  OS << "Unexpected definitions in module " << ModuleName
+     << ": " << Symbols;
+}
+
 AsynchronousSymbolQuery::AsynchronousSymbolQuery(
     const SymbolLookupSet &Symbols, SymbolState RequiredState,
     SymbolsResolvedCallback NotifyComplete)

diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index db88ec8517bf..4b9ffc9effbe 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -148,6 +148,36 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
     if (const auto &InitSym = MR.getInitializerSymbol())
       InternedResult[InitSym] = JITEvaluatedSymbol();
 
+    {
+      // Check that InternedResult matches up with MR.getSymbols().
+      // This guards against faulty transformations / compilers / object caches.
+
+      if (InternedResult.size() > MR.getSymbols().size()) {
+        SymbolNameVector ExtraSymbols;
+        for (auto &KV : InternedResult)
+          if (!MR.getSymbols().count(KV.first))
+            ExtraSymbols.push_back(KV.first);
+        ES.reportError(
+          make_error<UnexpectedSymbolDefinitions>(G.getName(),
+                                                  std::move(ExtraSymbols)));
+        MR.failMaterialization();
+        return;
+      }
+
+      SymbolNameVector MissingSymbols;
+      for (auto &KV : MR.getSymbols())
+        if (!InternedResult.count(KV.first))
+          MissingSymbols.push_back(KV.first);
+
+      if (!MissingSymbols.empty()) {
+        ES.reportError(
+          make_error<MissingSymbolDefinitions>(G.getName(),
+                                               std::move(MissingSymbols)));
+        MR.failMaterialization();
+        return;
+      }
+    }
+
     if (auto Err = MR.notifyResolved(InternedResult)) {
       Layer.getExecutionSession().reportError(std::move(Err));
       MR.failMaterialization();

diff  --git a/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp b/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp
index 5eab246d4b48..cc99e154fbec 100644
--- a/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp
+++ b/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp
@@ -61,6 +61,10 @@ class OrcErrorCategory : public std::error_category {
              "(Use StringError to get error message)";
     case OrcErrorCode::UnknownResourceHandle:
       return "Unknown resource handle";
+    case OrcErrorCode::MissingSymbolDefinitions:
+      return "MissingSymbolsDefinitions";
+    case OrcErrorCode::UnexpectedSymbolDefinitions:
+      return "UnexpectedSymbolDefinitions";
     }
     llvm_unreachable("Unhandled error code");
   }


        


More information about the llvm-commits mailing list