[PATCH] D20713: ValueMapper: fix assertion when null-mapping a constant for linking metadata

[Rafael Espíndola via llvm-commits]

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

On 26 May 2016 at 20:50, Mehdi AMINI <mehdi.amini at apple.com> wrote:
> mehdi_amini created this revision.
> mehdi_amini added reviewers: dexonsmith, rafael.
> mehdi_amini added a subscriber: llvm-commits.
>
> 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.
>
> http://reviews.llvm.org/D20713
>
> Files:
>   lib/Transforms/Utils/ValueMapper.cpp
>   test/Linker/null_mapping_constant.ll
>
> Index: test/Linker/null_mapping_constant.ll
> ===================================================================
> --- /dev/null
> +++ test/Linker/null_mapping_constant.ll
> @@ -0,0 +1,11 @@
> +; 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"
> + at 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}
> Index: lib/Transforms/Utils/ValueMapper.cpp
> ===================================================================
> --- lib/Transforms/Utils/ValueMapper.cpp
> +++ lib/Transforms/Utils/ValueMapper.cpp
> @@ -436,6 +436,12 @@
>    for (; OpNo != NumOperands; ++OpNo) {
>      Value *Op = C->getOperand(OpNo);
>      Mapped = mapValue(Op);
> +    if (!Mapped) {
> +      assert((Flags & RF_NullMapMissingGlobalValues) &&
> +             "Unexpected null mapping for constant operand without "
> +             "NullMapMissingGlobalValues flag");
> +      return nullptr;
> +    }
>      if (Mapped != Op)
>        break;
>    }
> @@ -462,8 +468,16 @@
>      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 = mapValue(C->getOperand(OpNo));
> +      if (!Mapped) {
> +        assert((Flags & RF_NullMapMissingGlobalValues) &&
> +               "Unexpected null mapping for constant operand without "
> +               "NullMapMissingGlobalValues flag");
> +        return nullptr;
> +      }
> +      Ops.push_back(cast<Constant>(Mapped));
> +    }
>    }
>    Type *NewSrcTy = nullptr;
>    if (TypeMapper)
>
>