ValueMapper: fix assertion when null-mapping a constant for linking metadata

[mehdi_amini]

Summary

When RF_NullMapMissingGlobalValues is set, mapValue can return null
for GlobalValue. When mapping the operands of a constant that is
referenced from metadata, we need to handle this case and actually
return null instead of mapping this constant.

[• rafael]

Maybe add a helper that call mapValue, does the assert and then
returns the result?

[mehdi_amini]

Refactor the assertion in a helper lambda

[mehdi_amini]

Fix the assert after extraction in the helper

[• rafael]

LGTM

lib/Transforms/Utils/ValueMapper.cpp

Value *Mapper::mapValue(const Value *V) {
   // is something that is not in the mapping table.
   Constant *C = const_cast<Constant*>(dyn_cast<Constant>(V));
   if (!C)
     return nullptr;
 
   if (BlockAddress *BA = dyn_cast<BlockAddress>(C))
     return mapBlockAddress(*BA);
 
+  auto mapValueOrNull = [this](Value *V) {
+    auto Mapped = mapValue(V);
+    assert((Mapped || (Flags & RF_NullMapMissingGlobalValues)) &&
+           "Unexpected null mapping for constant operand without "
+           "NullMapMissingGlobalValues flag");
+    return Mapped;
+  };
+
   // Otherwise, we have some other constant to remap.  Start by checking to see
   // if all operands have an identity remapping.
   unsigned OpNo = 0, NumOperands = C->getNumOperands();
   Value *Mapped = nullptr;
   for (; OpNo != NumOperands; ++OpNo) {
     Value *Op = C->getOperand(OpNo);
-    Mapped = mapValue(Op);
+    Mapped = mapValueOrNull(Op);
+    if (!Mapped)
+      return nullptr;
     if (Mapped != Op)
       break;
   }
 
   // See if the type mapper wants to remap the type as well.
   Type *NewTy = C->getType();
   if (TypeMapper)
     NewTy = TypeMapper->remapType(NewTy);
Value *Mapper::mapValue(const Value *V) {
   for (unsigned j = 0; j != OpNo; ++j)
     Ops.push_back(cast<Constant>(C->getOperand(j)));
 
   // If one of the operands mismatch, push it and the other mapped operands.
   if (OpNo != NumOperands) {
     Ops.push_back(cast<Constant>(Mapped));
 
     // Map the rest of the operands that aren't processed yet.
-    for (++OpNo; OpNo != NumOperands; ++OpNo)
-      Ops.push_back(cast<Constant>(mapValue(C->getOperand(OpNo))));
+    for (++OpNo; OpNo != NumOperands; ++OpNo) {
+      Mapped = mapValueOrNull(C->getOperand(OpNo));
+      if (!Mapped)
+        return nullptr;
+      Ops.push_back(cast<Constant>(Mapped));
+    }
   }
   Type *NewSrcTy = nullptr;
   if (TypeMapper)
     if (auto *GEPO = dyn_cast<GEPOperator>(C))
       NewSrcTy = TypeMapper->remapType(GEPO->getSourceElementType());
 
   if (ConstantExpr *CE = dyn_cast<ConstantExpr>(C))
     return getVM()[V] = CE->getWithOperands(Ops, NewTy, false, NewSrcTy);

test/Linker/null_mapping_constant.ll

+; RUN: llvm-link %s -S -o - | FileCheck %s
+; Check that the constant is not linked and the metadata is correctly referencing a nullptr
+; CHECK: !0 = !{!"foo", null, i64 16}
+
+
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.12.0"
+@foo = external unnamed_addr constant { [4 x i8*], [32 x i8] }, align 32
+!llvm.bitsets = !{!0}
+!0 = !{!"foo", [4 x i8*]* getelementptr inbounds ({ [4 x i8*], [32 x i8] }, { [4 x i8*], [32 x i8] }* @foo, i32 0, i32 0), i64 16}