Windows hotpatching support

Address PR feedback

* Simply use F.getName() for function name.
* Fix llvm/test/CodeGen/Generic/ms-hotpatch-direct-global-access.ll
* Add negative testing for functions that are not supposed to be hotpatched

Rename to Secure, in LLVM

rename more integration tests

finish renaming to SHP

Author: sivadeilra

clang/include/clang/Basic/CodeGenOptions.h

@@ -501,6 +501,13 @@ class CodeGenOptions : public CodeGenOptionsBase {
 
   /// A list of functions that are replacable by the loader.
   std::vector<std::string> LoaderReplaceableFunctionNames;
+  /// The name of a file that contains functions which will be compiled for
+  /// hotpatching. See -fms-secure-hotpatch-functions-file.
+  std::string MSSecureHotPatchFunctionsFile;
+
+  /// A list of functions which will be compiled for hotpatching.
+  /// See -fms-secure-hotpatch-functions-list.
+  std::vector<std::string> MSSecureHotPatchFunctionsList;
 
 public:
   // Define accessors/mutators for code generation options of enumeration type.

clang/include/clang/Driver/Options.td

@@ -3830,6 +3830,24 @@ def fms_hotpatch : Flag<["-"], "fms-hotpatch">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option, CLOption]>,
   HelpText<"Ensure that all functions can be hotpatched at runtime">,
   MarshallingInfoFlag<CodeGenOpts<"HotPatch">>;
+
+// See llvm/lib/CodeGen/WindowsSecureHotPatching.cpp
+def fms_secure_hotpatch_functions_file
+    : Joined<["-"], "fms-secure-hotpatch-functions-file=">,
+      Group<f_Group>,
+      Visibility<[ClangOption, CC1Option, CLOption]>,
+      MarshallingInfoString<CodeGenOpts<"MSSecureHotPatchFunctionsFile">>,
+      HelpText<"Path to a file that contains a list of mangled names of "
+               "functions that should be hot-patched for Windows Secure "
+               "Hot-Patching">;
+def fms_secure_hotpatch_functions_list
+    : CommaJoined<["-"], "fms-secure-hotpatch-functions-list=">,
+      Group<f_Group>,
+      Visibility<[ClangOption, CC1Option, CLOption]>,
+      MarshallingInfoStringVector<CodeGenOpts<"MSSecureHotPatchFunctionsList">>,
+      HelpText<"List of mangled symbol names of functions that should be "
+               "hot-patched for Windows Secure Hot-Patching">;
+
 def fpcc_struct_return : Flag<["-"], "fpcc-struct-return">, Group<f_Group>,
   Visibility<[ClangOption, CC1Option]>,
   HelpText<"Override the default ABI to return all structs on the stack">;

clang/lib/CodeGen/CGCall.cpp

@@ -2649,6 +2649,15 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
     // CPU/feature overrides.  addDefaultFunctionDefinitionAttributes
     // handles these separately to set them based on the global defaults.
     GetCPUAndFeaturesAttributes(CalleeInfo.getCalleeDecl(), FuncAttrs);
+
+    // Windows hotpatching support
+    if (!MSHotPatchFunctions.empty()) {
+      bool IsHotPatched = std::binary_search(MSHotPatchFunctions.begin(),
+                                             MSHotPatchFunctions.end(), Name);
+      if (IsHotPatched) {
+        FuncAttrs.addAttribute(llvm::Attribute::MarkedForWindowsHotPatching);
+      }
+    }
   }
 
   // Mark functions that are replaceable by the loader.

clang/lib/CodeGen/CodeGenModule.cpp

@@ -453,6 +453,35 @@ CodeGenModule::CodeGenModule(ASTContext &C,
   if (Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
     getModule().addModuleFlag(llvm::Module::Error, "NumRegisterParameters",
                               CodeGenOpts.NumRegisterParameters);
+
+  // If there are any functions that are marked for Windows secure hot-patching,
+  // then build the list of functions now.
+  if (!CGO.MSSecureHotPatchFunctionsFile.empty() ||
+      !CGO.MSSecureHotPatchFunctionsList.empty()) {
+    if (!CGO.MSSecureHotPatchFunctionsFile.empty()) {
+      auto BufOrErr =
+          llvm::MemoryBuffer::getFile(CGO.MSSecureHotPatchFunctionsFile);
+      if (BufOrErr) {
+        const llvm::MemoryBuffer &FileBuffer = **BufOrErr;
+        for (llvm::line_iterator I(FileBuffer.getMemBufferRef(), true), E;
+             I != E; ++I)
+          this->MSHotPatchFunctions.push_back(std::string{*I});
+      } else {
+        auto &DE = Context.getDiagnostics();
+        unsigned DiagID =
+            DE.getCustomDiagID(DiagnosticsEngine::Error,
+                               "failed to open hotpatch functions file "
+                               "(-fms-hotpatch-functions-file): %0 : %1");
+        DE.Report(DiagID) << CGO.MSSecureHotPatchFunctionsFile
+                          << BufOrErr.getError().message();
+      }
+    }
+
+    for (const auto &FuncName : CGO.MSSecureHotPatchFunctionsList)
+      this->MSHotPatchFunctions.push_back(FuncName);
+
+    llvm::sort(this->MSHotPatchFunctions);
+  }
 }
 
 CodeGenModule::~CodeGenModule() {}

clang/lib/CodeGen/CodeGenModule.h

@@ -678,6 +678,11 @@ class CodeGenModule : public CodeGenTypeCache {
 
   AtomicOptions AtomicOpts;
 
+  // A set of functions which should be hot-patched; see
+  // -fms-hotpatch-functions-file (and -list). This will nearly always be empty.
+  // The list is sorted for binary-searching.
+  std::vector<std::string> MSHotPatchFunctions;
+
 public:
   CodeGenModule(ASTContext &C, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
                 const HeaderSearchOptions &headersearchopts,

clang/lib/Driver/ToolChains/Clang.cpp

@@ -6946,6 +6946,14 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fms_secure_hotpatch_functions_file))
+    Args.AddLastArg(CmdArgs, options::OPT_fms_secure_hotpatch_functions_file);
+
+  for (const auto &A :
+       Args.getAllArgValues(options::OPT_fms_secure_hotpatch_functions_list))
+    CmdArgs.push_back(
+        Args.MakeArgString("-fms-secure-hotpatch-functions-list=" + Twine(A)));
+
   if (TC.SupportsProfiling()) {
     Args.AddLastArg(CmdArgs, options::OPT_pg);
 

clang/test/CodeGen/ms-secure-hotpatch-bad-file.c

@@ -0,0 +1,16 @@
+// This verifies that we correctly handle a -fms-secure-hotpatch-functions-file argument that points
+// to a missing file.
+//
+// RUN: not %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-file=%S/this-file-is-intentionally-missing-do-not-create-it.txt /Fo%t.obj %s 2>&1 | FileCheck %s
+// CHECK: failed to open hotpatch functions file
+
+void this_might_have_side_effects();
+
+int __declspec(noinline) this_gets_hotpatched() {
+    this_might_have_side_effects();
+    return 42;
+}
+
+int __declspec(noinline) this_does_not_get_hotpatched() {
+    return this_gets_hotpatched() + 100;
+}

clang/test/CodeGen/ms-secure-hotpatch-cpp.cpp

@@ -0,0 +1,22 @@
+// This verifies that hotpatch function attributes are correctly propagated when compiling directly to OBJ,
+// and that name mangling works as expected.
+//
+// RUN: %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-list=?this_gets_hotpatched@@YAHXZ /Fo%t.obj %s
+// RUN: llvm-readobj --codeview %t.obj | FileCheck %s
+
+void this_might_have_side_effects();
+
+int __declspec(noinline) this_gets_hotpatched() {
+    this_might_have_side_effects();
+    return 42;
+}
+
+// CHECK: Kind: S_HOTPATCHFUNC (0x1169)
+// CHECK-NEXT: Function: this_gets_hotpatched
+// CHECK-NEXT: Name: ?this_gets_hotpatched@@YAHXZ
+
+extern "C" int __declspec(noinline) this_does_not_get_hotpatched() {
+    return this_gets_hotpatched() + 100;
+}
+
+// CHECK-NOT: S_HOTPATCHFUNC

clang/test/CodeGen/ms-secure-hotpatch-lto.c

@@ -0,0 +1,18 @@
+// This verifies that hotpatch function attributes are correctly propagated through LLVM IR when compiling with LTO.
+//
+// RUN: %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-list=this_gets_hotpatched -flto /Fo%t.bc %s
+// RUN: llvm-dis %t.bc -o - | FileCheck %s
+//
+// CHECK: ; Function Attrs: marked_for_windows_hot_patching mustprogress nofree noinline norecurse nosync nounwind sspstrong willreturn memory(none) uwtable
+// CHECK-NEXT: define dso_local noundef i32 @this_gets_hotpatched() local_unnamed_addr #0 !dbg !13 {
+//
+// CHECK: ; Function Attrs: mustprogress nofree noinline norecurse nosync nounwind sspstrong willreturn memory(none) uwtable
+// CHECK-NEXT: define dso_local noundef i32 @this_does_not_get_hotpatched() local_unnamed_addr #1 !dbg !19 {
+
+int __declspec(noinline) this_gets_hotpatched() {
+    return 42;
+}
+
+int __declspec(noinline) this_does_not_get_hotpatched() {
+    return this_gets_hotpatched() + 100;
+}

clang/test/CodeGen/ms-secure-hotpatch.c

@@ -0,0 +1,21 @@
+// This verifies that hotpatch function attributes are correctly propagated when compiling directly to OBJ.
+//
+// RUN: echo this_gets_hotpatched > %t.patch-functions.txt
+// RUN: %clang_cl -c --target=x86_64-windows-msvc -O2 /Z7 -fms-secure-hotpatch-functions-file=%t.patch-functions.txt /Fo%t.obj %s
+// RUN: llvm-readobj --codeview %t.obj | FileCheck %s
+
+void this_might_have_side_effects();
+
+int __declspec(noinline) this_gets_hotpatched() {
+    this_might_have_side_effects();
+    return 42;
+}
+
+// CHECK: Kind: S_HOTPATCHFUNC (0x1169)
+// CHECK-NEXT: Function: this_gets_hotpatched
+
+int __declspec(noinline) this_does_not_get_hotpatched() {
+    return this_gets_hotpatched() + 100;
+}
+
+// CHECK-NOT: S_HOTPATCHFUNC

llvm/include/llvm/Bitcode/LLVMBitCodes.h

@@ -798,6 +798,8 @@ enum AttributeKindCodes {
   ATTR_KIND_NO_DIVERGENCE_SOURCE = 100,
   ATTR_KIND_SANITIZE_TYPE = 101,
   ATTR_KIND_CAPTURES = 102,
+  ATTR_KIND_ALLOW_DIRECT_ACCESS_IN_HOT_PATCH_FUNCTION = 103,
+  ATTR_KIND_MARKED_FOR_WINDOWS_HOT_PATCHING = 104,
 };
 
 enum ComdatSelectionKindCodes {

llvm/include/llvm/CodeGen/Passes.h

@@ -615,6 +615,9 @@ FunctionPass *createSelectOptimizePass();
 
 FunctionPass *createCallBrPass();
 
+/// Creates Windows Hot Patch pass. \see WindowsHotPatch.cpp
+ModulePass *createWindowsSecureHotPatching();
+
 /// Lowers KCFI operand bundles for indirect calls.
 FunctionPass *createKCFIPass();
 } // namespace llvm

llvm/include/llvm/DebugInfo/CodeView/CodeViewSymbols.def

@@ -256,6 +256,8 @@ SYMBOL_RECORD_ALIAS(S_GTHREAD32     , 0x1113, GlobalTLS, ThreadLocalDataSym)
 SYMBOL_RECORD(S_UNAMESPACE    , 0x1124, UsingNamespaceSym)
 SYMBOL_RECORD(S_ANNOTATION    , 0x1019, AnnotationSym)
 
+SYMBOL_RECORD(S_HOTPATCHFUNC  , 0x1169, HotPatchFuncSym)
+
 #undef CV_SYMBOL
 #undef SYMBOL_RECORD
 #undef SYMBOL_RECORD_ALIAS

llvm/include/llvm/DebugInfo/CodeView/SymbolRecord.h

@@ -176,6 +176,21 @@ class CallerSym : public SymbolRecord {
   uint32_t RecordOffset = 0;
 };
 
+class HotPatchFuncSym : public SymbolRecord {
+public:
+  explicit HotPatchFuncSym(SymbolRecordKind Kind) : SymbolRecord(Kind) {}
+  HotPatchFuncSym(uint32_t RecordOffset)
+      : SymbolRecord(SymbolRecordKind::HotPatchFuncSym),
+        RecordOffset(RecordOffset) {}
+
+  // This is an ItemID in the IPI stream, which points to an LF_FUNC_ID or
+  // LF_MFUNC_ID record.
+  TypeIndex Function;
+  StringRef Name;
+
+  uint32_t RecordOffset = 0;
+};
+
 struct DecodedAnnotation {
   StringRef Name;
   ArrayRef<uint8_t> Bytes;

llvm/include/llvm/IR/Attributes.td

@@ -389,6 +389,17 @@ def CoroDestroyOnlyWhenComplete : EnumAttr<"coro_only_destroy_when_complete", In
 /// pipeline to perform elide on the call or invoke instruction.
 def CoroElideSafe : EnumAttr<"coro_elide_safe", IntersectPreserve, [FnAttr]>;
 
+/// Function is marked for Windows Hot Patching
+def MarkedForWindowsHotPatching
+    : EnumAttr<"marked_for_windows_hot_patching", IntersectPreserve, [FnAttr]>;
+
+/// Global variable should not be accessed through a "__ref_" global variable in
+/// a hot patching function This attribute is applied to the global variable
+/// decl, not the hotpatched function.
+def AllowDirectAccessInHotPatchFunction
+    : EnumAttr<"allow_direct_access_in_hot_patch_function",
+               IntersectPreserve, [FnAttr]>;
+
 /// Target-independent string attributes.
 def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">;
 def NoInfsFPMath : StrBoolAttr<"no-infs-fp-math">;

llvm/include/llvm/InitializePasses.h

@@ -324,6 +324,7 @@ void initializeVirtRegMapWrapperLegacyPass(PassRegistry &);
 void initializeVirtRegRewriterLegacyPass(PassRegistry &);
 void initializeWasmEHPreparePass(PassRegistry &);
 void initializeWinEHPreparePass(PassRegistry &);
+void initializeWindowsSecureHotPatchingPass(PassRegistry &);
 void initializeWriteBitcodePassPass(PassRegistry &);
 void initializeXRayInstrumentationLegacyPass(PassRegistry &);
 

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

@@ -2247,6 +2247,10 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
     return Attribute::NoExt;
   case bitc::ATTR_KIND_CAPTURES:
     return Attribute::Captures;
+  case bitc::ATTR_KIND_ALLOW_DIRECT_ACCESS_IN_HOT_PATCH_FUNCTION:
+    return Attribute::AllowDirectAccessInHotPatchFunction;
+  case bitc::ATTR_KIND_MARKED_FOR_WINDOWS_HOT_PATCHING:
+    return Attribute::MarkedForWindowsHotPatching;
   }
 }
 

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -937,6 +937,10 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
     return bitc::ATTR_KIND_NO_EXT;
   case Attribute::Captures:
     return bitc::ATTR_KIND_CAPTURES;
+  case Attribute::AllowDirectAccessInHotPatchFunction:
+    return bitc::ATTR_KIND_ALLOW_DIRECT_ACCESS_IN_HOT_PATCH_FUNCTION;
+  case Attribute::MarkedForWindowsHotPatching:
+    return bitc::ATTR_KIND_MARKED_FOR_WINDOWS_HOT_PATCHING;
   case Attribute::EndAttrKinds:
     llvm_unreachable("Can not encode end-attribute kinds marker.");
   case Attribute::None:

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

@@ -627,8 +627,6 @@ void CodeViewDebug::beginModule(Module *M) {
 
   CurrentSourceLanguage = MapDWLangToCVLang(CU->getSourceLanguage());
 
-  collectGlobalVariableInfo();
-
   // Check if we should emit type record hashes.
   ConstantInt *GH =
       mdconst::extract_or_null<ConstantInt>(M->getModuleFlag("CodeViewGHash"));
@@ -639,6 +637,8 @@ void CodeViewDebug::endModule() {
   if (!Asm || !Asm->hasDebugInfo())
     return;
 
+  collectGlobalVariableInfo();
+
   // The COFF .debug$S section consists of several subsections, each starting
   // with a 4-byte control code (e.g. 0xF1, 0xF2, etc) and then a 4-byte length
   // of the payload followed by the payload itself.  The subsections are 4-byte
@@ -653,6 +653,8 @@ void CodeViewDebug::endModule() {
   emitCompilerInformation();
   endCVSubsection(CompilerInfo);
 
+  emitHotPatchInformation();
+
   emitInlineeLinesSubsection();
 
   // Emit per-function debug information.
@@ -807,6 +809,32 @@ void CodeViewDebug::emitObjName() {
   endSymbolRecord(CompilerEnd);
 }
 
+void CodeViewDebug::emitHotPatchInformation() {
+  MCSymbol *hotPatchInfo = nullptr;
+  for (const auto &F : MMI->getModule()->functions()) {
+    if (!F.isDeclarationForLinker() &&
+        F.hasFnAttribute(Attribute::MarkedForWindowsHotPatching)) {
+      if (hotPatchInfo == nullptr) {
+        hotPatchInfo = beginCVSubsection(DebugSubsectionKind::Symbols);
+      }
+      MCSymbol *HotPatchEnd = beginSymbolRecord(SymbolKind::S_HOTPATCHFUNC);
+      auto *SP = F.getSubprogram();
+      OS.AddComment("Function");
+      OS.emitInt32(getFuncIdForSubprogram(SP).getIndex());
+      OS.AddComment("Name");
+      llvm::StringRef Name = SP->getLinkageName();
+      if (Name.empty()) {
+        Name = F.getName();
+      }
+      emitNullTerminatedSymbolName(OS, Name);
+      endSymbolRecord(HotPatchEnd);
+    }
+  }
+  if (hotPatchInfo != nullptr) {
+    endCVSubsection(hotPatchInfo);
+  }
+}
+
 namespace {
 struct Version {
   int Part[4];

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h

@@ -333,6 +333,8 @@ class LLVM_LIBRARY_VISIBILITY CodeViewDebug : public DebugHandlerBase {
 
   void emitCompilerInformation();
 
+  void emitHotPatchInformation();
+
   void emitBuildInfo();
 
   void emitInlineeLinesSubsection();

llvm/lib/CodeGen/CMakeLists.txt

@@ -250,6 +250,7 @@ add_llvm_component_library(LLVMCodeGen
   VirtRegMap.cpp
   WasmEHPrepare.cpp
   WindowScheduler.cpp
+  WindowsSecureHotPatching.cpp
   WinEHPrepare.cpp
   XRayInstrumentation.cpp
   ${GeneratedMLSources}

llvm/lib/CodeGen/TargetPassConfig.cpp

@@ -893,6 +893,9 @@ void TargetPassConfig::addIRPasses() {
 
   if (EnableGlobalMergeFunc)
     addPass(createGlobalMergeFuncPass());
+
+  if (TM->getTargetTriple().isOSBinFormatCOFF())
+    addPass(createWindowsSecureHotPatching());
 }
 
 /// Turn exception handling constructs into something the code generators can