[llvm] r334771 - [ORC] Strip weak flags from a symbol once it is selected for materialization.

[Lang Hames via llvm-commits]

Author: lhames
Date: Thu Jun 14 14:16:29 2018
New Revision: 334771

URL: http://llvm.org/viewvc/llvm-project?rev=334771&view=rev
Log:
[ORC] Strip weak flags from a symbol once it is selected for materialization.

Once a symbol has been selected for materialization it can no longer be
overridden. Stripping the weak flag guarantees this (override attempts will
then be treated as duplicate definitions and result in a DuplicateDefinition
error).

Modified:
    llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
    llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp

Modified: llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp?rev=334771&r1=334770&r2=334771&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp Thu Jun 14 14:16:29 2018
@@ -468,6 +468,8 @@ void VSO::resolve(const SymbolMap &Resol
     for (const auto &KV : Resolved) {
       auto &Name = KV.first;
       auto Sym = KV.second;
+      JITSymbolFlags ResolvedFlags = Sym.getFlags();
+      ResolvedFlags &= ~JITSymbolFlags::Weak;
 
       assert(!Sym.getFlags().isLazy() && !Sym.getFlags().isMaterializing() &&
              "Materializing flags should be managed internally");
@@ -480,13 +482,11 @@ void VSO::resolve(const SymbolMap &Resol
              "Symbol should be materializing");
       assert(I->second.getAddress() == 0 && "Symbol has already been resolved");
 
-      assert(Sym.getFlags() ==
+      assert(ResolvedFlags ==
                  JITSymbolFlags::stripTransientFlags(I->second.getFlags()) &&
              "Resolved flags should match the declared flags");
 
       // Once resolved, symbols can never be weak.
-      JITSymbolFlags ResolvedFlags = Sym.getFlags();
-      ResolvedFlags &= ~JITSymbolFlags::Weak;
       ResolvedFlags |= JITSymbolFlags::Materializing;
       I->second = JITEvaluatedSymbol(Sym.getAddress(), ResolvedFlags);
 
@@ -748,6 +748,7 @@ void VSO::lookupImpl(std::shared_ptr<Asy
       for (auto &KV : MU->getSymbols()) {
         auto SymK = Symbols.find(KV.first);
         auto Flags = SymK->second.getFlags();
+        Flags &= ~JITSymbolFlags::Weak;
         Flags &= ~JITSymbolFlags::Lazy;
         Flags |= JITSymbolFlags::Materializing;
         SymK->second.setFlags(Flags);

Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp?rev=334771&r1=334770&r2=334771&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp Thu Jun 14 14:16:29 2018
@@ -10,6 +10,7 @@
 #include "OrcTestCommon.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/ExecutionEngine/Orc/Core.h"
+#include "llvm/ExecutionEngine/Orc/OrcError.h"
 #include "gtest/gtest.h"
 
 #include <set>
@@ -768,4 +769,50 @@ TEST(CoreAPIsTest, TestGetRequestedSymbo
   EXPECT_TRUE(BarMaterialized) << "Bar should be materialized now";
 }
 
+TEST(CoreAPIsTest, TestMaterializeWeakSymbol) {
+  // Confirm that once a weak definition is selected for materialization it is
+  // treated as strong.
+
+  constexpr JITTargetAddress FakeFooAddr = 0xdeadbeef;
+  JITSymbolFlags FooFlags = JITSymbolFlags::Exported;
+  FooFlags &= JITSymbolFlags::Weak;
+  auto FooSym = JITEvaluatedSymbol(FakeFooAddr, FooFlags);
+
+  ExecutionSession ES;
+  auto Foo = ES.getSymbolStringPool().intern("foo");
+
+  auto &V = ES.createVSO("V");
+
+  std::unique_ptr<MaterializationResponsibility> FooResponsibility;
+  auto MU = llvm::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Foo, FooFlags}}), [&](MaterializationResponsibility R) {
+        FooResponsibility =
+            llvm::make_unique<MaterializationResponsibility>(std::move(R));
+      });
+
+  cantFail(V.define(MU));
+  auto Q = std::make_shared<AsynchronousSymbolQuery>(
+      SymbolNameSet({Foo}),
+      [](Expected<AsynchronousSymbolQuery::ResolutionResult> R) {
+        cantFail(std::move(R));
+      },
+      [](Error Err) { cantFail(std::move(Err)); });
+  V.lookup(std::move(Q), SymbolNameSet({Foo}));
+
+  auto MU2 = llvm::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Foo, JITSymbolFlags::Exported}}),
+      [](MaterializationResponsibility R) {
+        llvm_unreachable("This unit should never be materialized");
+      });
+
+  auto Err = V.define(MU2);
+  EXPECT_TRUE(!!Err) << "Expected failure value";
+  EXPECT_TRUE(Err.isA<DuplicateDefinition>())
+      << "Expected a duplicate definition error";
+  consumeError(std::move(Err));
+
+  FooResponsibility->resolve(SymbolMap({{Foo, FooSym}}));
+  FooResponsibility->finalize();
+}
+
 } // namespace