[LLD] Implement /guard:[no]ehcont

[pengfei]

Add EHCont to load config table.

[pengfei]

Hi @rnk , I'm implementing EHCont guard in LLD referring to your longjmp patch. Currently the binary's EHCont table always be null. I guess it results from I haven't told lld where's the place it should put the EHCont table in the load config table.
But I didn't find any related code in longjmp patch. Could you point me where the code is? Or I misunderstood something?

[rnk]

In D99078#2644001, @pengfei wrote:

Hi @rnk , I'm implementing EHCont guard in LLD referring to your longjmp patch. Currently the binary's EHCont table always be null. I guess it results from I haven't told lld where's the place it should put the EHCont table in the load config table.
But I didn't find any related code in longjmp patch. Could you point me where the code is? Or I misunderstood something?

The CRT is actually responsible for providing a struct defining the load configuration. You can see in the LLD gfids tests that they all have a blob like this:

        .section .rdata,"dr"
.globl _load_config_used
_load_config_used:
        .long 256
        .fill 124, 1, 0
        .quad __guard_fids_table
        .quad __guard_fids_count
        .long __guard_flags
        .fill 128, 1, 0

To add ehcont tests, you will need to provide a similar load config that llvm-readobj can parse. Hopefully that answers your question.

[pengfei]

Solved the problems.

[pengfei]

In D99078#2645963, @rnk wrote:

In D99078#2644001, @pengfei wrote:

Hi @rnk , I'm implementing EHCont guard in LLD referring to your longjmp patch. Currently the binary's EHCont table always be null. I guess it results from I haven't told lld where's the place it should put the EHCont table in the load config table.
But I didn't find any related code in longjmp patch. Could you point me where the code is? Or I misunderstood something?

The CRT is actually responsible for providing a struct defining the load configuration. You can see in the LLD gfids tests that they all have a blob like this:

        .section .rdata,"dr"
.globl _load_config_used
_load_config_used:
        .long 256
        .fill 124, 1, 0
        .quad __guard_fids_table
        .quad __guard_fids_count
        .long __guard_flags
        .fill 128, 1, 0

To add ehcont tests, you will need to provide a similar load config that llvm-readobj can parse. Hopefully that answers your question.

Yes. It solves my problems. Thanks very much.
Now, the last question: I found it works with a given load configuration in the test (That's why I'm sure it's the right direction), but still doesn't work when using clang-cl or/and lld-link to build a cpp file. I guess it maybe related to the lld-link that doesn't use the latest CRT. Do you know how to fix this problem?

lld/test/COFF/pdb-natvis.test

17 ↗

(On Diff #333000)

I have no idea how this been affected by this patch.

[pengfei]

1. Remove lib dependency to make linux testing happy.
2. Revert the change to pdb-natvis.test.

[pengfei]

lld/test/COFF/pdb-natvis.test

17 ↗

(On Diff #333000)

Since the change results in the build server fail, I suspect if it is my local problem. Reverted it to see if the build server happy.

[rnk]

Looks good!

[pengfei]

In D99078#2687146, @rnk wrote:

Looks good!

Thanks for the review.

[alvinhochun]

lld/COFF/DriverUtils.cpp

104–114

This changed the behaviour of /guard:cf. Before this change, /guard:cf is equivalent to /guard:cf,longjmp by default, but after this change one must explicitly pass /guard:longjmp to`lld-link for it to generate the guard long jump table.

This also does not match the behaviour of MSVC's link.exe. From my quick testing, /guard:cf, /guard:longjmp and /guard:cf,longjmp all do the same thing for it to enable both CFGuard and the long jump table. Only when you pass /guard:cf,nolongjmp is the jump table not present in the final image.

[pengfei]

lld/COFF/DriverUtils.cpp

104–114

Thanks for raising the problem. I don't find any description about the longjmp option in official document. Unless there's explicit document about the relationship between /guard:cf, /guard:longjmp and /guard:cf,longjmp, I don't think we need to match the exact behavior with MSVC. The single /guard:cf has ambiguity since we are supporting ehcont. So I'm not sure if the current MSVC behavior is constant in future versions.

[alvinhochun]

lld/COFF/DriverUtils.cpp

104–114

I just think that, long jump guard seems to be a rather cheap hardening measure when control flow guard is already in use. Not enabling it by default perhaps seems a bit odd, especially since this feels like an unintentional change. But fine with me either way (I don't really use MSVC or Clang with MSVC target, just noticed the difference when investigating this LLD feature.)

[pengfei]

lld/COFF/DriverUtils.cpp

104–114

Thanks for the point. I'm not a MSVC/clang-cl user either. So I couldn't weight how cheap the long jump guard is. I designed it in the perspective of a junior user, to whom it's hard to understand the longjmp is on by default for some reason and need nolongjmp to disable it, while ehcont is on the contrary.
It would have value from the perspective of backward compatibility if we chose this way at the beginning. But the patch was merged more than one year... So let's keep it as is :)

[rnk]

lld/COFF/DriverUtils.cpp

104–114

I agree, longjump protections should be implied by /guard:cf. The nolongjump flag exists merely as a backwards compatibility escape hatch. Users should generally not specify /guard:longjump.

[mstorsjo]

lld/COFF/DriverUtils.cpp

104–114

Also, FWIW, even if the exact behaviour of Microsoft's tools isn't documented, such a detail like this would most probably not change randomly from one version to another. I also would prefer that we match link.exe's behaviour exactly on this point, instead of making up our own rules for how these flags interact.

[pengfei]

lld/COFF/DriverUtils.cpp

104–114

Thanks @rnk @mstorsjo ! That makes sense to me. I put D132901 to revert the behavior, please take a look, thanks!

lld/COFF/Config.h

 
 enum class DebugType {
   None  = 0x0,
   CV    = 0x1,  /// CodeView
   PData = 0x2,  /// Procedure Data
   Fixup = 0x4,  /// Relocation Table
 };
 
-enum class GuardCFLevel {
-  Off,
-  NoLongJmp, // Emit gfids but no longjmp tables
-  Full,      // Enable all protections.
+enum GuardCFLevel {
+  Off     = 0x0,
+  CF      = 0x1, /// Emit gfids tables
+  LongJmp = 0x2, /// Emit longjmp tables
+  EHCont  = 0x4, /// Emit ehcont tables
+  All     = 0x7  /// Enable all protections
 };
 
 enum class ICFLevel {
   None,
   Safe, // Safe ICF for all sections.
   All,  // Aggressive ICF for code, but safe ICF for data, similar to MSVC's
         // behavior.
 };
struct Configuration {
   bool hadExplicitExports;
   std::set<std::string> delayLoads;
   std::map<std::string, int> dllOrder;
   Symbol *delayLoadHelper = nullptr;
 
   bool saveTemps = false;
 
   // /guard:cf
-  GuardCFLevel guardCF = GuardCFLevel::Off;
+  int guardCF = GuardCFLevel::Off;
 
   // Used for SafeSEH.
   bool safeSEH = false;
   Symbol *sehTable = nullptr;
   Symbol *sehCount = nullptr;
   bool noSEH = false;
 
   // Used for /opt:lldlto=N

lld/COFF/Driver.cpp

void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
 
   symtab->addAbsolute(mangle("__guard_fids_count"), 0);
   symtab->addAbsolute(mangle("__guard_fids_table"), 0);
   symtab->addAbsolute(mangle("__guard_flags"), 0);
   symtab->addAbsolute(mangle("__guard_iat_count"), 0);
   symtab->addAbsolute(mangle("__guard_iat_table"), 0);
   symtab->addAbsolute(mangle("__guard_longjmp_count"), 0);
   symtab->addAbsolute(mangle("__guard_longjmp_table"), 0);
+  symtab->addAbsolute(mangle("__guard_ehcont_count"), 0);
+  symtab->addAbsolute(mangle("__guard_ehcont_table"), 0);
   // Needed for MSVC 2017 15.5 CRT.
   symtab->addAbsolute(mangle("__enclave_config"), 0);
 
   if (config->pseudoRelocs) {
     symtab->addAbsolute(mangle("__RUNTIME_PSEUDO_RELOC_LIST__"), 0);
     symtab->addAbsolute(mangle("__RUNTIME_PSEUDO_RELOC_LIST_END__"), 0);
   }
   if (config->mingw) {

lld/COFF/DriverUtils.cpp

 }
 
 void parseGuard(StringRef fullArg) {
   SmallVector<StringRef, 1> splitArgs;
   fullArg.split(splitArgs, ",");
   for (StringRef arg : splitArgs) {
     if (arg.equals_lower("no"))
       config->guardCF = GuardCFLevel::Off;
     else if (arg.equals_lower("nolongjmp"))
-      config->guardCF = GuardCFLevel::NoLongJmp;
-    else if (arg.equals_lower("cf") || arg.equals_lower("longjmp"))
-      config->guardCF = GuardCFLevel::Full;
+      config->guardCF &= ~GuardCFLevel::LongJmp;
+    else if (arg.equals_lower("noehcont"))
+      config->guardCF &= ~GuardCFLevel::EHCont;
+    else if (arg.equals_lower("cf"))
+      config->guardCF = GuardCFLevel::CF;
+    else if (arg.equals_lower("longjmp"))
+      config->guardCF |= GuardCFLevel::CF | GuardCFLevel::LongJmp;
+    else if (arg.equals_lower("ehcont"))
+      config->guardCF |= GuardCFLevel::CF | GuardCFLevel::EHCont;
     else
       fatal("invalid argument to /guard: " + arg);
   }
 }
 
 // Parses a string in the form of "<subsystem>[,<integer>[.<integer>]]".
 void parseSubsystem(StringRef arg, WindowsSubsystem *sys, uint32_t *major,
                     uint32_t *minor, bool *gotVersion) {
   StringRef sysStr, ver;

lld/COFF/InputFiles.h

public:
   void parse() override;
   MachineTypes getMachineType() override;
   ArrayRef<Chunk *> getChunks() { return chunks; }
   ArrayRef<SectionChunk *> getDebugChunks() { return debugChunks; }
   ArrayRef<SectionChunk *> getSXDataChunks() { return sxDataChunks; }
   ArrayRef<SectionChunk *> getGuardFidChunks() { return guardFidChunks; }
   ArrayRef<SectionChunk *> getGuardIATChunks() { return guardIATChunks; }
   ArrayRef<SectionChunk *> getGuardLJmpChunks() { return guardLJmpChunks; }
+  ArrayRef<SectionChunk *> getGuardEHContChunks() { return guardEHContChunks; }
   ArrayRef<Symbol *> getSymbols() { return symbols; }
 
   MutableArrayRef<Symbol *> getMutableSymbols() { return symbols; }
 
   ArrayRef<uint8_t> getDebugSection(StringRef secName);
 
   // Returns a Symbol object for the symbolIndex'th symbol in the
   // underlying object file.
public:
   // like stack guard, safeseh, /guard:cf, or other things.
   uint32_t feat00Flags = 0;
 
   // True if this object file is compatible with SEH.  COFF-specific and
   // x86-only. COFF spec 5.10.1. The .sxdata section.
   bool hasSafeSEH() { return feat00Flags & 0x1; }
 
   // True if this file was compiled with /guard:cf.
-  bool hasGuardCF() { return feat00Flags & 0x800; }
+  bool hasGuardCF() { return feat00Flags & 0x4800; }
 
   // Pointer to the PDB module descriptor builder. Various debug info records
   // will reference object files by "module index", which is here. Things like
   // source files and section contributions are also recorded here. Will be null
   // if we are not producing a PDB.
   llvm::pdb::DbiModuleDescriptorBuilder *moduleDBI = nullptr;
 
   const coff_section *addrsigSec = nullptr;
private:
   // CodeView debug info sections.
   std::vector<SectionChunk *> debugChunks;
 
   // Chunks containing symbol table indices of exception handlers. Only used for
   // 32-bit x86.
   std::vector<SectionChunk *> sxDataChunks;
 
   // Chunks containing symbol table indices of address taken symbols, address
-  // taken IAT entries, and longjmp targets. These are not linked into the
-  // final binary when /guard:cf is set.
+  // taken IAT entries, longjmp and ehcont targets. These are not linked into
+  // the final binary when /guard:cf is set.
   std::vector<SectionChunk *> guardFidChunks;
   std::vector<SectionChunk *> guardIATChunks;
   std::vector<SectionChunk *> guardLJmpChunks;
+  std::vector<SectionChunk *> guardEHContChunks;
 
   // This vector contains a list of all symbols defined or referenced by this
   // file. They are indexed such that you can get a Symbol by symbol
   // index. Nonexistent indices (which are occupied by auxiliary
   // symbols in the real symbol table) are filled with null pointers.
   std::vector<Symbol *> symbols;
 
   // This vector contains the same chunks as Chunks, but they are

lld/COFF/InputFiles.cpp

SectionChunk *ObjFile::readSection(uint32_t sectionNumber,
   if (c->isCodeView())
     debugChunks.push_back(c);
   else if (name == ".gfids$y")
     guardFidChunks.push_back(c);
   else if (name == ".giats$y")
     guardIATChunks.push_back(c);
   else if (name == ".gljmp$y")
     guardLJmpChunks.push_back(c);
+  else if (name == ".gehcont$y")
+    guardEHContChunks.push_back(c);
   else if (name == ".sxdata")
     sxDataChunks.push_back(c);
   else if (config->tailMerge && sec->NumberOfRelocations == 0 &&
            name == ".rdata" && leaderName.startswith("??_C@"))
     // COFF sections that look like string literal sections (i.e. no
     // relocations, in .rdata, leader symbol name matches the MSVC name mangling
     // for string literals) are subject to string tail merging.
     MergeChunk::addSection(c);

lld/COFF/Writer.cpp

 // Create the guard function id table. This is a table of RVAs of all
 // address-taken functions. It is sorted and uniqued, just like the safe SEH
 // table.
 void Writer::createGuardCFTables() {
   SymbolRVASet addressTakenSyms;
   SymbolRVASet giatsRVASet;
   std::vector<Symbol *> giatsSymbols;
   SymbolRVASet longJmpTargets;
+  SymbolRVASet ehContTargets;
   for (ObjFile *file : ObjFile::instances) {
     // If the object was compiled with /guard:cf, the address taken symbols
-    // are in .gfids$y sections, and the longjmp targets are in .gljmp$y
-    // sections. If the object was not compiled with /guard:cf, we assume there
-    // were no setjmp targets, and that all code symbols with relocations are
-    // possibly address-taken.
+    // are in .gfids$y sections, the longjmp targets are in .gljmp$y sections,
+    // and ehcont targets are in .gehcont$y sections. If the object was not
+    // compiled with /guard:cf, we assume there were no setjmp and ehcont
+    // targets, and that all code symbols with relocations are possibly
+    // address-taken.
     if (file->hasGuardCF()) {
       markSymbolsForRVATable(file, file->getGuardFidChunks(), addressTakenSyms);
       markSymbolsForRVATable(file, file->getGuardIATChunks(), giatsRVASet);
       getSymbolsFromSections(file, file->getGuardIATChunks(), giatsSymbols);
       markSymbolsForRVATable(file, file->getGuardLJmpChunks(), longJmpTargets);
+      markSymbolsForRVATable(file, file->getGuardEHContChunks(), ehContTargets);
     } else {
       markSymbolsWithRelocations(file, addressTakenSyms);
     }
   }
 
   // Mark the image entry as address-taken.
   if (config->entry)
     maybeAddAddressTakenFunction(addressTakenSyms, config->entry);
void Writer::createGuardCFTables() {
   maybeAddRVATable(std::move(addressTakenSyms), "__guard_fids_table",
                    "__guard_fids_count");
 
   // Add the Guard Address Taken IAT Entry Table (.giats).
   maybeAddRVATable(std::move(giatsRVASet), "__guard_iat_table",
                    "__guard_iat_count");
 
   // Add the longjmp target table unless the user told us not to.
-  if (config->guardCF == GuardCFLevel::Full)
+  if (config->guardCF & GuardCFLevel::LongJmp)
     maybeAddRVATable(std::move(longJmpTargets), "__guard_longjmp_table",
                      "__guard_longjmp_count");
 
+  // Add the ehcont target table unless the user told us not to.
+  if (config->guardCF & GuardCFLevel::EHCont)
+    maybeAddRVATable(std::move(ehContTargets), "__guard_ehcont_table",
+                     "__guard_ehcont_count");
+
   // Set __guard_flags, which will be used in the load config to indicate that
   // /guard:cf was enabled.
   uint32_t guardFlags = uint32_t(coff_guard_flags::CFInstrumented) |
                         uint32_t(coff_guard_flags::HasFidTable);
-  if (config->guardCF == GuardCFLevel::Full)
+  if (config->guardCF & GuardCFLevel::LongJmp)
     guardFlags |= uint32_t(coff_guard_flags::HasLongJmpTable);
+  if (config->guardCF & GuardCFLevel::EHCont)
+    guardFlags |= uint32_t(coff_guard_flags::HasEHContTable);
   Symbol *flagSym = symtab->findUnderscore("__guard_flags");
   cast<DefinedAbsolute>(flagSym)->setVA(guardFlags);
 }
 
 // Take a list of input sections containing symbol table indices and add those
 // symbols to a vector. The challenge is that symbol RVAs are not known and
 // depend on the table size, so we can't directly build a set of integers.
 void Writer::getSymbolsFromSections(ObjFile *file,

llvm/include/llvm/Object/COFF.h

 
 /// Bits in control flow guard flags as we understand them.
 enum class coff_guard_flags : uint32_t {
   CFInstrumented = 0x00000100,
   HasFidTable = 0x00000400,
   ProtectDelayLoadIAT = 0x00001000,
   DelayLoadIATSection = 0x00002000, // Delay load in separate section
   HasLongJmpTable = 0x00010000,
+  HasEHContTable = 0x00400000,
   FidTableHasFlags = 0x10000000, // Indicates that fid tables are 5 bytes
 };
 
 enum class frame_type : uint16_t { Fpo = 0, Trap = 1, Tss = 2, NonFpo = 3 };
 
 struct coff_load_config_code_integrity {
   support::ulittle16_t Flags;
   support::ulittle16_t Catalog;