[X86][ELF][CET] Adding the .note.gnu.property ELF section in X86

[mike.dvoretsky]

Summary

In preparation for the proposed linker ABI changes (https://github.com/hjl-tools/linux-abi/wiki/linux-abi-draft.pdf, https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-cet.pdf), this patch enables emission of the .note.gnu.property section to ELF object files when building CET-enabled modules.

[mike.dvoretsky]

Uploaded the version without the test for the section emission. Corrected.

[mike.dvoretsky]

Updated to use full 8/4 byte integers to store the flags rather than using 1 byte and padding.

[mike.dvoretsky]

Enabled readobj to parse sections that use 32 bits for the data on 64-bit targets (like gcc currently does) and added explicit alignment to the section generation.

[craig.topper]

llvm/tools/llvm-readobj/ELFDumper.cpp

3504

This format string is wrong for a uint64_t. If you set a bad bit in the upper 32-bits this justs prints <unknown flags: 0x0>

[grimar]

I think this should be splitted into 2 patches: one for codegen and one for llvm-readobj tool.

llvm/tools/llvm-readobj/ELFDumper.cpp

3475

That looks odd. ELFT::uint can be 4 or 8 and Elf_Word is 4, so It is the same as

if (DataSize == 4 || DataSize == 8)

I would go with that then.

It is also probably better to return early:

if (DataSize != 4 && DataSize != 8) {
 OS << format("<corrupt length: 0x%x>\n", DataSize);
 break;
}
...

3479

And here then you can use something like following then
support::endian::read32<ELFT::TargetEndianness>(Data.data()) and support::endian::read64<ELFT::TargetEndianness>(Data.data())

3480

You can move it below the if (CFProtection == 0) since it is only used later.

3489

I think you can avoid using if (First) here, because it is always true here, no?

3497

Actually, it seems you can get rid of the First variable and just check the flags:

if (CFProtection & GNU_PROPERTY_X86_FEATURE_1_IBT) {
  CFProtection &= ~GNU_PROPERTY_X86_FEATURE_1_IBT;
  OS << "IBT";
  if (CFProtection)
    OS << ", ";
}

if (CFProtection & GNU_PROPERTY_X86_FEATURE_1_SHSTK) {
  CFProtection &= ~GNU_PROPERTY_X86_FEATURE_1_SHSTK;
  OS << "SHSTK";
  if (CFProtection)
    OS << ", ";
}

if (CFProtection)
  OS << format("<unknown flags: 0x%x>", CFProtection);

[mike.dvoretsky]

Made changes to the readobj part per comments. Will wait on comment from @craig.topper before splitting the patch.

[craig.topper]

Go ahead and split the patch.

[mike.dvoretsky]

Removed the llvm-readobj part to release as a separate patch.

[jhenderson]

llvm/lib/Target/X86/X86AsmPrinter.cpp

566–569

Can't you use EmitBytes to achieve this?

[grimar]

llvm/lib/Target/X86/X86AsmPrinter.cpp

557

I would move this assert to the beginning of the scope
and also use isArch64Bit for consistency:

if (FeatureFlagsAnd) {
  assert((TT.isArch32Bit() || TT.isArch64Bit()) && ...

But actually.. is it really possible to arch be 16 bits in X86AsmPrinter?
It seems this assert is just redundant. And if that would be possible, it should be
an explicit error probably.

[mike.dvoretsky]

Replaced assertion with a more explicit error at the start of the code path. Changed the emission of the "GNU\0" byte sequence to use EmitBytes with the StringRef constructor specifically to include the trailing zero byte.

The ELF.h file is no longer affected by this patch since D47473 was commited.

[grimar]

llvm/lib/Target/X86/X86AsmPrinter.cpp

545

This could be uint8_t, though I think this better be just unsigned.

556

Do you need this cast?

558

This better be moved closer to the location of using

// Emitting note header.
int WordSize = TT.isArch64Bit() ? 8 : 4;
EmitAlignment(WordSize == 4 ? 2 : 3);

llvm/test/CodeGen/X86/note-cet-property.ll

2

Can you add a check for the 32-bit target too? (Since you implement both in patch).

[mike.dvoretsky]

Changes made per comments.

llvm/lib/Target/X86/X86AsmPrinter.cpp

556

Unfortunately, yes. The compilation fails without it.

[grimar]

llvm/lib/Target/X86/X86AsmPrinter.cpp

556

I think you just need to add
#include "llvm/MC/MCSectionELF.h"
to resolve the failure.

[grimar]

llvm/lib/Target/X86/X86AsmPrinter.cpp

547

This if looks not checked by the test case?

[mike.dvoretsky]

Added an extra flag to the test, so that both flag-reading paths are covered. Since flags are module-wide, if there need to be tests covering them separately they would have to be put in new files.

[grimar]

LGTM.

llvm/include/llvm/BinaryFormat/ELF.h

enum {
   NT_GNU_GOLD_VERSION = 4,
   NT_GNU_PROPERTY_TYPE_0 = 5,
 };
 
 // Property types used in GNU_PROPERTY_TYPE_0 notes.
 enum {
   GNU_PROPERTY_STACK_SIZE = 1,
   GNU_PROPERTY_NO_COPY_ON_PROTECTED = 2,
+  GNU_PROPERTY_X86_FEATURE_1_AND = 0xc0000002
+};
+
+// CET properties
+enum {
+  GNU_PROPERTY_X86_FEATURE_1_IBT = 1 << 0,
+  GNU_PROPERTY_X86_FEATURE_1_SHSTK = 1 << 1
 };
 
 // AMDGPU specific notes.
 enum {
   // Note types with values between 0 and 9 (inclusive) are reserved.
   NT_AMD_AMDGPU_HSA_METADATA = 10,
   NT_AMD_AMDGPU_ISA = 11,
   NT_AMD_AMDGPU_PAL_METADATA = 12

llvm/lib/Target/X86/X86AsmPrinter.cpp

 
 #include "X86AsmPrinter.h"
 #include "InstPrinter/X86ATTInstPrinter.h"
 #include "MCTargetDesc/X86BaseInfo.h"
 #include "MCTargetDesc/X86TargetStreamer.h"
 #include "X86InstrInfo.h"
 #include "X86MachineFunctionInfo.h"
 #include "llvm/BinaryFormat/COFF.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/CodeGen/MachineConstantPool.h"
 #include "llvm/CodeGen/MachineModuleInfoImpls.h"
 #include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/MC/MCCodeEmitter.h"
bool X86AsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
   }
   printMemReference(*this, MI, OpNo, O);
   return false;
 }
 
 void X86AsmPrinter::EmitStartOfAsmFile(Module &M) {
   const Triple &TT = TM.getTargetTriple();
 
+  if (TT.isOSBinFormatELF()) {
+    // Assemble feature flags that may require creation of a note section.
+    char FeatureFlagsAnd = 0;
+    if (M.getModuleFlag("cf-protection-branch"))
+      FeatureFlagsAnd |= ELF::GNU_PROPERTY_X86_FEATURE_1_IBT;
+    if (M.getModuleFlag("cf-protection-return"))
+      FeatureFlagsAnd |= ELF::GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+
+    if (FeatureFlagsAnd) {
+      // Emit a .note.gnu.property section with the flags.
+      MCSection *Cur = OutStreamer->getCurrentSectionOnly();
+      MCSection *Nt = (MCSection *)MMI->getContext().getELFSection(
+          ".note.gnu.property", ELF::SHT_NOTE, ELF::SHF_ALLOC);
+      int WordSize = TT.isArch64Bit() ? 8 : 4;
+      assert((TT.isArch32Bit() || WordSize == 8) &&
+             "CFProtection used on invalid architecture");
+      OutStreamer->SwitchSection(Nt);
+
+      // Emitting note header.
+      EmitAlignment(WordSize == 4 ? 2 : 3);
+      OutStreamer->EmitIntValue(4, 4 /*size*/); // data size for "GNU\0"
+      OutStreamer->EmitIntValue(8 + WordSize, 4 /*size*/); // Elf_Prop size
+      OutStreamer->EmitIntValue(ELF::NT_GNU_PROPERTY_TYPE_0, 4 /*size*/);
+      OutStreamer->EmitIntValue('G', 1 /*size*/); // note name "GNU"
+      OutStreamer->EmitIntValue('N', 1 /*size*/);
+      OutStreamer->EmitIntValue('U', 1 /*size*/);
+      OutStreamer->EmitIntValue(0, 1 /*size*/);
+
+      // Emitting an Elf_Prop for the CET properties.
+      OutStreamer->EmitIntValue(ELF::GNU_PROPERTY_X86_FEATURE_1_AND, 4);
+      OutStreamer->EmitIntValue(WordSize, 4);               // data size
+      OutStreamer->EmitIntValue(FeatureFlagsAnd, WordSize); // data
+      EmitAlignment(WordSize == 4 ? 2 : 3);                 // padding
+
+      OutStreamer->endSection(Nt);
+      OutStreamer->SwitchSection(Cur);
+    }
+  }
+
   if (TT.isOSBinFormatMachO())
     OutStreamer->SwitchSection(getObjFileLowering().getTextSection());
 
   if (TT.isOSBinFormatCOFF()) {
     // Emit an absolute @feat.00 symbol.  This appears to be some kind of
     // compiler features bitfield read by link.exe.
     if (TT.getArch() == Triple::x86) {
       MCSymbol *S = MMI->getContext().getOrCreateSymbol(StringRef("@feat.00"));

llvm/test/CodeGen/X86/note-cet-property.ll

+; RUN: llc -mtriple x86_64-pc-linux < %s | FileCheck %s
+
+; This test checks that the compiler emits a .note.gnu.property section for
+; modules with "cf-protection" module flags.
+
+; CHECK:      .section        .note.gnu.property,"a",@note
+; CHECK-NEXT: .p2align 3
+; CHECK-NEXT: .long   4
+; CHECK-NEXT: .long   16
+; CHECK-NEXT: .long   5
+; CHECK-NEXT: .byte   71
+; CHECK-NEXT: .byte   78
+; CHECK-NEXT: .byte   85
+; CHECK-NEXT: .byte   0
+; CHECK-NEXT: .long   3221225474
+; CHECK-NEXT: .long   8
+; CHECK-NEXT: .quad   2
+; CHECK-NEXT: .p2align 3
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"cf-protection-return", i32 1}

llvm/test/tools/llvm-readobj/note-gnu-property.s

 // REQUIRES: x86-registered-target
 // RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t
 // RUN: llvm-readobj -elf-output-style GNU --notes %t | FileCheck %s
 
-// CHECK:      Displaying notes found at file offset 0x00000040 with length 0x00000070:
+// CHECK:      Displaying notes found at file offset 0x00000040 with length 0x000000b8:
 // CHECK-NEXT:   Owner                 Data size       Description
-// CHECK-NEXT:   GNU                   0x00000060      NT_GNU_PROPERTY_TYPE_0 (property note)
+// CHECK-NEXT:   GNU                   0x000000a8      NT_GNU_PROPERTY_TYPE_0 (property note)
 // CHECK-NEXT:     Properties:  stack size: 0x100
 // CHECK-NEXT:     stack size: 0x100
 // CHECK-NEXT:     no copy on protected
+// CHECK-NEXT:     X86 features: SHSTK
+// CHECK-NEXT:     X86 features: IBT, SHSTK
+// CHECK-NEXT:     X86 features: none
 // CHECK-NEXT:     <application-specific type 0xfefefefe>
 // CHECK-NEXT:     stack size: <corrupt length: 0x0>
 // CHECK-NEXT:     stack size: <corrupt length: 0x4> 
 // CHECK-NEXT:     no copy on protected <corrupt length: 0x1>
+// CHECK-NEXT:     X86 features: <corrupt length: 0x0>
+// CHECK-NEXT:     X86 features: IBT, <unknown flags: 0xf000f000f000f000>
 // CHECK-NEXT:     <corrupt type (0x2) datasz: 0x1>
 
 .section ".note.gnu.property", "a"
 .align 4 
   .long 4           /* Name length is always 4 ("GNU") */
   .long end - begin /* Data length */
   .long 5           /* Type: NT_GNU_PROPERTY_TYPE_0 */
   .asciz "GNU"      /* Name */
   .p2align 3
 begin:
   .long 1           /* Type: GNU_PROPERTY_STACK_SIZE */
   .long 8           /* Data size */
   .quad 0x100       /* Data (stack size) */
   .p2align 3        /* Align to 8 byte for 64 bit */
-  
   /* Test we handle alignment properly */
   .long 1           /* Type: GNU_PROPERTY_STACK_SIZE */
   .long 8           /* Data size */
   .long 0x100       /* Data (stack size) */
   .p2align 3        /* Align to 8 byte for 64 bit */
   
   .long 2           /* Type: GNU_PROPERTY_NO_COPY_ON_PROTECTED */
   .long 0           /* Data size */
   .p2align 3        /* Align to 8 byte for 64 bit */
-  
+  /* CET property note */
+  .long 0xc0000002  /* Type: GNU_PROPERTY_X86_FEATURE_1_AND */
+  .long 8           /* Data size */
+  .quad 2           /* GNU_PROPERTY_X86_FEATURE_1_SHSTK */
+  .p2align 3        /* Align to 8 byte for 64 bit */
+
+  /* CET property note with padding */
+  .long 0xc0000002  /* Type: GNU_PROPERTY_X86_FEATURE_1_AND */
+  .long 4           /* Data size */
+  .long 3           /* Full CET support */
+  .p2align 3        /* Align to 8 byte for 64 bit */
+
+  .long 0xc0000002  /* Type: GNU_PROPERTY_X86_FEATURE_1_AND */
+  .long 8           /* Data size */
+  .quad 0           /* Empty flags, not an error */
+  .p2align 3        /* Align to 8 byte for 64 bit */
+  
   /* All notes below are broken. Test we are able to report them. */
   
   /* Broken note type */
   .long 0xfefefefe  /* Invalid type for testing */
   .long 0           /* Data size */
   .p2align 3        /* Align to 8 byte for 64 bit */
   
   /* GNU_PROPERTY_STACK_SIZE with zero stack size */
   .long 1           /* Type: GNU_PROPERTY_STACK_SIZE */
   .long 0           /* Data size */
   .p2align 3        /* Align to 8 byte for 64 bit */
   
   /* GNU_PROPERTY_STACK_SIZE with data size 4 (should be 8) */
   .long 1           /* Type: GNU_PROPERTY_STACK_SIZE */
   .long 4           /* Data size */
   .long 0x100       /* Data (stack size) */
   .p2align 3        /* Align to 8 byte for 64 bit */
   
   /* GNU_PROPERTY_NO_COPY_ON_PROTECTED with pr_datasz and some data */
   .long 2           /* Type: GNU_PROPERTY_NO_COPY_ON_PROTECTED */
   .long 1           /* Data size (corrupted) */
   .byte 1           /* Data */
   .p2align 3        /* Align to 8 byte for 64 bit */
-  
+  /* CET note with size zero */
+  .long 0xc0000002  /* Type: GNU_PROPERTY_X86_FEATURE_1_AND */
+  .long 0           /* Data size */
+  .p2align 3        /* Align to 8 byte for 64 bit */
+
+  /* CET note with bad flags */
+  .long 0xc0000002         /* Type: GNU_PROPERTY_X86_FEATURE_1_AND */
+  .long 8                  /* Data size */
+  .quad 0xf000f000f000f001 /* GNU_PROPERTY_X86_FEATURE_1_IBT and bad bits */
+  .p2align 3               /* Align to 8 byte for 64 bit */
+  
   /* GNU_PROPERTY_NO_COPY_ON_PROTECTED with pr_datasz and without data */
   .long 2           /* Type: GNU_PROPERTY_NO_COPY_ON_PROTECTED */
   .long 1           /* Data size (corrupted) */
   .p2align 3        /* Align to 8 byte for 64 bit */
 end:

llvm/tools/llvm-readobj/ELFDumper.cpp

static void printGNUProperty(raw_ostream &OS, uint32_t Type, uint32_t DataSize,
                              ArrayRef<uint8_t> Data) {
   switch (Type) {
   default:
     OS << format("    <application-specific type 0x%x>\n", Type);
     return;
   case GNU_PROPERTY_STACK_SIZE: {
     OS << "    stack size: ";
     if (DataSize == sizeof(typename ELFT::uint))
-      OS << format("0x%x\n",
+      OS << format("0x%lx\n",
                    (uint64_t)(*(const typename ELFT::Addr *)Data.data()));
     else
       OS << format("<corrupt length: 0x%x>\n", DataSize);
     break;
   }
   case GNU_PROPERTY_NO_COPY_ON_PROTECTED:
     OS << "    no copy on protected";
     if (DataSize)
       OS << format(" <corrupt length: 0x%x>", DataSize);
     OS << "\n";
     break;
+  case GNU_PROPERTY_X86_FEATURE_1_AND:
+    OS << "    X86 features: ";
+    if (DataSize != 4 && DataSize != 8) 
+      OS << format("<corrupt length: 0x%x>", DataSize);
+    else {
+      uint64_t CFProtection =
+          (DataSize == 4)
+              ? support::endian::read32<ELFT::TargetEndianness>(Data.data())
+              : support::endian::read64<ELFT::TargetEndianness>(Data.data());
+      if (CFProtection == 0) {
+        OS << "none\n";
+        break;
+      }
+      if (CFProtection & GNU_PROPERTY_X86_FEATURE_1_IBT) {
+        OS << "IBT";
+        CFProtection &= ~GNU_PROPERTY_X86_FEATURE_1_IBT;
+        if (CFProtection)
+          OS << ", ";
+      }
+      if (CFProtection & GNU_PROPERTY_X86_FEATURE_1_SHSTK) {
+        OS << "SHSTK";
+        CFProtection &= ~GNU_PROPERTY_X86_FEATURE_1_SHSTK;
+        if (CFProtection)
+          OS << ", ";
+      }
+      if (CFProtection)
+        OS << format("<unknown flags: 0x%lx>", CFProtection);
+    }
+    OS << "\n";
+    break;
   }
 }
 
 template <typename ELFT>
 static void printGNUNote(raw_ostream &OS, uint32_t NoteType,
                          ArrayRef<typename ELFT::Word> Words, size_t Size) {
   using Elf_Word = typename ELFT::Word;
 
   switch (NoteType) {
   default:
     return;