[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

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

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp

316

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

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

Done.

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp

557

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

[eugenis]

include/llvm/MC/MCTargetAsmParser.h

127

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

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).

include/llvm/MC/MCTargetAsmParser.h

public:
 
   void setSemaCallback(MCAsmParserSemaCallback *Callback) {
     SemaCallback = Callback;
   }
 
   virtual bool ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
                              SMLoc &EndLoc) = 0;
 
+  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.
   //

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");
 }

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

lib/Target/X86/AsmParser/X86AsmInstrumentation.cpp

 #include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetAsmParser.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/TargetRegistry.h"
 
 namespace llvm {
 namespace {
 
 static cl::opt<bool> ClAsanInstrumentAssembly(
     "asan-instrument-assembly",
     cl::desc("instrument assembly with AddressSanitizer checks"), cl::Hidden,
     cl::init(false));
void X86AddressSanitizer::InstrumentMOV(const MCInst &Inst,
   }
 }
 
 class X86AddressSanitizer32 : public X86AddressSanitizer {
 public:
   static const long kShadowOffset = 0x20000000;
 
   X86AddressSanitizer32(const MCSubtargetInfo &STI)
-      : X86AddressSanitizer(STI) {}
+      : X86AddressSanitizer(STI),
+        RegisterInfo(TheX86_32Target.createMCRegInfo(STI.getTargetTriple())) {}
+
   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 {
+    if (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(
+            RegisterInfo->getDwarfRegNum(X86::EBP, true /* IsEH */), 0);
+      }
+      EmitInstruction(
+          Out, MCInstBuilder(X86::MOV32rr).addReg(X86::EBP).addReg(FrameReg));
+      Out.EmitCFIRememberState();
+      Out.EmitCFIDefCfaRegister(
+          RegisterInfo->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 (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,
EmitInstruction(
         Out, MCInstBuilder(X86::PUSH32r).addReg(RegCtx.addressReg(MVT::i32)));
 
     const std::string &Fn = FuncName(AccessSize, IsWrite);
     MCSymbol *FnSym = Ctx.GetOrCreateSymbol(StringRef(Fn));
     const MCSymbolRefExpr *FnExpr =
         MCSymbolRefExpr::Create(FnSym, MCSymbolRefExpr::VK_PLT, Ctx);
     EmitInstruction(Out, MCInstBuilder(X86::CALLpcrel32).addExpr(FnExpr));
   }
+
+  std::unique_ptr<MCRegisterInfo> RegisterInfo;
 };
 
 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);
void X86AddressSanitizer32::InstrumentMOVSImpl(unsigned AccessSize,
   RestoreFlags(Out);
 }
 
 class X86AddressSanitizer64 : public X86AddressSanitizer {
 public:
   static const long kShadowOffset = 0x7fff8000;
 
   X86AddressSanitizer64(const MCSubtargetInfo &STI)
-      : X86AddressSanitizer(STI) {}
+      : X86AddressSanitizer(STI),
+        RegisterInfo(TheX86_64Target.createMCRegInfo(STI.getTargetTriple())) {}
   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 {
+    if (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 (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,
if (RegCtx.AddressReg != X86::RDI) {
                                RegCtx.addressReg(MVT::i64)));
     }
     const std::string &Fn = FuncName(AccessSize, IsWrite);
     MCSymbol *FnSym = Ctx.GetOrCreateSymbol(StringRef(Fn));
     const MCSymbolRefExpr *FnExpr =
         MCSymbolRefExpr::Create(FnSym, MCSymbolRefExpr::VK_PLT, Ctx);
     EmitInstruction(Out, MCInstBuilder(X86::CALL64pcrel32).addExpr(FnExpr));
   }
+
+  std::unique_ptr<MCRegisterInfo> RegisterInfo;
 };
 
 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);
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);
 }

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);
 }