[llvm] r208400 - [asan] Stop leaking X86Operands.

[Benjamin Kramer]

On 09.05.2014, at 17:15, David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, May 9, 2014 at 2:48 AM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> Author: d0k
>> Date: Fri May  9 04:48:03 2014
>> New Revision: 208400
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=208400&view=rev
>> Log:
>> [asan] Stop leaking X86Operands.
>> 
>> Modified:
>>    llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp
>> 
>> Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp?rev=208400&r1=208399&r2=208400&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp Fri May  9 04:48:03 2014
>> @@ -192,8 +192,8 @@ void X86AddressSanitizer64::InstrumentMe
>>     Inst.addOperand(MCOperand::CreateReg(X86::RSP));
>> 
>>     const MCExpr *Disp = MCConstantExpr::Create(-128, Ctx);
>> -    X86Operand *Op =
>> -        X86Operand::CreateMem(0, Disp, X86::RSP, 0, 1, SMLoc(), SMLoc());
>> +    std::unique_ptr<X86Operand> Op(
>> +        X86Operand::CreateMem(0, Disp, X86::RSP, 0, 1, SMLoc(), SMLoc()));
>>     Op->addMemOperands(Inst, 5);
> 
> This is a curious API... where does 'Op' go after these two lines? Is
> it destroyed, or is its ownership somehow transferred (I don't see any
> std::move, etc)
> 
> Why would we be creating temporary X86Operands that are disposed of
> like this? (I guess the answer is in "addMemOperands" somehow)

addMemOperands just takes the data out of the operand and adds it to an MCInst, we can get rid of the operand when that's done. X86Operand was never designed to be used outside of asm parsing and is in need of some refactoring :/

- Ben

> 
>>     EmitInstruction(Out, Inst);
>>   }
>> @@ -221,8 +221,8 @@ void X86AddressSanitizer64::InstrumentMe
>>     Inst.addOperand(MCOperand::CreateReg(X86::RSP));
>> 
>>     const MCExpr *Disp = MCConstantExpr::Create(128, Ctx);
>> -    X86Operand *Op =
>> -        X86Operand::CreateMem(0, Disp, X86::RSP, 0, 1, SMLoc(), SMLoc());
>> +    std::unique_ptr<X86Operand> Op(
>> +        X86Operand::CreateMem(0, Disp, X86::RSP, 0, 1, SMLoc(), SMLoc()));
>>     Op->addMemOperands(Inst, 5);
>>     EmitInstruction(Out, Inst);
>>   }
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits