[compiler-rt] 3d53b52 - [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args
Marco Elver via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 8 04:12:51 PST 2023
Author: Marco Elver
Date: 2023-02-08T13:12:33+01:00
New Revision: 3d53b52730034b9ec78871b8dcde8287dd811016
URL: https://github.com/llvm/llvm-project/commit/3d53b52730034b9ec78871b8dcde8287dd811016
DIFF: https://github.com/llvm/llvm-project/commit/3d53b52730034b9ec78871b8dcde8287dd811016.diff
LOG: [SanitizerBinaryMetadata] Optimize used space for features and UAR stack args
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.
Reviewed By: dvyukov
Differential Revision: https://reviews.llvm.org/D143482
Added:
Modified:
clang/test/CodeGen/sanitize-metadata.c
compiler-rt/test/metadata/common.h
compiler-rt/test/metadata/covered.cpp
compiler-rt/test/metadata/lit.site.cfg.py.in
compiler-rt/test/metadata/uar.cpp
llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll
Removed:
################################################################################
diff --git a/clang/test/CodeGen/sanitize-metadata.c b/clang/test/CodeGen/sanitize-metadata.c
index 3ab0fbc06fe0..881d148b1aff 100644
--- a/clang/test/CodeGen/sanitize-metadata.c
+++ b/clang/test/CodeGen/sanitize-metadata.c
@@ -22,15 +22,15 @@ int atomics() {
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"}
diff --git a/compiler-rt/test/metadata/common.h b/compiler-rt/test/metadata/common.h
index b121e871718e..9a7774f4b2f3 100644
--- a/compiler-rt/test/metadata/common.h
+++ b/compiler-rt/test/metadata/common.h
@@ -1,43 +1,54 @@
#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);
@@ -54,9 +65,6 @@ void __sanitizer_metadata_covered_del(uint32_t version, const char *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);
diff --git a/compiler-rt/test/metadata/covered.cpp b/compiler-rt/test/metadata/covered.cpp
index 609a8c044c0d..92abe4f5e8d8 100644
--- a/compiler-rt/test/metadata/covered.cpp
+++ b/compiler-rt/test/metadata/covered.cpp
@@ -6,11 +6,12 @@
// 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;
}
@@ -69,6 +70,20 @@ int with_atomic_escape(int *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:
@@ -78,7 +93,7 @@ int with_atomic_escape(int *p) {
// CHECK-CAU: ellipsis: features=1
void ellipsis(int *p, ...) {
escape(&p);
- volatile int x;
+ [[maybe_unused]] volatile int x;
x = 0;
}
@@ -100,6 +115,7 @@ int ellipsis_with_atomic(int *p, ...) {
FN(with_const_global); \
FN(with_atomic); \
FN(with_atomic_escape); \
+ FN(with_atomic_escape_lots_of_args); \
FN(ellipsis); \
FN(ellipsis_with_atomic); \
/**/
diff --git a/compiler-rt/test/metadata/lit.site.cfg.py.in b/compiler-rt/test/metadata/lit.site.cfg.py.in
index 7b95b4b0fb73..8993b31d3823 100644
--- a/compiler-rt/test/metadata/lit.site.cfg.py.in
+++ b/compiler-rt/test/metadata/lit.site.cfg.py.in
@@ -11,4 +11,5 @@ lit_config.load_config(config, "@COMPILER_RT_BINARY_DIR@/test/lit.common.configu
# 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 + [""])))
diff --git a/compiler-rt/test/metadata/uar.cpp b/compiler-rt/test/metadata/uar.cpp
index ed7f7dd8abe6..cbafe462c364 100644
--- a/compiler-rt/test/metadata/uar.cpp
+++ b/compiler-rt/test/metadata/uar.cpp
@@ -1,10 +1,11 @@
// 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;
}
@@ -44,20 +45,20 @@ void no_stack_args(long a0, long a1, long a2, long a3, long a4, long a5) {
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];
};
diff --git a/llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h b/llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
index beb67d51d55e..c41d5e3e58ef 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h
@@ -28,11 +28,14 @@ struct SanitizerBinaryMetadataOptions {
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";
diff --git a/llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp b/llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
index dd70a2f23e45..2107e00ee2e5 100644
--- a/llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
+++ b/llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp
@@ -57,7 +57,8 @@ bool MachineSanitizerBinaryMetadata::runOnMachineFunction(MachineFunction &MF) {
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.
@@ -69,12 +70,18 @@ bool MachineSanitizerBinaryMetadata::runOnMachineFunction(MachineFunction &MF) {
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;
}
diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
index e990ea204d42..0af5bce9574b 100644
--- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
+++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp
@@ -43,6 +43,7 @@
#include <array>
#include <cstdint>
+#include <limits>
using namespace llvm;
@@ -52,7 +53,7 @@ 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;
@@ -269,9 +270,10 @@ void SanitizerBinaryMetadata::runOn(Function &F, MetadataInfoSet &MIS) {
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}}}));
}
diff --git a/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll b/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll
index 984950e42b32..cef890f954a4 100644
--- a/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll
+++ b/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll
@@ -2039,7 +2039,7 @@ entry:
; 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
@@ -2048,7 +2048,7 @@ entry:
; 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
@@ -2057,7 +2057,7 @@ entry:
; 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
@@ -2066,11 +2066,11 @@ entry:
; 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"}
More information about the llvm-commits
mailing list