hctim updated this revision to Diff 444111.
hctim marked an inline comment as done.
hctim added a comment.
Final comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128950/new/
https://reviews.llvm.org/D128950
Files:
clang/lib/CodeGen/SanitizerMetadata.cpp
clang/test/CodeGen/memtag-globals.cpp
clang/test/CodeGen/sanitizer-special-case-list-globals.c
llvm/include/llvm/AsmParser/LLToken.h
llvm/include/llvm/IR/GlobalValue.h
llvm/lib/AsmParser/LLLexer.cpp
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/AsmWriter.cpp
llvm/test/Assembler/globalvariable-attributes.ll
llvm/test/Bitcode/compatibility.ll
Index: llvm/test/Bitcode/compatibility.ll
===================================================================
--- llvm/test/Bitcode/compatibility.ll
+++ llvm/test/Bitcode/compatibility.ll
@@ -206,14 +206,16 @@
; Global Variables -- sanitizers
@g.no_sanitize_address = global i32 0, no_sanitize_address
@g.no_sanitize_hwaddress = global i32 0, no_sanitize_hwaddress
-@g.no_sanitize_memtag = global i32 0, no_sanitize_memtag
-@g.no_sanitize_multiple = global i32 0, no_sanitize_address, no_sanitize_hwaddress, no_sanitize_memtag
+@g.sanitize_memtag = global i32 0, sanitize_memtag
+@g.no_sanitize_multiple = global i32 0, no_sanitize_address, no_sanitize_hwaddress
@g.sanitize_address_dyninit = global i32 0, sanitize_address_dyninit
+@g.sanitize_multiple = global i32 0, sanitize_memtag, sanitize_address_dyninit
; CHECK: @g.no_sanitize_address = global i32 0, no_sanitize_address
; CHECK: @g.no_sanitize_hwaddress = global i32 0, no_sanitize_hwaddress
-; CHECK: @g.no_sanitize_memtag = global i32 0, no_sanitize_memtag
-; CHECK: @g.no_sanitize_multiple = global i32 0, no_sanitize_address, no_sanitize_hwaddress, no_sanitize_memtag
+; CHECK: @g.sanitize_memtag = global i32 0, sanitize_memtag
+; CHECK: @g.no_sanitize_multiple = global i32 0, no_sanitize_address, no_sanitize_hwaddress
; CHECK: @g.sanitize_address_dyninit = global i32 0, sanitize_address_dyninit
+; CHECK: @g.sanitize_multiple = global i32 0, sanitize_memtag, sanitize_address_dyninit
;; Aliases
; Format: @<Name> = [Linkage] [Visibility] [DLLStorageClass] [ThreadLocal]
Index: llvm/test/Assembler/globalvariable-attributes.ll
===================================================================
--- llvm/test/Assembler/globalvariable-attributes.ll
+++ llvm/test/Assembler/globalvariable-attributes.ll
@@ -6,9 +6,9 @@
@g4 = global i32 2, align 4 "key5" = "value5" #0
@g5 = global i32 2, no_sanitize_address, align 4
@g6 = global i32 2, no_sanitize_hwaddress, align 4
-@g7 = global i32 2, no_sanitize_memtag, align 4
-@g8 = global i32 2, sanitize_address_dyninit, align 4
-@g9 = global i32 2, no_sanitize_address, no_sanitize_hwaddress, no_sanitize_memtag, align 4
+@g7 = global i32 2, sanitize_address_dyninit, align 4
+@g8 = global i32 2, sanitize_memtag, align 4
+@g9 = global i32 2, no_sanitize_address, no_sanitize_hwaddress, sanitize_memtag, align 4
attributes #0 = { "string" = "value" nobuiltin norecurse }
@@ -18,9 +18,9 @@
; CHECK: @g4 = global i32 2, align 4 #3
; CHECK: @g5 = global i32 2, no_sanitize_address, align 4
; CHECK: @g6 = global i32 2, no_sanitize_hwaddress, align 4
-; CHECK: @g7 = global i32 2, no_sanitize_memtag, align 4
-; CHECK: @g8 = global i32 2, sanitize_address_dyninit, align 4
-; CHECK: @g9 = global i32 2, no_sanitize_address, no_sanitize_hwaddress, no_sanitize_memtag, align 4
+; CHECK: @g7 = global i32 2, sanitize_address_dyninit, align 4
+; CHECK: @g8 = global i32 2, sanitize_memtag, align 4
+; CHECK: @g9 = global i32 2, no_sanitize_address, no_sanitize_hwaddress, sanitize_memtag, align 4
; CHECK: attributes #0 = { "key"="value" "key2"="value2" }
; CHECK: attributes #1 = { "key3"="value3" }
Index: llvm/lib/IR/AsmWriter.cpp
===================================================================
--- llvm/lib/IR/AsmWriter.cpp
+++ llvm/lib/IR/AsmWriter.cpp
@@ -3538,8 +3538,8 @@
Out << ", no_sanitize_address";
if (MD.NoHWAddress)
Out << ", no_sanitize_hwaddress";
- if (MD.NoMemtag)
- Out << ", no_sanitize_memtag";
+ if (MD.Memtag)
+ Out << ", sanitize_memtag";
if (MD.IsDynInit)
Out << ", sanitize_address_dyninit";
}
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===================================================================
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1232,7 +1232,7 @@
static unsigned
serializeSanitizerMetadata(const GlobalValue::SanitizerMetadata &Meta) {
return Meta.NoAddress | (Meta.NoHWAddress << 1) |
- (Meta.NoMemtag << 2) | (Meta.IsDynInit << 3);
+ (Meta.Memtag << 2) | (Meta.IsDynInit << 3);
}
/// Emit top-level description of module, including target triple, inline asm,
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===================================================================
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -3676,7 +3676,7 @@
if (V & (1 << 1))
Meta.NoHWAddress = true;
if (V & (1 << 2))
- Meta.NoMemtag = true;
+ Meta.Memtag = true;
if (V & (1 << 3))
Meta.IsDynInit = true;
return Meta;
Index: llvm/lib/AsmParser/LLParser.cpp
===================================================================
--- llvm/lib/AsmParser/LLParser.cpp
+++ llvm/lib/AsmParser/LLParser.cpp
@@ -1107,7 +1107,7 @@
switch (Kind) {
case lltok::kw_no_sanitize_address:
case lltok::kw_no_sanitize_hwaddress:
- case lltok::kw_no_sanitize_memtag:
+ case lltok::kw_sanitize_memtag:
case lltok::kw_sanitize_address_dyninit:
return true;
default:
@@ -1128,8 +1128,8 @@
case lltok::kw_no_sanitize_hwaddress:
Meta.NoHWAddress = true;
break;
- case lltok::kw_no_sanitize_memtag:
- Meta.NoMemtag = true;
+ case lltok::kw_sanitize_memtag:
+ Meta.Memtag = true;
break;
case lltok::kw_sanitize_address_dyninit:
Meta.IsDynInit = true;
Index: llvm/lib/AsmParser/LLLexer.cpp
===================================================================
--- llvm/lib/AsmParser/LLLexer.cpp
+++ llvm/lib/AsmParser/LLLexer.cpp
@@ -582,7 +582,6 @@
KEYWORD(no_sanitize_address);
KEYWORD(no_sanitize_hwaddress);
- KEYWORD(no_sanitize_memtag);
KEYWORD(sanitize_address_dyninit);
KEYWORD(ccc);
Index: llvm/include/llvm/IR/GlobalValue.h
===================================================================
--- llvm/include/llvm/IR/GlobalValue.h
+++ llvm/include/llvm/IR/GlobalValue.h
@@ -295,26 +295,38 @@
void setPartition(StringRef Part);
// ASan, HWASan and Memtag sanitizers have some instrumentation that applies
- // specifically to global variables. This instrumentation is implicitly
- // applied to all global variables when built with -fsanitize=*. What we need
- // is a way to persist the information that a certain global variable should
- // *not* have sanitizers applied, which occurs if:
- // 1. The global variable is in the sanitizer ignore list, or
- // 2. The global variable is created by the sanitizers itself for internal
- // usage, or
- // 3. The global variable has __attribute__((no_sanitize("..."))) or
- // __attribute__((disable_sanitizer_instrumentation)).
- //
- // This is important, a some IR passes like GlobalMerge can delete global
- // variables and replace them with new ones. If the old variables were marked
- // to be unsanitized, then the new ones should also be.
+ // specifically to global variables.
struct SanitizerMetadata {
SanitizerMetadata()
- : NoAddress(false), NoHWAddress(false), NoMemtag(false),
- IsDynInit(false) {}
+ : NoAddress(false), NoHWAddress(false),
+ Memtag(false), IsDynInit(false) {}
+ // For ASan and HWASan, this instrumentation is implicitly applied to all
+ // global variables when built with -fsanitize=*. What we need is a way to
+ // persist the information that a certain global variable should *not* have
+ // sanitizers applied, which occurs if:
+ // 1. The global variable is in the sanitizer ignore list, or
+ // 2. The global variable is created by the sanitizers itself for internal
+ // usage, or
+ // 3. The global variable has __attribute__((no_sanitize("..."))) or
+ // __attribute__((disable_sanitizer_instrumentation)).
+ //
+ // This is important, a some IR passes like GlobalMerge can delete global
+ // variables and replace them with new ones. If the old variables were
+ // marked to be unsanitized, then the new ones should also be.
unsigned NoAddress : 1;
unsigned NoHWAddress : 1;
- unsigned NoMemtag : 1;
+
+ // Memtag sanitization works differently: sanitization is requested by clang
+ // when `-fsanitize=memtag-globals` is provided, and the request can be
+ // denied (and the attribute removed) by the AArch64 global tagging pass if
+ // it can't be fulfilled (e.g. the global variable is a TLS variable).
+ // Memtag sanitization has to interact with other parts of LLVM (like
+ // supressing certain optimisations, emitting assembly directives, or
+ // creating special relocation sections).
+ //
+ // Use `GlobalValue::isTagged()` to check whether tagging should be enabled
+ // for a global variable.
+ unsigned Memtag : 1;
// ASan-specific metadata. Is this global variable dynamically initialized
// (from a C++ language perspective), and should therefore be checked for
@@ -331,6 +343,10 @@
void setSanitizerMetadata(SanitizerMetadata Meta);
void removeSanitizerMetadata();
+ bool isTagged() const {
+ return hasSanitizerMetadata() && getSanitizerMetadata().Memtag;
+ }
+
static LinkageTypes getLinkOnceLinkage(bool ODR) {
return ODR ? LinkOnceODRLinkage : LinkOnceAnyLinkage;
}
Index: llvm/include/llvm/AsmParser/LLToken.h
===================================================================
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -399,9 +399,6 @@
// GV's with __attribute__((no_sanitize("hwaddress"))), or things in
// -fsanitize-ignorelist when built with HWASan.
kw_no_sanitize_hwaddress,
- // GV's with __attribute__((no_sanitize("memtag"))), or things in
- // -fsanitize-ignorelist when built with memory tagging.
- kw_no_sanitize_memtag,
// GV's where the clang++ frontend (when ASan is used) notes that this is
// dynamically initialized, and thus needs ODR detection.
kw_sanitize_address_dyninit,
Index: clang/test/CodeGen/sanitizer-special-case-list-globals.c
===================================================================
--- clang/test/CodeGen/sanitizer-special-case-list-globals.c
+++ clang/test/CodeGen/sanitizer-special-case-list-globals.c
@@ -17,12 +17,9 @@
// RUN: -fsanitize-ignorelist=%S/Inputs/sanitizer-special-case-list-globals.txt \
// RUN: | FileCheck %s --check-prefix=HWASAN
-/// TODO(hctim): Move over to memtag-globals when it's implemented. For now
-/// though, it's fine, the frontend still annotates based on any memtag sanitizer
-/// being used.
-// RUN: %clang_cc1 -fsanitize=memtag-heap -triple=aarch64-linux-android31 -emit-llvm %s -o -\
+// RUN: %clang_cc1 -fsanitize=memtag-globals -triple=aarch64-linux-android31 \
// RUN: -fsanitize-ignorelist=%S/Inputs/sanitizer-special-case-list-globals.txt \
-// RUN: | FileCheck %s --check-prefix=MEMTAG
+// RUN: -emit-llvm %s -o - | FileCheck %s --check-prefix=MEMTAG
/// Check that the '[cfi-vcall|cfi-icall] src:*' rule in the ignorelist doesn't change
/// anything for ASan.
@@ -46,11 +43,12 @@
// RUN: -fsanitize-ignorelist=%S/Inputs/sanitizer-special-case-list-globals.txt \
// RUN: | FileCheck %s --check-prefix=NONE
-// NONE: @always_ignored ={{.*}} global
-// NONE-NOT: no_sanitize
-// ASAN: @always_ignored ={{.*}} global {{.*}}, no_sanitize_address
-// HWASAN: @always_ignored ={{.*}} global {{.*}}, no_sanitize_hwaddress
-// MEMTAG: @always_ignored ={{.*}} global {{.*}}, no_sanitize_memtag
+// NONE: @always_ignored ={{.*}} global
+// NONE-NOT: no_sanitize
+// ASAN: @always_ignored ={{.*}} global {{.*}}, no_sanitize_address
+// HWASAN: @always_ignored ={{.*}} global {{.*}}, no_sanitize_hwaddress
+// MEMTAG: @always_ignored ={{.*}} global
+// MEMTAG-NOT: sanitize_memtag
unsigned always_ignored;
// NONE: @hwasan_ignored ={{.*}} global
@@ -58,8 +56,7 @@
// ASAN: @hwasan_ignored ={{.*}} global
// ASAN-NOT: no_sanitize_address
// HWASAN: @hwasan_ignored ={{.*}} global {{.*}}, no_sanitize_hwaddress
-// MEMTAG: @hwasan_ignored ={{.*}} global
-// MEMTAG-NOT: no_sanitize_memtag
+// MEMTAG: @hwasan_ignored ={{.*}} global {{.*}} sanitize_memtag
unsigned hwasan_ignored;
// NONE: @asan_ignored ={{.*}} global
@@ -67,8 +64,7 @@
// ASAN: @asan_ignored ={{.*}} global {{.*}}, no_sanitize_address
// HWASAN: @asan_ignored.hwasan = {{.*}} global
// HWASAN-NOT: no_sanitize_hwaddress
-// MEMTAG: @asan_ignored ={{.*}} global
-// MEMTAG-NOT: no_sanitize_memtag
+// MEMTAG: @asan_ignored ={{.*}} global {{.*}} sanitize_memtag
unsigned asan_ignored;
// NONE: @memtag_ignored ={{.*}} global
@@ -77,7 +73,8 @@
// ASAN-NOT: no_sanitize_address
// HWASAN: @memtag_ignored.hwasan = {{.*}} global
// HWASAN-NOT: no_sanitize_hwaddress
-// MEMTAG: @memtag_ignored ={{.*}} global {{.*}}, no_sanitize_memtag
+// MEMTAG: @memtag_ignored ={{.*}} global
+// MEMTAG-NOT: sanitize_memtag
unsigned memtag_ignored;
// NONE: @never_ignored ={{.*}} global
@@ -86,6 +83,5 @@
// ASAN-NOT: no_sanitize_address
// HWASAN: @never_ignored.hwasan ={{.*}} global
// HWASAN-NOT: no_sanitize_hwaddress
-// MEMTAG: @never_ignored ={{.*}} global
-// MEMTAG-NOT: no_sanitize_memtag
+// MEMTAG: @never_ignored ={{.*}} global {{.*}} sanitize_memtag
unsigned never_ignored;
Index: clang/test/CodeGen/memtag-globals.cpp
===================================================================
--- clang/test/CodeGen/memtag-globals.cpp
+++ clang/test/CodeGen/memtag-globals.cpp
@@ -17,23 +17,30 @@
const char *literal = "Hello, world!";
}
-// CHECK: @{{.*}}extra_global{{.*}} =
-// CHECK-NOT: no_sanitize_memtag
-// CHECK: @{{.*}}global{{.*}} =
-// CHECK-NOT: no_sanitize_memtag
-// CHECK: @{{.*}}attributed_global{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// CHECK: @{{.*}}disable_instrumentation_global{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// CHECK: @{{.*}}ignorelisted_global{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// CHECK: @{{.*}}static_var{{.*}} =
-// CHECK-NOT: no_sanitize_memtag
-// CHECK: @{{.*}} = {{.*}} c"Hello, world!\00"
-// CHECK-NOT: no_sanitize_memtag
-
-// IGNORELIST: @{{.*}}extra_global{{.*}} ={{.*}} global
-// IGNORELIST-NOT: no_sanitize_memtag
-// IGNORELIST: @{{.*}}global{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// IGNORELIST: @{{.*}}attributed_global{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// IGNORELIST: @{{.*}}disable_instrumentation_global{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// IGNORELIST: @{{.*}}ignorelisted_globa{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// IGNORELIST: @{{.*}}static_var{{.*}} ={{.*}} global {{.*}}, no_sanitize_memtag
-// IGNORELIST: @{{.*}} = {{.*}} c"Hello, world!\00"{{.*}}, no_sanitize_memtag
+// CHECK: @{{.*}}extra_global{{.*}} ={{.*}} sanitize_memtag
+// CHECK: @{{.*}}global{{.*}} ={{.*}} sanitize_memtag
+
+// CHECK: @{{.*}}attributed_global{{.*}} =
+// CHECK-NOT: sanitize_memtag
+// CHECK: @{{.*}}disable_instrumentation_global{{.*}} =
+// CHECK-NOT: sanitize_memtag
+// CHECK: @{{.*}}ignorelisted_global{{.*}} =
+// CHECK-NOT: sanitize_memtag
+
+// CHECK: @{{.*}}static_var{{.*}} ={{.*}} sanitize_memtag
+// CHECK: @{{.*}} = {{.*}} c"Hello, world!\00"{{.*}} sanitize_memtag
+
+// IGNORELIST: @{{.*}}extra_global{{.*}} ={{.*}} sanitize_memtag
+
+// IGNORELIST: @{{.*}}global{{.*}} =
+// IGNORELIST-NOT: sanitize_memtag
+// IGNORELIST: @{{.*}}attributed_global{{.*}} =
+// IGNORELIST-NOT: sanitize_memtag
+// IGNORELIST: @{{.*}}disable_instrumentation_global{{.*}} =
+// IGNORELIST-NOT: sanitize_memtag
+// IGNORELIST: @{{.*}}ignorelisted_globa{{.*}} =
+// IGNORELIST-NOT: sanitize_memtag
+// IGNORELIST: @{{.*}}static_var{{.*}} =
+// IGNORELIST-NOT: sanitize_memtag
+// IGNORELIST: @{{.*}} = {{.*}} c"Hello, world!\00"{{.*}}
+// IGNORELIST-NOT: sanitize_memtag
Index: clang/lib/CodeGen/SanitizerMetadata.cpp
===================================================================
--- clang/lib/CodeGen/SanitizerMetadata.cpp
+++ clang/lib/CodeGen/SanitizerMetadata.cpp
@@ -60,8 +60,10 @@
Meta.NoHWAddress |= CGM.isInNoSanitizeList(
FsanitizeArgument.Mask & SanitizerKind::HWAddress, GV, Loc, Ty);
- Meta.NoMemtag |= NoSanitizeAttrSet.hasOneOf(SanitizerKind::MemTag);
- Meta.NoMemtag |= CGM.isInNoSanitizeList(
+ Meta.Memtag |=
+ static_cast<bool>(FsanitizeArgument.Mask & SanitizerKind::MemtagGlobals);
+ Meta.Memtag &= !NoSanitizeAttrSet.hasOneOf(SanitizerKind::MemTag);
+ Meta.Memtag &= !CGM.isInNoSanitizeList(
FsanitizeArgument.Mask & SanitizerKind::MemTag, GV, Loc, Ty);
Meta.IsDynInit = IsDynInit && !Meta.NoAddress &&
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
