[PATCH] D47969: [Verifier] Check that ValueAsMetadata belongs to the module

[Duncan P. N. Exon Smith via llvm-commits]

IIRC, sharing uniqued metadata across modules should only happen as a temporary state.  It's a trick for when loading modules that will be merged together, to avoid unnecessarily creating multiple copies of the graphs.

I feel like the verifier should reject this state, unless it's specifically being called in a context where modules-to-be-merged are still being loaded.

> On Jun 11, 2018, at 23:33, Roman Tereshin via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> rtereshin added a comment.
> 
>> Value::~Value() calls out ValueAsMetadata::handleDeletion() so it seems like it would not leave a stale reference
> 
> Looks like it. On the other hand, I would think that referencing a value from a different module is more tolerable if there is no copy of that value in the module directly referencing the metadata node. In all the tests cases we've got so far however it's not the case as the value referenced always has a copy in every module involved, so it seems having these cross-module references is just calling for trouble. Especially given this fair consideration:
> 
>> I'm also not sure if the code that handles DITemplateValueParameters would handle null Metadata entries well.
> 
> Even if it does, it's a loss of information for no apparent reason.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D47969
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180612/3f40793f/attachment.html>