[llvm] r268896 - [mips][microMIPS] Implement LWP and SWP instructions

[Daniel Sanders via llvm-commits]

I see the problem. There's two members of Op->RegIdx that aren't copied in the new version of CreateRegPair().

From: NAKAMURA Takumi [mailto:geek4civic at gmail.com]
Sent: 09 May 2016 17:15
To: Daniel Sanders; Benjamin Kramer; Zlatko Buljan; llvm-commits at lists.llvm.org
Subject: Re: [llvm] r268896 - [mips][microMIPS] Implement LWP and SWP instructions

It seems this still causes undefined behavior after fixes.

This is the log with MC/Mips/micromips-loadstore-instructions.s.
==55344== Conditional jump or move depends on uninitialised value(s)
==55344==    at 0x54A6C5: (anonymous namespace)::MipsOperand::addRegPairOperands(llvm::MCInst&, unsigned int) const (MipsAsmParser.cpp:1010)
==55344==    by 0x56A55A: (anonymous namespace)::MipsAsmParser::convertToMCInst(unsigned int, llvm::MCInst&, unsigned int, llvm::SmallVectorImpl<std::unique_ptr<llvm::MCParsedAsmOperand, std::default_delete<llvm::MCParsedAsmOperand> > > const&) (MipsGenAsmMatcher.inc:1306)
==55344==    by 0x577314: (anonymous namespace)::MipsAsmParser::MatchInstructionImpl(llvm::SmallVectorImpl<std::unique_ptr<llvm::MCParsedAsmOperand, std::default_delete<llvm::MCParsedAsmOperand> > > const&, llvm::MCInst&, unsigned long&, bool, unsigned int) (MipsGenAsmMatcher.inc:6342)
==55344==    by 0x554808: (anonymous namespace)::MipsAsmParser::MatchAndEmitInstruction(llvm::SMLoc, unsigned int&, llvm::SmallVectorImpl<std::unique_ptr<llvm::MCParsedAsmOperand, std::default_delete<llvm::MCParsedAsmOperand> > >&, llvm::MCStreamer&, unsigned long&, bool) (MipsAsmParser.cpp:3671)
==55344==    by 0x84F03C: (anonymous namespace)::AsmParser::parseStatement((anonymous namespace)::ParseStatementInfo&, llvm::MCAsmParserSemaCallback*) (AsmParser.cpp:1838)
==55344==    by 0x848F11: (anonymous namespace)::AsmParser::Run(bool, bool) (AsmParser.cpp:675)
==55344==    by 0x405F2F: AssembleInput(char const*, llvm::Target const*, llvm::SourceMgr&, llvm::MCContext&, llvm::MCStreamer&, llvm::MCAsmInfo&, llvm::MCSubtargetInfo&, llvm::MCInstrInfo&, llvm::MCTargetOptions&) (llvm-mc.cpp:364)
==55344==    by 0x4070D8: main (llvm-mc.cpp:536)


On Mon, May 9, 2016 at 10:27 PM Daniel Sanders via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
> > @@ -4673,9 +4692,9 @@ MipsAsmParser::parseRegisterPair(Operand
> >
> >    SMLoc E = Parser.getTok().getLoc();
> >    MipsOperand &Op = static_cast<MipsOperand &>(*Operands.back());
> > -  unsigned Reg = Op.getGPR32Reg();
> > +
> >    Operands.pop_back();
> > -  Operands.push_back(MipsOperand::CreateRegPair(Reg, S, E, *this));
> > +  Operands.push_back(MipsOperand::CreateRegPair(Op, S, E, *this));
>
> There's a use after free here because Op is still bound to
> Operands.back().  I tried to fix in r268901, but it broke tests. Can you take a look?

I've applied a quick fix for this in r268913. Op is now a copy of the last element so that Op isn't deleted when pop_back() causes std::unique_ptr<MipsOperand> to delete Operands.back().

I'm not sure why your fix didn't work. It looked ok to me.
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160509/c299a4ea/attachment.html>