[llvm] ba26b5e - [ORC] Mark late-claimed weak symbols as live in ObjectLinkingLayer.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2022-10-29T19:07:51Z
New Revision: ba26b5ef15dcbfc69f062b1aea6424cdb186e5b0

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

LOG: [ORC] Mark late-claimed weak symbols as live in ObjectLinkingLayer.

ObjectLinkingLayer attempts to claim responsibility for weak definitions that
are present in LinkGraphs, but not present in the corresponding
MaterializationResponsibility object. Where such a claim is successful, the
symbol should be marked as live to prevent it from being dead stripped.

(For the curious: Such "late-breaking" definitions are introduced somewhere in
the materialization pipeline after the initial responsibility set is calculated.
The usual source is the complier or assembler. Examples of common late-breaking
definitions include personality pointers, e.g. "DW.ref.__gxx_personality_v0",
and named constant pool entries, e.g. __realXX..XX.)

The failure to mark these symbols live caused few problems in practice because
late-breaking definitions are usually anchored by existing live definitions
within the graph (e.g. DW.ref.__gxx_personality_v0 is transitively referenced by
functions via eh-frame records), and so they usually survived dead-stripping
anyway. This accidental persistence isn't a principled solution though, and it
fails altogether if a late-breaking definition is not otherwise referenced by
the graph, with the result that the now-claimed symbol is stripped triggering a
"Failed to materialize symbols" error in ORC. Marking such symbols live is the
correct solution.

No testcase, as it's difficult to construct a situation where a late-breaking
definition is inserted without being referenced outside the context of new
backend bringup or plugin-specific shenanigans.

See discussion in https://reviews.llvm.org/D133452 and
https://reviews.llvm.org/D136877.

Added: 
    

Modified: 
    llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index 7b4da052ed368..1b5661f10eaad 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -449,9 +449,15 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
     // claim, at which point we'll externalize that symbol.
     cantFail(MR->defineMaterializing(std::move(NewSymbolsToClaim)));
 
-    for (auto &KV : NameToSym)
-      if (!MR->getSymbols().count(KV.first))
+    // Walk the list of symbols that we just tried to claim. Symbols that we're
+    // responsible for are marked live. Symbols that we're not responsible for
+    // are turned into external references.
+    for (auto &KV : NameToSym) {
+      if (MR->getSymbols().count(KV.first))
+        KV.second->setLive(true);
+      else
         G.makeExternal(*KV.second);
+    }
 
     return Error::success();
   }