[PATCH] D40531: Emit function IDs table for Control Flow Guard

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Take a look at llvm/lib/MC/MCParser/COFFAsmParser.cpp and add the .symidx directive following the example of ParseDirectiveSecIdx. You can test it in test/MC/COFF/symidx.s with llvm-readobj.



================
Comment at: llvm/include/llvm/MC/MCStreamer.h:484-485
 
+  /// \brief Emits the symbol table index of a Symbol into the .gfids$y section.
+  virtual void EmitCOFFCFGuard(MCSymbol const *Symbol);
+
----------------
I think what we really want is a new directive that emits the symbol table index of the given symbol. I'd suggest calling it ".symidx" since it's kind of similar to the other COFF-specific .secidx and .secrel32 directives that we already have. With that in mind, I'd rename this to `EmitCOFFSymbolIndex` and update the comment to say it emits the 32-bit symbol table index of the given symbol to the current section.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:384-385
+    Handlers.push_back(HandlerInfo(new WinCFGuard(this), CFGuardName,
+                                   CFGuardDescription, DWARFGroupName,
+                                   DWARFGroupDescription));
+
----------------
DWARFGroupName?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp:35
+  const Module *M = Asm->MMI->getModule();
+  if (mdconst::extract_or_null<ConstantInt>(M->getModuleFlag("cfguard"))) {
+    std::vector<const Function *> Functions;
----------------
I think we can remove this check. If this was added to the AsmPrinterHandler list, then we've already done this check.


================
Comment at: llvm/lib/MC/MCAsmStreamer.cpp:631
+void MCAsmStreamer::EmitCOFFCFGuard(MCSymbol const *Symbol) {
+  OS << MAI->getData16bitsDirective();
+  Symbol->print(OS, MAI);
----------------
This is where you'll want to emit our new assembler directive. Something like:
  OS << "\t.symidx ";
  Symbol->print(OS, MAI);
  EmitEOL();


================
Comment at: llvm/lib/MC/MCWinCOFFStreamer.cpp:196
 
+void MCWinCOFFStreamer::EmitCOFFCFGuard(MCSymbol const *Symbol) {
+  MCSection *CFGData = getCurrentSectionOnly();
----------------
This will ultimately be named EmitCOFFSymbolIndex and it'll then make sense that it makes an MCSymbolIdFragment.


================
Comment at: llvm/lib/MC/MCWinCOFFStreamer.cpp:197
+void MCWinCOFFStreamer::EmitCOFFCFGuard(MCSymbol const *Symbol) {
+  MCSection *CFGData = getCurrentSectionOnly();
+  getAssembler().registerSection(*CFGData);
----------------
I'd just call this variable `Sec` or something not specific to control flow guard.


https://reviews.llvm.org/D40531