[X86] Support the "ms-hotpatch" attribute.

[cdavis5x]

Summary

Based on two patches by Michael Mueller.

This is a target attribute that causes a function marked with it to be
emitted as "hotpatchable". This particular mechanism was originally
devised by Microsoft for patching their binaries (which they are
constantly updating to stay ahead of crackers, script kiddies, and other
ne'er-do-wells on the Internet), but is now commonly abused by Windows
programs to hook API functions.

This mechanism is target-specific. For x86, a two-byte no-op instruction
is emitted at the function's entry point; the entry point must be
immediately preceded by 64 (32-bit) or 128 (64-bit) bytes of padding.
This padding is where the patch code is written. The two byte no-op is
then overwritten with a short jump into this code. The no-op is usually
a movl %edi, %edi instruction; this is used as a magic value
indicating that this is a hotpatchable function.

[rnk]

This should use Sanjoy's "patchable-function" attribute:
http://reviews.llvm.org/D19046

lib/Target/X86/X86AsmPrinter.cpp

76 ↗

(On Diff #56107)

You should consider overriding EmitConstantPool instead, so the fill directive comes before the alignment directive:

.text
.fill 5, 1, 0xcc
.align 2
.global foo
foo:
  movl %edi, %edi
  retq

Are the hotpatching tools that we care about checking for 0xcc bytes, or will they overwrite alignment nops?

[cdavis5x]

• Use Sanjoy's "patchable-function" attribute.
• Override EmitConstantPool() instead of EmitFunctionEntryLabel().
• Clarify why we're emitting 0xcc bytes (int3 instructions) instead of NOPs.

[cdavis5x]

lib/Target/X86/X86AsmPrinter.cpp

76 ↗

(On Diff #56107)

Are the hotpatching tools that we care about checking for 0xcc bytes, or will they overwrite alignment nops?

It's not necessarily that tools care, it's that I think I'd prefer loudly crashing to getting stuck in an infinite loop if the patch forgets to jump back into the function. (I've added a comment clarifying this.)

[sanjoy]

docs/LangRef.rst

1435 ↗

(On Diff #57387)

So on x86-64, the 2 byte instruction can be anything? May be helpful to be explicit about that.

lib/CodeGen/PatchableFunction.cpp

43 ↗

(On Diff #57387)

I presume you're handling the x86-64 case here? If so, how about handling both the 32bit and 64bit case here? On 32 bits, you can just emit a mov %edi, %edi instead of a PATCHABLE_OP? That way related bits stay together.

[rnk]

lib/CodeGen/PatchableFunction.cpp

43 ↗

(On Diff #57387)

+1

49 ↗

(On Diff #57387)

GCC will give you a -Wparentheses warning on this.

lib/Target/X86/X86ISelLowering.cpp

2504–2507 ↗

(On Diff #57387)

Why do we need this change? The patchable nop comes before the prologue. Surely this isn't use for some kind of on-stack-replacement.

test/CodeGen/X86/ms-hotpatch-attr.ll

4 ↗

(On Diff #57387)

Can you add a check for "align"? Otherwise there's no difference between the constant pool entry and the function label override.

[cdavis5x]

lib/CodeGen/PatchableFunction.cpp

43 ↗

(On Diff #57387)

I'm not so sure about this. This looks like it's ultimately intended to be a target-independent pass. Putting target-specific code in here feels... wrong. If you really want me to do that anyway, I'll do it, but I'm just saying, I don't think that's such a good idea.

[cdavis5x]

• Clarify that x86-64 doesn't use a special magic no-op.
• Don't emit patch space with MSVC. The linker adds it for us there.
• Don't force 64-bit functions to use a stack frame. That's only needed for 32-bit.
• Test that, when we do emit the patch space, we emit it before the alignment directive.

[cdavis5x]

docs/LangRef.rst

1435 ↗

(On Diff #57387)

Theoretically, but GCC seems to emit a lea (%rsp), %rsp on x86-64 (which is 8 bytes long). I don't know if we need to do this.

@rnk: What does MSVC do in that situation?

lib/Target/X86/X86ISelLowering.cpp

2504–2507 ↗

(On Diff #57387)

GCC seems to emit a standard push %ebp; mov %esp, %ebp prologue along with the patchable no-op. It doesn't do this for 64-bit.

[sanjoy]

Sorry for the delayed response, I thought I pressed submit, when I hadn't.

lib/CodeGen/PatchableFunction.cpp

43 ↗

(On Diff #57850)

You'd have to add a TTI hook or re-purposes an existing one that generates the code for you (I agree that hard-coding X86 instructions here is a bad idea).

[sanjoy]

Marked as "needing changes" to get this off my pending queue.

[dberris]

docs/LangRef.rst

1446 ↗

(On Diff #57850)

At some point we probably have to reconcile how XRay will work with ms-hotpatch and prologue-short-redirect.

Is it absolutely required that the first instructions be specific ones, or would something like what XRay has for functions work too (like a short relative jump then a few more bytes of nops)?

[cdavis5x]

Factor emission of the patchable instruction into a TTI hook.

[cdavis5x]

docs/LangRef.rst

1463 ↗

(On Diff #62407)

tl;dr: You'll have problems on i386, but not anywhere else.

On 32-bit x86, a function marked ms-hotpatch must start with the bytes 8b ff (a mov edi, edi instruction). (Strictly speaking, it doesn't have to, but programs are abusing this to alter functionality of Windows API calls, and they use this as a sort of magic number to know if a function can be hotpatched or not, so unfortunately, it's too late to change this.) Later on, this gets replaced by a two-byte jmp short into the patch space (which is only guaranteed to be at least 5 bytes, though GCC--and therefore, this patch--gives you 64 bytes) which precedes this function. I believe the mov edi, edi must immediately be followed by a standard prologue (i.e. push ebp; mov ebp, esp), but I'm not sure about that. Based on all that, and on what you've said, I fear you may have problems getting XRay to play well with ms-hotpatch-style functions on i386. But it looks like you don't support anything other than x86-64 yet.

On any other architecture (including x64), no, it is not required (AFAICT) to be any specific instruction.

[cdavis5x]

Ping.

[rnk]

lgtm

[cdavis5x]

@sanjoy: Ping.

[sanjoy]

lgtm

[cdavis5x]

Unfortunately, this patch needs revision due to a merge conflict. :\

[cdavis5x]

Merge with a more recent revision.

[cdavis5x]

I think I've figured out why the build broke when I landed this. (Sorry that took so long.)

When I added the emitPatchableOp() method to TargetTransformInfo, I added a dependency of Analysis on CodeGen, as well. But, I didn't record that in the build files. I didn't see any problems because I normally build with LLVM_LINK_LLVM_DYLIB=ON, so all of LLVM gets linked into a giant dylib, and everything that uses LLVM gets linked to this giant dylib. The buildbots, on the other hand, don't do this. Since the build system didn't know that Analysis now depended on CodeGen, BOOM.

I could've just added the dependency and been done with it, but this got me thinking. CodeGen already depends on Analysis. Having Analysis depend on CodeGen would produce a circular dependency. I don't think that's a good thing. I also became convinced that adding emitPatchableOp() to TargetTransformInfo constituted a layering violation. So, I moved it somewhere else--into TargetInstrInfo instead. The base TargetInstrInfo is already part of CodeGen, so no new dependencies are added by this change.

I don't want to break the build again like that. I also made an (IMO) not insignificant change to this patch. So, I'd like you all to review this patch again. (Sorry, but this is the last time, I promise!) I've verified that this will not break the build again.

[cdavis5x]

Fix the build error we saw in trunk.

[aganea]

Hello! It seems this patch and another related patch were reverted and @cdavis5x has been inactive since.
Any objection if I tried to reland this? Is there a policy for abandonned patches?

[MaskRay]

In D19908#2051605, @aganea wrote:

Hello! It seems this patch and another related patch were reverted and @cdavis5x has been inactive since.
Any objection if I tried to reland this? Is there a policy for abandonned patches?

I don't know the policy is written down anywhere. You may create a new patch and state that it is based on @cdavis5x.
I took a quick look at the test llvm/test/CodeGen/X86/ms-hotpatch-attr.ll. It can be improved a bit.

According to https://reviews.llvm.org/D72215#1815586 , @reames is a user of the existing "patchable-function" and you may need to add him as a reviewer.
("patchable-function-entry" and ("xray-instruction-threashold"|"function-instrument"="xray-always") are similar but different instrumentations.)

[cdavis5x]

In D19908#2051605, @aganea wrote:

Hello! It seems this patch and another related patch were reverted and @cdavis5x has been inactive since.
Any objection if I tried to reland this? Is there a policy for abandonned patches?

I've actually been meaning to get back to this for a while now, but I've been busy with other stuff lately. If you want to drive this forward, that's fine by me.

[aganea]

This patch didn't apply cleanly on latest, for the reasons mentionned by @cdavis5x here: https://reviews.llvm.org/D19908#518374
I ended up reimplementing the end-feature of this patch in D81301.
@cdavis5x Could you please take a look at let me know if that patch covers what you intended to do?

llvm/lib/Target/X86/X86AsmPrinter.cpp

97 ↗

(On Diff #223567)

I'm not sure why this was done here. I vaguely recall that older versions of MSVC (VS2012 or before) were generating this kind of fill, but this was then replaced by /FUNCTIONPADMIN in the linker (it is supported by LLD).

llvm/trunk/docs/LangRef.rst

``optsize``
     passes make choices that keep the code size of this function low,
     and otherwise do optimizations specifically to reduce code size as
     long as they do not significantly impact runtime performance.
 ``"patchable-function"``
     This attribute tells the code generator that the code
     generated for this function needs to follow certain conventions that
     make it possible for a runtime function to patch over it later.
     The exact effect of this attribute depends on its string value,
-    for which there currently is one legal possibility:
+    for which there currently are two legal possiblities:
 
      * ``"prologue-short-redirect"`` - This style of patchable
        function is intended to support patching a function prologue to
        redirect control away from the function in a thread safe
        manner.  It guarantees that the first instruction of the
        function will be large enough to accommodate a short jump
        instruction, and will be sufficiently aligned to allow being
        fully changed via an atomic compare-and-swap instruction.
        While the first requirement can be satisfied by inserting large
        enough NOP, LLVM can and will try to re-purpose an existing
        instruction (i.e. one that would have to be emitted anyway) as
        the patchable instruction larger than a short jump.
 
        ``"prologue-short-redirect"`` is currently only supported on
        x86-64.
+     * ``"ms-hotpatch"`` - This style of patchable function is similar to
+       ``"prologue-short-redirect"``, but it also imposes several additional
+       guarantees to support the style of hotpatching used on Windows.  On
+       32-bit x86, the first instruction will be a ``mov %edi, %edi``
+       instruction; this is frequently used as a magic value indicating a
+       hotpatchable function.  On other architectures, however, the first
+       instruction can be anything allowed in a Windows-style prologue;
+       this is because all functions on the non-i386 architectures Windows
+       supports are assumed to be hotpatchable.  Additionally, when not
+       targeting a Visual C++-style toolchain, patch space will be provided
+       prior to the function's entry point of an architecturally specific
+       size.  These sizes are compatible with GCC: on 32-bit x86, the patch
+       space is 64 bytes long; on x86-64, it is 128 bytes long.  The patch
+       space is not provided for MSVC toolchains because the
+       `/FUNCTIONPADMIN <https://msdn.microsoft.com/en-us/library/ms173524.aspx>`_
+       option, which provides this space, is expected to be used there.
+
+       ``"ms-hotpatch"`` is currently only supported on x86 and x86-64.
 
     This attribute by itself does not imply restrictions on
     inter-procedural optimizations.  All of the semantic effects the
     patching may have to be separately conveyed via the linkage type.
 ``readnone``
     On a function, this attribute indicates that the function computes its
     result (or decides to unwind an exception) based strictly on its arguments,
     without dereferencing any pointer arguments or otherwise accessing

llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h

 /// use for querying the codegen.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ANALYSIS_TARGETTRANSFORMINFO_H
 #define LLVM_ANALYSIS_TARGETTRANSFORMINFO_H
 
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/DataTypes.h"
 #include <functional>
 
struct UnrollingPreferences {
     bool Force;
   };
 
   /// \brief Get target-customized preferences for the generic loop unrolling
   /// transformation. The caller will initialize UP with the current
   /// target-independent defaults.
   void getUnrollingPreferences(Loop *L, UnrollingPreferences &UP) const;
 
+  /// \brief Emit a patchable operation in the given basic block at the
+  /// given insertion point.
+  ///
+  /// Most of the time, this will be a straight-up \c TargetOpcode::PATCHABLE_OP
+  /// instruction, which will be lowered by the target to a no-op that can
+  /// be safely replaced with a short jump. However, some targets under certain
+  /// conditions can have peculiar requirements for this instruction; these
+  /// targets can provide their own implementation of this to emit the correct
+  /// instruction.
+  void emitPatchableOp(StringRef PatchType, MachineBasicBlock &MBB,
+                       MachineBasicBlock::iterator &MBBI) const;
+
   /// @}
 
   /// \name Scalar Target Information
   /// @{
 
   /// \brief Flags indicating the kind of support for population count.
   ///
   /// Compared to the SW implementation, HW support is supposed to
virtual int getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                                ArrayRef<Type *> ParamTys) = 0;
   virtual int getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
                                ArrayRef<const Value *> Arguments) = 0;
   virtual int getUserCost(const User *U) = 0;
   virtual bool hasBranchDivergence() = 0;
   virtual bool isSourceOfDivergence(const Value *V) = 0;
   virtual bool isLoweredToCall(const Function *F) = 0;
   virtual void getUnrollingPreferences(Loop *L, UnrollingPreferences &UP) = 0;
+  virtual void emitPatchableOp(StringRef Kind,
+                               MachineBasicBlock &MBB,
+                               MachineBasicBlock::iterator &MBBI) const = 0;
   virtual bool isLegalAddImmediate(int64_t Imm) = 0;
   virtual bool isLegalICmpImmediate(int64_t Imm) = 0;
   virtual bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV,
                                      int64_t BaseOffset, bool HasBaseReg,
                                      int64_t Scale,
                                      unsigned AddrSpace) = 0;
   virtual bool isLegalMaskedStore(Type *DataType) = 0;
   virtual bool isLegalMaskedLoad(Type *DataType) = 0;
bool isSourceOfDivergence(const Value *V) override {
     return Impl.isSourceOfDivergence(V);
   }
   bool isLoweredToCall(const Function *F) override {
     return Impl.isLoweredToCall(F);
   }
   void getUnrollingPreferences(Loop *L, UnrollingPreferences &UP) override {
     return Impl.getUnrollingPreferences(L, UP);
   }
+  void emitPatchableOp(StringRef Kind, MachineBasicBlock &MBB,
+                       MachineBasicBlock::iterator &MBBI) const override {
+    return Impl.emitPatchableOp(Kind, MBB, MBBI);
+  }
   bool isLegalAddImmediate(int64_t Imm) override {
     return Impl.isLegalAddImmediate(Imm);
   }
   bool isLegalICmpImmediate(int64_t Imm) override {
     return Impl.isLegalICmpImmediate(Imm);
   }
   bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV, int64_t BaseOffset,
                              bool HasBaseReg, int64_t Scale,

llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h

 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/Operator.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Analysis/VectorUtils.h"
+#include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/Target/TargetInstrInfo.h"
+#include "llvm/Target/TargetSubtargetInfo.h"
 
 namespace llvm {
 
 /// \brief Base class for use as a mix-in that aids implementing
 /// a TargetTransformInfo-compatible class.
 class TargetTransformInfoImplBase {
 protected:
   typedef TargetTransformInfo TTI;
if (Name == "pow" || Name == "powf" || Name == "powl" || Name == "exp2" ||
         Name == "llabs")
       return false;
 
     return true;
   }
 
   void getUnrollingPreferences(Loop *, TTI::UnrollingPreferences &) {}
 
+  void emitPatchableOp(StringRef, MachineBasicBlock &MBB,
+                       MachineBasicBlock::iterator &MBBI) const {
+    auto *TII = MBB.getParent()->getSubtarget().getInstrInfo();
+    auto MIB = BuildMI(MBB, MBBI, MBBI->getDebugLoc(),
+                       TII->get(TargetOpcode::PATCHABLE_OP))
+                   .addImm(2)
+                   .addImm(MBBI->getOpcode());
+
+    for (auto &MO : MBBI->operands())
+      MIB.addOperand(MO);
+
+    MBBI->eraseFromParent();
+  }
+
   bool isLegalAddImmediate(int64_t Imm) { return false; }
 
   bool isLegalICmpImmediate(int64_t Imm) { return false; }
 
   bool isLegalAddressingMode(Type *Ty, GlobalValue *BaseGV, int64_t BaseOffset,
                              bool HasBaseReg, int64_t Scale,
                              unsigned AddrSpace) {
     // Guess that only reg and reg+reg addressing is allowed. This heuristic is

llvm/trunk/lib/Analysis/TargetTransformInfo.cpp

bool TargetTransformInfo::isLoweredToCall(const Function *F) const {
   return TTIImpl->isLoweredToCall(F);
 }
 
 void TargetTransformInfo::getUnrollingPreferences(
     Loop *L, UnrollingPreferences &UP) const {
   return TTIImpl->getUnrollingPreferences(L, UP);
 }
 
+void TargetTransformInfo::emitPatchableOp(
+    StringRef PatchType, MachineBasicBlock &MBB,
+    MachineBasicBlock::iterator &MBBI) const {
+  return TTIImpl->emitPatchableOp(PatchType, MBB, MBBI);
+}
+
 bool TargetTransformInfo::isLegalAddImmediate(int64_t Imm) const {
   return TTIImpl->isLegalAddImmediate(Imm);
 }
 
 bool TargetTransformInfo::isLegalICmpImmediate(int64_t Imm) const {
   return TTIImpl->isLegalICmpImmediate(Imm);
 }
 

llvm/trunk/lib/CodeGen/PatchableFunction.cpp

 //===-- PatchableFunction.cpp - Patchable prologues for LLVM -------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements edits function bodies in place to support the
 // "patchable-function" attribute.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/Passes.h"
+#include "llvm/Analysis/TargetTransformInfo.h"
+#include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
-#include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/Target/TargetFrameLowering.h"
-#include "llvm/Target/TargetInstrInfo.h"
-#include "llvm/Target/TargetSubtargetInfo.h"
 
 using namespace llvm;
 
 namespace {
 struct PatchableFunction : public MachineFunctionPass {
   static char ID; // Pass identification, replacement for typeid
   PatchableFunction() : MachineFunctionPass(ID) {
     initializePatchableFunctionPass(*PassRegistry::getPassRegistry());
   }
 
+  void getAnalysisUsage(AnalysisUsage &AU) const override;
   bool runOnMachineFunction(MachineFunction &F) override;
-   MachineFunctionProperties getRequiredProperties() const override {
     return MachineFunctionProperties().set(
         MachineFunctionProperties::Property::AllVRegsAllocated);
   }
 };
 }
 
 /// Returns true if instruction \p MI will not result in actual machine code
 /// instructions.
 static bool doesNotGeneratecode(const MachineInstr &MI) {
   // TODO: Introduce an MCInstrDesc flag for this
   switch (MI.getOpcode()) {
   default: return false;
   case TargetOpcode::IMPLICIT_DEF:
   case TargetOpcode::KILL:
   case TargetOpcode::CFI_INSTRUCTION:
   case TargetOpcode::EH_LABEL:
   case TargetOpcode::GC_LABEL:
   case TargetOpcode::DBG_VALUE:
     return true;
   }
 }
 
+void PatchableFunction::getAnalysisUsage(AnalysisUsage &AU) const {
+  MachineFunctionPass::getAnalysisUsage(AU);
+  AU.addRequired<TargetTransformInfoWrapperPass>();
+}
+
 bool PatchableFunction::runOnMachineFunction(MachineFunction &MF) {
   if (!MF.getFunction()->hasFnAttribute("patchable-function"))
     return false;
 
-#ifndef NDEBUG
   Attribute PatchAttr = MF.getFunction()->getFnAttribute("patchable-function");
   StringRef PatchType = PatchAttr.getValueAsString();
-  assert(PatchType == "prologue-short-redirect" && "Only possibility today!");
-#endif
+  assert((PatchType == "prologue-short-redirect" ||
+          PatchType == "ms-hotpatch") && "Only possibilities today!");
 
   auto &FirstMBB = *MF.begin();
   MachineBasicBlock::iterator FirstActualI = FirstMBB.begin();
   for (; doesNotGeneratecode(*FirstActualI); ++FirstActualI)
     assert(FirstActualI != FirstMBB.end());
 
-  auto *TII = MF.getSubtarget().getInstrInfo();
-  auto MIB = BuildMI(FirstMBB, FirstActualI, FirstActualI->getDebugLoc(),
-                     TII->get(TargetOpcode::PATCHABLE_OP))
-                 .addImm(2)
-                 .addImm(FirstActualI->getOpcode());
-
-  for (auto &MO : FirstActualI->operands())
-    MIB.addOperand(MO);
+  const TargetTransformInfo &TTI =
+      getAnalysis<TargetTransformInfoWrapperPass>().getTTI(*MF.getFunction());
+  TTI.emitPatchableOp(PatchType, FirstMBB, FirstActualI);
 
-  FirstActualI->eraseFromParent();
   MF.ensureAlignment(4);
   return true;
 }
 
 char PatchableFunction::ID = 0;
 char &llvm::PatchableFunctionID = PatchableFunction::ID;
 INITIALIZE_PASS(PatchableFunction, "patchable-function",
                 "Implement the 'patchable-function' attribute", false, false)

llvm/trunk/lib/Target/X86/X86AsmPrinter.h

public:
   void EmitEndOfAsmFile(Module &M) override;
 
   void EmitInstruction(const MachineInstr *MI) override;
 
   void EmitBasicBlockEnd(const MachineBasicBlock &MBB) override {
     SMShadowTracker.emitShadowPadding(*OutStreamer, getSubtargetInfo());
   }
 
+  void EmitConstantPool() override;
+
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,
                        unsigned AsmVariant, const char *ExtraCode,
                        raw_ostream &OS) override;
   bool PrintAsmMemoryOperand(const MachineInstr *MI, unsigned OpNo,
                              unsigned AsmVariant, const char *ExtraCode,
                              raw_ostream &OS) override;
 
   /// \brief Return the symbol for the specified constant pool entry.

llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp

bool X86AsmPrinter::runOnMachineFunction(MachineFunction &MF) {
 
   // Emit the XRay table for this function.
   EmitXRayTable();
 
   // We didn't modify anything.
   return false;
 }
 
+void X86AsmPrinter::EmitConstantPool() {
+  if (MF) {
+    // If an MS hotpatch function, we need to ensure 64 (32-bit) or 128 (64-bit)
+    // bytes of padding precede the label.  This is the scratch space used
+    // by the hotpatching mechanism to insert the patch code.  The movl %edi,
+    // %edi instruction emitted as the very first instruction of a hotpatch
+    // function is usually overwritten with a short jump instruction when the
+    // patch is installed, so it will jump directly into this space.  (But
+    // don't add the space when targeting MSVC.  There, the /FUNCTIONPADMIN
+    // option to link.exe is expected to be used.)
+    const Function *Fn = MF->getFunction();
+    if (!Subtarget->isTargetKnownWindowsMSVC() &&
+        Fn->hasFnAttribute("patchable-function") &&
+        Fn->getFnAttribute("patchable-function").getValueAsString() ==
+            "ms-hotpatch") {
+      // Emit INT3 instructions instead of NOPs. If a patch runs off the end,
+      // best to let the patcher know with a crash/debug break than to silently
+      // continue, only to run into the jump back into the patch.
+      OutStreamer->emitFill(Subtarget->is64Bit() ? 128 : 64, 0xcc);
+    }
+  }
+
+  AsmPrinter::EmitConstantPool();
+}
+
 /// printSymbolOperand - Print a raw symbol reference operand.  This handles
 /// jump tables, constant pools, global address and external symbols, all of
 /// which print to a label with various suffixes for relocation types etc.
 static void printSymbolOperand(X86AsmPrinter &P, const MachineOperand &MO,
                                raw_ostream &O) {
   switch (MO.getType()) {
   default: llvm_unreachable("unknown symbol type!");
   case MachineOperand::MO_ConstantPoolIndex:

llvm/trunk/lib/Target/X86/X86FrameLowering.cpp

bool FnHasClrFunclet =
       MMI.hasEHFunclets() && Personality == EHPersonality::CoreCLR;
   bool IsClrFunclet = IsFunclet && FnHasClrFunclet;
   bool HasFP = hasFP(MF);
   bool IsWin64CC = STI.isCallingConvWin64(Fn->getCallingConv());
   bool IsWin64Prologue = MF.getTarget().getMCAsmInfo()->usesWindowsCFI();
   bool NeedsWinCFI = IsWin64Prologue && Fn->needsUnwindTableEntry();
   bool NeedsDwarfCFI =
       !IsWin64Prologue && (MMI.hasDebugInfo() || Fn->needsUnwindTableEntry());
+  bool IsMSHotpatch =
+      Fn->hasFnAttribute("patchable-function") &&
+      Fn->getFnAttribute("patchable-function").getValueAsString() ==
+          "ms-hotpatch";
   unsigned FramePtr = TRI->getFrameRegister(MF);
   const unsigned MachineFramePtr =
       STI.isTarget64BitILP32()
           ? getX86SubSuperRegister(FramePtr, 64) : FramePtr;
   unsigned BasePtr = TRI->getBaseRegister();
   
   // Debug location must be unknown since the first debug location is used
   // to determine the end of the prologue.
if (NeedsWinCFI) {
       BuildMI(MBB, MBBI, DL, TII.get(X86::SEH_PushReg))
           .addImm(FramePtr)
           .setMIFlag(MachineInstr::FrameSetup);
     }
 
     if (!IsWin64Prologue && !IsFunclet) {
       // Update EBP with the new base value.
       BuildMI(MBB, MBBI, DL,
-              TII.get(Uses64BitFramePtr ? X86::MOV64rr : X86::MOV32rr),
+              TII.get(IsMSHotpatch ?
+                      (Uses64BitFramePtr ? X86::MOV64rr_REV : X86::MOV32rr_REV):
+                      (Uses64BitFramePtr ? X86::MOV64rr : X86::MOV32rr)),
               FramePtr)
           .addReg(StackPtr)
           .setMIFlag(MachineInstr::FrameSetup);
 
       if (NeedsDwarfCFI) {
         // Mark effective beginning of when frame pointer becomes valid.
         // Define the current CFA to use the EBP/RBP register.
         unsigned DwarfFramePtr = TRI->getDwarfRegNum(MachineFramePtr, true);

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

 SDValue X86TargetLowering::LowerFormalArguments(
     SDValue Chain, CallingConv::ID CallConv, bool isVarArg,
     const SmallVectorImpl<ISD::InputArg> &Ins, const SDLoc &dl,
     SelectionDAG &DAG, SmallVectorImpl<SDValue> &InVals) const {
   MachineFunction &MF = DAG.getMachineFunction();
   X86MachineFunctionInfo *FuncInfo = MF.getInfo<X86MachineFunctionInfo>();
   const TargetFrameLowering &TFI = *Subtarget.getFrameLowering();
 
-  const Function *Fn = MF.getFunction();
-  if (Fn->hasExternalLinkage() &&
+  const Function* Fn = MF.getFunction();
+  if ((Fn->hasExternalLinkage() &&
        Subtarget.isTargetCygMing() &&
-      Fn->getName() == "main")
+       Fn->getName() == "main") ||
+      (!Subtarget.is64Bit() && Fn->hasFnAttribute("patchable-function") &&
+       Fn->getFnAttribute("patchable-function").getValueAsString() ==
+           "ms-hotpatch"))
     FuncInfo->setForceFramePointer(true);
 
   MachineFrameInfo &MFI = MF.getFrameInfo();
   bool Is64Bit = Subtarget.is64Bit();
   bool IsWin64 = Subtarget.isCallingConvWin64(CallConv);
 
   assert(!(isVarArg && canGuaranteeTCO(CallConv)) &&
          "Var args not supported with calling convention fastcc, ghc or hipe");

llvm/trunk/lib/Target/X86/X86TargetTransformInfo.h

public:
 
   // Provide value semantics. MSVC requires that we spell all of these out.
   X86TTIImpl(const X86TTIImpl &Arg)
       : BaseT(static_cast<const BaseT &>(Arg)), ST(Arg.ST), TLI(Arg.TLI) {}
   X86TTIImpl(X86TTIImpl &&Arg)
       : BaseT(std::move(static_cast<BaseT &>(Arg))), ST(std::move(Arg.ST)),
         TLI(std::move(Arg.TLI)) {}
 
+  /// \name Generic TTI Implementations
+  /// @{
+  void emitPatchableOp(StringRef PatchType, MachineBasicBlock &MBB,
+                       MachineBasicBlock::iterator &MBBI) const;
+
+  /// @}
+
   /// \name Scalar TTI Implementations
   /// @{
   TTI::PopcntSupportKind getPopcntSupport(unsigned TyWidth);
 
   /// @}
 
   /// \name Vector TTI Implementations
   /// @{

llvm/trunk/lib/Target/X86/X86TargetTransformInfo.cpp

bool X86TTIImpl::areInlineCompatible(const Function *Caller,
   const FeatureBitset &CalleeBits =
       TM.getSubtargetImpl(*Callee)->getFeatureBits();
 
   // FIXME: This is likely too limiting as it will include subtarget features
   // that we might not care about for inlining, but it is conservatively
   // correct.
   return (CallerBits & CalleeBits) == CalleeBits;
 }
+
+void X86TTIImpl::emitPatchableOp(StringRef PatchType,
+                                 MachineBasicBlock &MBB,
+                                 MachineBasicBlock::iterator &MBBI) const {
+  if (PatchType != "ms-hotpatch" || !ST->is32Bit()) {
+    BaseT::emitPatchableOp(PatchType, MBB, MBBI);
+    return;
+  }
+
+  auto &TII = *MBB.getParent()->getSubtarget().getInstrInfo();
+  BuildMI(MBB, MBBI, MBBI->getDebugLoc(),
+          TII.get(X86::MOV32rr_REV), X86::EDI)
+      .addReg(X86::EDI)
+      .setMIFlag(MachineInstr::FrameSetup);
+}

llvm/trunk/test/CodeGen/X86/ms-hotpatch-attr.ll

+; RUN: llc < %s -march=x86 -filetype=asm | FileCheck -check-prefix=CHECK-32 %s
+; RUN: llc < %s -march=x86-64 -filetype=asm | FileCheck -check-prefix=CHECK-64 %s
+; RUN: llc < %s -mtriple=i386-windows-msvc -filetype=asm | FileCheck -check-prefix=MSVC-32 %s
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -filetype=asm | FileCheck -check-prefix=MSVC-64 %s
+
+; CHECK-32: .space 64,204
+; CHECK-32: .p2align 4, 0x90
+; CHECK-32-LABEL: foo:
+; CHECK-32: movl %edi, %edi
+; CHECK-32-NEXT: pushl %ebp
+; CHECK-32-NEXT: movl %esp, %ebp
+; CHECK-64: .space 128,204
+; CHECK-64: .p2align 4, 0x90
+; CHECK-64-LABEL: foo:
+; CHECK-64: xchgw %ax, %ax
+; MSVC-32-NOT: .space 64,204
+; MSVC-32-LABEL: _foo:
+; MSVC-32: movl %edi, %edi
+; MSVC-32-NEXT: pushl %ebp
+; MSVC-32-NEXT: movl %esp, %ebp
+; MSVC-64-NOT: .space 128,204
+; MSVC-64-LABEL: foo:
+; MSVC-64: xchgw %ax, %ax
+define void @foo() nounwind "patchable-function"="ms-hotpatch" {
+entry:
+  ret void
+}