fixup! address reviewer comments round 2

Created using spr 1.3.8-beta.1

Author: melver

llvm/lib/Transforms/Instrumentation/AllocToken.cpp

@@ -113,13 +113,6 @@ cl::opt<bool> ClCoverReplaceableNew("alloc-token-cover-replaceable-new",
                                     cl::desc("Cover replaceable operator new"),
                                     cl::Hidden, cl::init(true));
 
-// strdup-family functions only operate on strings, covering them does not make
-// sense in most cases.
-cl::opt<bool>
-    ClCoverStrdup("alloc-token-cover-strdup",
-                  cl::desc("Cover strdup-family allocation functions"),
-                  cl::Hidden, cl::init(false));
-
 cl::opt<uint64_t> ClFallbackToken(
     "alloc-token-fallback",
     cl::desc("The default fallback token where none could be determined"),
@@ -128,7 +121,7 @@ cl::opt<uint64_t> ClFallbackToken(
 //===--- Statistics -------------------------------------------------------===//
 
 STATISTIC(NumFunctionsInstrumented, "Functions instrumented");
-STATISTIC(NumAllocations, "Allocations found");
+STATISTIC(NumAllocationsInstrumented, "Allocations instrumented");
 
 //===----------------------------------------------------------------------===//
 
@@ -194,12 +187,13 @@ class TypeHashMode : public ModeBase {
       MDString *S = cast<MDString>(N->getOperand(0));
       return boundedToken(xxHash64(S->getString()));
     }
-    remarkNoHint(CB, ORE);
+    remarkNoMetadata(CB, ORE);
     return ClFallbackToken;
   }
 
   /// Remark that there was no precise type information.
-  void remarkNoHint(const CallBase &CB, OptimizationRemarkEmitter &ORE) {
+  static void remarkNoMetadata(const CallBase &CB,
+                               OptimizationRemarkEmitter &ORE) {
     ORE.emit([&] {
       ore::NV FuncNV("Function", CB.getParent()->getParent());
       const Function *Callee = CB.getCalledFunction();
@@ -212,12 +206,12 @@ class TypeHashMode : public ModeBase {
 };
 
 // Apply opt overrides.
-AllocTokenOptions &&transformOptionsFromCl(AllocTokenOptions &&Opts) {
+AllocTokenOptions transformOptionsFromCl(AllocTokenOptions Opts) {
   if (!Opts.MaxTokens.has_value())
     Opts.MaxTokens = ClMaxTokens;
   Opts.FastABI |= ClFastABI;
   Opts.Extended |= ClExtended;
-  return std::move(Opts);
+  return Opts;
 }
 
 class AllocToken {
@@ -252,7 +246,7 @@ class AllocToken {
 
   /// Replace a call/invoke with a call/invoke to the allocation function
   /// with token ID.
-  void replaceAllocationCall(CallBase *CB, LibFunc Func,
+  bool replaceAllocationCall(CallBase *CB, LibFunc Func,
                              OptimizationRemarkEmitter &ORE,
                              const TargetLibraryInfo &TLI);
 
@@ -314,15 +308,13 @@ bool AllocToken::instrumentFunction(Function &F) {
     }
   }
 
-  if (AllocCalls.empty())
-    return false;
-
+  bool Modified = false;
   for (auto &[CB, Func] : AllocCalls)
-    replaceAllocationCall(CB, Func, ORE, TLI);
-  NumAllocations += AllocCalls.size();
-  NumFunctionsInstrumented++;
+    Modified |= replaceAllocationCall(CB, Func, ORE, TLI);
 
-  return true;
+  if (Modified)
+    NumFunctionsInstrumented++;
+  return Modified;
 }
 
 bool AllocToken::isInstrumentableLibFunc(LibFunc Func, const Value *V,
@@ -373,24 +365,26 @@ bool AllocToken::ignoreInstrumentableLibFunc(LibFunc Func) {
   case LibFunc_dunder_strdup:
   case LibFunc_strndup:
   case LibFunc_dunder_strndup:
-    return !ClCoverStrdup;
+    return true;
   default:
     return false;
   }
 }
 
-void AllocToken::replaceAllocationCall(CallBase *CB, LibFunc Func,
+bool AllocToken::replaceAllocationCall(CallBase *CB, LibFunc Func,
                                        OptimizationRemarkEmitter &ORE,
                                        const TargetLibraryInfo &TLI) {
   uint64_t TokenID = getToken(*CB, ORE);
 
   FunctionCallee TokenAlloc = getTokenAllocFunction(*CB, TokenID, Func);
   if (!TokenAlloc)
-    return;
+    return false;
+  NumAllocationsInstrumented++;
+
   if (Options.FastABI) {
     assert(TokenAlloc.getFunctionType()->getNumParams() == CB->arg_size());
     CB->setCalledFunction(TokenAlloc);
-    return;
+    return true;
   }
 
   IRBuilder<> IRB(CB);
@@ -416,6 +410,7 @@ void AllocToken::replaceAllocationCall(CallBase *CB, LibFunc Func,
   // Replace all uses and delete the old call.
   CB->replaceAllUsesWith(NewCall);
   CB->eraseFromParent();
+  return true;
 }
 
 FunctionCallee AllocToken::getTokenAllocFunction(const CallBase &CB,

llvm/test/Instrumentation/AllocToken/ignore.ll

@@ -1,7 +1,6 @@
 ; Test for all allocation functions that should be ignored by default.
 ;
-; RUN: opt < %s -passes=inferattrs,alloc-token -S | FileCheck %s --check-prefixes=CHECK,DEFAULT
-; RUN: opt < %s -passes=inferattrs,alloc-token -alloc-token-cover-strdup -S | FileCheck %s --check-prefixes=CHECK,COVER
+; RUN: opt < %s -passes=inferattrs,alloc-token -S | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -14,17 +13,13 @@ declare ptr @__strndup(ptr, i64)
 ; CHECK-LABEL: @test_ignorable_allocation_functions
 define ptr @test_ignorable_allocation_functions(ptr %ptr) sanitize_alloc_token {
 entry:
-  ; COVER:   call ptr @__alloc_token_strdup(
-  ; DEFAULT: call ptr @strdup(
+  ; CHECK: call ptr @strdup(
   %ptr1 = call ptr @strdup(ptr %ptr)
-  ; COVER:   call ptr @__alloc_token_strdup(
-  ; DEFAULT: call ptr @__strdup(
+  ; CHECK: call ptr @__strdup(
   %ptr2 = call ptr @__strdup(ptr %ptr)
-  ; COVER:   call ptr @__alloc_token_strndup(
-  ; DEFAULT: call ptr @strndup(
+  ; CHECK: call ptr @strndup(
   %ptr3 = call ptr @strndup(ptr %ptr, i64 42)
-  ; COVER:   call ptr @__alloc_token_strndup(
-  ; DEFAULT: call ptr @__strndup(
+  ; CHECK: call ptr @__strndup(
   %ptr4 = call ptr @__strndup(ptr %ptr, i64 42)
   ret ptr %ptr1
 }