[SanitizerBinaryMetadata] Optimize used space for features and UAR stack args

[melver]

Summary

Optimize the encoding of "covered" metadata by:

1. Reducing feature mask from 4 bytes to 1 byte (needs increase once we reach more than 8 features).
2. Only emitting UAR stack args size if it is non-zero, saving 4 bytes in the common case.

One caveat is that the emitted metadata for function PC (offset), size,
and UAR size (if enabled) are no longer aligned to 4 bytes.

SanitizerBinaryMetadata version base is increased to 2, since the change
is backwards incompatible.

[dvyukov]

Nice!

clang/test/CodeGen/sanitize-metadata.c

 // ATOMICS-SAME:                                  !pcsections ![[ATOMICS_COVERED:[0-9]+]]
 // ATOMICS-NEXT:  entry:
 // ATOMICS-NEXT:    atomicrmw add {{.*}} !pcsections ![[ATOMIC_OP:[0-9]+]]
 // ATOMICS-NOT:     load {{.*}} !pcsections
   __atomic_fetch_add(&x, 1, __ATOMIC_RELAXED);
   return y;
 }
 // ATOMICS-LABEL: __sanitizer_metadata_atomics.module_ctor
-// ATOMICS: call void @__sanitizer_metadata_atomics_add(i32 1, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
+// ATOMICS: call void @__sanitizer_metadata_atomics_add(i32 2, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
 // ATOMICS-LABEL: __sanitizer_metadata_atomics.module_dtor
-// ATOMICS: call void @__sanitizer_metadata_atomics_del(i32 1, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
+// ATOMICS: call void @__sanitizer_metadata_atomics_del(i32 2, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
 
 // CHECK-LABEL: __sanitizer_metadata_covered.module_ctor
-// CHECK: call void @__sanitizer_metadata_covered_add(i32 1, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
+// CHECK: call void @__sanitizer_metadata_covered_add(i32 2, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
 // CHECK-LABEL: __sanitizer_metadata_covered.module_dtor
-// CHECK: call void @__sanitizer_metadata_covered_del(i32 1, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
+// CHECK: call void @__sanitizer_metadata_covered_del(i32 2, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
 
 // ATOMICS: ![[ATOMICS_COVERED]] = !{!"sanmd_covered", ![[ATOMICS_COVERED_AUX:[0-9]+]]}
-// ATOMICS: ![[ATOMICS_COVERED_AUX]] = !{i32 1}
+// ATOMICS: ![[ATOMICS_COVERED_AUX]] = !{i8 1}
 // ATOMICS: ![[ATOMIC_OP]] = !{!"sanmd_atomics"}

compiler-rt/test/metadata/common.h

 #include <assert.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <string.h>
 
 int main() { printf("main\n"); }
 
-typedef unsigned long uptr;
-
+namespace {
 #define FN(X)                                                                  \
-  if (pc == reinterpret_cast<uptr>(X))                                         \
+  if (pc == reinterpret_cast<uintptr_t>(X))                                    \
   return #X
 
-const char *symbolize(uptr pc) {
+const char *symbolize(uintptr_t pc) {
   FUNCTIONS;
   return nullptr;
 }
 
 template <typename T> T consume(const char *&pos, const char *end) {
-  T v = *reinterpret_cast<const T *>(pos);
+  T v;
+  // We need to memcpy from pos, because it's not guaranteed that every entry
+  // has the required alignment of T.
+  memcpy(&v, pos, sizeof(T));
   pos += sizeof(T);
   assert(pos <= end);
   return v;
 }
 
+constexpr uint32_t kSanitizerBinaryMetadataUARHasSize = 1 << 2;
+
 uint32_t meta_version;
 const char *meta_start;
 const char *meta_end;
+const char *atomics_start;
+const char *atomics_end;
+}; // namespace
 
 extern "C" {
 void __sanitizer_metadata_covered_add(uint32_t version, const char *start,
                                       const char *end) {
-  printf("metadata add version %u\n", version);
+  const uint32_t version_base = version & 0xffff;
+  const bool offset_ptr_sized = version & (1 << 16);
+  assert(version_base == 2);
+  printf("metadata add version %u\n", version_base);
   for (const char *pos = start; pos < end;) {
-    const uptr base = reinterpret_cast<uptr>(pos);
-    const long offset = (version & (1 << 16)) ? consume<long>(pos, end)
-                                              : consume<int>(pos, end);
-    const uint32_t size = consume<uint32_t>(pos, end);
-    const uint32_t features = consume<uint32_t>(pos, end);
+    const auto base = reinterpret_cast<uintptr_t>(pos);
+    const intptr_t offset = offset_ptr_sized ? consume<intptr_t>(pos, end)
+                                             : consume<int32_t>(pos, end);
+    [[maybe_unused]] const uint32_t size = consume<uint32_t>(pos, end);
+    const uint32_t features = consume<uint8_t>(pos, end);
     uint32_t stack_args = 0;
-    if (features & (1 << 1))
+    if (features & kSanitizerBinaryMetadataUARHasSize)
       stack_args = consume<uint32_t>(pos, end);
     if (const char *name = symbolize(base + offset))
       printf("%s: features=%x stack_args=%u\n", name, features, stack_args);
   }
   meta_version = version;
   meta_start = start;
   meta_end = end;
 }
 
 void __sanitizer_metadata_covered_del(uint32_t version, const char *start,
                                       const char *end) {
   assert(version == meta_version);
   assert(start == meta_start);
   assert(end == meta_end);
 }
 
-const char *atomics_start;
-const char *atomics_end;
-
 void __sanitizer_metadata_atomics_add(uint32_t version, const char *start,
                                       const char *end) {
   assert(version == meta_version);
   assert(start);
   assert(end >= end);
   atomics_start = start;
   atomics_end = end;
 }
 
 void __sanitizer_metadata_atomics_del(uint32_t version, const char *start,
                                       const char *end) {
   assert(version == meta_version);
   assert(atomics_start == start);
   assert(atomics_end == end);
 }
 }

compiler-rt/test/metadata/covered.cpp

 // RUN: %clangxx %s -o %t && %t | FileCheck %s
 // RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=covered && %t | FileCheck -check-prefix=CHECK-C %s
 // RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=atomics && %t | FileCheck -check-prefix=CHECK-A %s
 // RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=uar && %t | FileCheck -check-prefix=CHECK-U %s
 // RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=covered,atomics && %t | FileCheck -check-prefix=CHECK-CA %s
 // RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck -check-prefix=CHECK-CU %s
 // RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=atomics,uar && %t | FileCheck -check-prefix=CHECK-AU %s
 // RUN: %clangxx %s -o %t -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
+// RUN: %clangxx %s -o %t -mcmodel=large -fexperimental-sanitize-metadata=covered,atomics,uar && %t | FileCheck -check-prefix=CHECK-CAU %s
 
 const int const_global = 42;
 
 __attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {
-  static const volatile void *sink;
+  [[maybe_unused]] static const volatile void *sink;
   sink = p;
 }
 
 // CHECK-NOT: metadata add
 // CHECK: main
 // CHECK-NOT: metadata del
 
 // CHECK-C:      empty: features=0

 // CHECK-CU:  with_atomic_escape: features=2
 // CHECK-AU:  with_atomic_escape: features=3
 // CHECK-CAU: with_atomic_escape: features=3
 int with_atomic_escape(int *p) {
   escape(&p);
   return __atomic_load_n(p, __ATOMIC_RELAXED);
 }
 
+// CHECK-C:   with_atomic_escape_lots_of_args: features=0
+// CHECK-A:   with_atomic_escape_lots_of_args: features=1
+// CHECK-U:   with_atomic_escape_lots_of_args: features=6
+// CHECK-CA:  with_atomic_escape_lots_of_args: features=1
+// CHECK-CU:  with_atomic_escape_lots_of_args: features=6
+// CHECK-AU:  with_atomic_escape_lots_of_args: features=7
+// CHECK-CAU: with_atomic_escape_lots_of_args: features=7
+long with_atomic_escape_lots_of_args(int *p, long a0, long a1, long a2, long a3,
+                                     long a4, long a5, long a6) {
+  escape(&p);
+  return a0 + a1 + a2 + a3 + a4 + a5 + a6 +
+         __atomic_load_n(p, __ATOMIC_RELAXED);
+}
+
 // CHECK-C:     ellipsis: features=0
 // CHECK-A:     ellipsis: features=1
 // CHECK-U-NOT: ellipsis:
 // CHECK-CA:    ellipsis: features=1
 // CHECK-CU:    ellipsis: features=0
 // CHECK-AU:    ellipsis: features=1
 // CHECK-CAU:   ellipsis: features=1
 void ellipsis(int *p, ...) {
   escape(&p);
-  volatile int x;
+  [[maybe_unused]] volatile int x;
   x = 0;
 }
 
 // CHECK-C:     ellipsis_with_atomic: features=0
 // CHECK-A:     ellipsis_with_atomic: features=1
 // CHECK-U-NOT: ellipsis_with_atomic:
 // CHECK-CA:    ellipsis_with_atomic: features=1
 // CHECK-CU:    ellipsis_with_atomic: features=0
 // CHECK-AU:    ellipsis_with_atomic: features=1
 // CHECK-CAU:   ellipsis_with_atomic: features=1
 int ellipsis_with_atomic(int *p, ...) {
   escape(&p);
   return __atomic_load_n(p, __ATOMIC_RELAXED);
 }
 
 #define FUNCTIONS                                                              \
   FN(empty);                                                                   \
   FN(normal);                                                                  \
   FN(with_const_global);                                                       \
   FN(with_atomic);                                                             \
   FN(with_atomic_escape);                                                      \
+  FN(with_atomic_escape_lots_of_args);                                         \
   FN(ellipsis);                                                                \
   FN(ellipsis_with_atomic);                                                    \
   /**/
 
 #include "common.h"

compiler-rt/test/metadata/lit.site.cfg.py.in

 @LIT_SITE_CFG_IN_HEADER@
 
 # Tool-specific config options.
 config.clang = "@METADATA_TEST_TARGET_CC@"
 config.target_cflags = "@METADATA_TEST_TARGET_CFLAGS@"
 config.target_arch = "x86_64"
 
 # Load common config for all compiler-rt lit tests.
 lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configured")
 
 # Load tool-specific config that would do the real work.
 lit_config.load_config(config, "@METADATA_LIT_SOURCE_DIR@/lit.cfg.py")
 
-config.substitutions.append(("%clangxx ", " " + config.clang + " " + config.target_cflags + " "))
+clangxx = [config.clang] + config.cxx_mode_flags + ["-Wall", config.target_cflags]
+config.substitutions.append(("%clangxx ", " ".join([""] + clangxx + [""])))

compiler-rt/test/metadata/uar.cpp

 // RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar && %t | FileCheck %s
-// RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow && %t | FileCheck %s
+// RUN: %clangxx %s -O1 -o %t -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow,alignment && %t | FileCheck %s
+// RUN: %clangxx %s -O1 -o %t -mcmodel=large -fexperimental-sanitize-metadata=covered,uar -fsanitize=address,signed-integer-overflow,alignment && %t | FileCheck %s
 
-// CHECK: metadata add version 1
+// CHECK: metadata add version 2
 
 __attribute__((noinline, not_tail_called)) void escape(const volatile void *p) {
-  static const volatile void *sink;
+  [[maybe_unused]] static const volatile void *sink;
   sink = p;
 }
 
 __attribute__((noinline, not_tail_called)) void use(int x) {
   static volatile int sink;
   sink += x;
 }
 

 }
 
 // CHECK: no_stack_args: features=2 stack_args=0
 void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
   int x;
   escape(&x);
 }
 
-// CHECK: stack_args: features=2 stack_args=16
+// CHECK: stack_args: features=6 stack_args=16
 void stack_args(long a0, long a1, long a2, long a3, long a4, long a5, long a6) {
   int x;
   escape(&x);
 }
 
-// CHECK: more_stack_args: features=2 stack_args=32
+// CHECK: more_stack_args: features=6 stack_args=32
 void more_stack_args(long a0, long a1, long a2, long a3, long a4, long a5,
                      long a6, long a7, long a8) {
   int x;
   escape(&x);
 }
 
-// CHECK: struct_stack_args: features=2 stack_args=144
+// CHECK: struct_stack_args: features=6 stack_args=144
 struct large {
   char x[131];
 };
 void struct_stack_args(large a) {
   int x;
   escape(&x);
 }
 

llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h

struct SanitizerBinaryMetadataOptions {
   bool Covered = false;
   bool Atomics = false;
   bool UAR = false;
   SanitizerBinaryMetadataOptions() = default;
 };
 
 inline constexpr int kSanitizerBinaryMetadataAtomicsBit = 0;
 inline constexpr int kSanitizerBinaryMetadataUARBit = 1;
+inline constexpr int kSanitizerBinaryMetadataUARHasSizeBit = 2;
 
 inline constexpr uint32_t kSanitizerBinaryMetadataAtomics =
     1 << kSanitizerBinaryMetadataAtomicsBit;
 inline constexpr uint32_t kSanitizerBinaryMetadataUAR =
     1 << kSanitizerBinaryMetadataUARBit;
+inline constexpr uint32_t kSanitizerBinaryMetadataUARHasSize =
+    1 << kSanitizerBinaryMetadataUARHasSizeBit;
 
 inline constexpr char kSanitizerBinaryMetadataCoveredSection[] =
     "sanmd_covered";
 inline constexpr char kSanitizerBinaryMetadataAtomicsSection[] =
     "sanmd_atomics";
 
 /// Public interface to the SanitizerBinaryMetadata module pass for emitting
 /// metadata for binary analysis sanitizers.

llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp

bool MachineSanitizerBinaryMetadata::runOnMachineFunction(MachineFunction &MF) {
   if (!MD)
     return false;
   const auto &Section = *cast<MDString>(MD->getOperand(0));
   if (!Section.getString().equals(kSanitizerBinaryMetadataCoveredSection))
     return false;
   auto &AuxMDs = *cast<MDTuple>(MD->getOperand(1));
   // Assume it currently only has features.
   assert(AuxMDs.getNumOperands() == 1);
-  auto *Features = cast<ConstantAsMetadata>(AuxMDs.getOperand(0))->getValue();
+  Constant *Features =
+      cast<ConstantAsMetadata>(AuxMDs.getOperand(0))->getValue();
   if (!Features->getUniqueInteger()[kSanitizerBinaryMetadataUARBit])
     return false;
   // Calculate size of stack args for the function.
   int64_t Size = 0;
   uint64_t Align = 0;
   const MachineFrameInfo &MFI = MF.getFrameInfo();
   for (int i = -1; i >= (int)-MFI.getNumFixedObjects(); --i) {
     Size = std::max(Size, MFI.getObjectOffset(i) + MFI.getObjectSize(i));
     Align = std::max(Align, MFI.getObjectAlign(i).value());
   }
   Size = (Size + Align - 1) & ~(Align - 1);
+  if (!Size)
+    return false;
+  // Non-zero size, update metadata.
   auto &F = MF.getFunction();
   IRBuilder<> IRB(F.getContext());
   MDBuilder MDB(F.getContext());
   // Keep the features and append size of stack args to the metadata.
-  F.setMetadata(LLVMContext::MD_pcsections,
-                MDB.createPCSections(
-                    {{Section.getString(), {Features, IRB.getInt32(Size)}}}));
+  APInt NewFeatures = Features->getUniqueInteger();
+  NewFeatures.setBit(kSanitizerBinaryMetadataUARHasSizeBit);
+  F.setMetadata(
+      LLVMContext::MD_pcsections,
+      MDB.createPCSections({{Section.getString(),
+                             {IRB.getInt(NewFeatures), IRB.getInt32(Size)}}}));
   return false;
 }

llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/TargetParser/Triple.h"
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 
 #include <array>
 #include <cstdint>
+#include <limits>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "sanmd"
 
 namespace {
 
 //===--- Constants --------------------------------------------------------===//
 
-constexpr uint32_t kVersionBase = 1;                // occupies lower 16 bits
+constexpr uint32_t kVersionBase = 2;                // occupies lower 16 bits
 constexpr uint32_t kVersionPtrSizeRel = (1u << 16); // offsets are pointer-sized
 constexpr int kCtorDtorPriority = 2;
 
 // Pairs of names of initialization callback functions and which section
 // contains the relevant metadata.
 class MetadataInfo {
 public:
   const StringRef FunctionPrefix;
void SanitizerBinaryMetadata::runOn(Function &F, MetadataInfoSet &MIS) {
   // Covered metadata is always emitted if explicitly requested, otherwise only
   // if some other metadata requires it to unambiguously interpret it for
   // modules compiled with SanitizerBinaryMetadata.
   if (Options.Covered || (FeatureMask && RequiresCovered)) {
     NumMetadataCovered++;
     const auto *MI = &MetadataInfo::Covered;
     MIS.insert(MI);
     const StringRef Section = getSectionName(MI->SectionSuffix);
-    // The feature mask will be placed after the size (32 bit) of the function,
-    // so in total one covered entry will use `sizeof(void*) + 4 + 4`.
-    Constant *CFM = IRB.getInt32(FeatureMask);
+    // The feature mask will be placed after the size of the function.
+    assert(FeatureMask <= std::numeric_limits<uint8_t>::max() &&
+           "Increase feature mask bytes and bump version");
+    Constant *CFM = IRB.getInt8(FeatureMask);
     F.setMetadata(LLVMContext::MD_pcsections,
                   MDB.createPCSections({{Section, {CFM}}}));
   }
 }
 
 bool isUARSafeCall(CallInst *CI) {
   auto *F = CI->getCalledFunction();
   // There are no intrinsic functions that leak arguments.

llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll

 ; CHECK: !pcsections !2
 
 ; Check that callbacks are emitted.
 
 ; CHECK-LABEL: __sanitizer_metadata_atomics.module_ctor
 ; CHECK-DAG: entry:
 ; CHECK-NEXT:  br i1 icmp ne (ptr @__sanitizer_metadata_atomics_add, ptr null), label %callfunc, label %ret
 ; CHECK-DAG: callfunc:
-; CHECK-NEXT:  call void @__sanitizer_metadata_atomics_add(i32 1, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
+; CHECK-NEXT:  call void @__sanitizer_metadata_atomics_add(i32 2, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
 ; CHECK-NEXT:  br label %ret
 ; CHECK-DAG: ret:
 ; CHECK-NEXT:  ret void
 
 ; CHECK-LABEL: __sanitizer_metadata_atomics.module_dtor
 ; CHECK-DAG: entry:
 ; CHECK-NEXT:  br i1 icmp ne (ptr @__sanitizer_metadata_atomics_del, ptr null), label %callfunc, label %ret
 ; CHECK-DAG: callfunc:
-; CHECK-NEXT:  call void @__sanitizer_metadata_atomics_del(i32 1, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
+; CHECK-NEXT:  call void @__sanitizer_metadata_atomics_del(i32 2, ptr @__start_sanmd_atomics, ptr @__stop_sanmd_atomics)
 ; CHECK-NEXT:  br label %ret
 ; CHECK-DAG: ret:
 ; CHECK-NEXT:  ret void
 
 ; CHECK-LABEL: __sanitizer_metadata_covered.module_ctor
 ; CHECK-DAG: entry:
 ; CHECK-NEXT:  br i1 icmp ne (ptr @__sanitizer_metadata_covered_add, ptr null), label %callfunc, label %ret
 ; CHECK-DAG: callfunc:
-; CHECK-NEXT:  call void @__sanitizer_metadata_covered_add(i32 1, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
+; CHECK-NEXT:  call void @__sanitizer_metadata_covered_add(i32 2, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
 ; CHECK-NEXT:  br label %ret
 ; CHECK-DAG: ret:
 ; CHECK-NEXT:  ret void
 
 ; CHECK-LABEL: __sanitizer_metadata_covered.module_dtor
 ; CHECK-DAG: entry:
 ; CHECK-NEXT:  br i1 icmp ne (ptr @__sanitizer_metadata_covered_del, ptr null), label %callfunc, label %ret
 ; CHECK-DAG: callfunc:
-; CHECK-NEXT:  call void @__sanitizer_metadata_covered_del(i32 1, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
+; CHECK-NEXT:  call void @__sanitizer_metadata_covered_del(i32 2, ptr @__start_sanmd_covered, ptr @__stop_sanmd_covered)
 ; CHECK-NEXT:  br label %ret
 ; CHECK-DAG: ret:
 ; CHECK-NEXT:  ret void
 
 ; CHECK: !0 = !{!"sanmd_covered", !1}
-; CHECK: !1 = !{i32 1}
+; CHECK: !1 = !{i8 1}
 ; CHECK: !2 = !{!"sanmd_atomics"}