diff --git a/clang/test/CodeGen/sanitize-metadata.c b/clang/test/CodeGen/sanitize-metadata.c --- a/clang/test/CodeGen/sanitize-metadata.c +++ b/clang/test/CodeGen/sanitize-metadata.c @@ -22,15 +22,15 @@ 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 --- a/compiler-rt/test/metadata/common.h +++ b/compiler-rt/test/metadata/common.h @@ -1,43 +1,54 @@ #include #include #include +#include int main() { printf("main\n"); } -typedef unsigned long uptr; - +namespace { #define FN(X) \ - if (pc == reinterpret_cast(X)) \ + if (pc == reinterpret_cast(X)) \ return #X -const char *symbolize(uptr pc) { +const char *symbolize(uintptr_t pc) { FUNCTIONS; return nullptr; } template T consume(const char *&pos, const char *end) { - T v = *reinterpret_cast(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(pos); - const long offset = (version & (1 << 16)) ? consume(pos, end) - : consume(pos, end); - const uint32_t size = consume(pos, end); - const uint32_t features = consume(pos, end); + const auto base = reinterpret_cast(pos); + const intptr_t offset = offset_ptr_sized ? consume(pos, end) + : consume(pos, end); + [[maybe_unused]] const uint32_t size = consume(pos, end); + const uint32_t features = consume(pos, end); uint32_t stack_args = 0; - if (features & (1 << 1)) + if (features & kSanitizerBinaryMetadataUARHasSize) stack_args = consume(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 @@ 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 --- 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 @@ 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 @@ // 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 @@ 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 --- a/compiler-rt/test/metadata/lit.site.cfg.py.in +++ b/compiler-rt/test/metadata/lit.site.cfg.py.in @@ -11,4 +11,5 @@ # 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 --- 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 @@ 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 --- a/llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h +++ b/llvm/include/llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h @@ -28,11 +28,14 @@ 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 --- a/llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp +++ b/llvm/lib/CodeGen/SanitizerBinaryMetadata.cpp @@ -57,7 +57,8 @@ auto &AuxMDs = *cast(MD->getOperand(1)); // Assume it currently only has features. assert(AuxMDs.getNumOperands() == 1); - auto *Features = cast(AuxMDs.getOperand(0))->getValue(); + Constant *Features = + cast(AuxMDs.getOperand(0))->getValue(); if (!Features->getUniqueInteger()[kSanitizerBinaryMetadataUARBit]) return false; // Calculate size of stack args for the function. @@ -69,12 +70,18 @@ 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 --- a/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp +++ b/llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp @@ -43,6 +43,7 @@ #include #include +#include using namespace llvm; @@ -52,7 +53,7 @@ //===--- 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 @@ 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::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 --- a/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll +++ b/llvm/test/Instrumentation/SanitizerBinaryMetadata/atomics.ll @@ -2039,7 +2039,7 @@ ; 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 @@ ; 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 @@ ; 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 @@ ; 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"}