[asan-asm-instrumentation] Fixed memory accesses with rbp as a base or an index register.
authorYuri Gorshenin <ygorshenin@google.com>
Tue, 21 Oct 2014 10:22:27 +0000 (10:22 +0000)
committerYuri Gorshenin <ygorshenin@google.com>
Tue, 21 Oct 2014 10:22:27 +0000 (10:22 +0000)
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

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp
test/Instrumentation/AddressSanitizer/X86/asm_cfi.s

index dbc5f28..ae6a5c9 100644 (file)
@@ -27,6 +27,8 @@
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Support/CommandLine.h"
 #include <algorithm>
+#include <cassert>
+#include <vector>
 
 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<unsigned> 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<X86Operand &>(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);
 
index ff7e4f7..417d7f3 100644 (file)
@@ -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