Skip to content

Commit e356bbb

Browse files
committed
fixup! address some comments
Created using spr 1.3.8-beta.1
2 parents e0fd16a + 137f0bc commit e356bbb

File tree

13 files changed

+173
-37
lines changed

13 files changed

+173
-37
lines changed

llvm/docs/LangRef.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8400,6 +8400,14 @@ functions, and contains richer semantic information about the type of the
84008400
allocation. This information is consumed by the ``alloc-token`` pass to
84018401
instrument such calls with allocation token IDs.
84028402

8403+
The metadata contains a string with the type of an allocation.
8404+
8405+
.. code-block:: none
8406+
8407+
call ptr @malloc(i64 64), !alloc_token !0
8408+
8409+
!0 = !{!"<type-name>"}
8410+
84038411
Module Flags Metadata
84048412
=====================
84058413

llvm/lib/IR/Verifier.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
535535
void visitDIAssignIDMetadata(Instruction &I, MDNode *MD);
536536
void visitMMRAMetadata(Instruction &I, MDNode *MD);
537537
void visitAnnotationMetadata(MDNode *Annotation);
538+
void visitAllocTokenMetadata(Instruction &I, MDNode *MD);
538539
void visitAliasScopeMetadata(const MDNode *MD);
539540
void visitAliasScopeListMetadata(const MDNode *MD);
540541
void visitAccessGroupMetadata(const MDNode *MD);
@@ -5332,6 +5333,12 @@ void Verifier::visitAccessGroupMetadata(const MDNode *MD) {
53325333
}
53335334
}
53345335

5336+
void Verifier::visitAllocTokenMetadata(Instruction &I, MDNode *MD) {
5337+
Check(isa<CallBase>(I), "!alloc_token should only exist on calls", &I);
5338+
Check(MD->getNumOperands() == 1, "!alloc_token must have 1 operand", MD);
5339+
Check(isa<MDString>(MD->getOperand(0)), "expected string", MD);
5340+
}
5341+
53355342
/// verifyInstruction - Verify that an instruction is well formed.
53365343
///
53375344
void Verifier::visitInstruction(Instruction &I) {
@@ -5559,6 +5566,9 @@ void Verifier::visitInstruction(Instruction &I) {
55595566
if (MDNode *Annotation = I.getMetadata(LLVMContext::MD_annotation))
55605567
visitAnnotationMetadata(Annotation);
55615568

5569+
if (MDNode *MD = I.getMetadata(LLVMContext::MD_alloc_token))
5570+
visitAllocTokenMetadata(I, MD);
5571+
55625572
if (MDNode *N = I.getDebugLoc().getAsMDNode()) {
55635573
CheckDI(isa<DILocation>(N), "invalid !dbg metadata attachment", &I, N);
55645574
visitMDNode(*N, AreDebugLocsAllowed::Yes);

llvm/lib/Transforms/Instrumentation/AllocToken.cpp

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,13 @@ class AllocToken {
236236
bool instrumentFunction(Function &F);
237237

238238
private:
239-
/// Returns true for !isAllocationFn() functions that are also eligible for
240-
/// instrumentation.
241-
static bool isInstrumentableLibFunc(LibFunc Func, const Value *V,
242-
const TargetLibraryInfo *TLI);
239+
/// Returns the LibFunc (or NotLibFunc) if this call should be instrumented.
240+
std::optional<LibFunc>
241+
shouldInstrumentCall(const CallBase &CB, const TargetLibraryInfo &TLI) const;
242+
243+
/// Returns true for functions that are eligible for instrumentation.
244+
static bool isInstrumentableLibFunc(LibFunc Func, const CallBase &CB,
245+
const TargetLibraryInfo &TLI);
243246

244247
/// Returns true for isAllocationFn() functions that we should ignore.
245248
static bool ignoreInstrumentableLibFunc(LibFunc Func);
@@ -291,21 +294,8 @@ bool AllocToken::instrumentFunction(Function &F) {
291294
auto *CB = dyn_cast<CallBase>(&I);
292295
if (!CB)
293296
continue;
294-
const Function *Callee = CB->getCalledFunction();
295-
if (!Callee)
296-
continue;
297-
// Ignore nobuiltin of the CallBase, so that we can cover nobuiltin libcalls
298-
// if requested via isInstrumentableLibFunc(). Note that isAllocationFn() is
299-
// returning false for nobuiltin calls.
300-
LibFunc Func;
301-
if (TLI.getLibFunc(*Callee, Func)) {
302-
if (ignoreInstrumentableLibFunc(Func))
303-
continue;
304-
if (isInstrumentableLibFunc(Func, CB, &TLI))
305-
AllocCalls.emplace_back(CB, Func);
306-
} else if (Options.Extended && getAllocTokenMetadata(*CB)) {
307-
AllocCalls.emplace_back(CB, NotLibFunc);
308-
}
297+
if (std::optional<LibFunc> Func = shouldInstrumentCall(*CB, TLI))
298+
AllocCalls.emplace_back(CB, Func.value());
309299
}
310300

311301
bool Modified = false;
@@ -317,18 +307,46 @@ bool AllocToken::instrumentFunction(Function &F) {
317307
return Modified;
318308
}
319309

320-
bool AllocToken::isInstrumentableLibFunc(LibFunc Func, const Value *V,
321-
const TargetLibraryInfo *TLI) {
322-
if (isAllocationFn(V, TLI))
310+
std::optional<LibFunc>
311+
AllocToken::shouldInstrumentCall(const CallBase &CB,
312+
const TargetLibraryInfo &TLI) const {
313+
const Function *Callee = CB.getCalledFunction();
314+
if (!Callee)
315+
return std::nullopt;
316+
317+
// Ignore nobuiltin of the CallBase, so that we can cover nobuiltin libcalls
318+
// if requested via isInstrumentableLibFunc(). Note that isAllocationFn() is
319+
// returning false for nobuiltin calls.
320+
LibFunc Func;
321+
if (TLI.getLibFunc(*Callee, Func)) {
322+
if (isInstrumentableLibFunc(Func, CB, TLI))
323+
return Func;
324+
} else if (Options.Extended && getAllocTokenMetadata(CB)) {
325+
return NotLibFunc;
326+
}
327+
328+
return std::nullopt;
329+
}
330+
331+
bool AllocToken::isInstrumentableLibFunc(LibFunc Func, const CallBase &CB,
332+
const TargetLibraryInfo &TLI) {
333+
if (ignoreInstrumentableLibFunc(Func))
334+
return false;
335+
336+
if (isAllocationFn(&CB, &TLI))
323337
return true;
324338

325339
switch (Func) {
340+
// These libfuncs don't return normal pointers, and are therefore not handled
341+
// by isAllocationFn().
326342
case LibFunc_posix_memalign:
327343
case LibFunc_size_returning_new:
328344
case LibFunc_size_returning_new_hot_cold:
329345
case LibFunc_size_returning_new_aligned:
330346
case LibFunc_size_returning_new_aligned_hot_cold:
331347
return true;
348+
349+
// See comment above ClCoverReplaceableNew.
332350
case LibFunc_Znwj:
333351
case LibFunc_ZnwjRKSt9nothrow_t:
334352
case LibFunc_ZnwjSt11align_val_t:
@@ -354,6 +372,7 @@ bool AllocToken::isInstrumentableLibFunc(LibFunc Func, const Value *V,
354372
case LibFunc_ZnamSt11align_val_tRKSt9nothrow_t:
355373
case LibFunc_ZnamSt11align_val_tRKSt9nothrow_t12__hot_cold_t:
356374
return ClCoverReplaceableNew;
375+
357376
default:
358377
return false;
359378
}
@@ -390,7 +409,7 @@ bool AllocToken::replaceAllocationCall(CallBase *CB, LibFunc Func,
390409
IRBuilder<> IRB(CB);
391410
// Original args.
392411
SmallVector<Value *, 4> NewArgs{CB->args()};
393-
// Add token ID.
412+
// Add token ID, truncated to IntPtrTy width.
394413
NewArgs.push_back(ConstantInt::get(IntPtrTy, TokenID));
395414
assert(TokenAlloc.getFunctionType()->getNumParams() == NewArgs.size());
396415

@@ -420,7 +439,7 @@ FunctionCallee AllocToken::getTokenAllocFunction(const CallBase &CB,
420439
if (OriginalFunc != NotLibFunc) {
421440
Key = std::make_pair(OriginalFunc, Options.FastABI ? TokenID : 0);
422441
auto It = TokenAllocFunctions.find(*Key);
423-
if (LLVM_LIKELY(It != TokenAllocFunctions.end()))
442+
if (It != TokenAllocFunctions.end())
424443
return It->second;
425444
}
426445

@@ -459,7 +478,7 @@ PreservedAnalyses AllocTokenPass::run(Module &M, ModuleAnalysisManager &MAM) {
459478
bool Modified = false;
460479

461480
for (Function &F : M) {
462-
if (LLVM_LIKELY(F.empty()))
481+
if (F.empty())
463482
continue; // declaration
464483
Modified |= Pass.instrumentFunction(F);
465484
}

llvm/lib/Transforms/Utils/Local.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3026,8 +3026,11 @@ static void combineMetadata(Instruction *K, const Instruction *J,
30263026
K->setMetadata(Kind, JMD);
30273027
break;
30283028
case LLVMContext::MD_alloc_token:
3029-
// Preserve !alloc_token if both K and J have it.
3030-
K->setMetadata(Kind, JMD);
3029+
// Preserve !alloc_token if both K and J have it, and they are equal.
3030+
if (KMD == JMD)
3031+
K->setMetadata(Kind, JMD);
3032+
else
3033+
K->setMetadata(Kind, nullptr);
30313034
break;
30323035
}
30333036
}

llvm/test/Instrumentation/AllocToken/basic.ll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
; RUN: opt < %s -passes=inferattrs,alloc-token -alloc-token-mode=increment -S | FileCheck %s
33

44
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
5-
target triple = "x86_64-unknown-linux-gnu"
65

76
declare ptr @malloc(i64)
87
declare ptr @calloc(i64, i64)

llvm/test/Instrumentation/AllocToken/basic32.ll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
; RUN: opt < %s -passes=inferattrs,alloc-token -alloc-token-mode=increment -S | FileCheck %s
33

44
target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32-S128"
5-
target triple = "i386-pc-linux-gnu"
65

76
declare ptr @malloc(i32)
87
declare ptr @_Znwm(i32)

llvm/test/Instrumentation/AllocToken/extralibfuncs.ll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
; RUN: opt < %s -passes=inferattrs,alloc-token -S | FileCheck %s
55

66
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
7-
target triple = "x86_64-unknown-linux-gnu"
87

98
declare {ptr, i64} @__size_returning_new(i64)
109

llvm/test/Instrumentation/AllocToken/fast.ll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
; RUN: opt < %s -passes=inferattrs,alloc-token -alloc-token-mode=increment -alloc-token-fast-abi -alloc-token-max=3 -S | FileCheck %s
33

44
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
5-
target triple = "x86_64-unknown-linux-gnu"
65

76
declare ptr @malloc(i64)
87
declare ptr @calloc(i64, i64)

llvm/test/Instrumentation/AllocToken/ignore.ll

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@
44
; RUN: opt < %s -passes=inferattrs,alloc-token -S | FileCheck %s
55

66
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
7-
target triple = "x86_64-unknown-linux-gnu"
87

98
declare ptr @strdup(ptr)
109
declare ptr @__strdup(ptr)
1110
declare ptr @strndup(ptr, i64)
1211
declare ptr @__strndup(ptr, i64)
1312

14-
define ptr @test_ignorable_allocation_functions(ptr %ptr) sanitize_alloc_token {
15-
; CHECK-LABEL: define ptr @test_ignorable_allocation_functions(
13+
define ptr @test_ignored_allocation_functions(ptr %ptr) sanitize_alloc_token {
14+
; CHECK-LABEL: define ptr @test_ignored_allocation_functions(
1615
; CHECK-SAME: ptr [[PTR:%.*]]) #[[ATTR2:[0-9]+]] {
1716
; CHECK-NEXT: [[ENTRY:.*:]]
1817
; CHECK-NEXT: [[PTR1:%.*]] = call ptr @strdup(ptr [[PTR]])

llvm/test/Instrumentation/AllocToken/invoke.ll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
; RUN: opt < %s -passes=inferattrs,alloc-token -alloc-token-mode=increment -S | FileCheck %s
33

44
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
5-
target triple = "x86_64-unknown-linux-gnu"
65

76
define ptr @test_invoke_malloc() sanitize_alloc_token personality ptr @__gxx_personality_v0 {
87
; CHECK-LABEL: define ptr @test_invoke_malloc(

0 commit comments

Comments
 (0)