[PATCH] D33621: Fix for -wrap linker option and LTO, PR 33145

Mikulin, Dmitry via llvm-commits llvm-commits at lists.llvm.org
Tue May 30 11:41:02 PDT 2017


> On May 29, 2017, at 11:06 AM, Rafael Avila de Espindola <rafael.espindola at gmail.com> wrote:
> 
> 
>> +  bool isBitcode() const { return IsBitcode; }
> 
> Why can't you use the existing File in SymbolBody?

Why indeed… Fixed.

> 
>> template <class ELFT> struct Symtab { static SymbolTable<ELFT> *X; };
>> Index: lld/ELF/SymbolTable.cpp
>> ===================================================================
>> --- lld/ELF/SymbolTable.cpp
>> +++ lld/ELF/SymbolTable.cpp
>> @@ -118,8 +118,9 @@
>>   // Compile bitcode files and replace bitcode symbols.
>>   LTO.reset(new BitcodeCompiler);
>>   for (BitcodeFile *F : BitcodeFiles)
>> -    LTO->add(*F);
>> +    LTO->add(*F, RenamedSymbols);
>> 
>> +  PostLTO = true;
> 
> Why do you need to store this flag? All LTO produced files are added
> from addCombinedLTOObject, so you should be able to pass any extra info
> from there.

Trying to minimize changes to unrelated interfaces. This flag would need to pass through several InputFile functions to get back to the SymbolTable. I’ll change it if it’s a more accepted practice.

> 
>> +template <class ELFT> void SymbolTable<ELFT>::addLTOSymbolWrap(StringRef Name) {
>> +  Twine WrappedName = "__wrap_" + Name;
>> +  Twine RealName = "__real_" + Name;
> 
> These temporary Twines are not safe and they are only used once anyway.

Changing this to strings.

> 
> Cheers,
> Rafael
> 



More information about the llvm-commits mailing list