[clang] 5af2433 - [clang-cl] Support the /HOTPATCH flag

Alexandre Ganea via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 20 09:57:25 PST 2022


Author: Alexandre Ganea
Date: 2022-01-20T12:57:19-05:00
New Revision: 5af2433e1794ebf7e58e848aa612c7912d71dc78

URL: https://github.com/llvm/llvm-project/commit/5af2433e1794ebf7e58e848aa612c7912d71dc78
DIFF: https://github.com/llvm/llvm-project/commit/5af2433e1794ebf7e58e848aa612c7912d71dc78.diff

LOG: [clang-cl] Support the /HOTPATCH flag

This patch adds support for the MSVC /HOTPATCH flag: https://docs.microsoft.com/sv-se/cpp/build/reference/hotpatch-create-hotpatchable-image?view=msvc-170&viewFallbackFrom=vs-2019

The flag is translated to a new -fms-hotpatch flag, which in turn adds a 'patchable-function' attribute for each function in the TU. This is then picked up by the PatchableFunction pass which would generate a TargetOpcode::PATCHABLE_OP of minsize = 2 (which means the target instruction must resolve to at least two bytes). TargetOpcode::PATCHABLE_OP is only implemented for x86/x64. When targetting ARM/ARM64, /HOTPATCH isn't required (instructions are always 2/4 bytes and suitable for hotpatching).

Additionally, when using /Z7, we generate a 'hot patchable' flag in the CodeView debug stream, in the S_COMPILE3 record. This flag is then picked up by LLD (or link.exe) and is used in conjunction with the linker /FUNCTIONPADMIN flag to generate extra space before each function, to accommodate for live patching long jumps. Please see: https://github.com/llvm/llvm-project/blob/d703b922961e0d02a5effdd4bfbb23ad50a3cc9f/lld/COFF/Writer.cpp#L1298

The outcome is that we can finally use Live++ or Recode along with clang-cl.

NOTE: It seems that MSVC cl.exe always enables /HOTPATCH on x64 by default, although if we did the same I thought we might generate sub-optimal code (if this flag was active by default). Additionally, MSVC always generates a .debug$S section and a S_COMPILE3 record, which Clang doesn't do without /Z7. Therefore, the following MSVC command-line "cl /c file.cpp" would have to be written with Clang such as "clang-cl /c file.cpp /HOTPATCH /Z7" in order to obtain the same result.

Depends on D43002, D80833 and D81301 for the full feature.

Differential Revision: https://reviews.llvm.org/D116511

Added: 
    clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp
    clang/test/CodeGenCXX/debug-info-hotpatch.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/CodeGenOptions.def
    clang/include/clang/Driver/Options.td
    clang/lib/CodeGen/BackendUtil.cpp
    clang/lib/CodeGen/CodeGenFunction.cpp
    clang/lib/Driver/ToolChains/Clang.cpp
    clang/lib/Driver/ToolChains/MSVC.cpp
    clang/test/CodeGen/patchable-function-entry.c
    clang/test/Driver/cl-options.c
    llvm/include/llvm/Target/TargetOptions.h
    llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
    llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll
    llvm/test/MC/AArch64/coff-debug.ll

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index de395b5d035ec..08e4d75299d2b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -136,6 +136,16 @@ Windows Support
   or pass ``/permissive`` to disable C++ operator names altogether. See
   `PR42427 <https://llvm.org/pr42427>` for more info.
 
+- Add support for MSVC-compatible ``/hotpatch`` flag in clang-cl, and equivalent
+  -cc1 flag ``-fms-hotpatch``. Along with the linker flag ``/functionpadmin``
+  this creates executable images suitable for runtime code patching. This flag
+  is only required for x86/x64 targets; ARM/ARM64 simply needs the linker
+  ``/functionpadmin``.
+
+  With this addition, clang-cl can be used in live code patching scenarios,
+  along with tools such as Live++ or Recode. Microsoft Edit and Continue isn't
+  currently supported.
+
 C Language Changes in Clang
 ---------------------------
 

diff  --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index d4742cddd00c9..3526b8a4a9044 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -139,6 +139,9 @@ VALUE_CODEGENOPT(XRaySelectedFunctionGroup, 32, 0)
 VALUE_CODEGENOPT(PatchableFunctionEntryCount , 32, 0) ///< Number of NOPs at function entry
 VALUE_CODEGENOPT(PatchableFunctionEntryOffset , 32, 0)
 
+CODEGENOPT(HotPatch, 1, 0) ///< Supports the Microsoft /HOTPATCH flag and
+                           ///< generates a 'patchable-function' attribute.
+
 CODEGENOPT(InstrumentForProfiling , 1, 0) ///< Set when -pg is enabled.
 CODEGENOPT(CallFEntry , 1, 0) ///< Set when -mfentry is enabled.
 CODEGENOPT(MNopMCount , 1, 0) ///< Set when -mnop-mcount is enabled.

diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 1a9fd61328f83..b66363b1d3e92 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2498,6 +2498,9 @@ defm pascal_strings : BoolFOption<"pascal-strings",
 def fpatchable_function_entry_EQ : Joined<["-"], "fpatchable-function-entry=">, Group<f_Group>, Flags<[CC1Option]>,
   MetaVarName<"<N,M>">, HelpText<"Generate M NOPs before function entry and N-M NOPs after function entry">,
   MarshallingInfoInt<CodeGenOpts<"PatchableFunctionEntryCount">>;
+def fms_hotpatch : Flag<["-"], "fms-hotpatch">, Group<f_Group>, Flags<[CC1Option, CoreOption]>,
+  HelpText<"Ensure that all functions can be hotpatched at runtime">,
+  MarshallingInfoFlag<CodeGenOpts<"HotPatch">>;
 def fpcc_struct_return : Flag<["-"], "fpcc-struct-return">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Override the default ABI to return all structs on the stack">;
 def fpch_preprocess : Flag<["-"], "fpch-preprocess">, Group<f_Group>;
@@ -6124,6 +6127,8 @@ def _SLASH_Gw_ : CLFlag<"Gw-">,
 def _SLASH_help : CLFlag<"help">, Alias<help>,
   HelpText<"Display available options">;
 def _SLASH_HELP : CLFlag<"HELP">, Alias<help>;
+def _SLASH_hotpatch : CLFlag<"hotpatch">, Alias<fms_hotpatch>,
+  HelpText<"Create hotpatchable image">;
 def _SLASH_I : CLJoinedOrSeparate<"I">,
   HelpText<"Add directory to include search path">, MetaVarName<"<dir>">,
   Alias<I>;
@@ -6480,7 +6485,6 @@ def _SLASH_headerUnit : CLJoinedOrSeparate<"headerUnit">;
 def _SLASH_headerUnitAngle : CLJoinedOrSeparate<"headerUnit:angle">;
 def _SLASH_headerUnitQuote : CLJoinedOrSeparate<"headerUnit:quote">;
 def _SLASH_homeparams : CLFlag<"homeparams">;
-def _SLASH_hotpatch : CLFlag<"hotpatch">;
 def _SLASH_kernel : CLFlag<"kernel">;
 def _SLASH_LN : CLFlag<"LN">;
 def _SLASH_MP : CLJoined<"MP">;

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 67fee7f35ca17..6b8e052305b49 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -645,6 +645,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
   Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
   Options.DebugStrictDwarf = CodeGenOpts.DebugStrictDwarf;
   Options.ObjectFilenameForDebug = CodeGenOpts.ObjectFilenameForDebug;
+  Options.Hotpatch = CodeGenOpts.HotPatch;
 
   return true;
 }

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 54ddbff3fb038..50e1638924d1a 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -882,6 +882,13 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
     if (Offset)
       Fn->addFnAttr("patchable-function-prefix", std::to_string(Offset));
   }
+  // Instruct that functions for COFF/CodeView targets should start with a
+  // patchable instruction, but only on x86/x64. Don't forward this to ARM/ARM64
+  // backends as they don't need it -- instructions on these architectures are
+  // always atomically patchable at runtime.
+  if (CGM.getCodeGenOpts().HotPatch &&
+      getContext().getTargetInfo().getTriple().isX86())
+    Fn->addFnAttr("patchable-function", "prologue-short-redirect");
 
   // Add no-jump-tables value.
   if (CGM.getCodeGenOpts().NoUseJumpTables)

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index c48c6fd59bec3..a22f03a488486 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6001,6 +6001,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
+  Args.AddLastArg(CmdArgs, options::OPT_fms_hotpatch);
+
   if (TC.SupportsProfiling()) {
     Args.AddLastArg(CmdArgs, options::OPT_pg);
 

diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index 4e15c3ab51cef..7e8636adc2728 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -511,6 +511,11 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   if (Args.hasArg(options::OPT_g_Group, options::OPT__SLASH_Z7))
     CmdArgs.push_back("-debug");
 
+  // If we specify /hotpatch, let the linker add padding in front of each
+  // function, like MSVC does.
+  if (Args.hasArg(options::OPT_fms_hotpatch, options::OPT__SLASH_hotpatch))
+    CmdArgs.push_back("-functionpadmin");
+
   // Pass on /Brepro if it was passed to the compiler.
   // Note that /Brepro maps to -mno-incremental-linker-compatible.
   bool DefaultIncrementalLinkerCompatible =

diff  --git a/clang/test/CodeGen/patchable-function-entry.c b/clang/test/CodeGen/patchable-function-entry.c
index 3065eb2efa551..6e8d0d743cf45 100644
--- a/clang/test/CodeGen/patchable-function-entry.c
+++ b/clang/test/CodeGen/patchable-function-entry.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fpatchable-function-entry=1 -o - | FileCheck --check-prefixes=CHECK,OPT %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -fms-hotpatch -o - | FileCheck --check-prefixes=HOTPATCH %s
 
 // CHECK: define{{.*}} void @f0() #0
 __attribute__((patchable_function_entry(0))) void f0() {}
@@ -34,3 +35,7 @@ void f() {}
 // CHECK: attributes #2 = { {{.*}} "patchable-function-entry"="0" "patchable-function-prefix"="4"
 // CHECK: attributes #3 = { {{.*}} "patchable-function-entry"="3" "patchable-function-prefix"="2"
 // OPT:   attributes #4 = { {{.*}} "patchable-function-entry"="1"
+// HOTPATCH: attributes #0 = { {{.*}} "patchable-function"="prologue-short-redirect"
+// HOTPATCH: attributes #1 = { {{.*}} "patchable-function"="prologue-short-redirect"
+// HOTPATCH: attributes #2 = { {{.*}} "patchable-function"="prologue-short-redirect"
+// HOTPATCH: attributes #3 = { {{.*}} "patchable-function"="prologue-short-redirect"

diff  --git a/clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp b/clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp
new file mode 100644
index 0000000000000..6176f1788760a
--- /dev/null
+++ b/clang/test/CodeGenCXX/debug-info-hotpatch-arm.cpp
@@ -0,0 +1,26 @@
+// REQUIRES: aarch64-registered-target || arm-registered-target
+///
+/// Check that using /hotpatch doesn't generate an error.
+/// Binaries are always hotpatchable on ARM/ARM64.
+///
+// RUN: %clang_cl --target=arm-pc-windows-msvc /c /hotpatch /Z7 -- %s 2>&1
+// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /hotpatch /Z7 -- %s 2>&1
+///
+/// Ensure that we set the hotpatchable flag in the debug information.
+///
+// RUN: %clang_cl --target=arm-pc-windows-msvc /c /Z7 -o %t.obj -- %s
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH
+// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c /Z7 -o %t.obj -- %s
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH
+// HOTPATCH: S_COMPILE3 [size = [[#]]]
+// HOTPATCH: flags = hot patchable
+///
+/// Unfortunately we need /Z7, Clang does not systematically generate S_COMPILE3.
+///
+// RUN: %clang_cl --target=aarch64-pc-windows-msvc /c -o %t.obj -- %s
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH
+// NO-HOTPATCH-NOT: flags = hot patchable
+
+int main() {
+  return 0;
+}

diff  --git a/clang/test/CodeGenCXX/debug-info-hotpatch.cpp b/clang/test/CodeGenCXX/debug-info-hotpatch.cpp
new file mode 100644
index 0000000000000..fde1a6ad085ea
--- /dev/null
+++ b/clang/test/CodeGenCXX/debug-info-hotpatch.cpp
@@ -0,0 +1,20 @@
+// REQUIRES: x86-registered-target
+///
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /hotpatch /Z7 -o %t.obj -- %s
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=HOTPATCH
+// HOTPATCH: S_COMPILE3 [size = [[#]]]
+// HOTPATCH: flags = hot patchable
+///
+// RUN: %clang_cl --target=x86_64-windows-msvc /c /Z7 -o %t.obj -- %s
+// RUN: llvm-pdbutil dump -symbols %t.obj | FileCheck %s --check-prefix=NO-HOTPATCH
+// NO-HOTPATCH-NOT: flags = hot patchable
+///
+// RUN: %clang_cl --target=x86_64-windows-msvc /hotpatch -### -- %s 2>&1 \
+// RUN:    | FileCheck %s --check-prefix=FUNCTIONPADMIN
+// FUNCTIONPADMIN: clang{{.*}}
+// FUNCTIONPADMIN: {{link.*"}}
+// FUNCTIONPADMIN: -functionpadmin
+
+int main() {
+  return 0;
+}

diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 618be2d230f94..f39db87660125 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -118,6 +118,9 @@
 // RUN: %clang_cl /Gw /Gw- -### -- %s 2>&1 | FileCheck -check-prefix=Gw_ %s
 // Gw_-NOT: -fdata-sections
 
+// RUN: %clang_cl /hotpatch -### -- %s 2>&1 | FileCheck -check-prefix=hotpatch %s
+// hotpatch: -fms-hotpatch
+
 // RUN: %clang_cl /Imyincludedir -### -- %s 2>&1 | FileCheck -check-prefix=SLASH_I %s
 // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck -check-prefix=SLASH_I %s
 // SLASH_I: "-I" "myincludedir"
@@ -483,7 +486,6 @@
 // RUN:     /GZ \
 // RUN:     /H \
 // RUN:     /homeparams \
-// RUN:     /hotpatch \
 // RUN:     /JMC \
 // RUN:     /kernel \
 // RUN:     /LN \

diff  --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h
index c639f326abc9d..a636c48228325 100644
--- a/llvm/include/llvm/Target/TargetOptions.h
+++ b/llvm/include/llvm/Target/TargetOptions.h
@@ -140,9 +140,9 @@ namespace llvm {
           EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
           EmitAddrsig(false), EmitCallSiteInfo(false),
           SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
-          ValueTrackingVariableLocations(false),
-          ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
-          DebugStrictDwarf(false),
+          ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
+          XRayOmitFunctionIndex(false), DebugStrictDwarf(false),
+          Hotpatch(false),
           FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
     /// DisableFramePointerElim - This returns true if frame pointer elimination
@@ -342,6 +342,9 @@ namespace llvm {
     /// By default, it is set to false.
     unsigned DebugStrictDwarf : 1;
 
+    /// Emit the hotpatch flag in CodeView debug.
+    unsigned Hotpatch : 1;
+
     /// Name of the stack usage file (i.e., .su file) if user passes
     /// -fstack-usage. If empty, it can be implied that -fstack-usage is not
     /// passed on the command line.

diff  --git a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
index 9a8188e5cb468..52c74713551cb 100644
--- a/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
@@ -846,6 +846,12 @@ void CodeViewDebug::emitCompilerInformation() {
   if (MMI->getModule()->getProfileSummary(/*IsCS*/ false) != nullptr) {
     Flags |= static_cast<uint32_t>(CompileSym3Flags::PGO);
   }
+  using ArchType = llvm::Triple::ArchType;
+  ArchType Arch = Triple(MMI->getModule()->getTargetTriple()).getArch();
+  if (Asm->TM.Options.Hotpatch || Arch == ArchType::thumb ||
+      Arch == ArchType::aarch64) {
+    Flags |= static_cast<uint32_t>(CompileSym3Flags::HotPatch);
+  }
 
   OS.AddComment("Flags and language");
   OS.emitInt32(Flags);

diff  --git a/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll b/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll
index d19cca330b2d3..781a4c65abc90 100644
--- a/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll
+++ b/llvm/test/DebugInfo/COFF/ARMNT/arm-register-variables.ll
@@ -24,7 +24,8 @@
 ;      OBJ:   Compile3Sym {
 ; OBJ-NEXT:     Kind: S_COMPILE3 (0x113C)
 ; OBJ-NEXT:     Language: C (0x0)
-; OBJ-NEXT:     Flags [ (0x0)
+; OBJ-NEXT:     Flags [ (0x4000)
+; OBJ-NEXT:       HotPatch (0x4000)
 ; OBJ-NEXT:     ]
 ; OBJ-NEXT:     Machine: ARMNT (0xF4)
 

diff  --git a/llvm/test/MC/AArch64/coff-debug.ll b/llvm/test/MC/AArch64/coff-debug.ll
index 6099b3d570b46..af3459ace3531 100644
--- a/llvm/test/MC/AArch64/coff-debug.ll
+++ b/llvm/test/MC/AArch64/coff-debug.ll
@@ -77,7 +77,8 @@ attributes #0 = { noinline nounwind optnone "correctly-rounded-divide-sqrt-fp-ma
 ; CHECK:     Compile3Sym {
 ; CHECK:       Kind: S_COMPILE3 (0x113C)
 ; CHECK:       Language: C (0x0)
-; CHECK:       Flags [ (0x0)
+; CHECK:       Flags [ (0x4000
+; CHECK:        HotPatch (0x4000)
 ; CHECK:       ]
 ; CHECK:     }
 ; CHECK:   ]


        


More information about the cfe-commits mailing list