linkage property destroyed by BitcodeReader::Dematerialize()?

[Duncan P. N. Exon Smith]

+rafael

> On 2014-Sep-17, at 16:28, Petar Jovanovic <petar.jovanovic at imgtec.com> wrote:
> 
> Hi everyone,
> 
> In NativeClient for MIPS, we have come across an issue [1] in which
> NaClBitcodeReader, which mimics BitcodeReader, calls Dematerialize()
> which calls F->deleteBody() eventually.
> The problem is that deleteBody() also converts the linkage to external
> and thus destroys original linkage type value. Lack of correct linkage
> type causes wrong relocations to be emitted later.
> 
> For NaClBitcodeReader, a proposal is to change Dematerialize() to
> call dropAllReferences() instead of deleteBody() like this:
> 
> Index: lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
> diff --git a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
> index d613d2d84836142db874b9a78b94c197ebf0f30f..d4465d17639f7f74114a08fc14b5ffc6a45b529c 100644
> --- a/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
> +++ b/lib/Bitcode/NaCl/Reader/NaClBitcodeReader.cpp
> @@ -1651,7 +1651,7 @@ void NaClBitcodeReader::Dematerialize(GlobalValue *GV) {
>   assert(DeferredFunctionInfo.count(F) && "No info to read function later?");
> 
>   // Just forget the function body, we can remat it later.
> -  F->deleteBody();
> +  F->dropAllReferences();
> }
> 
> 
> Still, there is a question if this change is applicable for
> BitcodeReader::Dematerialize() as well. Any opinions?

Seems reasonable to me, but I could be missing something.

> 
> Thanks in advance.
> 
> Kind regards,
> Petar
> 
> [1] Linkage property gets destroyed by NaClBitcodeReader::Dematerialize(),
> https://code.google.com/p/nativeclient/issues/detail?id=3943