[asan-assembly-instrumentation] Added CFI directives to the generated instrumentation code.

[ygorshenin]

Summary

[asan-assembly-instrumentation] Added CFI directives to the generated instrumentation code.

[eugenis]

Tests please.

[eugenis]

include/llvm/MC/MCTargetAsmParser.h

126 ↗

(On Diff #13250)

Please add a comment.
How does it behave when parsing top-level asm?

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp

316 ↗

(On Diff #13250)

Don't forget end-to-end tests for both cases (esp and non-esp), i.e. omit-frame-pointer and no-omit-frame-pointer.

557 ↗

(On Diff #13250)

Is it the right way to obtain MCRegisterInfo?
Direct calls to TheXXXTarget seem to be rather uncommon.

[ygorshenin]

Fixes.

[ygorshenin]

PTAL

End-to-end test is added in http://reviews.llvm.org/D5260.

include/llvm/MC/MCTargetAsmParser.h

126 ↗

(On Diff #13250)

Done.

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp

557 ↗

(On Diff #13250)

It seems that it's possible to get MCRegisterInfo from MCContext, thanks!

[eugenis]

include/llvm/MC/MCTargetAsmParser.h

127 ↗

(On Diff #13446)

It's better to document what the function does (or does not do), rather then when it is called.
For example, it looks like FrameReg stays uninitialized if it is not called, resulting in undefined behavior for top-level asm.

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll

45 ↗

(On Diff #13446)

attribute 1 and 3 seem to be unused
maybe remove all the fp-math stuff from 2 and 4, too

49 ↗

(On Diff #13446)

remove this, and all !srcloc.

[ygorshenin]

Fixes.

[ygorshenin]

PTAL

include/llvm/MC/MCTargetAsmParser.h

127 ↗

(On Diff #13446)

FrameReg is a field in X86AsmInstrumentation and initialized to X86::NoRegister in the ctor.

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll

45 ↗

(On Diff #13446)

Done.

49 ↗

(On Diff #13446)

Done.

[eugenis]

LGTM

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll

45 ↗

(On Diff #13449)

Please remove "stack-protector-buffer-size"="8", too.

[ygorshenin]

Fixes.

[ygorshenin]

Many thanks!

test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll

45 ↗

(On Diff #13449)

Done.

[ygorshenin]

Closed by commit rL217482 (authored by @ygorshenin).

llvm/trunk/include/llvm/MC/MCTargetAsmParser.h

public:
 
   void setSemaCallback(MCAsmParserSemaCallback *Callback) {
     SemaCallback = Callback;
   }
 
   virtual bool ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
                              SMLoc &EndLoc) = 0;
 
+  /// Sets frame register corresponding to the current MachineFunction.
+  virtual void SetFrameRegister(unsigned RegNo) {}
+
   /// ParseInstruction - Parse one assembly instruction.
   ///
   /// The parser is positioned following the instruction name. The target
   /// specific instruction parser should parse the entire instruction and
   /// construct the appropriate MCInst, or emit an error. On success, the entire
   /// line should be parsed up to and including the end-of-statement token. On
   /// failure, the parser is not required to read to the end of the line.
   //

llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCTargetAsmParser.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
+#include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
 
 namespace {
   struct SrcMgrDiagInfo {
     const MDNode *LocInfo;
if (MF)
     MCOptions = MF->getTarget().Options.MCOptions;
   std::unique_ptr<MCTargetAsmParser> TAP(
       TM.getTarget().createMCAsmParser(*STI, *Parser, *MII, MCOptions));
   if (!TAP)
     report_fatal_error("Inline asm not supported by this streamer because"
                        " we don't have an asm parser for this target\n");
   Parser->setAssemblerDialect(Dialect);
   Parser->setTargetParser(*TAP.get());
+  if (MF) {
+    const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
+    TAP->SetFrameRegister(TRI->getFrameRegister(*MF));
+  }
 
   // Don't implicitly switch to the text section before the asm.
   int Res = Parser->Run(/*NoInitialTextSection*/ true,
                         /*NoFinalize*/ true);
   emitInlineAsmEnd(STIOrig, STI.get());
   if (Res && !HasDiagHandler)
     report_fatal_error("Error parsing inline asm\n");
 }

llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.h

 X86AsmInstrumentation *
 CreateX86AsmInstrumentation(const MCTargetOptions &MCOptions,
                             const MCContext &Ctx, const MCSubtargetInfo &STI);
 
 class X86AsmInstrumentation {
 public:
   virtual ~X86AsmInstrumentation();
 
+  void SetFrameRegister(unsigned RegNo) {
+    FrameReg = RegNo;
+  }
+
   // Tries to instrument and emit instruction.
   virtual void InstrumentAndEmitInstruction(
       const MCInst &Inst,
       SmallVectorImpl<std::unique_ptr<MCParsedAsmOperand> > &Operands,
       MCContext &Ctx, const MCInstrInfo &MII, MCStreamer &Out);
 
 protected:
   friend X86AsmInstrumentation *
   CreateX86AsmInstrumentation(const MCTargetOptions &MCOptions,
                               const MCContext &Ctx, const MCSubtargetInfo &STI);
 
   X86AsmInstrumentation(const MCSubtargetInfo &STI);
 
   void EmitInstruction(MCStreamer &Out, const MCInst &Inst);
 
   const MCSubtargetInfo &STI;
+
+  unsigned FrameReg;
 };
 
 } // End llvm namespace
 
 #endif

llvm/trunk/lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp

 }
 
 class X86AddressSanitizer32 : public X86AddressSanitizer {
 public:
   static const long kShadowOffset = 0x20000000;
 
   X86AddressSanitizer32(const MCSubtargetInfo &STI)
       : X86AddressSanitizer(STI) {}
+
   virtual ~X86AddressSanitizer32() {}
 
   virtual void StoreFlags(MCStreamer &Out) override {
     EmitInstruction(Out, MCInstBuilder(X86::PUSHF32));
   }
 
   virtual void RestoreFlags(MCStreamer &Out) override {
     EmitInstruction(Out, MCInstBuilder(X86::POPF32));
   }
 
   virtual void InstrumentMemOperandPrologue(const RegisterContext &RegCtx,
                                             MCContext &Ctx,
                                             MCStreamer &Out) override {
+    const MCRegisterInfo* MRI = Ctx.getRegisterInfo();
+    if (MRI && FrameReg != X86::NoRegister) {
+      EmitInstruction(
+          Out, MCInstBuilder(X86::PUSH32r).addReg(X86::EBP));
+      if (FrameReg == X86::ESP) {
+        Out.EmitCFIAdjustCfaOffset(4 /* byte size of the FrameReg */);
+        Out.EmitCFIRelOffset(
+            MRI->getDwarfRegNum(X86::EBP, true /* IsEH */), 0);
+      }
+      EmitInstruction(
+          Out, MCInstBuilder(X86::MOV32rr).addReg(X86::EBP).addReg(FrameReg));
+      Out.EmitCFIRememberState();
+      Out.EmitCFIDefCfaRegister(
+          MRI->getDwarfRegNum(X86::EBP, true /* IsEH */));
+    }
+
     EmitInstruction(
         Out, MCInstBuilder(X86::PUSH32r).addReg(RegCtx.addressReg(MVT::i32)));
     EmitInstruction(
         Out, MCInstBuilder(X86::PUSH32r).addReg(RegCtx.shadowReg(MVT::i32)));
     if (RegCtx.ScratchReg != X86::NoRegister) {
       EmitInstruction(
           Out, MCInstBuilder(X86::PUSH32r).addReg(RegCtx.scratchReg(MVT::i32)));
     }
     StoreFlags(Out);
   }
 
   virtual void InstrumentMemOperandEpilogue(const RegisterContext &RegCtx,
                                             MCContext &Ctx,
                                             MCStreamer &Out) override {
     RestoreFlags(Out);
     if (RegCtx.ScratchReg != X86::NoRegister) {
       EmitInstruction(
           Out, MCInstBuilder(X86::POP32r).addReg(RegCtx.scratchReg(MVT::i32)));
     }
     EmitInstruction(
         Out, MCInstBuilder(X86::POP32r).addReg(RegCtx.shadowReg(MVT::i32)));
     EmitInstruction(
         Out, MCInstBuilder(X86::POP32r).addReg(RegCtx.addressReg(MVT::i32)));
+
+    if (Ctx.getRegisterInfo() && FrameReg != X86::NoRegister) {
+      EmitInstruction(
+          Out, MCInstBuilder(X86::POP32r).addReg(X86::EBP));
+      Out.EmitCFIRestoreState();
+      if (FrameReg == X86::ESP)
+        Out.EmitCFIAdjustCfaOffset(-4 /* byte size of the FrameReg */);
+    }
   }
 
   virtual void InstrumentMemOperandSmall(X86Operand &Op, unsigned AccessSize,
                                          bool IsWrite,
                                          const RegisterContext &RegCtx,
                                          MCContext &Ctx,
                                          MCStreamer &Out) override;
   virtual void InstrumentMemOperandLarge(X86Operand &Op, unsigned AccessSize,

 }
 
 class X86AddressSanitizer64 : public X86AddressSanitizer {
 public:
   static const long kShadowOffset = 0x7fff8000;
 
   X86AddressSanitizer64(const MCSubtargetInfo &STI)
       : X86AddressSanitizer(STI) {}
+
   virtual ~X86AddressSanitizer64() {}
 
   virtual void StoreFlags(MCStreamer &Out) override {
     EmitInstruction(Out, MCInstBuilder(X86::PUSHF64));
   }
 
   virtual void RestoreFlags(MCStreamer &Out) override {
     EmitInstruction(Out, MCInstBuilder(X86::POPF64));
   }
 
   virtual void InstrumentMemOperandPrologue(const RegisterContext &RegCtx,
                                             MCContext &Ctx,
                                             MCStreamer &Out) override {
+    const MCRegisterInfo *RegisterInfo = Ctx.getRegisterInfo();
+    if (RegisterInfo && FrameReg != X86::NoRegister) {
+      EmitInstruction(Out, MCInstBuilder(X86::PUSH64r).addReg(X86::RBP));
+      if (FrameReg == X86::RSP) {
+        Out.EmitCFIAdjustCfaOffset(8 /* byte size of the FrameReg */);
+        Out.EmitCFIRelOffset(
+            RegisterInfo->getDwarfRegNum(X86::RBP, true /* IsEH */), 0);
+      }
+      EmitInstruction(
+          Out, MCInstBuilder(X86::MOV64rr).addReg(X86::RBP).addReg(FrameReg));
+      Out.EmitCFIRememberState();
+      Out.EmitCFIDefCfaRegister(
+          RegisterInfo->getDwarfRegNum(X86::RBP, true /* IsEH */));
+    }
+
     EmitAdjustRSP(Ctx, Out, -128);
     EmitInstruction(
         Out, MCInstBuilder(X86::PUSH64r).addReg(RegCtx.shadowReg(MVT::i64)));
     EmitInstruction(
         Out, MCInstBuilder(X86::PUSH64r).addReg(RegCtx.addressReg(MVT::i64)));
     if (RegCtx.ScratchReg != X86::NoRegister) {
       EmitInstruction(
           Out, MCInstBuilder(X86::PUSH64r).addReg(RegCtx.scratchReg(MVT::i64)));
if (RegCtx.ScratchReg != X86::NoRegister) {
       EmitInstruction(
           Out, MCInstBuilder(X86::POP64r).addReg(RegCtx.scratchReg(MVT::i64)));
     }
     EmitInstruction(
         Out, MCInstBuilder(X86::POP64r).addReg(RegCtx.addressReg(MVT::i64)));
     EmitInstruction(
         Out, MCInstBuilder(X86::POP64r).addReg(RegCtx.shadowReg(MVT::i64)));
     EmitAdjustRSP(Ctx, Out, 128);
+
+    if (Ctx.getRegisterInfo() && FrameReg != X86::NoRegister) {
+      EmitInstruction(
+          Out, MCInstBuilder(X86::POP64r).addReg(X86::RBP));
+      Out.EmitCFIRestoreState();
+      if (FrameReg == X86::RSP)
+        Out.EmitCFIAdjustCfaOffset(-8 /* byte size of the FrameReg */);
+    }
   }
 
   virtual void InstrumentMemOperandSmall(X86Operand &Op, unsigned AccessSize,
                                          bool IsWrite,
                                          const RegisterContext &RegCtx,
                                          MCContext &Ctx,
                                          MCStreamer &Out) override;
   virtual void InstrumentMemOperandLarge(X86Operand &Op, unsigned AccessSize,
void X86AddressSanitizer64::InstrumentMOVSImpl(unsigned AccessSize,
 
   EmitLabel(Out, DoneSym);
   RestoreFlags(Out);
 }
 
 } // End anonymous namespace
 
 X86AsmInstrumentation::X86AsmInstrumentation(const MCSubtargetInfo &STI)
-    : STI(STI) {}
+    : STI(STI), FrameReg(X86::NoRegister) {}
 
 X86AsmInstrumentation::~X86AsmInstrumentation() {}
 
 void X86AsmInstrumentation::InstrumentAndEmitInstruction(
     const MCInst &Inst, OperandVector &Operands, MCContext &Ctx,
     const MCInstrInfo &MII, MCStreamer &Out) {
   EmitInstruction(Out, Inst);
 }

llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp

X86AsmParser(MCSubtargetInfo &sti, MCAsmParser &parser,
     // Initialize the set of available features.
     setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
     Instrumentation.reset(
         CreateX86AsmInstrumentation(Options, Parser.getContext(), STI));
   }
 
   bool ParseRegister(unsigned &RegNo, SMLoc &StartLoc, SMLoc &EndLoc) override;
 
+  void SetFrameRegister(unsigned RegNo) override;
+
   bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
                         SMLoc NameLoc, OperandVector &Operands) override;
 
   bool ParseDirective(AsmToken DirectiveID) override;
 };
 } // end anonymous namespace
 
 /// @name Auto-generated Match Functions
if (RegNo == 0) {
     return Error(StartLoc, "invalid register name",
                  SMRange(StartLoc, EndLoc));
   }
 
   Parser.Lex(); // Eat identifier token.
   return false;
 }
 
+void X86AsmParser::SetFrameRegister(unsigned RegNo) {
+  Instrumentation->SetFrameRegister(RegNo);
+}
+
 std::unique_ptr<X86Operand> X86AsmParser::DefaultMemSIOperand(SMLoc Loc) {
   unsigned basereg =
     is64BitMode() ? X86::RSI : (is32BitMode() ? X86::ESI : X86::SI);
   const MCExpr *Disp = MCConstantExpr::Create(0, getContext());
   return X86Operand::CreateMem(/*SegReg=*/0, Disp, /*BaseReg=*/basereg,
                                /*IndexReg=*/0, /*Scale=*/1, Loc, Loc, 0);
 }
 

llvm/trunk/test/Instrumentation/AddressSanitizer/X86/asm_cfi.ll

+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=corei7 -mattr=+sse2 -asm-instrumentation=address -asan-instrument-assembly | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: mov8b_rbp
+; CHECK: pushq %rbp
+; CHECK-NOT: .cfi_adjust_cfa_offset 8
+; CHECK: movq %rbp, %rbp
+; CHECK: .cfi_remember_state
+; CHECK: .cfi_def_cfa_register %rbp
+; CHECK: leaq -128(%rsp)
+; CHECK: callq __asan_report_load8@PLT
+; CHECK: leaq 128(%rsp)
+; CHECK: popq %rbp
+; CHECK: .cfi_restore_state
+; CHECK-NOT: .cfi_adjust_cfa_offset -8
+; CHECK: retq
+define void @mov8b_rbp(i64* %dst, i64* %src) #0 {
+entry:
+  tail call void asm sideeffect "movq ($0), %rax \0A\09movq %rax, ($1) \0A\09", "r,r,~{rax},~{memory},~{dirflag},~{fpsr},~{flags}"(i64* %src, i64* %dst)
+  ret void
+}
+
+; CHECK-LABEL: mov8b_rsp
+; CHECK: pushq %rbp
+; CHECK: .cfi_adjust_cfa_offset 8
+; CHECK: movq %rsp, %rbp
+; CHECK: .cfi_remember_state
+; CHECK: .cfi_def_cfa_register %rbp
+; CHECK: leaq -128(%rsp)
+; CHECK: callq __asan_report_load8@PLT
+; CHECK: leaq 128(%rsp)
+; CHECK: popq %rbp
+; CHECK: .cfi_restore_state
+; CHECK: .cfi_adjust_cfa_offset -8
+; CHECK: retq
+define void @mov8b_rsp(i64* %dst, i64* %src) #1 {
+entry:
+  tail call void asm sideeffect "movq ($0), %rax \0A\09movq %rax, ($1) \0A\09", "r,r,~{rax},~{memory},~{dirflag},~{fpsr},~{flags}"(i64* %src, i64* %dst)
+  ret void
+}
+
+attributes #0 = { nounwind sanitize_address uwtable "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" }
+attributes #1 = { nounwind sanitize_address uwtable "no-frame-pointer-elim"="false" }