From 7ffc5bb51ad3b086eaf68d257ae5c25840a64bf4 Mon Sep 17 00:00:00 2001 From: Yuri Gorshenin Date: Tue, 21 Oct 2014 10:22:27 +0000 Subject: [PATCH] [asan-asm-instrumentation] Fixed memory accesses with rbp as a base or an index register. Summary: Fixed memory accesses with rbp as a base or an index register. Reviewers: eugenis Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D5819 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220283 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../X86/AsmParser/X86AsmInstrumentation.cpp | 184 ++++++++++++------ .../AddressSanitizer/X86/asm_cfi.s | 34 ++-- 2 files changed, 135 insertions(+), 83 deletions(-) diff --git a/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp b/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp index dbc5f281461..ae6a5c9f036 100644 --- a/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp +++ b/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp @@ -27,6 +27,8 @@ #include "llvm/MC/MCTargetOptions.h" #include "llvm/Support/CommandLine.h" #include +#include +#include namespace llvm { namespace { @@ -61,26 +63,60 @@ std::string FuncName(unsigned AccessSize, bool IsWrite) { class X86AddressSanitizer : public X86AsmInstrumentation { public: struct RegisterContext { + private: + enum RegOffset { + REG_OFFSET_ADDRESS = 0, + REG_OFFSET_SHADOW, + REG_OFFSET_SCRATCH + }; + + public: RegisterContext(unsigned AddressReg, unsigned ShadowReg, - unsigned ScratchReg) - : AddressReg(AddressReg), ShadowReg(ShadowReg), ScratchReg(ScratchReg) { + unsigned ScratchReg) { + for (unsigned Reg : { AddressReg, ShadowReg, ScratchReg }) { + BusyRegs.push_back(convReg(Reg, MVT::i64)); + } + } + + unsigned AddressReg(MVT::SimpleValueType VT) const { + return convReg(BusyRegs[REG_OFFSET_ADDRESS], VT); + } + + unsigned ShadowReg(MVT::SimpleValueType VT) const { + return convReg(BusyRegs[REG_OFFSET_SHADOW], VT); + } + + unsigned ScratchReg(MVT::SimpleValueType VT) const { + return convReg(BusyRegs[REG_OFFSET_SCRATCH], VT); + } + + void AddBusyReg(unsigned Reg) { + if (Reg != X86::NoRegister) + BusyRegs.push_back(convReg(Reg, MVT::i64)); } - unsigned addressReg(MVT::SimpleValueType VT) const { - return getX86SubSuperRegister(AddressReg, VT); + void AddBusyRegs(const X86Operand &Op) { + AddBusyReg(Op.getMemBaseReg()); + AddBusyReg(Op.getMemIndexReg()); } - unsigned shadowReg(MVT::SimpleValueType VT) const { - return getX86SubSuperRegister(ShadowReg, VT); + unsigned ChooseFrameReg(MVT::SimpleValueType VT) const { + static const unsigned Candidates[] = { X86::RBP, X86::RAX, X86::RBX, + X86::RCX, X86::RDX, X86::RDI, + X86::RSI }; + for (unsigned Reg : Candidates) { + if (!std::count(BusyRegs.begin(), BusyRegs.end(), Reg)) + return convReg(Reg, VT); + } + return X86::NoRegister; } - unsigned scratchReg(MVT::SimpleValueType VT) const { - return getX86SubSuperRegister(ScratchReg, VT); + private: + unsigned convReg(unsigned Reg, MVT::SimpleValueType VT) const { + return Reg == X86::NoRegister ? Reg : getX86SubSuperRegister(Reg, VT); } - const unsigned AddressReg; - const unsigned ShadowReg; - const unsigned ScratchReg; + std::vector BusyRegs; }; X86AddressSanitizer(const MCSubtargetInfo &STI) @@ -191,6 +227,9 @@ void X86AddressSanitizer::InstrumentMOVSBase(unsigned DstReg, unsigned SrcReg, IsSmallMemAccess(AccessSize) ? X86::RBX : X86::NoRegister /* ScratchReg */); + RegCtx.AddBusyReg(DstReg); + RegCtx.AddBusyReg(SrcReg); + RegCtx.AddBusyReg(CntReg); InstrumentMemOperandPrologue(RegCtx, Ctx, Out); @@ -297,16 +336,17 @@ void X86AddressSanitizer::InstrumentMOV(const MCInst &Inst, } const bool IsWrite = MII.get(Inst.getOpcode()).mayStore(); - RegisterContext RegCtx(X86::RDI /* AddressReg */, X86::RAX /* ShadowReg */, - IsSmallMemAccess(AccessSize) - ? X86::RCX - : X86::NoRegister /* ScratchReg */); for (unsigned Ix = 0; Ix < Operands.size(); ++Ix) { assert(Operands[Ix]); MCParsedAsmOperand &Op = *Operands[Ix]; if (Op.isMem()) { X86Operand &MemOp = static_cast(Op); + RegisterContext RegCtx( + X86::RDI /* AddressReg */, X86::RAX /* ShadowReg */, + IsSmallMemAccess(AccessSize) ? X86::RCX + : X86::NoRegister /* ScratchReg */); + RegCtx.AddBusyRegs(MemOp); InstrumentMemOperandPrologue(RegCtx, Ctx, Out); InstrumentMemOperand(MemOp, AccessSize, IsWrite, RegCtx, Ctx, Out); InstrumentMemOperandEpilogue(RegCtx, Ctx, Out); @@ -414,42 +454,51 @@ public: virtual void InstrumentMemOperandPrologue(const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) override { + unsigned LocalFrameReg = RegCtx.ChooseFrameReg(MVT::i32); + assert(LocalFrameReg != X86::NoRegister); + const MCRegisterInfo *MRI = Ctx.getRegisterInfo(); unsigned FrameReg = GetFrameReg(Ctx, Out); if (MRI && FrameReg != X86::NoRegister) { - SpillReg(Out, X86::EBP); + SpillReg(Out, LocalFrameReg); if (FrameReg == X86::ESP) { - Out.EmitCFIAdjustCfaOffset(4 /* byte size of the FrameReg */); - Out.EmitCFIRelOffset(MRI->getDwarfRegNum(X86::EBP, true /* IsEH */), 0); + Out.EmitCFIAdjustCfaOffset(4 /* byte size of the LocalFrameReg */); + Out.EmitCFIRelOffset( + MRI->getDwarfRegNum(LocalFrameReg, true /* IsEH */), 0); } EmitInstruction( - Out, MCInstBuilder(X86::MOV32rr).addReg(X86::EBP).addReg(FrameReg)); + Out, + MCInstBuilder(X86::MOV32rr).addReg(LocalFrameReg).addReg(FrameReg)); Out.EmitCFIRememberState(); - Out.EmitCFIDefCfaRegister(MRI->getDwarfRegNum(X86::EBP, true /* IsEH */)); + Out.EmitCFIDefCfaRegister( + MRI->getDwarfRegNum(LocalFrameReg, true /* IsEH */)); } - SpillReg(Out, RegCtx.addressReg(MVT::i32)); - SpillReg(Out, RegCtx.shadowReg(MVT::i32)); - if (RegCtx.ScratchReg != X86::NoRegister) - SpillReg(Out, RegCtx.scratchReg(MVT::i32)); + SpillReg(Out, RegCtx.AddressReg(MVT::i32)); + SpillReg(Out, RegCtx.ShadowReg(MVT::i32)); + if (RegCtx.ScratchReg(MVT::i32) != X86::NoRegister) + SpillReg(Out, RegCtx.ScratchReg(MVT::i32)); StoreFlags(Out); } virtual void InstrumentMemOperandEpilogue(const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) override { + unsigned LocalFrameReg = RegCtx.ChooseFrameReg(MVT::i32); + assert(LocalFrameReg != X86::NoRegister); + RestoreFlags(Out); - if (RegCtx.ScratchReg != X86::NoRegister) - RestoreReg(Out, RegCtx.scratchReg(MVT::i32)); - RestoreReg(Out, RegCtx.shadowReg(MVT::i32)); - RestoreReg(Out, RegCtx.addressReg(MVT::i32)); + if (RegCtx.ScratchReg(MVT::i32) != X86::NoRegister) + RestoreReg(Out, RegCtx.ScratchReg(MVT::i32)); + RestoreReg(Out, RegCtx.ShadowReg(MVT::i32)); + RestoreReg(Out, RegCtx.AddressReg(MVT::i32)); unsigned FrameReg = GetFrameReg(Ctx, Out); if (Ctx.getRegisterInfo() && FrameReg != X86::NoRegister) { - RestoreReg(Out, X86::EBP); + RestoreReg(Out, LocalFrameReg); Out.EmitCFIRestoreState(); if (FrameReg == X86::ESP) - Out.EmitCFIAdjustCfaOffset(-4 /* byte size of the FrameReg */); + Out.EmitCFIAdjustCfaOffset(-4 /* byte size of the LocalFrameReg */); } } @@ -477,7 +526,7 @@ private: .addReg(X86::ESP) .addImm(-16)); EmitInstruction( - Out, MCInstBuilder(X86::PUSH32r).addReg(RegCtx.addressReg(MVT::i32))); + Out, MCInstBuilder(X86::PUSH32r).addReg(RegCtx.AddressReg(MVT::i32))); const std::string &Fn = FuncName(AccessSize, IsWrite); MCSymbol *FnSym = Ctx.GetOrCreateSymbol(StringRef(Fn)); @@ -490,12 +539,12 @@ private: void X86AddressSanitizer32::InstrumentMemOperandSmall( X86Operand &Op, unsigned AccessSize, bool IsWrite, const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) { - unsigned AddressRegI32 = RegCtx.addressReg(MVT::i32); - unsigned ShadowRegI32 = RegCtx.shadowReg(MVT::i32); - unsigned ShadowRegI8 = RegCtx.shadowReg(MVT::i8); + unsigned AddressRegI32 = RegCtx.AddressReg(MVT::i32); + unsigned ShadowRegI32 = RegCtx.ShadowReg(MVT::i32); + unsigned ShadowRegI8 = RegCtx.ShadowReg(MVT::i8); - assert(RegCtx.ScratchReg != X86::NoRegister); - unsigned ScratchRegI32 = RegCtx.scratchReg(MVT::i32); + assert(RegCtx.ScratchReg(MVT::i32) != X86::NoRegister); + unsigned ScratchRegI32 = RegCtx.ScratchReg(MVT::i32); ComputeMemOperandAddress(Op, MVT::i32, AddressRegI32, Ctx, Out); @@ -565,8 +614,8 @@ void X86AddressSanitizer32::InstrumentMemOperandSmall( void X86AddressSanitizer32::InstrumentMemOperandLarge( X86Operand &Op, unsigned AccessSize, bool IsWrite, const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) { - unsigned AddressRegI32 = RegCtx.addressReg(MVT::i32); - unsigned ShadowRegI32 = RegCtx.shadowReg(MVT::i32); + unsigned AddressRegI32 = RegCtx.AddressReg(MVT::i32); + unsigned ShadowRegI32 = RegCtx.ShadowReg(MVT::i32); ComputeMemOperandAddress(Op, MVT::i32, AddressRegI32, Ctx, Out); @@ -663,44 +712,53 @@ public: virtual void InstrumentMemOperandPrologue(const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) override { + unsigned LocalFrameReg = RegCtx.ChooseFrameReg(MVT::i64); + assert(LocalFrameReg != X86::NoRegister); + const MCRegisterInfo *MRI = Ctx.getRegisterInfo(); unsigned FrameReg = GetFrameReg(Ctx, Out); if (MRI && FrameReg != X86::NoRegister) { SpillReg(Out, X86::RBP); if (FrameReg == X86::RSP) { - Out.EmitCFIAdjustCfaOffset(8 /* byte size of the FrameReg */); - Out.EmitCFIRelOffset(MRI->getDwarfRegNum(X86::RBP, true /* IsEH */), 0); + Out.EmitCFIAdjustCfaOffset(8 /* byte size of the LocalFrameReg */); + Out.EmitCFIRelOffset( + MRI->getDwarfRegNum(LocalFrameReg, true /* IsEH */), 0); } EmitInstruction( - Out, MCInstBuilder(X86::MOV64rr).addReg(X86::RBP).addReg(FrameReg)); + Out, + MCInstBuilder(X86::MOV64rr).addReg(LocalFrameReg).addReg(FrameReg)); Out.EmitCFIRememberState(); - Out.EmitCFIDefCfaRegister(MRI->getDwarfRegNum(X86::RBP, true /* IsEH */)); + Out.EmitCFIDefCfaRegister( + MRI->getDwarfRegNum(LocalFrameReg, true /* IsEH */)); } EmitAdjustRSP(Ctx, Out, -128); - SpillReg(Out, RegCtx.shadowReg(MVT::i64)); - SpillReg(Out, RegCtx.addressReg(MVT::i64)); - if (RegCtx.ScratchReg != X86::NoRegister) - SpillReg(Out, RegCtx.scratchReg(MVT::i64)); + SpillReg(Out, RegCtx.ShadowReg(MVT::i64)); + SpillReg(Out, RegCtx.AddressReg(MVT::i64)); + if (RegCtx.ScratchReg(MVT::i64) != X86::NoRegister) + SpillReg(Out, RegCtx.ScratchReg(MVT::i64)); StoreFlags(Out); } virtual void InstrumentMemOperandEpilogue(const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) override { + unsigned LocalFrameReg = RegCtx.ChooseFrameReg(MVT::i64); + assert(LocalFrameReg != X86::NoRegister); + RestoreFlags(Out); - if (RegCtx.ScratchReg != X86::NoRegister) - RestoreReg(Out, RegCtx.scratchReg(MVT::i64)); - RestoreReg(Out, RegCtx.addressReg(MVT::i64)); - RestoreReg(Out, RegCtx.shadowReg(MVT::i64)); + if (RegCtx.ScratchReg(MVT::i64) != X86::NoRegister) + RestoreReg(Out, RegCtx.ScratchReg(MVT::i64)); + RestoreReg(Out, RegCtx.AddressReg(MVT::i64)); + RestoreReg(Out, RegCtx.ShadowReg(MVT::i64)); EmitAdjustRSP(Ctx, Out, 128); unsigned FrameReg = GetFrameReg(Ctx, Out); if (Ctx.getRegisterInfo() && FrameReg != X86::NoRegister) { - RestoreReg(Out, X86::RBP); + RestoreReg(Out, LocalFrameReg); Out.EmitCFIRestoreState(); if (FrameReg == X86::RSP) - Out.EmitCFIAdjustCfaOffset(-8 /* byte size of the FrameReg */); + Out.EmitCFIAdjustCfaOffset(-8 /* byte size of the LocalFrameReg */); } } @@ -736,9 +794,9 @@ private: .addReg(X86::RSP) .addImm(-16)); - if (RegCtx.AddressReg != X86::RDI) { + if (RegCtx.AddressReg(MVT::i64) != X86::RDI) { EmitInstruction(Out, MCInstBuilder(X86::MOV64rr).addReg(X86::RDI).addReg( - RegCtx.addressReg(MVT::i64))); + RegCtx.AddressReg(MVT::i64))); } const std::string &Fn = FuncName(AccessSize, IsWrite); MCSymbol *FnSym = Ctx.GetOrCreateSymbol(StringRef(Fn)); @@ -751,14 +809,14 @@ private: void X86AddressSanitizer64::InstrumentMemOperandSmall( X86Operand &Op, unsigned AccessSize, bool IsWrite, const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) { - unsigned AddressRegI64 = RegCtx.addressReg(MVT::i64); - unsigned AddressRegI32 = RegCtx.addressReg(MVT::i32); - unsigned ShadowRegI64 = RegCtx.shadowReg(MVT::i64); - unsigned ShadowRegI32 = RegCtx.shadowReg(MVT::i32); - unsigned ShadowRegI8 = RegCtx.shadowReg(MVT::i8); + unsigned AddressRegI64 = RegCtx.AddressReg(MVT::i64); + unsigned AddressRegI32 = RegCtx.AddressReg(MVT::i32); + unsigned ShadowRegI64 = RegCtx.ShadowReg(MVT::i64); + unsigned ShadowRegI32 = RegCtx.ShadowReg(MVT::i32); + unsigned ShadowRegI8 = RegCtx.ShadowReg(MVT::i8); - assert(RegCtx.ScratchReg != X86::NoRegister); - unsigned ScratchRegI32 = RegCtx.scratchReg(MVT::i32); + assert(RegCtx.ScratchReg(MVT::i32) != X86::NoRegister); + unsigned ScratchRegI32 = RegCtx.ScratchReg(MVT::i32); ComputeMemOperandAddress(Op, MVT::i64, AddressRegI64, Ctx, Out); @@ -827,8 +885,8 @@ void X86AddressSanitizer64::InstrumentMemOperandSmall( void X86AddressSanitizer64::InstrumentMemOperandLarge( X86Operand &Op, unsigned AccessSize, bool IsWrite, const RegisterContext &RegCtx, MCContext &Ctx, MCStreamer &Out) { - unsigned AddressRegI64 = RegCtx.addressReg(MVT::i64); - unsigned ShadowRegI64 = RegCtx.shadowReg(MVT::i64); + unsigned AddressRegI64 = RegCtx.AddressReg(MVT::i64); + unsigned ShadowRegI64 = RegCtx.ShadowReg(MVT::i64); ComputeMemOperandAddress(Op, MVT::i64, AddressRegI64, Ctx, Out); diff --git a/test/Instrumentation/AddressSanitizer/X86/asm_cfi.s b/test/Instrumentation/AddressSanitizer/X86/asm_cfi.s index ff7e4f76a75..417d7f3fd3e 100644 --- a/test/Instrumentation/AddressSanitizer/X86/asm_cfi.s +++ b/test/Instrumentation/AddressSanitizer/X86/asm_cfi.s @@ -3,20 +3,20 @@ # RUN: llvm-mc %s -triple=i386-unknown-linux-gnu -asm-instrumentation=address -asan-instrument-assembly | FileCheck %s -# CHECK-LABEL: swap_cfa_rbp -# CHECK: pushl %ebp +# CHECK-LABEL: load4b_cfa_rbp +# CHECK: pushl %ebx # CHECK-NOT: .cfi_adjust_cfa_offset 8 -# CHECK: movl %ebp, %ebp +# CHECK: movl %ebp, %ebx # CHECK: .cfi_remember_state -# CHECK: .cfi_def_cfa_register %ebp -# CHECK: popl %ebp +# CHECK: .cfi_def_cfa_register %ebx +# CHECK: popl %ebx # CHECK: .cfi_restore_state # CHECK-NOT: .cfi_adjust_cfa_offset -8 # CHECK: retl .text - .globl swap_cfa_rbp - .type swap_cfa_rbp,@function + .globl load4b_cfa_rbp + .type load4b_cfa_rbp,@function swap_cfa_rbp: # @swap_cfa_rbp .cfi_startproc pushl %ebp @@ -25,34 +25,28 @@ swap_cfa_rbp: # @swap_cfa_rbp movl %esp, %ebp .cfi_def_cfa_register %ebp movl 8(%ebp), %eax - movl 12(%ebp), %ecx - movl (%ecx), %ecx - movl %ecx, (%eax) popl %ebp retl .cfi_endproc -# CHECK-LABEL: swap_cfa_rsp -# CHECK: pushl %ebp +# CHECK-LABEL: load4b_cfa_rsp +# CHECK: pushl %ebx # CHECK: .cfi_adjust_cfa_offset 4 -# CHECK: movl %esp, %ebp +# CHECK: movl %esp, %ebx # CHECK: .cfi_remember_state -# CHECK: .cfi_def_cfa_register %ebp -# CHECK: popl %ebp +# CHECK: .cfi_def_cfa_register %ebx +# CHECK: popl %ebx # CHECK: .cfi_restore_state # CHECK: retl - .globl swap_cfa_rsp - .type swap_cfa_rsp,@function + .globl load4b_cfa_rsp + .type load4b_cfa_rsp,@function swap_cfa_rsp: # @swap_cfa_rsp .cfi_startproc pushl %ebp .cfi_offset %ebp, 0 movl %esp, %ebp movl 8(%ebp), %eax - movl 12(%ebp), %ecx - movl (%ecx), %ecx - movl %ecx, (%eax) popl %ebp retl .cfi_endproc -- 2.34.1