llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-llvm-ir Author: Tarcísio Fischer (tarcisiofischer) <details> <summary>Changes</summary> The current behaviour is that any global variable inside sections is not MTE tagged, and (to the best of my knowledge) there's no way to opt-in. This PR proposes a way to opt-in. One example of where this could potentially be useful is the ProtectedMemory class on Chromium, which adds all the "protected" variables inside a single section to control it's access (https://source.chromium.org/chromium/chromium/src/+/main:base/memory/protected_memory.h;l=157;drc=299864be12719dba5dee143363b6aaa150cb96f8). (Reopen from https://github.com/llvm/llvm-project/pull/168535) --- Full diff: https://github.com/llvm/llvm-project/pull/182028.diff 11 Files Affected: - (modified) clang/include/clang/Basic/Attr.td (+6) - (modified) clang/include/clang/Basic/AttrDocs.td (+13) - (modified) clang/lib/CodeGen/SanitizerMetadata.cpp (+4-2) - (modified) clang/lib/CodeGen/SanitizerMetadata.h (+2-1) - (modified) clang/lib/Sema/SemaDeclAttr.cpp (+7) - (modified) clang/test/CodeGen/memtag-globals-asm.cpp (+2) - (modified) clang/test/CodeGen/memtag-globals.cpp (+1) - (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (+1) - (modified) llvm/include/llvm/IR/GlobalValue.h (+10-3) - (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (+4) - (modified) llvm/lib/Target/TargetLoweringObjectFile.cpp (+6) ``````````diff diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 2621d178d99e8..1791ef5908742 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3927,6 +3927,12 @@ def X86ForceAlignArgPointer : InheritableAttr, TargetSpecificAttr<TargetAnyX86> let Documentation = [X86ForceAlignArgPointerDocs]; } +def ForceMemtag : InheritableAttr { + let Spellings = [ClangGCC<"force_memtag">]; + let Subjects = SubjectList<[GlobalVar]>; + let Documentation = [ForceMemtagDocs]; +} + def NoSanitize : InheritableAttr { let Spellings = [ClangGCC<"no_sanitize">]; let Args = [VariadicStringArgument<"Sanitizers">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index f91fe2c7b0259..34bf91c6ab954 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3667,6 +3667,19 @@ full list of supported sanitizer flags. }]; } +def ForceMemtagDocs : Documentation { + let Category = DocCatVariable; + let Content = [{ +Use the ``force_memtag`` attribute on a global variable declaration that also +has the attribute section. If the memory tag sanitizer is active, will force +the global variable to be MTE tagged. Global variables under sections are not +tagged by default, so you need to explicitly opt-in using this attribute. +Please note that, unfortunately, in non-trivial cases where memory layout +is assumed, forcing memory tagging might lead to extremely hard to debug bugs, +depending on what the code assuming the alignment does. + }]; +} + def DisableSanitizerInstrumentationDocs : Documentation { let Category = DocCatFunction; let Content = [{ diff --git a/clang/lib/CodeGen/SanitizerMetadata.cpp b/clang/lib/CodeGen/SanitizerMetadata.cpp index 288f7f6415f44..d9fc20e4bdf0f 100644 --- a/clang/lib/CodeGen/SanitizerMetadata.cpp +++ b/clang/lib/CodeGen/SanitizerMetadata.cpp @@ -36,7 +36,8 @@ void SanitizerMetadata::reportGlobal(llvm::GlobalVariable *GV, SourceLocation Loc, StringRef Name, QualType Ty, SanitizerMask NoSanitizeAttrMask, - bool IsDynInit) { + bool IsDynInit, + bool HasForcedSanitizeMemtag) { SanitizerSet FsanitizeArgument = CGM.getLangOpts().Sanitize; if (!isAsanHwasanMemTagOrTysan(FsanitizeArgument)) return; @@ -63,6 +64,7 @@ void SanitizerMetadata::reportGlobal(llvm::GlobalVariable *GV, Meta.Memtag &= !NoSanitizeAttrSet.hasOneOf(SanitizerKind::MemTag); Meta.Memtag &= !CGM.isInNoSanitizeList( FsanitizeArgument.Mask & SanitizerKind::MemTag, GV, Loc, Ty); + Meta.ForceMemtag |= HasForcedSanitizeMemtag; Meta.IsDynInit = IsDynInit && !Meta.NoAddress && FsanitizeArgument.has(SanitizerKind::Address) && @@ -119,7 +121,7 @@ void SanitizerMetadata::reportGlobal(llvm::GlobalVariable *GV, const VarDecl &D, }; reportGlobal(GV, D.getLocation(), QualName, D.getType(), getNoSanitizeMask(D), - IsDynInit); + IsDynInit, D.hasAttr<ForceMemtagAttr>()); } void SanitizerMetadata::disableSanitizerForGlobal(llvm::GlobalVariable *GV) { diff --git a/clang/lib/CodeGen/SanitizerMetadata.h b/clang/lib/CodeGen/SanitizerMetadata.h index 000f02cf8dcf1..20627991e051b 100644 --- a/clang/lib/CodeGen/SanitizerMetadata.h +++ b/clang/lib/CodeGen/SanitizerMetadata.h @@ -42,7 +42,8 @@ class SanitizerMetadata { void reportGlobal(llvm::GlobalVariable *GV, SourceLocation Loc, StringRef Name, QualType Ty = {}, SanitizerMask NoSanitizeAttrMask = {}, - bool IsDynInit = false); + bool IsDynInit = false, + bool HasForcedSanitizeMemtag = false); void disableSanitizerForGlobal(llvm::GlobalVariable *GV); }; } // end namespace CodeGen diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 5dbff18fff7a9..06ff06460bfdb 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6665,6 +6665,10 @@ static bool isSanitizerAttributeAllowedOnGlobals(StringRef Sanitizer) { Sanitizer == "memtag"; } +static void handleForceMemtagAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + D->addAttr(ForceMemtagAttr::CreateImplicit(S.Context, AL)); +} + static void handleNoSanitizeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (!AL.checkAtLeastNumArgs(S, 1)) return; @@ -7938,6 +7942,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_PtGuardedVar: handlePtGuardedVarAttr(S, D, AL); break; + case ParsedAttr::AT_ForceMemtag: + handleForceMemtagAttr(S, D, AL); + break; case ParsedAttr::AT_NoSanitize: handleNoSanitizeAttr(S, D, AL); break; diff --git a/clang/test/CodeGen/memtag-globals-asm.cpp b/clang/test/CodeGen/memtag-globals-asm.cpp index fb3958dd8bcb6..eb62523ce7cc9 100644 --- a/clang/test/CodeGen/memtag-globals-asm.cpp +++ b/clang/test/CodeGen/memtag-globals-asm.cpp @@ -292,6 +292,7 @@ CONSTRUCTOR(".preinit_array") func_t preinit_array = func_constructor; CONSTRUCTOR("array_of_globals") int global1; CONSTRUCTOR("array_of_globals") int global2; CONSTRUCTOR("array_of_globals") int global_string; +CONSTRUCTOR("opt_in_memtag") __attribute__((force_memtag)) int tagged_global; // CHECK-NOT: .memtag func_constructor // CHECK-NOT: .memtag func_init @@ -304,3 +305,4 @@ CONSTRUCTOR("array_of_globals") int global_string; // CHECK-NOT: .memtag global1 // CHECK-NOT: .memtag global2 // CHECK-NOT: .memtag global_string +// CHECK: .memtag tagged_global diff --git a/clang/test/CodeGen/memtag-globals.cpp b/clang/test/CodeGen/memtag-globals.cpp index 407eea1c37cfa..5743e608111ad 100644 --- a/clang/test/CodeGen/memtag-globals.cpp +++ b/clang/test/CodeGen/memtag-globals.cpp @@ -28,6 +28,7 @@ void func() { // This DOES NOT mean we are instrumenting the section global, // but we are ignoring it in AsmPrinter rather than in clang. // CHECK: @{{.*}}section_global{{.*}} ={{.*}} sanitize_memtag + // CHECK: @{{.*}}attributed_global{{.*}} = // CHECK-NOT: sanitize_memtag // CHECK: @{{.*}}disable_instrumentation_global{{.*}} = diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 626a0743238d5..cca875f722292 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -86,6 +86,7 @@ // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable)) // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum) // CHECK-NEXT: Flatten (SubjectMatchRule_function) +// CHECK-NEXT: ForceMemtag (SubjectMatchRule_variable_is_global) // CHECK-NEXT: FunctionReturnThunks (SubjectMatchRule_function) // CHECK-NEXT: GCCStruct (SubjectMatchRule_record) // CHECK-NEXT: GNUInline (SubjectMatchRule_function) diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h index 83e695cdd27d9..f1492b90dc014 100644 --- a/llvm/include/llvm/IR/GlobalValue.h +++ b/llvm/include/llvm/IR/GlobalValue.h @@ -318,8 +318,8 @@ class GlobalValue : public Constant { // specifically to global variables. struct SanitizerMetadata { SanitizerMetadata() - : NoAddress(false), NoHWAddress(false), - Memtag(false), IsDynInit(false) {} + : NoAddress(false), NoHWAddress(false), Memtag(false), + ForceMemtag(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 @@ -346,7 +346,13 @@ class GlobalValue : public Constant { // // Use `GlobalValue::isTagged()` to check whether tagging should be enabled // for a global variable. + // + // Since not all global variables will be tagged (more specifically, + // variables inside sections will not be tagged), ForceMemtag can be used + // to opt-in memory taggig in those cases. For more information see + // __attribute__((force_memtag)). unsigned Memtag : 1; + unsigned ForceMemtag : 1; // ASan-specific metadata. Is this global variable dynamically initialized // (from a C++ language perspective), and should therefore be checked for @@ -365,7 +371,8 @@ class GlobalValue : public Constant { LLVM_ABI void setNoSanitizeMetadata(); bool isTagged() const { - return hasSanitizerMetadata() && getSanitizerMetadata().Memtag; + return hasSanitizerMetadata() && (getSanitizerMetadata().Memtag || + getSanitizerMetadata().ForceMemtag); } static LinkageTypes getLinkOnceLinkage(bool ODR) { diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index d968ead83e5ed..df380efc0497c 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -2662,6 +2662,10 @@ static uint64_t globalSize(const llvm::GlobalVariable &G) { } static bool shouldTagGlobal(const llvm::GlobalVariable &G) { + auto Meta = G.getSanitizerMetadata(); + if (Meta.ForceMemtag) + return true; + // We used to do this in clang, but there are optimization passes that turn // non-constant globals into constants. So now, clang only tells us whether // it would *like* a global to be tagged, but we still make the decision here. diff --git a/llvm/lib/Target/TargetLoweringObjectFile.cpp b/llvm/lib/Target/TargetLoweringObjectFile.cpp index ae31cd90b37ab..bf26656314dc3 100644 --- a/llvm/lib/Target/TargetLoweringObjectFile.cpp +++ b/llvm/lib/Target/TargetLoweringObjectFile.cpp @@ -298,6 +298,12 @@ SectionKind TargetLoweringObjectFile::getKindForGlobal(const GlobalObject *GO, // If the global is marked constant, we can put it into a mergable section, // a mergable string section, or general .data if it contains relocations. if (GVar->isConstant()) { + if (GVar->hasSanitizerMetadata()) { + auto Meta = GVar->getSanitizerMetadata(); + if (Meta.ForceMemtag) + return SectionKind::getData(); + } + // If the initializer for the global contains something that requires a // relocation, then we may have to drop this into a writable data section // even though it is marked const. `````````` </details> https://github.com/llvm/llvm-project/pull/182028 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
