Fix mapping of unmaterialized global values during metadata linking
authorTeresa Johnson <tejohnson@google.com>
Sun, 15 Nov 2015 14:50:14 +0000 (14:50 +0000)
committerTeresa Johnson <tejohnson@google.com>
Sun, 15 Nov 2015 14:50:14 +0000 (14:50 +0000)
commit78b70af914869966c1b90415d7f020c396699e8c
tree89d990b65620e6f4823b0050b2bbe35ea5c7fd1f
parentfb7bc3560222f3efa26a492485eec70969ab3d8a
Fix mapping of unmaterialized global values during metadata linking

Summary:
The patch to move metadata linking after global value linking didn't
correctly map unmaterialized global values to null as desired. They
were in fact mapped to the source copy. It largely worked by accident
since most module linker clients destroyed the source module which
caused the source GVs to be replaced by null, but caused a failure with
LTO linking on Windows:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20151109/312869.html

The problem is that a null return value from materializeValueFor is
handled by mapping the value to self. This is the desired behavior when
materializeValueFor is passed a non-GlobalValue. The problem is how to
distinguish that case from the case where we really do want to map to
null.

This patch addresses this by passing in a new flag to the value mapper
indicating that unmapped global values should be mapped to null. Other
Value types are handled as before.

Note that the documented behavior of asserting on unmapped values when
the flag RF_IgnoreMissingValues isn't set is currently disabled with
FIXME notes due to bootstrap failures. I modified these disabled asserts
so when they are eventually enabled again it won't assert for the
unmapped values when the new RF_NullMapMissingGlobalValues flag is set.

I also considered using a callback into the value materializer, but a
flag seemed cleaner given that there are already existing flags.
I also considered modifying materializeValueFor to return the input
value when we want to map to source and then treat a null return
to mean map to null. However, there are other value materializer
subclasses that implement materializeValueFor, and they would all need
to be audited and the return values possibly changed, which seemed
error-prone.

Reviewers: dexonsmith, joker.eph

Subscribers: pcc, llvm-commits

Differential Revision: http://reviews.llvm.org/D14682

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@253170 91177308-0d34-0410-b5e6-96231b3b80d8
include/llvm/Transforms/Utils/ValueMapper.h
lib/Linker/LinkModules.cpp
lib/Transforms/Utils/ValueMapper.cpp