Skip to content

[MTE] decide whether to tag global in AsmPrinter #135891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 17, 2025
Merged

[MTE] decide whether to tag global in AsmPrinter #135891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf7c14e
[𝘀𝗽𝗿] initial version
fmayer Apr 16, 2025
1ab14cf
upd
fmayer Apr 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 5 additions & 40 deletions clang/lib/CodeGen/SanitizerMetadata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,37 +32,6 @@ static SanitizerMask expandKernelSanitizerMasks(SanitizerMask Mask) {
return Mask;
}

static bool shouldTagGlobal(const llvm::GlobalVariable &G) {
// For now, don't instrument constant data, as it'll be in .rodata anyway. It
// may be worth instrumenting these in future to stop them from being used as
// gadgets.
if (G.getName().starts_with("llvm.") || G.isThreadLocal() || G.isConstant())
return false;

// Globals can be placed implicitly or explicitly in sections. There's two
// different types of globals that meet this criteria that cause problems:
// 1. Function pointers that are going into various init arrays (either
// explicitly through `__attribute__((section(<foo>)))` or implicitly
// through `__attribute__((constructor)))`, such as ".(pre)init(_array)",
// ".fini(_array)", ".ctors", and ".dtors". These function pointers end up
// overaligned and overpadded, making iterating over them problematic, and
// each function pointer is individually tagged (so the iteration over
// them causes SIGSEGV/MTE[AS]ERR).
// 2. Global variables put into an explicit section, where the section's name
// is a valid C-style identifier. The linker emits a `__start_<name>` and
// `__stop_<name>` symbol for the section, so that you can iterate over
// globals within this section. Unfortunately, again, these globals would
// be tagged and so iteration causes SIGSEGV/MTE[AS]ERR.
//
// To mitigate both these cases, and because specifying a section is rare
// outside of these two cases, disable MTE protection for globals in any
// section.
if (G.hasSection())
return false;

return true;
}

void SanitizerMetadata::reportGlobal(llvm::GlobalVariable *GV,
SourceLocation Loc, StringRef Name,
QualType Ty,
Expand All @@ -89,15 +58,11 @@ void SanitizerMetadata::reportGlobal(llvm::GlobalVariable *GV,
Meta.NoHWAddress |= CGM.isInNoSanitizeList(
FsanitizeArgument.Mask & SanitizerKind::HWAddress, GV, Loc, Ty);

if (shouldTagGlobal(*GV)) {
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);
} else {
Meta.Memtag = false;
}
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 &&
FsanitizeArgument.has(SanitizerKind::Address) &&
Expand Down
7 changes: 4 additions & 3 deletions clang/test/CodeGen/memtag-globals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ void func() {
// CHECK: @{{.*}}extra_global{{.*}} ={{.*}} sanitize_memtag
// CHECK: @{{.*}}global{{.*}} ={{.*}} sanitize_memtag

// CHECK: @{{.*}}section_global{{.*}} =
// CHECK-NOT: sanitize_memtag
// 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{{.*}} =
Expand All @@ -35,7 +36,7 @@ void func() {
// CHECK-NOT: sanitize_memtag

// CHECK: @{{.*}}static_var{{.*}} ={{.*}} sanitize_memtag
// CHECK: @{{.*}} = {{.*}} c"Hello, world!\00"{{.*}}
// CHECK: @{{.*}} = {{.*}} c"Hello, world!\00"{{.*}} sanitize_memtag
// CHECK: @{{.*}}external_global{{.*}} ={{.*}} sanitize_memtag

// IGNORELIST: @{{.*}}extra_global{{.*}} ={{.*}} sanitize_memtag
Expand Down
47 changes: 47 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2398,6 +2398,41 @@ void AsmPrinter::emitRemarksSection(remarks::RemarkStreamer &RS) {
OutStreamer->emitBinaryData(Buf);
}

static bool shouldTagGlobal(const llvm::GlobalVariable &G) {
// 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.
//
// For now, don't instrument constant data, as it'll be in .rodata anyway. It
// may be worth instrumenting these in future to stop them from being used as
// gadgets.
if (G.getName().starts_with("llvm.") || G.isThreadLocal() || G.isConstant())
return false;

// Globals can be placed implicitly or explicitly in sections. There's two
// different types of globals that meet this criteria that cause problems:
// 1. Function pointers that are going into various init arrays (either
// explicitly through `__attribute__((section(<foo>)))` or implicitly
// through `__attribute__((constructor)))`, such as ".(pre)init(_array)",
// ".fini(_array)", ".ctors", and ".dtors". These function pointers end up
// overaligned and overpadded, making iterating over them problematic, and
// each function pointer is individually tagged (so the iteration over
// them causes SIGSEGV/MTE[AS]ERR).
// 2. Global variables put into an explicit section, where the section's name
// is a valid C-style identifier. The linker emits a `__start_<name>` and
// `__stop_<name>` symbol for the section, so that you can iterate over
// globals within this section. Unfortunately, again, these globals would
// be tagged and so iteration causes SIGSEGV/MTE[AS]ERR.
//
// To mitigate both these cases, and because specifying a section is rare
// outside of these two cases, disable MTE protection for globals in any
// section.
if (G.hasSection())
return false;

return true;
}

static void tagGlobalDefinition(Module &M, GlobalVariable *G) {
Constant *Initializer = G->getInitializer();
uint64_t SizeInBytes =
Expand Down Expand Up @@ -2430,6 +2465,12 @@ static void tagGlobalDefinition(Module &M, GlobalVariable *G) {
G->setUnnamedAddr(GlobalValue::UnnamedAddr::None);
}

static void removeMemtagFromGlobal(GlobalVariable &G) {
auto Meta = G.getSanitizerMetadata();
Meta.Memtag = false;
G.setSanitizerMetadata(Meta);
}

bool AsmPrinter::doFinalization(Module &M) {
// Set the MachineFunction to nullptr so that we can catch attempted
// accesses to MF specific features at the module level and so that
Expand All @@ -2440,6 +2481,12 @@ bool AsmPrinter::doFinalization(Module &M) {
for (GlobalVariable &G : M.globals()) {
if (G.isDeclaration() || !G.isTagged())
continue;
if (!shouldTagGlobal(G)) {
assert(G.hasSanitizerMetadata()); // because isTagged.
removeMemtagFromGlobal(G);
assert(!G.isTagged());
continue;
}
GlobalsToTag.push_back(&G);
}
for (GlobalVariable *G : GlobalsToTag)
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/IR/Verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,10 +808,6 @@ void Verifier::visitGlobalValue(const GlobalValue &GV) {
"visibility must be dso_local!",
&GV);

if (GV.isTagged()) {
Check(!GV.hasSection(), "tagged GlobalValue must not be in section.", &GV);
}

forEachUser(&GV, GlobalValueVisited, [&](const Value *V) -> bool {
if (const Instruction *I = dyn_cast<Instruction>(V)) {
if (!I->getParent() || !I->getParent()->getParent())
Expand Down
21 changes: 0 additions & 21 deletions llvm/unittests/IR/VerifierTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,26 +416,5 @@ TEST(VerifierTest, GetElementPtrInst) {
<< Error;
}

TEST(VerifierTest, DetectTaggedGlobalInSection) {
LLVMContext C;
Module M("M", C);
GlobalVariable *GV = new GlobalVariable(
Type::getInt64Ty(C), false, GlobalValue::InternalLinkage,
ConstantInt::get(Type::getInt64Ty(C), 1));
GV->setDSOLocal(true);
GlobalValue::SanitizerMetadata MD{};
MD.Memtag = true;
GV->setSanitizerMetadata(MD);
GV->setSection("foo");
M.insertGlobalVariable(GV);

std::string Error;
raw_string_ostream ErrorOS(Error);
EXPECT_TRUE(verifyModule(M, &ErrorOS));
EXPECT_TRUE(
StringRef(Error).starts_with("tagged GlobalValue must not be in section"))
<< Error;
}

} // end anonymous namespace
} // end namespace llvm
Loading