Skip to content

Commit 9ed4c70

Browse files
fmayerfmayer
authored
[MTE] decide whether to tag global in AsmPrinter (#135891)
there are llvm passes that would invalidate the decision we make in clang.
1 parent 52e10e6 commit 9ed4c70

File tree

5 files changed

+56
-68
lines changed

5 files changed

+56
-68
lines changed

‎clang/lib/CodeGen/SanitizerMetadata.cpp‎

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -32,37 +32,6 @@ static SanitizerMask expandKernelSanitizerMasks(SanitizerMask Mask) {
3232
return Mask;
3333
}
3434

35-
static bool shouldTagGlobal(const llvm::GlobalVariable &G) {
36-
// For now, don't instrument constant data, as it'll be in .rodata anyway. It
37-
// may be worth instrumenting these in future to stop them from being used as
38-
// gadgets.
39-
if (G.getName().starts_with("llvm.") || G.isThreadLocal() || G.isConstant())
40-
return false;
41-
42-
// Globals can be placed implicitly or explicitly in sections. There's two
43-
// different types of globals that meet this criteria that cause problems:
44-
// 1. Function pointers that are going into various init arrays (either
45-
// explicitly through `__attribute__((section(<foo>)))` or implicitly
46-
// through `__attribute__((constructor)))`, such as ".(pre)init(_array)",
47-
// ".fini(_array)", ".ctors", and ".dtors". These function pointers end up
48-
// overaligned and overpadded, making iterating over them problematic, and
49-
// each function pointer is individually tagged (so the iteration over
50-
// them causes SIGSEGV/MTE[AS]ERR).
51-
// 2. Global variables put into an explicit section, where the section's name
52-
// is a valid C-style identifier. The linker emits a `__start_<name>` and
53-
// `__stop_<name>` symbol for the section, so that you can iterate over
54-
// globals within this section. Unfortunately, again, these globals would
55-
// be tagged and so iteration causes SIGSEGV/MTE[AS]ERR.
56-
//
57-
// To mitigate both these cases, and because specifying a section is rare
58-
// outside of these two cases, disable MTE protection for globals in any
59-
// section.
60-
if (G.hasSection())
61-
return false;
62-
63-
return true;
64-
}
65-
6635
void SanitizerMetadata::reportGlobal(llvm::GlobalVariable *GV,
6736
SourceLocation Loc, StringRef Name,
6837
QualType Ty,
@@ -89,15 +58,11 @@ void SanitizerMetadata::reportGlobal(llvm::GlobalVariable *GV,
8958
Meta.NoHWAddress |= CGM.isInNoSanitizeList(
9059
FsanitizeArgument.Mask & SanitizerKind::HWAddress, GV, Loc, Ty);
9160

92-
if (shouldTagGlobal(*GV)) {
93-
Meta.Memtag |= static_cast<bool>(FsanitizeArgument.Mask &
94-
SanitizerKind::MemtagGlobals);
95-
Meta.Memtag &= !NoSanitizeAttrSet.hasOneOf(SanitizerKind::MemTag);
96-
Meta.Memtag &= !CGM.isInNoSanitizeList(
97-
FsanitizeArgument.Mask & SanitizerKind::MemTag, GV, Loc, Ty);
98-
} else {
99-
Meta.Memtag = false;
100-
}
61+
Meta.Memtag |=
62+
static_cast<bool>(FsanitizeArgument.Mask & SanitizerKind::MemtagGlobals);
63+
Meta.Memtag &= !NoSanitizeAttrSet.hasOneOf(SanitizerKind::MemTag);
64+
Meta.Memtag &= !CGM.isInNoSanitizeList(
65+
FsanitizeArgument.Mask & SanitizerKind::MemTag, GV, Loc, Ty);
10166

10267
Meta.IsDynInit = IsDynInit && !Meta.NoAddress &&
10368
FsanitizeArgument.has(SanitizerKind::Address) &&

‎clang/test/CodeGen/memtag-globals.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ void func() {
2525
// CHECK: @{{.*}}extra_global{{.*}} ={{.*}} sanitize_memtag
2626
// CHECK: @{{.*}}global{{.*}} ={{.*}} sanitize_memtag
2727

28-
// CHECK: @{{.*}}section_global{{.*}} =
29-
// CHECK-NOT: sanitize_memtag
28+
// This DOES NOT mean we are instrumenting the section global,
29+
// but we are ignoring it in AsmPrinter rather than in clang.
30+
// CHECK: @{{.*}}section_global{{.*}} ={{.*}} sanitize_memtag
3031
// CHECK: @{{.*}}attributed_global{{.*}} =
3132
// CHECK-NOT: sanitize_memtag
3233
// CHECK: @{{.*}}disable_instrumentation_global{{.*}} =
@@ -35,7 +36,7 @@ void func() {
3536
// CHECK-NOT: sanitize_memtag
3637

3738
// CHECK: @{{.*}}static_var{{.*}} ={{.*}} sanitize_memtag
38-
// CHECK: @{{.*}} = {{.*}} c"Hello, world!\00"{{.*}}
39+
// CHECK: @{{.*}} = {{.*}} c"Hello, world!\00"{{.*}} sanitize_memtag
3940
// CHECK: @{{.*}}external_global{{.*}} ={{.*}} sanitize_memtag
4041

4142
// IGNORELIST: @{{.*}}extra_global{{.*}} ={{.*}} sanitize_memtag

‎llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2398,6 +2398,41 @@ void AsmPrinter::emitRemarksSection(remarks::RemarkStreamer &RS) {
23982398
OutStreamer->emitBinaryData(Buf);
23992399
}
24002400

2401+
static bool shouldTagGlobal(const llvm::GlobalVariable &G) {
2402+
// We used to do this in clang, but there are optimization passes that turn
2403+
// non-constant globals into constants. So now, clang only tells us whether
2404+
// it would *like* a global to be tagged, but we still make the decision here.
2405+
//
2406+
// For now, don't instrument constant data, as it'll be in .rodata anyway. It
2407+
// may be worth instrumenting these in future to stop them from being used as
2408+
// gadgets.
2409+
if (G.getName().starts_with("llvm.") || G.isThreadLocal() || G.isConstant())
2410+
return false;
2411+
2412+
// Globals can be placed implicitly or explicitly in sections. There's two
2413+
// different types of globals that meet this criteria that cause problems:
2414+
// 1. Function pointers that are going into various init arrays (either
2415+
// explicitly through `__attribute__((section(<foo>)))` or implicitly
2416+
// through `__attribute__((constructor)))`, such as ".(pre)init(_array)",
2417+
// ".fini(_array)", ".ctors", and ".dtors". These function pointers end up
2418+
// overaligned and overpadded, making iterating over them problematic, and
2419+
// each function pointer is individually tagged (so the iteration over
2420+
// them causes SIGSEGV/MTE[AS]ERR).
2421+
// 2. Global variables put into an explicit section, where the section's name
2422+
// is a valid C-style identifier. The linker emits a `__start_<name>` and
2423+
// `__stop_<name>` symbol for the section, so that you can iterate over
2424+
// globals within this section. Unfortunately, again, these globals would
2425+
// be tagged and so iteration causes SIGSEGV/MTE[AS]ERR.
2426+
//
2427+
// To mitigate both these cases, and because specifying a section is rare
2428+
// outside of these two cases, disable MTE protection for globals in any
2429+
// section.
2430+
if (G.hasSection())
2431+
return false;
2432+
2433+
return true;
2434+
}
2435+
24012436
static void tagGlobalDefinition(Module &M, GlobalVariable *G) {
24022437
Constant *Initializer = G->getInitializer();
24032438
uint64_t SizeInBytes =
@@ -2430,6 +2465,12 @@ static void tagGlobalDefinition(Module &M, GlobalVariable *G) {
24302465
G->setUnnamedAddr(GlobalValue::UnnamedAddr::None);
24312466
}
24322467

2468+
static void removeMemtagFromGlobal(GlobalVariable &G) {
2469+
auto Meta = G.getSanitizerMetadata();
2470+
Meta.Memtag = false;
2471+
G.setSanitizerMetadata(Meta);
2472+
}
2473+
24332474
bool AsmPrinter::doFinalization(Module &M) {
24342475
// Set the MachineFunction to nullptr so that we can catch attempted
24352476
// accesses to MF specific features at the module level and so that
@@ -2440,6 +2481,12 @@ bool AsmPrinter::doFinalization(Module &M) {
24402481
for (GlobalVariable &G : M.globals()) {
24412482
if (G.isDeclaration() || !G.isTagged())
24422483
continue;
2484+
if (!shouldTagGlobal(G)) {
2485+
assert(G.hasSanitizerMetadata()); // because isTagged.
2486+
removeMemtagFromGlobal(G);
2487+
assert(!G.isTagged());
2488+
continue;
2489+
}
24432490
GlobalsToTag.push_back(&G);
24442491
}
24452492
for (GlobalVariable *G : GlobalsToTag)

‎llvm/lib/IR/Verifier.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -807,10 +807,6 @@ void Verifier::visitGlobalValue(const GlobalValue &GV) {
807807
"visibility must be dso_local!",
808808
&GV);
809809

810-
if (GV.isTagged()) {
811-
Check(!GV.hasSection(), "tagged GlobalValue must not be in section.", &GV);
812-
}
813-
814810
forEachUser(&GV, GlobalValueVisited, [&](const Value *V) -> bool {
815811
if (const Instruction *I = dyn_cast<Instruction>(V)) {
816812
if (!I->getParent() || !I->getParent()->getParent())

‎llvm/unittests/IR/VerifierTest.cpp‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -416,26 +416,5 @@ TEST(VerifierTest, GetElementPtrInst) {
416416
<< Error;
417417
}
418418

419-
TEST(VerifierTest, DetectTaggedGlobalInSection) {
420-
LLVMContext C;
421-
Module M("M", C);
422-
GlobalVariable *GV = new GlobalVariable(
423-
Type::getInt64Ty(C), false, GlobalValue::InternalLinkage,
424-
ConstantInt::get(Type::getInt64Ty(C), 1));
425-
GV->setDSOLocal(true);
426-
GlobalValue::SanitizerMetadata MD{};
427-
MD.Memtag = true;
428-
GV->setSanitizerMetadata(MD);
429-
GV->setSection("foo");
430-
M.insertGlobalVariable(GV);
431-
432-
std::string Error;
433-
raw_string_ostream ErrorOS(Error);
434-
EXPECT_TRUE(verifyModule(M, &ErrorOS));
435-
EXPECT_TRUE(
436-
StringRef(Error).starts_with("tagged GlobalValue must not be in section"))
437-
<< Error;
438-
}
439-
440419
} // end anonymous namespace
441420
} // end namespace llvm

0 commit comments

Comments
 (0)