[PATCH] D24659: Trying to fix Mangler memory leak in TargetLoweringObjectFile.

[Mehdi Amini via llvm-commits]

Can you please try to add the proper include and switch to a unique_ptr?



> On Sep 16, 2016, at 9:11 AM, Eric Liu <ioeric at google.com> wrote:
> 
> I actually tried to use unique, but `Mangler` is forward declared in the header, and unique didn't compile...To not further complicate the issue, I didn't pursue further.
> 
> On Fri, Sep 16, 2016 at 5:56 PM Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
> Raises one to me as well, but it was already a pretty large change. :)
> 
> -eric
> 
> On Fri, Sep 16, 2016 at 8:48 AM Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
> I mentioned it in the commit thread: any reason not to use make_unique?
> The delete/new  sequence raise a flag to me :)
> 
> 
> Sent from my iPhone
> On Sep 16, 2016, at 8:37 AM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
> 
>> LGTM. Thanks.
>> 
>> I was wondering why the original code pattern was there - no tests seemed to exercise it - though it looks like asan found it after as well. So thanks for the comment in the code too. :)
>> 
>> 
>> On Fri, Sep 16, 2016, 4:59 AM Eric Liu <ioeric at google.com <mailto:ioeric at google.com>> wrote:
>> This revision was automatically updated to reflect the committed changes.
>> Closed by commit rL281718: Trying to fix Mangler memory leak in TargetLoweringObjectFile. (authored by ioeric).
>> 
>> Changed prior to commit:
>>   https://reviews.llvm.org/D24659?vs=71616&id=71617#toc <https://reviews.llvm.org/D24659?vs=71616&id=71617#toc>
>> 
>> Repository:
>>   rL LLVM
>> 
>> https://reviews.llvm.org/D24659 <https://reviews.llvm.org/D24659>
>> 
>> Files:
>>   llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h
>>   llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>> 
>> Index: llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h
>> ===================================================================
>> --- llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h
>> +++ llvm/trunk/include/llvm/Target/TargetLoweringObjectFile.h
>> @@ -38,7 +38,7 @@
>>    MCContext *Ctx;
>> 
>>    /// Name-mangler for global names.
>> -  Mangler *Mang;
>> +  Mangler *Mang = nullptr;
>> 
>>    TargetLoweringObjectFile(
>>      const TargetLoweringObjectFile&) = delete;
>> Index: llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>> ===================================================================
>> --- llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>> +++ llvm/trunk/lib/Target/TargetLoweringObjectFile.cpp
>> @@ -43,6 +43,8 @@
>>  void TargetLoweringObjectFile::Initialize(MCContext &ctx,
>>                                            const TargetMachine &TM) {
>>    Ctx = &ctx;
>> +  // `Initialize` can be called more than once.
>> +  if (Mang != nullptr) delete Mang;
>>    Mang = new Mangler();
>>    InitMCObjectFileInfo(TM.getTargetTriple(), TM.isPositionIndependent(),
>>                         TM.getCodeModel(), *Ctx);
>> 
>> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160916/437a529c/attachment.html>